Validate beta features for inline pipelines and tasks

Prior to this commit, validation of beta features was performed for v1 referenced Tasks and Pipelines (both local and remote), but not v1 inline Tasks and Pipelines. This commit ensures that beta feature validation is the same for both inline and referenced Tasks and Pipelines.
JeromeJu · Aug 30, 2023 · 8c6bd54 · 8c6bd54
1 parent 5da863a
commit 8c6bd54
Show file tree

Hide file tree

Showing 4 changed files with 284 additions and 0 deletions.
diff --git a/pkg/apis/pipeline/v1/pipelinerun_validation.go b/pkg/apis/pipeline/v1/pipelinerun_validation.go
@@ -67,6 +67,11 @@ func (ps *PipelineRunSpec) Validate(ctx context.Context) (errs *apis.FieldError)
 	// Validate PipelineSpec if it's present
 	if ps.PipelineSpec != nil {
 		errs = errs.Also(ps.PipelineSpec.Validate(ctx).ViaField("pipelineSpec"))
+		// Validate beta fields separately for inline Pipeline definitions.
+		// This prevents validation from failing in the reconciler when a Pipeline is converted to a different API version.
+		// See https://github.com/tektoncd/pipeline/issues/6616 for more information.
+		// TODO(#6592): Decouple API versioning from feature versioning
+		errs = errs.Also(ps.PipelineSpec.ValidateBetaFields(ctx).ViaField("pipelineSpec"))
 	}
 
 	// Validate PipelineRun parameters

diff --git a/pkg/apis/pipeline/v1/pipelinerun_validation_test.go b/pkg/apis/pipeline/v1/pipelinerun_validation_test.go
@@ -1409,3 +1409,205 @@ func TestPipelineRunWithTimeout_Validate(t *testing.T) {
 		})
 	}
 }
