From 2a4353e77cb47e189f8f9cd78f64d858d2bde9a9 Mon Sep 17 00:00:00 2001 From: JeromeJu Date: Wed, 30 Aug 2023 15:24:36 +0000 Subject: [PATCH] Move Validation of Task and Pipeline to TaskSpec and PipelineSpec This commit moves the Task.Validate and Pipeline.Validate back to TaskSpec.Validate and PipelineSpec.Validate. This serves the same purpose of fixing #7077. --- pkg/apis/pipeline/v1/pipeline_validation.go | 6 +----- pkg/apis/pipeline/v1/pipelinerun_validation.go | 5 ----- pkg/apis/pipeline/v1/task_validation.go | 6 +----- pkg/apis/pipeline/v1/taskrun_validation.go | 5 ----- pkg/apis/pipeline/v1beta1/pipeline_validation.go | 8 ++------ pkg/apis/pipeline/v1beta1/pipelinerun_validation.go | 5 ----- pkg/apis/pipeline/v1beta1/task_validation.go | 8 ++------ pkg/apis/pipeline/v1beta1/taskrun_validation.go | 4 ---- pkg/reconciler/pipelinerun/resources/pipelineref.go | 3 --- pkg/reconciler/taskrun/resources/taskref.go | 2 -- 10 files changed, 6 insertions(+), 46 deletions(-) diff --git a/pkg/apis/pipeline/v1/pipeline_validation.go b/pkg/apis/pipeline/v1/pipeline_validation.go index 71b002d9bcd..76ac5b045d8 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1/pipeline_validation.go @@ -59,11 +59,6 @@ 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 } @@ -93,6 +88,7 @@ func (ps *PipelineSpec) Validate(ctx context.Context) (errs *apis.FieldError) { errs = errs.Also(validateMatrix(ctx, ps.Tasks).ViaField("tasks")) errs = errs.Also(validateMatrix(ctx, ps.Finally).ViaField("finally")) errs = errs.Also(validateResultsFromMatrixedPipelineTasksNotConsumed(ps.Tasks, ps.Finally)) + errs = errs.Also(ps.ValidateBetaFields(ctx)) return errs } diff --git a/pkg/apis/pipeline/v1/pipelinerun_validation.go b/pkg/apis/pipeline/v1/pipelinerun_validation.go index dc95f47ccbe..1e4365e5f29 100644 --- a/pkg/apis/pipeline/v1/pipelinerun_validation.go +++ b/pkg/apis/pipeline/v1/pipelinerun_validation.go @@ -67,11 +67,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")) } // Validate PipelineRun parameters diff --git a/pkg/apis/pipeline/v1/task_validation.go b/pkg/apis/pipeline/v1/task_validation.go index 8a051618fe0..c23c8de91b2 100644 --- a/pkg/apis/pipeline/v1/task_validation.go +++ b/pkg/apis/pipeline/v1/task_validation.go @@ -66,11 +66,6 @@ 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 } @@ -99,6 +94,7 @@ func (ts *TaskSpec) Validate(ctx context.Context) (errs *apis.FieldError) { 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")) + errs = errs.Also(ts.ValidateBetaFields(ctx)) return errs } diff --git a/pkg/apis/pipeline/v1/taskrun_validation.go b/pkg/apis/pipeline/v1/taskrun_validation.go index c818a5760c2..7fb04691e98 100644 --- a/pkg/apis/pipeline/v1/taskrun_validation.go +++ b/pkg/apis/pipeline/v1/taskrun_validation.go @@ -63,11 +63,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")) diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation.go b/pkg/apis/pipeline/v1beta1/pipeline_validation.go index c103b5ef966..ba598c221d1 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation.go @@ -59,12 +59,7 @@ 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")) - 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 + return errs.Also(p.Spec.validatePipelineWorkspacesUsage().ViaField("spec")) } // Validate checks that taskNames in the Pipeline are valid and that the graph @@ -96,6 +91,7 @@ func (ps *PipelineSpec) Validate(ctx context.Context) (errs *apis.FieldError) { errs = errs.Also(validateMatrix(ctx, ps.Tasks).ViaField("tasks")) errs = errs.Also(validateMatrix(ctx, ps.Finally).ViaField("finally")) errs = errs.Also(validateResultsFromMatrixedPipelineTasksNotConsumed(ps.Tasks, ps.Finally)) + errs = errs.Also(ps.ValidateBetaFields(ctx)) return errs } diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go b/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go index e990f332bc8..307f7b8f2e6 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go @@ -72,11 +72,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")) } // Validate PipelineRun parameters diff --git a/pkg/apis/pipeline/v1beta1/task_validation.go b/pkg/apis/pipeline/v1beta1/task_validation.go index 9d525d7eeb6..9e5a5509d9f 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation.go +++ b/pkg/apis/pipeline/v1beta1/task_validation.go @@ -65,12 +65,7 @@ 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. - errs = errs.Also(ValidateUsageOfDeclaredParameters(ctx, t.Spec.Steps, t.Spec.Params).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(t.Spec.ValidateBetaFields(ctx)) - return errs + return errs.Also(ValidateUsageOfDeclaredParameters(ctx, t.Spec.Steps, t.Spec.Params).ViaField("spec")) } // Validate implements apis.Validatable @@ -101,6 +96,7 @@ func (ts *TaskSpec) Validate(ctx context.Context) (errs *apis.FieldError) { if ts.Resources != nil { errs = errs.Also(apis.ErrDisallowedFields("resources")) } + errs = errs.Also(ts.ValidateBetaFields(ctx)) return errs } diff --git a/pkg/apis/pipeline/v1beta1/taskrun_validation.go b/pkg/apis/pipeline/v1beta1/taskrun_validation.go index 4bcb8e94312..070dd2e1caf 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_validation.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_validation.go @@ -63,10 +63,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. - errs = errs.Also(ts.TaskSpec.ValidateBetaFields(ctx).ViaField("taskSpec")) } errs = errs.Also(ValidateParameters(ctx, ts.Params).ViaField("params")) diff --git a/pkg/reconciler/pipelinerun/resources/pipelineref.go b/pkg/reconciler/pipelinerun/resources/pipelineref.go index 12396d5631a..ffbbbbfd5a0 100644 --- a/pkg/reconciler/pipelinerun/resources/pipelineref.go +++ b/pkg/reconciler/pipelinerun/resources/pipelineref.go @@ -140,9 +140,6 @@ func readRuntimeObjectAsPipeline(ctx context.Context, namespace string, obj runt vr := trustedresources.VerifyResource(ctx, obj, k8s, refSource, verificationPolicies) // Issue a dry-run request to create the remote Pipeline, so that it can undergo validation from validating admission webhooks // without actually creating the Pipeline on the cluster. - // Validation must happen before the v1beta1 Pipeline is converted into the storage version of the API, - // since validation of beta features differs between v1 and v1beta1 - // TODO(#6592): Decouple API versioning from feature versioning if err := apiserver.DryRunValidate(ctx, namespace, obj, tekton); err != nil { return nil, nil, err } diff --git a/pkg/reconciler/taskrun/resources/taskref.go b/pkg/reconciler/taskrun/resources/taskref.go index c33ec14c903..a2b547cdbcc 100644 --- a/pkg/reconciler/taskrun/resources/taskref.go +++ b/pkg/reconciler/taskrun/resources/taskref.go @@ -158,8 +158,6 @@ func readRuntimeObjectAsTask(ctx context.Context, namespace string, obj runtime. vr := trustedresources.VerifyResource(ctx, obj, k8s, refSource, verificationPolicies) // Issue a dry-run request to create the remote Task, so that it can undergo validation from validating admission webhooks // without actually creating the Task on the cluster. - // Validation must happen before converting the Task into the storage version of the API, - // since validation differs between API versions. if err := apiserver.DryRunValidate(ctx, namespace, obj, tekton); err != nil { return nil, nil, err }