Fix showing error message when validation fail

When step action is used in step, and feature flag is not enabled, the error message is not shown correctly and would show a %s. We are now using fmt.Sprintf to format the error message and show the feature flag that need to be enabled properly. apis.ErrGeneric need a empty string as second argument to be able to show the path steps so adding this. Fixes bug #7493 Signed-off-by: Chmouel Boudjnah <[email protected]>
tektoncd · Dec 20, 2023 · 36de579 · 36de579
1 parent c43d040
commit 36de579
Show file tree

Hide file tree

Showing 8 changed files with 25 additions and 27 deletions.
diff --git a/pkg/apis/pipeline/v1/result_validation.go b/pkg/apis/pipeline/v1/result_validation.go
@@ -74,7 +74,7 @@ func (tr TaskResult) validateValue(ctx context.Context) (errs *apis.FieldError)
 		return nil
 	}
 	if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableStepActions {
-		return apis.ErrGeneric("feature flag %s should be set to true to fetch Results from Steps using StepActions.", config.EnableStepActions)
+		return apis.ErrGeneric(fmt.Sprintf("feature flag %s should be set to true to fetch Results from Steps using StepActions.", config.EnableStepActions))
 	}
 	if tr.Value.Type != ParamTypeString {
 		return &apis.FieldError{

diff --git a/pkg/apis/pipeline/v1/result_validation_test.go b/pkg/apis/pipeline/v1/result_validation_test.go
@@ -175,8 +175,7 @@ func TestResultsValidateValueError(t *testing.T) {
 		},
 		enableStepActions: false,
 		expectedError: apis.FieldError{
-			Message: "feature flag %s should be set to true to fetch Results from Steps using StepActions.",
-			Paths:   []string{"enable-step-actions"},
+			Message: "feature flag enable-step-actions should be set to true to fetch Results from Steps using StepActions.",
 		},
 	}, {
 		name: "invalid result value type array",

diff --git a/pkg/apis/pipeline/v1/task_validation.go b/pkg/apis/pipeline/v1/task_validation.go
@@ -325,7 +325,7 @@ func isCreateOrUpdateAndDiverged(ctx context.Context, s Step) bool {
 func validateStep(ctx context.Context, s Step, names sets.String) (errs *apis.FieldError) {
 	if s.Ref != nil {
 		if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableStepActions && isCreateOrUpdateAndDiverged(ctx, s) {
-			return apis.ErrGeneric("feature flag %s should be set to true to reference StepActions in Steps.", config.EnableStepActions)
+			return apis.ErrGeneric(fmt.Sprintf("feature flag %s should be set to true to reference StepActions in Steps.", config.EnableStepActions), "")
 		}
 		errs = errs.Also(s.Ref.Validate(ctx))
 		if s.Image != "" {
@@ -385,7 +385,7 @@ func validateStep(ctx context.Context, s Step, names sets.String) (errs *apis.Fi
 		}
 		if len(s.Results) > 0 {
 			if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableStepActions && isCreateOrUpdateAndDiverged(ctx, s) {
-				return apis.ErrGeneric("feature flag %s should be set to true in order to use Results in Steps.", config.EnableStepActions)
+				return apis.ErrGeneric(fmt.Sprintf("feature flag %s should be set to true in order to use Results in Steps.", config.EnableStepActions), "")
 			}
 		}
 		if s.Image == "" {

diff --git a/pkg/apis/pipeline/v1/task_validation_test.go b/pkg/apis/pipeline/v1/task_validation_test.go
@@ -1429,8 +1429,8 @@ func TestTaskSpecValidateErrorWithStepActionRef_CreateUpdateEvent(t *testing.T)
 		isCreate: true,
 		isUpdate: false,
 		expectedError: apis.FieldError{
-			Message: "feature flag %s should be set to true to reference StepActions in Steps.",
-			Paths:   []string{"steps[0].enable-step-actions"},
+			Message: "feature flag enable-step-actions should be set to true to reference StepActions in Steps.",
+			Paths:   []string{"steps[0]"},
 		},
 	}, {
 		name: "is update ctx",
@@ -1442,8 +1442,8 @@ func TestTaskSpecValidateErrorWithStepActionRef_CreateUpdateEvent(t *testing.T)
 		isCreate: false,
 		isUpdate: true,
 		expectedError: apis.FieldError{
-			Message: "feature flag %s should be set to true to reference StepActions in Steps.",
-			Paths:   []string{"steps[0].enable-step-actions"},
+			Message: "feature flag enable-step-actions should be set to true to reference StepActions in Steps.",
+			Paths:   []string{"steps[0]"},
 		},
 	}, {
 		name: "ctx is not create or update",
@@ -2642,8 +2642,8 @@ func TestTaskSpecValidate_StepResults_Error(t *testing.T) {
 		enableStepActions: false,
 		isCreate:          true,
 		expectedError: apis.FieldError{
-			Message: "feature flag %s should be set to true in order to use Results in Steps.",
-			Paths:   []string{"steps[0].enable-step-actions"},
+			Message: "feature flag enable-step-actions should be set to true in order to use Results in Steps.",
+			Paths:   []string{"steps[0]"},
 		},
 	}, {
 		name: "step result not allowed without enable step actions - update and diverged event",
@@ -2664,8 +2664,8 @@ func TestTaskSpecValidate_StepResults_Error(t *testing.T) {
 			},
 		},
 		expectedError: apis.FieldError{
-			Message: "feature flag %s should be set to true in order to use Results in Steps.",
-			Paths:   []string{"steps[0].enable-step-actions"},
+			Message: "feature flag enable-step-actions should be set to true in order to use Results in Steps.",
+			Paths:   []string{"steps[0]"},
 		},
 	}, {
 		name: "step result allowed without enable step actions - update but not diverged",

diff --git a/pkg/apis/pipeline/v1beta1/result_validation.go b/pkg/apis/pipeline/v1beta1/result_validation.go
@@ -74,7 +74,7 @@ func (tr TaskResult) validateValue(ctx context.Context) (errs *apis.FieldError)
 		return nil
 	}
 	if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableStepActions {
-		return apis.ErrGeneric("feature flag %s should be set to true to fetch Results from Steps using StepActions.", config.EnableStepActions)
+		return apis.ErrGeneric(fmt.Sprintf("feature flag %s should be set to true to fetch Results from Steps using StepActions.", config.EnableStepActions))
 	}
 	if tr.Value.Type != ParamTypeString {
 		return &apis.FieldError{
@@ -89,7 +89,7 @@ func (tr TaskResult) validateValue(ctx context.Context) (errs *apis.FieldError)
 		stepName, resultName, err := v1.ExtractStepResultName(tr.Value.StringVal)
 		if err != nil {
 			return &apis.FieldError{
-				Message: fmt.Sprintf("%v", err),
+				Message: err.Error(),
 				Paths:   []string{fmt.Sprintf("%s.value", tr.Name)},
 			}
 		}

diff --git a/pkg/apis/pipeline/v1beta1/result_validation_test.go b/pkg/apis/pipeline/v1beta1/result_validation_test.go
@@ -176,8 +176,7 @@ func TestResultsValidateValueError(t *testing.T) {
 		},
 		enableStepActions: false,
 		expectedError: apis.FieldError{
-			Message: "feature flag %s should be set to true to fetch Results from Steps using StepActions.",
-			Paths:   []string{"enable-step-actions"},
+			Message: "feature flag enable-step-actions should be set to true to fetch Results from Steps using StepActions.",
 		},
 	}, {
 		name: "invalid result value type array",

diff --git a/pkg/apis/pipeline/v1beta1/task_validation.go b/pkg/apis/pipeline/v1beta1/task_validation.go
@@ -314,7 +314,7 @@ func isCreateOrUpdateAndDiverged(ctx context.Context, s Step) bool {
 func validateStep(ctx context.Context, s Step, names sets.String) (errs *apis.FieldError) {
 	if s.Ref != nil {
 		if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableStepActions && isCreateOrUpdateAndDiverged(ctx, s) {
-			return apis.ErrGeneric("feature flag %s should be set to true to reference StepActions in Steps.", config.EnableStepActions)
+			return apis.ErrGeneric(fmt.Sprintf("feature flag %s should be set to true to reference StepActions in Steps.", config.EnableStepActions), "")
 		}
 		errs = errs.Also(s.Ref.Validate(ctx))
 		if s.Image != "" {
@@ -374,7 +374,7 @@ func validateStep(ctx context.Context, s Step, names sets.String) (errs *apis.Fi
 		}
 		if len(s.Results) > 0 {
 			if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableStepActions && isCreateOrUpdateAndDiverged(ctx, s) {
-				return apis.ErrGeneric("feature flag %s should be set to true in order to use Results in Steps.", config.EnableStepActions)
+				return apis.ErrGeneric(fmt.Sprintf("feature flag %s should be set to true in order to use Results in Steps.", config.EnableStepActions), "")
 			}
 		}
 		if s.Image == "" {

diff --git a/pkg/apis/pipeline/v1beta1/task_validation_test.go b/pkg/apis/pipeline/v1beta1/task_validation_test.go
@@ -1442,8 +1442,8 @@ func TestTaskSpecValidateErrorWithStepActionRef_CreateUpdateEvent(t *testing.T)
 		isCreate: true,
 		isUpdate: false,
 		expectedError: apis.FieldError{
-			Message: "feature flag %s should be set to true to reference StepActions in Steps.",
-			Paths:   []string{"steps[0].enable-step-actions"},
+			Message: "feature flag enable-step-actions should be set to true to reference StepActions in Steps.",
+			Paths:   []string{"steps[0]"},
 		},
 	}, {
 		name: "is update ctx",
@@ -1455,8 +1455,8 @@ func TestTaskSpecValidateErrorWithStepActionRef_CreateUpdateEvent(t *testing.T)
 		isCreate: false,
 		isUpdate: true,
 		expectedError: apis.FieldError{
-			Message: "feature flag %s should be set to true to reference StepActions in Steps.",
-			Paths:   []string{"steps[0].enable-step-actions"},
+			Message: "feature flag enable-step-actions should be set to true to reference StepActions in Steps.",
+			Paths:   []string{"steps[0]"},
 		},
 	}, {
 		name: "ctx is not create or update",
@@ -2428,8 +2428,8 @@ func TestTaskSpecValidate_StepResults_Error(t *testing.T) {
 		enableStepActions: false,
 		isCreate:          true,
 		expectedError: apis.FieldError{
-			Message: "feature flag %s should be set to true in order to use Results in Steps.",
-			Paths:   []string{"steps[0].enable-step-actions"},
+			Message: "feature flag enable-step-actions should be set to true in order to use Results in Steps.",
+			Paths:   []string{"steps[0]"},
 		},
 	}, {
 		name: "step result not allowed without enable step actions - update and diverged event",
@@ -2450,8 +2450,8 @@ func TestTaskSpecValidate_StepResults_Error(t *testing.T) {
 			},
 		},
 		expectedError: apis.FieldError{
-			Message: "feature flag %s should be set to true in order to use Results in Steps.",
-			Paths:   []string{"steps[0].enable-step-actions"},
+			Message: "feature flag enable-step-actions should be set to true in order to use Results in Steps.",
+			Paths:   []string{"steps[0]"},
 		},
 	}, {
 		name: "step result allowed without enable step actions - update but not diverged",