Hygiene: enabled presets and various linters.

.golangci.yml

@@ -37,20 +37,62 @@ linters:
   - gosec
   - gosimple
   - govet
+  - maintidx
+  - makezero
   - misspell
+  - musttag
+  - nakedret
+  - nilerr
+  - nilnil
+  - noctx
+  - nolintlint
+  - nosprintfhostport
   - thelper
   - typecheck
   - unconvert
   - unused
   - usestdlibvars
   - whitespace
   disable:
+  - cyclop
+  - dupl
+  - exhaustruct
+  - forcetypeassert
+  - funlen
+  - gci
+  - gochecknoglobals
+  - gochecknoinits
+  - gocognit
+  - gocyclo
+  - godot
+  - godox
+  - goerr113
+  - gofumpt
+  - gomnd
+  - gomoddirectives
+  - ireturn
+  - lll
+  - nestif
+  - nlreturn
+  - nonamedreturns
+  - paralleltest
+  - prealloc
+  - predeclared
   - revive
   - staticcheck
-# Enabling presets means that new linters that we automatically adopt new
-# linters that augment a preset. This also opts us in for replacement linters
-# when a linter is deprecated.
-presets:
+  - stylecheck
+  - tagliatelle
+  - testpackage
+  - tparallel
+  - unparam
+  - varnamelen
+  - wastedassign
+  - wrapcheck
+  - wsl
+  # Enabling presets means that new linters that we automatically adopt new
+  # linters that augment a preset. This also opts us in for replacement linters
+  # when a linter is deprecated.
+  presets:
   - bugs
   - comment
   - complexity
@@ -67,41 +109,71 @@ presets:
 output:
   uniq-by-line: false
 issues:
+  # Note: path identifiers are regular expressions, hence the \.go suffixes.
   exclude-rules:
+  - path: main\.go
+    linters:
+    - forbidigo
+  - path: test/build_logs\.go
+    linters:
+    - typecheck
   - path: _test\.go
     linters:
-    - goconst
     - dogsled
     - errcheck
+    - goconst
     - gosec
     - ineffassign
+    - maintidx
+    - typecheck
   - path: test/pipelinerun_test\.go
     linters:
     - unused
-  - path: pkg/apis/config/feature_flags_test.go
+  - path: pkg/apis/config/feature_flags_test\.go
     linters:
     - containedctx
-  - path: pkg/pipelinerunmetrics/injection.go
+  - path: pkg/pipelinerunmetrics/injection\.go
     linters:
     - containedctx
-  - path: pkg/pod/creds_init_test.go
+  - path: pkg/pod/pod\.go
+    linters:
+    - maintidx
+  - path: pkg/pod/creds_init_test\.go
     linters:
     - containedctx
-  - path: pkg/taskrunmetrics/injection.go
+  - path: pkg/reconciler/pipelinerun/pipelinerun\.go
+    linters:
+    - maintidx
+  - path: pkg/taskrunmetrics/injection\.go
     linters:
     - containedctx
-  - path: test/controller.go
+  - path: test/controller\.go
     linters:
     - containedctx
+  - path: internal/sidecarlogresults/sidecarlogresults\.go
+    linters:
+    - musttag
   - path: internal/sidecarlogresults/sidecarlogresults_test\.go
     linters:
     - errchkjson
+  - path: pkg/apis/pipeline/v1.*/param_types\.go
+    linters:
+    - musttag
+  - path: pkg/resolution/resolver/framework/testing/fakecontroller\.go
+    linters:
+    - contextcheck
+  - path: pkg/pipelinerunmetrics/metrics\.go
+    linters:
+    - contextcheck
+  - path: pkg/reconciler/pipelinerun/pipelinerun\.go
+    linters:
+    - contextcheck
   max-issues-per-linter: 0
   max-same-issues: 0
   include:
   # Enable off-by-default rules for revive requiring that all exported elements have a properly formatted comment.
-  - EXC0012
-  - EXC0014
+  - EXC0012 # https://golangci-lint.run/usage/false-positives/#exc0012
+  - EXC0014 # https://golangci-lint.run/usage/false-positives/#exc0014
 run:
   issues-exit-code: 1
   build-tags:
