From 3900a4de7aac460ba6d416a2a9da411db32a70c1 Mon Sep 17 00:00:00 2001 From: JeromeJu Date: Wed, 30 Aug 2023 03:25:32 +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 only require enable-api-fields to be set to beta in v1 apiVersion but not in v1beta1. This PR removes the gap between the validations of the stable features in v1 and v1beta1 by syncing up the validations in v1beta1. Fixes: #6592 --- api_compatibility_policy.md | 27 ++- docs/additional-configs.md | 18 +- .../pipeline/v1beta1/pipeline_types_test.go | 13 +- .../pipeline/v1beta1/pipeline_validation.go | 61 +++++- .../v1beta1/pipeline_validation_test.go | 200 +++++++++++++++++ .../v1beta1/pipelineref_validation.go | 3 + .../v1beta1/pipelineref_validation_test.go | 56 ++++- .../v1beta1/pipelinerun_validation.go | 5 + .../v1beta1/pipelinerun_validation_test.go | 202 ++++++++++++++++++ pkg/apis/pipeline/v1beta1/task_validation.go | 36 +++- .../pipeline/v1beta1/task_validation_test.go | 70 ++++++ .../pipeline/v1beta1/taskref_validation.go | 4 + .../v1beta1/taskref_validation_test.go | 28 ++- .../pipeline/v1beta1/taskrun_validation.go | 9 + .../v1beta1/taskrun_validation_test.go | 72 +++++++ 15 files changed, 765 insertions(+), 39 deletions(-) diff --git a/api_compatibility_policy.md b/api_compatibility_policy.md index 36324e59a41..3ddf9e01838 100644 --- a/api_compatibility_policy.md +++ b/api_compatibility_policy.md @@ -90,26 +90,25 @@ 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. Features enabled by API fields at different levels of stability can be enabled using the flag `enable-api-fields`: -* `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; 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). +| `enable-api-fields` value | stable features enabled | beta features enabled | alpha features enabled | +|----------------------------|-------------------------|-----------------------|------------------------| +| stable | x | | | +| beta | x | x | | +| alpha | x | x | x | ### Alpha features -- Alpha feature in beta or GA CRDs are disabled by default and must be enabled by [setting `enable-api-fields` to `alpha`](https://github.com/tektoncd/pipeline/blob/main/docs/additional-configs.md#alpha-features) +- Alpha features are disabled by default and must be enabled by [setting `enable-api-fields` to `alpha`](https://github.com/tektoncd/pipeline/blob/main/docs/additional-configs.md#alpha-features) - These features may be dropped or backwards incompatible changes made at any time, though one release worth of warning will be provided. @@ -125,14 +124,12 @@ See the current list of [alpha features](https://github.com/tektoncd/pipeline/bl i.e. by providing a 9 month support period. - Beta features are reviewed for promotion to GA/Stable on a regular basis. However, there is no guarantee that they will be promoted to GA/stable. - -- For beta API versions, beta is the highest level of stability possible for any feature. ### GA/Stable features -- GA/Stable features are present in a [GA CRD](#ga-crds) only. +- GA/Stable features are enabled by default. -- GA/Stable features are enabled by default +- GA/Stable features are features that have been promoted from beta to the highest level of stability. They cannot be disabled in any CRD version. - GA/Stable features will not be removed or changed in a backwards incompatible manner without incrementing the API Version. diff --git a/docs/additional-configs.md b/docs/additional-configs.md index 598f6352acf..0740e881a20 100644 --- a/docs/additional-configs.md +++ b/docs/additional-configs.md @@ -325,19 +325,15 @@ To enable these features, set the `enable-api-fields` feature flag to `"beta"` i the `feature-flags` ConfigMap alongside your Tekton Pipelines deployment via `kubectl patch cm feature-flags -n tekton-pipelines -p '{"data":{"enable-api-fields":"beta"}}'`. - -For beta versions of Tekton CRDs, setting `enable-api-fields` to "beta" is the same as setting it to "stable", -except where otherwise noted. - Features currently in "beta" are: -| Feature | Proposal | Alpha Release | Beta Release | Individual Flag | `enable-api-fields=beta` required for `v1beta1` | -|:-------------------------------------------------------------------|:------------------------------------------------------------------------------------------------|:---------------------------------------------------------------------|:---------------------------------------------------------------------|:----------------|:---| -| [Array Results and Array Indexing](pipelineruns.md#specifying-parameters) | [TEP-0076](https://github.com/tektoncd/community/blob/main/teps/0076-array-result-types.md) | [v0.38.0](https://github.com/tektoncd/pipeline/releases/tag/v0.38.0) | [v0.45.0](https://github.com/tektoncd/pipeline/releases/tag/v0.45.0) | | No | -| [Object Parameters and Results](pipelineruns.md#specifying-parameters) | [TEP-0075](https://github.com/tektoncd/community/blob/main/teps/0075-object-param-and-result-types.md) | | [v0.46.0](https://github.com/tektoncd/pipeline/releases/tag/v0.46.0) | | No | -| [Remote Tasks](./taskruns.md#remote-tasks) and [Remote Pipelines](./pipelineruns.md#remote-pipelines) | [TEP-0060](https://github.com/tektoncd/community/blob/main/teps/0060-remote-resolution.md) | | [v0.41.0](https://github.com/tektoncd/pipeline/releases/tag/v0.41.0) | | No | -| [`Provenance` field in Status](pipeline-api.md#provenance)| [issue#5550](https://github.com/tektoncd/pipeline/issues/5550)| [v0.41.0](https://github.com/tektoncd/pipeline/releases/tag/v0.41.0)| [v0.48.0](https://github.com/tektoncd/pipeline/releases/tag/v0.48.0) | `enable-provenance-in-status`| No | -| [Isolated `Step` & `Sidecar` `Workspaces`](./workspaces.md#isolated-workspaces) | [TEP-0029](https://github.com/tektoncd/community/blob/main/teps/0029-step-workspaces.md) | [v0.24.0](https://github.com/tektoncd/pipeline/releases/tag/v0.24.0) | [v0.50.0](https://github.com/tektoncd/pipeline/releases/tag/v0.50.0) | | Yes | +| Feature | Proposal | Alpha Release | Beta Release | Individual Flag | +|:-------------------------------------------------------------------|:------------------------------------------------------------------------------------------------|:---------------------------------------------------------------------|:---------------------------------------------------------------------|:----------------| +| [Array Results and Array Indexing](pipelineruns.md#specifying-parameters) | [TEP-0076](https://github.com/tektoncd/community/blob/main/teps/0076-array-result-types.md) | [v0.38.0](https://github.com/tektoncd/pipeline/releases/tag/v0.38.0) | [v0.45.0](https://github.com/tektoncd/pipeline/releases/tag/v0.45.0) | | +| [Object Parameters and Results](pipelineruns.md#specifying-parameters) | [TEP-0075](https://github.com/tektoncd/community/blob/main/teps/0075-object-param-and-result-types.md) | | [v0.46.0](https://github.com/tektoncd/pipeline/releases/tag/v0.46.0) | | +| [Remote Tasks](./taskruns.md#remote-tasks) and [Remote Pipelines](./pipelineruns.md#remote-pipelines) | [TEP-0060](https://github.com/tektoncd/community/blob/main/teps/0060-remote-resolution.md) | | [v0.41.0](https://github.com/tektoncd/pipeline/releases/tag/v0.41.0) | | +| [`Provenance` field in Status](pipeline-api.md#provenance)| [issue#5550](https://github.com/tektoncd/pipeline/issues/5550)| [v0.41.0](https://github.com/tektoncd/pipeline/releases/tag/v0.41.0)| [v0.48.0](https://github.com/tektoncd/pipeline/releases/tag/v0.48.0) | `enable-provenance-in-status`| +| [Isolated `Step` & `Sidecar` `Workspaces`](./workspaces.md#isolated-workspaces) | [TEP-0029](https://github.com/tektoncd/community/blob/main/teps/0029-step-workspaces.md) | [v0.24.0](https://github.com/tektoncd/pipeline/releases/tag/v0.24.0) | [v0.50.0](https://github.com/tektoncd/pipeline/releases/tag/v0.50.0) | | ## Enabling larger results using sidecar logs diff --git a/pkg/apis/pipeline/v1beta1/pipeline_types_test.go b/pkg/apis/pipeline/v1beta1/pipeline_types_test.go index 34b887d4d27..40890dfe5da 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_types_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_types_test.go @@ -451,7 +451,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", @@ -459,29 +459,34 @@ func TestPipelineTask_ValidateRegularTask_Success(t *testing.T) { Name: "foo", TaskRef: &TaskRef{Name: "example.com/my-foo-task"}, }, + enableAPIFields: "stable", }, { name: "pipeline task - valid taskSpec", tasks: PipelineTask{ Name: "foo", TaskSpec: &EmbeddedTask{TaskSpec: getTaskSpec()}, }, + enableAPIFields: "stable", }, { name: "pipeline task - use of resolver", 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{ Name: "foo", TaskRef: &TaskRef{Name: "bar", Bundle: "docker.io/foo"}, }, - enableBundles: true, + enableBundles: true, + enableAPIFields: "stable", }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -489,8 +494,8 @@ func TestPipelineTask_ValidateRegularTask_Success(t *testing.T) { cfg := &config.Config{ FeatureFlags: &config.FeatureFlags{}, } - if tt.enableAPIFields { - cfg.FeatureFlags.EnableAPIFields = config.AlphaAPIFields + if tt.enableAPIFields != "" { + 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 7529c273b4f..c103b5ef966 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation.go @@ -59,7 +59,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 @@ -94,6 +99,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 1ebd0604678..ed805d297a4 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go @@ -3879,6 +3879,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", + spec: PipelineSpec{ + Tasks: []PipelineTask{{ + Name: "uses-resolver", + TaskRef: &TaskRef{ResolverRef: ResolverRef{Resolver: "bar"}}, + }}, + }, + }, { + name: "pipeline tasks - use of resolver params", + spec: PipelineSpec{ + Tasks: []PipelineTask{{ + Name: "uses-resolver-params", + TaskRef: &TaskRef{ResolverRef: ResolverRef{Resolver: "bar", Params: Params{{}}}}, + }}, + }, + }, { + name: "finally tasks - use of resolver", + 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", + 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/pipelineref_validation.go b/pkg/apis/pipeline/v1beta1/pipelineref_validation.go index 6186c177a28..27929532551 100644 --- a/pkg/apis/pipeline/v1beta1/pipelineref_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipelineref_validation.go @@ -22,6 +22,7 @@ import ( "github.com/google/go-containerregistry/pkg/name" "github.com/tektoncd/pipeline/pkg/apis/config" + "github.com/tektoncd/pipeline/pkg/apis/version" "knative.dev/pkg/apis" ) @@ -34,6 +35,7 @@ func (ref *PipelineRef) Validate(ctx context.Context) (errs *apis.FieldError) { if 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")) } @@ -42,6 +44,7 @@ func (ref *PipelineRef) Validate(ctx context.Context) (errs *apis.FieldError) { } } if ref.Params != nil { + errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "resolver params", config.BetaAPIFields).ViaField("params")) if ref.Name != "" { errs = errs.Also(apis.ErrMultipleOneOf("name", "params")) } diff --git a/pkg/apis/pipeline/v1beta1/pipelineref_validation_test.go b/pkg/apis/pipeline/v1beta1/pipelineref_validation_test.go index ea1ae14a1b2..075dafcfe5e 100644 --- a/pkg/apis/pipeline/v1beta1/pipelineref_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipelineref_validation_test.go @@ -22,6 +22,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/tektoncd/pipeline/pkg/apis/config" + cfgtesting "github.com/tektoncd/pipeline/pkg/apis/config/testing" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/test/diff" corev1 "k8s.io/api/core/v1" @@ -62,6 +63,24 @@ func TestPipelineRef_Invalid(t *testing.T) { name: "pipelineRef without Pipeline Name", ref: &v1beta1.PipelineRef{}, wantErr: apis.ErrMissingField("name"), + }, { + name: "pipelineref resolver disallowed without beta feature gate", + ref: &v1beta1.PipelineRef{ + ResolverRef: v1beta1.ResolverRef{ + Resolver: "foo", + }, + }, + withContext: cfgtesting.EnableStableAPIFields, + wantErr: apis.ErrGeneric("resolver requires \"enable-api-fields\" feature gate to be \"alpha\" or \"beta\" but it is \"stable\""), + }, { + name: "pipelineref params disallowed without beta feature gate", + ref: &v1beta1.PipelineRef{ + ResolverRef: v1beta1.ResolverRef{ + Params: v1beta1.Params{}, + }, + }, + withContext: cfgtesting.EnableStableAPIFields, + wantErr: apis.ErrMissingField("resolver").Also(apis.ErrGeneric("resolver params requires \"enable-api-fields\" feature gate to be \"alpha\" or \"beta\" but it is \"stable\"")), }, { name: "pipelineref params disallowed without resolver", ref: &v1beta1.PipelineRef{ @@ -69,7 +88,8 @@ func TestPipelineRef_Invalid(t *testing.T) { Params: v1beta1.Params{}, }, }, - wantErr: apis.ErrMissingField("resolver"), + wantErr: apis.ErrMissingField("resolver"), + withContext: cfgtesting.EnableBetaAPIFields, }, { name: "pipelineref resolver disallowed in conjunction with pipelineref name", ref: &v1beta1.PipelineRef{ @@ -78,7 +98,8 @@ func TestPipelineRef_Invalid(t *testing.T) { Resolver: "bar", }, }, - wantErr: apis.ErrMultipleOneOf("name", "resolver"), + wantErr: apis.ErrMultipleOneOf("name", "resolver"), + withContext: cfgtesting.EnableBetaAPIFields, }, { name: "pipelineref resolver disallowed in conjunction with pipelineref bundle", ref: &v1beta1.PipelineRef{ @@ -103,7 +124,8 @@ func TestPipelineRef_Invalid(t *testing.T) { }}, }, }, - wantErr: apis.ErrMultipleOneOf("name", "params").Also(apis.ErrMissingField("resolver")), + wantErr: apis.ErrMultipleOneOf("name", "params").Also(apis.ErrMissingField("resolver")), + withContext: cfgtesting.EnableBetaAPIFields, }, { name: "pipelineref params disallowed in conjunction with pipelineref bundle", ref: &v1beta1.PipelineRef{ @@ -120,6 +142,24 @@ func TestPipelineRef_Invalid(t *testing.T) { }, wantErr: apis.ErrMultipleOneOf("bundle", "params").Also(apis.ErrMissingField("resolver")), withContext: enableTektonOCIBundles(t), + }, { + name: "pipelineref param object not allowed in stable", + ref: &v1beta1.PipelineRef{ + ResolverRef: v1beta1.ResolverRef{ + Resolver: "some-resolver", + Params: v1beta1.Params{{ + Name: "foo", + Value: v1beta1.ParamValue{ + Type: v1beta1.ParamTypeObject, + ObjectVal: map[string]string{"bar": "baz"}, + }, + }}, + }, + }, + wantErr: apis.ErrGeneric("resolver requires \"enable-api-fields\" feature gate to be \"alpha\" or \"beta\" but it is \"stable\"").Also( + apis.ErrGeneric("resolver params requires \"enable-api-fields\" feature gate to be \"alpha\" or \"beta\" but it is \"stable\"")).Also( + apis.ErrGeneric("object type parameter requires \"enable-api-fields\" feature gate to be \"alpha\" or \"beta\" but it is \"stable\"")), + withContext: cfgtesting.EnableStableAPIFields, }} for _, tc := range tests { @@ -145,10 +185,15 @@ func TestPipelineRef_Valid(t *testing.T) { name: "no pipelineRef", ref: nil, }, { - name: "valid resolver", + name: "beta feature: valid resolver", + ref: &v1beta1.PipelineRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git"}}, + wc: cfgtesting.EnableAlphaAPIFields, + }, { + name: "beta feature: valid resolver with alpha flag", ref: &v1beta1.PipelineRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git"}}, + wc: cfgtesting.EnableAlphaAPIFields, }, { - name: "valid resolver with params", + name: "beta feature: valid resolver with params with alpha flag", ref: &v1beta1.PipelineRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git", Params: v1beta1.Params{{ Name: "repo", Value: v1beta1.ParamValue{ @@ -162,6 +207,7 @@ func TestPipelineRef_Valid(t *testing.T) { StringVal: "baz", }, }}}}, + wc: cfgtesting.EnableAlphaAPIFields, }} for _, ts := range tests { diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go b/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go index 307f7b8f2e6..e990f332bc8 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go @@ -72,6 +72,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/v1beta1/pipelinerun_validation_test.go b/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go index de041cf3066..9a9ad4c4d86 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go @@ -1545,3 +1545,205 @@ func TestPipelineRunWithTimeout_Validate(t *testing.T) { }) } } + +func TestPipelineRunSpecBetaFeatures(t *testing.T) { + tts := []struct { + name string + spec v1beta1.PipelineSpec + }{{ + name: "array indexing in Tasks", + spec: v1beta1.PipelineSpec{ + Params: []v1beta1.ParamSpec{ + {Name: "first-param", Type: v1beta1.ParamTypeArray, Default: v1beta1.NewStructuredValues("default-value", "default-value-again")}, + }, + Tasks: []v1beta1.PipelineTask{{ + Name: "foo", + Params: v1beta1.Params{ + {Name: "first-task-first-param", Value: *v1beta1.NewStructuredValues("$(params.first-param[0])")}, + }, + TaskRef: &v1beta1.TaskRef{Name: "foo"}, + }}, + }, + }, { + name: "array indexing in Finally", + spec: v1beta1.PipelineSpec{ + Params: []v1beta1.ParamSpec{ + {Name: "first-param", Type: v1beta1.ParamTypeArray, Default: v1beta1.NewStructuredValues("default-value", "default-value-again")}, + }, + Tasks: []v1beta1.PipelineTask{{ + Name: "foo", + TaskRef: &v1beta1.TaskRef{Name: "foo"}, + }}, + Finally: []v1beta1.PipelineTask{{ + Name: "bar", + Params: v1beta1.Params{ + {Name: "first-task-first-param", Value: *v1beta1.NewStructuredValues("$(params.first-param[0])")}, + }, + TaskRef: &v1beta1.TaskRef{Name: "bar"}, + }}, + }, + }, { + name: "pipeline tasks - use of resolver", + spec: v1beta1.PipelineSpec{ + Tasks: []v1beta1.PipelineTask{{ + Name: "uses-resolver", + TaskRef: &v1beta1.TaskRef{ResolverRef: v1beta1.ResolverRef{Resolver: "bar"}}, + }}, + }, + }, { + name: "pipeline tasks - use of resolver params", + spec: v1beta1.PipelineSpec{ + Tasks: []v1beta1.PipelineTask{{ + Name: "uses-resolver-params", + TaskRef: &v1beta1.TaskRef{ResolverRef: v1beta1.ResolverRef{Resolver: "bar", Params: v1beta1.Params{{}}}}, + }}, + }, + }, { + name: "finally tasks - use of resolver", + spec: v1beta1.PipelineSpec{ + Tasks: []v1beta1.PipelineTask{{ + Name: "valid-pipeline-task", + TaskRef: &v1beta1.TaskRef{Name: "foo-task"}, + }}, + Finally: []v1beta1.PipelineTask{{ + Name: "uses-resolver", + TaskRef: &v1beta1.TaskRef{ResolverRef: v1beta1.ResolverRef{Resolver: "bar"}}, + }}, + }, + }, { + name: "finally tasks - use of resolver params", + spec: v1beta1.PipelineSpec{ + Tasks: []v1beta1.PipelineTask{{ + Name: "valid-pipeline-task", + TaskRef: &v1beta1.TaskRef{Name: "foo-task"}, + }}, + Finally: []v1beta1.PipelineTask{{ + Name: "uses-resolver-params", + TaskRef: &v1beta1.TaskRef{ResolverRef: v1beta1.ResolverRef{Resolver: "bar", Params: v1beta1.Params{{}}}}, + }}, + }, + }, { + name: "object params", + spec: v1beta1.PipelineSpec{ + Params: []v1beta1.ParamSpec{ + {Name: "first-param", Type: v1beta1.ParamTypeObject, Properties: map[string]v1beta1.PropertySpec{}}, + }, + Tasks: []v1beta1.PipelineTask{{ + Name: "foo", + TaskRef: &v1beta1.TaskRef{Name: "foo"}, + }}, + }, + }, { + name: "object params in Tasks", + spec: v1beta1.PipelineSpec{ + Tasks: []v1beta1.PipelineTask{{ + Name: "valid-pipeline-task", + TaskSpec: &v1beta1.EmbeddedTask{TaskSpec: v1beta1.TaskSpec{ + Steps: []v1beta1.Step{{Image: "busybox", Script: "echo hello"}}, + Params: []v1beta1.ParamSpec{{Name: "my-object-param", Type: v1beta1.ParamTypeObject, Properties: map[string]v1beta1.PropertySpec{}}}, + }}, + }}, + }, + }, { + name: "object params in Finally", + spec: v1beta1.PipelineSpec{ + Tasks: []v1beta1.PipelineTask{{ + Name: "foo", + TaskRef: &v1beta1.TaskRef{Name: "foo"}, + }}, + Finally: []v1beta1.PipelineTask{{ + Name: "valid-finally-task", + TaskSpec: &v1beta1.EmbeddedTask{TaskSpec: v1beta1.TaskSpec{ + Steps: []v1beta1.Step{{Image: "busybox", Script: "echo hello"}}, + Params: []v1beta1.ParamSpec{{Name: "my-object-param", Type: v1beta1.ParamTypeObject, Properties: map[string]v1beta1.PropertySpec{}}}, + }}, + }}, + }, + }, { + name: "array results", + spec: v1beta1.PipelineSpec{ + Tasks: []v1beta1.PipelineTask{{ + Name: "valid-pipeline-task", + TaskRef: &v1beta1.TaskRef{Name: "foo-task"}, + }}, + Results: []v1beta1.PipelineResult{{Name: "my-array-result", Type: v1beta1.ResultsTypeArray, Value: *v1beta1.NewStructuredValues("$(tasks.valid-pipeline-task.results.foo[*])")}}, + }, + }, { + name: "array results in Tasks", + spec: v1beta1.PipelineSpec{ + Tasks: []v1beta1.PipelineTask{{ + Name: "valid-pipeline-task", + TaskSpec: &v1beta1.EmbeddedTask{TaskSpec: v1beta1.TaskSpec{ + Steps: []v1beta1.Step{{Image: "busybox", Script: "echo hello"}}, + Results: []v1beta1.TaskResult{{Name: "my-array-result", Type: v1beta1.ResultsTypeArray}}, + }}, + }}, + }, + }, { + name: "array results in Finally", + spec: v1beta1.PipelineSpec{ + Tasks: []v1beta1.PipelineTask{{ + Name: "valid-pipeline-task", + TaskRef: &v1beta1.TaskRef{Name: "foo-task"}, + }}, + Finally: []v1beta1.PipelineTask{{ + Name: "valid-finally-task", + TaskSpec: &v1beta1.EmbeddedTask{TaskSpec: v1beta1.TaskSpec{ + Steps: []v1beta1.Step{{Image: "busybox", Script: "echo hello"}}, + Results: []v1beta1.TaskResult{{Name: "my-array-result", Type: v1beta1.ResultsTypeArray}}, + }}, + }}, + }, + }, { + name: "object results", + spec: v1beta1.PipelineSpec{ + Tasks: []v1beta1.PipelineTask{{ + Name: "valid-pipeline-task", + TaskRef: &v1beta1.TaskRef{Name: "foo-task"}, + }}, + Results: []v1beta1.PipelineResult{{Name: "my-object-result", Type: v1beta1.ResultsTypeObject, Value: *v1beta1.NewStructuredValues("$(tasks.valid-pipeline-task.results.foo[*])")}}, + }, + }, { + name: "object results in Tasks", + spec: v1beta1.PipelineSpec{ + Tasks: []v1beta1.PipelineTask{{ + Name: "valid-pipeline-task", + TaskSpec: &v1beta1.EmbeddedTask{TaskSpec: v1beta1.TaskSpec{ + Steps: []v1beta1.Step{{Image: "busybox", Script: "echo hello"}}, + Results: []v1beta1.TaskResult{{Name: "my-object-result", Type: v1beta1.ResultsTypeObject, Properties: map[string]v1beta1.PropertySpec{}}}, + }}, + }}, + }, + }, { + name: "object results in Finally", + spec: v1beta1.PipelineSpec{ + Tasks: []v1beta1.PipelineTask{{ + Name: "valid-pipeline-task", + TaskRef: &v1beta1.TaskRef{Name: "foo-task"}, + }}, + Finally: []v1beta1.PipelineTask{{ + Name: "valid-finally-task", + TaskSpec: &v1beta1.EmbeddedTask{TaskSpec: v1beta1.TaskSpec{ + Steps: []v1beta1.Step{{Image: "busybox", Script: "echo hello"}}, + Results: []v1beta1.TaskResult{{Name: "my-object-result", Type: v1beta1.ResultsTypeObject, Properties: map[string]v1beta1.PropertySpec{}}}, + }}, + }}, + }, + }} + for _, tt := range tts { + t.Run(tt.name, func(t *testing.T) { + pr := v1beta1.PipelineRun{ObjectMeta: metav1.ObjectMeta{Name: "foo"}, Spec: v1beta1.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/v1beta1/task_validation.go b/pkg/apis/pipeline/v1beta1/task_validation.go index aec142cf047..9d525d7eeb6 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation.go +++ b/pkg/apis/pipeline/v1beta1/task_validation.go @@ -65,7 +65,12 @@ 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")) + // 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(t.Spec.ValidateBetaFields(ctx)) + return errs } // Validate implements apis.Validatable @@ -99,6 +104,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..05f67374d98 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 params", config.BetaAPIFields).ViaField("params")) if ref.Name != "" { errs = errs.Also(apis.ErrMultipleOneOf("name", "params")) } diff --git a/pkg/apis/pipeline/v1beta1/taskref_validation_test.go b/pkg/apis/pipeline/v1beta1/taskref_validation_test.go index 424f40c3dd3..8ec39f75fa9 100644 --- a/pkg/apis/pipeline/v1beta1/taskref_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/taskref_validation_test.go @@ -21,6 +21,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + cfgtesting "github.com/tektoncd/pipeline/pkg/apis/config/testing" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/test/diff" "knative.dev/pkg/apis" @@ -37,10 +38,15 @@ func TestTaskRef_Valid(t *testing.T) { name: "simple taskref", taskRef: &v1beta1.TaskRef{Name: "taskrefname"}, }, { - name: "valid resolver", + name: "beta feature: valid resolver", taskRef: &v1beta1.TaskRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git"}}, + wc: cfgtesting.EnableBetaAPIFields, }, { - name: "valid resolver with params", + name: "beta feature: valid resolver with alpha flag", + taskRef: &v1beta1.TaskRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git"}}, + wc: cfgtesting.EnableAlphaAPIFields, + }, { + name: "beta feature: valid resolver with params", taskRef: &v1beta1.TaskRef{ResolverRef: v1beta1.ResolverRef{Resolver: "git", Params: v1beta1.Params{{ Name: "repo", Value: v1beta1.ParamValue{ @@ -171,6 +177,24 @@ func TestTaskRef_Invalid(t *testing.T) { Message: `invalid value: name part must consist of alphanumeric characters, '-', '_' or '.', and must start and end with an alphanumeric character (e.g. 'MyName', or 'my.name', or '123-abc', regex used for validation is '([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]')`, Paths: []string{"name"}, }, + }, { + name: "taskref param object requires beta", + taskRef: &v1beta1.TaskRef{ + ResolverRef: v1beta1.ResolverRef{ + Resolver: "some-resolver", + Params: v1beta1.Params{{ + Name: "foo", + Value: v1beta1.ParamValue{ + Type: v1beta1.ParamTypeObject, + ObjectVal: map[string]string{"bar": "baz"}, + }, + }}, + }, + }, + wc: cfgtesting.EnableStableAPIFields, + wantErr: apis.ErrGeneric("resolver requires \"enable-api-fields\" feature gate to be \"alpha\" or \"beta\" but it is \"stable\"").Also( + apis.ErrGeneric("resolver params requires \"enable-api-fields\" feature gate to be \"alpha\" or \"beta\" but it is \"stable\"")).Also( + apis.ErrGeneric("object type parameter requires \"enable-api-fields\" feature gate to be \"alpha\" or \"beta\" but it is \"stable\"")), }} for _, ts := range tests { t.Run(ts.name, func(t *testing.T) { diff --git a/pkg/apis/pipeline/v1beta1/taskrun_validation.go b/pkg/apis/pipeline/v1beta1/taskrun_validation.go index 22ef336c95d..9acd35dcb7a 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_validation.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_validation.go @@ -63,6 +63,10 @@ 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. + errs = errs.Also(ts.TaskSpec.ValidateBetaFields(ctx).ViaField("taskSpec")) } errs = errs.Also(ValidateParameters(ctx, ts.Params).ViaField("params")) @@ -240,6 +244,11 @@ func ValidateWorkspaceBindings(ctx context.Context, wb []WorkspaceBinding) (errs func ValidateParameters(ctx context.Context, params Params) (errs *apis.FieldError) { var names []string for _, p := range params { + if p.Value.Type == ParamTypeObject { + // Object type parameter is a beta feature and will fail validation if it's used in a taskrun spec + // when the enable-api-fields feature gate is not "alpha" or "beta". + errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "object type parameter", config.BetaAPIFields)) + } names = append(names, p.Name) } return errs.Also(validateNoDuplicateNames(names, false)) diff --git a/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go b/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go index 1aaa409741c..991df62b070 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go @@ -904,3 +904,75 @@ func TestTaskRunSpec_Validate(t *testing.T) { }) } } + +func TestTaskRunBetaFields(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()) + tr := v1beta1.TaskRun{ObjectMeta: metav1.ObjectMeta{Name: "foo"}, Spec: v1beta1.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) + } + }) + } +}