Hygiene: enable linters for error conventions.

There are no expected functional changes in this PR.

Made non-functional code changes to adhere to linter standards.

Context: #5899

See related style docs:
- https://github.com/golang/go/wiki/Errors#naming
- https://google.github.io/styleguide/go/guide#concision
- https://go.dev/blog/errors-are-values

.golangci.yml

@@ -25,6 +25,9 @@ linters:
   - dogsled
   - dupword
   - errcheck
+  - errchkjson
+  - errname
+  # errorlint TODO: cleanup the issues this linter uncovers https://github.com/tektoncd/pipeline/issues/5899
   - goconst
   - gocritic
   - gofmt
@@ -89,6 +92,9 @@ issues:
   - path: test/controller.go
     linters:
     - containedctx
+  - path: internal/sidecarlogresults/sidecarlogresults_test\.go
+    linters:
+    - errchkjson
   max-issues-per-linter: 0
   max-same-issues: 0
   include:

cmd/entrypoint/main.go

@@ -98,7 +98,7 @@ func main() {
 	if err := subcommands.Process(flag.CommandLine.Args()); err != nil {
 		log.Println(err.Error())
 		switch err.(type) {
-		case subcommands.SubcommandSuccessful:
+		case subcommands.OK:
 			return
 		default:
 			os.Exit(1)

cmd/entrypoint/subcommands/subcommands.go

@@ -20,15 +20,28 @@ import (
 	"fmt"
 )
 
-// SubcommandSuccessful is returned for successful subcommand executions.
-type SubcommandSuccessful struct {
+// OK is returned for successful subcommand executions.
+type OK struct {
 	message string
 }
 
-func (err SubcommandSuccessful) Error() string {
+func (err OK) Error() string {
 	return err.message
 }
 
+// SubcommandSuccessful is an alias for the OK type.
+//
+// Deprecated: replace usage with OK type.
+type SubcommandSuccessful = OK
+
+var (
+	// Compile-time check that OK is an error type.
+	_ error = OK{}
+	// Compile-time check that objects of type OK are cast to deprecated
+	// SubcommandSuccessful type.
+	_ SubcommandSuccessful = OK{}
+)
+
 // SubcommandError is returned for failed subcommand executions.
 type SubcommandError struct {
 	subcommand string
@@ -43,7 +56,7 @@ func (err SubcommandError) Error() string {
 // subcommand that the args call for. An error is returned to the caller to
 // indicate that a subcommand was matched and to pass back its success/fail
 // state. The returned error will be nil if no subcommand was matched to the
-// passed args, SubcommandSuccessful if args matched and the subcommand
+// passed args, OK if args matched and the subcommand
 // succeeded, or any other error if the args matched but the subcommand failed.
 func Process(args []string) error {
 	if len(args) == 0 {
@@ -60,7 +73,7 @@ func Process(args []string) error {
 			if err := entrypointInit(src, dst, steps); err != nil {
 				return SubcommandError{subcommand: InitCommand, message: err.Error()}
 			}
-			return SubcommandSuccessful{message: "Entrypoint initialization"}
+			return OK{message: "Entrypoint initialization"}
 		}
 	case CopyCommand:
 		// If invoked in "cp mode" (`entrypoint cp <src> <dst>`), simply copy
@@ -72,7 +85,7 @@ func Process(args []string) error {
 			if err := cp(src, dst); err != nil {
 				return SubcommandError{subcommand: CopyCommand, message: err.Error()}
 			}
-			return SubcommandSuccessful{message: fmt.Sprintf("Copied %s to %s", src, dst)}
+			return OK{message: fmt.Sprintf("Copied %s to %s", src, dst)}
 		}
 	case DecodeScriptCommand:
 		// If invoked in "decode-script" mode (`entrypoint decode-script <src>`),
@@ -82,13 +95,13 @@ func Process(args []string) error {
 			if err := decodeScript(src); err != nil {
 				return SubcommandError{subcommand: DecodeScriptCommand, message: err.Error()}
 			}
-			return SubcommandSuccessful{message: fmt.Sprintf("Decoded script %s", src)}
+			return OK{message: fmt.Sprintf("Decoded script %s", src)}
 		}
 	case StepInitCommand:
 		if err := stepInit(args[1:]); err != nil {
 			return SubcommandError{subcommand: StepInitCommand, message: err.Error()}
 		}
-		return SubcommandSuccessful{message: "Setup /step directories"}
+		return OK{message: "Setup /step directories"}
 	default:
 	}
 	return nil

cmd/entrypoint/subcommands/subcommands_test.go

@@ -39,7 +39,7 @@ func TestProcessSuccessfulSubcommands(t *testing.T) {
 	} {
 		t.Run(tc.command, func(t *testing.T) {
 			returnValue := Process(append([]string{tc.command}, tc.args...))
-			if _, ok := returnValue.(SubcommandSuccessful); !ok {
+			if _, ok := returnValue.(OK); !ok {
 				t.Errorf("unexpected return value from command: %v", returnValue)
 			}
 		})
@@ -49,12 +49,12 @@ func TestProcessSuccessfulSubcommands(t *testing.T) {
 		tektonRoot = tmp
 
 		returnValue := Process([]string{StepInitCommand})
-		if _, ok := returnValue.(SubcommandSuccessful); !ok {
+		if _, ok := returnValue.(OK); !ok {
 			t.Errorf("unexpected return value from step-init command: %v", returnValue)
 		}
 
 		returnValue = Process([]string{StepInitCommand, "foo", "bar"})
-		if _, ok := returnValue.(SubcommandSuccessful); !ok {
+		if _, ok := returnValue.(OK); !ok {
 			t.Errorf("unexpected return value from step-init command w/ params: %v", returnValue)
 		}
 	})

pkg/pipelinerunmetrics/metrics.go

@@ -81,9 +81,9 @@ type Recorder struct {
 // initializes this singleton and returns the same recorder across any
 // subsequent invocations.
 var (
-	once        sync.Once
-	r           *Recorder
-	recorderErr error
+	once           sync.Once
+	r              *Recorder
+	errRegistering error
 )
 
 // NewRecorder creates a new metrics recorder instance
@@ -98,14 +98,14 @@ func NewRecorder(ctx context.Context) (*Recorder, error) {
 		}
 
 		cfg := config.FromContextOrDefaults(ctx)
-		recorderErr = viewRegister(cfg.Metrics)
-		if recorderErr != nil {
+		errRegistering = viewRegister(cfg.Metrics)
+		if errRegistering != nil {
 			r.initialized = false
 			return
 		}
 	})
 
-	return r, recorderErr
+	return r, errRegistering
 }
 
 func viewRegister(cfg *config.Metrics) error {

pkg/pipelinerunmetrics/metrics_test.go

@@ -381,5 +381,5 @@ func unregisterMetrics() {
 	// Allow the recorder singleton to be recreated.
 	once = sync.Once{}
 	r = nil
-	recorderErr = nil
+	errRegistering = nil
 }

pkg/reconciler/pipelinerun/pipelinerun.go

@@ -357,10 +357,10 @@ func (c *Reconciler) resolvePipelineState(
 			if tresources.IsGetTaskErrTransient(err) {
 				return nil, err
 			}
-			if errors.Is(err, remote.ErrorRequestInProgress) {
+			if errors.Is(err, remote.ErrRequestInProgress) {
 				return nil, err
 			}
-			if errors.Is(err, trustedresources.ErrorResourceVerificationFailed) {
+			if errors.Is(err, trustedresources.ErrResourceVerificationFailed) {
 				message := fmt.Sprintf("PipelineRun %s/%s referred task %s failed signature verification", pr.Namespace, pr.Name, task.Name)
 				pr.Status.MarkFailed(ReasonResourceVerificationFailed, message)
 				return nil, controller.NewPermanentError(err)
@@ -397,11 +397,11 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get
 
 	pipelineMeta, pipelineSpec, err := rprp.GetPipelineData(ctx, pr, getPipelineFunc)
 	switch {
-	case errors.Is(err, remote.ErrorRequestInProgress):
+	case errors.Is(err, remote.ErrRequestInProgress):
 		message := fmt.Sprintf("PipelineRun %s/%s awaiting remote resource", pr.Namespace, pr.Name)
 		pr.Status.MarkRunning(ReasonResolvingPipelineRef, message)
 		return nil
-	case errors.Is(err, trustedresources.ErrorResourceVerificationFailed):
+	case errors.Is(err, trustedresources.ErrResourceVerificationFailed):
 		message := fmt.Sprintf("PipelineRun %s/%s referred pipeline failed signature verification", pr.Namespace, pr.Name)
 		pr.Status.MarkFailed(ReasonResourceVerificationFailed, message)
 		return controller.NewPermanentError(err)
@@ -521,7 +521,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get
 	}
 	pipelineRunState, err := c.resolvePipelineState(ctx, tasks, pipelineMeta.ObjectMeta, pr)
 	switch {
-	case errors.Is(err, remote.ErrorRequestInProgress):
+	case errors.Is(err, remote.ErrRequestInProgress):
 		message := fmt.Sprintf("PipelineRun %s/%s awaiting remote resource", pr.Namespace, pr.Name)
 		pr.Status.MarkRunning(v1beta1.TaskRunReasonResolvingTaskRef, message)
 		return nil

pkg/reconciler/pipelinerun/resources/pipelineref.go

@@ -122,7 +122,7 @@ func GetVerifiedPipelineFunc(ctx context.Context, k8s kubernetes.Interface, tekt
 			if err := trustedresources.VerifyPipeline(ctx, p, k8s, source, verificationpolicies); err != nil {
 				if config.CheckEnforceResourceVerificationMode(ctx) {
 					logger.Errorf("GetVerifiedPipelineFunc failed: %v", err)
-					return nil, nil, fmt.Errorf("GetVerifiedPipelineFunc failed: %w: %v", trustedresources.ErrorResourceVerificationFailed, err)
+					return nil, nil, fmt.Errorf("GetVerifiedPipelineFunc failed: %w: %v", trustedresources.ErrResourceVerificationFailed, err)
 				}
 				logger.Warnf("GetVerifiedPipelineFunc failed: %v", err)
 				return p, s, nil

pkg/reconciler/pipelinerun/resources/pipelineref_test.go

@@ -651,13 +651,13 @@ func TestGetVerifiedPipelineFunc_VerifyError(t *testing.T) {
 			requester:                requesterUnsigned,
 			resourceVerificationMode: config.EnforceResourceVerificationMode,
 			expected:                 nil,
-			expectedErr:              trustedresources.ErrorResourceVerificationFailed,
+			expectedErr:              trustedresources.ErrResourceVerificationFailed,
 		}, {
 			name:                     "tampered pipeline with enforce policy",
 			requester:                requesterTampered,
 			resourceVerificationMode: config.EnforceResourceVerificationMode,
 			expected:                 nil,
-			expectedErr:              trustedresources.ErrorResourceVerificationFailed,
+			expectedErr:              trustedresources.ErrResourceVerificationFailed,
 		},
 	}
 	for _, tc := range testcases {

pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go

@@ -714,9 +714,9 @@ func resolveTask(
 			// Instead, the child TaskRun's status will be the place recording the source of individual task.
 			t, _, err = getTask(ctx, pipelineTask.TaskRef.Name)
 			switch {
-			case errors.Is(err, remote.ErrorRequestInProgress):
+			case errors.Is(err, remote.ErrRequestInProgress):
 				return v1beta1.TaskSpec{}, "", "", err
-			case errors.Is(err, trustedresources.ErrorResourceVerificationFailed):
+			case errors.Is(err, trustedresources.ErrResourceVerificationFailed):
 				return v1beta1.TaskSpec{}, "", "", err
 			case err != nil:
 				return v1beta1.TaskSpec{}, "", "", &TaskNotFoundError{

pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go

@@ -1963,7 +1963,7 @@ func TestResolvePipelineRun_VerificationFailed(t *testing.T) {
 		}}}
 
 	getTask := func(ctx context.Context, name string) (v1beta1.TaskObject, *v1beta1.ConfigSource, error) {
-		return nil, nil, trustedresources.ErrorResourceVerificationFailed
+		return nil, nil, trustedresources.ErrResourceVerificationFailed
 	}
 	getTaskRun := func(name string) (*v1beta1.TaskRun, error) { return nil, nil }
 	pr := v1beta1.PipelineRun{
@@ -1976,8 +1976,8 @@ func TestResolvePipelineRun_VerificationFailed(t *testing.T) {
 		if err == nil {
 			t.Errorf("expected to get err but got nil")
 		}
-		if err != trustedresources.ErrorResourceVerificationFailed {
-			t.Errorf("expected to get %v but got %v", trustedresources.ErrorResourceVerificationFailed, err)
+		if err != trustedresources.ErrResourceVerificationFailed {
+			t.Errorf("expected to get %v but got %v", trustedresources.ErrResourceVerificationFailed, err)
 		}
 	}
 }

pkg/reconciler/taskrun/resources/taskref.go

@@ -100,7 +100,7 @@ func GetVerifiedTaskFunc(ctx context.Context, k8s kubernetes.Interface, tekton c
 			if err := trustedresources.VerifyTask(ctx, t, k8s, source, verificationpolicies); err != nil {
 				if config.CheckEnforceResourceVerificationMode(ctx) {
 					logger.Errorf("GetVerifiedTaskFunc failed: %v", err)
-					return nil, nil, fmt.Errorf("GetVerifiedTaskFunc failed: %w: %v", trustedresources.ErrorResourceVerificationFailed, err)
+					return nil, nil, fmt.Errorf("GetVerifiedTaskFunc failed: %w: %v", trustedresources.ErrResourceVerificationFailed, err)
 				}
 				logger.Warnf("GetVerifiedTaskFunc failed: %v", err)
 				return t, s, nil

pkg/reconciler/taskrun/resources/taskref_test.go

@@ -857,13 +857,13 @@ func TestGetVerifiedTaskFunc_VerifyError(t *testing.T) {
 		requester:                requesterUnsigned,
 		resourceVerificationMode: config.EnforceResourceVerificationMode,
 		expected:                 nil,
-		expectedErr:              trustedresources.ErrorResourceVerificationFailed,
+		expectedErr:              trustedresources.ErrResourceVerificationFailed,
 	}, {
 		name:                     "tampered task with enforce policy",
 		requester:                requesterTampered,
 		resourceVerificationMode: config.EnforceResourceVerificationMode,
 		expected:                 nil,
-		expectedErr:              trustedresources.ErrorResourceVerificationFailed,
+		expectedErr:              trustedresources.ErrResourceVerificationFailed,
 	},
 	}
 	for _, tc := range testcases {

pkg/reconciler/taskrun/taskrun.go

@@ -345,11 +345,11 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1beta1.TaskRun) (*v1beta1
 
 	taskMeta, taskSpec, err := resources.GetTaskData(ctx, tr, getTaskfunc)
 	switch {
-	case errors.Is(err, remote.ErrorRequestInProgress):
+	case errors.Is(err, remote.ErrRequestInProgress):
 		message := fmt.Sprintf("TaskRun %s/%s awaiting remote resource", tr.Namespace, tr.Name)
 		tr.Status.MarkResourceOngoing(v1beta1.TaskRunReasonResolvingTaskRef, message)
 		return nil, nil, err
-	case errors.Is(err, trustedresources.ErrorResourceVerificationFailed):
+	case errors.Is(err, trustedresources.ErrResourceVerificationFailed):
 		logger.Errorf("TaskRun %s/%s referred task failed signature verification", tr.Namespace, tr.Name)
 		tr.Status.MarkResourceFailed(podconvert.ReasonResourceVerificationFailed, err)
 		return nil, nil, controller.NewPermanentError(err)

pkg/reconciler/taskrun/taskrun_test.go

@@ -4974,12 +4974,12 @@ status:
 		{
 			name:          "unsigned task fails verification",
 			task:          []*v1beta1.Task{ts},
-			expectedError: trustedresources.ErrorResourceVerificationFailed,
+			expectedError: trustedresources.ErrResourceVerificationFailed,
 		},
 		{
 			name:          "modified task fails verification",
 			task:          []*v1beta1.Task{tamperedTask},
-			expectedError: trustedresources.ErrorResourceVerificationFailed,
+			expectedError: trustedresources.ErrResourceVerificationFailed,
 		},
 	}
 

pkg/remote/errors.go

@@ -17,7 +17,15 @@ package remote
 
 import "errors"
 
-// ErrorRequestInProgress is a sentinel value that indicates
-// resolving a remote file like a pipeline in a bundle or
-// a task in git hasn't completed yet.
-var ErrorRequestInProgress = errors.New("resource request in progress")
+var (
+	// ErrRequestInProgress is a sentinel value that indicates
+	// resolving a remote file like a pipeline in a bundle or
+	// a task in git hasn't completed yet.
+	ErrRequestInProgress = errors.New("resource request in progress")
+
+	// ErrorRequestInProgress is an alias for ErrRequestInProgress and will be
+	// removed in a future release..
+	//
+	// Deprecated: use ErrRequestInProgress instead
+	ErrorRequestInProgress = ErrRequestInProgress
+)