Cleanup: addressed various nolint items.

- Addressed newly identified `errorlint` items. - Narrowed `nolint` --> `nolint:gocritic`. - Removed 2 `nolint` comments by addressing identified issues. - Also: cleaned up some test logic in pipelinerun_test.go. There are no expected functional changes in this PR. Context: #5899 /kind cleanup
tektoncd · Apr 3, 2023 · b90eac7 · b90eac7
1 parent 8cb2529
commit b90eac7
Show file tree

Hide file tree

Showing 6 changed files with 25 additions and 18 deletions.
diff --git a/pkg/pod/pod.go b/pkg/pod/pod.go
@@ -223,7 +223,7 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec
 	// Superceded by podTemplate envs
 	if len(implicitEnvVars) > 0 {
 		for i, s := range stepContainers {
-			env := append(implicitEnvVars, s.Env...) //nolint
+			env := append(implicitEnvVars, s.Env...) // nolint:gocritic
 			stepContainers[i].Env = env
 		}
 	}
@@ -235,15 +235,15 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec
 	}
 	if len(podTemplate.Env) > 0 {
 		for i, s := range stepContainers {
-			env := append(s.Env, filteredEnvs...) //nolint
+			env := append(s.Env, filteredEnvs...) // nolint:gocritic
 			stepContainers[i].Env = env
 		}
 	}
 	// Add env var if hermetic execution was requested & if the alpha API is enabled
 	if taskRun.Annotations[ExecutionModeAnnotation] == ExecutionModeHermetic && alphaAPIEnabled {
 		for i, s := range stepContainers {
 			// Add it at the end so it overrides
-			env := append(s.Env, corev1.EnvVar{Name: TektonHermeticEnvVar, Value: "1"}) //nolint
+			env := append(s.Env, corev1.EnvVar{Name: TektonHermeticEnvVar, Value: "1"}) // nolint:gocritic
 			stepContainers[i].Env = env
 		}
 	}
@@ -280,7 +280,7 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec
 				toAdd = append(toAdd, imp)
 			}
 		}
-		vms := append(s.VolumeMounts, toAdd...) //nolint
+		vms := append(s.VolumeMounts, toAdd...) // nolint:gocritic
 		stepContainers[i].VolumeMounts = vms
 	}
 
@@ -301,7 +301,7 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec
 					toAdd = append(toAdd, imp)
 				}
 			}
-			vms := append(s.VolumeMounts, toAdd...) //nolint
+			vms := append(s.VolumeMounts, toAdd...) // nolint:gocritic
 			sidecarContainers[i].VolumeMounts = vms
 		}
 	}

diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go
@@ -1155,7 +1155,7 @@ spec:
 			prt := newPipelineRunTest(t, d)
 			defer prt.Cancel()
 
