diff --git a/api_compatibility_policy.md b/api_compatibility_policy.md index 36324e59a41..5d2cc231003 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,13 @@ 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 API-driven features are no longer controlled by the `enable-api-fields` flag because they cannot be disabled. -- 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. @@ -145,7 +143,7 @@ Features are first released as experimental in alpha, refined in beta, and final #### Promoting a feature to `beta` - After feedback of the usage of the alpha features, once the needs and motivations are validated, a feature could be promoted to `beta`. This stage is where features are further tested and refined. -- The dedicated feature flag for this feature will change the stability level for validation to `beta`. It will continue to be disabled by default. +- The dedicated feature flag for this feature will change its stability level to `beta`. It will continue to be disabled by default. #### Graduating a feature to `stable` - This is the final stage of feature graduation process, where features are considered to be complete and ready to be released for the public. diff --git a/docs/additional-configs.md b/docs/additional-configs.md index a5a5a796d94..11b95952590 100644 --- a/docs/additional-configs.md +++ b/docs/additional-configs.md @@ -325,22 +325,17 @@ 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 | -| [Matrix](./matrix.md) | [TEP-0090](https://github.com/tektoncd/community/blob/main/teps/0090-matrix.md) | [v0.38.0](https://github.com/tektoncd/pipeline/releases/tag/v0.38.0) | [v0.53.0](https://github.com/tektoncd/pipeline/releases/tag/v0.53.0) | | No | -| [Task-level Resource Requirements](compute-resources.md#task-level-compute-resources-configuration) | [TEP-0104](https://github.com/tektoncd/community/blob/main/teps/0104-tasklevel-resource-requirements.md) | [v0.39.0](https://github.com/tektoncd/pipeline/releases/tag/v0.39.0) | | - +| 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) | | +| [Matrix](./matrix.md) | [TEP-0090](https://github.com/tektoncd/community/blob/main/teps/0090-matrix.md) | [v0.38.0](https://github.com/tektoncd/pipeline/releases/tag/v0.38.0) | [v0.53.0](https://github.com/tektoncd/pipeline/releases/tag/v0.53.0) | | +| [Task-level Resource Requirements](compute-resources.md#task-level-compute-resources-configuration) | [TEP-0104](https://github.com/tektoncd/community/blob/main/teps/0104-tasklevel-resource-requirements.md) | [v0.39.0](https://github.com/tektoncd/pipeline/releases/tag/v0.39.0) | [v0.53.0](https://github.com/tektoncd/pipeline/releases/tag/v0.53.0) | | ## Enabling larger results using sidecar logs diff --git a/pkg/apis/pipeline/v1/pipeline_validation.go b/pkg/apis/pipeline/v1/pipeline_validation.go index 3067303e87c..2552692d87a 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1/pipeline_validation.go @@ -58,17 +58,13 @@ func (p *Pipeline) Validate(ctx context.Context) *apis.FieldError { // Validate that all params and workspaces it uses are declared. errs = errs.Also(p.Spec.validatePipelineParameterUsage(ctx).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. - // TODO(#6592): Decouple API versioning from feature versioning - errs = errs.Also(p.Spec.ValidateBetaFields(ctx)) return errs } // Validate checks that taskNames in the Pipeline are valid and that the graph // of Tasks expressed in the Pipeline makes sense. func (ps *PipelineSpec) Validate(ctx context.Context) (errs *apis.FieldError) { + errs = errs.Also(ps.ValidateBetaFields(ctx)) if equality.Semantic.DeepEqual(ps, &PipelineSpec{}) { errs = errs.Also(apis.ErrGeneric("expected at least one, got none", "description", "params", "resources", "tasks", "workspaces")) } diff --git a/pkg/apis/pipeline/v1/pipelinerun_validation.go b/pkg/apis/pipeline/v1/pipelinerun_validation.go index e554263705a..859fe311ff4 100644 --- a/pkg/apis/pipeline/v1/pipelinerun_validation.go +++ b/pkg/apis/pipeline/v1/pipelinerun_validation.go @@ -66,11 +66,6 @@ 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/task_validation.go b/pkg/apis/pipeline/v1/task_validation.go index a7fa0d442a7..e85179d60d1 100644 --- a/pkg/apis/pipeline/v1/task_validation.go +++ b/pkg/apis/pipeline/v1/task_validation.go @@ -65,16 +65,12 @@ func (t *Task) Validate(ctx context.Context) *apis.FieldError { // 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. errs = errs.Also(ValidateUsageOfDeclaredParameters(ctx, t.Spec.Steps, t.Spec.Params).ViaField("spec")) - // 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 } // Validate implements apis.Validatable func (ts *TaskSpec) Validate(ctx context.Context) (errs *apis.FieldError) { + errs = errs.Also(ts.ValidateBetaFields(ctx)) if len(ts.Steps) == 0 { errs = errs.Also(apis.ErrMissingField("steps")) } diff --git a/pkg/apis/pipeline/v1/taskrun_validation.go b/pkg/apis/pipeline/v1/taskrun_validation.go index 87294c4cdc1..b9495138fc4 100644 --- a/pkg/apis/pipeline/v1/taskrun_validation.go +++ b/pkg/apis/pipeline/v1/taskrun_validation.go @@ -62,11 +62,6 @@ 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/v1beta1/pipeline_types_test.go b/pkg/apis/pipeline/v1beta1/pipeline_types_test.go index 680b6b2cafe..7b58610f61e 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_types_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_types_test.go @@ -543,7 +543,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", @@ -551,29 +551,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) { @@ -581,8 +586,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 118fff45a15..bf384b1c815 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation.go @@ -64,6 +64,7 @@ func (p *Pipeline) Validate(ctx context.Context) *apis.FieldError { // Validate checks that taskNames in the Pipeline are valid and that the graph // of Tasks expressed in the Pipeline makes sense. func (ps *PipelineSpec) Validate(ctx context.Context) (errs *apis.FieldError) { + errs = errs.Also(ps.ValidateBetaFields(ctx)) if equality.Semantic.DeepEqual(ps, &PipelineSpec{}) { errs = errs.Also(apis.ErrGeneric("expected at least one, got none", "description", "params", "resources", "tasks", "workspaces")) } @@ -92,6 +93,60 @@ func (ps *PipelineSpec) Validate(ctx context.Context) (errs *apis.FieldError) { return errs } +// ValidateBetaFields returns an error if the PipelineSpec uses beta specifications governed by +// `enable-api-fields` 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(config.ValidateEnabledAPIFields(ctx, "object type parameter", config.BetaAPIFields).ViaFieldIndex("params", i)) + } + } + // Indexing into array parameters + arrayParamIndexingRefs := ps.GetIndexingReferencesToArrayParams() + if len(arrayParamIndexingRefs) != 0 { + errs = errs.Also(config.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(config.ValidateEnabledAPIFields(ctx, "object results", config.BetaAPIFields).ViaFieldIndex("results", i)) + case ResultsTypeArray: + errs = errs.Also(config.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("finally", 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(config.ValidateEnabledAPIFields(ctx, "taskref.resolver", config.BetaAPIFields)) + } + if len(pt.TaskRef.Params) > 0 { + errs = errs.Also(config.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 13d862d61a9..00ceb53d210 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go @@ -4119,6 +4119,209 @@ func enableFeatures(t *testing.T, features []string) func(context.Context) conte } } +// TestPipelineWithBetaFields tests the beta API-driven features of +// PipelineSpec are correctly governed `enable-api-fields`, which must +// be set to "alpha" or "beta". +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..7131f6c62f5 100644 --- a/pkg/apis/pipeline/v1beta1/pipelineref_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipelineref_validation.go @@ -34,6 +34,7 @@ func (ref *PipelineRef) Validate(ctx context.Context) (errs *apis.FieldError) { if ref.Resolver != "" || ref.Params != nil { if ref.Resolver != "" { + errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "resolver", config.BetaAPIFields).ViaField("resolver")) if ref.Name != "" { errs = errs.Also(apis.ErrMultipleOneOf("name", "resolver")) } @@ -42,6 +43,7 @@ func (ref *PipelineRef) Validate(ctx context.Context) (errs *apis.FieldError) { } } if ref.Params != nil { + errs = errs.Also(config.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_test.go b/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go index b21439ef949..d0666d019a4 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go @@ -1557,3 +1557,208 @@ func TestPipelineRunWithTimeout_Validate(t *testing.T) { }) } } + +// TestPipelineRunSpecBetaFeatures tests the beta API-driven features +// of PipelineRunSpec are correctly governed `enable-api-fields`, which +// must be set to "alpha" or "beta". +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 cfdd84433c6..36646a4d1d2 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation.go +++ b/pkg/apis/pipeline/v1beta1/task_validation.go @@ -69,6 +69,7 @@ func (t *Task) Validate(ctx context.Context) *apis.FieldError { // Validate implements apis.Validatable func (ts *TaskSpec) Validate(ctx context.Context) (errs *apis.FieldError) { + errs = errs.Also(ts.ValidateBetaFields(ctx)) if len(ts.Steps) == 0 { errs = errs.Also(apis.ErrMissingField("steps")) } @@ -98,6 +99,35 @@ func (ts *TaskSpec) Validate(ctx context.Context) (errs *apis.FieldError) { return errs } +// ValidateBetaFields returns an error if the TaskSpec uses beta specifications governed by +// `enable-api-fields` 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(config.ValidateEnabledAPIFields(ctx, "object type parameter", config.BetaAPIFields).ViaFieldIndex("params", i)) + } + } + // Indexing into array parameters + arrayIndexParamRefs := ts.GetIndexingReferencesToArrayParams() + if len(arrayIndexParamRefs) != 0 { + errs = errs.Also(config.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(config.ValidateEnabledAPIFields(ctx, "object results", config.BetaAPIFields).ViaFieldIndex("results", i)) + case ResultsTypeArray: + errs = errs.Also(config.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..a8bbe480ae8 100644 --- a/pkg/apis/pipeline/v1beta1/taskref_validation.go +++ b/pkg/apis/pipeline/v1beta1/taskref_validation.go @@ -21,6 +21,7 @@ import ( "strings" "github.com/google/go-containerregistry/pkg/name" + "github.com/tektoncd/pipeline/pkg/apis/config" "k8s.io/apimachinery/pkg/util/validation" "knative.dev/pkg/apis" ) @@ -35,6 +36,7 @@ func (ref *TaskRef) Validate(ctx context.Context) (errs *apis.FieldError) { switch { case ref.Resolver != "" || ref.Params != nil: if ref.Resolver != "" { + errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "resolver", config.BetaAPIFields).ViaField("resolver")) if ref.Name != "" { errs = errs.Also(apis.ErrMultipleOneOf("name", "resolver")) } @@ -43,6 +45,7 @@ func (ref *TaskRef) Validate(ctx context.Context) (errs *apis.FieldError) { } } if ref.Params != nil { + errs = errs.Also(config.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 e5108a6464c..0baf7dfc7ab 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_validation.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_validation.go @@ -239,6 +239,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(config.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 6e604fb6e36..3207331b0fd 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) + } + }) + } +} diff --git a/pkg/reconciler/pipelinerun/resources/pipelineref.go b/pkg/reconciler/pipelinerun/resources/pipelineref.go index 12396d5631a..ffbbbbfd5a0 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelineref.go +++ b/pkg/reconciler/pipelinerun/resources/pipelineref.go @@ -140,9 +140,6 @@ func readRuntimeObjectAsPipeline(ctx context.Context, namespace string, obj runt vr := trustedresources.VerifyResource(ctx, obj, k8s, refSource, verificationPolicies) // Issue a dry-run request to create the remote Pipeline, so that it can undergo validation from validating admission webhooks // 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 } diff --git a/pkg/reconciler/taskrun/resources/taskref.go b/pkg/reconciler/taskrun/resources/taskref.go index c33ec14c903..a2b547cdbcc 100644 --- a/pkg/reconciler/taskrun/resources/taskref.go +++ b/pkg/reconciler/taskrun/resources/taskref.go @@ -158,8 +158,6 @@ func readRuntimeObjectAsTask(ctx context.Context, namespace string, obj runtime. vr := trustedresources.VerifyResource(ctx, obj, k8s, refSource, verificationPolicies) // Issue a dry-run request to create the remote Task, so that it can undergo validation from validating admission webhooks // without actually creating the Task on the cluster. - // Validation must happen before converting the Task into the storage version of the API, - // since validation differs between API versions. if err := apiserver.DryRunValidate(ctx, namespace, obj, tekton); err != nil { return nil, nil, err }