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

pkg/apis/pipeline/v1/container_types.go

@@ -129,9 +129,7 @@ type Step struct {
 
 	// OnError defines the exiting behavior of a container on error
 	// 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 +138,16 @@ type Step struct {
 	StderrConfig *StepOutputConfig `json:"stderrConfig,omitempty"`
 }
 
+// OnErrorType defines a list of supported exiting behavior of a container on error
+type OnErrorType string
+
+const (
+	// StopAndFail indicates exit the taskRun if the container exits with non-zero exit code
+	StopAndFail OnErrorType = "stopAndFail"
+	// Continue indicates continue executing the rest of the steps irrespective of the container exit code
+	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/swagger.json

@@ -872,7 +872,7 @@
           "default": ""
         },
         "onError": {
-          "description": "OnError defines the exiting behavior of a container on error 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",
+          "description": "OnError defines the exiting behavior of a container on error can be set to [ continue | stopAndFail ]",
           "type": "string"
         },
         "resources": {

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

@@ -1386,14 +1386,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

@@ -188,9 +188,7 @@ type Step struct {
 
 	// OnError defines the exiting behavior of a container on error
 	// 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 +198,16 @@ type Step struct {
 	StderrConfig *StepOutputConfig `json:"stderrConfig,omitempty"`
 }
 
+// OnErrorType defines a list of supported exiting behavior of a container on error
+type OnErrorType string
+
+const (
+	// StopAndFail indicates exit the taskRun if the container exits with non-zero exit code
+	StopAndFail OnErrorType = "stopAndFail"
+	// Continue indicates continue executing the rest of the steps irrespective of the container exit code
+	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/swagger.json

@@ -1822,7 +1822,7 @@
           "default": ""
         },
         "onError": {
-          "description": "OnError defines the exiting behavior of a container on error 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",
+          "description": "OnError defines the exiting behavior of a container on error can be set to [ continue | stopAndFail ]",
           "type": "string"
         },
         "ports": {

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/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{{