From 2732ab956c48c74b2571dc45010a2b8c832bd4a1 Mon Sep 17 00:00:00 2001 From: sivchari Date: Tue, 17 Sep 2024 16:00:45 +0900 Subject: [PATCH 1/8] upgrade golangci-lint to v1.61.0 Signed-off-by: sivchari --- .github/workflows/golangci-lint.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml index 578b58086c..3f16cb7d6c 100644 --- a/.github/workflows/golangci-lint.yml +++ b/.github/workflows/golangci-lint.yml @@ -34,6 +34,6 @@ jobs: - name: golangci-lint uses: golangci/golangci-lint-action@aaa42aa0628b4ae2578232a66b541047968fac86 # tag=v6.1.0 with: - version: v1.57.2 + version: v1.61.0 args: --out-format=colored-line-number working-directory: ${{matrix.working-directory}} From cc145e20f16037bbc0450c2a3711a822ab621d10 Mon Sep 17 00:00:00 2001 From: sivchari Date: Tue, 17 Sep 2024 16:03:18 +0900 Subject: [PATCH 2/8] apply nolintlint Signed-off-by: sivchari --- pkg/cache/cache_test.go | 6 +++--- tools/setup-envtest/store/store.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/cache/cache_test.go b/pkg/cache/cache_test.go index 7a21c87c37..c915fc7cdf 100644 --- a/pkg/cache/cache_test.go +++ b/pkg/cache/cache_test.go @@ -1544,7 +1544,7 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca return obtainedPodNames }, ConsistOf(tc.expectedPods))) for _, pod := range obtainedStructuredPodList.Items { - Expect(informer.Get(context.Background(), client.ObjectKeyFromObject(&pod), &pod)).To(Succeed()) //nolint:gosec // We don't retain the pointer + Expect(informer.Get(context.Background(), client.ObjectKeyFromObject(&pod), &pod)).To(Succeed()) // We don't retain the pointer } By("Checking with unstructured") @@ -1564,7 +1564,7 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca return obtainedPodNames }, ConsistOf(tc.expectedPods))) for _, pod := range obtainedUnstructuredPodList.Items { - Expect(informer.Get(context.Background(), client.ObjectKeyFromObject(&pod), &pod)).To(Succeed()) //nolint:gosec // We don't retain the pointer + Expect(informer.Get(context.Background(), client.ObjectKeyFromObject(&pod), &pod)).To(Succeed()) // We don't retain the pointer } By("Checking with metadata") @@ -1584,7 +1584,7 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca return obtainedPodNames }, ConsistOf(tc.expectedPods))) for _, pod := range obtainedMetadataPodList.Items { - Expect(informer.Get(context.Background(), client.ObjectKeyFromObject(&pod), &pod)).To(Succeed()) //nolint:gosec // We don't retain the pointer + Expect(informer.Get(context.Background(), client.ObjectKeyFromObject(&pod), &pod)).To(Succeed()) // We don't retain the pointer } }, Entry("when selectors are empty it has to inform about all the pods", selectorsTestCase{ diff --git a/tools/setup-envtest/store/store.go b/tools/setup-envtest/store/store.go index 2ee0b64dec..c551d221bf 100644 --- a/tools/setup-envtest/store/store.go +++ b/tools/setup-envtest/store/store.go @@ -167,7 +167,7 @@ func (s *Store) Add(ctx context.Context, item Item, contents io.Reader) (resErr // preferfing our own scheme. targetPath := filepath.Base(header.Name) log.V(1).Info("writing archive file to disk", "archive file", header.Name, "on-disk file", targetPath) - perms := 0555 & header.Mode // make sure we're at most r+x + perms := 0555 & uint32(header.Mode) // make sure we're at most r+x binOut, err := itemPath.OpenFile(targetPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, os.FileMode(perms)) if err != nil { return fmt.Errorf("unable to create file %s from archive to disk for version-platform pair %s", targetPath, itemName) From e04c2f05e131efefe0f5f1e15e782e79cab270ac Mon Sep 17 00:00:00 2001 From: sivchari Date: Tue, 17 Sep 2024 16:25:15 +0900 Subject: [PATCH 3/8] disable gosec G115 Signed-off-by: sivchari --- .golangci.yml | 3 +++ tools/setup-envtest/store/store.go | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/.golangci.yml b/.golangci.yml index 4c43665e2b..b2e1b675cc 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -162,6 +162,9 @@ issues: - linters: - gosec text: "G304: Potential file inclusion via variable" + - linters: + - gosec + text: "G115: integer overflow conversion" - linters: - dupl path: _test\.go diff --git a/tools/setup-envtest/store/store.go b/tools/setup-envtest/store/store.go index c551d221bf..2ee0b64dec 100644 --- a/tools/setup-envtest/store/store.go +++ b/tools/setup-envtest/store/store.go @@ -167,7 +167,7 @@ func (s *Store) Add(ctx context.Context, item Item, contents io.Reader) (resErr // preferfing our own scheme. targetPath := filepath.Base(header.Name) log.V(1).Info("writing archive file to disk", "archive file", header.Name, "on-disk file", targetPath) - perms := 0555 & uint32(header.Mode) // make sure we're at most r+x + perms := 0555 & header.Mode // make sure we're at most r+x binOut, err := itemPath.OpenFile(targetPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, os.FileMode(perms)) if err != nil { return fmt.Errorf("unable to create file %s from archive to disk for version-platform pair %s", targetPath, itemName) From 6dcfe17629f7ca1f44545a064ce062f7aa75028c Mon Sep 17 00:00:00 2001 From: sivchari Date: Wed, 2 Oct 2024 11:18:45 +0900 Subject: [PATCH 4/8] revert configuration Signed-off-by: sivchari --- .golangci.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index b2e1b675cc..4c43665e2b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -162,9 +162,6 @@ issues: - linters: - gosec text: "G304: Potential file inclusion via variable" - - linters: - - gosec - text: "G115: integer overflow conversion" - linters: - dupl path: _test\.go From 96513c1ba22b94b001af82039cc9ecec337cc0b4 Mon Sep 17 00:00:00 2001 From: sivchari Date: Wed, 2 Oct 2024 11:18:50 +0900 Subject: [PATCH 5/8] clean up comments Signed-off-by: sivchari --- pkg/cache/cache_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/cache/cache_test.go b/pkg/cache/cache_test.go index c915fc7cdf..f6b7b03c47 100644 --- a/pkg/cache/cache_test.go +++ b/pkg/cache/cache_test.go @@ -1544,7 +1544,7 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca return obtainedPodNames }, ConsistOf(tc.expectedPods))) for _, pod := range obtainedStructuredPodList.Items { - Expect(informer.Get(context.Background(), client.ObjectKeyFromObject(&pod), &pod)).To(Succeed()) // We don't retain the pointer + Expect(informer.Get(context.Background(), client.ObjectKeyFromObject(&pod), &pod)).To(Succeed()) } By("Checking with unstructured") @@ -1564,7 +1564,7 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca return obtainedPodNames }, ConsistOf(tc.expectedPods))) for _, pod := range obtainedUnstructuredPodList.Items { - Expect(informer.Get(context.Background(), client.ObjectKeyFromObject(&pod), &pod)).To(Succeed()) // We don't retain the pointer + Expect(informer.Get(context.Background(), client.ObjectKeyFromObject(&pod), &pod)).To(Succeed()) } By("Checking with metadata") @@ -1584,7 +1584,7 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca return obtainedPodNames }, ConsistOf(tc.expectedPods))) for _, pod := range obtainedMetadataPodList.Items { - Expect(informer.Get(context.Background(), client.ObjectKeyFromObject(&pod), &pod)).To(Succeed()) // We don't retain the pointer + Expect(informer.Get(context.Background(), client.ObjectKeyFromObject(&pod), &pod)).To(Succeed()) } }, Entry("when selectors are empty it has to inform about all the pods", selectorsTestCase{ From 9d07df73d6360dd048619cb164e92d770ae4173e Mon Sep 17 00:00:00 2001 From: sivchari Date: Wed, 2 Oct 2024 11:29:03 +0900 Subject: [PATCH 6/8] add nolint:gosec about G115 rule Signed-off-by: sivchari --- pkg/log/zap/flags.go | 2 +- pkg/webhook/admission/response.go | 2 +- tools/setup-envtest/store/store.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/log/zap/flags.go b/pkg/log/zap/flags.go index fb492b14da..555f91f749 100644 --- a/pkg/log/zap/flags.go +++ b/pkg/log/zap/flags.go @@ -85,7 +85,7 @@ func (ev *levelFlag) Set(flagValue string) error { } if logLevel > 0 { intLevel := -1 * logLevel - ev.setFunc(zap.NewAtomicLevelAt(zapcore.Level(int8(intLevel)))) + ev.setFunc(zap.NewAtomicLevelAt(zapcore.Level(int8(intLevel)))) //nolint:gosec // We don't care about G115. } else { return fmt.Errorf("invalid log level \"%s\"", flagValue) } diff --git a/pkg/webhook/admission/response.go b/pkg/webhook/admission/response.go index ec1c88c989..3cc3e4906b 100644 --- a/pkg/webhook/admission/response.go +++ b/pkg/webhook/admission/response.go @@ -71,7 +71,7 @@ func ValidationResponse(allowed bool, message string) Response { AdmissionResponse: admissionv1.AdmissionResponse{ Allowed: allowed, Result: &metav1.Status{ - Code: int32(code), + Code: int32(code), //nolint:gosec // We don't care about G115. Reason: reason, }, }, diff --git a/tools/setup-envtest/store/store.go b/tools/setup-envtest/store/store.go index 2ee0b64dec..e86f0b3f23 100644 --- a/tools/setup-envtest/store/store.go +++ b/tools/setup-envtest/store/store.go @@ -168,7 +168,7 @@ func (s *Store) Add(ctx context.Context, item Item, contents io.Reader) (resErr targetPath := filepath.Base(header.Name) log.V(1).Info("writing archive file to disk", "archive file", header.Name, "on-disk file", targetPath) perms := 0555 & header.Mode // make sure we're at most r+x - binOut, err := itemPath.OpenFile(targetPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, os.FileMode(perms)) + binOut, err := itemPath.OpenFile(targetPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, os.FileMode(perms)) //nolint:gosec // We don't care about G115. if err != nil { return fmt.Errorf("unable to create file %s from archive to disk for version-platform pair %s", targetPath, itemName) } From fff8ec81ebabdd70aa6114ff9f56ad5670250e97 Mon Sep 17 00:00:00 2001 From: sivchari Date: Wed, 2 Oct 2024 11:42:42 +0900 Subject: [PATCH 7/8] fix: gofmt Signed-off-by: sivchari --- tools/setup-envtest/store/store.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/setup-envtest/store/store.go b/tools/setup-envtest/store/store.go index e86f0b3f23..5405e66477 100644 --- a/tools/setup-envtest/store/store.go +++ b/tools/setup-envtest/store/store.go @@ -167,7 +167,7 @@ func (s *Store) Add(ctx context.Context, item Item, contents io.Reader) (resErr // preferfing our own scheme. targetPath := filepath.Base(header.Name) log.V(1).Info("writing archive file to disk", "archive file", header.Name, "on-disk file", targetPath) - perms := 0555 & header.Mode // make sure we're at most r+x + perms := 0555 & header.Mode // make sure we're at most r+x binOut, err := itemPath.OpenFile(targetPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, os.FileMode(perms)) //nolint:gosec // We don't care about G115. if err != nil { return fmt.Errorf("unable to create file %s from archive to disk for version-platform pair %s", targetPath, itemName) From e60ba99c1645ed3c8423d4679833ad7d514a9d49 Mon Sep 17 00:00:00 2001 From: Stefan Bueringer Date: Fri, 11 Oct 2024 15:48:25 +0200 Subject: [PATCH 8/8] fix review findings --- .golangci.yml | 2 +- pkg/client/client_test.go | 1 - pkg/envtest/crd.go | 2 -- pkg/envtest/envtest_test.go | 1 - pkg/envtest/webhook.go | 2 -- pkg/log/zap/flags.go | 2 +- pkg/webhook/admission/response.go | 2 +- tools/setup-envtest/store/store.go | 2 +- 8 files changed, 4 insertions(+), 10 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 4c43665e2b..6ff1142911 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -5,13 +5,13 @@ linters: - asciicheck - bidichk - bodyclose + - copyloopvar - dogsled - dupl - errcheck - errchkjson - errorlint - exhaustive - - exportloopref - ginkgolinter - goconst - gocritic diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 59ddf13664..42a04c5b06 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -1979,7 +1979,6 @@ U5wwSivyi7vmegHKmblOzNVKA5qPO8zWzqBC // Test this with an integrated type and a CRD to make sure it covers both proto // and json deserialization. for idx, object := range []client.Object{&corev1.ConfigMap{}, &pkg.ChaosPod{}} { - idx, object := idx, object It(fmt.Sprintf("should not retain any data in the obj variable that is not on the server for %T", object), func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) diff --git a/pkg/envtest/crd.go b/pkg/envtest/crd.go index 5fdd657cd7..49f6b149be 100644 --- a/pkg/envtest/crd.go +++ b/pkg/envtest/crd.go @@ -229,7 +229,6 @@ func UninstallCRDs(config *rest.Config, options CRDInstallOptions) error { // Uninstall each CRD for _, crd := range options.CRDs { - crd := crd log.V(1).Info("uninstalling CRD", "crd", crd.GetName()) if err := cs.Delete(context.TODO(), crd); err != nil { // If CRD is not found, we can consider success @@ -251,7 +250,6 @@ func CreateCRDs(config *rest.Config, crds []*apiextensionsv1.CustomResourceDefin // Create each CRD for _, crd := range crds { - crd := crd log.V(1).Info("installing CRD", "crd", crd.GetName()) existingCrd := crd.DeepCopy() err := cs.Get(context.TODO(), client.ObjectKey{Name: crd.GetName()}, existingCrd) diff --git a/pkg/envtest/envtest_test.go b/pkg/envtest/envtest_test.go index 21464e10be..7214697e9d 100644 --- a/pkg/envtest/envtest_test.go +++ b/pkg/envtest/envtest_test.go @@ -57,7 +57,6 @@ var _ = Describe("Test", func() { // Cleanup CRDs AfterEach(func() { for _, crd := range crds { - crd := crd // Delete only if CRD exists. crdObjectKey := client.ObjectKey{ Name: crd.GetName(), diff --git a/pkg/envtest/webhook.go b/pkg/envtest/webhook.go index e4e54e472c..51bcb4311e 100644 --- a/pkg/envtest/webhook.go +++ b/pkg/envtest/webhook.go @@ -313,14 +313,12 @@ func createWebhooks(config *rest.Config, mutHooks []*admissionv1.MutatingWebhook // Create each webhook for _, hook := range mutHooks { - hook := hook log.V(1).Info("installing mutating webhook", "webhook", hook.GetName()) if err := ensureCreated(cs, hook); err != nil { return err } } for _, hook := range valHooks { - hook := hook log.V(1).Info("installing validating webhook", "webhook", hook.GetName()) if err := ensureCreated(cs, hook); err != nil { return err diff --git a/pkg/log/zap/flags.go b/pkg/log/zap/flags.go index 555f91f749..c69254b0b4 100644 --- a/pkg/log/zap/flags.go +++ b/pkg/log/zap/flags.go @@ -85,7 +85,7 @@ func (ev *levelFlag) Set(flagValue string) error { } if logLevel > 0 { intLevel := -1 * logLevel - ev.setFunc(zap.NewAtomicLevelAt(zapcore.Level(int8(intLevel)))) //nolint:gosec // We don't care about G115. + ev.setFunc(zap.NewAtomicLevelAt(zapcore.Level(int8(intLevel)))) //nolint:gosec // We are not worried about integer overflows (G115) here. } else { return fmt.Errorf("invalid log level \"%s\"", flagValue) } diff --git a/pkg/webhook/admission/response.go b/pkg/webhook/admission/response.go index 3cc3e4906b..c503a971e1 100644 --- a/pkg/webhook/admission/response.go +++ b/pkg/webhook/admission/response.go @@ -71,7 +71,7 @@ func ValidationResponse(allowed bool, message string) Response { AdmissionResponse: admissionv1.AdmissionResponse{ Allowed: allowed, Result: &metav1.Status{ - Code: int32(code), //nolint:gosec // We don't care about G115. + Code: int32(code), //nolint:gosec // Integer overflows (G115) cannot occur here. Reason: reason, }, }, diff --git a/tools/setup-envtest/store/store.go b/tools/setup-envtest/store/store.go index 5405e66477..0097ab9c64 100644 --- a/tools/setup-envtest/store/store.go +++ b/tools/setup-envtest/store/store.go @@ -168,7 +168,7 @@ func (s *Store) Add(ctx context.Context, item Item, contents io.Reader) (resErr targetPath := filepath.Base(header.Name) log.V(1).Info("writing archive file to disk", "archive file", header.Name, "on-disk file", targetPath) perms := 0555 & header.Mode // make sure we're at most r+x - binOut, err := itemPath.OpenFile(targetPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, os.FileMode(perms)) //nolint:gosec // We don't care about G115. + binOut, err := itemPath.OpenFile(targetPath, os.O_RDWR|os.O_CREATE|os.O_TRUNC, os.FileMode(perms)) //nolint:gosec // Integer overflows (G115) seem unlikely here. if err != nil { return fmt.Errorf("unable to create file %s from archive to disk for version-platform pair %s", targetPath, itemName) }