+
+func TestPipelineRunSpecBetaFeatures(t *testing.T) {
+	tts := []struct {
+		name string
+		spec v1.PipelineSpec
+	}{{
+		name: "array indexing in Tasks",
+		spec: v1.PipelineSpec{
+			Params: []v1.ParamSpec{
+				{Name: "first-param", Type: v1.ParamTypeArray, Default: v1.NewStructuredValues("default-value", "default-value-again")},
+			},
+			Tasks: []v1.PipelineTask{{
+				Name: "foo",
+				Params: v1.Params{
+					{Name: "first-task-first-param", Value: *v1.NewStructuredValues("$(params.first-param[0])")},
+				},
+				TaskRef: &v1.TaskRef{Name: "foo"},
+			}},
+		},
+	}, {
+		name: "array indexing in Finally",
+		spec: v1.PipelineSpec{
+			Params: []v1.ParamSpec{
+				{Name: "first-param", Type: v1.ParamTypeArray, Default: v1.NewStructuredValues("default-value", "default-value-again")},
+			},
+			Tasks: []v1.PipelineTask{{
+				Name:    "foo",
+				TaskRef: &v1.TaskRef{Name: "foo"},
+			}},
+			Finally: []v1.PipelineTask{{
+				Name: "bar",
+				Params: v1.Params{
+					{Name: "first-task-first-param", Value: *v1.NewStructuredValues("$(params.first-param[0])")},
+				},
+				TaskRef: &v1.TaskRef{Name: "bar"},
+			}},
+		},
+	}, {
+		name: "pipeline tasks - use of resolver without the feature flag set",
+		spec: v1.PipelineSpec{
+			Tasks: []v1.PipelineTask{{
+				Name:    "uses-resolver",
+				TaskRef: &v1.TaskRef{ResolverRef: v1.ResolverRef{Resolver: "bar"}},
+			}},
+		},
+	}, {
+		name: "pipeline tasks - use of resolver params without the feature flag set",
+		spec: v1.PipelineSpec{
+			Tasks: []v1.PipelineTask{{
+				Name:    "uses-resolver-params",
+				TaskRef: &v1.TaskRef{ResolverRef: v1.ResolverRef{Resolver: "bar", Params: v1.Params{{}}}},
+			}},
+		},
+	}, {
+		name: "finally tasks - use of resolver without the feature flag set",
+		spec: v1.PipelineSpec{
+			Tasks: []v1.PipelineTask{{
+				Name:    "valid-pipeline-task",
+				TaskRef: &v1.TaskRef{Name: "foo-task"},
+			}},
+			Finally: []v1.PipelineTask{{
+				Name:    "uses-resolver",
+				TaskRef: &v1.TaskRef{ResolverRef: v1.ResolverRef{Resolver: "bar"}},
+			}},
+		},
+	}, {
+		name: "finally tasks - use of resolver params without the feature flag set",
+		spec: v1.PipelineSpec{
+			Tasks: []v1.PipelineTask{{
+				Name:    "valid-pipeline-task",
+				TaskRef: &v1.TaskRef{Name: "foo-task"},
+			}},
+			Finally: []v1.PipelineTask{{
+				Name:    "uses-resolver-params",
+				TaskRef: &v1.TaskRef{ResolverRef: v1.ResolverRef{Resolver: "bar", Params: v1.Params{{}}}},
+			}},
+		},
+	}, {
+		name: "object params",
+		spec: v1.PipelineSpec{
+			Params: []v1.ParamSpec{
+				{Name: "first-param", Type: v1.ParamTypeObject, Properties: map[string]v1.PropertySpec{}},
+			},
+			Tasks: []v1.PipelineTask{{
+				Name:    "foo",
+				TaskRef: &v1.TaskRef{Name: "foo"},
+			}},
+		},
+	}, {
+		name: "object params in Tasks",
+		spec: v1.PipelineSpec{
+			Tasks: []v1.PipelineTask{{
+				Name: "valid-pipeline-task",
+				TaskSpec: &v1.EmbeddedTask{TaskSpec: v1.TaskSpec{
+					Steps:  []v1.Step{{Image: "busybox", Script: "echo hello"}},
+					Params: []v1.ParamSpec{{Name: "my-object-param", Type: v1.ParamTypeObject, Properties: map[string]v1.PropertySpec{}}},
+				}},
+			}},
+		},
+	}, {
+		name: "object params in Finally",
+		spec: v1.PipelineSpec{
+			Tasks: []v1.PipelineTask{{
+				Name:    "foo",
+				TaskRef: &v1.TaskRef{Name: "foo"},
+			}},
+			Finally: []v1.PipelineTask{{
+				Name: "valid-finally-task",
+				TaskSpec: &v1.EmbeddedTask{TaskSpec: v1.TaskSpec{
+					Steps:  []v1.Step{{Image: "busybox", Script: "echo hello"}},
+					Params: []v1.ParamSpec{{Name: "my-object-param", Type: v1.ParamTypeObject, Properties: map[string]v1.PropertySpec{}}},
+				}},
+			}},
+		},
+	}, {
+		name: "array results",
+		spec: v1.PipelineSpec{
+			Tasks: []v1.PipelineTask{{
+				Name:    "valid-pipeline-task",
+				TaskRef: &v1.TaskRef{Name: "foo-task"},
+			}},
+			Results: []v1.PipelineResult{{Name: "my-array-result", Type: v1.ResultsTypeArray, Value: *v1.NewStructuredValues("$(tasks.valid-pipeline-task.results.foo[*])")}},
+		},
+	}, {
+		name: "array results in Tasks",
+		spec: v1.PipelineSpec{
+			Tasks: []v1.PipelineTask{{
+				Name: "valid-pipeline-task",
+				TaskSpec: &v1.EmbeddedTask{TaskSpec: v1.TaskSpec{
+					Steps:   []v1.Step{{Image: "busybox", Script: "echo hello"}},
+					Results: []v1.TaskResult{{Name: "my-array-result", Type: v1.ResultsTypeArray}},
+				}},
+			}},
+		},
+	}, {
+		name: "array results in Finally",
+		spec: v1.PipelineSpec{
+			Tasks: []v1.PipelineTask{{
+				Name:    "valid-pipeline-task",
+				TaskRef: &v1.TaskRef{Name: "foo-task"},
+			}},
+			Finally: []v1.PipelineTask{{
+				Name: "valid-finally-task",
+				TaskSpec: &v1.EmbeddedTask{TaskSpec: v1.TaskSpec{
+					Steps:   []v1.Step{{Image: "busybox", Script: "echo hello"}},
+					Results: []v1.TaskResult{{Name: "my-array-result", Type: v1.ResultsTypeArray}},
+				}},
+			}},
+		},
+	}, {
+		name: "object results",
+		spec: v1.PipelineSpec{
+			Tasks: []v1.PipelineTask{{
+				Name:    "valid-pipeline-task",
+				TaskRef: &v1.TaskRef{Name: "foo-task"},
+			}},
+			Results: []v1.PipelineResult{{Name: "my-object-result", Type: v1.ResultsTypeObject, Value: *v1.NewStructuredValues("$(tasks.valid-pipeline-task.results.foo[*])")}},
+		},
+	}, {
+		name: "object results in Tasks",
+		spec: v1.PipelineSpec{
+			Tasks: []v1.PipelineTask{{
+				Name: "valid-pipeline-task",
+				TaskSpec: &v1.EmbeddedTask{TaskSpec: v1.TaskSpec{
+					Steps:   []v1.Step{{Image: "busybox", Script: "echo hello"}},
+					Results: []v1.TaskResult{{Name: "my-object-result", Type: v1.ResultsTypeObject, Properties: map[string]v1.PropertySpec{}}},
+				}},
+			}},
+		},
+	}, {
+		name: "object results in Finally",
+		spec: v1.PipelineSpec{
+			Tasks: []v1.PipelineTask{{
+				Name:    "valid-pipeline-task",
+				TaskRef: &v1.TaskRef{Name: "foo-task"},
+			}},
+			Finally: []v1.PipelineTask{{
+				Name: "valid-finally-task",
+				TaskSpec: &v1.EmbeddedTask{TaskSpec: v1.TaskSpec{
+					Steps:   []v1.Step{{Image: "busybox", Script: "echo hello"}},
+					Results: []v1.TaskResult{{Name: "my-object-result", Type: v1.ResultsTypeObject, Properties: map[string]v1.PropertySpec{}}},
+				}},
+			}},
+		},
+	}}
+	for _, tt := range tts {
+		t.Run(tt.name, func(t *testing.T) {
+			pr := v1.PipelineRun{ObjectMeta: metav1.ObjectMeta{Name: "foo"}, Spec: v1.PipelineRunSpec{
+				PipelineSpec: &tt.spec,
+			}}
+			ctx := cfgtesting.EnableStableAPIFields(context.Background())
+			if err := pr.Validate(ctx); err == nil {
+				t.Errorf("no error when using beta field when `enable-api-fields` is stable")
+			}
+
+			ctx = cfgtesting.EnableBetaAPIFields(context.Background())
+			if err := pr.Validate(ctx); err != nil {
+				t.Errorf("unexpected error when using beta field: %s", err)
+			}
+		})
+	}
+}
diff --git a/pkg/apis/pipeline/v1/taskrun_validation.go b/pkg/apis/pipeline/v1/taskrun_validation.go
@@ -63,6 +63,11 @@ func (ts *TaskRunSpec) Validate(ctx context.Context) (errs *apis.FieldError) {
 	// Validate TaskSpec if it's present.
 	if ts.TaskSpec != nil {
 		errs = errs.Also(ts.TaskSpec.Validate(ctx).ViaField("taskSpec"))
+		// Validate beta fields separately for inline Task definitions.
+		// This prevents validation from failing in the reconciler when a Task is converted to a different API version.
+		// See https://github.com/tektoncd/pipeline/issues/6616 for more information.
+		// TODO(#6592): Decouple API versioning from feature versioning
+		errs = errs.Also(ts.TaskSpec.ValidateBetaFields(ctx).ViaField("taskSpec"))
 	}
 
 	errs = errs.Also(ValidateParameters(ctx, ts.Params).ViaField("params"))

diff --git a/pkg/apis/pipeline/v1/taskrun_validation_test.go b/pkg/apis/pipeline/v1/taskrun_validation_test.go
@@ -894,3 +894,75 @@ func TestTaskRunSpec_Validate(t *testing.T) {
 		})
 	}
 }
