From 8a8c0c39ca42a757077a8808aad0a9fcf749281e Mon Sep 17 00:00:00 2001 From: Quan Zhang Date: Tue, 24 Oct 2023 11:26:42 -0400 Subject: [PATCH] [TEP-0144] Validate PipelineRun for Param Enum Part of [#7270][#7270]. In [TEP-0144][tep-0144] we proposed a new `enum` field to support built-in param input validation. This commit adds validation logic for PipelineRun against Param Enum /kind feature [#7270]: https://github.com/tektoncd/pipeline/issues/7270 [tep-0144]: https://github.com/tektoncd/community/blob/main/teps/0144-param-enum.md --- config/config-feature-flags.yaml | 1 - docs/additional-configs.md | 1 + docs/pipeline-api.md | 3 + docs/pipelineruns.md | 12 + docs/pipelines.md | 72 ++++- docs/taskruns.md | 4 +- docs/tasks.md | 7 +- .../v1/pipelineruns/alpha/param-enum.yaml | 42 +++ pkg/apis/pipeline/v1/pipelinerun_types.go | 2 + pkg/reconciler/pipelinerun/pipelinerun.go | 42 ++- .../pipelinerun/pipelinerun_test.go | 215 +++++++++++++++ .../resources/pipelinerunresolution.go | 71 +++++ .../resources/pipelinerunresolution_test.go | 251 ++++++++++++++++++ 13 files changed, 710 insertions(+), 13 deletions(-) create mode 100644 examples/v1/pipelineruns/alpha/param-enum.yaml diff --git a/config/config-feature-flags.yaml b/config/config-feature-flags.yaml index 9911dd3fdb3..83c8ee98e01 100644 --- a/config/config-feature-flags.yaml +++ b/config/config-feature-flags.yaml @@ -127,5 +127,4 @@ data: # This feature is in preview mode and not implemented yet. Please check #7259 for updates. enable-step-actions: "false" # Setting this flag to "true" will enable the built-in param input validation via param enum. - # NOTE (#7270): this feature is still under development and not yet functional. enable-param-enum: "false" diff --git a/docs/additional-configs.md b/docs/additional-configs.md index 11b95952590..16a48b36950 100644 --- a/docs/additional-configs.md +++ b/docs/additional-configs.md @@ -317,6 +317,7 @@ Features currently in "alpha" are: | [Coschedule](./affinityassistants.md) | [TEP-0135](https://github.com/tektoncd/community/blob/main/teps/0135-coscheduling-pipelinerun-pods.md) | N/A |`coschedule` | | [keep pod on cancel](./taskruns.md#cancelling-a-taskrun) | N/A | v0.52 | keep-pod-on-cancel | | [CEL in WhenExpression](./taskruns.md#cancelling-a-taskrun) | [TEP-0145](https://github.com/tektoncd/community/blob/main/teps/0145-cel-in-whenexpression.md) | N/A | enable-cel-in-whenexpression | +| [Param Enum](./taskruns.md#parameter-enums) | [TEP-0144](https://github.com/tektoncd/community/blob/main/teps/0144-param-enum.md) | N/A | `enable-param-enum` | ### Beta Features diff --git a/docs/pipeline-api.md b/docs/pipeline-api.md index 3ae32f60afd..ffd32222dcd 100644 --- a/docs/pipeline-api.md +++ b/docs/pipeline-api.md @@ -1997,6 +1997,9 @@ associated Pipeline is an invalid graph (a.k.a wrong order, cycle, …)

"InvalidMatrixParameterTypes"

ReasonInvalidMatrixParameterTypes indicates a matrix contains invalid parameter types

+

"InvalidParamValue"

+

PipelineRunReasonInvalidParamValue indicates that the PipelineRun Param input value is not allowed.

+

"InvalidTaskResultReference"

ReasonInvalidTaskResultReference indicates a task result was declared but was not initialized by that task

diff --git a/docs/pipelineruns.md b/docs/pipelineruns.md index a8ab9cf2187..f308dcced8a 100644 --- a/docs/pipelineruns.md +++ b/docs/pipelineruns.md @@ -271,6 +271,18 @@ case is when your CI system autogenerates `PipelineRuns` and it has `Parameters` provide to all `PipelineRuns`. Because you can pass in extra `Parameters`, you don't have to go through the complexity of checking each `Pipeline` and providing only the required params. +#### Parameter Enums + +> :seedling: **`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. + +If a `Parameter` is guarded by `Enum` in the `Pipeline`, you can only provide `Parameter` values in the `PipelineRun` that are predefined in the `Param.Enum` in the `Pipeline`. The `PipelineRun` will fail with reason `InvalidParamValue` otherwise. + +Tekton will also the validate the `param` values passed to any referenced `Tasks` (via `taskRef`) if `Enum` is specified for the `Task`. The `PipelineRun` will fail with reason `InvalidParamValue` if `Enum` validation is failed for any of the `PipelineTask`. + +You can also specify `Enum` in an embedded `Pipeline` in a `PipelineRun`. The same `Param` validation will be executed in this scenario. + +See more details in [Param.Enum](./pipelines.md#param-enum). + #### Propagated Parameters When using an inlined spec, parameters from the parent `PipelineRun` will be diff --git a/docs/pipelines.md b/docs/pipelines.md index e734ed1de73..3bccd4e4f94 100644 --- a/docs/pipelines.md +++ b/docs/pipelines.md @@ -272,11 +272,77 @@ spec: ``` #### Param enum -> :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: **`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. +Parameter declarations can include `enum` which is a predefine set of valid values that can be accepted by the `Pipeline` `Param`. If a `Param` has both `enum` and default value, the default value must be in the `enum` set. For example, the valid/allowed values for `Param` "message" is bounded to `v1` and `v2`: -Parameter declarations can include `enum` which is a predefine set of valid values that can be accepted by the `Pipeline`. +``` yaml +apiVersion: tekton.dev/v1 +kind: Pipeline +metadata: + name: pipeline-param-enum +spec: + params: + - name: message + enum: ["v1", "v2"] + default: "v1" + tasks: + - name: task1 + params: + - name: message + value: $(params.message) + steps: + - name: build + image: bash:3.2 + script: | + echo "$(params.message)" +``` + +If the `Param` value passed in by `PipelineRun` is **NOT** in the predefined `enum` list, the `PipelineRun` will fail with reason `InvalidParamValue`. + +If a `PipelineTask` references a `Task` with `enum`, the `enums` specified in the Pipeline `spec.params` (pipeline-level `enum`) must be +a **subset** of the `enums` specified in the referenced `Task` (task-level `enum`). Note that an empty pipeline-level `enum` is invalid +in this scenario since an empty `enum` set indicates a "universal set" which allows all possible values. In the below example, the referenced `Task` accepts `v1` and `v2` as valid values, the `Pipeline` further restricts the valid values to `v1`. + +``` yaml +apiVersion: tekton.dev/v1 +kind: Task +metadata: + name: param-enum-demo +spec: + params: + - name: message + type: string + enum: ["v1", "v2"] + steps: + - name: build + image: bash:latest + script: | + echo "$(params.message)" +``` + +``` yaml +apiVersion: tekton.dev/v1 +kind: Pipeline +metadata: + name: pipeline-param-enum +spec: + params: + - name: message + enum: ["v1"] # note that an empty enum set is invalid + tasks: + - name: task1 + params: + - name: message + value: $(params.message) + taskRef: + name: param-enum-demo +``` + +Tekton validates user-provided values in a `PipelineRun` against the `enum` specified in the `PipelineSpec.params`. Tekton also validates +any resolved `param` value against the `enum` specified in each `PipelineTask` before creating the `TaskRun`. + +See usage in this [example](../examples/v1/pipelineruns/alpha/param-enum.yaml) ## Adding `Tasks` to the `Pipeline` diff --git a/docs/taskruns.md b/docs/taskruns.md index ba56b68ed1f..eeae6d776c3 100644 --- a/docs/taskruns.md +++ b/docs/taskruns.md @@ -404,9 +404,7 @@ go through the complexity of checking each `Task` and providing only the require #### 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. +> :seedling: **`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. 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. diff --git a/docs/tasks.md b/docs/tasks.md index d7a79f3feda..e6977d45956 100644 --- a/docs/tasks.md +++ b/docs/tasks.md @@ -713,11 +713,9 @@ spec: ``` #### Param enum -> :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: **`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. - -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`: +Parameter declarations can include `enum` which is a predefine set of valid values that can be accepted by the `Param`. If a `Param` has both `enum` and default value, the default value must be in the `enum` set. For example, the valid/allowed values for `Param` "message" is bounded to `v1`, `v2` and `v3`: ``` yaml apiVersion: tekton.dev/v1 @@ -729,6 +727,7 @@ spec: - name: message type: string enum: ["v1", "v2", "v3"] + default: "v1" steps: - name: build image: bash:latest diff --git a/examples/v1/pipelineruns/alpha/param-enum.yaml b/examples/v1/pipelineruns/alpha/param-enum.yaml new file mode 100644 index 00000000000..9f6758bc891 --- /dev/null +++ b/examples/v1/pipelineruns/alpha/param-enum.yaml @@ -0,0 +1,42 @@ +apiVersion: tekton.dev/v1 +kind: Task +metadata: + name: task-param-enum +spec: + params: + - name: message + type: string + enum: ["v1", "v2", "v3"] + steps: + - name: build + image: bash:latest + script: | + echo "$(params.message)" +--- +apiVersion: tekton.dev/v1 +kind: Pipeline +metadata: + name: pipeline-param-enum +spec: + params: + - name: message + enum: ["v1", "v2"] + default: "v1" + tasks: + - name: task1 + params: + - name: message + value: $(params.message) + taskRef: + name: task-param-enum +--- +apiVersion: tekton.dev/v1 +kind: PipelineRun +metadata: + name: pipelinerun-param-enum +spec: + pipelineRef: + name: pipeline-param-enum + params: + - name: message + value: "v1" diff --git a/pkg/apis/pipeline/v1/pipelinerun_types.go b/pkg/apis/pipeline/v1/pipelinerun_types.go index 26afd74faaf..c9db1bff010 100644 --- a/pkg/apis/pipeline/v1/pipelinerun_types.go +++ b/pkg/apis/pipeline/v1/pipelinerun_types.go @@ -408,6 +408,8 @@ const ( PipelineRunReasonCreateRunFailed PipelineRunReason = "CreateRunFailed" // ReasonCELEvaluationFailed indicates the pipeline fails the CEL evaluation PipelineRunReasonCELEvaluationFailed PipelineRunReason = "CELEvaluationFailed" + // PipelineRunReasonInvalidParamValue indicates that the PipelineRun Param input value is not allowed. + PipelineRunReasonInvalidParamValue PipelineRunReason = "InvalidParamValue" ) func (t PipelineRunReason) String() string { diff --git a/pkg/reconciler/pipelinerun/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 2d06ccd3ead..d0bc0901695 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -487,6 +487,16 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1.PipelineRun, getPipel return controller.NewPermanentError(err) } + if config.FromContextOrDefaults(ctx).FeatureFlags.EnableParamEnum { + if err := taskrun.ValidateEnumParam(ctx, pr.Spec.Params, pipelineSpec.Params); err != nil { + logger.Errorf("PipelineRun %q Param Enum validation failed: %v", pr.Name, err) + pr.Status.MarkFailed(v1.PipelineRunReasonInvalidParamValue.String(), + "PipelineRun %s/%s parameters have invalid value: %s", + pr.Namespace, pr.Name, err) + return controller.NewPermanentError(err) + } + } + // Ensure that the keys of an object param declared in PipelineSpec are not missed in the PipelineRunSpec if err = resources.ValidateObjectParamRequiredKeys(pipelineSpec.Params, pr.Spec.Params); err != nil { // This Run has failed, so we need to mark it as failed and stop reconciling it @@ -521,6 +531,12 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1.PipelineRun, getPipel return controller.NewPermanentError(err) } + // Make a deep copy of the Pipeline and its Tasks before value substution. + // This is used to find referenced pipeline-level params at each PipelineTask when validate param enum subset requirement + originalPipeline := pipelineSpec.DeepCopy() + originalTasks := originalPipeline.Tasks + originalTasks = append(originalTasks, originalPipeline.Finally...) + // Apply parameter substitution from the PipelineRun pipelineSpec = resources.ApplyParameters(ctx, pipelineSpec, pr) pipelineSpec = resources.ApplyContexts(pipelineSpec, pipelineMeta.Name, pr) @@ -615,7 +631,7 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1.PipelineRun, getPipel pipelineRunFacts.TimeoutsState.PipelineTimeout = &pipelineTimeout } - for _, rpt := range pipelineRunFacts.State { + for i, rpt := range pipelineRunFacts.State { if !rpt.IsCustomTask() { err := taskrun.ValidateResolvedTask(ctx, rpt.PipelineTask.Params, rpt.PipelineTask.Matrix, rpt.ResolvedTask) if err != nil { @@ -623,6 +639,14 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1.PipelineRun, getPipel pr.Status.MarkFailed(v1.PipelineRunReasonFailedValidation.String(), err.Error()) return controller.NewPermanentError(err) } + + if config.FromContextOrDefaults(ctx).FeatureFlags.EnableParamEnum { + if err := resources.ValidateParamEnumSubset(originalTasks[i].Params, pipelineSpec.Params, rpt.ResolvedTask); err != nil { + logger.Errorf("Failed to validate pipelinerun %q with error %v", pr.Name, err) + pr.Status.MarkFailed(v1.PipelineRunReasonFailedValidation.String(), err.Error()) + return controller.NewPermanentError(err) + } + } } } @@ -870,10 +894,24 @@ func (c *Reconciler) createTaskRuns(ctx context.Context, rpt *resources.Resolved defer span.End() var taskRuns []*v1.TaskRun var matrixCombinations []v1.Params - if rpt.PipelineTask.IsMatrixed() { matrixCombinations = rpt.PipelineTask.Matrix.FanOut() } + // validate the param values meet resolved Task Param Enum requirements before creating TaskRuns + if config.FromContextOrDefaults(ctx).FeatureFlags.EnableParamEnum { + for i := range rpt.TaskRunNames { + var params v1.Params + if len(matrixCombinations) > i { + params = matrixCombinations[i] + } + params = append(params, rpt.PipelineTask.Params...) + if err := taskrun.ValidateEnumParam(ctx, params, rpt.ResolvedTask.TaskSpec.Params); err != nil { + err = fmt.Errorf("Invalid param value from PipelineTask \"%s\": %w", rpt.PipelineTask.Name, err) + pr.Status.MarkFailed(v1.PipelineRunReasonInvalidParamValue.String(), err.Error()) + return nil, controller.NewPermanentError(err) + } + } + } for i, taskRunName := range rpt.TaskRunNames { var params v1.Params if len(matrixCombinations) > i { diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index e57d2883714..55b048c22ba 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -4529,6 +4529,221 @@ spec: checkPipelineRunConditionStatusAndReason(t, pipelineRun, corev1.ConditionFalse, string(v1.PipelineRunReasonCELEvaluationFailed)) } +func TestReconcile_Enum_With_Matrix_Pass(t *testing.T) { + ps := []*v1.Pipeline{parse.MustParseV1Pipeline(t, ` +metadata: + name: test-pipeline-level-enum + namespace: foo +spec: + params: + - name: version + type: string + enum: ["v1"] + - name: tag + type: string + tasks: + - name: a-task + params: + - name: version + value: $(params.version) + - name: tag + value: $(params.tag) + taskSpec: + name: a-task + params: + - name: version + enum: ["v1", "v3"] + - name: tag + steps: + - name: s1 + image: alpine + script: | + echo $(params.version) + $(params.tag) + - name: b-task + params: + - name: ref-p1 + value: $(params.version) + - name: ref-p2 + value: "v3" + taskRef: + name: ref-task + - name: c-task-matrixed + matrix: + params: + - name: ref-p1 + value: [v1, v2] + - name: ref-p2 + value: [v3, v4] + taskRef: + name: ref-task +`)} + + refTasks := []*v1.Task{parse.MustParseV1Task(t, ` +metadata: + name: ref-task + namespace: foo +spec: + params: + - name: ref-p1 + enum: ["v1", "v2"] + - name: ref-p2 + enum: ["v3", "v4"] +`)} + + prs := []*v1.PipelineRun{parse.MustParseV1PipelineRun(t, ` +metadata: + name: test-pipeline-level-enum-run + namespace: foo +spec: + params: + - name: version + value: "v1" + - name: tag + value: "t1" + pipelineRef: + name: test-pipeline-level-enum +`)} + cms := []*corev1.ConfigMap{ + { + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()}, + Data: map[string]string{ + "enable-param-enum": "true", + }, + }, + } + d := test.Data{ + Tasks: refTasks, + PipelineRuns: prs, + Pipelines: ps, + ConfigMaps: cms, + } + prt := newPipelineRunTest(t, d) + defer prt.Cancel() + pipelineRun, _ := prt.reconcileRun("foo", "test-pipeline-level-enum-run", []string{}, false) + // PipelineRun in running status indicates the param enum has passed validation + checkPipelineRunConditionStatusAndReason(t, pipelineRun, corev1.ConditionUnknown, v1.PipelineRunReasonRunning.String()) +} + +func TestReconcile_Enum_Subset_Validation_Failed(t *testing.T) { + ps := []*v1.Pipeline{parse.MustParseV1Pipeline(t, ` +metadata: + name: test-pipeline-level-enum + namespace: foo +spec: + params: + - name: version + type: string + enum: ["v1", "v2"] + tasks: + - name: a-task + params: + - name: version + value: $(params.version) + taskSpec: + name: a-task + params: + - name: version + enum: ["v1", "v3"] + steps: + - name: s1 + image: alpine + script: | + echo $(params.version) +`)} + prs := []*v1.PipelineRun{parse.MustParseV1PipelineRun(t, ` +metadata: + name: test-pipeline-level-enum-run + namespace: foo +spec: + params: + - name: version + value: "v1" + pipelineRef: + name: test-pipeline-level-enum +`)} + cms := []*corev1.ConfigMap{ + { + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()}, + Data: map[string]string{ + "enable-param-enum": "true", + }, + }, + } + d := test.Data{ + PipelineRuns: prs, + Pipelines: ps, + ConfigMaps: cms, + } + prt := newPipelineRunTest(t, d) + defer prt.Cancel() + pipelineRun, _ := prt.reconcileRun("foo", "test-pipeline-level-enum-run", []string{}, true) + checkPipelineRunConditionStatusAndReason(t, pipelineRun, corev1.ConditionFalse, string(v1.PipelineRunReasonFailedValidation)) +} + +func TestReconcile_PipelineTask_Level_Enum_Failed(t *testing.T) { + ps := []*v1.Pipeline{parse.MustParseV1Pipeline(t, ` +metadata: + name: test-pipelineTask-level-enum + namespace: foo +spec: + params: + - name: version + enum: [v1] + type: string + tasks: + - name: a-task + params: + - name: ref-version + value: $(params.version) + - name: ref-p2 + value: invalid + taskRef: + name: ref-task +`)} + prs := []*v1.PipelineRun{parse.MustParseV1PipelineRun(t, ` +metadata: + name: test-pipelineTask-level-enum-run + namespace: foo +spec: + params: + - name: version + value: "v1" + pipelineRef: + name: test-pipelineTask-level-enum +`)} + + refTasks := []*v1.Task{parse.MustParseV1Task(t, ` +metadata: + name: ref-task + namespace: foo +spec: + params: + - name: ref-version + enum: ["v1", "v2"] + - name: ref-p2 + enum: ["v3", "v4"] +`)} + + cms := []*corev1.ConfigMap{ + { + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()}, + Data: map[string]string{ + "enable-param-enum": "true", + }, + }, + } + d := test.Data{ + Tasks: refTasks, + PipelineRuns: prs, + Pipelines: ps, + ConfigMaps: cms, + } + prt := newPipelineRunTest(t, d) + defer prt.Cancel() + pipelineRun, _ := prt.reconcileRun("foo", "test-pipelineTask-level-enum-run", []string{}, true) + checkPipelineRunConditionStatusAndReason(t, pipelineRun, corev1.ConditionFalse, string(v1.PipelineRunReasonInvalidParamValue)) +} + // TestReconcileWithAffinityAssistantStatefulSet tests that given a pipelineRun with workspaces, // an Affinity Assistant StatefulSet is created for each PVC workspace and // that the Affinity Assistant names is propagated to TaskRuns. diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go index 99430adce49..e3c7652252f 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go @@ -29,6 +29,7 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources" "github.com/tektoncd/pipeline/pkg/remote" + "github.com/tektoncd/pipeline/pkg/substitution" kerrors "k8s.io/apimachinery/pkg/api/errors" "knative.dev/pkg/apis" "knative.dev/pkg/kmeta" @@ -808,3 +809,73 @@ func createResultsCacheMatrixedTaskRuns(rpt *ResolvedPipelineTask) (resultsCache } return resultsCache } + +// ValidateParamEnumSubset finds the referenced pipeline-level params in the resolved pipelineTask. +// It then validates if the referenced pipeline-level param enums are subsets of the resolved pipelineTask-level param enums +func ValidateParamEnumSubset(pipelineTaskParams []v1.Param, pipelineParamSpecs []v1.ParamSpec, rt *resources.ResolvedTask) error { + for _, p := range pipelineTaskParams { + // calculate referenced param enums + res, present, errString := substitution.ExtractVariablesFromString(p.Value.StringVal, "params") + if !present { + continue + } + if errString != "" { + return fmt.Errorf("unexpected error in ExtractVariablesFromString: %s", errString) + } + if len(res) != 1 { + return fmt.Errorf("unexpected resolved param in ExtractVariablesFromString, expect 1 but got %v", len(res)) + } + + // resolve pipeline-level and pipelineTask-level enums + paramName := substitution.TrimArrayIndex(res[0]) + pipelineParam := getParamFromName(paramName, pipelineParamSpecs) + resolvedTaskParam := getParamFromName(p.Name, rt.TaskSpec.Params) + + // param enum is only supported for string param type, + // we only validate the enum subset requirement for string typed param. + // If there is no task-level enum (allowing any value), any pipeline-level enum is allowed + if pipelineParam.Type != v1.ParamTypeString || len(resolvedTaskParam.Enum) == 0 { + return nil + } + + // if pipelin-level enum is empty (allowing any value) but task-level enum is not, it is not a "subset" + if len(pipelineParam.Enum) == 0 && len(resolvedTaskParam.Enum) > 0 { + return fmt.Errorf("pipeline param \"%s\" has no enum, but referenced in \"%s\" task has enums: %v", pipelineParam.Name, rt.TaskName, resolvedTaskParam.Enum) + } + + // validate if pipeline-level enum is a subset of pipelineTask-level enum + if isValid := isSubset(pipelineParam.Enum, resolvedTaskParam.Enum); !isValid { + return fmt.Errorf("pipeline param \"%s\" enum: %v is not a subset of the referenced in \"%s\" task param enum: %v", pipelineParam.Name, pipelineParam.Enum, rt.TaskName, resolvedTaskParam.Enum) + } + } + + return nil +} + +func isSubset(pipelineEnum, taskEnum []string) bool { + pipelineEnumMap := make(map[string]bool) + TaskEnumMap := make(map[string]bool) + for _, e := range pipelineEnum { + pipelineEnumMap[e] = true + } + for _, e := range taskEnum { + TaskEnumMap[e] = true + } + + for e := range pipelineEnumMap { + if !TaskEnumMap[e] { + return false + } + } + + return true +} + +func getParamFromName(name string, pss v1.ParamSpecs) v1.ParamSpec { + for _, ps := range pss { + if ps.Name == name { + return ps + } + } + return v1.ParamSpec{} +} diff --git a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go index cc6b38e5c5e..90118f30288 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go +++ b/pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go @@ -4999,3 +4999,254 @@ func TestEvaluateCEL_invalid(t *testing.T) { }) } } + +func TestValidateParamEnumSubset_Valid(t *testing.T) { + tcs := []struct { + name string + params []v1.Param + pipelinePs []v1.ParamSpec + rt *resources.ResolvedTask + }{ + { + name: "no enum - pass", + params: []v1.Param{ + { + Name: "resolved-task-p1", + Value: v1.ParamValue{ + StringVal: "$(params.p1)", + }, + }, + }, + pipelinePs: []v1.ParamSpec{ + { + Name: "p1", + Type: v1.ParamTypeString, + }, + }, + rt: &resources.ResolvedTask{ + TaskSpec: &v1.TaskSpec{ + Params: v1.ParamSpecs{ + { + Name: "resolved-task-p1", + }, + }, + }, + }, + }, { + name: "no task-level enum - pass", + params: []v1.Param{ + { + Name: "resolved-task-p1", + Value: v1.ParamValue{ + StringVal: "$(params.p1)", + }, + }, + }, + pipelinePs: []v1.ParamSpec{ + { + Name: "p1", + Type: v1.ParamTypeString, + Enum: []string{"v1", "v2"}, + }, + }, + rt: &resources.ResolvedTask{ + TaskSpec: &v1.TaskSpec{ + Params: v1.ParamSpecs{ + { + Name: "resolved-task-p1", + }, + }, + }, + }, + }, { + name: "task and pipeline level enum - pass", + params: []v1.Param{ + { + Name: "resolved-task-p1", + Value: v1.ParamValue{ + StringVal: "$(params.p1)", + }, + }, { + Name: "resolved-task-p2", + Value: v1.ParamValue{ + StringVal: "$(params.p2)", + }, + }, + }, + pipelinePs: []v1.ParamSpec{ + { + Name: "p1", + Type: v1.ParamTypeString, + Enum: []string{"v1", "v2"}, + }, + { + Name: "p2", + Type: v1.ParamTypeString, + Enum: []string{"v3", "v4"}, + }, + }, + rt: &resources.ResolvedTask{ + TaskSpec: &v1.TaskSpec{ + Params: v1.ParamSpecs{ + { + Name: "resolved-task-p1", + Enum: []string{"v1", "v2", "v3"}, + }, + { + Name: "resolved-task-p2", + Enum: []string{"v3", "v4"}, + }, + }, + }, + }, + }, { + name: "no referenced param enum - pass", + params: []v1.Param{ + { + Name: "resolved-task-p1", + Value: v1.ParamValue{ + StringVal: "v1", + }, + }, { + Name: "resolved-task-p2", + Value: v1.ParamValue{ + StringVal: "$(task1.results.r1)", + }, + }, + }, + }, { + name: "non-string typed param - pass", + params: []v1.Param{ + { + Name: "resolved-task-p1", + Value: v1.ParamValue{ + StringVal: "$(params.object1.key)", + }, + }, { + Name: "resolved-task-p2", + Value: v1.ParamValue{ + StringVal: "$(params.array1[0])", + }, + }, + }, + pipelinePs: []v1.ParamSpec{ + { + Name: "array1", + Type: v1.ParamTypeArray, + }, + { + Name: "object1", + Type: v1.ParamTypeObject, + }, + }, + rt: &resources.ResolvedTask{ + TaskSpec: &v1.TaskSpec{ + Params: v1.ParamSpecs{ + { + Name: "resolved-task-p1", + Enum: []string{"v1", "v2"}, + }, + { + Name: "resolved-task-p2", + Enum: []string{"v3", "v4"}, + }, + }, + }, + }, + }, + } + + for _, tc := range tcs { + if err := ValidateParamEnumSubset(tc.params, tc.pipelinePs, tc.rt); err != nil { + t.Errorf("unexpected error in ValidateParamEnumSubset: %s", err) + } + } +} + +func TestValidateParamEnumSubset_Invalid(t *testing.T) { + tcs := []struct { + name string + params []v1.Param + pipelinePs []v1.ParamSpec + rt *resources.ResolvedTask + wantErr error + }{{ + name: "pipeline enum is not a subset - fail", + params: []v1.Param{ + { + Name: "resolved-task-p1", + Value: v1.ParamValue{ + StringVal: "$(params.p1)", + }, + }, + }, + pipelinePs: []v1.ParamSpec{ + { + Name: "p1", + Type: v1.ParamTypeString, + Enum: []string{"v1", "v2"}, + }, + }, + rt: &resources.ResolvedTask{ + TaskName: "ref1", + TaskSpec: &v1.TaskSpec{ + Params: v1.ParamSpecs{ + { + Name: "resolved-task-p1", + Enum: []string{"v1", "v3"}, + }, + }, + }, + }, + wantErr: fmt.Errorf("pipeline param \"p1\" enum: [v1 v2] is not a subset of the referenced in \"ref1\" task param enum: [v1 v3]"), + }, { + name: "empty pipeline enum with non-empty task enum - fail", + params: []v1.Param{ + { + Name: "resolved-task-p1", + Value: v1.ParamValue{ + StringVal: "$(params.p1)", + }, + }, + }, + pipelinePs: []v1.ParamSpec{ + { + Name: "p1", + Type: v1.ParamTypeString, + }, + }, + rt: &resources.ResolvedTask{ + TaskName: "ref1", + TaskSpec: &v1.TaskSpec{ + Params: v1.ParamSpecs{ + { + Name: "resolved-task-p1", + Enum: []string{"v1", "v3"}, + }, + }, + }, + }, + wantErr: fmt.Errorf("pipeline param \"p1\" has no enum, but referenced in \"ref1\" task has enums: [v1 v3]"), + }, { + name: "invalid param syntax - failure", + params: []v1.Param{ + { + Name: "resolved-task-p1", + Value: v1.ParamValue{ + StringVal: "$(params.p1.aaa.bbb)", + }, + }, + }, + wantErr: fmt.Errorf("unexpected error in ExtractVariablesFromString: Invalid referencing of parameters in \"$(params.p1.aaa.bbb)\"! Only two dot-separated components after the prefix \"params\" are allowed."), + }} + + for _, tc := range tcs { + err := ValidateParamEnumSubset(tc.params, tc.pipelinePs, tc.rt) + if err == nil { + t.Errorf("expecting error in ValidateParamEnumSubset: %s, but got nil", err) + } + if d := cmp.Diff(tc.wantErr.Error(), err.Error()); d != "" { + t.Errorf("expecting error does not match in ValidateParamEnumSubset: %s", diff.PrintWantGot(d)) + } + } +}