diff --git a/pkg/apis/pipeline/errors/errors.go b/pkg/apis/pipeline/errors/errors.go index a7dd38dad98..2e40ff08eb3 100644 --- a/pkg/apis/pipeline/errors/errors.go +++ b/pkg/apis/pipeline/errors/errors.go @@ -59,3 +59,12 @@ func LabelUserError(messageFormat string, messageA []interface{}) string { } return messageFormat } + +// LabelUserError returns the error message with the user error label if it is of type user +// error +func GetErrorMessage(err error) string { + if ue, ok := err.(*UserError); ok { + return ue.Reason + err.Error() + } + return err.Error() +} diff --git a/pkg/apis/pipeline/errors/errors_test.go b/pkg/apis/pipeline/errors/errors_test.go index 44ebdb51681..de7dd8fca46 100644 --- a/pkg/apis/pipeline/errors/errors_test.go +++ b/pkg/apis/pipeline/errors/errors_test.go @@ -76,12 +76,12 @@ func TestLabelsUserError(t *testing.T) { messages []interface{} expected string }{{ - description: "error messags with user error", + description: "error messages with user error", messageFormat: v1.PipelineRunReasonInvalidGraph.String(), messages: makeMessages(hasUserError), expected: "[User error] " + v1.PipelineRunReasonInvalidGraph.String(), }, { - description: "error messags without user error", + description: "error messages without user error", messages: makeMessages(!hasUserError), messageFormat: v1.PipelineRunReasonInvalidGraph.String(), expected: v1.PipelineRunReasonInvalidGraph.String(), @@ -96,3 +96,29 @@ func TestLabelsUserError(t *testing.T) { } } } + +func TestGetErrorMess(t *testing.T) { + original := errors.New("orignal error") + tcs := []struct { + description string + err error + expected string + }{{ + description: "error messages with user error", + err: pipelineErrors.WrapUserError(original), + expected: "[User error] " + original.Error(), + }, { + description: "error messages without user error", + err: original, + expected: original.Error(), + }} + for _, tc := range tcs { + { + msg := pipelineErrors.GetErrorMessage(tc.err) + + if msg != tc.expected { + t.Errorf("failure messageFormat expected: %s; but got %s", tc.expected, msg) + } + } + } +} diff --git a/pkg/apis/pipeline/v1/taskrun_types.go b/pkg/apis/pipeline/v1/taskrun_types.go index 7c3cf232ee5..ff94ed6c330 100644 --- a/pkg/apis/pipeline/v1/taskrun_types.go +++ b/pkg/apis/pipeline/v1/taskrun_types.go @@ -21,6 +21,7 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/config" apisconfig "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline" + pipelineErrors "github.com/tektoncd/pipeline/pkg/apis/pipeline/errors" pod "github.com/tektoncd/pipeline/pkg/apis/pipeline/pod" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -242,7 +243,7 @@ func (trs *TaskRunStatus) MarkResourceFailed(reason TaskRunReason, err error) { Type: apis.ConditionSucceeded, Status: corev1.ConditionFalse, Reason: reason.String(), - Message: err.Error(), + Message: pipelineErrors.GetErrorMessage(err), }) succeeded := trs.GetCondition(apis.ConditionSucceeded) trs.CompletionTime = &succeeded.LastTransitionTime.Inner diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index b05dfcc5e50..92e80c84767 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -196,7 +196,7 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, tr *v1.TaskRun) pkgrecon logger.Errorf("Reconcile: %v", err.Error()) if errors.Is(err, sidecarlogresults.ErrSizeExceeded) { cfg := config.FromContextOrDefaults(ctx) - message := fmt.Sprintf("TaskRun \"%q\" failed: results exceeded size limit %d bytes", tr.Name, cfg.FeatureFlags.MaxResultSize) + message := fmt.Sprintf("[User error] TaskRun \"%q\" failed: results exceeded size limit %d bytes", tr.Name, cfg.FeatureFlags.MaxResultSize) err := c.failTaskRun(ctx, tr, v1.TaskRunReasonResultLargerThanAllowedLimit, message) return c.finishReconcileUpdateEmitEvents(ctx, tr, before, err) } diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index c25b39c8cce..8493fa27a6a 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -1679,14 +1679,14 @@ status: - reason: ToBeRetried status: Unknown type: Succeeded - message: "Provided results don't match declared results; may be invalid JSON or missing result declaration: \"aResult\": task result is expected to be \"array\" type but was initialized to a different type \"string\"" + message: "[User error] Provided results don't match declared results; may be invalid JSON or missing result declaration: \"aResult\": task result is expected to be \"array\" type but was initialized to a different type \"string\"" sideCars: retriesStatus: - conditions: - reason: TaskRunValidationFailed status: "False" type: "Succeeded" - message: "Provided results don't match declared results; may be invalid JSON or missing result declaration: \"aResult\": task result is expected to be \"array\" type but was initialized to a different type \"string\"" + message: "[User error] Provided results don't match declared results; may be invalid JSON or missing result declaration: \"aResult\": task result is expected to be \"array\" type but was initialized to a different type \"string\"" startTime: "2021-12-31T23:59:59Z" completionTime: "2022-01-01T00:00:00Z" podName: "test-taskrun-results-type-mismatched-pod" diff --git a/pkg/reconciler/taskrun/validate_taskrun.go b/pkg/reconciler/taskrun/validate_taskrun.go index e7bcb94812c..9407108a4a5 100644 --- a/pkg/reconciler/taskrun/validate_taskrun.go +++ b/pkg/reconciler/taskrun/validate_taskrun.go @@ -23,6 +23,7 @@ import ( "strings" "github.com/hashicorp/go-multierror" + pipelineErrors "github.com/tektoncd/pipeline/pkg/apis/pipeline/errors" v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "github.com/tektoncd/pipeline/pkg/list" "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources" @@ -170,7 +171,7 @@ func ValidateResolvedTask(ctx context.Context, params []v1.Param, matrix *v1.Mat paramSpecs = rtr.TaskSpec.Params } if err := validateParams(ctx, paramSpecs, params, matrix.GetAllParams()); err != nil { - return fmt.Errorf("invalid input params for task %s: %w", rtr.TaskName, err) + return pipelineErrors.WrapUserError(fmt.Errorf("invalid input params for task %s: %w", rtr.TaskName, err)) } return nil } @@ -198,7 +199,7 @@ func ValidateEnumParam(ctx context.Context, params []v1.Param, paramSpecs v1.Par } if !slices.Contains(paramSpecNameToEnum[p.Name], p.Value.StringVal) { - return fmt.Errorf("param `%s` value: %s is not in the enum list", p.Name, p.Value.StringVal) + return pipelineErrors.WrapUserError(fmt.Errorf("param `%s` value: %s is not in the enum list", p.Name, p.Value.StringVal)) } } return nil @@ -211,13 +212,13 @@ func validateTaskSpecRequestResources(taskSpec *v1.TaskSpec) error { // First validate the limit in step if limit, ok := step.ComputeResources.Limits[k]; ok { if (&limit).Cmp(request) == -1 { - return fmt.Errorf("invalid request resource value: %v must be less or equal to limit %v", request.String(), limit.String()) + return pipelineErrors.WrapUserError(fmt.Errorf("invalid request resource value: %v must be less or equal to limit %v", request.String(), limit.String())) } } else if taskSpec.StepTemplate != nil { // If step doesn't configure the limit, validate the limit in stepTemplate if limit, ok := taskSpec.StepTemplate.ComputeResources.Limits[k]; ok { if (&limit).Cmp(request) == -1 { - return fmt.Errorf("invalid request resource value: %v must be less or equal to limit %v", request.String(), limit.String()) + return pipelineErrors.WrapUserError(fmt.Errorf("invalid request resource value: %v must be less or equal to limit %v", request.String(), limit.String())) } } } @@ -243,7 +244,7 @@ func validateStepOverrides(ts *v1.TaskSpec, trs *v1.TaskRunSpec) error { } for _, stepOverride := range trs.StepSpecs { if !stepNames.Has(stepOverride.Name) { - err = multierror.Append(err, fmt.Errorf("invalid StepOverride: No Step named %s", stepOverride.Name)) + err = multierror.Append(err, pipelineErrors.WrapUserError(fmt.Errorf("invalid StepOverride: No Step named %s", stepOverride.Name))) } } return err @@ -257,7 +258,7 @@ func validateSidecarOverrides(ts *v1.TaskSpec, trs *v1.TaskRunSpec) error { } for _, sidecarOverride := range trs.SidecarSpecs { if !sidecarNames.Has(sidecarOverride.Name) { - err = multierror.Append(err, fmt.Errorf("invalid SidecarOverride: No Sidecar named %s", sidecarOverride.Name)) + err = multierror.Append(err, pipelineErrors.WrapUserError(fmt.Errorf("invalid SidecarOverride: No Sidecar named %s", sidecarOverride.Name))) } } return err @@ -281,12 +282,12 @@ func validateTaskRunResults(tr *v1.TaskRun, resolvedTaskSpec *v1.TaskSpec) error s = append(s, fmt.Sprintf(" \"%v\": %v", k, v)) } sort.Strings(s) - return fmt.Errorf("Provided results don't match declared results; may be invalid JSON or missing result declaration: %v", strings.Join(s, ",")) + return pipelineErrors.WrapUserError(fmt.Errorf("Provided results don't match declared results; may be invalid JSON or missing result declaration: %v", strings.Join(s, ","))) } // When get the results, for object value need to check if they have missing keys. if missingKeysObjectNames := missingKeysofObjectResults(tr, specResults); len(missingKeysObjectNames) != 0 { - return fmt.Errorf("missing keys for these results which are required in TaskResult's properties %v", missingKeysObjectNames) + return pipelineErrors.WrapUserError(fmt.Errorf("missing keys for these results which are required in TaskResult's properties %v", missingKeysObjectNames)) } return nil } diff --git a/pkg/workspace/validate.go b/pkg/workspace/validate.go index c048a5c0710..69a7d8b8ccf 100644 --- a/pkg/workspace/validate.go +++ b/pkg/workspace/validate.go @@ -20,6 +20,7 @@ import ( "context" "fmt" + pipelineErrors "github.com/tektoncd/pipeline/pkg/apis/pipeline/errors" v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "k8s.io/apimachinery/pkg/util/sets" ) @@ -31,7 +32,7 @@ func ValidateBindings(ctx context.Context, decls []v1.WorkspaceDeclaration, bind // reason we'll invoke the same validation here. for _, b := range binds { if err := b.Validate(ctx); err != nil { - return fmt.Errorf("binding %q is invalid: %w", b.Name, err) + return pipelineErrors.WrapUserError(fmt.Errorf("binding %q is invalid: %w", b.Name, err)) } } @@ -49,12 +50,12 @@ func ValidateBindings(ctx context.Context, decls []v1.WorkspaceDeclaration, bind continue } if !bindNames.Has(decl.Name) { - return fmt.Errorf("declared workspace %q is required but has not been bound", decl.Name) + return pipelineErrors.WrapUserError(fmt.Errorf("declared workspace %q is required but has not been bound", decl.Name)) } } for _, bind := range binds { if !declNames.Has(bind.Name) { - return fmt.Errorf("workspace binding %q does not match any declared workspace", bind.Name) + return pipelineErrors.WrapUserError(fmt.Errorf("workspace binding %q does not match any declared workspace", bind.Name)) } } @@ -78,7 +79,7 @@ func ValidateOnlyOnePVCIsUsed(wb []v1.WorkspaceBinding) error { } if len(workspaceVolumes) > 1 { - return fmt.Errorf("more than one PersistentVolumeClaim is bound") + return pipelineErrors.WrapUserError(fmt.Errorf("more than one PersistentVolumeClaim is bound")) } return nil }