Skip to content

Commit

Permalink
validate beta fields only for Task and Pipeline
Browse files Browse the repository at this point in the history
  • Loading branch information
lbernick committed May 23, 2023
1 parent a309bcd commit 3485ffb
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 8 deletions.
10 changes: 8 additions & 2 deletions pkg/apis/pipeline/v1/pipeline_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,12 @@ func (p *Pipeline) SupportedVerbs() []admissionregistrationv1.OperationType {
func (p *Pipeline) Validate(ctx context.Context) *apis.FieldError {
errs := validate.ObjectMetadata(p.GetObjectMeta()).ViaField("metadata")
ctx = config.SkipValidationDueToPropagatedParametersAndWorkspaces(ctx, false)
return errs.Also(p.Spec.Validate(apis.WithinSpec(ctx)).ViaField("spec"))
errs = errs.Also(p.Spec.Validate(apis.WithinSpec(ctx)).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
Expand All @@ -66,7 +71,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 Expand Up @@ -135,6 +139,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
}
Expand Down
5 changes: 3 additions & 2 deletions pkg/apis/pipeline/v1/pipeline_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3560,12 +3560,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)
}
})
Expand Down
8 changes: 6 additions & 2 deletions pkg/apis/pipeline/v1/task_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,12 @@ var objectVariableNameFormatRegex = regexp.MustCompile(objectVariableNameFormat)
func (t *Task) Validate(ctx context.Context) *apis.FieldError {
errs := validate.ObjectMetadata(t.GetObjectMeta()).ViaField("metadata")
ctx = config.SkipValidationDueToPropagatedParametersAndWorkspaces(ctx, false)
return errs.Also(t.Spec.Validate(apis.WithinSpec(ctx)).ViaField("spec"))
errs = errs.Also(t.Spec.Validate(apis.WithinSpec(ctx)).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
Expand Down Expand Up @@ -95,7 +100,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 @@ -1931,12 +1931,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 3485ffb

Please sign in to comment.