Hygiene: enabled presets and various linters.

Explicitly enabled:
- maintidx
- makezero
- mustag
- nakedret
- nilerr
- nilnil
- noctx
- nolintlint
- nosprintfhostport

Implicitly enabled: all other default linters in the presets.

- increased timeout from 10m --> 20m to allow time for additional tests
- fixed `presets` configuration, which wasn't formated properly and
  did not in fact activate the presets
- explicitly disabled some linters (enabled by `presets`) that highlight
  too many issues to resolve in this PR
- fixed host-port issues identified by `nosprintfhostport`
- fixed issues in step_output_test.go found by `exportloopref`
- removed defunct `nolint:revive` pragmas
- fixed format of `//nolint` directives in conformance with `nolintlint`
- added needed `//nolint` directives.

Author: bendory

.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()