Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[TEP0138] Decouple v1beta1 beta feature validation #6941

Merged
merged 2 commits into from
Oct 24, 2023
Merged

[TEP0138] Decouple v1beta1 beta feature validation #6941

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ffc8040
Decouple beta feature validation in v1beta1
JeromeJu Aug 30, 2023
48b1586
Move Validation of Task and Pipeline to TaskSpec and PipelineSpec
JeromeJu Aug 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 14 additions & 16 deletions api_compatibility_policy.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
lbernick marked this conversation as resolved.
Show resolved Hide resolved
|---------------------|----|------|-------|
| 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.

Expand All @@ -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.
JeromeJu marked this conversation as resolved.
Show resolved Hide resolved
- 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.
afrittoli marked this conversation as resolved.
Show resolved Hide resolved

- GA/Stable features will not be removed or changed in a backwards incompatible manner without incrementing the API Version.

Expand All @@ -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.
Expand Down
23 changes: 9 additions & 14 deletions docs/additional-configs.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
6 changes: 1 addition & 5 deletions pkg/apis/pipeline/v1/pipeline_validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
JeromeJu marked this conversation as resolved.
Show resolved Hide resolved
if equality.Semantic.DeepEqual(ps, &PipelineSpec{}) {
errs = errs.Also(apis.ErrGeneric("expected at least one, got none", "description", "params", "resources", "tasks", "workspaces"))
}
Expand Down
5 changes: 0 additions & 5 deletions pkg/apis/pipeline/v1/pipelinerun_validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
JeromeJu marked this conversation as resolved.
Show resolved Hide resolved
}

// Validate PipelineRun parameters
Expand Down
6 changes: 1 addition & 5 deletions pkg/apis/pipeline/v1/task_validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
}
Expand Down
5 changes: 0 additions & 5 deletions pkg/apis/pipeline/v1/taskrun_validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down
13 changes: 9 additions & 4 deletions pkg/apis/pipeline/v1beta1/pipeline_types_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -543,46 +543,51 @@ func TestPipelineTask_ValidateRegularTask_Success(t *testing.T) {
tests := []struct {
name string
tasks PipelineTask
enableAPIFields bool
enableAPIFields string
JeromeJu marked this conversation as resolved.
Show resolved Hide resolved
enableBundles bool
}{{
name: "pipeline task - valid taskRef name",
tasks: PipelineTask{
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) {
ctx := context.Background()
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
Expand Down
55 changes: 55 additions & 0 deletions pkg/apis/pipeline/v1beta1/pipeline_validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
}
Expand Down Expand Up @@ -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:
chitrangpatel marked this conversation as resolved.
Show resolved Hide resolved
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))
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JeromeJu I am sorry to go back on this but we had two new promotions - matrix and task level resources:

https://github.com/tektoncd/pipeline/blob/main/docs/additional-configs.md#beta-features

Do we need them here as well? or I think the features themselves have the check, is that sufficient?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this shall be sufficient. Task-level resources beta promotion and Matrix beta promotion shall already have the beta validations covered.

And this function was meant to bring back the validations that were once missing at the stage TEP0138 was not in place and while feature & API versioning were coupled.


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 {
Expand Down
Loading
Loading