From 32e728f686395027d25f1ad4ed13e9562f0c309f 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. part of: #7177 related: TEP0138 /kind misc --- 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 7b2de9f77ff..675a125716e 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1/pipeline_validation.go @@ -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)) if equality.Semantic.DeepEqual(ps, &PipelineSpec{}) { errs = errs.Also(apis.ErrGeneric("expected at least one, got none", "description", "params", "resources", "tasks", "workspaces")) } diff --git a/pkg/apis/pipeline/v1/pipelinerun_validation.go b/pkg/apis/pipeline/v1/pipelinerun_validation.go index ee7a6dc2c53..dd930641c93 100644 --- a/pkg/apis/pipeline/v1/pipelinerun_validation.go +++ b/pkg/apis/pipeline/v1/pipelinerun_validation.go @@ -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")) } // Validate PipelineRun parameters diff --git a/pkg/apis/pipeline/v1/task_validation.go b/pkg/apis/pipeline/v1/task_validation.go index a7fa0d442a7..e85179d60d1 100644 --- a/pkg/apis/pipeline/v1/task_validation.go +++ b/pkg/apis/pipeline/v1/task_validation.go @@ -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")) } diff --git a/pkg/apis/pipeline/v1/taskrun_validation.go b/pkg/apis/pipeline/v1/taskrun_validation.go index d3915dc5d87..86022f48118 100644 --- a/pkg/apis/pipeline/v1/taskrun_validation.go +++ b/pkg/apis/pipeline/v1/taskrun_validation.go @@ -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")) diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation.go b/pkg/apis/pipeline/v1beta1/pipeline_validation.go index 4c37070e0be..bee9f21b20b 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation.go @@ -58,17 +58,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")) - 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 // 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")) } diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go b/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go index a3ffa7d1354..ad6d5cbbdb4 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go @@ -71,11 +71,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 74f78d05afc..36646a4d1d2 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation.go +++ b/pkg/apis/pipeline/v1beta1/task_validation.go @@ -64,16 +64,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. - 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 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")) } diff --git a/pkg/apis/pipeline/v1beta1/taskrun_validation.go b/pkg/apis/pipeline/v1beta1/taskrun_validation.go index dac33333b03..0f9b0c73304 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_validation.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_validation.go @@ -62,10 +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. - 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 }