Convert step onError from string to "OnErrorType" type

Prior to this commit, the step onError field is defined as normal string and the 2 supported string values "continue" and "stopAndFail" are directly used across the codebase. This is error-prone and it introduces maintenance difficulty. This commit updates the onError field to be typed string "OnErrorType", with constants defined for the 2 supported values, and updates all the related references

cmd/entrypoint/main.go

@@ -30,6 +30,7 @@ import (
 	"github.com/containerd/containerd/platforms"
 	"github.com/tektoncd/pipeline/cmd/entrypoint/subcommands"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline"
+	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	"github.com/tektoncd/pipeline/pkg/credentials"
 	"github.com/tektoncd/pipeline/pkg/credentials/dockercreds"
 	"github.com/tektoncd/pipeline/pkg/credentials/gitcreds"
@@ -146,7 +147,7 @@ func main() {
 		Results:             strings.Split(*results, ","),
 		Timeout:             timeout,
 		BreakpointOnFailure: *breakpointOnFailure,
-		OnError:             *onError,
+		OnError:             v1beta1.OnErrorType(*onError),
 		StepMetadataDir:     *stepMetadataDir,
 	}
 
@@ -175,12 +176,12 @@ func main() {
 			if status, ok := t.Sys().(syscall.WaitStatus); ok {
 				checkForBreakpointOnFailure(e, breakpointExitPostFile)
 				// ignore a step error i.e. do not exit if a container terminates with a non-zero exit code when onError is set to "continue"
-				if e.OnError != entrypoint.ContinueOnError {
+				if e.OnError != v1beta1.StopAndFail {
 					os.Exit(status.ExitStatus())
 				}
 			}
 			// log and exit only if a step error must cause run failure
-			if e.OnError != entrypoint.ContinueOnError {
+			if e.OnError != v1beta1.Continue {
 				log.Fatalf("Error executing command (ExitError): %v", err)
 			}
 		default:

pkg/apis/pipeline/v1/container_types.go

@@ -131,7 +131,7 @@ type Step struct {
 	// can be set to [ continue | stopAndFail ]
 	// stopAndFail indicates exit the taskRun if the container exits with non-zero exit code
 	// continue indicates continue executing the rest of the steps irrespective of the container exit code
-	OnError string `json:"onError,omitempty"`
+	OnError OnErrorType `json:"onError,omitempty"`
 	// Stores configuration for the stdout stream of the step.
 	// +optional
 	StdoutConfig *StepOutputConfig `json:"stdoutConfig,omitempty"`
@@ -140,6 +140,13 @@ type Step struct {
 	StderrConfig *StepOutputConfig `json:"stderrConfig,omitempty"`
 }
 
+type OnErrorType string
+
+const (
+	StopAndFail OnErrorType = "stopAndFail"
+	Continue    OnErrorType = "continue"
+)
+
 // StepOutputConfig stores configuration for a step output stream.
 type StepOutputConfig struct {
 	// Path to duplicate stdout stream to on container's local filesystem.

pkg/apis/pipeline/v1/task_validation.go

@@ -242,7 +242,7 @@ func validateStep(ctx context.Context, s Step, names sets.String) (errs *apis.Fi
 	}
 
 	if s.OnError != "" {
-		if s.OnError != "continue" && s.OnError != "stopAndFail" {
+		if s.OnError != Continue && s.OnError != StopAndFail {
 			errs = errs.Also(&apis.FieldError{
 				Message: fmt.Sprintf("invalid value: %v", s.OnError),
 				Paths:   []string{"onError"},

pkg/apis/pipeline/v1/task_validation_test.go

@@ -1389,14 +1389,14 @@ func TestStepOnError(t *testing.T) {
 	}{{
 		name: "valid step - valid onError usage - set to continue - alpha API",
 		steps: []v1.Step{{
-			OnError: "continue",
+			OnError: v1.Continue,
 			Image:   "image",
 			Args:    []string{"arg"},
 		}},
 	}, {
 		name: "valid step - valid onError usage - set to stopAndFail - alpha API",
 		steps: []v1.Step{{
-			OnError: "stopAndFail",
+			OnError: v1.StopAndFail,
 			Image:   "image",
 			Args:    []string{"arg"},
 		}},

pkg/apis/pipeline/v1beta1/container_conversion.go

@@ -28,7 +28,7 @@ func (s Step) convertTo(ctx context.Context, sink *v1.Step) {
 		w.convertTo(ctx, &new)
 		sink.Workspaces = append(sink.Workspaces, new)
 	}
-	sink.OnError = s.OnError
+	sink.OnError = (v1.OnErrorType)(s.OnError)
 	sink.StdoutConfig = (*v1.StepOutputConfig)(s.StdoutConfig)
 	sink.StderrConfig = (*v1.StepOutputConfig)(s.StderrConfig)
 
@@ -59,7 +59,7 @@ func (s *Step) convertFrom(ctx context.Context, source v1.Step) {
 		new.convertFrom(ctx, w)
 		s.Workspaces = append(s.Workspaces, new)
 	}
-	s.OnError = source.OnError
+	s.OnError = (OnErrorType)(source.OnError)
 	s.StdoutConfig = (*StepOutputConfig)(source.StdoutConfig)
 	s.StderrConfig = (*StepOutputConfig)(source.StderrConfig)
 }

pkg/apis/pipeline/v1beta1/container_types.go

@@ -190,7 +190,7 @@ type Step struct {
 	// can be set to [ continue | stopAndFail ]
 	// stopAndFail indicates exit the taskRun if the container exits with non-zero exit code
 	// continue indicates continue executing the rest of the steps irrespective of the container exit code
-	OnError string `json:"onError,omitempty"`
+	OnError OnErrorType `json:"onError,omitempty"`
 
 	// Stores configuration for the stdout stream of the step.
 	// +optional
@@ -200,6 +200,13 @@ type Step struct {
 	StderrConfig *StepOutputConfig `json:"stderrConfig,omitempty"`
 }
 
+type OnErrorType string
+
+const (
+	StopAndFail OnErrorType = "stopAndFail"
+	Continue    OnErrorType = "continue"
+)
+
 // StepOutputConfig stores configuration for a step output stream.
 type StepOutputConfig struct {
 	// Path to duplicate stdout stream to on container's local filesystem.

pkg/apis/pipeline/v1beta1/task_conversion_test.go

@@ -95,7 +95,7 @@ func TestTaskConversion(t *testing.T) {
 					Script:          "echo hello",
 					Timeout:         &metav1.Duration{Duration: time.Hour},
 					Workspaces:      []v1beta1.WorkspaceUsage{{Name: "workspace"}},
-					OnError:         "continue",
+					OnError:         v1beta1.Continue,
 					StdoutConfig:    &v1beta1.StepOutputConfig{Path: "/path"},
 					StderrConfig:    &v1beta1.StepOutputConfig{Path: "/another-path"},
 				}},

pkg/apis/pipeline/v1beta1/task_validation.go

@@ -243,9 +243,8 @@ func validateStep(ctx context.Context, s Step, names sets.String) (errs *apis.Fi
 		}
 	}
 
-	// validate static values in onError if specified - onError can only be set to continue or stopAndFail
 	if s.OnError != "" {
-		if !isParamRefs(s.OnError) && s.OnError != "continue" && s.OnError != "stopAndFail" {
+		if !isParamRefs(string(s.OnError)) && s.OnError != Continue && s.OnError != StopAndFail {
 			errs = errs.Also(&apis.FieldError{
 				Message: fmt.Sprintf("invalid value: %v", s.OnError),
 				Paths:   []string{"onError"},
@@ -603,7 +602,7 @@ func validateStepVariables(ctx context.Context, step Step, prefix string, vars s
 		errs = errs.Also(validateTaskVariable(v.MountPath, prefix, vars).ViaField("MountPath").ViaFieldIndex("volumeMount", i))
 		errs = errs.Also(validateTaskVariable(v.SubPath, prefix, vars).ViaField("SubPath").ViaFieldIndex("volumeMount", i))
 	}
-	errs = errs.Also(validateTaskVariable(step.OnError, prefix, vars).ViaField("onError"))
+	errs = errs.Also(validateTaskVariable(string(step.OnError), prefix, vars).ViaField("onError"))
 	return errs
 }
 

pkg/apis/pipeline/v1beta1/task_validation_test.go

@@ -1501,22 +1501,22 @@ func TestStepOnError(t *testing.T) {
 	}{{
 		name: "valid step - valid onError usage - set to continue",
 		steps: []v1beta1.Step{{
-			OnError: "continue",
+			OnError: v1beta1.Continue,
 			Image:   "image",
 			Args:    []string{"arg"},
 		}},
 	}, {
 		name: "valid step - valid onError usage - set to stopAndFail",
 		steps: []v1beta1.Step{{
-			OnError: "stopAndFail",
+			OnError: v1beta1.StopAndFail,
 			Image:   "image",
 			Args:    []string{"arg"},
 		}},
 	}, {
 		name: "valid step - valid onError usage - set to a task parameter",
 		params: []v1beta1.ParamSpec{{
 			Name:    "CONTINUE",
-			Default: &v1beta1.ArrayOrString{Type: v1beta1.ParamTypeString, StringVal: "continue"},
+			Default: &v1beta1.ArrayOrString{Type: v1beta1.ParamTypeString, StringVal: string(v1beta1.Continue)},
 		}},
 		steps: []v1beta1.Step{{
 			OnError: "$(params.CONTINUE)",

pkg/container/step_replacements.go

@@ -24,7 +24,7 @@ import (
 // ApplyStepReplacements applies variable interpolation on a Step.
 func ApplyStepReplacements(step *v1beta1.Step, stringReplacements map[string]string, arrayReplacements map[string][]string) {
 	step.Script = substitution.ApplyReplacements(step.Script, stringReplacements)
-	step.OnError = substitution.ApplyReplacements(step.OnError, stringReplacements)
+	step.OnError = (v1beta1.OnErrorType)(substitution.ApplyReplacements(string(step.OnError), stringReplacements))
 	if step.StdoutConfig != nil {
 		step.StdoutConfig.Path = substitution.ApplyReplacements(step.StdoutConfig.Path, stringReplacements)
 	}

pkg/entrypoint/entrypointer.go

@@ -37,9 +37,7 @@ import (
 
 // RFC3339 with millisecond
 const (
-	timeFormat      = "2006-01-02T15:04:05.000Z07:00"
-	ContinueOnError = "continue"
-	FailOnError     = "stopAndFail"
+	timeFormat = "2006-01-02T15:04:05.000Z07:00"
 )
 
 // Entrypointer holds fields for running commands with redirected
@@ -77,7 +75,7 @@ type Entrypointer struct {
 	// OnError defines exiting behavior of the entrypoint
 	// set it to "stopAndFail" to indicate the entrypoint to exit the taskRun if the container exits with non zero exit code
 	// set it to "continue" to indicate the entrypoint to continue executing the rest of the steps irrespective of the container exit code
-	OnError string
+	OnError v1beta1.OnErrorType
 	// StepMetadataDir is the directory for a step where the step related metadata can be stored
 	StepMetadataDir string
 }
@@ -162,7 +160,7 @@ func (e Entrypointer) Go() error {
 	switch {
 	case err != nil && e.BreakpointOnFailure:
 		logger.Info("Skipping writing to PostFile")
-	case e.OnError == ContinueOnError && errors.As(err, &ee):
+	case e.OnError == v1beta1.Continue && errors.As(err, &ee):
 		// with continue on error and an ExitError, write non-zero exit code and a post file
 		exitCode := strconv.Itoa(ee.ExitCode())
 		output = append(output, v1beta1.PipelineResourceResult{

pkg/entrypoint/entrypointer_test.go

@@ -355,32 +355,33 @@ func TestEntrypointer_ReadBreakpointExitCodeFromDisk(t *testing.T) {
 
 func TestEntrypointer_OnError(t *testing.T) {
 	for _, c := range []struct {
-		desc, postFile, onError string
-		runner                  Runner
-		expectedError           bool
+		desc, postFile string
+		onError        v1beta1.OnErrorType
+		runner         Runner
+		expectedError  bool
 	}{{
 		desc:          "the step is exiting with 1, ignore the step error when onError is set to continue",
 		runner:        &fakeExitErrorRunner{},
 		postFile:      "step-one",
-		onError:       ContinueOnError,
+		onError:       v1beta1.Continue,
 		expectedError: true,
 	}, {
 		desc:          "the step is exiting with 0, ignore the step error irrespective of no error with onError set to continue",
 		runner:        &fakeRunner{},
 		postFile:      "step-one",
-		onError:       ContinueOnError,
+		onError:       v1beta1.Continue,
 		expectedError: false,
 	}, {
 		desc:          "the step is exiting with 1, treat the step error as failure with onError set to stopAndFail",
 		runner:        &fakeExitErrorRunner{},
 		expectedError: true,
 		postFile:      "step-one",
-		onError:       FailOnError,
+		onError:       v1beta1.StopAndFail,
 	}, {
 		desc:          "the step is exiting with 0, treat the step error (but there is none) as failure with onError set to stopAndFail",
 		runner:        &fakeRunner{},
 		postFile:      "step-one",
-		onError:       FailOnError,
+		onError:       v1beta1.StopAndFail,
 		expectedError: false,
 	}} {
 		t.Run(c.desc, func(t *testing.T) {
@@ -407,7 +408,7 @@ func TestEntrypointer_OnError(t *testing.T) {
 				t.Fatalf("Entrypointer didn't fail")
 			}
 
-			if c.onError == ContinueOnError {
+			if c.onError == v1beta1.Continue {
 				switch {
 				case fpw.wrote == nil:
 					t.Error("Wanted post file written, got nil")
@@ -422,7 +423,7 @@ func TestEntrypointer_OnError(t *testing.T) {
 				}
 			}
 
-			if c.onError == FailOnError {
+			if c.onError == v1beta1.StopAndFail {
 				switch {
 				case fpw.wrote == nil:
 					t.Error("Wanted post file written, got nil")

pkg/pod/entrypoint.go

@@ -28,7 +28,6 @@ import (
 
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
-	"github.com/tektoncd/pipeline/pkg/entrypoint"
 	"gomodules.xyz/jsonpatch/v2"
 	corev1 "k8s.io/api/core/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
@@ -138,11 +137,11 @@ func orderContainers(commonExtraEntrypointArgs []string, steps []corev1.Containe
 		if taskSpec != nil {
 			if taskSpec.Steps != nil && len(taskSpec.Steps) >= i+1 {
 				if taskSpec.Steps[i].OnError != "" {
-					if taskSpec.Steps[i].OnError != entrypoint.ContinueOnError && taskSpec.Steps[i].OnError != entrypoint.FailOnError {
+					if taskSpec.Steps[i].OnError != v1beta1.Continue && taskSpec.Steps[i].OnError != v1beta1.StopAndFail {
 						return nil, fmt.Errorf("task step onError must be either %s or %s but it is set to an invalid value %s",
-							entrypoint.ContinueOnError, entrypoint.FailOnError, taskSpec.Steps[i].OnError)
+							v1beta1.Continue, v1beta1.StopAndFail, taskSpec.Steps[i].OnError)
 					}
-					argsForEntrypoint = append(argsForEntrypoint, "-on_error", taskSpec.Steps[i].OnError)
+					argsForEntrypoint = append(argsForEntrypoint, "-on_error", string(taskSpec.Steps[i].OnError))
 				}
 				if taskSpec.Steps[i].Timeout != nil {
 					argsForEntrypoint = append(argsForEntrypoint, "-timeout", taskSpec.Steps[i].Timeout.Duration.String())

pkg/pod/entrypoint_test.go

@@ -24,7 +24,6 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
-	"github.com/tektoncd/pipeline/pkg/entrypoint"
 	"github.com/tektoncd/pipeline/test/diff"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -358,9 +357,9 @@ func TestEntryPointOnError(t *testing.T) {
 	}{{
 		taskSpec: v1beta1.TaskSpec{
 			Steps: []v1beta1.Step{{
-				OnError: entrypoint.ContinueOnError,
+				OnError: v1beta1.Continue,
 			}, {
-				OnError: entrypoint.FailOnError,
+				OnError: v1beta1.StopAndFail,
 			}},
 		},
 		wantContainers: []corev1.Container{{