Skip to content

Commit

Permalink
Validate beta features only when v1 Tasks and Pipelines are defined
Browse files Browse the repository at this point in the history
Currently, validation differs between api versions: when beta features are used in v1 APIs,
"enable-api-fields" must be set to "alpha" or "beta", but when beta features are used in beta APIs,
"enable-api-fields" may be set to "alpha", "beta", or "stable".

We also validate the specs of referenced Tasks or Pipelines in the TaskRun/PipelineRun reconciler.
This presents a problem when referencing a Task or Pipeline declared locally, since the Task or Pipeline may be converted into a different API version when it's stored.

This commit moves validation for beta features to apply only to Tasks and Pipelines when they are created or updated,
and not called when a Task spec or Pipeline spec is validated.

This commit will allow us to swap the storage version of our API without user-facing impact.
Separately, we plan to decouple feature versioning from API versioning, as it is a better long-term solution.

This commit contains no expected functional changes, since the TaskRun and PipelineRun reconcilers do not
currently validate that"enable-api-fields" is set to "alpha" or "beta" when beta features are used in
referenced Tasks or Pipelines.

This validation will be added for v1 remote Tasks and Pipelines in a separate commit.
  • Loading branch information
lbernick committed May 26, 2023
1 parent 264476b commit 17da549
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 6 deletions.
9 changes: 7 additions & 2 deletions pkg/apis/pipeline/v1/pipeline_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,13 @@ func (p *Pipeline) Validate(ctx context.Context) *apis.FieldError {
// we do not support propagated parameters and workspaces.
// Validate that all params and workspaces it uses are declared.
errs = errs.Also(p.Spec.validatePipelineParameterUsage(ctx).ViaField("spec"))
return errs.Also(p.Spec.validatePipelineWorkspacesUsage().ViaField("spec"))
errs = errs.Also(p.Spec.validatePipelineWorkspacesUsage().ViaField("spec"))
// Validate beta fields when a Pipeline is defined, but not as part of validating a Pipeline spec.
// This prevents validation from failing when a Pipeline is converted to a different API version.
// See https://github.com/tektoncd/pipeline/issues/6616 for more information.
// 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
Expand All @@ -70,7 +76,6 @@ func (ps *PipelineSpec) Validate(ctx context.Context) (errs *apis.FieldError) {
errs = errs.Also(validatePipelineContextVariables(ps.Tasks).ViaField("tasks"))
errs = errs.Also(validatePipelineContextVariables(ps.Finally).ViaField("finally"))
errs = errs.Also(validateExecutionStatusVariables(ps.Tasks, ps.Finally))
errs = errs.Also(ps.ValidateBetaFields(ctx))
// Validate the pipeline's workspaces.
errs = errs.Also(validatePipelineWorkspacesDeclarations(ps.Workspaces))
// Validate the pipeline's results
Expand Down
9 changes: 7 additions & 2 deletions pkg/apis/pipeline/v1/task_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,13 @@ func (t *Task) Validate(ctx context.Context) *apis.FieldError {
errs = errs.Also(t.Spec.Validate(apis.WithinSpec(ctx)).ViaField("spec"))
// When a Task is created directly, instead of declared inline in a TaskRun or PipelineRun,
// we do not support propagated parameters. Validate that all params it uses are declared.
return errs.Also(ValidateUsageOfDeclaredParameters(ctx, t.Spec.Steps, t.Spec.Params).ViaField("spec"))
errs = errs.Also(ValidateUsageOfDeclaredParameters(ctx, t.Spec.Steps, t.Spec.Params).ViaField("spec"))
// Validate beta fields when a 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
Expand All @@ -90,7 +96,6 @@ func (ts *TaskSpec) Validate(ctx context.Context) (errs *apis.FieldError) {
errs = errs.Also(validateSidecarNames(ts.Sidecars))
errs = errs.Also(ValidateParameterTypes(ctx, ts.Params).ViaField("params"))
errs = errs.Also(ValidateParameterVariables(ctx, ts.Steps, ts.Params))
errs = errs.Also(ts.ValidateBetaFields(ctx))
errs = errs.Also(validateTaskContextVariables(ctx, ts.Steps))
errs = errs.Also(validateTaskResultsVariables(ctx, ts.Steps, ts.Results))
errs = errs.Also(validateResults(ctx, ts.Results).ViaField("results"))
Expand Down
5 changes: 3 additions & 2 deletions pkg/apis/pipeline/v1/task_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1925,12 +1925,13 @@ func TestTaskSpecBetaFields(t *testing.T) {
}}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if err := tt.spec.Validate(context.Background()); err == nil {
task := v1.Task{ObjectMeta: metav1.ObjectMeta{Name: "foo"}, Spec: tt.spec}
if err := task.Validate(context.Background()); err == nil {
t.Errorf("no error when using beta field when `enable-api-fields` is stable")
}

ctx := config.EnableBetaAPIFields(context.Background())
if err := tt.spec.Validate(ctx); err != nil {
if err := task.Validate(ctx); err != nil {
t.Errorf("unexpected error when using beta field: %s", err)
}
})
Expand Down

0 comments on commit 17da549

Please sign in to comment.