+
+func TestTaskRunBetaFields(t *testing.T) {
+	tests := []struct {
+		name string
+		spec v1.TaskSpec
+	}{{
+		name: "array param indexing",
+		spec: v1.TaskSpec{
+			Params: []v1.ParamSpec{{Name: "foo", Type: v1.ParamTypeArray}},
+			Steps: []v1.Step{{
+				Name:  "my-step",
+				Image: "my-image",
+				Script: `
+					#!/usr/bin/env  bash
+					echo $(params.foo[1])`,
+			}},
+		},
+	}, {
+		name: "object params",
+		spec: v1.TaskSpec{
+			Params: []v1.ParamSpec{{Name: "foo", Type: v1.ParamTypeObject, Properties: map[string]v1.PropertySpec{"bar": {Type: v1.ParamTypeString}}}},
+			Steps: []v1.Step{{
+				Name:  "my-step",
+				Image: "my-image",
+				Script: `
+					#!/usr/bin/env  bash
+					echo $(params.foo.bar)`,
+			}},
+		},
+	}, {
+		name: "array results",
+		spec: v1.TaskSpec{
+			Results: []v1.TaskResult{{Name: "array-result", Type: v1.ResultsTypeArray}},
+			Steps: []v1.Step{{
+				Name:  "my-step",
+				Image: "my-image",
+				Script: `
+					#!/usr/bin/env  bash
+					echo -n "[\"hello\",\"world\"]" | tee $(results.array-result.path)`,
+			}},
+		},
+	}, {
+		name: "object results",
+		spec: v1.TaskSpec{
+			Results: []v1.TaskResult{{Name: "object-result", Type: v1.ResultsTypeObject,
+				Properties: map[string]v1.PropertySpec{}}},
+			Steps: []v1.Step{{
+				Name:  "my-step",
+				Image: "my-image",
+				Script: `
+					#!/usr/bin/env  bash
+					echo -n "{\"hello\":\"world\"}" | tee $(results.object-result.path)`,
+			}},
+		},
+	}}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ctx := cfgtesting.EnableStableAPIFields(context.Background())
+			tr := v1.TaskRun{ObjectMeta: metav1.ObjectMeta{Name: "foo"}, Spec: v1.TaskRunSpec{
+				TaskSpec: &tt.spec,
+			}}
+			if err := tr.Validate(ctx); err == nil {
+				t.Errorf("no error when using beta field when `enable-api-fields` is stable")
+			}
+
+			ctx = cfgtesting.EnableBetaAPIFields(context.Background())
+			if err := tr.Validate(ctx); err != nil {
+				t.Errorf("unexpected error when using beta field: %s", err)
+			}
+		})
+	}
+}