From b5595edd5eb712cc61a2994b63906d67d84ffe5e Mon Sep 17 00:00:00 2001 From: JeromeJu Date: Tue, 29 Aug 2023 14:39:14 +0000 Subject: [PATCH] Decouple beta feature validation in v1beta1 This commit decouples the existing beta feature validation in v1beta1. Prior to this change, beta features are regarded as stable in v1beta1 apiVersion and they are validated only in v1 api behind beta enable-api-fields but not in v1beta1. This PR removes the gap between the validations of the stable features in v1 and v1beta1. It also updates the api compatibility policy to clarify that feature stability levels are independent on CRD apiVersions. Fixes: #6592 --- api_compatibility_policy.md | 14 +- pkg/apis/pipeline/v1/pipeline_validation.go | 1 - pkg/apis/pipeline/v1/task_validation.go | 1 - .../pipeline/v1beta1/pipeline_types_test.go | 8 +- .../pipeline/v1beta1/pipeline_validation.go | 61 +++++- .../v1beta1/pipeline_validation_test.go | 201 ++++++++++++++++++ pkg/apis/pipeline/v1beta1/task_validation.go | 33 ++- .../pipeline/v1beta1/task_validation_test.go | 70 ++++++ .../pipeline/v1beta1/taskref_validation.go | 4 + .../pipelinerun/resources/pipelineref.go | 1 - 10 files changed, 375 insertions(+), 19 deletions(-) diff --git a/api_compatibility_policy.md b/api_compatibility_policy.md index 0f77c60d892..39c667078f6 100644 --- a/api_compatibility_policy.md +++ b/api_compatibility_policy.md @@ -90,19 +90,13 @@ For more information on support windows, see the [deprecations table](./docs/dep ## Feature Gates -CRD API versions gate the overall stability of the CRD and its default behaviors. Within a particular CRD version, certain opt-in features may be at a lower stability level as described in [TEP-33](https://github.com/tektoncd/community/blob/main/teps/0033-tekton-feature-gates.md). These fields may be disabled by default and can be enabled by setting the right `enable-api-fields` feature-flag as described in TEP-33: +Stability levels of feature gates are independent from CRD apiVersions. The current group api-driven feature flag `enable-api-fields` gates the overall stability levels of features in the CRDs and their default behaviors. These fields may be disabled by default and can be enabled by setting the `enable-api-fields` feature-flag value: -* `stable` - This value indicates that only fields of the highest stability level are enabled; For `beta` CRDs, this means only beta stability fields are enabled, i.e. `alpha` fields are not enabled. For `GA` CRDs, this means only `GA` fields are enabled, i.e. `beta` and `alpha` fields would not be enabled. TODO(#6592): Decouple feature stability from API stability. -* `beta` (default) - This value indicates that only fields which are of `beta` (or greater) stability are enabled, i.e. `alpha` fields are not enabled. -* `alpha` - This value indicates that fields of all stability levels are enabled, specifically `alpha`, `beta` and `GA`. +* `stable` - This value indicates that only fields of the highest stability level are enabled; For CRDs in all apiVersions, this means only stable fields are enabled, i.e. `alpha` and `beta` fields are not enabled. +* `beta` (default) - This value indicates that only fields which are of `beta` (or greater) stability are enabled, i.e. `alpha` fields are not enabled. -| Feature Versions -> | v1 | beta | alpha | -|---------------------|----|------|-------| -| stable | x | | | -| beta | x | x | | -| alpha | x | x | x | - +* `alpha` - This value indicates that fields of all stability levels are enabled, specifically `alpha`, `beta` and `GA`(`stable`). See the current list of [alpha features](https://github.com/tektoncd/pipeline/blob/main/docs/additional-configs.md#alpha-features) and [beta features](https://github.com/tektoncd/pipeline/blob/main/docs/additional-configs.md#beta-features). diff --git a/pkg/apis/pipeline/v1/pipeline_validation.go b/pkg/apis/pipeline/v1/pipeline_validation.go index e1a7464c354..8f3bb380f8b 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1/pipeline_validation.go @@ -55,7 +55,6 @@ func (p *Pipeline) Validate(ctx context.Context) *apis.FieldError { // Validate beta fields when a Pipeline is defined, but not as part of validating a Pipeline spec. // This prevents validation from failing 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(p.Spec.ValidateBetaFields(ctx)) return errs } diff --git a/pkg/apis/pipeline/v1/task_validation.go b/pkg/apis/pipeline/v1/task_validation.go index 8a051618fe0..c672d8a6912 100644 --- a/pkg/apis/pipeline/v1/task_validation.go +++ b/pkg/apis/pipeline/v1/task_validation.go @@ -69,7 +69,6 @@ func (t *Task) Validate(ctx context.Context) *apis.FieldError { // Validate beta fields when a Task is defined, but not as part of validating a Task spec. // This prevents validation from failing 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(t.Spec.ValidateBetaFields(ctx)) return errs } diff --git a/pkg/apis/pipeline/v1beta1/pipeline_types_test.go b/pkg/apis/pipeline/v1beta1/pipeline_types_test.go index 6005cb80062..7c1e04f2543 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_types_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_types_test.go @@ -223,7 +223,7 @@ func TestPipelineTask_ValidateRegularTask_Success(t *testing.T) { tests := []struct { name string tasks PipelineTask - enableAPIFields bool + enableAPIFields string enableBundles bool }{{ name: "pipeline task - valid taskRef name", @@ -242,11 +242,13 @@ func TestPipelineTask_ValidateRegularTask_Success(t *testing.T) { tasks: PipelineTask{ TaskRef: &TaskRef{ResolverRef: ResolverRef{Resolver: "bar"}}, }, + enableAPIFields: "beta", }, { name: "pipeline task - use of params", tasks: PipelineTask{ TaskRef: &TaskRef{ResolverRef: ResolverRef{Resolver: "bar", Params: Params{}}}, }, + enableAPIFields: "beta", }, { name: "pipeline task - use of bundle with the feature flag set", tasks: PipelineTask{ @@ -261,9 +263,7 @@ func TestPipelineTask_ValidateRegularTask_Success(t *testing.T) { cfg := &config.Config{ FeatureFlags: &config.FeatureFlags{}, } - if tt.enableAPIFields { - cfg.FeatureFlags.EnableAPIFields = config.AlphaAPIFields - } + cfg.FeatureFlags.EnableAPIFields = tt.enableAPIFields if tt.enableBundles { cfg.FeatureFlags.EnableTektonOCIBundles = true } diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation.go b/pkg/apis/pipeline/v1beta1/pipeline_validation.go index 6411d3f09d4..250d5ef5eb2 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation.go @@ -52,7 +52,12 @@ func (p *Pipeline) Validate(ctx context.Context) *apis.FieldError { // we do not support propagated parameters and workspaces. // Validate that all params and workspaces it uses are declared. errs = errs.Also(p.Spec.validatePipelineParameterUsage(ctx).ViaField("spec")) - return errs.Also(p.Spec.validatePipelineWorkspacesUsage().ViaField("spec")) + errs = errs.Also(p.Spec.validatePipelineWorkspacesUsage().ViaField("spec")) + // Validate beta fields when a Pipeline is defined, but not as part of validating a Pipeline spec. + // This prevents validation from failing when a Pipeline is converted to a different API version. + // See https://github.com/tektoncd/pipeline/issues/6616 for more information. + errs = errs.Also(p.Spec.ValidateBetaFields(ctx)) + return errs } // Validate checks that taskNames in the Pipeline are valid and that the graph @@ -87,6 +92,60 @@ func (ps *PipelineSpec) Validate(ctx context.Context) (errs *apis.FieldError) { return errs } +// ValidateBetaFields returns an error if the Pipeline spec uses beta features but does not +// have "enable-api-fields" set to "alpha" or "beta". +func (ps *PipelineSpec) ValidateBetaFields(ctx context.Context) *apis.FieldError { + var errs *apis.FieldError + // Object parameters + for i, p := range ps.Params { + if p.Type == ParamTypeObject { + errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "object type parameter", config.BetaAPIFields).ViaFieldIndex("params", i)) + } + } + // Indexing into array parameters + arrayParamIndexingRefs := ps.GetIndexingReferencesToArrayParams() + if len(arrayParamIndexingRefs) != 0 { + errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "indexing into array parameters", config.BetaAPIFields)) + } + // array and object results + for i, result := range ps.Results { + switch result.Type { + case ResultsTypeObject: + errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "object results", config.BetaAPIFields).ViaFieldIndex("results", i)) + case ResultsTypeArray: + errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "array results", config.BetaAPIFields).ViaFieldIndex("results", i)) + case ResultsTypeString: + default: + } + } + for i, pt := range ps.Tasks { + errs = errs.Also(pt.validateBetaFields(ctx).ViaFieldIndex("tasks", i)) + } + for i, pt := range ps.Finally { + errs = errs.Also(pt.validateBetaFields(ctx).ViaFieldIndex("tasks", i)) + } + + return errs +} + +// validateBetaFields returns an error if the PipelineTask uses beta features but does not +// have "enable-api-fields" set to "alpha" or "beta". +func (pt *PipelineTask) validateBetaFields(ctx context.Context) *apis.FieldError { + var errs *apis.FieldError + if pt.TaskRef != nil { + // Resolvers + if pt.TaskRef.Resolver != "" { + errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "taskref.resolver", config.BetaAPIFields)) + } + if len(pt.TaskRef.Params) > 0 { + errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "taskref.params", config.BetaAPIFields)) + } + } else if pt.TaskSpec != nil { + errs = errs.Also(pt.TaskSpec.ValidateBetaFields(ctx)) + } + return errs +} + // ValidatePipelineTasks ensures that pipeline tasks has unique label, pipeline tasks has specified one of // taskRef or taskSpec, and in case of a pipeline task with taskRef, it has a reference to a valid task (task name) func ValidatePipelineTasks(ctx context.Context, tasks []PipelineTask, finalTasks []PipelineTask) *apis.FieldError { diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go index a2d691937c8..119772c6e93 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go @@ -24,6 +24,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/tektoncd/pipeline/pkg/apis/config" + cfgtesting "github.com/tektoncd/pipeline/pkg/apis/config/testing" "github.com/tektoncd/pipeline/test/diff" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -3409,6 +3410,206 @@ func enableFeatures(t *testing.T, features []string) func(context.Context) conte } } +func TestPipelineWithBetaFields(t *testing.T) { + tts := []struct { + name string + spec PipelineSpec + }{{ + name: "array indexing in Tasks", + spec: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + }, + Tasks: []PipelineTask{{ + Name: "foo", + Params: Params{ + {Name: "first-task-first-param", Value: *NewStructuredValues("$(params.first-param[0])")}, + }, + TaskRef: &TaskRef{Name: "foo"}, + }}, + }, + }, { + name: "array indexing in Finally", + spec: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeArray, Default: NewStructuredValues("default-value", "default-value-again")}, + }, + Tasks: []PipelineTask{{ + Name: "foo", + TaskRef: &TaskRef{Name: "foo"}, + }}, + Finally: []PipelineTask{{ + Name: "bar", + Params: Params{ + {Name: "first-task-first-param", Value: *NewStructuredValues("$(params.first-param[0])")}, + }, + TaskRef: &TaskRef{Name: "bar"}, + }}, + }, + }, { + name: "pipeline tasks - use of resolver without the feature flag set", + spec: PipelineSpec{ + Tasks: []PipelineTask{{ + Name: "uses-resolver", + TaskRef: &TaskRef{ResolverRef: ResolverRef{Resolver: "bar"}}, + }}, + }, + }, { + name: "pipeline tasks - use of resolver params without the feature flag set", + spec: PipelineSpec{ + Tasks: []PipelineTask{{ + Name: "uses-resolver-params", + TaskRef: &TaskRef{ResolverRef: ResolverRef{Resolver: "bar", Params: Params{{}}}}, + }}, + }, + }, { + name: "finally tasks - use of resolver without the feature flag set", + spec: PipelineSpec{ + Tasks: []PipelineTask{{ + Name: "valid-pipeline-task", + TaskRef: &TaskRef{Name: "foo-task"}, + }}, + Finally: []PipelineTask{{ + Name: "uses-resolver", + TaskRef: &TaskRef{ResolverRef: ResolverRef{Resolver: "bar"}}, + }}, + }, + }, { + name: "finally tasks - use of resolver params without the feature flag set", + spec: PipelineSpec{ + Tasks: []PipelineTask{{ + Name: "valid-pipeline-task", + TaskRef: &TaskRef{Name: "foo-task"}, + }}, + Finally: []PipelineTask{{ + Name: "uses-resolver-params", + TaskRef: &TaskRef{ResolverRef: ResolverRef{Resolver: "bar", Params: Params{{}}}}, + }}, + }, + }, { + name: "object params", + spec: PipelineSpec{ + Params: []ParamSpec{ + {Name: "first-param", Type: ParamTypeObject, Properties: map[string]PropertySpec{}}, + }, + Tasks: []PipelineTask{{ + Name: "foo", + TaskRef: &TaskRef{Name: "foo"}, + }}, + }, + }, { + name: "object params in Tasks", + spec: PipelineSpec{ + Tasks: []PipelineTask{{ + Name: "valid-pipeline-task", + TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ + Steps: []Step{{Image: "busybox", Script: "echo hello"}}, + Params: []ParamSpec{{Name: "my-object-param", Type: ParamTypeObject, Properties: map[string]PropertySpec{}}}, + }}, + }}, + }, + }, { + name: "object params in Finally", + spec: PipelineSpec{ + Tasks: []PipelineTask{{ + Name: "foo", + TaskRef: &TaskRef{Name: "foo"}, + }}, + Finally: []PipelineTask{{ + Name: "valid-finally-task", + TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ + Steps: []Step{{Image: "busybox", Script: "echo hello"}}, + Params: []ParamSpec{{Name: "my-object-param", Type: ParamTypeObject, Properties: map[string]PropertySpec{}}}, + }}, + }}, + }, + }, { + name: "array results", + spec: PipelineSpec{ + Tasks: []PipelineTask{{ + Name: "valid-pipeline-task", + TaskRef: &TaskRef{Name: "foo-task"}, + }}, + Results: []PipelineResult{{Name: "my-array-result", Type: ResultsTypeArray, Value: *NewStructuredValues("$(tasks.valid-pipeline-task.results.foo[*])")}}, + }, + }, { + name: "array results in Tasks", + spec: PipelineSpec{ + Tasks: []PipelineTask{{ + Name: "valid-pipeline-task", + TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ + Steps: []Step{{Image: "busybox", Script: "echo hello"}}, + Results: []TaskResult{{Name: "my-array-result", Type: ResultsTypeArray}}, + }}, + }}, + }, + }, { + name: "array results in Finally", + spec: PipelineSpec{ + Tasks: []PipelineTask{{ + Name: "valid-pipeline-task", + TaskRef: &TaskRef{Name: "foo-task"}, + }}, + Finally: []PipelineTask{{ + Name: "valid-finally-task", + TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ + Steps: []Step{{Image: "busybox", Script: "echo hello"}}, + Results: []TaskResult{{Name: "my-array-result", Type: ResultsTypeArray}}, + }}, + }}, + }, + }, { + name: "object results", + spec: PipelineSpec{ + Tasks: []PipelineTask{{ + Name: "valid-pipeline-task", + TaskRef: &TaskRef{Name: "foo-task"}, + }}, + Results: []PipelineResult{{Name: "my-object-result", Type: ResultsTypeObject, Value: *NewStructuredValues("$(tasks.valid-pipeline-task.results.foo[*])")}}, + }, + }, { + name: "object results in Tasks", + spec: PipelineSpec{ + Tasks: []PipelineTask{{ + Name: "valid-pipeline-task", + TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ + Steps: []Step{{Image: "busybox", Script: "echo hello"}}, + Results: []TaskResult{{Name: "my-object-result", Type: ResultsTypeObject, Properties: map[string]PropertySpec{}}}, + }}, + }}, + }, + }, { + name: "object results in Finally", + spec: PipelineSpec{ + Tasks: []PipelineTask{{ + Name: "valid-pipeline-task", + TaskRef: &TaskRef{Name: "foo-task"}, + }}, + Finally: []PipelineTask{{ + Name: "valid-finally-task", + TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ + Steps: []Step{{Image: "busybox", Script: "echo hello"}}, + Results: []TaskResult{{Name: "my-object-result", Type: ResultsTypeObject, Properties: map[string]PropertySpec{}}}, + }}, + }}, + }, + }} + for _, tt := range tts { + t.Run(tt.name, func(t *testing.T) { + pipeline := Pipeline{ObjectMeta: metav1.ObjectMeta{Name: "foo"}, Spec: tt.spec} + ctx := cfgtesting.EnableStableAPIFields(context.Background()) + if err := pipeline.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 := pipeline.Validate(ctx); err != nil { + t.Errorf("unexpected error when using beta field: %s", err) + } + }) + } +} + func TestGetIndexingReferencesToArrayParams(t *testing.T) { for _, tt := range []struct { name string diff --git a/pkg/apis/pipeline/v1beta1/task_validation.go b/pkg/apis/pipeline/v1beta1/task_validation.go index aec142cf047..d2c641f6378 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation.go +++ b/pkg/apis/pipeline/v1beta1/task_validation.go @@ -65,7 +65,9 @@ func (t *Task) Validate(ctx context.Context) *apis.FieldError { errs = errs.Also(t.Spec.Validate(apis.WithinSpec(ctx)).ViaField("spec")) // When a Task is created directly, instead of declared inline in a TaskRun or PipelineRun, // we do not support propagated parameters. Validate that all params it uses are declared. - return errs.Also(ValidateUsageOfDeclaredParameters(ctx, t.Spec.Steps, t.Spec.Params).ViaField("spec")) + errs = errs.Also(ValidateUsageOfDeclaredParameters(ctx, t.Spec.Steps, t.Spec.Params).ViaField("spec")) + errs = errs.Also(t.Spec.ValidateBetaFields(ctx)) + return errs } // Validate implements apis.Validatable @@ -99,6 +101,35 @@ func (ts *TaskSpec) Validate(ctx context.Context) (errs *apis.FieldError) { return errs } +// ValidateBetaFields returns an error if the Task spec uses beta features but does not +// have "enable-api-fields" set to "alpha" or "beta". +func (ts *TaskSpec) ValidateBetaFields(ctx context.Context) *apis.FieldError { + var errs *apis.FieldError + // Object parameters + for i, p := range ts.Params { + if p.Type == ParamTypeObject { + errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "object type parameter", config.BetaAPIFields).ViaFieldIndex("params", i)) + } + } + // Indexing into array parameters + arrayIndexParamRefs := ts.GetIndexingReferencesToArrayParams() + if len(arrayIndexParamRefs) != 0 { + errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "indexing into array parameters", config.BetaAPIFields)) + } + // Array and object results + for i, result := range ts.Results { + switch result.Type { + case ResultsTypeObject: + errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "object results", config.BetaAPIFields).ViaFieldIndex("results", i)) + case ResultsTypeArray: + errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "array results", config.BetaAPIFields).ViaFieldIndex("results", i)) + case ResultsTypeString: + default: + } + } + return errs +} + // ValidateUsageOfDeclaredParameters validates that all parameters referenced in the Task are declared by the Task. func ValidateUsageOfDeclaredParameters(ctx context.Context, steps []Step, params ParamSpecs) *apis.FieldError { var errs *apis.FieldError diff --git a/pkg/apis/pipeline/v1beta1/task_validation_test.go b/pkg/apis/pipeline/v1beta1/task_validation_test.go index c89bc74a8ae..484c90cb748 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/task_validation_test.go @@ -1707,6 +1707,76 @@ func TestIncompatibleAPIVersions(t *testing.T) { } } +func TestTaskBetaFields(t *testing.T) { + tests := []struct { + name string + spec v1beta1.TaskSpec + }{{ + name: "array param indexing", + spec: v1beta1.TaskSpec{ + Params: []v1beta1.ParamSpec{{Name: "foo", Type: v1beta1.ParamTypeArray}}, + Steps: []v1beta1.Step{{ + Name: "my-step", + Image: "my-image", + Script: ` + #!/usr/bin/env bash + echo $(params.foo[1])`, + }}, + }, + }, { + name: "object params", + spec: v1beta1.TaskSpec{ + Params: []v1beta1.ParamSpec{{Name: "foo", Type: v1beta1.ParamTypeObject, Properties: map[string]v1beta1.PropertySpec{"bar": {Type: v1beta1.ParamTypeString}}}}, + Steps: []v1beta1.Step{{ + Name: "my-step", + Image: "my-image", + Script: ` + #!/usr/bin/env bash + echo $(params.foo.bar)`, + }}, + }, + }, { + name: "array results", + spec: v1beta1.TaskSpec{ + Results: []v1beta1.TaskResult{{Name: "array-result", Type: v1beta1.ResultsTypeArray}}, + Steps: []v1beta1.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: v1beta1.TaskSpec{ + Results: []v1beta1.TaskResult{{Name: "object-result", Type: v1beta1.ResultsTypeObject, + Properties: map[string]v1beta1.PropertySpec{}}}, + Steps: []v1beta1.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()) + task := v1beta1.Task{ObjectMeta: metav1.ObjectMeta{Name: "foo"}, Spec: tt.spec} + if err := task.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 := task.Validate(ctx); err != nil { + t.Errorf("unexpected error when using beta field: %s", err) + } + }) + } +} + func TestGetArrayIndexParamRefs(t *testing.T) { tcs := []struct { name string diff --git a/pkg/apis/pipeline/v1beta1/taskref_validation.go b/pkg/apis/pipeline/v1beta1/taskref_validation.go index 4e42b7aa2e2..eddf7d8a780 100644 --- a/pkg/apis/pipeline/v1beta1/taskref_validation.go +++ b/pkg/apis/pipeline/v1beta1/taskref_validation.go @@ -21,6 +21,8 @@ import ( "strings" "github.com/google/go-containerregistry/pkg/name" + "github.com/tektoncd/pipeline/pkg/apis/config" + "github.com/tektoncd/pipeline/pkg/apis/version" "k8s.io/apimachinery/pkg/util/validation" "knative.dev/pkg/apis" ) @@ -35,6 +37,7 @@ func (ref *TaskRef) Validate(ctx context.Context) (errs *apis.FieldError) { switch { case ref.Resolver != "" || ref.Params != nil: if ref.Resolver != "" { + errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "resolver", config.BetaAPIFields).ViaField("resolver")) if ref.Name != "" { errs = errs.Also(apis.ErrMultipleOneOf("name", "resolver")) } @@ -43,6 +46,7 @@ func (ref *TaskRef) Validate(ctx context.Context) (errs *apis.FieldError) { } } if ref.Params != nil { + errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "resolver", config.BetaAPIFields).ViaField("resolver")) if ref.Name != "" { errs = errs.Also(apis.ErrMultipleOneOf("name", "params")) } diff --git a/pkg/reconciler/pipelinerun/resources/pipelineref.go b/pkg/reconciler/pipelinerun/resources/pipelineref.go index 12396d5631a..40c3dec8e0c 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelineref.go +++ b/pkg/reconciler/pipelinerun/resources/pipelineref.go @@ -142,7 +142,6 @@ func readRuntimeObjectAsPipeline(ctx context.Context, namespace string, obj runt // without actually creating the Pipeline on the cluster. // Validation must happen before the v1beta1 Pipeline is converted into the storage version of the API, // since validation of beta features differs between v1 and v1beta1 - // TODO(#6592): Decouple API versioning from feature versioning if err := apiserver.DryRunValidate(ctx, namespace, obj, tekton); err != nil { return nil, nil, err }