Merge pull request kubernetes-sigs#790 from mallow111/add-lint-part3

chore: add more linters and fix issues

.golangci.yml

@@ -11,8 +11,10 @@ linters:
     - durationcheck
     - exportloopref
     - errcheck
+    - errorlint
     - exhaustive
     - goconst
+    - gocyclo
     - gofmt
     - goimports
     - gosimple
@@ -23,6 +25,7 @@ linters:
     - misspell
     - nilerr
     - revive
+    - staticcheck
     - structcheck
     - stylecheck
     - typecheck

controllers/secretproviderclasspodstatus_controller.go

@@ -111,7 +111,7 @@ func (r *SecretProviderClassPodStatusReconciler) Patcher(ctx context.Context) er
 	// get a list of all spc pod status that belong to the node
 	err := r.reader.List(ctx, spcPodStatusList, r.ListOptionsLabelSelector())
 	if err != nil {
-		return fmt.Errorf("failed to list secret provider class pod status, err: %+v", err)
+		return fmt.Errorf("failed to list secret provider class pod status, err: %w", err)
 	}
 
 	spcPodStatuses := spcPodStatusList.Items
@@ -124,15 +124,15 @@ func (r *SecretProviderClassPodStatusReconciler) Patcher(ctx context.Context) er
 			spc = &val
 		} else {
 			if err := r.reader.Get(ctx, client.ObjectKey{Namespace: namespace, Name: spcName}, spc); err != nil {
-				return fmt.Errorf("failed to get spc %s, err: %+v", spcName, err)
+				return fmt.Errorf("failed to get spc %s, err: %w", spcName, err)
 			}
 			spcMap[namespace+"/"+spcName] = *spc
 		}
 		// get the pod and check if the pod has a owner reference
 		pod := &corev1.Pod{}
 		err = r.reader.Get(ctx, client.ObjectKey{Namespace: namespace, Name: spcPodStatuses[i].Status.PodName}, pod)
 		if err != nil {
-			return fmt.Errorf("failed to fetch pod during patching, err: %+v", err)
+			return fmt.Errorf("failed to fetch pod during patching, err: %w", err)
 		}
 		var ownerRefs []metav1.OwnerReference
 		for _, ownerRef := range pod.GetOwnerReferences() {
@@ -289,7 +289,7 @@ func (r *SecretProviderClassPodStatusReconciler) Reconcile(ctx context.Context,
 
 		if err = secretutil.ValidateSecretObject(*secretObj); err != nil {
 			klog.ErrorS(err, "failed to validate secret object in spc", "spc", klog.KObj(spc), "pod", klog.KObj(pod), "spcps", klog.KObj(spcPodStatus))
-			errs = append(errs, fmt.Errorf("failed to validate secret object in spc %s/%s, err: %+v", spc.Namespace, spc.Name, err))
+			errs = append(errs, fmt.Errorf("failed to validate secret object in spc %s/%s, err: %w", spc.Namespace, spc.Name, err))
 			continue
 		}
 		exists, err := r.secretExists(ctx, secretName, req.Namespace)
@@ -300,7 +300,7 @@ func (r *SecretProviderClassPodStatusReconciler) Reconcile(ctx context.Context,
 			if apierrors.IsForbidden(err) {
 				klog.Warning(SyncSecretForbiddenWarning)
 			}
-			errs = append(errs, fmt.Errorf("failed to check if secret %s exists, err: %+v", secretName, err))
+			errs = append(errs, fmt.Errorf("failed to check if secret %s exists, err: %w", secretName, err))
 			continue
 		}
 
@@ -313,7 +313,7 @@ func (r *SecretProviderClassPodStatusReconciler) Reconcile(ctx context.Context,
 			if datamap, err = secretutil.GetSecretData(secretObj.Data, secretType, files); err != nil {
 				r.generateEvent(pod, corev1.EventTypeWarning, secretCreationFailedReason, fmt.Sprintf("failed to get data in spc %s/%s for secret %s, err: %+v", req.Namespace, spcName, secretName, err))
 				klog.ErrorS(err, "failed to get data in spc for secret", "spc", klog.KObj(spc), "pod", klog.KObj(pod), "secret", klog.ObjectRef{Namespace: req.Namespace, Name: secretName}, "spcps", klog.KObj(spcPodStatus))
-				errs = append(errs, fmt.Errorf("failed to get data in spc %s/%s for secret %s, err: %+v", req.Namespace, spcName, secretName, err))
+				errs = append(errs, fmt.Errorf("failed to get data in spc %s/%s for secret %s, err: %w", req.Namespace, spcName, secretName, err))
 				continue
 			}
 

pkg/rotation/reconciler.go

@@ -228,6 +228,7 @@ func (r *Reconciler) processNextItem() bool {
 	return true
 }
 
+//gocyclo:ignore
 func (r *Reconciler) reconcile(ctx context.Context, spcps *secretsstorev1.SecretProviderClassPodStatus) (err error) {
 	begin := time.Now()
 	errorReason := internalerrors.FailedToRotate
@@ -257,7 +258,7 @@ func (r *Reconciler) reconcile(ctx context.Context, spcps *secretsstorev1.Secret
 	)
 	if err != nil {
 		errorReason = internalerrors.PodNotFound
-		return fmt.Errorf("failed to get pod %s/%s, err: %+v", spcps.Namespace, spcps.Status.PodName, err)
+		return fmt.Errorf("failed to get pod %s/%s, err: %w", spcps.Namespace, spcps.Status.PodName, err)
 	}
 	// skip rotation if the pod is being terminated
 	// or the pod is in succeeded state (for jobs that complete aren't gc yet)
@@ -280,7 +281,7 @@ func (r *Reconciler) reconcile(ctx context.Context, spcps *secretsstorev1.Secret
 	)
 	if err != nil {
 		errorReason = internalerrors.SecretProviderClassNotFound
-		return fmt.Errorf("failed to get secret provider class %s/%s, err: %+v", spcps.Namespace, spcps.Status.SecretProviderClassName, err)
+		return fmt.Errorf("failed to get secret provider class %s/%s, err: %w", spcps.Namespace, spcps.Status.SecretProviderClassName, err)
 	}
 
 	// determine which pod volume this is associated with
@@ -312,11 +313,11 @@ func (r *Reconciler) reconcile(ctx context.Context, spcps *secretsstorev1.Secret
 
 	paramsJSON, err := json.Marshal(parameters)
 	if err != nil {
-		return fmt.Errorf("failed to marshal parameters, err: %+v", err)
+		return fmt.Errorf("failed to marshal parameters, err: %w", err)
 	}
 	permissionJSON, err := json.Marshal(permission)
 	if err != nil {
-		return fmt.Errorf("failed to marshal permission, err: %+v", err)
+		return fmt.Errorf("failed to marshal permission, err: %w", err)
 	}
 
 	// check if the volume pertaining to the current spc is using nodePublishSecretRef for
@@ -338,7 +339,7 @@ func (r *Reconciler) reconcile(ctx context.Context, spcps *secretsstorev1.Secret
 			}
 			errorReason = internalerrors.NodePublishSecretRefNotFound
 			r.generateEvent(pod, corev1.EventTypeWarning, mountRotationFailedReason, fmt.Sprintf("failed to get node publish secret %s/%s, err: %+v", spcps.Namespace, nodePublishSecretRef.Name, err))
-			return fmt.Errorf("failed to get node publish secret %s/%s, err: %+v", spcps.Namespace, nodePublishSecretRef.Name, err)
+			return fmt.Errorf("failed to get node publish secret %s/%s, err: %w", spcps.Namespace, nodePublishSecretRef.Name, err)
 		}
 
 		for k, v := range secret.Data {
@@ -349,7 +350,7 @@ func (r *Reconciler) reconcile(ctx context.Context, spcps *secretsstorev1.Secret
 	secretsJSON, err = json.Marshal(nodePublishSecretData)
 	if err != nil {
 		r.generateEvent(pod, corev1.EventTypeWarning, mountRotationFailedReason, fmt.Sprintf("failed to marshal node publish secret data, err: %+v", err))
-		return fmt.Errorf("failed to marshal node publish secret data, err: %+v", err)
+		return fmt.Errorf("failed to marshal node publish secret data, err: %w", err)
 	}
 
 	// generate a map with the current object versions stored in spc pod status
@@ -371,7 +372,7 @@ func (r *Reconciler) reconcile(ctx context.Context, spcps *secretsstorev1.Secret
 	newObjectVersions, errorReason, err := secretsstore.MountContent(ctx, providerClient, string(paramsJSON), string(secretsJSON), spcps.Status.TargetPath, string(permissionJSON), oldObjectVersions)
 	if err != nil {
 		r.generateEvent(pod, corev1.EventTypeWarning, mountRotationFailedReason, fmt.Sprintf("provider mount err: %+v", err))
-		return fmt.Errorf("failed to rotate objects for pod %s/%s, err: %+v", spcps.Namespace, spcps.Status.PodName, err)
+		return fmt.Errorf("failed to rotate objects for pod %s/%s, err: %w", spcps.Namespace, spcps.Status.PodName, err)
 	}
 
 	// compare the old object versions and new object versions to check if any of the objects
@@ -424,7 +425,7 @@ func (r *Reconciler) reconcile(ctx context.Context, spcps *secretsstorev1.Secret
 			Jitter:   0.1,
 		}, updateFn); err != nil {
 			r.generateEvent(pod, corev1.EventTypeWarning, mountRotationFailedReason, fmt.Sprintf("failed to update versions in spc pod status %s, err: %+v", spc.Name, err))
-			return fmt.Errorf("failed to update spc pod status, err: %+v", err)
+			return fmt.Errorf("failed to update spc pod status, err: %w", err)
 		}
 	}
 
@@ -435,7 +436,7 @@ func (r *Reconciler) reconcile(ctx context.Context, spcps *secretsstorev1.Secret
 	files, err := fileutil.GetMountedFiles(spcps.Status.TargetPath)
 	if err != nil {
 		r.generateEvent(pod, corev1.EventTypeWarning, k8sSecretRotationFailedReason, fmt.Sprintf("failed to get mounted files, err: %+v", err))
-		return fmt.Errorf("failed to get mounted files, err: %+v", err)
+		return fmt.Errorf("failed to get mounted files, err: %w", err)
 	}
 	for _, secretObj := range spc.Spec.SecretObjects {
 		secretName := strings.TrimSpace(secretObj.SecretName)
@@ -525,11 +526,11 @@ func (r *Reconciler) patchSecret(ctx context.Context, name, namespace string, da
 
 	currentDataSHA, err := secretutil.GetSHAFromSecret(secret.Data)
 	if err != nil {
-		return fmt.Errorf("failed to compute SHA for %s/%s old data, err: %+v", namespace, name, err)
+		return fmt.Errorf("failed to compute SHA for %s/%s old data, err: %w", namespace, name, err)
 	}
 	newDataSHA, err := secretutil.GetSHAFromSecret(data)
 	if err != nil {
-		return fmt.Errorf("failed to compute SHA for %s/%s new data, err: %+v", namespace, name, err)
+		return fmt.Errorf("failed to compute SHA for %s/%s new data, err: %w", namespace, name, err)
 	}
 	// if the SHA for the current data and new data match then skip
 	// the redundant API call to patch the same data
@@ -541,18 +542,18 @@ func (r *Reconciler) patchSecret(ctx context.Context, name, namespace string, da
 	newSecret.Data = data
 	oldData, err := json.Marshal(secret)
 	if err != nil {
-		return fmt.Errorf("failed to marshal old secret, err: %+v", err)
+		return fmt.Errorf("failed to marshal old secret, err: %w", err)
 	}
 	secret.Data = data
 	newData, err := json.Marshal(&newSecret)
 	if err != nil {
-		return fmt.Errorf("failed to marshal new secret, err: %+v", err)
+		return fmt.Errorf("failed to marshal new secret, err: %w", err)
 	}
 	// Patching data replaces values for existing data keys
 	// and appends new keys if it doesn't already exist
 	patch, err := strategicpatch.CreateTwoWayMergePatch(oldData, newData, secret)
 	if err != nil {
-		return fmt.Errorf("failed to create patch, err: %+v", err)
+		return fmt.Errorf("failed to create patch, err: %w", err)
 	}
 	_, err = r.kubeClient.CoreV1().Secrets(namespace).Patch(ctx, name, types.MergePatchType, patch, metav1.PatchOptions{})
 	return err

pkg/secrets-store/nodeserver.go

@@ -209,15 +209,15 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 	mounted = true
 	var objectVersions map[string]string
 	if objectVersions, errorReason, err = ns.mountSecretsStoreObjectContent(ctx, providerName, string(parametersStr), string(secretStr), targetPath, string(permissionStr), podName); err != nil {
-		return nil, fmt.Errorf("failed to mount secrets store objects for pod %s/%s, err: %v", podNamespace, podName, err)
+		return nil, fmt.Errorf("failed to mount secrets store objects for pod %s/%s, err: %w", podNamespace, podName, err)
 	}
 
 	// create or update the secret provider class pod status object
 	// SPCPS is created the first time after the pod mount is complete. Update is required in scenarios where
 	// the pod with same name (pods created by statefulsets) is moved to a different node and the old SPCPS
 	// has not yet been garbage collected.
 	if err = createOrUpdateSecretProviderClassPodStatus(ctx, ns.client, ns.reader, podName, podNamespace, podUID, secretProviderClass, targetPath, ns.nodeID, true, objectVersions); err != nil {
-		return nil, fmt.Errorf("failed to create secret provider class pod status for pod %s/%s, err: %v", podNamespace, podName, err)
+		return nil, fmt.Errorf("failed to create secret provider class pod status for pod %s/%s, err: %w", podNamespace, podName, err)
 	}
 
 	klog.InfoS("node publish volume complete", "targetPath", targetPath, "pod", klog.ObjectRef{Namespace: podNamespace, Name: podName})

pkg/secrets-store/provider_client_test.go

@@ -426,7 +426,7 @@ func TestPluginClientBuilderErrorNotFound(t *testing.T) {
 	cb := NewPluginClientBuilder(path)
 	ctx := context.Background()
 
-	if _, err := cb.Get(ctx, "notfoundprovider"); errors.Unwrap(err) != ErrProviderNotFound {
+	if _, err := cb.Get(ctx, "notfoundprovider"); !errors.Is(err, ErrProviderNotFound) {
 		t.Errorf("Get(%s) = %v, want %v", "notfoundprovider", err, ErrProviderNotFound)
 	}
 
@@ -448,7 +448,7 @@ func TestPluginClientBuilderErrorInvalid(t *testing.T) {
 	cb := NewPluginClientBuilder(path)
 	ctx := context.Background()
 
-	if _, err := cb.Get(ctx, "bad/provider/name"); errors.Unwrap(err) != ErrInvalidProvider {
+	if _, err := cb.Get(ctx, "bad/provider/name"); !errors.Is(err, ErrInvalidProvider) {
 		t.Errorf("Get(%s) = %v, want %v", "bad/provider/name", err, ErrInvalidProvider)
 	}
 }

pkg/secrets-store/utils.go

@@ -83,7 +83,7 @@ func getSecretProviderItem(ctx context.Context, c client.Client, name, namespace
 		Name:      name,
 	}
 	if err := c.Get(ctx, spcKey, spc); err != nil {
-		return nil, fmt.Errorf("failed to get secretproviderclass %s/%s, error: %+v", namespace, name, err)
+		return nil, fmt.Errorf("failed to get secretproviderclass %s/%s, error: %w", namespace, name, err)
 	}
 	return spc, nil
 }

pkg/util/secretutil/secret.go

@@ -166,13 +166,13 @@ func GetSecretData(secretObjData []*secretsstorev1.SecretObjectData, secretType
 		}
 		content, err := os.ReadFile(file)
 		if err != nil {
-			return datamap, fmt.Errorf("failed to read file %s, err: %v", objectName, err)
+			return datamap, fmt.Errorf("failed to read file %s, err: %w", objectName, err)
 		}
 		datamap[dataKey] = content
 		if secretType == corev1.SecretTypeTLS {
 			c, err := GetCertPart(content, dataKey)
 			if err != nil {
-				return datamap, fmt.Errorf("failed to get cert data from file %s, err: %+v", file, err)
+				return datamap, fmt.Errorf("failed to get cert data from file %s, err: %w", file, err)
 			}
 			datamap[dataKey] = c
 		}

pkg/util/secretutil/secret_test.go

@@ -337,7 +337,12 @@ func createTestFile(tmpDir, fileName string) (string, error) {
 	if fileName != "" {
 		filePath := fmt.Sprintf("%s/%s", tmpDir, fileName)
 		f, err := os.Create(filePath)
-		defer f.Close()
+		defer func() {
+			err = f.Close()
+			if err != nil {
+				return
+			}
+		}()
 		if err != nil {
 			return filePath, err
 		}

provider/fake/fake_server.go

@@ -109,13 +109,13 @@ func (m *MockCSIProviderServer) Mount(ctx context.Context, req *v1alpha1.MountRe
 		return &v1alpha1.MountResponse{}, m.returnErr
 	}
 	if err = json.Unmarshal([]byte(req.GetAttributes()), &attrib); err != nil {
-		return nil, fmt.Errorf("failed to unmarshal attributes, error: %+v", err)
+		return nil, fmt.Errorf("failed to unmarshal attributes, error: %w", err)
 	}
 	if err = json.Unmarshal([]byte(req.GetSecrets()), &secret); err != nil {
-		return nil, fmt.Errorf("failed to unmarshal secrets, error: %+v", err)
+		return nil, fmt.Errorf("failed to unmarshal secrets, error: %w", err)
 	}
 	if err = json.Unmarshal([]byte(req.GetPermission()), &filePermission); err != nil {
-		return nil, fmt.Errorf("failed to unmarshal file permission, error: %+v", err)
+		return nil, fmt.Errorf("failed to unmarshal file permission, error: %w", err)
 	}
 	if len(req.GetTargetPath()) == 0 {
 		return nil, fmt.Errorf("missing target path")

test/e2eprovider/server/server.go

@@ -124,13 +124,13 @@ func (s *Server) Mount(ctx context.Context, req *v1alpha1.MountRequest) (*v1alph
 	}
 
 	if err = json.Unmarshal([]byte(req.GetAttributes()), &attrib); err != nil {
-		return nil, fmt.Errorf("failed to unmarshal attributes, error: %+v", err)
+		return nil, fmt.Errorf("failed to unmarshal attributes, error: %w", err)
 	}
 	if err = json.Unmarshal([]byte(req.GetSecrets()), &secret); err != nil {
-		return nil, fmt.Errorf("failed to unmarshal secrets, error: %+v", err)
+		return nil, fmt.Errorf("failed to unmarshal secrets, error: %w", err)
 	}
 	if err = json.Unmarshal([]byte(req.GetPermission()), &filePermission); err != nil {
-		return nil, fmt.Errorf("failed to unmarshal file permission, error: %+v", err)
+		return nil, fmt.Errorf("failed to unmarshal file permission, error: %w", err)
 	}
 	if len(req.GetTargetPath()) == 0 {
 		return nil, fmt.Errorf("missing target path")