Skip to content

Commit

Permalink
Move Validation of Task and Pipeline to TaskSpec and PipelineSpec
Browse files Browse the repository at this point in the history
This commit moves the Task.Validate and Pipeline.Validate back to
TaskSpec.Validate and PipelineSpec.Validate. This serves the same
purpose of fixing #7077.
  • Loading branch information
JeromeJu committed Oct 4, 2023
1 parent 59ec2ab commit 2a4353e
Show file tree
Hide file tree
Showing 10 changed files with 6 additions and 46 deletions.
6 changes: 1 addition & 5 deletions pkg/apis/pipeline/v1/pipeline_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down
5 changes: 0 additions & 5 deletions pkg/apis/pipeline/v1/pipelinerun_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 1 addition & 5 deletions pkg/apis/pipeline/v1/task_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down
5 changes: 0 additions & 5 deletions pkg/apis/pipeline/v1/taskrun_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down
8 changes: 2 additions & 6 deletions pkg/apis/pipeline/v1beta1/pipeline_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down
5 changes: 0 additions & 5 deletions pkg/apis/pipeline/v1beta1/pipelinerun_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 2 additions & 6 deletions pkg/apis/pipeline/v1beta1/task_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down
4 changes: 0 additions & 4 deletions pkg/apis/pipeline/v1beta1/taskrun_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down
3 changes: 0 additions & 3 deletions pkg/reconciler/pipelinerun/resources/pipelineref.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 0 additions & 2 deletions pkg/reconciler/taskrun/resources/taskref.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down

0 comments on commit 2a4353e

Please sign in to comment.