Skip to content

Commit 3950963

Browse files
[TEP-0144] Validate TaskRun for Param Enum
Part of [tektoncd#7270][tektoncd#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 TaskRun against Param Enum /kind feature [tektoncd#7270]: tektoncd#7270 [tep-0144]: https://github.com/tektoncd/community/blob/main/teps/0144-param-enum.md
1 parent 8fd372f commit 3950963

15 files changed

+303
-7
lines changed

docs/taskruns.md

+10
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,16 @@ case is when your CI system autogenerates `TaskRuns` and it has `Parameters` it
402402
provide to all `TaskRuns`. Because you can pass in extra `Parameters`, you don't have to
403403
go through the complexity of checking each `Task` and providing only the required params.
404404

405+
#### Parameter Enums
406+
407+
> :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.
408+
409+
> :seedling: This feature is WIP and not yet supported/implemented. Documentation to be completed.
410+
411+
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.
412+
413+
See more details in [Param.Enum](./tasks.md#param-enum).
414+
405415
### Specifying `Resource` limits
406416

407417
Each Step in a Task can specify its resource requirements. See

docs/tasks.md

+20-1
Original file line numberDiff line numberDiff line change
@@ -717,7 +717,26 @@ spec:
717717

718718
> :seedling: This feature is WIP and not yet supported/implemented. Documentation to be completed.
719719

720-
Parameter declarations can include `enum` which is a predefine set of valid values that can be accepted by the `Task`.
720+
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`:
721+
722+
``` yaml
723+
apiVersion: tekton.dev/v1
724+
kind: Task
725+
metadata:
726+
name: param-enum-demo
727+
spec:
728+
params:
729+
- name: message
730+
type: string
731+
enum: ["v1", "v2", "v3"]
732+
steps:
733+
- name: build
734+
image: bash:latest
735+
script: |
736+
echo "$(params.message)"
737+
```
738+
739+
If the `Param` value passed in by `TaskRuns` is **NOT** in the predefined `enum` list, the `TaskRuns` will fail with reason `InvalidParamValue`.
721740

722741
### Specifying `Workspaces`
723742

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
apiVersion: tekton.dev/v1
2+
kind: Task
3+
metadata:
4+
name: task-param-enum
5+
spec:
6+
params:
7+
- name: message
8+
type: string
9+
enum: ["v1", "v2", "v3"]
10+
steps:
11+
- name: build
12+
image: bash:3.2
13+
script: |
14+
echo "$(params.message)"
15+
---
16+
apiVersion: tekton.dev/v1
17+
kind: TaskRun
18+
metadata:
19+
name: taskrun-param-enum
20+
spec:
21+
taskRef:
22+
name: task-param-enum
23+
params:
24+
- name: message
25+
value: "v1"

pkg/apis/pipeline/v1/param_types.go

+7-1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"github.com/tektoncd/pipeline/pkg/substitution"
2727
corev1 "k8s.io/api/core/v1"
2828
"k8s.io/apimachinery/pkg/util/sets"
29+
"k8s.io/utils/strings/slices"
2930
"knative.dev/pkg/apis"
3031
)
3132

@@ -153,14 +154,19 @@ func (ps ParamSpecs) validateParamEnums(ctx context.Context) *apis.FieldError {
153154
continue
154155
}
155156
if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableParamEnum {
156-
errs = errs.Also(errs, apis.ErrGeneric(fmt.Sprintf("feature flag `%s` should be set to true to use Enum", config.EnableParamEnum), "").ViaKey(p.Name))
157+
errs = errs.Also(errs, apis.ErrGeneric(fmt.Sprintf("feature flag %s should be set to true to use Enum", config.EnableParamEnum), "").ViaKey(p.Name))
157158
}
158159
if p.Type != ParamTypeString {
159160
errs = errs.Also(apis.ErrGeneric("enum can only be set with string type param", "").ViaKey(p.Name))
160161
}
161162
for dup := range findDups(p.Enum) {
162163
errs = errs.Also(apis.ErrGeneric(fmt.Sprintf("parameter enum value %v appears more than once", dup), "").ViaKey(p.Name))
163164
}
165+
if p.Default != nil && p.Default.StringVal != "" {
166+
if !slices.Contains(p.Enum, p.Default.StringVal) {
167+
errs = errs.Also(apis.ErrGeneric(fmt.Sprintf("param default value %v not in the enum list", p.Default.StringVal), "").ViaKey(p.Name))
168+
}
169+
}
164170
}
165171
return errs
166172
}

pkg/apis/pipeline/v1/pipeline_validation_test.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -1300,9 +1300,9 @@ func TestValidatePipelineParameterVariables_Success(t *testing.T) {
13001300
tasks []PipelineTask
13011301
configMap map[string]string
13021302
}{{
1303-
name: "valid string parameter variables",
1303+
name: "valid string parameter variables with enum",
13041304
params: []ParamSpec{{
1305-
Name: "baz", Type: ParamTypeString,
1305+
Name: "baz", Type: ParamTypeString, Enum: []string{"v1", "v2"},
13061306
}, {
13071307
Name: "foo-is-baz", Type: ParamTypeString,
13081308
}},
@@ -1313,6 +1313,7 @@ func TestValidatePipelineParameterVariables_Success(t *testing.T) {
13131313
Name: "a-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.baz) and $(params.foo-is-baz)"},
13141314
}},
13151315
}},
1316+
configMap: map[string]string{"enable-param-enum": "true"},
13161317
}, {
13171318
name: "valid string parameter variables with enum",
13181319
params: []ParamSpec{{

pkg/apis/pipeline/v1/taskrun_types.go

+2
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,8 @@ const (
185185
TaskRunReasonResultLargerThanAllowedLimit TaskRunReason = "TaskRunResultLargerThanAllowedLimit"
186186
// TaskRunReasonStopSidecarFailed indicates that the sidecar is not properly stopped.
187187
TaskRunReasonStopSidecarFailed = "TaskRunStopSidecarFailed"
188+
// TaskRunReasonInvalidParamValue indicates that the TaskRun Param input value is not allowed.
189+
TaskRunReasonInvalidParamValue = "InvalidParamValue"
188190
)
189191

190192
func (t TaskRunReason) String() string {

pkg/apis/pipeline/v1beta1/param_types.go

+7-1
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"github.com/tektoncd/pipeline/pkg/substitution"
2727
corev1 "k8s.io/api/core/v1"
2828
"k8s.io/apimachinery/pkg/util/sets"
29+
"k8s.io/utils/strings/slices"
2930
"knative.dev/pkg/apis"
3031
)
3132

@@ -144,14 +145,19 @@ func (ps ParamSpecs) validateParamEnums(ctx context.Context) *apis.FieldError {
144145
continue
145146
}
146147
if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableParamEnum {
147-
errs = errs.Also(errs, apis.ErrGeneric(fmt.Sprintf("feature flag `%s` should be set to true to use Enum", config.EnableParamEnum), "").ViaKey(p.Name))
148+
errs = errs.Also(errs, apis.ErrGeneric(fmt.Sprintf("feature flag %s should be set to true to use Enum", config.EnableParamEnum), "").ViaKey(p.Name))
148149
}
149150
if p.Type != ParamTypeString {
150151
errs = errs.Also(apis.ErrGeneric("enum can only be set with string type param", "").ViaKey(p.Name))
151152
}
152153
for dup := range findDups(p.Enum) {
153154
errs = errs.Also(apis.ErrGeneric(fmt.Sprintf("parameter enum value %v appears more than once", dup), "").ViaKey(p.Name))
154155
}
156+
if p.Default != nil && p.Default.StringVal != "" {
157+
if !slices.Contains(p.Enum, p.Default.StringVal) {
158+
errs = errs.Also(apis.ErrGeneric(fmt.Sprintf("param default value %v not in the enum list", p.Default.StringVal), "").ViaKey(p.Name))
159+
}
160+
}
155161
}
156162
return errs
157163
}

pkg/apis/pipeline/v1beta1/pipeline_validation_test.go

+3-2
Original file line numberDiff line numberDiff line change
@@ -1343,9 +1343,9 @@ func TestValidatePipelineParameterVariables_Success(t *testing.T) {
13431343
tasks []PipelineTask
13441344
configMap map[string]string
13451345
}{{
1346-
name: "valid string parameter variables",
1346+
name: "valid string parameter variables with enum",
13471347
params: []ParamSpec{{
1348-
Name: "baz", Type: ParamTypeString,
1348+
Name: "baz", Type: ParamTypeString, Enum: []string{"v1", "v2"},
13491349
}, {
13501350
Name: "foo-is-baz", Type: ParamTypeString,
13511351
}},
@@ -1356,6 +1356,7 @@ func TestValidatePipelineParameterVariables_Success(t *testing.T) {
13561356
Name: "a-param", Value: ParamValue{Type: ParamTypeString, StringVal: "$(params.baz) and $(params.foo-is-baz)"},
13571357
}},
13581358
}},
1359+
configMap: map[string]string{"enable-param-enum": "true"},
13591360
}, {
13601361
name: "valid string parameter variables with enum",
13611362
params: []ParamSpec{{

pkg/reconciler/taskrun/taskrun.go

+6
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,12 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1.TaskRun) (*v1.TaskSpec,
404404
return nil, nil, controller.NewPermanentError(err)
405405
}
406406

407+
if err := ValidateEnumParam(ctx, tr.Spec.Params, rtr.TaskSpec.Params); err != nil {
408+
logger.Errorf("TaskRun %q Param Enum validation failed: %v", tr.Name, err)
409+
tr.Status.MarkResourceFailed(v1.TaskRunReasonInvalidParamValue, err)
410+
return nil, nil, controller.NewPermanentError(err)
411+
}
412+
407413
if err := resources.ValidateParamArrayIndex(rtr.TaskSpec, tr.Spec.Params); err != nil {
408414
logger.Errorf("TaskRun %q Param references are invalid: %v", tr.Name, err)
409415
tr.Status.MarkResourceFailed(podconvert.ReasonFailedValidation, err)

pkg/reconciler/taskrun/taskrun_test.go

+104
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,20 @@ var (
145145
Steps: []v1.Step{simpleStep},
146146
},
147147
}
148+
simpleTaskWithParamEnum = &v1.Task{
149+
ObjectMeta: objectMeta("test-task-param-enum", "foo"),
150+
TypeMeta: metav1.TypeMeta{
151+
APIVersion: "tekton.dev/v1",
152+
Kind: "Task",
153+
},
154+
Spec: v1.TaskSpec{
155+
Params: []v1.ParamSpec{{
156+
Name: "param1",
157+
Enum: []string{"v1", "v2"},
158+
}},
159+
Steps: []v1.Step{simpleStep},
160+
},
161+
}
148162
resultsTask = &v1.Task{
149163
ObjectMeta: objectMeta("test-results-task", "foo"),
150164
Spec: v1.TaskSpec{
@@ -4999,6 +5013,96 @@ status:
49995013
}
50005014
}
50015015

5016+
func TestReconcile_TaskRunWithParam_Enum_valid(t *testing.T) {
5017+
taskRunWithParamValid := parse.MustParseV1TaskRun(t, `
5018+
metadata:
5019+
name: test-taskrun-with-param-enum-valid
5020+
namespace: foo
5021+
spec:
5022+
params:
5023+
- name: param1
5024+
value: v1
5025+
taskRef:
5026+
name: test-task-param-enum
5027+
`)
5028+
5029+
d := test.Data{
5030+
TaskRuns: []*v1.TaskRun{taskRunWithParamValid},
5031+
Tasks: []*v1.Task{simpleTaskWithParamEnum},
5032+
ConfigMaps: []*corev1.ConfigMap{{
5033+
ObjectMeta: metav1.ObjectMeta{Namespace: system.Namespace(), Name: config.GetFeatureFlagsConfigName()},
5034+
Data: map[string]string{
5035+
"enable-param-enum": "true",
5036+
},
5037+
}},
5038+
}
5039+
5040+
testAssets, cancel := getTaskRunController(t, d)
5041+
defer cancel()
5042+
createServiceAccount(t, testAssets, taskRunWithParamValid.Spec.ServiceAccountName, taskRunWithParamValid.Namespace)
5043+
5044+
// Reconcile the TaskRun
5045+
if err := testAssets.Controller.Reconciler.Reconcile(testAssets.Ctx, getRunName(taskRunWithParamValid)); err == nil {
5046+
t.Error("wanted a wrapped requeue error, but got nil.")
5047+
} else if ok, _ := controller.IsRequeueKey(err); !ok {
5048+
t.Errorf("expected no error. Got error %v", err)
5049+
}
5050+
5051+
tr, err := testAssets.Clients.Pipeline.TektonV1().TaskRuns(taskRunWithParamValid.Namespace).Get(testAssets.Ctx, taskRunWithParamValid.Name, metav1.GetOptions{})
5052+
if err != nil {
5053+
t.Fatalf("getting updated taskrun: %v", err)
5054+
}
5055+
condition := tr.Status.GetCondition(apis.ConditionSucceeded)
5056+
if condition.Type != apis.ConditionSucceeded || condition.Reason == string(corev1.ConditionFalse) {
5057+
t.Errorf("Expected TaskRun to succeed but it did not. Final conditions were:\n%#v", tr.Status.Conditions)
5058+
}
5059+
}
5060+
5061+
func TestReconcile_TaskRunWithParam_Enum_invalid(t *testing.T) {
5062+
taskRunWithParamInvalid := parse.MustParseV1TaskRun(t, `
5063+
metadata:
5064+
name: test-taskrun-with-param-enum-invalid
5065+
namespace: foo
5066+
spec:
5067+
params:
5068+
- name: param1
5069+
value: invalid
5070+
taskRef:
5071+
name: test-task-param-enum
5072+
`)
5073+
5074+
d := test.Data{
5075+
TaskRuns: []*v1.TaskRun{taskRunWithParamInvalid},
5076+
Tasks: []*v1.Task{simpleTaskWithParamEnum},
5077+
ConfigMaps: []*corev1.ConfigMap{{
5078+
ObjectMeta: metav1.ObjectMeta{Namespace: system.Namespace(), Name: config.GetFeatureFlagsConfigName()},
5079+
Data: map[string]string{
5080+
"enable-param-enum": "true",
5081+
},
5082+
}},
5083+
}
5084+
5085+
expectedErr := fmt.Errorf("1 error occurred:\n\t* param `param1` value: invalid is not in the enum list")
5086+
expectedFailureReason := "InvalidParamValue"
5087+
testAssets, cancel := getTaskRunController(t, d)
5088+
defer cancel()
5089+
createServiceAccount(t, testAssets, taskRunWithParamInvalid.Spec.ServiceAccountName, taskRunWithParamInvalid.Namespace)
5090+
5091+
// Reconcile the TaskRun
5092+
err := testAssets.Controller.Reconciler.Reconcile(testAssets.Ctx, getRunName(taskRunWithParamInvalid))
5093+
if d := cmp.Diff(expectedErr.Error(), strings.TrimSuffix(err.Error(), "\n\n")); d != "" {
5094+
t.Errorf("Expected: %v, but Got: %v", expectedErr, err)
5095+
}
5096+
tr, err := testAssets.Clients.Pipeline.TektonV1().TaskRuns(taskRunWithParamInvalid.Namespace).Get(testAssets.Ctx, taskRunWithParamInvalid.Name, metav1.GetOptions{})
5097+
if err != nil {
5098+
t.Fatalf("Error getting updated taskrun: %v", err)
5099+
}
5100+
condition := tr.Status.GetCondition(apis.ConditionSucceeded)
5101+
if condition.Type != apis.ConditionSucceeded || condition.Status != corev1.ConditionFalse || condition.Reason != expectedFailureReason {
5102+
t.Errorf("Expected TaskRun to fail with reason \"%s\" but it did not. Final conditions were:\n%#v", expectedFailureReason, tr.Status.Conditions)
5103+
}
5104+
}
5105+
50025106
func TestReconcile_validateTaskRunResults_valid(t *testing.T) {
50035107
taskRunResultsTypeMatched := parse.MustParseV1TaskRun(t, `
50045108
metadata:

pkg/reconciler/taskrun/validate_taskrun.go

+21
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources"
2929

3030
"k8s.io/apimachinery/pkg/util/sets"
31+
"k8s.io/utils/strings/slices"
3132
)
3233

3334
// validateParams validates that all Pipeline Task, Matrix.Params and Matrix.Include parameters all have values, match the specified
@@ -174,6 +175,26 @@ func ValidateResolvedTask(ctx context.Context, params []v1.Param, matrix *v1.Mat
174175
return nil
175176
}
176177

178+
func ValidateEnumParam(ctx context.Context, params []v1.Param, paramSpec v1.ParamSpecs) error {
179+
paramMap := map[string]string{}
180+
for _, param := range params {
181+
if param.Value.StringVal == "" {
182+
continue
183+
}
184+
paramMap[param.Name] = param.Value.StringVal
185+
}
186+
187+
for _, param := range paramSpec {
188+
if len(param.Enum) == 0 {
189+
continue
190+
}
191+
if !slices.Contains(param.Enum, paramMap[param.Name]) {
192+
return fmt.Errorf("param `%s` value: %s is not in the enum list", param.Name, paramMap[param.Name])
193+
}
194+
}
195+
return nil
196+
}
197+
177198
func validateTaskSpecRequestResources(taskSpec *v1.TaskSpec) error {
178199
if taskSpec != nil {
179200
for _, step := range taskSpec.Steps {

0 commit comments

Comments
 (0)