diff --git a/docs/additional-configs.md b/docs/additional-configs.md index b568a76d9e1..aceb6622b5e 100644 --- a/docs/additional-configs.md +++ b/docs/additional-configs.md @@ -333,7 +333,6 @@ Features currently in "beta" are: | 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) | | | [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) | | diff --git a/docs/pipeline-api.md b/docs/pipeline-api.md index 4d09b848f60..f7e0439660c 100644 --- a/docs/pipeline-api.md +++ b/docs/pipeline-api.md @@ -2026,8 +2026,7 @@ PipelineRun has provided an invalid binding.

misses some keys required for the object param declared in Pipeline spec.

"ParamArrayIndexingInvalid"

-

ReasonParamArrayIndexingInvalid indicates that the use of param array indexing is not under correct api fields feature gate -or the array is out of bound.

+

ReasonParamArrayIndexingInvalid indicates that the use of param array indexing is out of bound.

"ParameterMissing"

ReasonParameterMissing indicates that the reason for the failure status is that the diff --git a/docs/pipelines.md b/docs/pipelines.md index 3ce6e029e7f..2de52d549c0 100644 --- a/docs/pipelines.md +++ b/docs/pipelines.md @@ -1375,8 +1375,7 @@ results: For an end-to-end example, see [`Results` in a `PipelineRun`](../examples/v1/pipelineruns/pipelinerun-results.yaml). -Array result is a beta feature, -see [`Array and Object Results` in a `PipelineRun`](../examples/v1/pipelineruns/beta/pipeline-emitting-results.yaml). +In the example below, the `Pipeline` collects array and object results from `Tasks`. ```yaml results: @@ -1398,6 +1397,7 @@ see [`Array and Object Results` in a `PipelineRun`](../examples/v1/pipelineruns/ value: $(tasks.task2.results.object-results.foo) ``` +For an end-to-end example see [`Array and Object Results` in a `PipelineRun`](../examples/v1/pipelineruns/pipeline-emitting-results.yaml). A `Pipeline Result` is not emitted if any of the following are true: - A `PipelineTask` referenced by the `Pipeline Result` failed. The `PipelineRun` will also diff --git a/docs/stepactions.md b/docs/stepactions.md index 8dc3ed91c5b..570c9f55b51 100644 --- a/docs/stepactions.md +++ b/docs/stepactions.md @@ -110,7 +110,7 @@ spec: ``` > :seedling: **`params` cannot be directly used in a `script` in `StepActions`.** -> Directly substituting `params` in `scripts` makes the workload prone to shell attacks. Therefore, we do not allow direct usage of `params` in `scripts` in `StepActions`. Instead, rely on passing `params` to `env` variables and reference them in `scripts`. We cannot do the same for `inlined-steps` because it breaks `v1 API` compatibility for existing users. +> Directly substituting `params` in `scripts` makes the workload prone to shell attacks. Therefore, we do not allow direct usage of `params` in `scripts` in `StepActions`. Instead, rely on passing `params` to `env` variables and reference them in `scripts`. We cannot do the same for `inlined-steps` because it breaks `v1 API` compatibility for existing users. #### Passing Params to StepAction @@ -159,7 +159,7 @@ spec: date | tee $(results.current-date-human-readable.path) ``` -It is possible that a `StepAction` with `Results` is used multiple times in the same `Task` or multiple `StepActions` in the same `Task` produce `Results` with the same name. Resolving the `Result` names becomes critical otherwise there could be unexpected outcomes. The `Task` needs to be able to resolve these `Result` names clashes by mapping it to a different `Result` name. For this reason, we introduce the capability to store results on a `Step` level. +It is possible that a `StepAction` with `Results` is used multiple times in the same `Task` or multiple `StepActions` in the same `Task` produce `Results` with the same name. Resolving the `Result` names becomes critical otherwise there could be unexpected outcomes. The `Task` needs to be able to resolve these `Result` names clashes by mapping it to a different `Result` name. For this reason, we introduce the capability to store results on a `Step` level. `StepActions` can also emit `Results` to `$(step.results..path)`. @@ -243,7 +243,6 @@ spec: `StepResults` (i.e. results written to `$(step.results..path)`, NOT `$(results..path)`) can be shared with following steps via replacement variable `$(steps..results.)`. Pipeline supports two new types of results and parameters: array `[]string` and object `map[string]string`. -Array and Object result is a beta feature and can be enabled by setting `enable-api-fields` to `alpha` or `beta`. | Result Type | Parameter Type | Specification | `enable-api-fields` | |-------------|----------------|--------------------------------------------------|---------------------| @@ -276,12 +275,12 @@ spec: IMAGE_URL: type: string IMAGE_DIGEST: - type: string + type: string image: alpine script: | echo -n "[\"image1\", \"image2\", \"image3\"]" | tee $(step.results.result1.path) echo -n "foo" | tee $(step.results.result2.path) - echo -n "{\"IMAGE_URL\":\"ar.com\", \"IMAGE_DIGEST\":\"sha234\"}" | tee $(step.results.result3.path) + echo -n "{\"IMAGE_URL\":\"ar.com\", \"IMAGE_DIGEST\":\"sha234\"}" | tee $(step.results.result3.path) - name: action-runner ref: name: step-action @@ -314,7 +313,7 @@ spec: ``` The `Task` using the `StepAction` has more context about how the `Steps` have been orchestrated. As such, the `Task` should be able to update the `workingDir` of the `StepAction` so that the `StepAction` is executed from the correct location. -The `StepAction` can parametrize the `workingDir` and work relative to it. This way, the `Task` does not really need control over the workingDir, it just needs to pass the path as a parameter. +The `StepAction` can parametrize the `workingDir` and work relative to it. This way, the `Task` does not really need control over the workingDir, it just needs to pass the path as a parameter. ```yaml apiVersion: tekton.dev/v1alpha1 diff --git a/docs/tasks.md b/docs/tasks.md index 82e6f97278b..0529e5ff8ee 100644 --- a/docs/tasks.md +++ b/docs/tasks.md @@ -570,7 +570,7 @@ For example, `foo.Is-Bar_` is a valid parameter name for string or array type, b > 2. If a parameter name contains dots (.), it must be referenced by using the [bracket notation](#substituting-parameters-and-resources) with either single or double quotes i.e. `$(params['foo.bar'])`, `$(params["foo.bar"])`. See the following example for more information. #### Parameter type -Each declared parameter has a `type` field, which can be set to `string`, `array` (beta feature) or `object` (beta feature). +Each declared parameter has a `type` field, which can be set to `string`, `array` or `object`. ##### `object` type @@ -588,13 +588,12 @@ spec: type: string ``` -Refer to the [TaskRun example](../examples/v1/taskruns/beta/object-param-result.yaml) and the [PipelineRun example](../examples/v1/pipelineruns/beta/pipeline-object-param-and-result.yaml) in which `object` parameters are demonstrated. +Refer to the [TaskRun example](../examples/v1/taskruns/object-param-result.yaml) and the [PipelineRun example](../examples/v1/pipelineruns/pipeline-object-param-and-result.yaml) in which `object` parameters are demonstrated. > NOTE: - > - `object` param is a `beta` feature and gated by the `alpha` or `beta` feature flag. > - `object` param must specify the `properties` section to define the schema i.e. what keys are available for this object param. See how to define `properties` section in the following example and the [TEP-0075](https://github.com/tektoncd/community/blob/main/teps/0075-object-param-and-result-types.md#defaulting-to-string-types-for-values). - > - When providing value for an `object` param, one may provide values for just a subset of keys in spec's `default`, and provide values for the rest of keys at runtime ([example](../examples/v1/taskruns/beta/object-param-result.yaml)). - > - When using object in variable replacement, users can only access its individual key ("child" member) of the object by its name i.e. `$(params.gitrepo.url)`. Using an entire object as a value is only allowed when the value is also an object like [this example](https://github.com/tektoncd/pipeline/blob/55665765e4de35b3a4fb541549ae8cdef0996641/examples/v1/pipelineruns/beta/pipeline-object-param-and-result.yaml#L64-L65). See more details about using object param from the [TEP-0075](https://github.com/tektoncd/community/blob/main/teps/0075-object-param-and-result-types.md#using-objects-in-variable-replacement). + > - When providing value for an `object` param, one may provide values for just a subset of keys in spec's `default`, and provide values for the rest of keys at runtime ([example](../examples/v1/taskruns/object-param-result.yaml)). + > - When using object in variable replacement, users can only access its individual key ("child" member) of the object by its name i.e. `$(params.gitrepo.url)`. Using an entire object as a value is only allowed when the value is also an object like [this example](../examples/v1/pipelineruns/pipeline-object-param-and-result.yaml). See more details about using object param from the [TEP-0075](https://github.com/tektoncd/community/blob/main/teps/0075-object-param-and-result-types.md#using-objects-in-variable-replacement). ##### `array` type diff --git a/examples/v1/pipelineruns/beta/pipelinerun-array-results-substitution.yaml b/examples/v1/pipelineruns/pipelinerun-array-results-substitution.yaml similarity index 100% rename from examples/v1/pipelineruns/beta/pipelinerun-array-results-substitution.yaml rename to examples/v1/pipelineruns/pipelinerun-array-results-substitution.yaml diff --git a/examples/v1/pipelineruns/beta/pipelinerun-param-array-indexing.yaml b/examples/v1/pipelineruns/pipelinerun-param-array-indexing.yaml similarity index 100% rename from examples/v1/pipelineruns/beta/pipelinerun-param-array-indexing.yaml rename to examples/v1/pipelineruns/pipelinerun-param-array-indexing.yaml diff --git a/pkg/apis/pipeline/v1/pipeline_validation.go b/pkg/apis/pipeline/v1/pipeline_validation.go index c6d7d0ee9de..164046412e8 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1/pipeline_validation.go @@ -95,22 +95,6 @@ func (ps *PipelineSpec) Validate(ctx context.Context) (errs *apis.FieldError) { // have "enable-api-fields" set to "alpha" or "beta". func (ps *PipelineSpec) ValidateBetaFields(ctx context.Context) *apis.FieldError { var errs *apis.FieldError - // 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: - // stable feature - 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)) } @@ -133,8 +117,6 @@ func (pt *PipelineTask) validateBetaFields(ctx context.Context) *apis.FieldError 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 } diff --git a/pkg/apis/pipeline/v1/pipeline_validation_test.go b/pkg/apis/pipeline/v1/pipeline_validation_test.go index cf7344a7b35..4ee7acc508c 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1/pipeline_validation_test.go @@ -4150,38 +4150,6 @@ func TestPipelineWithBetaFields(t *testing.T) { 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{{ @@ -4221,41 +4189,6 @@ func TestPipelineWithBetaFields(t *testing.T) { TaskRef: &TaskRef{ResolverRef: ResolverRef{Resolver: "bar", Params: Params{{}}}}, }}, }, - }, { - 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}}, - }}, - }}, - }, }} for _, tt := range tts { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/apis/pipeline/v1/pipelinerun_types.go b/pkg/apis/pipeline/v1/pipelinerun_types.go index c85389c249d..951a0d42621 100644 --- a/pkg/apis/pipeline/v1/pipelinerun_types.go +++ b/pkg/apis/pipeline/v1/pipelinerun_types.go @@ -370,8 +370,7 @@ const ( // ReasonObjectParameterMissKeys indicates that the object param value provided from PipelineRun spec // misses some keys required for the object param declared in Pipeline spec. PipelineRunReasonObjectParameterMissKeys PipelineRunReason = "ObjectParameterMissKeys" - // ReasonParamArrayIndexingInvalid indicates that the use of param array indexing is not under correct api fields feature gate - // or the array is out of bound. + // ReasonParamArrayIndexingInvalid indicates that the use of param array indexing is out of bound. PipelineRunReasonParamArrayIndexingInvalid PipelineRunReason = "ParamArrayIndexingInvalid" // ReasonCouldntGetTask indicates that the reason for the failure status is that the // associated Pipeline's Tasks couldn't all be retrieved diff --git a/pkg/apis/pipeline/v1/pipelinerun_validation_test.go b/pkg/apis/pipeline/v1/pipelinerun_validation_test.go index f6862bb7357..825002d0a36 100644 --- a/pkg/apis/pipeline/v1/pipelinerun_validation_test.go +++ b/pkg/apis/pipeline/v1/pipelinerun_validation_test.go @@ -1415,38 +1415,6 @@ func TestPipelineRunSpecBetaFeatures(t *testing.T) { name string spec v1.PipelineSpec }{{ - name: "array indexing in Tasks", - spec: v1.PipelineSpec{ - Params: []v1.ParamSpec{ - {Name: "first-param", Type: v1.ParamTypeArray, Default: v1.NewStructuredValues("default-value", "default-value-again")}, - }, - Tasks: []v1.PipelineTask{{ - Name: "foo", - Params: v1.Params{ - {Name: "first-task-first-param", Value: *v1.NewStructuredValues("$(params.first-param[0])")}, - }, - TaskRef: &v1.TaskRef{Name: "foo"}, - }}, - }, - }, { - name: "array indexing in Finally", - spec: v1.PipelineSpec{ - Params: []v1.ParamSpec{ - {Name: "first-param", Type: v1.ParamTypeArray, Default: v1.NewStructuredValues("default-value", "default-value-again")}, - }, - Tasks: []v1.PipelineTask{{ - Name: "foo", - TaskRef: &v1.TaskRef{Name: "foo"}, - }}, - Finally: []v1.PipelineTask{{ - Name: "bar", - Params: v1.Params{ - {Name: "first-task-first-param", Value: *v1.NewStructuredValues("$(params.first-param[0])")}, - }, - TaskRef: &v1.TaskRef{Name: "bar"}, - }}, - }, - }, { name: "pipeline tasks - use of resolver", spec: v1.PipelineSpec{ Tasks: []v1.PipelineTask{{ @@ -1486,41 +1454,6 @@ func TestPipelineRunSpecBetaFeatures(t *testing.T) { TaskRef: &v1.TaskRef{ResolverRef: v1.ResolverRef{Resolver: "bar", Params: v1.Params{{}}}}, }}, }, - }, { - name: "array results", - spec: v1.PipelineSpec{ - Tasks: []v1.PipelineTask{{ - Name: "valid-pipeline-task", - TaskRef: &v1.TaskRef{Name: "foo-task"}, - }}, - Results: []v1.PipelineResult{{Name: "my-array-result", Type: v1.ResultsTypeArray, Value: *v1.NewStructuredValues("$(tasks.valid-pipeline-task.results.foo[*])")}}, - }, - }, { - name: "array results in Tasks", - spec: v1.PipelineSpec{ - Tasks: []v1.PipelineTask{{ - Name: "valid-pipeline-task", - TaskSpec: &v1.EmbeddedTask{TaskSpec: v1.TaskSpec{ - Steps: []v1.Step{{Image: "busybox", Script: "echo hello"}}, - Results: []v1.TaskResult{{Name: "my-array-result", Type: v1.ResultsTypeArray}}, - }}, - }}, - }, - }, { - name: "array results in Finally", - spec: v1.PipelineSpec{ - Tasks: []v1.PipelineTask{{ - Name: "valid-pipeline-task", - TaskRef: &v1.TaskRef{Name: "foo-task"}, - }}, - Finally: []v1.PipelineTask{{ - Name: "valid-finally-task", - TaskSpec: &v1.EmbeddedTask{TaskSpec: v1.TaskSpec{ - Steps: []v1.Step{{Image: "busybox", Script: "echo hello"}}, - Results: []v1.TaskResult{{Name: "my-array-result", Type: v1.ResultsTypeArray}}, - }}, - }}, - }, }} for _, tt := range tts { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/apis/pipeline/v1/task_validation.go b/pkg/apis/pipeline/v1/task_validation.go index 8fadf0940e4..5c6b8b27edc 100644 --- a/pkg/apis/pipeline/v1/task_validation.go +++ b/pkg/apis/pipeline/v1/task_validation.go @@ -72,7 +72,6 @@ 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")) } @@ -99,29 +98,6 @@ 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 - // 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: - // stable feature - 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/v1/task_validation_test.go b/pkg/apis/pipeline/v1/task_validation_test.go index 098947b6fc4..e8a1d1f0427 100644 --- a/pkg/apis/pipeline/v1/task_validation_test.go +++ b/pkg/apis/pipeline/v1/task_validation_test.go @@ -2109,51 +2109,6 @@ func TestIncompatibleAPIVersions(t *testing.T) { } } -func TestTaskBetaFields(t *testing.T) { - tests := []struct { - name string - spec v1.TaskSpec - }{{ - name: "array param indexing", - spec: v1.TaskSpec{ - Params: []v1.ParamSpec{{Name: "foo", Type: v1.ParamTypeArray}}, - Steps: []v1.Step{{ - Name: "my-step", - Image: "my-image", - Script: ` - #!/usr/bin/env bash - echo $(params.foo[1])`, - }}, - }, - }, { - name: "array results", - spec: v1.TaskSpec{ - Results: []v1.TaskResult{{Name: "array-result", Type: v1.ResultsTypeArray}}, - Steps: []v1.Step{{ - Name: "my-step", - Image: "my-image", - Script: ` - #!/usr/bin/env bash - echo -n "[\"hello\",\"world\"]" | tee $(results.array-result.path)`, - }}, - }, - }} - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - ctx := cfgtesting.EnableStableAPIFields(context.Background()) - task := v1.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 TestTaskSpecValidateUsageOfDeclaredParams(t *testing.T) { tests := []struct { name string diff --git a/pkg/apis/pipeline/v1/taskrun_validation_test.go b/pkg/apis/pipeline/v1/taskrun_validation_test.go index 06a99b840df..e418256c042 100644 --- a/pkg/apis/pipeline/v1/taskrun_validation_test.go +++ b/pkg/apis/pipeline/v1/taskrun_validation_test.go @@ -898,50 +898,3 @@ func TestTaskRunSpec_Validate(t *testing.T) { }) } } - -func TestTaskRunBetaFields(t *testing.T) { - tests := []struct { - name string - spec v1.TaskSpec - }{{ - name: "array param indexing", - spec: v1.TaskSpec{ - Params: []v1.ParamSpec{{Name: "foo", Type: v1.ParamTypeArray}}, - Steps: []v1.Step{{ - Name: "my-step", - Image: "my-image", - Script: ` - #!/usr/bin/env bash - echo $(params.foo[1])`, - }}, - }, - }, { - name: "array results", - spec: v1.TaskSpec{ - Results: []v1.TaskResult{{Name: "array-result", Type: v1.ResultsTypeArray}}, - Steps: []v1.Step{{ - Name: "my-step", - Image: "my-image", - Script: ` - #!/usr/bin/env bash - echo -n "[\"hello\",\"world\"]" | tee $(results.array-result.path)`, - }}, - }, - }} - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - ctx := cfgtesting.EnableStableAPIFields(context.Background()) - tr := v1.TaskRun{ObjectMeta: metav1.ObjectMeta{Name: "foo"}, Spec: v1.TaskRunSpec{ - TaskSpec: &tt.spec, - }} - if err := tr.Validate(ctx); err == nil { - t.Errorf("no error when using beta field when `enable-api-fields` is stable") - } - - ctx = cfgtesting.EnableBetaAPIFields(context.Background()) - if err := tr.Validate(ctx); err != nil { - t.Errorf("unexpected error when using beta field: %s", err) - } - }) - } -} diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation.go b/pkg/apis/pipeline/v1beta1/pipeline_validation.go index a84f79e79a7..1179249626d 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation.go @@ -98,29 +98,12 @@ func (ps *PipelineSpec) Validate(ctx context.Context) (errs *apis.FieldError) { // `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 - // 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: - // stable feature - 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 } @@ -136,8 +119,6 @@ func (pt *PipelineTask) validateBetaFields(ctx context.Context) *apis.FieldError 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 } diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go index 01d6995f668..9f37d95586e 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go @@ -4212,38 +4212,6 @@ func TestPipelineWithBetaFields(t *testing.T) { 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{{ @@ -4283,41 +4251,6 @@ func TestPipelineWithBetaFields(t *testing.T) { TaskRef: &TaskRef{ResolverRef: ResolverRef{Resolver: "bar", Params: Params{{}}}}, }}, }, - }, { - 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}}, - }}, - }}, - }, }} for _, tt := range tts { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go b/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go index 7b0a9794fae..61cdd7bfe47 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go @@ -1566,38 +1566,6 @@ func TestPipelineRunSpecBetaFeatures(t *testing.T) { 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{{ @@ -1637,41 +1605,6 @@ func TestPipelineRunSpecBetaFeatures(t *testing.T) { TaskRef: &v1beta1.TaskRef{ResolverRef: v1beta1.ResolverRef{Resolver: "bar", Params: v1beta1.Params{{}}}}, }}, }, - }, { - 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}}, - }}, - }}, - }, }} for _, tt := range tts { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/apis/pipeline/v1beta1/task_validation.go b/pkg/apis/pipeline/v1beta1/task_validation.go index 80a4bf7f47b..3c26d5bf0fe 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation.go +++ b/pkg/apis/pipeline/v1beta1/task_validation.go @@ -72,7 +72,6 @@ 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")) } @@ -102,29 +101,6 @@ 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 - // 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: - // stable feature - 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 90b8e117720..6d5729617d5 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/task_validation_test.go @@ -2072,51 +2072,6 @@ 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: "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)`, - }}, - }, - }} - 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/taskrun_validation_test.go b/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go index 4020f293ba6..6e604fb6e36 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go @@ -904,50 +904,3 @@ 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: "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)`, - }}, - }, - }} - 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/pipelinerun.go b/pkg/reconciler/pipelinerun/pipelinerun.go index 00e5a557408..b9a01f558e5 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/pipelinerun/pipelinerun.go @@ -97,8 +97,7 @@ var ( // ReasonObjectParameterMissKeys indicates that the object param value provided from PipelineRun spec // misses some keys required for the object param declared in Pipeline spec. ReasonObjectParameterMissKeys = v1.PipelineRunReasonObjectParameterMissKeys.String() - // ReasonParamArrayIndexingInvalid indicates that the use of param array indexing is not under correct api fields feature gate - // or the array is out of bound. + // ReasonParamArrayIndexingInvalid indicates that the use of param array indexing is out of bound. ReasonParamArrayIndexingInvalid = v1.PipelineRunReasonParamArrayIndexingInvalid.String() // ReasonCouldntGetTask indicates that the reason for the failure status is that the // associated Pipeline's Tasks couldn't all be retrieved