Skip to content

Commit

Permalink
[TEP-0144] Validate TaskRun for Param Enum
Browse files Browse the repository at this point in the history
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 TaskRun against Param Enum

/kind feature

[#7270]: #7270
[tep-0144]: https://github.com/tektoncd/community/blob/main/teps/0144-param-enum.md
  • Loading branch information
QuanZhang-William committed Nov 2, 2023
1 parent 8fd372f commit 6833e8d
Show file tree
Hide file tree
Showing 15 changed files with 382 additions and 1 deletion.
10 changes: 10 additions & 0 deletions docs/taskruns.md
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,16 @@ 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.

See more details in [Param.Enum](./tasks.md#param-enum).

### Specifying `Resource` limits

Each Step in a Task can specify its resource requirements. See
Expand Down
23 changes: 22 additions & 1 deletion docs/tasks.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`

Expand Down
25 changes: 25 additions & 0 deletions examples/v1/taskruns/alpha/param-enum.yaml
Original file line number Diff line number Diff line change
@@ -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"
6 changes: 6 additions & 0 deletions pkg/apis/pipeline/v1/param_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
}
Expand Down
15 changes: 15 additions & 0 deletions pkg/apis/pipeline/v1/task_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 2 additions & 0 deletions pkg/apis/pipeline/v1/taskrun_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 6 additions & 0 deletions pkg/apis/pipeline/v1beta1/param_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -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
}
Expand Down
15 changes: 15 additions & 0 deletions pkg/apis/pipeline/v1beta1/task_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
6 changes: 6 additions & 0 deletions pkg/reconciler/taskrun/taskrun.go
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,12 @@ func (c *Reconciler) prepare(ctx context.Context, tr *v1.TaskRun) (*v1.TaskSpec,
return nil, nil, controller.NewPermanentError(err)
}

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)
Expand Down
108 changes: 108 additions & 0 deletions pkg/reconciler/taskrun/taskrun_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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:
Expand Down
25 changes: 25 additions & 0 deletions pkg/reconciler/taskrun/validate_taskrun.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -174,6 +175,30 @@ 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.
// An 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
}
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 {
Expand Down
Loading

0 comments on commit 6833e8d

Please sign in to comment.