From 7ad93df6e666a73cfc24f3c0e17c51f9c92aa7f1 Mon Sep 17 00:00:00 2001 From: Lee Bernick Date: Mon, 22 May 2023 15:35:28 -0400 Subject: [PATCH] validate beta fields only for Task and Pipeline --- pkg/apis/pipeline/v1/pipeline_validation.go | 10 ++++++++-- pkg/apis/pipeline/v1/pipeline_validation_test.go | 5 +++-- pkg/apis/pipeline/v1/task_validation.go | 8 ++++++-- pkg/apis/pipeline/v1/task_validation_test.go | 5 +++-- 4 files changed, 20 insertions(+), 8 deletions(-) diff --git a/pkg/apis/pipeline/v1/pipeline_validation.go b/pkg/apis/pipeline/v1/pipeline_validation.go index 0e52ce0bd71..44df5e52af5 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1/pipeline_validation.go @@ -51,7 +51,12 @@ func (p *Pipeline) Validate(ctx context.Context) *apis.FieldError { // we do not support propagated parameters and workspaces. // Validate that all params and workspaces it uses are declared. errs = errs.Also(p.Spec.validatePipelineParameterUsage().ViaField("spec")) - return errs.Also(p.Spec.validatePipelineWorkspacesUsage().ViaField("spec")) + errs = errs.Also(p.Spec.validatePipelineWorkspacesUsage().ViaField("spec")) + // Validate beta fields when a Pipeline is defined, but not as part of validating a Pipeline spec. + // This prevents validation from failing when a Pipeline is converted to a different API version. + // See https://github.com/tektoncd/pipeline/issues/6616 for more information. + errs = errs.Also(p.Spec.validateBetaFields(ctx)) + return errs } // Validate checks that taskNames in the Pipeline are valid and that the graph @@ -70,7 +75,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 @@ -132,6 +136,8 @@ func (pt *PipelineTask) validateBetaFields(ctx context.Context) *apis.FieldError if len(pt.TaskRef.Params) > 0 { errs = errs.Also(version.ValidateEnabledAPIFields(ctx, "taskref.params", config.BetaAPIFields)) } + } else if pt.TaskSpec != nil { + errs = errs.Also(pt.TaskSpec.validateBetaFields(ctx)) } return errs } diff --git a/pkg/apis/pipeline/v1/pipeline_validation_test.go b/pkg/apis/pipeline/v1/pipeline_validation_test.go index 327879074bf..c433df09f8c 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1/pipeline_validation_test.go @@ -3555,12 +3555,13 @@ func TestPipelineWithBetaFields(t *testing.T) { }} for _, tt := range tts { t.Run(tt.name, func(t *testing.T) { - if err := tt.spec.Validate(context.Background()); err == nil { + pipeline := Pipeline{ObjectMeta: metav1.ObjectMeta{Name: "foo"}, Spec: tt.spec} + if err := pipeline.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 := pipeline.Validate(ctx); err != nil { t.Errorf("unexpected error when using beta field: %s", err) } }) diff --git a/pkg/apis/pipeline/v1/task_validation.go b/pkg/apis/pipeline/v1/task_validation.go index da95dc04cd9..2912ed98698 100644 --- a/pkg/apis/pipeline/v1/task_validation.go +++ b/pkg/apis/pipeline/v1/task_validation.go @@ -65,7 +65,12 @@ func (t *Task) Validate(ctx context.Context) *apis.FieldError { errs = errs.Also(t.Spec.Validate(apis.WithinSpec(ctx)).ViaField("spec")) // When a Task is created directly, instead of declared inline in a TaskRun or PipelineRun, // we do not support propagated parameters. Validate that all params it uses are declared. - return errs.Also(ValidateUsageOfDeclaredParameters(ctx, t.Spec.Steps, t.Spec.Params).ViaField("spec")) + errs = errs.Also(ValidateUsageOfDeclaredParameters(ctx, t.Spec.Steps, t.Spec.Params).ViaField("spec")) + // Validate beta fields when a 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. + errs = errs.Also(t.Spec.validateBetaFields(ctx)) + return errs } // Validate implements apis.Validatable @@ -90,7 +95,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")) diff --git a/pkg/apis/pipeline/v1/task_validation_test.go b/pkg/apis/pipeline/v1/task_validation_test.go index 0e647179258..0057ae95b64 100644 --- a/pkg/apis/pipeline/v1/task_validation_test.go +++ b/pkg/apis/pipeline/v1/task_validation_test.go @@ -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) } })