@@ -113,5 +185,5 @@ run:
   - vendor
   - pkg/client
   - pkg/spire/test
-  timeout: 10m
+  timeout: 20m
   modules-download-mode: vendor

cmd/entrypoint/main.go

@@ -174,7 +174,7 @@ func main() {
 
 	if err := e.Go(); err != nil {
 		breakpointExitPostFile := e.PostFile + breakpointExitSuffix
-		switch t := err.(type) { // nolint -- checking for multiple types with errors.As is ugly.
+		switch t := err.(type) { //nolint:errorlint // checking for multiple types with errors.As is ugly.
 		case skipError:
 			log.Print("Skipping step because a previous step failed")
 			os.Exit(1)

cmd/webhook/main.go

@@ -150,7 +150,6 @@ func newConfigValidationController(name string) func(context.Context, configmap.
 }
 
 func newConversionController(ctx context.Context, cmw configmap.Watcher) *controller.Impl {
-	// nolint: revive
 	var (
 		v1beta1GroupVersion            = v1beta1.SchemeGroupVersion.Version
 		v1GroupVersion                 = v1.SchemeGroupVersion.Version

pkg/apis/config/feature_flags.go

@@ -81,7 +81,7 @@ const (
 	disableCredsInitKey                 = "disable-creds-init"
 	runningInEnvWithInjectedSidecarsKey = "running-in-environment-with-injected-sidecars"
 	awaitSidecarReadinessKey            = "await-sidecar-readiness"
-	requireGitSSHSecretKnownHostsKey    = "require-git-ssh-secret-known-hosts" // nolint: gosec
+	requireGitSSHSecretKnownHostsKey    = "require-git-ssh-secret-known-hosts" //nolint:gosec
 	enableTektonOCIBundles              = "enable-tekton-oci-bundles"
 	enableAPIFields                     = "enable-api-fields"
 	sendCloudEventsForRuns              = "send-cloudevents-for-runs"
@@ -94,6 +94,8 @@ const (
 
 // FeatureFlags holds the features configurations
 // +k8s:deepcopy-gen=true
+//
+//nolint:musttag
 type FeatureFlags struct {
 	DisableAffinityAssistant         bool
 	DisableCredsInit                 bool

pkg/apis/pipeline/controller.go

@@ -18,11 +18,9 @@ package pipeline
 
 const (
 	// PipelineRunControllerName holds the name of the PipelineRun controller
-	// nolint: revive
 	PipelineRunControllerName = "PipelineRun"
 
 	// PipelineControllerName holds the name of the Pipeline controller
-	// nolint: revive
 	PipelineControllerName = "Pipeline"
 
 	// TaskRunControllerName holds the name of the TaskRun controller

pkg/apis/pipeline/v1/taskrun_types.go

@@ -415,7 +415,7 @@ func (tr *TaskRun) GetTimeout(ctx context.Context) time.Duration {
 	// Use the platform default is no timeout is set
 	if tr.Spec.Timeout == nil {
 		defaultTimeout := time.Duration(config.FromContextOrDefaults(ctx).Defaults.DefaultTimeoutMinutes)
-		return defaultTimeout * time.Minute
+		return defaultTimeout * time.Minute //nolint:durationcheck
 	}
 	return tr.Spec.Timeout.Duration
 }

pkg/apis/pipeline/v1beta1/pipelineref_validation.go

@@ -64,7 +64,7 @@ func (ref *PipelineRef) Validate(ctx context.Context) (errs *apis.FieldError) {
 			}
 		}
 	}
-	return
+	return //nolint:nakedret
 }
 
 func validateBundleFeatureFlag(ctx context.Context, featureName string, wantValue bool) *apis.FieldError {

pkg/apis/pipeline/v1beta1/taskref_validation.go

@@ -62,5 +62,5 @@ func (ref *TaskRef) Validate(ctx context.Context) (errs *apis.FieldError) {
 			}
 		}
 	}
-	return
+	return //nolint:nakedret
 }

pkg/apis/pipeline/v1beta1/taskrun_types.go

@@ -494,7 +494,7 @@ func (tr *TaskRun) GetTimeout(ctx context.Context) time.Duration {
 	// Use the platform default is no timeout is set
 	if tr.Spec.Timeout == nil {
 		defaultTimeout := time.Duration(config.FromContextOrDefaults(ctx).Defaults.DefaultTimeoutMinutes)
-		return defaultTimeout * time.Minute
+		return defaultTimeout * time.Minute //nolint:durationcheck
 	}
 	return tr.Spec.Timeout.Duration
 }

pkg/credentials/initialize.go

@@ -83,7 +83,7 @@ func SortAnnotations(secrets map[string]string, annotationPrefix string) []strin
 // /tekton/creds directory is not considered an error.
 func CopyCredsToHome(credPaths []string) error {
 	if info, err := os.Stat(pipeline.CredsDir); err != nil || !info.IsDir() {
-		return nil
+		return nil //nolint:nilerr // safe to ignore error; no credentials available to copy
 	}
 
 	homepath, err := homedir.Dir()

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:gocritic
+			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:gocritic
+			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:gocritic
+			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:gocritic
+		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:gocritic
+			vms := append(s.VolumeMounts, toAdd...) //nolint:gocritic
 			sidecarContainers[i].VolumeMounts = vms
 		}
 	}

pkg/pod/status.go

@@ -335,7 +335,7 @@ func extractStartedAtTimeFromResults(results []result.RunResult) (*metav1.Time,
 			return &startedAt, nil
 		}
 	}
-	return nil, nil
+	return nil, nil //nolint:nilnil // would be more ergonomic to return a sentinel error
 }
 
 func extractExitCodeFromResults(results []result.RunResult) (*int32, error) {
@@ -350,7 +350,7 @@ func extractExitCodeFromResults(results []result.RunResult) (*int32, error) {
 			return &exitCode, nil
 		}
 	}
-	return nil, nil
+	return nil, nil //nolint:nilnil // would be more ergonomic to return a sentinel error
 }
 
 func updateCompletedTaskRunStatus(logger *zap.SugaredLogger, trs *v1beta1.TaskRunStatus, pod *corev1.Pod) {

pkg/reconciler/pipelinerun/resources/pipelineref.go

@@ -119,7 +119,7 @@ func GetVerifiedPipelineFunc(ctx context.Context, k8s kubernetes.Interface, tekt
 		if err := trustedresources.VerifyPipeline(ctx, p, k8s, refSource, verificationpolicies); err != nil {
 			// 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 nil, nil, fmt.Errorf("GetVerifiedPipelineFunc failed: %w: %v", trustedresources.ErrResourceVerificationFailed, err) //nolint:errorlint
 		}
 		return p, s, nil
 	}

pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go

@@ -1944,7 +1944,7 @@ func TestResolvePipelineRun_VerificationFailed(t *testing.T) {
 	getTask := func(ctx context.Context, name string) (*v1beta1.Task, *v1beta1.RefSource, error) {
 		return nil, nil, trustedresources.ErrResourceVerificationFailed
 	}
-	getTaskRun := func(name string) (*v1beta1.TaskRun, error) { return nil, nil }
+	getTaskRun := func(name string) (*v1beta1.TaskRun, error) { return nil, nil } //nolint:nilnil
 	pr := v1beta1.PipelineRun{
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "pipelinerun",
@@ -2214,7 +2214,7 @@ func TestIsCustomTask(t *testing.T) {
 	getTask := func(ctx context.Context, name string) (*v1beta1.Task, *v1beta1.RefSource, error) {
 		return task, nil, nil
 	}
-	getTaskRun := func(name string) (*v1beta1.TaskRun, error) { return nil, nil }
+	getTaskRun := func(name string) (*v1beta1.TaskRun, error) { return nil, nil } //nolint:nilnil
 	getRun := func(name string) (v1beta1.RunObject, error) { return nil, nil }
 
 	for _, tc := range []struct {

pkg/reconciler/taskrun/resources/taskref.go

@@ -95,7 +95,7 @@ func GetVerifiedTaskFunc(ctx context.Context, k8s kubernetes.Interface, tekton c
 			refSource = s.URI
 		}
 		if err := trustedresources.VerifyTask(ctx, t, k8s, refSource, verificationpolicies); err != nil {
-			return nil, nil, fmt.Errorf("GetVerifiedTaskFunc failed: %w: %v", trustedresources.ErrResourceVerificationFailed, err) // nolint:errorlint
+			return nil, nil, fmt.Errorf("GetVerifiedTaskFunc failed: %w: %v", trustedresources.ErrResourceVerificationFailed, err) //nolint:errorlint
 		}
 		return t, s, nil
 	}

pkg/resolution/resolver/hub/resolver.go

@@ -132,7 +132,7 @@ func (r *Resolver) Resolve(ctx context.Context, params []pipelinev1beta1.Param)
 	case ArtifactHubType:
 		url := fmt.Sprintf(r.ArtifactHubURL, paramsMap[ParamKind], paramsMap[ParamCatalog], paramsMap[ParamName], paramsMap[ParamVersion])
 		resp := artifactHubResponse{}
-		if err := fetchHubResource(url, &resp); err != nil {
+		if err := fetchHubResource(ctx, url, &resp); err != nil {
 			return nil, fmt.Errorf("fail to fetch Artifact Hub resource: %w", err)
 		}
 		return &ResolvedHubResource{
@@ -142,7 +142,7 @@ func (r *Resolver) Resolve(ctx context.Context, params []pipelinev1beta1.Param)
 	case TektonHubType:
 		url := fmt.Sprintf(r.TektonHubURL, paramsMap[ParamCatalog], paramsMap[ParamKind], paramsMap[ParamName], paramsMap[ParamVersion])
 		resp := tektonHubResponse{}
-		if err := fetchHubResource(url, &resp); err != nil {
+		if err := fetchHubResource(ctx, url, &resp); err != nil {
 			return nil, fmt.Errorf("fail to fetch Tekton Hub resource: %w", err)
 		}
 		return &ResolvedHubResource{
@@ -192,11 +192,16 @@ func (r *Resolver) isDisabled(ctx context.Context) bool {
 	return !cfg.FeatureFlags.EnableHubResolver
 }
 
-func fetchHubResource(apiEndpoint string, v interface{}) error {
+func fetchHubResource(ctx context.Context, apiEndpoint string, v interface{}) error {
 	// #nosec G107 -- URL cannot be constant in this case.
-	resp, err := http.Get(apiEndpoint)
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, apiEndpoint, nil)
 	if err != nil {
-		return fmt.Errorf("error requesting resource from Hub: %w", err)
+		return fmt.Errorf("constructing request: %w", err)
+	}
+
+	resp, err := http.DefaultClient.Do(req)
+	if err != nil {
+		return fmt.Errorf("requesting resource from Hub: %w", err)
 	}
 	if resp.StatusCode != http.StatusOK {
 		return fmt.Errorf("requested resource '%s' not found on hub", apiEndpoint)

pkg/result/result.go

@@ -51,7 +51,8 @@ type RunResult struct {
 // ResultType used to find out whether a RunResult is from a task result or not
 // Note that ResultsType is another type which is used to define the data type
 // (e.g. string, array, etc) we used for Results
-// nolint:revive // revive complains about stutter of `result.ResultType`.
+//
+//nolint:revive // revive complains about stutter of `result.ResultType`.
 type ResultType int
 
 // UnmarshalJSON unmarshals either an int or a string into a ResultType. String

pkg/spire/verify.go

@@ -339,7 +339,7 @@ func getResultValue(result result.RunResult) (string, error) {
 		valList = append(valList, aos.ArrayVal...)
 		return strings.Join(valList, ","), nil
 	case v1beta1.ParamTypeObject:
-		keys := make([]string, len(aos.ObjectVal))
+		keys := make([]string, 0, len(aos.ObjectVal))
 		for k := range aos.ObjectVal {
 			keys = append(keys, k)
 		}