-			wantEvents := append(tc.wantEvents, "Warning InternalError 1 error occurred") //nolint
+			wantEvents := append(tc.wantEvents, "Warning InternalError 1 error occurred") //nolint:gocritic
 			reconciledRun, _ := prt.reconcileRun("foo", tc.pipelineRun.Name, wantEvents, tc.permanentError)
 
 			if reconciledRun.Status.CompletionTime == nil {
@@ -7156,11 +7156,14 @@ func checkPipelineRunConditionStatusAndReason(t *testing.T, reconciledRun *v1bet
 	t.Helper()
 
 	condition := reconciledRun.Status.GetCondition(apis.ConditionSucceeded)
-	if condition == nil || condition.Status != conditionStatus {
-		t.Errorf("Expected PipelineRun status to be %s, but was %v", conditionStatus, condition)
+	if condition == nil {
+		t.Fatalf("want condition, got nil")
 	}
-	if condition != nil && condition.Reason != conditionReason {
-		t.Errorf("Expected reason %s but was %s", conditionReason, condition.Reason)
+	if condition.Status != conditionStatus {
+		t.Errorf("want status %v, got %v", conditionStatus, condition.Status)
+	}
+	if condition.Reason != conditionReason {
+		t.Errorf("want reason %v, got %v", conditionReason, condition.Reason)
 	}
 }
 

diff --git a/pkg/reconciler/pipelinerun/resources/pipelineref.go b/pkg/reconciler/pipelinerun/resources/pipelineref.go
@@ -117,7 +117,9 @@ func GetVerifiedPipelineFunc(ctx context.Context, k8s kubernetes.Interface, tekt
 			source = s.URI
 		}
 		if err := trustedresources.VerifyPipeline(ctx, p, k8s, source, verificationpolicies); err != nil {
-			return nil, nil, fmt.Errorf("GetVerifiedPipelineFunc failed: %w: %v", trustedresources.ErrResourceVerificationFailed, err)
+			// FixMe: the below %v should be %w (and the nolint pragma removed)
+			// but making that change causes e2e test failures.
+			return nil, nil, fmt.Errorf("GetVerifiedPipelineFunc failed: %w: %v", trustedresources.ErrResourceVerificationFailed, err) // nolint:errorlint
 		}
 		return p, s, nil
 	}

diff --git a/pkg/reconciler/taskrun/resources/taskref.go b/pkg/reconciler/taskrun/resources/taskref.go
@@ -95,7 +95,7 @@ func GetVerifiedTaskFunc(ctx context.Context, k8s kubernetes.Interface, tekton c
 			source = s.URI
 		}
 		if err := trustedresources.VerifyTask(ctx, t, k8s, source, verificationpolicies); err != nil {
-			return nil, nil, fmt.Errorf("GetVerifiedTaskFunc failed: %w: %v", trustedresources.ErrResourceVerificationFailed, err)
+			return nil, nil, fmt.Errorf("GetVerifiedTaskFunc failed: %w: %v", trustedresources.ErrResourceVerificationFailed, err) // nolint:errorlint
 		}
 		return t, s, nil
 	}

diff --git a/pkg/trustedresources/verifier/verifier.go b/pkg/trustedresources/verifier/verifier.go
@@ -97,7 +97,7 @@ func fromKeyRef(ctx context.Context, keyRef string, hashAlgorithm crypto.Hash, k
 	}
 	raw, err := os.ReadFile(filepath.Clean(keyRef))
 	if err != nil {
-		return nil, fmt.Errorf("%w: %v", ErrFailedLoadKeyFile, err) // nolint -- needs errorlint cleanup
+		return nil, fmt.Errorf("%w: %v", ErrFailedLoadKeyFile, err) // nolint:errorlint
 	}
 	v, err := fromData(raw, hashAlgorithm)
 	if err != nil {
@@ -136,11 +136,11 @@ func fromSecret(ctx context.Context, secretRef string, hashAlgorithm crypto.Hash
 func fromData(raw []byte, hashAlgorithm crypto.Hash) (signature.Verifier, error) {
 	pubKey, err := cryptoutils.UnmarshalPEMToPublicKey(raw)
 	if err != nil {
-		return nil, fmt.Errorf("%w: %v", ErrDecodeKey, err) // nolint -- needs errorlint cleanup
+		return nil, fmt.Errorf("%w: %v", ErrDecodeKey, err) // nolint:errorlint
 	}
 	v, err := signature.LoadVerifier(pubKey, hashAlgorithm)
 	if err != nil {
-		return nil, fmt.Errorf("%w: %v", ErrLoadVerifier, err) // nolint -- needs errorlint cleanup
+		return nil, fmt.Errorf("%w: %v", ErrLoadVerifier, err) // nolint:errorlint
 	}
 	return v, nil
 }
@@ -156,7 +156,7 @@ func getKeyPairSecret(ctx context.Context, k8sRef string, k8s kubernetes.Interfa
 
 	s, err := k8s.CoreV1().Secrets(namespace).Get(ctx, name, metav1.GetOptions{})
 	if err != nil {
-		return nil, fmt.Errorf("%w: %v", ErrSecretNotFound, err) // nolint -- needs errorlint cleanup
+		return nil, fmt.Errorf("%w: %v", ErrSecretNotFound, err) // nolint:errorlint
 	}
 
 	return s, nil

diff --git a/pkg/trustedresources/verify.go b/pkg/trustedresources/verify.go
@@ -119,7 +119,8 @@ func getMatchedPolicies(resourceName string, source string, policies []*v1alpha1
 		for _, r := range p.Spec.Resources {
 			matching, err := regexp.MatchString(r.Pattern, source)
 			if err != nil {
-				return matchedPolicies, fmt.Errorf("%v: %w", err, ErrRegexMatch) // nolint -- needs errorlint cleanup
+				// FixMe: changing %v to %w breaks integration tests.
+				return matchedPolicies, fmt.Errorf("%v: %w", err, ErrRegexMatch) // nolint:errorlint
 			}
 			if matching {
 				matchedPolicies = append(matchedPolicies, p)
@@ -177,7 +178,8 @@ func verifyInterface(obj interface{}, verifier signature.Verifier, signature []b
 	h.Write(ts)
 
 	if err := verifier.VerifySignature(bytes.NewReader(signature), bytes.NewReader(h.Sum(nil))); err != nil {
-		return fmt.Errorf("%w:%v", ErrResourceVerificationFailed, err.Error()) // nolint -- needs errorlint cleanup
+		// FixMe: changing %v to %w breaks integration tests.
+		return fmt.Errorf("%w:%v", ErrResourceVerificationFailed, err.Error()) // nolint:errorlint
 	}
 
 	return nil