From 500dd360773c810ce35023c579bd433bd7771b72 Mon Sep 17 00:00:00 2001 From: travisn Date: Mon, 4 Dec 2023 17:20:00 -0700 Subject: [PATCH] core: fix golang linter issues with variables in loops Loop variables cannot be reliably uses since they will change with each iteration. Update these loop variable uses to be safe by indexing the slice rather than using the loop variable directly. Also suppress the linter issues for passwords used in tests. Signed-off-by: travisn (cherry picked from commit 6f8e42422d8b031d3b95412088e0e77d3e7218e7) --- .github/workflows/golangci-lint.yaml | 4 ++-- pkg/apis/ceph.rook.io/v1/status_test.go | 10 +++++----- pkg/daemon/multus/resources.go | 4 ++-- pkg/operator/ceph/csi/controller.go | 2 +- pkg/operator/ceph/csi/spec.go | 4 ++-- pkg/operator/ceph/object/controller_test.go | 1 + pkg/operator/ceph/object/objectstore_test.go | 1 + pkg/operator/ceph/object/topic/controller_test.go | 7 ++++--- pkg/operator/ceph/object/user/controller_test.go | 1 + pkg/operator/k8sutil/pvc_test.go | 3 ++- 10 files changed, 21 insertions(+), 16 deletions(-) diff --git a/.github/workflows/golangci-lint.yaml b/.github/workflows/golangci-lint.yaml index 45943f80a..5a78cbed3 100644 --- a/.github/workflows/golangci-lint.yaml +++ b/.github/workflows/golangci-lint.yaml @@ -29,12 +29,12 @@ jobs: fetch-depth: 0 - uses: actions/setup-go@v4 with: - go-version: "1.21" + go-version: "1.20" - name: golangci-lint uses: golangci/golangci-lint-action@v3 with: # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version. - version: v1.53 + version: v1.55 # Optional: working directory, useful for monorepos # working-directory: somedir diff --git a/pkg/apis/ceph.rook.io/v1/status_test.go b/pkg/apis/ceph.rook.io/v1/status_test.go index cdf9a6223..935c23e16 100644 --- a/pkg/apis/ceph.rook.io/v1/status_test.go +++ b/pkg/apis/ceph.rook.io/v1/status_test.go @@ -88,11 +88,11 @@ func TestSetStatusCondition(t *testing.T) { }, } - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - SetStatusCondition(&test.conditions, test.toAdd) - if !reflect.DeepEqual(test.conditions, test.expected) { - t.Error(test.conditions) + for i := range tests { + t.Run(tests[i].name, func(t *testing.T) { + SetStatusCondition(&tests[i].conditions, tests[i].toAdd) + if !reflect.DeepEqual(tests[i].conditions, tests[i].expected) { + t.Error(tests[i].conditions) } }) } diff --git a/pkg/daemon/multus/resources.go b/pkg/daemon/multus/resources.go index e0aa1478b..a058e4862 100644 --- a/pkg/daemon/multus/resources.go +++ b/pkg/daemon/multus/resources.go @@ -279,8 +279,8 @@ func (vt *ValidationTest) getImagePullPodCountPerNodeType( } numsScheduled := perNodeTypeCount{} - for _, d := range dsets.Items { - nodeType := getNodeType(&d.ObjectMeta) + for i, d := range dsets.Items { + nodeType := getNodeType(&dsets.Items[i].ObjectMeta) numScheduled := d.Status.CurrentNumberScheduled if numScheduled == 0 { return emptyCount, fmt.Errorf("image pull daemonset for node type %q expects zero scheduled pods", nodeType) diff --git a/pkg/operator/ceph/csi/controller.go b/pkg/operator/ceph/csi/controller.go index 64d9a3bb5..e380d9ea4 100644 --- a/pkg/operator/ceph/csi/controller.go +++ b/pkg/operator/ceph/csi/controller.go @@ -208,7 +208,7 @@ func (r *ReconcileCSI) reconcile(request reconcile.Request) (reconcile.Result, e // If so deploy the plugin holder with the fsid attached if holderEnabled { // Load cluster info for later use in updating the ceph-csi configmap - clusterInfo, _, _, err := opcontroller.LoadClusterInfo(r.context, r.opManagerContext, cluster.Namespace, &cluster.Spec) + clusterInfo, _, _, err := opcontroller.LoadClusterInfo(r.context, r.opManagerContext, cluster.Namespace, &cephClusters.Items[i].Spec) if err != nil { // This avoids a requeue with exponential backoff and allows the controller to reconcile // more quickly when the cluster is ready. diff --git a/pkg/operator/ceph/csi/spec.go b/pkg/operator/ceph/csi/spec.go index d63a36658..1ce4e12cd 100644 --- a/pkg/operator/ceph/csi/spec.go +++ b/pkg/operator/ceph/csi/spec.go @@ -713,9 +713,9 @@ func (r *ReconcileCSI) applyCephClusterNetworkConfig(ctx context.Context, object if err != nil { return errors.Wrap(err, "failed to find CephClusters") } - for _, cephCluster := range cephClusters.Items { + for i, cephCluster := range cephClusters.Items { if cephCluster.Spec.Network.IsMultus() { - err = k8sutil.ApplyMultus(cephCluster.GetNamespace(), &cephCluster.Spec.Network, objectMeta) + err = k8sutil.ApplyMultus(cephCluster.GetNamespace(), &cephClusters.Items[i].Spec.Network, objectMeta) if err != nil { return errors.Wrapf(err, "failed to apply multus configuration to CephCluster %q", cephCluster.Name) } diff --git a/pkg/operator/ceph/object/controller_test.go b/pkg/operator/ceph/object/controller_test.go index efdd30353..96f10bd29 100644 --- a/pkg/operator/ceph/object/controller_test.go +++ b/pkg/operator/ceph/object/controller_test.go @@ -152,6 +152,7 @@ const ( "ceph version 17.2.1 (0000000000000000) quincy (stable)": 3 } }` + //nolint:gosec // only test values, not a real secret userCreateJSON = `{ "user_id": "my-user", "display_name": "my-user", diff --git a/pkg/operator/ceph/object/objectstore_test.go b/pkg/operator/ceph/object/objectstore_test.go index 393686431..c9c078154 100644 --- a/pkg/operator/ceph/object/objectstore_test.go +++ b/pkg/operator/ceph/object/objectstore_test.go @@ -38,6 +38,7 @@ import ( ) const ( + //nolint:gosec // only test values, not a real secret dashboardAdminCreateJSON = `{ "user_id": "dashboard-admin", "display_name": "dashboard-admin", diff --git a/pkg/operator/ceph/object/topic/controller_test.go b/pkg/operator/ceph/object/topic/controller_test.go index 67cbdfa27..b0dcb1c5b 100644 --- a/pkg/operator/ceph/object/topic/controller_test.go +++ b/pkg/operator/ceph/object/topic/controller_test.go @@ -43,9 +43,10 @@ import ( ) var ( - name = "topic-a" - namespace = "rook-ceph" - store = "test-store" + name = "topic-a" + namespace = "rook-ceph" + store = "test-store" + //nolint:gosec // only test values, not a real secret userCreateJSON = `{ "user_id": "rgw-admin-ops-user", "display_name": "RGW Admin Ops User", diff --git a/pkg/operator/ceph/object/user/controller_test.go b/pkg/operator/ceph/object/user/controller_test.go index 4094b1a71..44001cc2b 100644 --- a/pkg/operator/ceph/object/user/controller_test.go +++ b/pkg/operator/ceph/object/user/controller_test.go @@ -49,6 +49,7 @@ import ( ) const ( + //nolint:gosec // only test values, not a real secret userCreateJSON = `{ "user_id": "my-user", "display_name": "my-user", diff --git a/pkg/operator/k8sutil/pvc_test.go b/pkg/operator/k8sutil/pvc_test.go index 5f7c98b32..356b5324b 100644 --- a/pkg/operator/k8sutil/pvc_test.go +++ b/pkg/operator/k8sutil/pvc_test.go @@ -96,7 +96,8 @@ func TestExpandPVCIfRequired(t *testing.T) { for _, tc := range testcases { desiredPVC.Spec.Resources.Requests[v1.ResourceStorage] = apiresource.MustParse(tc.currentPVCSize) - storageClass.AllowVolumeExpansion = &tc.expansionAllowed + expansionAllowed := tc.expansionAllowed + storageClass.AllowVolumeExpansion = &expansionAllowed // create fake client with PVC cl := fake.NewClientBuilder().WithRuntimeObjects(desiredPVC, storageClass).Build()