From e47877c5f252b223402ed19a389e4af23b29fc4b Mon Sep 17 00:00:00 2001 From: David Bendory Date: Fri, 31 Mar 2023 08:26:46 -0400 Subject: [PATCH] Cleanup: addressed various `nolint` items. - Addressed newly identified `errorlint` items. - Narrowed `nolint` --> `nolint:gocritic`. - Removed 2 `nolint` comments by addressing identified issues. There are no expected functional changes in this PR. Context: https://github.com/tektoncd/pipeline/issues/5899 /kind cleanup --- pkg/pod/pod.go | 10 +++++----- pkg/reconciler/pipelinerun/pipelinerun_test.go | 2 +- pkg/reconciler/pipelinerun/resources/pipelineref.go | 2 +- pkg/reconciler/taskrun/resources/taskref.go | 2 +- pkg/trustedresources/verifier/verifier.go | 8 ++++---- pkg/trustedresources/verify.go | 5 +++-- 6 files changed, 15 insertions(+), 14 deletions(-) diff --git a/pkg/pod/pod.go b/pkg/pod/pod.go index 820aac62663..3bdd2fc49a6 100644 --- 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,7 +235,7 @@ 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 } } @@ -243,7 +243,7 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec 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 index 34972d45a91..4c1aa1541f5 100644 --- 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 { diff --git a/pkg/reconciler/pipelinerun/resources/pipelineref.go b/pkg/reconciler/pipelinerun/resources/pipelineref.go index e1ab2444283..ec4145a49ed 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelineref.go +++ b/pkg/reconciler/pipelinerun/resources/pipelineref.go @@ -117,7 +117,7 @@ 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) + return nil, nil, fmt.Errorf("GetVerifiedPipelineFunc failed: %w: %w", trustedresources.ErrResourceVerificationFailed, err) } return p, s, nil } diff --git a/pkg/reconciler/taskrun/resources/taskref.go b/pkg/reconciler/taskrun/resources/taskref.go index ebd36d0b21a..e241f927716 100644 --- 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: %w", trustedresources.ErrResourceVerificationFailed, err) } return t, s, nil } diff --git a/pkg/trustedresources/verifier/verifier.go b/pkg/trustedresources/verifier/verifier.go index db24b5df845..271ef484eb0 100644 --- 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: %w", ErrFailedLoadKeyFile, err) } 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: %w", ErrDecodeKey, err) } 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: %w", ErrLoadVerifier, err) } 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: %w", ErrSecretNotFound, err) } return s, nil diff --git a/pkg/trustedresources/verify.go b/pkg/trustedresources/verify.go index f09bf9c977b..915602fa1d9 100644 --- a/pkg/trustedresources/verify.go +++ b/pkg/trustedresources/verify.go @@ -119,7 +119,7 @@ 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 + return matchedPolicies, fmt.Errorf("%w: %w", err, ErrRegexMatch) } if matching { matchedPolicies = append(matchedPolicies, p) @@ -177,7 +177,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 this %v to a %w results in failed integration tests. + return fmt.Errorf("%w:%v", ErrResourceVerificationFailed, err.Error()) // nolint:errorlint } return nil