Skip to content

Commit

Permalink
Fix enum validation with multiple param references
Browse files Browse the repository at this point in the history
Fixes [#7476][7476]. TEP-0144 requires that the pipeline-level `enum` must be a subset of referenced task-level `enum`.

Prior to this commit, the enum subset validation logic assumes that a task-level param only referenced only one pipeline-level `enum`,
and does not support scenario where multiple pipeline-level `enums` are referenced (e.g., "$(params.p1) and $(params.p2)").

This commit adds the handling logic for such compound references, skipping the subset validation in this scenario as there is no directly associated
params at pipeline level.

/kind bug

[7476]: #7476
  • Loading branch information
QuanZhang-William authored and tekton-robot committed Dec 14, 2023
1 parent 5c47c88 commit 8e9fd46
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 7 deletions.
29 changes: 27 additions & 2 deletions docs/pipelines.md
Original file line number Diff line number Diff line change
Expand Up @@ -301,8 +301,10 @@ spec:
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`.
a **subset** of the `enums` specified in the referenced `Task` (task-level `enum`). An empty pipeline-level `enum` is invalid
in this scenario since an empty `enum` set indicates a "universal set" which allows all possible values. The same rules apply to `Pipelines` with embbeded `Tasks`.

In the below example, the referenced `Task` accepts `v1` and `v2` as valid values, the `Pipeline` further restricts the valid value to `v1`.

``` yaml
apiVersion: tekton.dev/v1
Expand Down Expand Up @@ -339,6 +341,29 @@ spec:
name: param-enum-demo
```

Note that this subset restriction only applies to the task-level `params` with a **direct single** reference to pipeline-level `params`. If a task-level `param` references multiple pipeline-level `params`, the subset validation is not applied.

``` yaml
apiVersion: tekton.dev/v1
kind: Pipeline
...
spec:
params:
- name: message1
enum: ["v1"]
- name: message2
enum: ["v2"]
tasks:
- name: task1
params:
- name: message
value: "$(params.message1) and $(params.message2)"
taskSpec:
params: message
enum: [...] # the message enum is not required to be a subset of message1 or message2
...
```

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`.

Expand Down
9 changes: 4 additions & 5 deletions pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go
Original file line number Diff line number Diff line change
Expand Up @@ -818,14 +818,13 @@ func ValidateParamEnumSubset(pipelineTaskParams []v1.Param, pipelineParamSpecs [
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))

// if multiple params are extracted, that means the task-level param is a compounded value, skip subset validation
if !present || len(res) > 1 {
continue
}

// resolve pipeline-level and pipelineTask-level enums
Expand Down
32 changes: 32 additions & 0 deletions pkg/reconciler/pipelinerun/resources/pipelinerunresolution_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5153,6 +5153,38 @@ func TestValidateParamEnumSubset_Valid(t *testing.T) {
},
},
},
}, {
name: "compound task param with enum - pass",
params: []v1.Param{
{
Name: "resolved-task-p1",
Value: v1.ParamValue{
StringVal: "$(params.p1) and $(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{"e1", "e2"},
},
},
},
},
},
}

Expand Down

0 comments on commit 8e9fd46

Please sign in to comment.