diff --git a/pkg/apis/pipeline/v1/pipeline_validation.go b/pkg/apis/pipeline/v1/pipeline_validation.go index 5b29b5af4ed..e38b9ebaba0 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1/pipeline_validation.go @@ -50,7 +50,7 @@ func (p *Pipeline) Validate(ctx context.Context) *apis.FieldError { // When a Pipeline is created directly, instead of declared inline in a PipelineRun, // 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")) + errs = errs.Also(p.Spec.validatePipelineParameterUsage(ctx).ViaField("spec")) return errs.Also(p.Spec.validatePipelineWorkspacesUsage().ViaField("spec")) } @@ -110,6 +110,16 @@ func (l PipelineTaskList) Validate(ctx context.Context, taskNames sets.String, p return errs } +// validateUsageOfDeclaredPipelineTaskParameters validates that all parameters referenced in the pipeline Task are declared by the pipeline Task. +func (l PipelineTaskList) validateUsageOfDeclaredPipelineTaskParameters(ctx context.Context, path string) (errs *apis.FieldError) { + for i, t := range l { + if t.TaskSpec != nil { + errs = errs.Also(ValidateUsageOfDeclaredParameters(ctx, t.TaskSpec.Steps, t.TaskSpec.Params).ViaFieldIndex(path, i)) + } + } + return errs +} + // ValidateName checks whether the PipelineTask's name is a valid DNS label func (pt PipelineTask) ValidateName() *apis.FieldError { if err := validation.IsDNS1123Label(pt.Name); len(err) > 0 { @@ -300,7 +310,9 @@ func validatePipelineWorkspacesDeclarations(wss []PipelineWorkspaceDeclaration) } // validatePipelineParameterUsage validates that parameters referenced in the Pipeline are declared by the Pipeline -func (ps *PipelineSpec) validatePipelineParameterUsage() (errs *apis.FieldError) { +func (ps *PipelineSpec) validatePipelineParameterUsage(ctx context.Context) (errs *apis.FieldError) { + errs = errs.Also(PipelineTaskList(ps.Tasks).validateUsageOfDeclaredPipelineTaskParameters(ctx, "tasks")) + errs = errs.Also(PipelineTaskList(ps.Finally).validateUsageOfDeclaredPipelineTaskParameters(ctx, "finally")) errs = errs.Also(validatePipelineTaskParameterUsage(ps.Tasks, ps.Params).ViaField("tasks")) errs = errs.Also(validatePipelineTaskParameterUsage(ps.Finally, ps.Params).ViaField("finally")) return errs diff --git a/pkg/apis/pipeline/v1/pipeline_validation_test.go b/pkg/apis/pipeline/v1/pipeline_validation_test.go index f3ea1f762c0..41073808f40 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1/pipeline_validation_test.go @@ -140,6 +140,58 @@ func TestPipeline_Validate_Failure(t *testing.T) { Message: `expected at least one, got none`, Paths: []string{"spec.description", "spec.params", "spec.resources", "spec.tasks", "spec.workspaces"}, }, + }, { + name: "invalid parameter usage in pipeline task", + p: &Pipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "pipeline"}, + Spec: PipelineSpec{ + Tasks: []PipelineTask{{ + Name: "invalid-pipeline-task", + TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ + Steps: []Step{{ + Name: "some-step", + Image: "some-image", + Script: "$(params.doesnotexist)", + }}, + }}, + }}, + }, + }, + expectedError: apis.FieldError{ + Message: `non-existent variable in "$(params.doesnotexist)"`, + Paths: []string{"spec.tasks[0].steps[0].script"}, + }, + }, { + name: "invalid parameter usage in finally pipeline task", + p: &Pipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "pipeline"}, + Spec: PipelineSpec{ + Tasks: []PipelineTask{{ + Name: "pipeline-task", + TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ + Steps: []Step{{ + Name: "some-step", + Image: "some-image", + Command: []string{"cmd"}, + }}, + }}, + }}, + Finally: []PipelineTask{{ + Name: "invalid-pipeline-task", + TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ + Steps: []Step{{ + Name: "some-step", + Image: "some-image", + Script: "$(params.doesnotexist)", + }}, + }}, + }}, + }, + }, + expectedError: apis.FieldError{ + Message: `non-existent variable in "$(params.doesnotexist)"`, + Paths: []string{"spec.finally[0].steps[0].script"}, + }, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation.go b/pkg/apis/pipeline/v1beta1/pipeline_validation.go index 59996d17af2..2e2ecadf1e8 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation.go @@ -51,7 +51,7 @@ func (p *Pipeline) Validate(ctx context.Context) *apis.FieldError { // When a Pipeline is created directly, instead of declared inline in a PipelineRun, // 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")) + errs = errs.Also(p.Spec.validatePipelineParameterUsage(ctx).ViaField("spec")) return errs.Also(p.Spec.validatePipelineWorkspacesUsage().ViaField("spec")) } @@ -113,6 +113,16 @@ func (l PipelineTaskList) Validate(ctx context.Context, taskNames sets.String, p return errs } +// validateUsageOfDeclaredPipelineTaskParameters validates that all parameters referenced in the pipeline Task are declared by the pipeline Task. +func (l PipelineTaskList) validateUsageOfDeclaredPipelineTaskParameters(ctx context.Context, path string) (errs *apis.FieldError) { + for i, t := range l { + if t.TaskSpec != nil { + errs = errs.Also(ValidateUsageOfDeclaredParameters(ctx, t.TaskSpec.Steps, t.TaskSpec.Params).ViaFieldIndex(path, i)) + } + } + return errs +} + // ValidateName checks whether the PipelineTask's name is a valid DNS label func (pt PipelineTask) ValidateName() *apis.FieldError { if err := validation.IsDNS1123Label(pt.Name); len(err) > 0 { @@ -321,7 +331,9 @@ func validatePipelineWorkspacesDeclarations(wss []PipelineWorkspaceDeclaration) } // validatePipelineParameterUsage validates that parameters referenced in the Pipeline are declared by the Pipeline -func (ps *PipelineSpec) validatePipelineParameterUsage() (errs *apis.FieldError) { +func (ps *PipelineSpec) validatePipelineParameterUsage(ctx context.Context) (errs *apis.FieldError) { + errs = errs.Also(PipelineTaskList(ps.Tasks).validateUsageOfDeclaredPipelineTaskParameters(ctx, "tasks")) + errs = errs.Also(PipelineTaskList(ps.Finally).validateUsageOfDeclaredPipelineTaskParameters(ctx, "finally")) errs = errs.Also(validatePipelineTaskParameterUsage(ps.Tasks, ps.Params).ViaField("tasks")) errs = errs.Also(validatePipelineTaskParameterUsage(ps.Finally, ps.Params).ViaField("finally")) return errs diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go index 215698ecb79..f35aef60ea2 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go @@ -142,6 +142,58 @@ func TestPipeline_Validate_Failure(t *testing.T) { Message: `expected at least one, got none`, Paths: []string{"spec.description", "spec.params", "spec.resources", "spec.tasks", "spec.workspaces"}, }, + }, { + name: "invalid parameter usage in pipeline task", + p: &Pipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "pipeline"}, + Spec: PipelineSpec{ + Tasks: []PipelineTask{{ + Name: "invalid-pipeline-task", + TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ + Steps: []Step{{ + Name: "some-step", + Image: "some-image", + Script: "$(params.doesnotexist)", + }}, + }}, + }}, + }, + }, + expectedError: apis.FieldError{ + Message: `non-existent variable in "$(params.doesnotexist)"`, + Paths: []string{"spec.tasks[0].steps[0].script"}, + }, + }, { + name: "invalid parameter usage in finally pipeline task", + p: &Pipeline{ + ObjectMeta: metav1.ObjectMeta{Name: "pipeline"}, + Spec: PipelineSpec{ + Tasks: []PipelineTask{{ + Name: "pipeline-task", + TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ + Steps: []Step{{ + Name: "some-step", + Image: "some-image", + Command: []string{"cmd"}, + }}, + }}, + }}, + Finally: []PipelineTask{{ + Name: "invalid-pipeline-task", + TaskSpec: &EmbeddedTask{TaskSpec: TaskSpec{ + Steps: []Step{{ + Name: "some-step", + Image: "some-image", + Script: "$(params.doesnotexist)", + }}, + }}, + }}, + }, + }, + expectedError: apis.FieldError{ + Message: `non-existent variable in "$(params.doesnotexist)"`, + Paths: []string{"spec.finally[0].steps[0].script"}, + }, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {