diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation.go b/pkg/apis/pipeline/v1beta1/pipeline_validation.go index 38c7067e217..b8925d79e47 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation.go @@ -149,7 +149,8 @@ func validatePipelineParameterVariables(ctx context.Context, tasks []PipelineTas } } } - return errs.Also(validatePipelineParametersVariables(tasks, "params", parameterNames, arrayParameterNames, objectParameterNameKeys)) + errs = errs.Also(validatePipelineParametersVariables(tasks, "params", parameterNames, arrayParameterNames, objectParameterNameKeys)) + return errs } func validatePipelineParametersVariables(tasks []PipelineTask, prefix string, paramNames sets.String, arrayParamNames sets.String, objectParamNameKeys map[string][]string) (errs *apis.FieldError) { diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go index 2a157db221d..aa791f469d5 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go @@ -1590,6 +1590,7 @@ func TestValidatePipelineParameterVariables_Failure(t *testing.T) { params []ParamSpec tasks []PipelineTask expectedError apis.FieldError + api string }{{ name: "invalid pipeline task with a parameter which is missing from the param declarations", tasks: []PipelineTask{{ @@ -1921,6 +1922,7 @@ func TestValidatePipelineParameterVariables_Failure(t *testing.T) { Message: `non-existent variable in "$(params.myObject.non-exist-key)"`, Paths: []string{"[0].when[0].input"}, }, + api: "alpha", }, { name: "invalid object key in the Values of the when expression", params: []ParamSpec{{ @@ -1944,6 +1946,7 @@ func TestValidatePipelineParameterVariables_Failure(t *testing.T) { Message: `non-existent variable in "$(params.myObject.non-exist-key)"`, Paths: []string{"[0].when[0].values"}, }, + api: "alpha", }, { name: "invalid object key is used to provide values for array params", params: []ParamSpec{{ @@ -1965,6 +1968,7 @@ func TestValidatePipelineParameterVariables_Failure(t *testing.T) { Message: `non-existent variable in "$(params.myObject.non-exist-key)"`, Paths: []string{"[0].params[a-param].value[0]"}, }, + api: "alpha", }, { name: "invalid object key is used to provide values for string params", params: []ParamSpec{{ @@ -1986,6 +1990,7 @@ func TestValidatePipelineParameterVariables_Failure(t *testing.T) { Message: `non-existent variable in "$(params.myObject.non-exist-key)"`, Paths: []string{"[0].params[a-param]"}, }, + api: "alpha", }, { name: "invalid object key is used to provide values for object params", params: []ParamSpec{{ @@ -2013,6 +2018,7 @@ func TestValidatePipelineParameterVariables_Failure(t *testing.T) { Message: `non-existent variable in "$(params.myObject.non-exist-key)"`, Paths: []string{"[0].params[an-object-param].properties[url]"}, }, + api: "alpha", }, { name: "invalid object key is used to provide values for matrix params", params: []ParamSpec{{ @@ -2036,10 +2042,14 @@ func TestValidatePipelineParameterVariables_Failure(t *testing.T) { Message: `non-existent variable in "$(params.myObject.non-exist-key)"`, Paths: []string{"[0].matrix[b-param].value[0]"}, }, + api: "alpha", }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ctx := config.EnableAlphaAPIFields(context.Background()) + ctx := context.Background() + if tt.api == "alpha" { + ctx = config.EnableAlphaAPIFields(context.Background()) + } err := validatePipelineParameterVariables(ctx, tt.tasks, tt.params) if err == nil { t.Errorf("Pipeline.validatePipelineParameterVariables() did not return error for invalid pipeline parameters") diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go b/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go index 334ffa1a3c2..8d93369e727 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go @@ -107,7 +107,7 @@ func (ps *PipelineRunSpec) Validate(ctx context.Context) (errs *apis.FieldError) wsNames[ws.Name] = idx } } - + errs = errs.Also(validateParameters(ctx, ps)) for idx, trs := range ps.TaskRunSpecs { errs = errs.Also(validateTaskRunSpec(ctx, trs).ViaIndex(idx).ViaField("taskRunSpecs")) } @@ -115,6 +115,70 @@ func (ps *PipelineRunSpec) Validate(ctx context.Context) (errs *apis.FieldError) return errs } +func validateParameters(ctx context.Context, ps *PipelineRunSpec) (errs *apis.FieldError) { + var paramSpec []ParamSpec + if ps.Params == nil || ps.PipelineSpec == nil { + return errs + } + for _, p := range ps.Params { + pSpec := ParamSpec{ + Name: p.Name, + Default: &p.Value, + } + paramSpec = append(paramSpec, pSpec) + } + for _, p := range ps.PipelineSpec.Params { + skip := false + for _, ps := range paramSpec { + if ps.Name == p.Name { + skip = true + break + } + } + if !skip { + paramSpec = append(paramSpec, p) + } + } + for _, pt := range ps.PipelineSpec.Tasks { + for _, p := range pt.Params { + skip := false + for _, ps := range paramSpec { + if ps.Name == p.Name { + skip = true + break + } + } + if !skip { + pSpec := ParamSpec{ + Name: p.Name, + Default: &p.Value, + } + paramSpec = append(paramSpec, pSpec) + } + } + for _, p := range pt.TaskSpec.Params { + skip := false + for _, ps := range paramSpec { + if ps.Name == p.Name { + skip = true + break + } + } + if !skip { + paramSpec = append(paramSpec, p) + } + } + } + if ps.PipelineSpec != nil && ps.PipelineSpec.Tasks != nil { + for _, pt := range ps.PipelineSpec.Tasks { + if pt.TaskSpec != nil && pt.TaskSpec.Steps != nil { + errs = errs.Also(ValidateParameterVariables(ctx, pt.TaskSpec.Steps, paramSpec, false)) + } + } + } + return errs +} + func validateSpecStatus(status PipelineRunSpecStatus) *apis.FieldError { switch status { case "": diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go b/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go index d6eb2dcbd4f..cc13fce8627 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_validation_test.go @@ -92,6 +92,38 @@ func TestPipelineRun_Invalid(t *testing.T) { }, }, want: apis.ErrInvalidValue("PipelineRunCancell should be Cancelled, CancelledRunFinally, StoppedRunFinally or PipelineRunPending", "spec.status"), + }, { + name: "propagating params with pipelinespec and taskspec", + pr: v1beta1.PipelineRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "pipelinelinename", + }, + Spec: v1beta1.PipelineRunSpec{ + Params: []v1beta1.Param{{ + Name: "pipeline-words", + Value: v1beta1.ArrayOrString{ + ArrayVal: []string{"hello", "pipeline"}, + }, + }}, + PipelineSpec: &v1beta1.PipelineSpec{ + Tasks: []v1beta1.PipelineTask{{ + Name: "echoit", + TaskSpec: &v1beta1.EmbeddedTask{TaskSpec: v1beta1.TaskSpec{ + Steps: []v1beta1.Step{{ + Name: "echo", + Image: "ubuntu", + Command: []string{"echo"}, + Args: []string{"$(params.task-words[*])"}, + }}, + }}, + }}, + }, + }, + }, + want: &apis.FieldError{ + Message: `non-existent variable in "$(params.task-words[*])"`, + Paths: []string{"spec.pipelineSpec.tasks[0].taskSpec.steps[0].args[0], spec.steps[0].args[0]"}, + }, }, { name: "pipelinerun pending while running", pr: v1beta1.PipelineRun{ diff --git a/pkg/apis/pipeline/v1beta1/task_validation.go b/pkg/apis/pipeline/v1beta1/task_validation.go index 58647f6c2cb..9f6c7a106d6 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation.go +++ b/pkg/apis/pipeline/v1beta1/task_validation.go @@ -88,7 +88,7 @@ func (ts *TaskSpec) Validate(ctx context.Context) (errs *apis.FieldError) { errs = errs.Also(validateSteps(ctx, mergedSteps).ViaField("steps")) errs = errs.Also(ts.Resources.Validate(ctx).ViaField("resources")) errs = errs.Also(ValidateParameterTypes(ctx, ts.Params).ViaField("params")) - errs = errs.Also(ValidateParameterVariables(ctx, ts.Steps, ts.Params)) + errs = errs.Also(ValidateParameterVariables(ctx, ts.Steps, ts.Params, true)) errs = errs.Also(ValidateResourcesVariables(ctx, ts.Steps, ts.Resources)) errs = errs.Also(validateTaskContextVariables(ctx, ts.Steps)) errs = errs.Also(validateResults(ctx, ts.Results).ViaField("results")) @@ -329,7 +329,7 @@ func (p ParamSpec) ValidateObjectType() *apis.FieldError { } // ValidateParameterVariables validates all variables within a slice of ParamSpecs against a slice of Steps -func ValidateParameterVariables(ctx context.Context, steps []Step, params []ParamSpec) *apis.FieldError { +func ValidateParameterVariables(ctx context.Context, steps []Step, params []ParamSpec, skipVariableValidation bool) *apis.FieldError { allParameterNames := sets.NewString() stringParameterNames := sets.NewString() arrayParameterNames := sets.NewString() @@ -351,9 +351,10 @@ func ValidateParameterVariables(ctx context.Context, steps []Step, params []Para stringParameterNames.Insert(p.Name) } } - errs = errs.Also(validateNameFormat(stringParameterNames.Insert(arrayParameterNames.List()...), objectParamSpecs)) - errs = errs.Also(validateVariables(ctx, steps, "params", allParameterNames)) + if !(skipVariableValidation && config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields == "alpha") { + errs = errs.Also(validateVariables(ctx, steps, "params", allParameterNames)) + } errs = errs.Also(validateArrayUsage(steps, "params", arrayParameterNames)) errs = errs.Also(validateObjectDefault(objectParamSpecs)) return errs.Also(validateObjectUsage(ctx, steps, objectParamSpecs)) @@ -574,9 +575,7 @@ func validateStepVariables(ctx context.Context, step Step, prefix string, vars s errs := validateTaskVariable(step.Name, prefix, vars).ViaField("name") errs = errs.Also(validateTaskVariable(step.Image, prefix, vars).ViaField("image")) errs = errs.Also(validateTaskVariable(step.WorkingDir, prefix, vars).ViaField("workingDir")) - if !(config.FromContextOrDefaults(ctx).FeatureFlags.EnableAPIFields == "alpha" && prefix == "params") { - errs = errs.Also(validateTaskVariable(step.Script, prefix, vars).ViaField("script")) - } + errs = errs.Also(validateTaskVariable(step.Script, prefix, vars).ViaField("script")) for i, cmd := range step.Command { errs = errs.Also(validateTaskVariable(cmd, prefix, vars).ViaFieldIndex("command", i)) } diff --git a/pkg/apis/pipeline/v1beta1/task_validation_test.go b/pkg/apis/pipeline/v1beta1/task_validation_test.go index a0b62cf0371..9f11fd9a187 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/task_validation_test.go @@ -250,6 +250,26 @@ func TestTaskSpecValidate(t *testing.T) { Image: "some-image", }, }, + }, { + name: "propagating params valid step with script", + fields: fields{ + Steps: []v1beta1.Step{{ + Image: "my-image", + Script: ` + #!/usr/bin/env bash + $(params.message)`, + }}, + }, + }, { + name: "propagating params valid step with args", + fields: fields{ + Steps: []v1beta1.Step{{ + Name: "mystep", + Image: "my-image", + Command: []string{"$(params.command)"}, + Args: []string{"$(params.message)"}, + }}, + }, }, { name: "valid step with script", fields: fields{ @@ -450,6 +470,7 @@ func TestTaskSpecValidateError(t *testing.T) { name string fields fields expectedError apis.FieldError + api string }{{ name: "empty spec", expectedError: apis.FieldError{ @@ -593,6 +614,7 @@ func TestTaskSpecValidateError(t *testing.T) { Paths: []string{"params"}, Details: "Object Names: \nMust only contain alphanumeric characters, hyphens (-), underscores (_) \nMust begin with a letter or an underscore (_)", }, + api: "alpha", }, { name: "duplicated param names", fields: fields{ @@ -697,6 +719,7 @@ func TestTaskSpecValidateError(t *testing.T) { Message: `"object" type does not match default value's type: "string"`, Paths: []string{"params.task.type", "params.task.default.type"}, }, + api: "alpha", }, { name: "the spec of object type parameter misses the definition of properties", fields: fields{ @@ -708,6 +731,7 @@ func TestTaskSpecValidateError(t *testing.T) { Steps: validSteps, }, expectedError: *apis.ErrMissingField(fmt.Sprintf("params.task.properties")), + api: "alpha", }, { name: "PropertySpec type is set with unsupported type", fields: fields{ @@ -726,6 +750,7 @@ func TestTaskSpecValidateError(t *testing.T) { Message: fmt.Sprintf("The value type specified for these keys %v is invalid", []string{"key1"}), Paths: []string{"params.task.properties"}, }, + api: "alpha", }, { name: "keys defined in properties are missed in default", fields: fields{ @@ -747,6 +772,7 @@ func TestTaskSpecValidateError(t *testing.T) { Message: fmt.Sprintf("Required key(s) %s are missing in the value provider.", []string{"key2"}), Paths: []string{"myobjectParam.properties", "myobjectParam.default"}, }, + api: "alpha", }, { name: "invalid step", fields: fields{ @@ -961,6 +987,7 @@ func TestTaskSpecValidateError(t *testing.T) { Message: `variable type invalid in "$(params.gitrepo)"`, Paths: []string{"steps[0].image"}, }, + api: "alpha", }, { name: "object star used in a string field", fields: fields{ @@ -983,6 +1010,7 @@ func TestTaskSpecValidateError(t *testing.T) { Message: `variable type invalid in "$(params.gitrepo[*])"`, Paths: []string{"steps[0].image"}, }, + api: "alpha", }, { name: "object used in a field that can accept array type", fields: fields{ @@ -1005,6 +1033,7 @@ func TestTaskSpecValidateError(t *testing.T) { Message: `variable type invalid in "$(params.gitrepo)"`, Paths: []string{"steps[0].args[0]"}, }, + api: "alpha", }, { name: "object star used in a field that can accept array type", fields: fields{ @@ -1027,6 +1056,7 @@ func TestTaskSpecValidateError(t *testing.T) { Message: `variable type invalid in "$(params.gitrepo[*])"`, Paths: []string{"steps[0].args[0]"}, }, + api: "alpha", }, { name: "Inexistent param variable in volumeMount with existing", fields: fields{ @@ -1299,7 +1329,10 @@ func TestTaskSpecValidateError(t *testing.T) { Workspaces: tt.fields.Workspaces, Results: tt.fields.Results, } - ctx := config.EnableAlphaAPIFields(context.Background()) + ctx := context.Background() + if tt.api == "alpha" { + ctx = config.EnableAlphaAPIFields(context.Background()) + } ts.SetDefaults(ctx) err := ts.Validate(ctx) if err == nil { diff --git a/pkg/apis/pipeline/v1beta1/taskrun_validation.go b/pkg/apis/pipeline/v1beta1/taskrun_validation.go index 2eed5c9e609..b9504daa306 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_validation.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_validation.go @@ -59,6 +59,7 @@ func (ts *TaskRunSpec) Validate(ctx context.Context) (errs *apis.FieldError) { } errs = errs.Also(ValidateParameters(ctx, ts.Params).ViaField("params")) + errs = errs.Also(validateInlineParameters(ctx, ts).ViaField("params")) errs = errs.Also(ValidateWorkspaceBindings(ctx, ts.Workspaces).ViaField("workspaces")) errs = errs.Also(ts.Resources.Validate(ctx).ViaField("resources")) if ts.Debug != nil { @@ -118,6 +119,37 @@ func ValidateWorkspaceBindings(ctx context.Context, wb []WorkspaceBinding) (errs return errs } +func validateInlineParameters(ctx context.Context, ts *TaskRunSpec) (errs *apis.FieldError) { + params := ts.Params + var paramSpec []ParamSpec + if ts.Params == nil || ts.TaskSpec == nil { + return errs + } + for _, p := range params { + pSpec := ParamSpec{ + Name: p.Name, + Default: &p.Value, + } + paramSpec = append(paramSpec, pSpec) + } + for _, p := range ts.TaskSpec.Params { + skip := false + for _, ps := range paramSpec { + if ps.Name == p.Name { + skip = true + break + } + } + if !skip { + paramSpec = append(paramSpec, p) + } + } + if ts.TaskSpec != nil && ts.TaskSpec.Steps != nil { + errs = errs.Also(ValidateParameterVariables(ctx, ts.TaskSpec.Steps, paramSpec, false)) + } + return errs +} + // ValidateParameters makes sure the params for the Task are valid. func ValidateParameters(ctx context.Context, params []Param) (errs *apis.FieldError) { var names []string diff --git a/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go b/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go index 94c2392e367..01cfc16ed65 100644 --- a/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/taskrun_validation_test.go @@ -64,6 +64,28 @@ func TestTaskRun_Validate(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "taskrname"}, }, wc: apis.WithinDelete, + }, { + name: "propagating params with taskrun", + taskRun: &v1beta1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{Name: "tr"}, + Spec: v1beta1.TaskRunSpec{ + Params: []v1beta1.Param{{ + Name: "task-words", + Value: v1beta1.ArrayOrString{ + ArrayVal: []string{"hello", "task run"}, + }, + }}, + TaskSpec: &v1beta1.TaskSpec{ + Steps: []v1beta1.Step{{ + Name: "echo", + Image: "ubuntu", + Command: []string{"echo"}, + Args: []string{"$(params.task-words[*])"}, + }}, + }, + }, + }, + wc: config.EnableAlphaAPIFields, }, { name: "alpha feature: valid step and sidecar overrides", taskRun: &v1beta1.TaskRun{