From df511fb58f79a55ae38813b71efe3a8eb3c464bd Mon Sep 17 00:00:00 2001 From: Michael Adam Date: Tue, 10 Dec 2024 20:35:53 +0100 Subject: [PATCH] ci: update golangci-lint to the latest version (v1.62) The ci was using a pretty old version og golangci-lint. This updates to the latest version. Additionally, it silences some gosec integer conversion overflow false positves and fixes some real errors of this category and string format errors found by golangci-lint, while at it. Co-authored-by: Blaine Gardner Co-authored-by: Travis Nielsen Signed-off-by: Michael Adam --- .github/workflows/golangci-lint.yaml | 2 +- pkg/daemon/ceph/client/crush_rule.go | 5 +++-- pkg/daemon/ceph/client/image.go | 4 ++-- pkg/daemon/ceph/client/pool_test.go | 3 ++- pkg/daemon/ceph/osd/kms/kmip.go | 11 +++++++++-- pkg/daemon/ceph/util/util.go | 6 +++--- pkg/operator/ceph/cluster/cephstatus_test.go | 3 +++ pkg/operator/ceph/cluster/mgr/drain_test.go | 1 + pkg/operator/ceph/cluster/mgr/mgr_test.go | 1 + pkg/operator/ceph/cluster/mgr/spec.go | 6 +++--- pkg/operator/ceph/cluster/mon/drain_test.go | 1 + pkg/operator/ceph/cluster/osd/create.go | 9 +++------ pkg/operator/ceph/cluster/rbd/spec.go | 1 + pkg/operator/ceph/controller/spec.go | 12 ++++++++---- pkg/operator/ceph/controller/spec_test.go | 1 + pkg/operator/ceph/csi/config.go | 2 +- .../disruption/clusterdisruption/osd_test.go | 2 +- pkg/operator/ceph/file/dependent.go | 2 +- pkg/operator/ceph/file/filesystem.go | 12 +++++++----- pkg/operator/ceph/file/mds/mds.go | 18 ++++++++++-------- .../ceph/file/subvolumegroup/controller.go | 2 +- pkg/operator/ceph/object/dependents.go | 4 ++-- pkg/operator/ceph/object/user/controller.go | 2 +- pkg/operator/ceph/pool/controller.go | 2 +- pkg/operator/ceph/reporting/reporting.go | 2 +- pkg/operator/k8sutil/delete.go | 4 ++-- pkg/operator/k8sutil/deployment_test.go | 18 +++++++++++------- pkg/operator/k8sutil/service.go | 2 +- pkg/util/error.go | 2 +- pkg/util/exec/exec.go | 2 +- tests/framework/utils/k8s_helper.go | 4 ++-- tests/integration/ceph_base_object_test.go | 1 + tests/integration/ceph_smoke_test.go | 1 + 33 files changed, 88 insertions(+), 60 deletions(-) diff --git a/.github/workflows/golangci-lint.yaml b/.github/workflows/golangci-lint.yaml index 0d7198753..6ed1e527c 100644 --- a/.github/workflows/golangci-lint.yaml +++ b/.github/workflows/golangci-lint.yaml @@ -34,7 +34,7 @@ jobs: uses: golangci/golangci-lint-action@971e284b6050e8a5849b72094c50ab08da042db8 # v6.1.1 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.55 + version: v1.62 # Optional: working directory, useful for monorepos # working-directory: somedir diff --git a/pkg/daemon/ceph/client/crush_rule.go b/pkg/daemon/ceph/client/crush_rule.go index 97575b2a8..1843927da 100644 --- a/pkg/daemon/ceph/client/crush_rule.go +++ b/pkg/daemon/ceph/client/crush_rule.go @@ -150,8 +150,9 @@ func buildTwoStepCrushSteps(pool cephv1.PoolSpec) []stepSpec { // Step three stepTakeSubFailureDomain := &stepSpec{ Operation: "chooseleaf_firstn", - Number: int(pool.Replicated.ReplicasPerFailureDomain), - Type: pool.Replicated.SubFailureDomain, + // nolint:gosec // G115 - casting uint to int has no overflow + Number: int(pool.Replicated.ReplicasPerFailureDomain), + Type: pool.Replicated.SubFailureDomain, } steps = append(steps, *stepTakeSubFailureDomain) steps = append(steps, *stepEmit) diff --git a/pkg/daemon/ceph/client/image.go b/pkg/daemon/ceph/client/image.go index 8ea707d9c..cb2c7ab4a 100644 --- a/pkg/daemon/ceph/client/image.go +++ b/pkg/daemon/ceph/client/image.go @@ -156,11 +156,11 @@ func CreateImage(context *clusterd.Context, clusterInfo *ClusterInfo, name, pool // Roundup the size of the volume image since we only create images on 1MB boundaries and we should never create an image // size that's smaller than the requested one, e.g, requested 1048698 bytes should be 2MB while not be truncated to 1MB - sizeMB := int((size + ImageMinSize - 1) / ImageMinSize) + sizeMB := (size + ImageMinSize - 1) / ImageMinSize imageSpec := getImageSpec(name, poolName) - args := []string{"create", imageSpec, "--size", strconv.Itoa(sizeMB)} + args := []string{"create", imageSpec, "--size", fmt.Sprintf("%d", size)} if dataPoolName != "" { args = append(args, fmt.Sprintf("--data-pool=%s", dataPoolName)) diff --git a/pkg/daemon/ceph/client/pool_test.go b/pkg/daemon/ceph/client/pool_test.go index 3afe66b82..a6afc4691 100644 --- a/pkg/daemon/ceph/client/pool_test.go +++ b/pkg/daemon/ceph/client/pool_test.go @@ -572,7 +572,8 @@ func testCreatePoolWithReplicasPerFailureDomain(t *testing.T, failureDomain, cru assert.Equal(t, args[4], "size") poolSize, err := strconv.Atoi(args[5]) assert.NoError(t, err) - assert.Equal(t, uint(poolSize), poolSpec.Replicated.Size) + uPoolSize := uint(poolSize) // nolint:gosec // G115 : we know it is not too big + assert.Equal(t, uPoolSize, poolSpec.Replicated.Size) poolRuleSet = true return "", nil } diff --git a/pkg/daemon/ceph/osd/kms/kmip.go b/pkg/daemon/ceph/osd/kms/kmip.go index 8e8ba3aa3..491df3ade 100644 --- a/pkg/daemon/ceph/osd/kms/kmip.go +++ b/pkg/daemon/ceph/osd/kms/kmip.go @@ -24,6 +24,7 @@ import ( "encoding/base64" "fmt" "io" + "math" "strconv" "time" @@ -97,14 +98,20 @@ func InitKMIP(config map[string]string) (*kmipKMS, error) { kms.readTimeout = kmipDefaultReadTimeout timeout, err := strconv.Atoi(GetParam(config, kmipReadTimeOut)) if err == nil { - kms.readTimeout = uint8(timeout) + if timeout > math.MaxUint8 { + return nil, fmt.Errorf("read timeout %d is too big", timeout) + } + kms.readTimeout = uint8(timeout) // nolint:gosec // G115 : already checked if too big } // optional kms.writeTimeout = kmipDefaultWriteTimeout timeout, err = strconv.Atoi(GetParam(config, kmipWriteTimeOut)) if err == nil { - kms.writeTimeout = uint8(timeout) + if timeout > math.MaxUint8 { + return nil, fmt.Errorf("read timeout %d is too big", timeout) + } + kms.writeTimeout = uint8(timeout) // nolint:gosec // G115 : already checked if too big } caCert := GetParam(config, KmipCACert) diff --git a/pkg/daemon/ceph/util/util.go b/pkg/daemon/ceph/util/util.go index 8722357dd..8990578d3 100644 --- a/pkg/daemon/ceph/util/util.go +++ b/pkg/daemon/ceph/util/util.go @@ -42,18 +42,18 @@ func GetIPFromEndpoint(endpoint string) string { // GetPortFromEndpoint return the port from an endpoint string (192.168.0.1:6789) func GetPortFromEndpoint(endpoint string) int32 { - var port int + var port int64 _, portString, err := net.SplitHostPort(endpoint) if err != nil { logger.Errorf("failed to split host and port for endpoint %q, assuming default Ceph port %q. %v", endpoint, portString, err) } else { //nolint:gosec // using Atoi to convert type into int is not a real risk - port, err = strconv.Atoi(portString) + port, err = strconv.ParseInt(portString, 10, 32) if err != nil { logger.Errorf("failed to convert %q to integer. %v", portString, err) } } - //#nosec G109 -- This is just a lower bit to bigger bit conversion + //nolint:gosec // G109 No overflow: we parsed to 32 bits above portInt32 := int32(port) return portInt32 } diff --git a/pkg/operator/ceph/cluster/cephstatus_test.go b/pkg/operator/ceph/cluster/cephstatus_test.go index 183a4b4c5..94af861f1 100644 --- a/pkg/operator/ceph/cluster/cephstatus_test.go +++ b/pkg/operator/ceph/cluster/cephstatus_test.go @@ -104,6 +104,7 @@ func TestCephStatus(t *testing.T) { PgMap: cephclient.PgMap{TotalBytes: 0}, } aggregateStatus = toCustomResourceStatus(currentStatus, newStatus) + // nolint:gosec // G115 no overflow expected in the test assert.Equal(t, 0, int(aggregateStatus.Capacity.TotalBytes)) assert.Equal(t, "", aggregateStatus.Capacity.LastUpdated) @@ -112,6 +113,7 @@ func TestCephStatus(t *testing.T) { PgMap: cephclient.PgMap{TotalBytes: 1024}, } aggregateStatus = toCustomResourceStatus(currentStatus, newStatus) + // nolint:gosec // G115 no overflow expected in the test assert.Equal(t, 1024, int(aggregateStatus.Capacity.TotalBytes)) assert.Equal(t, formatTime(time.Now().UTC()), aggregateStatus.Capacity.LastUpdated) @@ -123,6 +125,7 @@ func TestCephStatus(t *testing.T) { } aggregateStatus = toCustomResourceStatus(currentStatus, newStatus) + // nolint:gosec // G115 no overflow expected in the test assert.Equal(t, 1024, int(aggregateStatus.Capacity.TotalBytes)) assert.Equal(t, formatTime(time.Now().Add(-time.Minute).UTC()), formatTime(time.Now().Add(-time.Minute).UTC())) } diff --git a/pkg/operator/ceph/cluster/mgr/drain_test.go b/pkg/operator/ceph/cluster/mgr/drain_test.go index 166cbc8bc..67d5bf52f 100644 --- a/pkg/operator/ceph/cluster/mgr/drain_test.go +++ b/pkg/operator/ceph/cluster/mgr/drain_test.go @@ -84,6 +84,7 @@ func TestReconcileMgrPDB(t *testing.T) { assert.Error(t, err) } assert.NoError(t, err) + // nolint:gosec // G115 no overflow expected in the test assert.Equalf(t, testCases.expectedMaxUnAvailable, int32(existingPDBV1.Spec.MaxUnavailable.IntValue()), "[%s]: incorrect minAvailable count in pdb", testCases.name) // reconcile mon PDB again to test update diff --git a/pkg/operator/ceph/cluster/mgr/mgr_test.go b/pkg/operator/ceph/cluster/mgr/mgr_test.go index 55bc84f09..16cba1af8 100644 --- a/pkg/operator/ceph/cluster/mgr/mgr_test.go +++ b/pkg/operator/ceph/cluster/mgr/mgr_test.go @@ -173,6 +173,7 @@ func validateServices(t *testing.T, c *Cluster) { assert.Equal(t, ds.Spec.Ports[0].Port, int32(dashboardPortHTTPS)) } else { // non-zero ports are configured as-is + // nolint:gosec // G115 no overflow expected in the test assert.Equal(t, ds.Spec.Ports[0].Port, int32(c.spec.Dashboard.Port)) } } else { diff --git a/pkg/operator/ceph/cluster/mgr/spec.go b/pkg/operator/ceph/cluster/mgr/spec.go index bc8adf10c..2e002733e 100644 --- a/pkg/operator/ceph/cluster/mgr/spec.go +++ b/pkg/operator/ceph/cluster/mgr/spec.go @@ -193,7 +193,7 @@ func (c *Cluster) makeMgrDaemonContainer(mgrConfig *mgrConfig) v1.Container { }, { Name: "dashboard", - ContainerPort: int32(c.dashboardInternalPort()), + ContainerPort: int32(c.dashboardInternalPort()), // nolint:gosec // G115 port numbers will not overflow an int32 Protocol: v1.ProtocolTCP, }, }, @@ -337,9 +337,9 @@ func (c *Cluster) makeDashboardService(name string) (*v1.Service, error) { Ports: []v1.ServicePort{ { Name: portName, - Port: int32(c.dashboardPublicPort()), + Port: int32(c.dashboardPublicPort()), // nolint:gosec // G115 port numbers will not overflow an int32 TargetPort: intstr.IntOrString{ - IntVal: int32(c.dashboardInternalPort()), + IntVal: int32(c.dashboardInternalPort()), // nolint:gosec // G115 port numbers will not overflow an int32 }, Protocol: v1.ProtocolTCP, }, diff --git a/pkg/operator/ceph/cluster/mon/drain_test.go b/pkg/operator/ceph/cluster/mon/drain_test.go index d01101ff3..c8ef5bc64 100644 --- a/pkg/operator/ceph/cluster/mon/drain_test.go +++ b/pkg/operator/ceph/cluster/mon/drain_test.go @@ -117,6 +117,7 @@ func TestReconcileMonPDB(t *testing.T) { continue } assert.NoError(t, err) + // nolint:gosec // G115 no overflow expected in the test assert.Equalf(t, tc.expectedMaxUnAvailable, int32(existingPDBV1.Spec.MaxUnavailable.IntValue()), "[%s]: incorrect minAvailable count in pdb", tc.name) // reconcile mon PDB again to test update diff --git a/pkg/operator/ceph/cluster/osd/create.go b/pkg/operator/ceph/cluster/osd/create.go index fd9324a51..c06780c80 100644 --- a/pkg/operator/ceph/cluster/osd/create.go +++ b/pkg/operator/ceph/cluster/osd/create.go @@ -203,8 +203,7 @@ func (c *Cluster) startProvisioningOverPVCs(config *provisionConfig, errs *provi // create encryption Kubernetes Secret if the PVC is encrypted key, err := GenerateDmCryptKey() if err != nil { - errMsg := fmt.Sprintf("failed to generate dmcrypt key for osd claim %q. %v", osdProps.pvc.ClaimName, err) - errs.addError(errMsg) + errs.addError("failed to generate dmcrypt key for osd claim %q. %v", osdProps.pvc.ClaimName, err) continue } @@ -216,8 +215,7 @@ func (c *Cluster) startProvisioningOverPVCs(config *provisionConfig, errs *provi if c.spec.Security.KeyManagementService.IsTokenAuthEnabled() && c.spec.Security.KeyManagementService.IsVaultKMS() { err := kms.SetTokenToEnvVar(c.clusterInfo.Context, c.context, c.spec.Security.KeyManagementService.TokenSecretName, kmsConfig.Provider, c.clusterInfo.Namespace) if err != nil { - errMsg := fmt.Sprintf("failed to fetch kms token secret %q. %v", c.spec.Security.KeyManagementService.TokenSecretName, err) - errs.addError(errMsg) + errs.addError("failed to fetch kms token secret %q. %v", c.spec.Security.KeyManagementService.TokenSecretName, err) continue } } @@ -227,8 +225,7 @@ func (c *Cluster) startProvisioningOverPVCs(config *provisionConfig, errs *provi // no risk of overwriting an existing key. err = kmsConfig.PutSecret(osdProps.pvc.ClaimName, key) if err != nil { - errMsg := fmt.Sprintf("failed to store secret. %v", err) - errs.addError(errMsg) + errs.addError("failed to store secret. %v", err) continue } } diff --git a/pkg/operator/ceph/cluster/rbd/spec.go b/pkg/operator/ceph/cluster/rbd/spec.go index e37fe79f5..b7b7a2ccf 100644 --- a/pkg/operator/ceph/cluster/rbd/spec.go +++ b/pkg/operator/ceph/cluster/rbd/spec.go @@ -74,6 +74,7 @@ func (r *ReconcileCephRBDMirror) makeDeployment(daemonConfig *daemonConfig, rbdM } rbdMirror.Spec.Placement.ApplyToPodSpec(&podSpec.Spec) + // nolint:gosec // G115 no overflow expected for rbd mirror count replicas := int32(rbdMirror.Spec.Count) d := &apps.Deployment{ ObjectMeta: metav1.ObjectMeta{ diff --git a/pkg/operator/ceph/controller/spec.go b/pkg/operator/ceph/controller/spec.go index b37d5e49f..5c818bf1a 100644 --- a/pkg/operator/ceph/controller/spec.go +++ b/pkg/operator/ceph/controller/spec.go @@ -497,19 +497,23 @@ func CheckPodMemory(name string, resources v1.ResourceRequirements, cephPodMinim if !podMemoryLimit.IsZero() { // This means LIMIT and REQUEST are either identical or different but still we use LIMIT as a reference - if uint64(podMemoryLimit.Value()) < display.MbTob(cephPodMinimumMemory) { + // nolint:gosec // G115 int64 to uint64 conversion is reasonabe here + upodMemoryLimit := uint64(podMemoryLimit.Value()) + if upodMemoryLimit < display.MbTob(cephPodMinimumMemory) { // allow the configuration if less than the min, but print a warning - logger.Warningf("running the %q daemon(s) with %dMB of ram, but at least %dMB is recommended", name, display.BToMb(uint64(podMemoryLimit.Value())), cephPodMinimumMemory) + logger.Warningf("running the %q daemon(s) with %dMB of ram, but at least %dMB is recommended", name, display.BToMb(upodMemoryLimit), cephPodMinimumMemory) } // This means LIMIT < REQUEST // Kubernetes will refuse to schedule that pod however it's still valuable to indicate that user's input was incorrect - if uint64(podMemoryLimit.Value()) < uint64(podMemoryRequest.Value()) { + // nolint:gosec // G115 int64 to uint64 conversion is reasonabe here + upodMemoryRequest := uint64(podMemoryRequest.Value()) + if upodMemoryLimit < upodMemoryRequest { extraErrorLine := `\n User has specified a pod memory limit %dmb below the pod memory request %dmb in the cluster CR.\n Rook will create pods that are expected to fail to serve as a more apparent error indicator to the user.` - return errors.Errorf(extraErrorLine, display.BToMb(uint64(podMemoryLimit.Value())), display.BToMb(uint64(podMemoryRequest.Value()))) + return errors.Errorf(extraErrorLine, display.BToMb(upodMemoryLimit), display.BToMb(upodMemoryRequest)) } } diff --git a/pkg/operator/ceph/controller/spec_test.go b/pkg/operator/ceph/controller/spec_test.go index 1c61e3f9c..ae741177a 100644 --- a/pkg/operator/ceph/controller/spec_test.go +++ b/pkg/operator/ceph/controller/spec_test.go @@ -85,6 +85,7 @@ func TestCheckPodMemory(t *testing.T) { name := "test" // A value for the memory used in the tests + // nolint:gosec // G115 no overflow expected in the test var memory_value = int64(PodMinimumMemory * 8 * uint64(math.Pow10(6))) // Case 1: No memory limits, no memory requested diff --git a/pkg/operator/ceph/csi/config.go b/pkg/operator/ceph/csi/config.go index 1d733aa69..66168602d 100644 --- a/pkg/operator/ceph/csi/config.go +++ b/pkg/operator/ceph/csi/config.go @@ -134,7 +134,7 @@ func CreateDefaultClientProfile(c client.Client, clusterInfo *cephclient.Cluster if r := recover(); r != nil { logger.Errorf("Panic when creating the default client profile: %+v", r) logger.Errorf("Stack trace:") - logger.Errorf(string(debug.Stack())) + logger.Errorf("%s", string(debug.Stack())) } }() diff --git a/pkg/operator/ceph/disruption/clusterdisruption/osd_test.go b/pkg/operator/ceph/disruption/clusterdisruption/osd_test.go index 92d8af82d..63cd1a28c 100644 --- a/pkg/operator/ceph/disruption/clusterdisruption/osd_test.go +++ b/pkg/operator/ceph/disruption/clusterdisruption/osd_test.go @@ -88,7 +88,7 @@ func fakeOSDDeployment(id, readyReplicas int) appsv1.Deployment { }, }, Status: appsv1.DeploymentStatus{ - ReadyReplicas: int32(readyReplicas), + ReadyReplicas: int32(readyReplicas), // nolint:gosec // G115 no overflow expected for ready replicas }, } return osd diff --git a/pkg/operator/ceph/file/dependent.go b/pkg/operator/ceph/file/dependent.go index 84b300fb9..53fdcfa0d 100644 --- a/pkg/operator/ceph/file/dependent.go +++ b/pkg/operator/ceph/file/dependent.go @@ -80,7 +80,7 @@ func cephFilesystemDependents(clusterdCtx *clusterd.Context, clusterInfo *cephcl if fsExists { deps, err = subvolumeGroupDependents(clusterdCtx, clusterInfo, filesystem) if err != nil { - return deps, errors.Wrapf(err, baseErrMsg) + return deps, errors.Wrapf(err, "%s", baseErrMsg) } } diff --git a/pkg/operator/ceph/file/filesystem.go b/pkg/operator/ceph/file/filesystem.go index d7f3cb20a..c8a304c65 100644 --- a/pkg/operator/ceph/file/filesystem.go +++ b/pkg/operator/ceph/file/filesystem.go @@ -218,11 +218,13 @@ func (f *Filesystem) updateFilesystem(context *clusterd.Context, clusterInfo *ce // Even if the fs already exists, the num active mdses may have changed if err := cephclient.SetNumMDSRanks(context, clusterInfo, f.Name, spec.MetadataServer.ActiveCount); err != nil { logger.Errorf( - fmt.Sprintf("failed to set num mds ranks (max_mds) to %d for filesystem %s, still continuing. ", spec.MetadataServer.ActiveCount, f.Name) + - "this error is not critical, but mdses may not be as failure tolerant as desired. " + - fmt.Sprintf("USER should verify that the number of active mdses is %d with 'ceph fs get %s'", spec.MetadataServer.ActiveCount, f.Name) + - fmt.Sprintf(". %v", err), - ) + "failed to set num mds ranks (max_mds) to %d for filesystem %s, still continuing. "+ + "this error is not critical, but mdses may not be as failure tolerant as desired. "+ + "USER should verify that the number of active mdses is %d with 'ceph fs get %s'. %v", + spec.MetadataServer.ActiveCount, + f.Name, + spec.MetadataServer.ActiveCount, f.Name, + err) } if err := createOrUpdatePools(f, context, clusterInfo, clusterSpec, spec); err != nil { diff --git a/pkg/operator/ceph/file/mds/mds.go b/pkg/operator/ceph/file/mds/mds.go index de2656f34..cdd17e45e 100644 --- a/pkg/operator/ceph/file/mds/mds.go +++ b/pkg/operator/ceph/file/mds/mds.go @@ -313,9 +313,10 @@ func (c *Cluster) scaleDownDeployments(replicas int32, activeCount int32, desire deps, err := getMdsDeployments(c.clusterInfo.Context, c.context, c.fs.Namespace, c.fs.Name) if err != nil { return errors.Wrapf(err, - fmt.Sprintf("cannot verify the removal of extraneous mds deployments for filesystem %s. ", c.fs.Name)+ - fmt.Sprintf("USER should make sure that only deployments %+v exist which match the filesystem's label selector", desiredDeployments), - ) + "cannot verify the removal of extraneous mds deployments for filesystem %s. "+ + "USER should make sure that only deployments %+v exist which match the filesystem's label selector", + c.fs.Name, + desiredDeployments) } if !(len(deps.Items) > int(replicas)) { // It's possible to check if there are fewer deployments than desired here, but that's @@ -332,11 +333,12 @@ func (c *Cluster) scaleDownDeployments(replicas int32, activeCount int32, desire if err := cephclient.WaitForActiveRanks(c.context, c.clusterInfo, c.fs.Name, activeCount, true, fsWaitForActiveTimeout); err != nil { errCount++ logger.Errorf( - "number of active mds ranks is not as desired. it is potentially unsafe to continue with extraneous mds deletion, so stopping. " + - fmt.Sprintf("USER should delete undesired mds daemons once filesystem %s is healthy. ", c.fs.Name) + - fmt.Sprintf("desired mds deployments for this filesystem are %+v", desiredDeployments) + - fmt.Sprintf(". %v", err), - ) + "number of active mds ranks is not as desired. it is potentially unsafe to continue with extraneous mds deletion, so stopping. "+ + "USER should delete undesired mds daemons once filesystem %s is healthy. "+ + "desired mds deployments for this filesystem are %+v. %v", + c.fs.Name, + desiredDeployments, + err) break // stop trying to delete daemons, but continue to reporting any errors below } diff --git a/pkg/operator/ceph/file/subvolumegroup/controller.go b/pkg/operator/ceph/file/subvolumegroup/controller.go index d74abe149..e5605785a 100644 --- a/pkg/operator/ceph/file/subvolumegroup/controller.go +++ b/pkg/operator/ceph/file/subvolumegroup/controller.go @@ -357,7 +357,7 @@ func (r *ReconcileCephFilesystemSubVolumeGroup) deleteSubVolumeGroup(cephFilesys msg = fmt.Sprintf("failed to delete ceph filesystem subvolume group %q, started clean up job to delete the subvolumes", cephFilesystemSubVolumeGroup.Name) } - return errors.Wrapf(err, msg) + return errors.Wrapf(err, "%s", msg) } return errors.Wrapf(err, "failed to delete ceph filesystem subvolume group %q", cephFilesystemSubVolumeGroup.Name) diff --git a/pkg/operator/ceph/object/dependents.go b/pkg/operator/ceph/object/dependents.go index b9d0cc240..2a88c2384 100644 --- a/pkg/operator/ceph/object/dependents.go +++ b/pkg/operator/ceph/object/dependents.go @@ -51,7 +51,7 @@ func CephObjectStoreDependents( // stores in multisite configs have different conditions that change what dependents should be checked zoneIsMaster, err := CheckZoneIsMaster(objCtx) if err != nil { - return deps, errors.Wrapf(err, baseErrMsg) + return deps, errors.Wrapf(err, "%s", baseErrMsg) } if !zoneIsMaster { // zone is a peer, and the master is the source of truth @@ -79,7 +79,7 @@ func CephObjectStoreDependents( // way of knowing if the bucket was created due to an ObjectBucketClaim or COSI Bucket. err := getBucketDependents(deps, clusterdCtx, clusterInfo, store, objCtx, opsCtx) if err != nil { - return deps, errors.Wrapf(err, baseErrMsg) + return deps, errors.Wrapf(err, "%s", baseErrMsg) } // CephObjectStoreUsers diff --git a/pkg/operator/ceph/object/user/controller.go b/pkg/operator/ceph/object/user/controller.go index f4f910d05..6a74e4cee 100644 --- a/pkg/operator/ceph/object/user/controller.go +++ b/pkg/operator/ceph/object/user/controller.go @@ -366,7 +366,7 @@ func (r *ReconcileObjectStoreUser) createOrUpdateCephUser(u *cephv1.CephObjectSt } // Set access and secret key - if r.userConfig.Keys == nil || len(r.userConfig.Keys) == 0 { + if len(r.userConfig.Keys) == 0 { r.userConfig.Keys = make([]admin.UserKeySpec, 1) } r.userConfig.Keys[0].AccessKey = user.Keys[0].AccessKey diff --git a/pkg/operator/ceph/pool/controller.go b/pkg/operator/ceph/pool/controller.go index c3c0d7732..f1d9ddd7c 100644 --- a/pkg/operator/ceph/pool/controller.go +++ b/pkg/operator/ceph/pool/controller.go @@ -577,7 +577,7 @@ func (r *ReconcileCephBlockPool) disableMirroring(pool string) error { if len(*mirroredPools.Images) > 0 { msg := fmt.Sprintf("there are images in the pool %q. Please manually disable mirroring for each image", pool) - logger.Errorf(msg) + logger.Errorf("%s", msg) return errors.New(msg) } } diff --git a/pkg/operator/ceph/reporting/reporting.go b/pkg/operator/ceph/reporting/reporting.go index 26c35b954..8a3cf28a4 100644 --- a/pkg/operator/ceph/reporting/reporting.go +++ b/pkg/operator/ceph/reporting/reporting.go @@ -144,7 +144,7 @@ func ReportReconcileResult( errorMsg := fmt.Sprintf("failed to reconcile %s %q. %v", kind, nsName, err) // 1. log - logger.Errorf(errorMsg) + logger.Errorf("%s", errorMsg) // 2. event recorder.Event(objCopy, corev1.EventTypeWarning, string(cephv1.ReconcileFailed), errorMsg) diff --git a/pkg/operator/k8sutil/delete.go b/pkg/operator/k8sutil/delete.go index 0c879f858..f6e9ed37f 100644 --- a/pkg/operator/k8sutil/delete.go +++ b/pkg/operator/k8sutil/delete.go @@ -104,8 +104,8 @@ func DeleteResource( msg := fmt.Sprintf("failed to delete %s. gave up waiting after %d retries every %v seconds. %+v", resource, retries, retryInterval/time.Second, err) if opts.ErrorOnTimeout { - return fmt.Errorf(msg) + return fmt.Errorf("%s", msg) } - logger.Warningf(msg) + logger.Warningf("%s", msg) return nil } diff --git a/pkg/operator/k8sutil/deployment_test.go b/pkg/operator/k8sutil/deployment_test.go index e604b99ef..9521b515d 100644 --- a/pkg/operator/k8sutil/deployment_test.go +++ b/pkg/operator/k8sutil/deployment_test.go @@ -72,23 +72,25 @@ func TestUpdateMultipleDeploymentsAndWait(t *testing.T) { return d } - timesCalled := 0 + timesCalled := int32(0) // generate a status that is not ready when first called but becomes ready later - status := func(timesCalled int) appsv1.DeploymentStatus { + status := func(timesCalled int32) appsv1.DeploymentStatus { if timesCalled < 0 { timesCalled = 0 } return appsv1.DeploymentStatus{ ObservedGeneration: int64(timesCalled), - UpdatedReplicas: int32(timesCalled), - ReadyReplicas: int32(timesCalled), + //nolint:gosec // G115 widening cast - no integer overflow + UpdatedReplicas: int32(timesCalled), + //nolint:gosec // G115 widening cast - no integer overflow + ReadyReplicas: int32(timesCalled), Conditions: []appsv1.DeploymentCondition{ {Type: appsv1.DeploymentProgressing, Reason: "DoinStuff"}, }, } } // generate a deployment with no errors and status=status(timesCalled) - okayDeployment := func(name string, timesCalled int) appsv1.Deployment { + okayDeployment := func(name string, timesCalled int32) appsv1.Deployment { return appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -274,8 +276,10 @@ func TestWaitForDeploymentsToUpdate(t *testing.T) { } return appsv1.DeploymentStatus{ ObservedGeneration: int64(timesCalled), - UpdatedReplicas: int32(timesCalled), - ReadyReplicas: int32(timesCalled), + // nolint:gosec // G115 No overflow: widening cast. + UpdatedReplicas: int32(timesCalled), + // nolint:gosec // G115 No overflow: widening cast. + ReadyReplicas: int32(timesCalled), Conditions: []appsv1.DeploymentCondition{ {Type: appsv1.DeploymentProgressing, Reason: "DoinStuff"}, }, diff --git a/pkg/operator/k8sutil/service.go b/pkg/operator/k8sutil/service.go index 56506240e..00f388166 100644 --- a/pkg/operator/k8sutil/service.go +++ b/pkg/operator/k8sutil/service.go @@ -165,7 +165,7 @@ func verifyExportedService(ctx context.Context, client *mcsv1Client.Clientset, n for _, condition := range exportedService.Status.Conditions { if condition.Type == mcsv1a1.ServiceExportValid && condition.Status == v1.ConditionFalse { - return fmt.Errorf(*condition.Message) + return fmt.Errorf("%s", *condition.Message) } } diff --git a/pkg/util/error.go b/pkg/util/error.go index 8ef13b89b..f4f7b5433 100644 --- a/pkg/util/error.go +++ b/pkg/util/error.go @@ -41,5 +41,5 @@ func AggregateErrors(errs []error, format string, args ...interface{}) error { for _, err := range errs { errString = fmt.Sprintf("%s\n %s", errString, err.Error()) } - return errors.Errorf(errString) + return errors.Errorf("%s", errString) } diff --git a/pkg/util/exec/exec.go b/pkg/util/exec/exec.go index 54f40cbcb..31dc65733 100644 --- a/pkg/util/exec/exec.go +++ b/pkg/util/exec/exec.go @@ -284,7 +284,7 @@ func ExtractExitCode(err error) (int, error) { return int(errType.ErrStatus.Code), nil default: - logger.Debugf(err.Error()) + logger.Debugf("%s", err.Error()) // This is ugly, but it's a decent backup just in case the error isn't a type above. if strings.Contains(err.Error(), "command terminated with exit code") { a := strings.SplitAfter(err.Error(), "command terminated with exit code") diff --git a/tests/framework/utils/k8s_helper.go b/tests/framework/utils/k8s_helper.go index d7ebb81f3..207dfa1c0 100644 --- a/tests/framework/utils/k8s_helper.go +++ b/tests/framework/utils/k8s_helper.go @@ -365,7 +365,7 @@ func (k8sh *K8sHelper) WaitForStatusPhase(namespace, kind, name, desiredPhase st return true, nil } - logger.Infof(baseErr) + logger.Infof("%s", baseErr) return false, nil }) if err != nil { @@ -635,7 +635,7 @@ func (k8sh *K8sHelper) IsCRDPresent(crdName string) bool { for i := 0; i < RetryLoop; i++ { _, err := k8sh.Kubectl(cmdArgs...) if err == nil { - k8slogger.Infof("Found the CRD resource: " + crdName) + k8slogger.Infof("Found the CRD resource: %s", crdName) return true } time.Sleep(RetryInterval * time.Second) diff --git a/tests/integration/ceph_base_object_test.go b/tests/integration/ceph_base_object_test.go index 1730a03fa..fcf0b538e 100644 --- a/tests/integration/ceph_base_object_test.go +++ b/tests/integration/ceph_base_object_test.go @@ -98,6 +98,7 @@ func createCephObjectStore(t *testing.T, helper *clients.TestClient, k8sh *utils }) } t.Run("create CephObjectStore", func(t *testing.T) { + // nolint:gosec // G115 no overflow in test err := helper.ObjectClient.Create(namespace, storeName, int32(replicaSize), tlsEnable, swiftAndKeystone) assert.Nil(t, err) }) diff --git a/tests/integration/ceph_smoke_test.go b/tests/integration/ceph_smoke_test.go index 9a9c3c802..153db60e6 100644 --- a/tests/integration/ceph_smoke_test.go +++ b/tests/integration/ceph_smoke_test.go @@ -234,6 +234,7 @@ func (s *SmokeSuite) TestPoolResize() { require.NoError(s.T(), err) if details.Size > 1 { logger.Infof("pool %s size was updated", poolName) + // nolint:gosec // G115 no overflow expected in the test require.Equal(s.T(), 2, int(details.Size)) poolResized = true