diff --git a/docs/taskruns.md b/docs/taskruns.md index f6046ee2ed2..ba56b68ed1f 100644 --- a/docs/taskruns.md +++ b/docs/taskruns.md @@ -402,6 +402,18 @@ case is when your CI system autogenerates `TaskRuns` and it has `Parameters` it provide to all `TaskRuns`. Because you can pass in extra `Parameters`, you don't have to go through the complexity of checking each `Task` and providing only the required params. +#### Parameter Enums + +> :seedling: **Specifying `enum` is an [alpha](additional-configs.md#alpha-features) feature.** The `enable-param-enum` feature flag must be set to `"true"` to enable this feature. + +> :seedling: This feature is WIP and not yet supported/implemented. Documentation to be completed. + +If a `Parameter` is guarded by `Enum` in the `Task`, you can only provide `Parameter` values in the `TaskRun` that are predefined in the `Param.Enum` in the `Task`. The `TaskRun` will fail with reason `InvalidParamValue` otherwise. + +You can also specify `Enum` for [`TaskRun` with an embedded `Task`](#example-taskrun-with-an-embedded-task). The same param validation will be executed in this scenario. + +See more details in [Param.Enum](./tasks.md#param-enum). + ### Specifying `Resource` limits Each Step in a Task can specify its resource requirements. See diff --git a/docs/tasks.md b/docs/tasks.md index 09576bb6634..d7a79f3feda 100644 --- a/docs/tasks.md +++ b/docs/tasks.md @@ -717,7 +717,28 @@ spec: > :seedling: This feature is WIP and not yet supported/implemented. Documentation to be completed. -Parameter declarations can include `enum` which is a predefine set of valid values that can be accepted by the `Task`. +Parameter declarations can include `enum` which is a predefine set of valid values that can be accepted by the `Param`. For example, the valid/allowed values for `Param` "message" is bounded to `v1`, `v2` and `v3`: + +``` yaml +apiVersion: tekton.dev/v1 +kind: Task +metadata: + name: param-enum-demo +spec: + params: + - name: message + type: string + enum: ["v1", "v2", "v3"] + steps: + - name: build + image: bash:latest + script: | + echo "$(params.message)" +``` + +If the `Param` value passed in by `TaskRuns` is **NOT** in the predefined `enum` list, the `TaskRuns` will fail with reason `InvalidParamValue`. + +See usage in this [example](../examples/v1/taskruns/alpha/param-enum.yaml) ### Specifying `Workspaces` diff --git a/examples/v1/taskruns/alpha/param-enum.yaml b/examples/v1/taskruns/alpha/param-enum.yaml new file mode 100644 index 00000000000..77deb40549f --- /dev/null +++ b/examples/v1/taskruns/alpha/param-enum.yaml @@ -0,0 +1,25 @@ +apiVersion: tekton.dev/v1 +kind: Task +metadata: + name: task-param-enum +spec: + params: + - name: message + enum: ["v1", "v2", "v3"] + default: "v1" + steps: + - name: build + image: bash:3.2 + script: | + echo "$(params.message)" +--- +apiVersion: tekton.dev/v1 +kind: TaskRun +metadata: + name: taskrun-param-enum +spec: + taskRef: + name: task-param-enum + params: + - name: message + value: "v1" diff --git a/pkg/apis/pipeline/v1/param_types.go b/pkg/apis/pipeline/v1/param_types.go index 4f09b5bc649..9a964641bc0 100644 --- a/pkg/apis/pipeline/v1/param_types.go +++ b/pkg/apis/pipeline/v1/param_types.go @@ -26,6 +26,7 @@ import ( "github.com/tektoncd/pipeline/pkg/substitution" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/utils/strings/slices" "knative.dev/pkg/apis" ) @@ -161,6 +162,11 @@ func (ps ParamSpecs) validateParamEnums(ctx context.Context) *apis.FieldError { for dup := range findDups(p.Enum) { errs = errs.Also(apis.ErrGeneric(fmt.Sprintf("parameter enum value %v appears more than once", dup), "").ViaKey(p.Name)) } + if p.Default != nil && p.Default.StringVal != "" { + if !slices.Contains(p.Enum, p.Default.StringVal) { + errs = errs.Also(apis.ErrGeneric(fmt.Sprintf("param default value %v not in the enum list", p.Default.StringVal), "").ViaKey(p.Name)) + } + } } return errs } diff --git a/pkg/apis/pipeline/v1/task_validation_test.go b/pkg/apis/pipeline/v1/task_validation_test.go index 05799e14e2e..81bddd44c39 100644 --- a/pkg/apis/pipeline/v1/task_validation_test.go +++ b/pkg/apis/pipeline/v1/task_validation_test.go @@ -2300,6 +2300,21 @@ func TestParamEnum_Failure(t *testing.T) { configMap map[string]string expectedErr error }{{ + name: "param default val not in enum list - failure", + params: []v1.ParamSpec{{ + Name: "param1", + Type: v1.ParamTypeString, + Default: &v1.ParamValue{ + Type: v1.ParamTypeString, + StringVal: "v4", + }, + Enum: []string{"v1", "v2"}, + }}, + configMap: map[string]string{ + "enable-param-enum": "true", + }, + expectedErr: fmt.Errorf("param default value v4 not in the enum list: params[param1]"), + }, { name: "param enum with array type - failure", params: []v1.ParamSpec{{ Name: "param1", diff --git a/pkg/apis/pipeline/v1/taskrun_types.go b/pkg/apis/pipeline/v1/taskrun_types.go index cc273950a91..083f3e13971 100644 --- a/pkg/apis/pipeline/v1/taskrun_types.go +++ b/pkg/apis/pipeline/v1/taskrun_types.go @@ -185,6 +185,8 @@ const ( TaskRunReasonResultLargerThanAllowedLimit TaskRunReason = "TaskRunResultLargerThanAllowedLimit" // TaskRunReasonStopSidecarFailed indicates that the sidecar is not properly stopped. TaskRunReasonStopSidecarFailed = "TaskRunStopSidecarFailed" + // TaskRunReasonInvalidParamValue indicates that the TaskRun Param input value is not allowed. + TaskRunReasonInvalidParamValue = "InvalidParamValue" ) func (t TaskRunReason) String() string { diff --git a/pkg/apis/pipeline/v1beta1/param_types.go b/pkg/apis/pipeline/v1beta1/param_types.go index 1252ec71a1b..689770d663e 100644 --- a/pkg/apis/pipeline/v1beta1/param_types.go +++ b/pkg/apis/pipeline/v1beta1/param_types.go @@ -26,6 +26,7 @@ import ( "github.com/tektoncd/pipeline/pkg/substitution" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/utils/strings/slices" "knative.dev/pkg/apis" ) @@ -152,6 +153,11 @@ func (ps ParamSpecs) validateParamEnums(ctx context.Context) *apis.FieldError { for dup := range findDups(p.Enum) { errs = errs.Also(apis.ErrGeneric(fmt.Sprintf("parameter enum value %v appears more than once", dup), "").ViaKey(p.Name)) } + if p.Default != nil && p.Default.StringVal != "" { + if !slices.Contains(p.Enum, p.Default.StringVal) { + errs = errs.Also(apis.ErrGeneric(fmt.Sprintf("param default value %v not in the enum list", p.Default.StringVal), "").ViaKey(p.Name)) + } + } } return errs } diff --git a/pkg/apis/pipeline/v1beta1/task_validation_test.go b/pkg/apis/pipeline/v1beta1/task_validation_test.go index f47263654d3..8cb950c41ad 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/task_validation_test.go @@ -2137,6 +2137,21 @@ func TestParamEnum_Failure(t *testing.T) { configMap map[string]string expectedErr error }{{ + name: "param default val not in enum list - failure", + params: []v1beta1.ParamSpec{{ + Name: "param1", + Type: v1beta1.ParamTypeString, + Default: &v1beta1.ParamValue{ + Type: v1beta1.ParamTypeString, + StringVal: "v4", + }, + Enum: []string{"v1", "v2"}, + }}, + configMap: map[string]string{ + "enable-param-enum": "true", + }, + expectedErr: fmt.Errorf("param default value v4 not in the enum list: params[param1]"), + }, { name: "param enum with array type - failure", params: []v1beta1.ParamSpec{{ Name: "param1", diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index 049365e897e..40cfb854994 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -417,6 +417,14 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1.TaskRun) (*v1.TaskSpec, return nil, nil, controller.NewPermanentError(err) } + if config.FromContextOrDefaults(ctx).FeatureFlags.EnableParamEnum { + if err := ValidateEnumParam(ctx, tr.Spec.Params, rtr.TaskSpec.Params); err != nil { + logger.Errorf("TaskRun %q Param Enum validation failed: %v", tr.Name, err) + tr.Status.MarkResourceFailed(v1.TaskRunReasonInvalidParamValue, err) + return nil, nil, controller.NewPermanentError(err) + } + } + if err := resources.ValidateParamArrayIndex(rtr.TaskSpec, tr.Spec.Params); err != nil { logger.Errorf("TaskRun %q Param references are invalid: %v", tr.Name, err) tr.Status.MarkResourceFailed(podconvert.ReasonFailedValidation, err) diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index b2813dd5a82..f129fcaa04a 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -145,6 +145,24 @@ var ( Steps: []v1.Step{simpleStep}, }, } + simpleTaskWithParamEnum = &v1.Task{ + ObjectMeta: objectMeta("test-task-param-enum", "foo"), + TypeMeta: metav1.TypeMeta{ + APIVersion: "tekton.dev/v1", + Kind: "Task", + }, + Spec: v1.TaskSpec{ + Params: []v1.ParamSpec{{ + Name: "param1", + Enum: []string{"v1", "v2"}, + }, { + Name: "param2", + Enum: []string{"v1", "v2"}, + Default: &v1.ParamValue{Type: v1.ParamTypeString, StringVal: "v1"}, + }}, + Steps: []v1.Step{simpleStep}, + }, + } resultsTask = &v1.Task{ ObjectMeta: objectMeta("test-results-task", "foo"), Spec: v1.TaskSpec{ @@ -4999,6 +5017,96 @@ status: } } +func TestReconcile_TaskRunWithParam_Enum_valid(t *testing.T) { + taskRunWithParamValid := parse.MustParseV1TaskRun(t, ` +metadata: + name: test-taskrun-with-param-enum-valid + namespace: foo +spec: + params: + - name: param1 + value: v1 + taskRef: + name: test-task-param-enum +`) + + d := test.Data{ + TaskRuns: []*v1.TaskRun{taskRunWithParamValid}, + Tasks: []*v1.Task{simpleTaskWithParamEnum}, + ConfigMaps: []*corev1.ConfigMap{{ + ObjectMeta: metav1.ObjectMeta{Namespace: system.Namespace(), Name: config.GetFeatureFlagsConfigName()}, + Data: map[string]string{ + "enable-param-enum": "true", + }, + }}, + } + + testAssets, cancel := getTaskRunController(t, d) + defer cancel() + createServiceAccount(t, testAssets, taskRunWithParamValid.Spec.ServiceAccountName, taskRunWithParamValid.Namespace) + + // Reconcile the TaskRun + if err := testAssets.Controller.Reconciler.Reconcile(testAssets.Ctx, getRunName(taskRunWithParamValid)); err == nil { + t.Error("wanted a wrapped requeue error, but got nil.") + } else if ok, _ := controller.IsRequeueKey(err); !ok { + t.Errorf("expected no error. Got error %v", err) + } + + tr, err := testAssets.Clients.Pipeline.TektonV1().TaskRuns(taskRunWithParamValid.Namespace).Get(testAssets.Ctx, taskRunWithParamValid.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("getting updated taskrun: %v", err) + } + condition := tr.Status.GetCondition(apis.ConditionSucceeded) + if condition.Type != apis.ConditionSucceeded || condition.Reason == string(corev1.ConditionFalse) { + t.Errorf("Expected TaskRun to succeed but it did not. Final conditions were:\n%#v", tr.Status.Conditions) + } +} + +func TestReconcile_TaskRunWithParam_Enum_invalid(t *testing.T) { + taskRunWithParamInvalid := parse.MustParseV1TaskRun(t, ` +metadata: + name: test-taskrun-with-param-enum-invalid + namespace: foo +spec: + params: + - name: param1 + value: invalid + taskRef: + name: test-task-param-enum +`) + + d := test.Data{ + TaskRuns: []*v1.TaskRun{taskRunWithParamInvalid}, + Tasks: []*v1.Task{simpleTaskWithParamEnum}, + ConfigMaps: []*corev1.ConfigMap{{ + ObjectMeta: metav1.ObjectMeta{Namespace: system.Namespace(), Name: config.GetFeatureFlagsConfigName()}, + Data: map[string]string{ + "enable-param-enum": "true", + }, + }}, + } + + expectedErr := fmt.Errorf("1 error occurred:\n\t* param `param1` value: invalid is not in the enum list") + expectedFailureReason := "InvalidParamValue" + testAssets, cancel := getTaskRunController(t, d) + defer cancel() + createServiceAccount(t, testAssets, taskRunWithParamInvalid.Spec.ServiceAccountName, taskRunWithParamInvalid.Namespace) + + // Reconcile the TaskRun + err := testAssets.Controller.Reconciler.Reconcile(testAssets.Ctx, getRunName(taskRunWithParamInvalid)) + if d := cmp.Diff(expectedErr.Error(), strings.TrimSuffix(err.Error(), "\n\n")); d != "" { + t.Errorf("Expected: %v, but Got: %v", expectedErr, err) + } + tr, err := testAssets.Clients.Pipeline.TektonV1().TaskRuns(taskRunWithParamInvalid.Namespace).Get(testAssets.Ctx, taskRunWithParamInvalid.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Error getting updated taskrun: %v", err) + } + condition := tr.Status.GetCondition(apis.ConditionSucceeded) + if condition.Type != apis.ConditionSucceeded || condition.Status != corev1.ConditionFalse || condition.Reason != expectedFailureReason { + t.Errorf("Expected TaskRun to fail with reason \"%s\" but it did not. Final conditions were:\n%#v", expectedFailureReason, tr.Status.Conditions) + } +} + func TestReconcile_validateTaskRunResults_valid(t *testing.T) { taskRunResultsTypeMatched := parse.MustParseV1TaskRun(t, ` metadata: diff --git a/pkg/reconciler/taskrun/validate_taskrun.go b/pkg/reconciler/taskrun/validate_taskrun.go index 505fc8e6c81..c273bfddfce 100644 --- a/pkg/reconciler/taskrun/validate_taskrun.go +++ b/pkg/reconciler/taskrun/validate_taskrun.go @@ -28,6 +28,7 @@ import ( "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources" "k8s.io/apimachinery/pkg/util/sets" + "k8s.io/utils/strings/slices" ) // validateParams validates that all Pipeline Task, Matrix.Params and Matrix.Include parameters all have values, match the specified @@ -174,6 +175,35 @@ func ValidateResolvedTask(ctx context.Context, params []v1.Param, matrix *v1.Mat return nil } +// ValidateEnumParam validates the param values are in the defined enum list in the corresponding paramSpecs if provided. +// A validation error is returned otherwise. +func ValidateEnumParam(ctx context.Context, params []v1.Param, paramSpecs v1.ParamSpecs) error { + paramSpecNameToEnum := map[string][]string{} + for _, ps := range paramSpecs { + if len(ps.Enum) == 0 { + continue + } + paramSpecNameToEnum[ps.Name] = ps.Enum + } + + for _, p := range params { + // skip validation for and non-string typed and optional params (using default value) + // the default value of param is validated at validation webhook dryrun + if p.Value.Type != v1.ParamTypeString || p.Value.StringVal == "" { + continue + } + // skip validation for paramSpec without enum + if _, ok := paramSpecNameToEnum[p.Name]; !ok { + continue + } + + 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 nil +} + func validateTaskSpecRequestResources(taskSpec *v1.TaskSpec) error { if taskSpec != nil { for _, step := range taskSpec.Steps { diff --git a/pkg/reconciler/taskrun/validate_taskrun_test.go b/pkg/reconciler/taskrun/validate_taskrun_test.go index 1ab71eb49de..04dac06da69 100644 --- a/pkg/reconciler/taskrun/validate_taskrun_test.go +++ b/pkg/reconciler/taskrun/validate_taskrun_test.go @@ -18,6 +18,7 @@ package taskrun import ( "context" + "fmt" "testing" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -826,3 +827,129 @@ func TestValidateResult(t *testing.T) { }) } } + +func TestEnumValidation_Success(t *testing.T) { + tcs := []struct { + name string + params []v1.Param + paramSpecs v1.ParamSpecs + }{{ + name: "no enum list - success", + params: []v1.Param{ + { + Name: "p1", + Value: v1.ParamValue{ + Type: v1.ParamTypeString, + StringVal: "v1", + }, + }, + }, + paramSpecs: v1.ParamSpecs{ + { + Name: "p1", + }, + }, + }, { + name: "optional param validation skipped - success", + params: []v1.Param{ + { + Name: "p1", + Value: v1.ParamValue{ + Type: v1.ParamTypeString, + }, + }, + }, + paramSpecs: v1.ParamSpecs{ + { + Name: "p1", + Enum: []string{"v1", "v2", "v3"}, + Default: &v1.ParamValue{Type: v1.ParamTypeString, StringVal: "v1"}, + }, + }, + }, { + name: "non-string param validation skipped - success", + params: []v1.Param{ + { + Name: "p1", + Value: v1.ParamValue{ + Type: v1.ParamTypeArray, + ArrayVal: []string{"a1", "a2"}, + }, + }, + }, + paramSpecs: v1.ParamSpecs{ + { + Name: "p1", + Type: v1.ParamTypeArray, + Enum: []string{"v1", "v2", "v3"}, + }, + }, + }, { + name: "value in the enum list - success", + params: []v1.Param{ + { + Name: "p1", + Value: v1.ParamValue{ + StringVal: "v1", + }, + }, + { + Name: "p2", + Value: v1.ParamValue{ + StringVal: "v2", + }, + }, + }, + paramSpecs: v1.ParamSpecs{ + { + Name: "p1", + Enum: []string{"v1", "v2", "v3"}, + }, + { + Name: "p2", + Enum: []string{"v1", "v2", "v3"}, + }, + }, + }} + + for _, tc := range tcs { + if err := ValidateEnumParam(context.Background(), tc.params, tc.paramSpecs); err != nil { + t.Errorf("expected err is nil, but got %v", err) + } + } +} + +func TestEnumValidation_Failure(t *testing.T) { + tcs := []struct { + name string + params []v1.Param + paramSpecs v1.ParamSpecs + expectedErr error + }{{ + name: "value not in the enum list - failure", + params: []v1.Param{ + { + Name: "p1", + Value: v1.ParamValue{ + StringVal: "v4", + Type: v1.ParamTypeString, + }, + }, + }, + paramSpecs: v1.ParamSpecs{ + { + Name: "p1", + Enum: []string{"v1", "v2", "v3"}, + }, + }, + expectedErr: fmt.Errorf("param `p1` value: v4 is not in the enum list"), + }} + + for _, tc := range tcs { + if err := ValidateEnumParam(context.Background(), tc.params, tc.paramSpecs); err == nil { + t.Errorf("expected error from ValidateEnumParam() = %v, but got none", tc.expectedErr) + } else if d := cmp.Diff(tc.expectedErr.Error(), err.Error()); d != "" { + t.Errorf("expected error does not match: %s", diff.PrintWantGot(d)) + } + } +} diff --git a/test/custom_task_test.go b/test/custom_task_test.go index ba972b6abaf..3b557ce3f64 100644 --- a/test/custom_task_test.go +++ b/test/custom_task_test.go @@ -734,6 +734,7 @@ func getFeatureFlagsBaseOnAPIFlag(t *testing.T) *config.FeatureFlags { "enable-tekton-oci-bundles": "true", "enable-step-actions": "true", "enable-cel-in-whenexpression": "true", + "enable-param-enum": "true", }) if err != nil { t.Fatalf("error creating alpha feature flags configmap: %v", err) diff --git a/test/e2e-tests-kind-prow-alpha.env b/test/e2e-tests-kind-prow-alpha.env index 3b5bc6a771b..221744ca073 100644 --- a/test/e2e-tests-kind-prow-alpha.env +++ b/test/e2e-tests-kind-prow-alpha.env @@ -6,3 +6,4 @@ E2E_GO_TEST_TIMEOUT=40m RESULTS_FROM=sidecar-logs ENABLE_STEP_ACTIONS=true ENABLE_CEL_IN_WHENEXPRESSION=true +ENABLE_PARAM_ENUM=true diff --git a/test/e2e-tests.sh b/test/e2e-tests.sh index 5d7923df00b..c33545dcaab 100755 --- a/test/e2e-tests.sh +++ b/test/e2e-tests.sh @@ -29,6 +29,7 @@ E2E_GO_TEST_TIMEOUT=${E2E_GO_TEST_TIMEOUT:="20m"} RESULTS_FROM=${RESULTS_FROM:-termination-message} ENABLE_STEP_ACTIONS=${ENABLE_STEP_ACTIONS:="false"} ENABLE_CEL_IN_WHENEXPRESSION=${ENABLE_CEL_IN_WHENEXPRESSION:="false"} +ENABLE_PARAM_ENUM=${ENABLE_PARAM_ENUM:="false"} failed=0 # Script entry point. @@ -102,6 +103,18 @@ function set_cel_in_whenexpression() { kubectl patch configmap feature-flags -n tekton-pipelines -p "$jsonpatch" } +function set_enable_param_enum() { + local method="$1" + if [ "$method" != "false" ] && [ "$method" != "true" ]; then + printf "Invalid value for enable-param-enum %s\n" ${method} + exit 255 + fi + printf "Setting enable-param-enum to %s\n", ${method} + jsonpatch=$(printf "{\"data\": {\"enable-param-enum\": \"%s\"}}" $1) + echo "feature-flags ConfigMap patch: ${jsonpatch}" + kubectl patch configmap feature-flags -n tekton-pipelines -p "$jsonpatch" +} + function run_e2e() { # Run the integration tests header "Running Go e2e tests" @@ -123,6 +136,7 @@ set_feature_gate "$PIPELINE_FEATURE_GATE" set_result_extraction_method "$RESULTS_FROM" set_enable_step_actions "$ENABLE_STEP_ACTIONS" set_cel_in_whenexpression "$ENABLE_CEL_IN_WHENEXPRESSION" +set_enable_param_enum "$ENABLE_PARAM_ENUM" run_e2e (( failed )) && fail_test