diff --git a/pkg/apis/pipeline/v1alpha1/param_types.go b/pkg/apis/pipeline/v1alpha1/param_types.go index 0c30fffd52e..cb18f1e108d 100644 --- a/pkg/apis/pipeline/v1alpha1/param_types.go +++ b/pkg/apis/pipeline/v1alpha1/param_types.go @@ -16,12 +16,24 @@ limitations under the License. package v1alpha1 +import ( + "context" + "encoding/json" + "fmt" + + "github.com/tektoncd/pipeline/pkg/templating" +) + // ParamSpec defines arbitrary parameters needed beyond typed inputs (such as // resources). Parameter values are provided by users as inputs on a TaskRun // or PipelineRun. type ParamSpec struct { // Name declares the name by which a parameter is referenced. Name string `json:"name"` + // Type is the user-specified type of the parameter. The possible types + // are currently "string" and "array", and "string" is the default. + // +optional + Type ParamType `json:"type,omitempty"` // Description is a user-facing description of the parameter that may be // used to populate a UI. // +optional @@ -30,11 +42,88 @@ type ParamSpec struct { // default is set, a Task may be executed without a supplied value for the // parameter. // +optional - Default string `json:"default,omitempty"` + Default *ArrayOrString `json:"default,omitempty"` +} + +func (pp *ParamSpec) SetDefaults(ctx context.Context) { + if pp != nil && pp.Type == "" { + if pp.Default != nil { + // propagate the parsed ArrayOrString's type to the parent ParamSpec's type + pp.Type = pp.Default.Type + } else { + // ParamTypeString is the default value (when no type can be inferred from the default value) + pp.Type = ParamTypeString + } + } } -// Param declares a value to use for the Param called Name. +// Param declares a value to use for the Param called Name, and is used in the +// specific context of PipelineResources. type Param struct { Name string `json:"name"` Value string `json:"value"` } + +// ArrayOrStringParam declares an ArrayOrString to use for the parameter called name. +type ArrayOrStringParam struct { + Name string `json:"name"` + Value ArrayOrString `json:"value"` +} + +// ParamType indicates the type of an input parameter; +// Used to distinguish between a single string and an array of strings. +type ParamType string + +// Valid ParamTypes: +const ( + ParamTypeString ParamType = "string" + ParamTypeArray ParamType = "array" +) + +// AllParamTypes can be used for ParamType validation. +var AllParamTypes = []ParamType{ParamTypeString, ParamTypeArray} + +// ArrayOrString is modeled after IntOrString in kubernetes/apimachinery: + +// ArrayOrString is a type that can hold a single string or string array. +// Used in JSON unmarshalling so that a single JSON field can accept +// either an individual string or an array of strings. +type ArrayOrString struct { + Type ParamType // Represents the stored type of ArrayOrString. + StringVal string + ArrayVal []string +} + +// UnmarshalJSON implements the json.Unmarshaller interface. +func (arrayOrString *ArrayOrString) UnmarshalJSON(value []byte) error { + if value[0] == '"' { + arrayOrString.Type = ParamTypeString + return json.Unmarshal(value, &arrayOrString.StringVal) + } + arrayOrString.Type = ParamTypeArray + return json.Unmarshal(value, &arrayOrString.ArrayVal) +} + +// MarshalJSON implements the json.Marshaller interface. +func (arrayOrString ArrayOrString) MarshalJSON() ([]byte, error) { + switch arrayOrString.Type { + case ParamTypeString: + return json.Marshal(arrayOrString.StringVal) + case ParamTypeArray: + return json.Marshal(arrayOrString.ArrayVal) + default: + return []byte{}, fmt.Errorf("impossible ArrayOrString.Type: %q", arrayOrString.Type) + } +} + +func (arrayOrString *ArrayOrString) ApplyReplacements(stringReplacements map[string]string, arrayReplacements map[string][]string) { + if arrayOrString.Type == ParamTypeString { + arrayOrString.StringVal = templating.ApplyReplacements(arrayOrString.StringVal, stringReplacements) + } else { + var newArrayVal []string + for _, v := range arrayOrString.ArrayVal { + newArrayVal = append(newArrayVal, templating.ApplyArrayReplacements(v, stringReplacements, arrayReplacements)...) + } + arrayOrString.ArrayVal = newArrayVal + } +} diff --git a/pkg/apis/pipeline/v1alpha1/param_types_test.go b/pkg/apis/pipeline/v1alpha1/param_types_test.go new file mode 100644 index 00000000000..9afda576430 --- /dev/null +++ b/pkg/apis/pipeline/v1alpha1/param_types_test.go @@ -0,0 +1,189 @@ +/* +Copyright 2019 The Tekton Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha1_test + +import ( + "context" + "encoding/json" + "reflect" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" + "github.com/tektoncd/pipeline/test/builder" +) + +func TestParamSpec_SetDefaults(t *testing.T) { + tests := []struct { + name string + before *v1alpha1.ParamSpec + defaultsApplied *v1alpha1.ParamSpec + }{{ + name: "inferred string type", + before: &v1alpha1.ParamSpec{ + Name: "parametername", + }, + defaultsApplied: &v1alpha1.ParamSpec{ + Name: "parametername", + Type: v1alpha1.ParamTypeString, + }, + }, { + name: "inferred type from default value", + before: &v1alpha1.ParamSpec{ + Name: "parametername", + Default: builder.ArrayOrString("an", "array"), + }, + defaultsApplied: &v1alpha1.ParamSpec{ + Name: "parametername", + Type: v1alpha1.ParamTypeArray, + Default: builder.ArrayOrString("an", "array"), + }, + }, { + name: "fully defined ParamSpec", + before: &v1alpha1.ParamSpec{ + Name: "parametername", + Type: v1alpha1.ParamTypeArray, + Description: "a description", + Default: builder.ArrayOrString("an", "array"), + }, + defaultsApplied: &v1alpha1.ParamSpec{ + Name: "parametername", + Type: v1alpha1.ParamTypeArray, + Description: "a description", + Default: builder.ArrayOrString("an", "array"), + }, + }} + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() + tc.before.SetDefaults(ctx) + if d := cmp.Diff(tc.before, tc.defaultsApplied); d != "" { + t.Errorf("ParamSpec.SetDefaults/%s (-want, +got) = %v", tc.name, d) + } + }) + } +} + +func TestArrayOrString_ApplyReplacements(t *testing.T) { + type args struct { + input *v1alpha1.ArrayOrString + stringReplacements map[string]string + arrayReplacements map[string][]string + } + tests := []struct { + name string + args args + expectedOutput *v1alpha1.ArrayOrString + }{{ + name: "no replacements on array", + args: args{ + input: builder.ArrayOrString("an", "array"), + stringReplacements: map[string]string{"some": "value", "anotherkey": "value"}, + arrayReplacements: map[string][]string{"arraykey": {"array", "value"}, "sdfdf": {"sdf", "sdfsd"}}, + }, + expectedOutput: builder.ArrayOrString("an", "array"), + }, { + name: "string replacements on string", + args: args{ + input: builder.ArrayOrString("astring${some} asdf ${anotherkey}"), + stringReplacements: map[string]string{"some": "value", "anotherkey": "value"}, + arrayReplacements: map[string][]string{"arraykey": {"array", "value"}, "sdfdf": {"asdf", "sdfsd"}}, + }, + expectedOutput: builder.ArrayOrString("astringvalue asdf value"), + }, { + name: "single array replacement", + args: args{ + input: builder.ArrayOrString("firstvalue", "${arraykey}", "lastvalue"), + stringReplacements: map[string]string{"some": "value", "anotherkey": "value"}, + arrayReplacements: map[string][]string{"arraykey": {"array", "value"}, "sdfdf": {"asdf", "sdfsd"}}, + }, + expectedOutput: builder.ArrayOrString("firstvalue", "array", "value", "lastvalue"), + }, { + name: "multiple array replacement", + args: args{ + input: builder.ArrayOrString("firstvalue", "${arraykey}", "lastvalue", "${sdfdf}"), + stringReplacements: map[string]string{"some": "value", "anotherkey": "value"}, + arrayReplacements: map[string][]string{"arraykey": {"array", "value"}, "sdfdf": {"asdf", "sdfsd"}}, + }, + expectedOutput: builder.ArrayOrString("firstvalue", "array", "value", "lastvalue", "asdf", "sdfsd"), + }, { + name: "empty array replacement", + args: args{ + input: builder.ArrayOrString("firstvalue", "${arraykey}", "lastvalue"), + stringReplacements: map[string]string{"some": "value", "anotherkey": "value"}, + arrayReplacements: map[string][]string{"arraykey": {}}, + }, + expectedOutput: builder.ArrayOrString("firstvalue", "lastvalue"), + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + tt.args.input.ApplyReplacements(tt.args.stringReplacements, tt.args.arrayReplacements) + if d := cmp.Diff(tt.expectedOutput, tt.args.input); d != "" { + t.Errorf("ApplyReplacements() output did not match expected value %s", d) + } + }) + } +} + +type ArrayOrStringHolder struct { + AOrS v1alpha1.ArrayOrString `json:"val"` +} + +func TestArrayOrString_UnmarshalJSON(t *testing.T) { + cases := []struct { + input string + result v1alpha1.ArrayOrString + }{ + {"{\"val\": \"123\"}", *builder.ArrayOrString("123")}, + {"{\"val\": \"\"}", *builder.ArrayOrString("")}, + {"{\"val\":[]}", v1alpha1.ArrayOrString{Type: v1alpha1.ParamTypeArray, ArrayVal: []string{}}}, + {"{\"val\":[\"oneelement\"]}", v1alpha1.ArrayOrString{Type: v1alpha1.ParamTypeArray, ArrayVal: []string{"oneelement"}}}, + {"{\"val\":[\"multiple\", \"elements\"]}", v1alpha1.ArrayOrString{Type: v1alpha1.ParamTypeArray, ArrayVal: []string{"multiple", "elements"}}}, + } + + for _, c := range cases { + var result ArrayOrStringHolder + if err := json.Unmarshal([]byte(c.input), &result); err != nil { + t.Errorf("Failed to unmarshal input '%v': %v", c.input, err) + } + if !reflect.DeepEqual(result.AOrS, c.result) { + t.Errorf("Failed to unmarshal input '%v': expected %+v, got %+v", c.input, c.result, result) + } + } +} + +func TestArrayOrString_MarshalJSON(t *testing.T) { + cases := []struct { + input v1alpha1.ArrayOrString + result string + }{ + {*builder.ArrayOrString("123"), "{\"val\":\"123\"}"}, + {*builder.ArrayOrString("123", "1234"), "{\"val\":[\"123\",\"1234\"]}"}, + {*builder.ArrayOrString("a", "a", "a"), "{\"val\":[\"a\",\"a\",\"a\"]}"}, + } + + for _, c := range cases { + input := ArrayOrStringHolder{c.input} + result, err := json.Marshal(&input) + if err != nil { + t.Errorf("Failed to marshal input '%v': %v", input, err) + } + if string(result) != c.result { + t.Errorf("Failed to marshal input '%v': expected: %+v, got %q", input, c.result, string(result)) + } + } +} diff --git a/pkg/apis/pipeline/v1alpha1/pipeline_defaults.go b/pkg/apis/pipeline/v1alpha1/pipeline_defaults.go index 1eb539b91f4..104fc508c93 100644 --- a/pkg/apis/pipeline/v1alpha1/pipeline_defaults.go +++ b/pkg/apis/pipeline/v1alpha1/pipeline_defaults.go @@ -28,4 +28,7 @@ func (ps *PipelineSpec) SetDefaults(ctx context.Context) { pt.TaskRef.Kind = NamespacedTaskKind } } + for i := range ps.Params { + ps.Params[i].SetDefaults(ctx) + } } diff --git a/pkg/apis/pipeline/v1alpha1/pipeline_types.go b/pkg/apis/pipeline/v1alpha1/pipeline_types.go index 59d1f83cd3a..add3c8ba79a 100644 --- a/pkg/apis/pipeline/v1alpha1/pipeline_types.go +++ b/pkg/apis/pipeline/v1alpha1/pipeline_types.go @@ -98,7 +98,7 @@ type PipelineTask struct { Resources *PipelineTaskResources `json:"resources,omitempty"` // Parameters declares parameters passed to this task. // +optional - Params []Param `json:"params,omitempty"` + Params []ArrayOrStringParam `json:"params,omitempty"` } // PipelineTaskParam is used to provide arbitrary string parameters to a Task. diff --git a/pkg/apis/pipeline/v1alpha1/pipeline_validation.go b/pkg/apis/pipeline/v1alpha1/pipeline_validation.go index 2f9ad98b821..175b10a72d5 100644 --- a/pkg/apis/pipeline/v1alpha1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1alpha1/pipeline_validation.go @@ -151,17 +151,61 @@ func (ps *PipelineSpec) Validate(ctx context.Context) *apis.FieldError { func validatePipelineParameterVariables(tasks []PipelineTask, params []ParamSpec) *apis.FieldError { parameterNames := map[string]struct{}{} + arrayParameterNames := map[string]struct{}{} + for _, p := range params { + // Verify that p is a valid type. + validType := false + for _, allowedType := range AllParamTypes { + if p.Type == allowedType { + validType = true + } + } + if !validType { + return apis.ErrInvalidValue(string(p.Type), fmt.Sprintf("spec.params.%s.type", p.Name)) + } + + // If a default value is provided, ensure its type matches param's declared type. + if (p.Default != nil) && (p.Default.Type != p.Type) { + return &apis.FieldError{ + Message: fmt.Sprintf( + "\"%v\" type does not match default value's type: \"%v\"", p.Type, p.Default.Type), + Paths: []string{ + fmt.Sprintf("spec.params.%s.type", p.Name), + fmt.Sprintf("spec.params.%s.default.type", p.Name), + }, + } + } + + // Add parameter name to parameterNames, and to arrayParameterNames if type is array. parameterNames[p.Name] = struct{}{} + if p.Type == ParamTypeArray { + arrayParameterNames[p.Name] = struct{}{} + } } - return validatePipelineVariables(tasks, "params", parameterNames) + + return validatePipelineVariables(tasks, "params", parameterNames, arrayParameterNames) } -func validatePipelineVariables(tasks []PipelineTask, prefix string, vars map[string]struct{}) *apis.FieldError { +func validatePipelineVariables(tasks []PipelineTask, prefix string, paramNames map[string]struct{}, arrayParamNames map[string]struct{}) *apis.FieldError { for _, task := range tasks { for _, param := range task.Params { - if err := validatePipelineVariable(fmt.Sprintf("param[%s]", param.Name), param.Value, prefix, vars); err != nil { - return err + if param.Value.Type == ParamTypeString { + if err := validatePipelineVariable(fmt.Sprintf("param[%s]", param.Name), param.Value.StringVal, prefix, paramNames); err != nil { + return err + } + if err := validatePipelineNoArrayReferenced(fmt.Sprintf("param[%s]", param.Name), param.Value.StringVal, prefix, arrayParamNames); err != nil { + return err + } + } else { + for _, arrayElement := range param.Value.ArrayVal { + if err := validatePipelineVariable(fmt.Sprintf("param[%s]", param.Name), arrayElement, prefix, paramNames); err != nil { + return err + } + if err := validatePipelineArraysIsolated(fmt.Sprintf("param[%s]", param.Name), arrayElement, prefix, arrayParamNames); err != nil { + return err + } + } } } } @@ -171,3 +215,11 @@ func validatePipelineVariables(tasks []PipelineTask, prefix string, vars map[str func validatePipelineVariable(name, value, prefix string, vars map[string]struct{}) *apis.FieldError { return templating.ValidateVariable(name, value, prefix, "", "task parameter", "pipelinespec.params", vars) } + +func validatePipelineNoArrayReferenced(name, value, prefix string, vars map[string]struct{}) *apis.FieldError { + return templating.ValidateVariableProhibited(name, value, prefix, "", "task parameter", "pipelinespec.params", vars) +} + +func validatePipelineArraysIsolated(name, value, prefix string, vars map[string]struct{}) *apis.FieldError { + return templating.ValidateVariableIsolated(name, value, prefix, "", "task parameter", "pipelinespec.params", vars) +} diff --git a/pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go b/pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go index 012c1c0c8b8..b0f4e1d5839 100644 --- a/pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go +++ b/pkg/apis/pipeline/v1alpha1/pipeline_validation_test.go @@ -118,16 +118,25 @@ func TestPipelineSpec_Validate(t *testing.T) { }, { name: "valid parameter variables", p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec( - tb.PipelineParam("baz"), - tb.PipelineParam("foo-is-baz"), + tb.PipelineParam("baz", v1alpha1.ParamTypeString), + tb.PipelineParam("foo-is-baz", v1alpha1.ParamTypeString), tb.PipelineTask("bar", "bar-task", tb.PipelineTaskParam("a-param", "${baz} and ${foo-is-baz}")), )), failureExpected: false, + }, { + name: "valid array parameter variables", + p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec( + tb.PipelineParam("baz", v1alpha1.ParamTypeArray, tb.PipelineParamDefault("some", "default")), + tb.PipelineParam("foo-is-baz", v1alpha1.ParamTypeArray), + tb.PipelineTask("bar", "bar-task", + tb.PipelineTaskParam("a-param", "${baz}", "and", "${foo-is-baz}")), + )), + failureExpected: false, }, { name: "pipeline parameter nested in task parameter", p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec( - tb.PipelineParam("baz"), + tb.PipelineParam("baz", v1alpha1.ParamTypeString), tb.PipelineTask("bar", "bar-task", tb.PipelineTaskParam("a-param", "${input.workspace.${baz}}")), )), @@ -203,10 +212,51 @@ func TestPipelineSpec_Validate(t *testing.T) { }, { name: "not defined parameter variable with defined", p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec( - tb.PipelineParam("foo", tb.PipelineParamDefault("something")), + tb.PipelineParam("foo", v1alpha1.ParamTypeString, tb.PipelineParamDefault("something")), tb.PipelineTask("foo", "foo-task", tb.PipelineTaskParam("a-param", "${params.foo} and ${params.does-not-exist}")))), failureExpected: true, + }, { + name: "invalid parameter type", + p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec( + tb.PipelineParam("baz", "invalidtype", tb.PipelineParamDefault("some", "default")), + tb.PipelineParam("foo-is-baz", v1alpha1.ParamTypeArray), + tb.PipelineTask("bar", "bar-task", + tb.PipelineTaskParam("a-param", "${baz}", "and", "${foo-is-baz}")), + )), + failureExpected: true, + }, { + name: "array parameter mismatching default type", + p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec( + tb.PipelineParam("baz", v1alpha1.ParamTypeArray, tb.PipelineParamDefault("astring")), + tb.PipelineTask("bar", "bar-task", + tb.PipelineTaskParam("a-param", "arrayelement", "${baz}")), + )), + failureExpected: true, + }, { + name: "string parameter mismatching default type", + p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec( + tb.PipelineParam("baz", v1alpha1.ParamTypeString, tb.PipelineParamDefault("anarray", "elements")), + tb.PipelineTask("bar", "bar-task", + tb.PipelineTaskParam("a-param", "arrayelement", "${baz}")), + )), + failureExpected: true, + }, { + name: "array parameter used as string", + p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec( + tb.PipelineParam("baz", v1alpha1.ParamTypeArray, tb.PipelineParamDefault("anarray", "elements")), + tb.PipelineTask("bar", "bar-task", + tb.PipelineTaskParam("a-param", "${params.baz}")), + )), + failureExpected: true, + }, { + name: "array parameter string template not isolated", + p: tb.Pipeline("pipeline", "namespace", tb.PipelineSpec( + tb.PipelineParam("baz", v1alpha1.ParamTypeArray, tb.PipelineParamDefault("anarray", "elements")), + tb.PipelineTask("bar", "bar-task", + tb.PipelineTaskParam("a-param", "first", "value: ${params.baz}", "last")), + )), + failureExpected: true, }, { name: "invalid dependency graph between the tasks", p: tb.Pipeline("foo", "namespace", tb.PipelineSpec( diff --git a/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go b/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go index f59113cefaf..476af6c409f 100644 --- a/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go +++ b/pkg/apis/pipeline/v1alpha1/pipelinerun_types.go @@ -48,7 +48,7 @@ type PipelineRunSpec struct { // it needs. Resources []PipelineResourceBinding `json:"resources,omitempty"` // Params is a list of parameter names and values. - Params []Param `json:"params,omitempty"` + Params []ArrayOrStringParam `json:"params,omitempty"` // +optional ServiceAccount string `json:"serviceAccount"` // +optional diff --git a/pkg/apis/pipeline/v1alpha1/task_defaults.go b/pkg/apis/pipeline/v1alpha1/task_defaults.go index f8b2a3e9942..11f427a566c 100644 --- a/pkg/apis/pipeline/v1alpha1/task_defaults.go +++ b/pkg/apis/pipeline/v1alpha1/task_defaults.go @@ -36,4 +36,13 @@ func (ts *TaskSpec) SetDefaults(ctx context.Context) { } } } + if ts.Inputs != nil { + ts.Inputs.SetDefaults(ctx) + } +} + +func (inputs *Inputs) SetDefaults(ctx context.Context) { + for i := range inputs.Params { + inputs.Params[i].SetDefaults(ctx) + } } diff --git a/pkg/apis/pipeline/v1alpha1/task_validation.go b/pkg/apis/pipeline/v1alpha1/task_validation.go index 6935dcd12f0..a6fba5056fb 100644 --- a/pkg/apis/pipeline/v1alpha1/task_validation.go +++ b/pkg/apis/pipeline/v1alpha1/task_validation.go @@ -80,6 +80,9 @@ func (ts *TaskSpec) Validate(ctx context.Context) *apis.FieldError { if err := checkForDuplicates(ts.Inputs.Resources, "taskspec.Inputs.Resources.Name"); err != nil { return err } + if err := validateInputParameterTypes(ts.Inputs); err != nil { + return err + } } if ts.Outputs != nil { for _, resource := range ts.Outputs.Resources { @@ -143,14 +146,51 @@ func validateSteps(steps []corev1.Container) *apis.FieldError { return nil } +func validateInputParameterTypes(inputs *Inputs) *apis.FieldError { + for _, p := range inputs.Params { + // Ensure param has a valid type. + validType := false + for _, allowedType := range AllParamTypes { + if p.Type == allowedType { + validType = true + } + } + if !validType { + return apis.ErrInvalidValue(p.Type, fmt.Sprintf("taskspec.inputs.params.%s.type", p.Name)) + } + + // If a default value is provided, ensure its type matches param's declared type. + if (p.Default != nil) && (p.Default.Type != p.Type) { + return &apis.FieldError{ + Message: fmt.Sprintf( + "\"%v\" type does not match default value's type: \"%v\"", p.Type, p.Default.Type), + Paths: []string{ + fmt.Sprintf("taskspec.inputs.params.%s.type", p.Name), + fmt.Sprintf("taskspec.inputs.params.%s.default.type", p.Name), + }, + } + } + } + return nil +} + func validateInputParameterVariables(steps []corev1.Container, inputs *Inputs) *apis.FieldError { parameterNames := map[string]struct{}{} + arrayParameterNames := map[string]struct{}{} + if inputs != nil { for _, p := range inputs.Params { parameterNames[p.Name] = struct{}{} + if p.Type == ParamTypeArray { + arrayParameterNames[p.Name] = struct{}{} + } } } - return validateVariables(steps, "params", parameterNames) + + if err := validateVariables(steps, "params", parameterNames); err != nil { + return err + } + return validateArrayUsage(steps, "params", arrayParameterNames) } func validateResourceVariables(steps []corev1.Container, inputs *Inputs, outputs *Outputs) *apis.FieldError { @@ -173,6 +213,47 @@ func validateResourceVariables(steps []corev1.Container, inputs *Inputs, outputs return validateVariables(steps, "resources", resourceNames) } +func validateArrayUsage(steps []corev1.Container, prefix string, vars map[string]struct{}) *apis.FieldError { + for _, step := range steps { + if err := validateTaskNoArrayReferenced("name", step.Name, prefix, vars); err != nil { + return err + } + if err := validateTaskNoArrayReferenced("image", step.Image, prefix, vars); err != nil { + return err + } + if err := validateTaskNoArrayReferenced("workingDir", step.WorkingDir, prefix, vars); err != nil { + return err + } + for i, cmd := range step.Command { + if err := validateTaskArraysIsolated(fmt.Sprintf("command[%d]", i), cmd, prefix, vars); err != nil { + return err + } + } + for i, arg := range step.Args { + if err := validateTaskArraysIsolated(fmt.Sprintf("arg[%d]", i), arg, prefix, vars); err != nil { + return err + } + } + for _, env := range step.Env { + if err := validateTaskNoArrayReferenced(fmt.Sprintf("env[%s]", env.Name), env.Value, prefix, vars); err != nil { + return err + } + } + for i, v := range step.VolumeMounts { + if err := validateTaskNoArrayReferenced(fmt.Sprintf("volumeMount[%d].Name", i), v.Name, prefix, vars); err != nil { + return err + } + if err := validateTaskNoArrayReferenced(fmt.Sprintf("volumeMount[%d].MountPath", i), v.MountPath, prefix, vars); err != nil { + return err + } + if err := validateTaskNoArrayReferenced(fmt.Sprintf("volumeMount[%d].SubPath", i), v.SubPath, prefix, vars); err != nil { + return err + } + } + } + return nil +} + func validateVariables(steps []corev1.Container, prefix string, vars map[string]struct{}) *apis.FieldError { for _, step := range steps { if err := validateTaskVariable("name", step.Name, prefix, vars); err != nil { @@ -218,6 +299,14 @@ func validateTaskVariable(name, value, prefix string, vars map[string]struct{}) return templating.ValidateVariable(name, value, prefix, "(?:inputs|outputs).", "step", "taskspec.steps", vars) } +func validateTaskNoArrayReferenced(name, value, prefix string, arrayNames map[string]struct{}) *apis.FieldError { + return templating.ValidateVariableProhibited(name, value, prefix, "(?:inputs|outputs).", "step", "taskspec.steps", arrayNames) +} + +func validateTaskArraysIsolated(name, value, prefix string, arrayNames map[string]struct{}) *apis.FieldError { + return templating.ValidateVariableIsolated(name, value, prefix, "(?:inputs|outputs).", "step", "taskspec.steps", arrayNames) +} + func checkForDuplicates(resources []TaskResource, path string) *apis.FieldError { encountered := map[string]struct{}{} for _, r := range resources { diff --git a/pkg/apis/pipeline/v1alpha1/task_validation_test.go b/pkg/apis/pipeline/v1alpha1/task_validation_test.go index 3217f35d955..b46be60f403 100644 --- a/pkg/apis/pipeline/v1alpha1/task_validation_test.go +++ b/pkg/apis/pipeline/v1alpha1/task_validation_test.go @@ -20,6 +20,8 @@ import ( "context" "testing" + "github.com/tektoncd/pipeline/test/builder" + "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/knative/pkg/apis" @@ -68,15 +70,31 @@ func TestTaskSpecValidate(t *testing.T) { }}, }, }, { - name: "valid inputs", + name: "valid inputs (type implied)", + fields: fields{ + Inputs: &v1alpha1.Inputs{ + Resources: []v1alpha1.TaskResource{validResource}, + Params: []v1alpha1.ParamSpec{ + { + Name: "task", + Description: "param", + Default: builder.ArrayOrString("default"), + }, + }, + }, + BuildSteps: validBuildSteps, + }, + }, { + name: "valid inputs type explicit", fields: fields{ Inputs: &v1alpha1.Inputs{ Resources: []v1alpha1.TaskResource{validResource}, Params: []v1alpha1.ParamSpec{ { Name: "task", + Type: v1alpha1.ParamTypeString, Description: "param", - Default: "default", + Default: builder.ArrayOrString("default"), }, }, }, @@ -136,6 +154,33 @@ func TestTaskSpecValidate(t *testing.T) { WorkingDir: "/foo/bar/${outputs.resources.source}", }}, }, + }, { + name: "valid array template variable", + fields: fields{ + Inputs: &v1alpha1.Inputs{ + Resources: []v1alpha1.TaskResource{{ + Name: "foo", + Type: v1alpha1.PipelineResourceTypeImage, + }}, + Params: []v1alpha1.ParamSpec{{ + Name: "baz", + Type: v1alpha1.ParamTypeArray, + }, { + Name: "foo-is-baz", + Type: v1alpha1.ParamTypeArray, + }}, + }, + Outputs: &v1alpha1.Outputs{ + Resources: []v1alpha1.TaskResource{validResource}, + }, + BuildSteps: []corev1.Container{{ + Name: "mystep", + Image: "${inputs.resources.foo.url}", + Command: []string{"${inputs.param.foo-is-baz}"}, + Args: []string{"${inputs.params.baz}", "middle string", "${input.params.foo-is-baz}"}, + WorkingDir: "/foo/bar/${outputs.resources.source}", + }}, + }, }, { name: "step template included in validation", fields: fields{ @@ -247,6 +292,71 @@ func TestTaskSpecValidateError(t *testing.T) { Message: `invalid value: what`, Paths: []string{"taskspec.Inputs.Resources.source.Type"}, }, + }, { + name: "invalid input type", + fields: fields{ + Inputs: &v1alpha1.Inputs{ + Resources: []v1alpha1.TaskResource{validResource}, + Params: []v1alpha1.ParamSpec{ + { + Name: "validparam", + Type: v1alpha1.ParamTypeString, + Description: "parameter", + Default: builder.ArrayOrString("default"), + }, { + Name: "param-with-invalid-type", + Type: "invalidtype", + Description: "invalidtypedesc", + Default: builder.ArrayOrString("default"), + }, + }, + }, + BuildSteps: validBuildSteps, + }, + expectedError: apis.FieldError{ + Message: `invalid value: invalidtype`, + Paths: []string{"taskspec.inputs.params.param-with-invalid-type.type"}, + }, + }, { + name: "input mismatching default/type 1", + fields: fields{ + Inputs: &v1alpha1.Inputs{ + Resources: []v1alpha1.TaskResource{validResource}, + Params: []v1alpha1.ParamSpec{ + { + Name: "task", + Type: v1alpha1.ParamTypeArray, + Description: "param", + Default: builder.ArrayOrString("default"), + }, + }, + }, + BuildSteps: validBuildSteps, + }, + expectedError: apis.FieldError{ + Message: `"array" type does not match default value's type: "string"`, + Paths: []string{"taskspec.inputs.params.task.type", "taskspec.inputs.params.task.default.type"}, + }, + }, { + name: "input mismatching default/type 2", + fields: fields{ + Inputs: &v1alpha1.Inputs{ + Resources: []v1alpha1.TaskResource{validResource}, + Params: []v1alpha1.ParamSpec{ + { + Name: "task", + Type: v1alpha1.ParamTypeString, + Description: "param", + Default: builder.ArrayOrString("default", "array"), + }, + }, + }, + BuildSteps: validBuildSteps, + }, + expectedError: apis.FieldError{ + Message: `"string" type does not match default value's type: "array"`, + Paths: []string{"taskspec.inputs.params.task.type", "taskspec.inputs.params.task.default.type"}, + }, }, { name: "one invalid output", fields: fields{ @@ -348,6 +458,148 @@ func TestTaskSpecValidateError(t *testing.T) { Message: `non-existent variable in "--flag=${inputs.params.inexistent}" for step arg[0]`, Paths: []string{"taskspec.steps.arg[0]"}, }, + }, { + name: "array used in unaccepted field", + fields: fields{ + Inputs: &v1alpha1.Inputs{ + Resources: []v1alpha1.TaskResource{{ + Name: "foo", + Type: v1alpha1.PipelineResourceTypeImage, + }}, + Params: []v1alpha1.ParamSpec{{ + Name: "baz", + Type: v1alpha1.ParamTypeArray, + }, { + Name: "foo-is-baz", + Type: v1alpha1.ParamTypeArray, + }}, + }, + Outputs: &v1alpha1.Outputs{ + Resources: []v1alpha1.TaskResource{validResource}, + }, + BuildSteps: []corev1.Container{{ + Name: "mystep", + Image: "${inputs.params.baz}", + Command: []string{"${inputs.param.foo-is-baz}"}, + Args: []string{"${inputs.params.baz}", "middle string", "${input.resources.foo.url}"}, + WorkingDir: "/foo/bar/${outputs.resources.source}", + }}, + }, + expectedError: apis.FieldError{ + Message: `variable type invalid in "${inputs.params.baz}" for step image`, + Paths: []string{"taskspec.steps.image"}, + }, + }, { + name: "array not properly isolated", + fields: fields{ + Inputs: &v1alpha1.Inputs{ + Resources: []v1alpha1.TaskResource{{ + Name: "foo", + Type: v1alpha1.PipelineResourceTypeImage, + }}, + Params: []v1alpha1.ParamSpec{{ + Name: "baz", + Type: v1alpha1.ParamTypeArray, + }, { + Name: "foo-is-baz", + Type: v1alpha1.ParamTypeArray, + }}, + }, + Outputs: &v1alpha1.Outputs{ + Resources: []v1alpha1.TaskResource{validResource}, + }, + BuildSteps: []corev1.Container{{ + Name: "mystep", + Image: "someimage", + Command: []string{"${inputs.param.foo-is-baz}"}, + Args: []string{"not isolated: ${inputs.params.baz}", "middle string", "${input.resources.foo.url}"}, + WorkingDir: "/foo/bar/${outputs.resources.source}", + }}, + }, + expectedError: apis.FieldError{ + Message: `variable is not properly isolated in "not isolated: ${inputs.params.baz}" for step arg[0]`, + Paths: []string{"taskspec.steps.arg[0]"}, + }, + }, { + name: "array not properly isolated", + fields: fields{ + Inputs: &v1alpha1.Inputs{ + Resources: []v1alpha1.TaskResource{{ + Name: "foo", + Type: v1alpha1.PipelineResourceTypeImage, + }}, + Params: []v1alpha1.ParamSpec{{ + Name: "baz", + Type: v1alpha1.ParamTypeArray, + }, { + Name: "foo-is-baz", + Type: v1alpha1.ParamTypeArray, + }}, + }, + Outputs: &v1alpha1.Outputs{ + Resources: []v1alpha1.TaskResource{validResource}, + }, + BuildSteps: []corev1.Container{{ + Name: "mystep", + Image: "someimage", + Command: []string{"${inputs.param.foo-is-baz}"}, + Args: []string{"not isolated: ${inputs.params.baz}", "middle string", "${input.resources.foo.url}"}, + WorkingDir: "/foo/bar/${outputs.resources.source}", + }}, + }, + expectedError: apis.FieldError{ + Message: `variable is not properly isolated in "not isolated: ${inputs.params.baz}" for step arg[0]`, + Paths: []string{"taskspec.steps.arg[0]"}, + }, + }, { + name: "inexistent input resource variable", + fields: fields{ + BuildSteps: []corev1.Container{{ + Name: "mystep", + Image: "myimage:${inputs.resources.inputs}", + }}, + }, + expectedError: apis.FieldError{ + Message: `non-existent variable in "myimage:${inputs.resources.inputs}" for step image`, + Paths: []string{"taskspec.steps.image"}, + }, + }, { + name: "inferred array not properly isolated", + fields: fields{ + Inputs: &v1alpha1.Inputs{ + Resources: []v1alpha1.TaskResource{{ + Name: "foo", + Type: v1alpha1.PipelineResourceTypeImage, + }}, + Params: []v1alpha1.ParamSpec{{ + Name: "baz", + Default: &v1alpha1.ArrayOrString{ + Type: v1alpha1.ParamTypeArray, + ArrayVal: []string{"implied", "array", "type"}, + }, + }, { + Name: "foo-is-baz", + Default: &v1alpha1.ArrayOrString{ + Type: v1alpha1.ParamTypeArray, + ArrayVal: []string{"implied", "array", "type"}, + }, + }}, + }, + Outputs: &v1alpha1.Outputs{ + Resources: []v1alpha1.TaskResource{validResource}, + }, + BuildSteps: []corev1.Container{{ + Name: "mystep", + Image: "someimage", + Command: []string{"${inputs.param.foo-is-baz}"}, + Args: []string{"not isolated: ${inputs.params.baz}", "middle string", "${input.resources.foo.url}"}, + WorkingDir: "/foo/bar/${outputs.resources.source}", + }}, + }, + expectedError: apis.FieldError{ + Message: `variable is not properly isolated in "not isolated: ${inputs.params.baz}" for step arg[0]`, + Paths: []string{"taskspec.steps.arg[0]"}, + }, }, { name: "inexistent input resource variable", fields: fields{ @@ -381,7 +633,7 @@ func TestTaskSpecValidateError(t *testing.T) { { Name: "foo", Description: "param", - Default: "default", + Default: builder.ArrayOrString("default"), }, }, }, @@ -403,6 +655,8 @@ func TestTaskSpecValidateError(t *testing.T) { Outputs: tt.fields.Outputs, Steps: tt.fields.BuildSteps, } + ctx := context.Background() + ts.SetDefaults(ctx) err := ts.Validate(context.Background()) if err == nil { t.Fatalf("Expected an error, got nothing for %v", ts) diff --git a/pkg/apis/pipeline/v1alpha1/taskrun_types.go b/pkg/apis/pipeline/v1alpha1/taskrun_types.go index dc51212ff14..592607a31de 100644 --- a/pkg/apis/pipeline/v1alpha1/taskrun_types.go +++ b/pkg/apis/pipeline/v1alpha1/taskrun_types.go @@ -85,7 +85,7 @@ type TaskRunInputs struct { // +optional Resources []TaskResourceBinding `json:"resources,omitempty"` // +optional - Params []Param `json:"params,omitempty"` + Params []ArrayOrStringParam `json:"params,omitempty"` } // TaskResourceBinding points to the PipelineResource that diff --git a/pkg/apis/pipeline/v1alpha1/taskrun_validation.go b/pkg/apis/pipeline/v1alpha1/taskrun_validation.go index 438a76b9b9d..f47d42c695e 100644 --- a/pkg/apis/pipeline/v1alpha1/taskrun_validation.go +++ b/pkg/apis/pipeline/v1alpha1/taskrun_validation.go @@ -123,7 +123,7 @@ func validatePipelineResources(ctx context.Context, resources []TaskResourceBind return nil } -func validateParameters(params []Param) *apis.FieldError { +func validateParameters(params []ArrayOrStringParam) *apis.FieldError { // Template must not duplicate parameter names. seen := map[string]struct{}{} for _, p := range params { diff --git a/pkg/apis/pipeline/v1alpha1/taskrun_validation_test.go b/pkg/apis/pipeline/v1alpha1/taskrun_validation_test.go index e89e88897cb..cf1cdbc3548 100644 --- a/pkg/apis/pipeline/v1alpha1/taskrun_validation_test.go +++ b/pkg/apis/pipeline/v1alpha1/taskrun_validation_test.go @@ -21,6 +21,8 @@ import ( "testing" "time" + "github.com/tektoncd/pipeline/test/builder" + "github.com/google/go-cmp/cmp" "github.com/knative/pkg/apis" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" @@ -182,9 +184,9 @@ func TestTaskRunSpec_Validate(t *testing.T) { func TestInput_Validate(t *testing.T) { i := v1alpha1.TaskRunInputs{ - Params: []v1alpha1.Param{{ + Params: []v1alpha1.ArrayOrStringParam{{ Name: "name", - Value: "value", + Value: *builder.ArrayOrString("value"), }}, Resources: []v1alpha1.TaskResourceBinding{{ ResourceRef: v1alpha1.PipelineResourceRef{ @@ -230,12 +232,12 @@ func TestInput_Invalidate(t *testing.T) { }, Name: "resource", }}, - Params: []v1alpha1.Param{{ + Params: []v1alpha1.ArrayOrStringParam{{ Name: "name", - Value: "value", + Value: *builder.ArrayOrString("value"), }, { Name: "name", - Value: "value", + Value: *builder.ArrayOrString("value"), }}, }, wantErr: apis.ErrMultipleOneOf("spec.inputs.params"), diff --git a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go index e3866d79799..cc706f536d1 100644 --- a/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1alpha1/zz_generated.deepcopy.go @@ -26,6 +26,44 @@ import ( runtime "k8s.io/apimachinery/pkg/runtime" ) +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ArrayOrString) DeepCopyInto(out *ArrayOrString) { + *out = *in + if in.ArrayVal != nil { + in, out := &in.ArrayVal, &out.ArrayVal + *out = make([]string, len(*in)) + copy(*out, *in) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ArrayOrString. +func (in *ArrayOrString) DeepCopy() *ArrayOrString { + if in == nil { + return nil + } + out := new(ArrayOrString) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ArrayOrStringParam) DeepCopyInto(out *ArrayOrStringParam) { + *out = *in + in.Value.DeepCopyInto(&out.Value) + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ArrayOrStringParam. +func (in *ArrayOrStringParam) DeepCopy() *ArrayOrStringParam { + if in == nil { + return nil + } + out := new(ArrayOrStringParam) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ArtifactBucket) DeepCopyInto(out *ArtifactBucket) { *out = *in @@ -265,7 +303,9 @@ func (in *Inputs) DeepCopyInto(out *Inputs) { if in.Params != nil { in, out := &in.Params, &out.Params *out = make([]ParamSpec, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } return } @@ -364,6 +404,11 @@ func (in *Param) DeepCopy() *Param { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ParamSpec) DeepCopyInto(out *ParamSpec) { *out = *in + if in.Default != nil { + in, out := &in.Default, &out.Default + *out = new(ArrayOrString) + (*in).DeepCopyInto(*out) + } return } @@ -694,8 +739,10 @@ func (in *PipelineRunSpec) DeepCopyInto(out *PipelineRunSpec) { } if in.Params != nil { in, out := &in.Params, &out.Params - *out = make([]Param, len(*in)) - copy(*out, *in) + *out = make([]ArrayOrStringParam, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } if in.ServiceAccounts != nil { in, out := &in.ServiceAccounts, &out.ServiceAccounts @@ -845,7 +892,9 @@ func (in *PipelineSpec) DeepCopyInto(out *PipelineSpec) { if in.Params != nil { in, out := &in.Params, &out.Params *out = make([]ParamSpec, len(*in)) - copy(*out, *in) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } return } @@ -892,8 +941,10 @@ func (in *PipelineTask) DeepCopyInto(out *PipelineTask) { } if in.Params != nil { in, out := &in.Params, &out.Params - *out = make([]Param, len(*in)) - copy(*out, *in) + *out = make([]ArrayOrStringParam, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } return } @@ -1281,8 +1332,10 @@ func (in *TaskRunInputs) DeepCopyInto(out *TaskRunInputs) { } if in.Params != nil { in, out := &in.Params, &out.Params - *out = make([]Param, len(*in)) - copy(*out, *in) + *out = make([]ArrayOrStringParam, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } } return } diff --git a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go index 6b64f77e1fe..bd283e6c5b4 100644 --- a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go +++ b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun.go @@ -52,6 +52,10 @@ const ( // ReasonInvalidBindings indicates that the reason for the failure status is that the // PipelineResources bound in the PipelineRun didn't match those declared in the Pipeline ReasonInvalidBindings = "InvalidPipelineResourceBindings" + // ReasonParameterTypeMismatch indicates that the reason for the failure status is that + // parameter(s) declared in the PipelineRun do not have the some declared type as the + // parameters(s) declared in the Pipeline that they are supposed to override. + ReasonParameterTypeMismatch = "ParameterTypeMismatch" // ReasonCouldntGetTask indicates that the reason for the failure status is that the // associated Pipeline's Tasks couldn't all be retrieved ReasonCouldntGetTask = "CouldntGetTask" @@ -227,6 +231,21 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1alpha1.PipelineRun) er return nil } + // Ensure that the parameters from the PipelineRun are overriding Pipeline parameters with the same type. + // Weird templating issues can occur if this is not validated (ApplyParameters() does not verify type). + err = resources.ValidateParamTypesMatching(p, pr) + if err != nil { + // This Run has failed, so we need to mark it as failed and stop reconciling it + pr.Status.SetCondition(&apis.Condition{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionFalse, + Reason: ReasonParameterTypeMismatch, + Message: fmt.Sprintf("PipelineRun %s parameters have mismatching types with Pipeline %s's parameters: %s", + fmt.Sprintf("%s/%s", pr.Namespace, pr.Name), fmt.Sprintf("%s/%s", pr.Namespace, pr.Spec.PipelineRef.Name), err), + }) + return nil + } + // Apply parameter templating from the PipelineRun p = resources.ApplyParameters(p, pr) diff --git a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go index 22abd63a2df..98c54bea025 100644 --- a/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/v1alpha1/pipelinerun/pipelinerun_test.go @@ -80,8 +80,8 @@ func TestReconcile(t *testing.T) { tb.PipelineSpec( tb.PipelineDeclaredResource("git-repo", "git"), tb.PipelineDeclaredResource("best-image", "image"), - tb.PipelineParam("pipeline-param", tb.PipelineParamDefault("somethingdifferent")), - tb.PipelineParam("rev-param", tb.PipelineParamDefault("revision")), + tb.PipelineParam("pipeline-param", v1alpha1.ParamTypeString, tb.PipelineParamDefault("somethingdifferent")), + tb.PipelineParam("rev-param", v1alpha1.ParamTypeString, tb.PipelineParamDefault("revision")), // unit-test-3 uses runAfter to indicate it should run last tb.PipelineTask("unit-test-3", "unit-test-task", funParam, moreFunParam, templatedParam, @@ -116,7 +116,7 @@ func TestReconcile(t *testing.T) { tb.Task("unit-test-task", "foo", tb.TaskSpec( tb.TaskInputs( tb.InputsResource("workspace", v1alpha1.PipelineResourceTypeGit), - tb.InputsParam("foo"), tb.InputsParam("bar"), tb.InputsParam("templatedparam"), + tb.InputsParam("foo", v1alpha1.ParamTypeString), tb.InputsParam("bar", v1alpha1.ParamTypeString), tb.InputsParam("templatedparam", v1alpha1.ParamTypeString), ), tb.TaskOutputs( tb.OutputsResource("image-to-use", v1alpha1.PipelineResourceTypeImage), @@ -131,7 +131,7 @@ func TestReconcile(t *testing.T) { tb.ClusterTask("unit-test-cluster-task", tb.ClusterTaskSpec( tb.TaskInputs( tb.InputsResource("workspace", v1alpha1.PipelineResourceTypeGit), - tb.InputsParam("foo"), tb.InputsParam("bar"), tb.InputsParam("templatedparam"), + tb.InputsParam("foo", v1alpha1.ParamTypeString), tb.InputsParam("bar", v1alpha1.ParamTypeString), tb.InputsParam("templatedparam", v1alpha1.ParamTypeString), ), tb.TaskOutputs( tb.OutputsResource("image-to-use", v1alpha1.PipelineResourceTypeImage), @@ -244,7 +244,9 @@ func TestReconcile_InvalidPipelineRuns(t *testing.T) { ts := []*v1alpha1.Task{ tb.Task("a-task-that-exists", "foo"), tb.Task("a-task-that-needs-params", "foo", tb.TaskSpec( - tb.TaskInputs(tb.InputsParam("some-param")))), + tb.TaskInputs(tb.InputsParam("some-param", v1alpha1.ParamTypeString)))), + tb.Task("a-task-that-needs-array-params", "foo", tb.TaskSpec( + tb.TaskInputs(tb.InputsParam("some-param", v1alpha1.ParamTypeArray)))), } ps := []*v1alpha1.Pipeline{ tb.Pipeline("pipeline-missing-tasks", "foo", tb.PipelineSpec( @@ -259,6 +261,9 @@ func TestReconcile_InvalidPipelineRuns(t *testing.T) { tb.Pipeline("a-pipeline-that-should-be-caught-by-admission-control", "foo", tb.PipelineSpec( tb.PipelineTask("some-task", "a-task-that-exists", tb.PipelineTaskInputResource("needed-resource", "a-resource")))), + tb.Pipeline("a-pipeline-with-array-params", "foo", tb.PipelineSpec( + tb.PipelineParam("some-param", v1alpha1.ParamTypeArray), + tb.PipelineTask("some-task", "a-task-that-needs-array-params"))), } prs := []*v1alpha1.PipelineRun{ tb.PipelineRun("invalid-pipeline", "foo", tb.PipelineRunSpec("pipeline-not-exist")), @@ -268,6 +273,7 @@ func TestReconcile_InvalidPipelineRuns(t *testing.T) { tb.PipelineRun("pipeline-resources-dont-exist", "foo", tb.PipelineRunSpec("a-fine-pipeline", tb.PipelineRunResourceBinding("a-resource", tb.PipelineResourceBindingRef("missing-resource")))), tb.PipelineRun("pipeline-resources-not-declared", "foo", tb.PipelineRunSpec("a-pipeline-that-should-be-caught-by-admission-control")), + tb.PipelineRun("pipeline-mismatching-param-type", "foo", tb.PipelineRunSpec("a-pipeline-with-array-params", tb.PipelineRunParam("some-param", "stringval"))), } d := test.Data{ Tasks: ts, @@ -303,6 +309,10 @@ func TestReconcile_InvalidPipelineRuns(t *testing.T) { name: "invalid-pipeline-missing-declared-resource-shd-stop-reconciling", pipelineRun: prs[5], reason: ReasonFailedValidation, + }, { + name: "invalid-pipeline-mismatching-parameter-types", + pipelineRun: prs[6], + reason: ReasonParameterTypeMismatch, }, } diff --git a/pkg/reconciler/v1alpha1/pipelinerun/resources/apply.go b/pkg/reconciler/v1alpha1/pipelinerun/resources/apply.go index 723063e30ec..035ceb4da12 100644 --- a/pkg/reconciler/v1alpha1/pipelinerun/resources/apply.go +++ b/pkg/reconciler/v1alpha1/pipelinerun/resources/apply.go @@ -20,29 +20,41 @@ import ( "fmt" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" - "github.com/tektoncd/pipeline/pkg/templating" ) // ApplyParameters applies the params from a PipelineRun.Params to a PipelineSpec. func ApplyParameters(p *v1alpha1.Pipeline, pr *v1alpha1.PipelineRun) *v1alpha1.Pipeline { // This assumes that the PipelineRun inputs have been validated against what the Pipeline requests. - replacements := map[string]string{} - // Set all the default replacements + + // stringReplacements is used for standard single-string stringReplacements, while arrayReplacements contains arrays + // that need to be further processed. + stringReplacements := map[string]string{} + arrayReplacements := map[string][]string{} + + // Set all the default stringReplacements for _, p := range p.Spec.Params { - if p.Default != "" { - replacements[fmt.Sprintf("params.%s", p.Name)] = p.Default + if p.Default != nil { + if p.Default.Type == v1alpha1.ParamTypeString { + stringReplacements[fmt.Sprintf("params.%s", p.Name)] = p.Default.StringVal + } else { + arrayReplacements[fmt.Sprintf("params.%s", p.Name)] = p.Default.ArrayVal + } } } // Set and overwrite params with the ones from the PipelineRun for _, p := range pr.Spec.Params { - replacements[fmt.Sprintf("params.%s", p.Name)] = p.Value + if p.Value.Type == v1alpha1.ParamTypeString { + stringReplacements[fmt.Sprintf("params.%s", p.Name)] = p.Value.StringVal + } else { + arrayReplacements[fmt.Sprintf("params.%s", p.Name)] = p.Value.ArrayVal + } } - return ApplyReplacements(p, replacements) + return ApplyReplacements(p, stringReplacements, arrayReplacements) } // ApplyReplacements replaces placeholders for declared parameters with the specified replacements. -func ApplyReplacements(p *v1alpha1.Pipeline, replacements map[string]string) *v1alpha1.Pipeline { +func ApplyReplacements(p *v1alpha1.Pipeline, replacements map[string]string, arrayReplacements map[string][]string) *v1alpha1.Pipeline { p = p.DeepCopy() tasks := p.Spec.Tasks @@ -51,7 +63,7 @@ func ApplyReplacements(p *v1alpha1.Pipeline, replacements map[string]string) *v1 params := tasks[i].Params for j := range params { - params[j].Value = templating.ApplyReplacements(params[j].Value, replacements) + params[j].Value.ApplyReplacements(replacements, arrayReplacements) } tasks[i].Params = params diff --git a/pkg/reconciler/v1alpha1/pipelinerun/resources/apply_test.go b/pkg/reconciler/v1alpha1/pipelinerun/resources/apply_test.go index ceeee1d850c..ddbdc5a4b83 100644 --- a/pkg/reconciler/v1alpha1/pipelinerun/resources/apply_test.go +++ b/pkg/reconciler/v1alpha1/pipelinerun/resources/apply_test.go @@ -1,5 +1,5 @@ /* - Copyright 2019 Knative Authors LLC + Copyright 2019 The Tekton Authors Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at @@ -27,49 +27,71 @@ func TestApplyParameters(t *testing.T) { original *v1alpha1.Pipeline run *v1alpha1.PipelineRun expected *v1alpha1.Pipeline - }{ - { - name: "single parameter", - original: tb.Pipeline("test-pipeline", "foo", - tb.PipelineSpec( - tb.PipelineParam("first-param", tb.PipelineParamDefault("default-value")), - tb.PipelineParam("second-param"), - tb.PipelineTask("first-task-1", "first-task", - tb.PipelineTaskParam("first-task-first-param", "${params.first-param}"), - tb.PipelineTaskParam("first-task-second-param", "${params.second-param}"), - tb.PipelineTaskParam("first-task-third-param", "static value"), - ))), - run: tb.PipelineRun("test-pipeline-run", "foo", - tb.PipelineRunSpec("test-pipeline", - tb.PipelineRunParam("second-param", "second-value"))), - expected: tb.Pipeline("test-pipeline", "foo", - tb.PipelineSpec( - tb.PipelineParam("first-param", tb.PipelineParamDefault("default-value")), - tb.PipelineParam("second-param"), - tb.PipelineTask("first-task-1", "first-task", - tb.PipelineTaskParam("first-task-first-param", "default-value"), - tb.PipelineTaskParam("first-task-second-param", "second-value"), - tb.PipelineTaskParam("first-task-third-param", "static value"), - ))), - }, - { - name: "pipeline parameter nested inside task parameter", - original: tb.Pipeline("test-pipeline", "foo", - tb.PipelineSpec( - tb.PipelineParam("first-param", tb.PipelineParamDefault("default-value")), - tb.PipelineTask("first-task-1", "first-task", - tb.PipelineTaskParam("first-task-first-param", "${input.workspace.${params.first-param}}"), - ))), - run: tb.PipelineRun("test-pipeline-run", "foo", - tb.PipelineRunSpec("test-pipeline")), - expected: tb.Pipeline("test-pipeline", "foo", - tb.PipelineSpec( - tb.PipelineParam("first-param", tb.PipelineParamDefault("default-value")), - tb.PipelineTask("first-task-1", "first-task", - tb.PipelineTaskParam("first-task-first-param", "${input.workspace.default-value}"), - ))), - }, - } + }{{ + name: "single parameter", + original: tb.Pipeline("test-pipeline", "foo", + tb.PipelineSpec( + tb.PipelineParam("first-param", v1alpha1.ParamTypeString, tb.PipelineParamDefault("default-value")), + tb.PipelineParam("second-param", v1alpha1.ParamTypeString), + tb.PipelineTask("first-task-1", "first-task", + tb.PipelineTaskParam("first-task-first-param", "${params.first-param}"), + tb.PipelineTaskParam("first-task-second-param", "${params.second-param}"), + tb.PipelineTaskParam("first-task-third-param", "static value"), + ))), + run: tb.PipelineRun("test-pipeline-run", "foo", + tb.PipelineRunSpec("test-pipeline", + tb.PipelineRunParam("second-param", "second-value"))), + expected: tb.Pipeline("test-pipeline", "foo", + tb.PipelineSpec( + tb.PipelineParam("first-param", v1alpha1.ParamTypeString, tb.PipelineParamDefault("default-value")), + tb.PipelineParam("second-param", v1alpha1.ParamTypeString), + tb.PipelineTask("first-task-1", "first-task", + tb.PipelineTaskParam("first-task-first-param", "default-value"), + tb.PipelineTaskParam("first-task-second-param", "second-value"), + tb.PipelineTaskParam("first-task-third-param", "static value"), + ))), + }, { + name: "pipeline parameter nested inside task parameter", + original: tb.Pipeline("test-pipeline", "foo", + tb.PipelineSpec( + tb.PipelineParam("first-param", v1alpha1.ParamTypeString, tb.PipelineParamDefault("default-value")), + tb.PipelineTask("first-task-1", "first-task", + tb.PipelineTaskParam("first-task-first-param", "${input.workspace.${params.first-param}}"), + ))), + run: tb.PipelineRun("test-pipeline-run", "foo", + tb.PipelineRunSpec("test-pipeline")), + expected: tb.Pipeline("test-pipeline", "foo", + tb.PipelineSpec( + tb.PipelineParam("first-param", v1alpha1.ParamTypeString, tb.PipelineParamDefault("default-value")), + tb.PipelineTask("first-task-1", "first-task", + tb.PipelineTaskParam("first-task-first-param", "${input.workspace.default-value}"), + ))), + }, { + name: "single array parameter", + original: tb.Pipeline("test-pipeline", "foo", + tb.PipelineSpec( + tb.PipelineParam("first-param", v1alpha1.ParamTypeArray, tb.PipelineParamDefault( + "default", "array", "value")), + tb.PipelineParam("second-param", v1alpha1.ParamTypeArray), + tb.PipelineTask("first-task-1", "first-task", + tb.PipelineTaskParam("first-task-first-param", "firstelement", "${params.first-param}"), + tb.PipelineTaskParam("first-task-second-param", "first", "${params.second-param}"), + tb.PipelineTaskParam("first-task-third-param", "static value"), + ))), + run: tb.PipelineRun("test-pipeline-run", "foo", + tb.PipelineRunSpec("test-pipeline", + tb.PipelineRunParam("second-param", "second-value", "array"))), + expected: tb.Pipeline("test-pipeline", "foo", + tb.PipelineSpec( + tb.PipelineParam("first-param", v1alpha1.ParamTypeArray, tb.PipelineParamDefault( + "default", "array", "value")), + tb.PipelineParam("second-param", v1alpha1.ParamTypeArray), + tb.PipelineTask("first-task-1", "first-task", + tb.PipelineTaskParam("first-task-first-param", "firstelement", "default", "array", "value"), + tb.PipelineTaskParam("first-task-second-param", "first", "second-value", "array"), + tb.PipelineTaskParam("first-task-third-param", "static value"), + ))), + }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { got := ApplyParameters(tt.original, tt.run) diff --git a/pkg/reconciler/v1alpha1/pipelinerun/resources/validate_params.go b/pkg/reconciler/v1alpha1/pipelinerun/resources/validate_params.go new file mode 100644 index 00000000000..e50ada40ac4 --- /dev/null +++ b/pkg/reconciler/v1alpha1/pipelinerun/resources/validate_params.go @@ -0,0 +1,47 @@ +/* +Copyright 2019 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package resources + +import ( + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" + "golang.org/x/xerrors" +) + +// Validate that parameters in PipelineRun override corresponding parameters in Pipeline of the same type. +func ValidateParamTypesMatching(p *v1alpha1.Pipeline, pr *v1alpha1.PipelineRun) error { + // Build a map of parameter names/types declared in p. + paramTypes := make(map[string]v1alpha1.ParamType) + for _, param := range p.Spec.Params { + paramTypes[param.Name] = param.Type + } + + // Build a list of parameter names from pr that have mismatching types with the map created above. + var wrongTypeParamNames []string + for _, param := range pr.Spec.Params { + if paramType, ok := paramTypes[param.Name]; ok { + if param.Value.Type != paramType { + wrongTypeParamNames = append(wrongTypeParamNames, param.Name) + } + } + } + + // Return an error with the misconfigured parameters' names, or return nil if there are none. + if len(wrongTypeParamNames) != 0 { + return xerrors.Errorf("parameters have inconsistent types : %s", wrongTypeParamNames) + } + return nil +} diff --git a/pkg/reconciler/v1alpha1/pipelinerun/resources/validate_params_test.go b/pkg/reconciler/v1alpha1/pipelinerun/resources/validate_params_test.go new file mode 100644 index 00000000000..2c4ccfd3da1 --- /dev/null +++ b/pkg/reconciler/v1alpha1/pipelinerun/resources/validate_params_test.go @@ -0,0 +1,123 @@ +/* +Copyright 2019 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package resources + +import ( + "testing" + + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" + tb "github.com/tektoncd/pipeline/test/builder" +) + +func TestValidateParamTypesMatching_Valid(t *testing.T) { + tcs := []struct { + name string + p *v1alpha1.Pipeline + pr *v1alpha1.PipelineRun + errorExpected bool + }{{ + name: "proper param types", + p: tb.Pipeline("a-pipeline", namespace, tb.PipelineSpec( + tb.PipelineParam("correct-type-1", v1alpha1.ParamTypeString), + tb.PipelineParam("mismatching-type", v1alpha1.ParamTypeString), + tb.PipelineParam("correct-type-2", v1alpha1.ParamTypeArray))), + pr: tb.PipelineRun("a-pipelinerun", namespace, tb.PipelineRunSpec( + "test-pipeline", + tb.PipelineRunParam("correct-type-1", "somestring"), + tb.PipelineRunParam("mismatching-type", "astring"), + tb.PipelineRunParam("correct-type-2", "another", "array"))), + errorExpected: false, + }, { + name: "no params to get wrong", + p: tb.Pipeline("a-pipeline", namespace), + pr: tb.PipelineRun("a-pipelinerun", namespace), + errorExpected: false, + }, { + name: "string-array mismatch", + p: tb.Pipeline("a-pipeline", namespace, tb.PipelineSpec( + tb.PipelineParam("correct-type-1", v1alpha1.ParamTypeString), + tb.PipelineParam("mismatching-type", v1alpha1.ParamTypeString), + tb.PipelineParam("correct-type-2", v1alpha1.ParamTypeArray))), + pr: tb.PipelineRun("a-pipelinerun", namespace, + tb.PipelineRunSpec("test-pipeline", + tb.PipelineRunParam("correct-type-1", "somestring"), + tb.PipelineRunParam("mismatching-type", "an", "array"), + tb.PipelineRunParam("correct-type-2", "another", "array"))), + errorExpected: true, + }, { + name: "array-string mismatch", + p: tb.Pipeline("a-pipeline", namespace, tb.PipelineSpec( + tb.PipelineParam("correct-type-1", v1alpha1.ParamTypeString), + tb.PipelineParam("mismatching-type", v1alpha1.ParamTypeArray), + tb.PipelineParam("correct-type-2", v1alpha1.ParamTypeArray))), + pr: tb.PipelineRun("a-pipelinerun", namespace, + tb.PipelineRunSpec("test-pipeline", + tb.PipelineRunParam("correct-type-1", "somestring"), + tb.PipelineRunParam("mismatching-type", "astring"), + tb.PipelineRunParam("correct-type-2", "another", "array"))), + errorExpected: true, + }} + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + err := ValidateParamTypesMatching(tc.p, tc.pr) + if (!tc.errorExpected) && (err != nil) { + t.Errorf("Pipeline.Validate() returned error: %v", err) + } + + if tc.errorExpected && (err == nil) { + t.Error("Pipeline.Validate() did not return error, wanted error") + } + }) + } +} + +func TestValidateParamTypesMatching_Invalid(t *testing.T) { + tcs := []struct { + name string + p *v1alpha1.Pipeline + pr *v1alpha1.PipelineRun + }{{ + name: "string-array mismatch", + p: tb.Pipeline("a-pipeline", namespace, tb.PipelineSpec( + tb.PipelineParam("correct-type-1", v1alpha1.ParamTypeString), + tb.PipelineParam("mismatching-type", v1alpha1.ParamTypeString), + tb.PipelineParam("correct-type-2", v1alpha1.ParamTypeArray))), + pr: tb.PipelineRun("a-pipelinerun", namespace, + tb.PipelineRunSpec("test-pipeline", + tb.PipelineRunParam("correct-type-1", "somestring"), + tb.PipelineRunParam("mismatching-type", "an", "array"), + tb.PipelineRunParam("correct-type-2", "another", "array"))), + }, { + name: "array-string mismatch", + p: tb.Pipeline("a-pipeline", namespace, tb.PipelineSpec( + tb.PipelineParam("correct-type-1", v1alpha1.ParamTypeString), + tb.PipelineParam("mismatching-type", v1alpha1.ParamTypeArray), + tb.PipelineParam("correct-type-2", v1alpha1.ParamTypeArray))), + pr: tb.PipelineRun("a-pipelinerun", namespace, + tb.PipelineRunSpec("test-pipeline", + tb.PipelineRunParam("correct-type-1", "somestring"), + tb.PipelineRunParam("mismatching-type", "astring"), + tb.PipelineRunParam("correct-type-2", "another", "array"))), + }} + for _, tc := range tcs { + t.Run(tc.name, func(t *testing.T) { + if err := ValidateParamTypesMatching(tc.p, tc.pr); err == nil { + t.Errorf("Expected to see error when validating PipelineRun/Pipeline param types but saw none") + } + }) + } +} diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/apply.go b/pkg/reconciler/v1alpha1/taskrun/resources/apply.go index b07b0964e82..67be1a05112 100644 --- a/pkg/reconciler/v1alpha1/taskrun/resources/apply.go +++ b/pkg/reconciler/v1alpha1/taskrun/resources/apply.go @@ -27,19 +27,32 @@ import ( // ApplyParameters applies the params from a TaskRun.Input.Parameters to a TaskSpec func ApplyParameters(spec *v1alpha1.TaskSpec, tr *v1alpha1.TaskRun, defaults ...v1alpha1.ParamSpec) *v1alpha1.TaskSpec { // This assumes that the TaskRun inputs have been validated against what the Task requests. - replacements := map[string]string{} - // Set all the default replacements + + // stringReplacements is used for standard single-string stringReplacements, while arrayReplacements contains arrays + // that need to be further processed. + stringReplacements := map[string]string{} + arrayReplacements := map[string][]string{} + + // Set all the default stringReplacements for _, p := range defaults { - if p.Default != "" { - replacements[fmt.Sprintf("inputs.params.%s", p.Name)] = p.Default + if p.Default != nil { + if p.Default.Type == v1alpha1.ParamTypeString { + stringReplacements[fmt.Sprintf("inputs.params.%s", p.Name)] = p.Default.StringVal + } else { + arrayReplacements[fmt.Sprintf("inputs.params.%s", p.Name)] = p.Default.ArrayVal + } } } // Set and overwrite params with the ones from the TaskRun for _, p := range tr.Spec.Inputs.Params { - replacements[fmt.Sprintf("inputs.params.%s", p.Name)] = p.Value + if p.Value.Type == v1alpha1.ParamTypeString { + stringReplacements[fmt.Sprintf("inputs.params.%s", p.Name)] = p.Value.StringVal + } else { + arrayReplacements[fmt.Sprintf("inputs.params.%s", p.Name)] = p.Value.ArrayVal + } } - return ApplyReplacements(spec, replacements) + return ApplyReplacements(spec, stringReplacements, arrayReplacements) } // ApplyResources applies the templating from values in resources which are referenced in spec as subitems @@ -51,82 +64,93 @@ func ApplyResources(spec *v1alpha1.TaskSpec, resolvedResources map[string]v1alph replacements[fmt.Sprintf("%s.resources.%s.%s", replacementStr, name, k)] = v } } - return ApplyReplacements(spec, replacements) + return ApplyReplacements(spec, replacements, map[string][]string{}) } // ApplyReplacements replaces placeholders for declared parameters with the specified replacements. -func ApplyReplacements(spec *v1alpha1.TaskSpec, replacements map[string]string) *v1alpha1.TaskSpec { +func ApplyReplacements(spec *v1alpha1.TaskSpec, stringReplacements map[string]string, arrayReplacements map[string][]string) *v1alpha1.TaskSpec { spec = spec.DeepCopy() // Apply variable expansion to steps fields. steps := spec.Steps for i := range steps { - applyContainerReplacements(&steps[i], replacements) + applyContainerReplacements(&steps[i], stringReplacements, arrayReplacements) } // Apply variable expansion to containerTemplate fields. // Should eventually be removed; ContainerTemplate is the deprecated previous name of the StepTemplate field (#977). if spec.ContainerTemplate != nil { - applyContainerReplacements(spec.ContainerTemplate, replacements) + applyContainerReplacements(spec.ContainerTemplate, stringReplacements, arrayReplacements) } // Apply variable expansion to stepTemplate fields. if spec.StepTemplate != nil { - applyContainerReplacements(spec.StepTemplate, replacements) + applyContainerReplacements(spec.StepTemplate, stringReplacements, arrayReplacements) } // Apply variable expansion to the build's volumes for i, v := range spec.Volumes { - spec.Volumes[i].Name = templating.ApplyReplacements(v.Name, replacements) + spec.Volumes[i].Name = templating.ApplyReplacements(v.Name, stringReplacements) if v.VolumeSource.ConfigMap != nil { - spec.Volumes[i].ConfigMap.Name = templating.ApplyReplacements(v.ConfigMap.Name, replacements) + spec.Volumes[i].ConfigMap.Name = templating.ApplyReplacements(v.ConfigMap.Name, stringReplacements) } if v.VolumeSource.Secret != nil { - spec.Volumes[i].Secret.SecretName = templating.ApplyReplacements(v.Secret.SecretName, replacements) + spec.Volumes[i].Secret.SecretName = templating.ApplyReplacements(v.Secret.SecretName, stringReplacements) } if v.PersistentVolumeClaim != nil { - spec.Volumes[i].PersistentVolumeClaim.ClaimName = templating.ApplyReplacements(v.PersistentVolumeClaim.ClaimName, replacements) + spec.Volumes[i].PersistentVolumeClaim.ClaimName = templating.ApplyReplacements(v.PersistentVolumeClaim.ClaimName, stringReplacements) } } return spec } -func applyContainerReplacements(container *corev1.Container, replacements map[string]string) { - container.Name = templating.ApplyReplacements(container.Name, replacements) - container.Image = templating.ApplyReplacements(container.Image, replacements) - for ia, a := range container.Args { - container.Args[ia] = templating.ApplyReplacements(a, replacements) +func applyContainerReplacements(container *corev1.Container, stringReplacements map[string]string, arrayReplacements map[string][]string) { + container.Name = templating.ApplyReplacements(container.Name, stringReplacements) + container.Image = templating.ApplyReplacements(container.Image, stringReplacements) + + //Use ApplyArrayReplacements here, as additional args may be added via an array parameter. + var newArgs []string + for _, a := range container.Args { + newArgs = append(newArgs, templating.ApplyArrayReplacements(a, stringReplacements, arrayReplacements)...) } + container.Args = newArgs + for ie, e := range container.Env { - container.Env[ie].Value = templating.ApplyReplacements(e.Value, replacements) + container.Env[ie].Value = templating.ApplyReplacements(e.Value, stringReplacements) if container.Env[ie].ValueFrom != nil { if e.ValueFrom.SecretKeyRef != nil { - container.Env[ie].ValueFrom.SecretKeyRef.LocalObjectReference.Name = templating.ApplyReplacements(e.ValueFrom.SecretKeyRef.LocalObjectReference.Name, replacements) - container.Env[ie].ValueFrom.SecretKeyRef.Key = templating.ApplyReplacements(e.ValueFrom.SecretKeyRef.Key, replacements) + container.Env[ie].ValueFrom.SecretKeyRef.LocalObjectReference.Name = templating.ApplyReplacements(e.ValueFrom.SecretKeyRef.LocalObjectReference.Name, stringReplacements) + container.Env[ie].ValueFrom.SecretKeyRef.Key = templating.ApplyReplacements(e.ValueFrom.SecretKeyRef.Key, stringReplacements) } if e.ValueFrom.ConfigMapKeyRef != nil { - container.Env[ie].ValueFrom.ConfigMapKeyRef.LocalObjectReference.Name = templating.ApplyReplacements(e.ValueFrom.ConfigMapKeyRef.LocalObjectReference.Name, replacements) - container.Env[ie].ValueFrom.ConfigMapKeyRef.Key = templating.ApplyReplacements(e.ValueFrom.ConfigMapKeyRef.Key, replacements) + container.Env[ie].ValueFrom.ConfigMapKeyRef.LocalObjectReference.Name = templating.ApplyReplacements(e.ValueFrom.ConfigMapKeyRef.LocalObjectReference.Name, stringReplacements) + container.Env[ie].ValueFrom.ConfigMapKeyRef.Key = templating.ApplyReplacements(e.ValueFrom.ConfigMapKeyRef.Key, stringReplacements) } } } + for ie, e := range container.EnvFrom { - container.EnvFrom[ie].Prefix = templating.ApplyReplacements(e.Prefix, replacements) + container.EnvFrom[ie].Prefix = templating.ApplyReplacements(e.Prefix, stringReplacements) if e.ConfigMapRef != nil { - container.EnvFrom[ie].ConfigMapRef.LocalObjectReference.Name = templating.ApplyReplacements(e.ConfigMapRef.LocalObjectReference.Name, replacements) + container.EnvFrom[ie].ConfigMapRef.LocalObjectReference.Name = templating.ApplyReplacements(e.ConfigMapRef.LocalObjectReference.Name, stringReplacements) } if e.SecretRef != nil { - container.EnvFrom[ie].SecretRef.LocalObjectReference.Name = templating.ApplyReplacements(e.SecretRef.LocalObjectReference.Name, replacements) + container.EnvFrom[ie].SecretRef.LocalObjectReference.Name = templating.ApplyReplacements(e.SecretRef.LocalObjectReference.Name, stringReplacements) } } - container.WorkingDir = templating.ApplyReplacements(container.WorkingDir, replacements) - for ic, c := range container.Command { - container.Command[ic] = templating.ApplyReplacements(c, replacements) + container.WorkingDir = templating.ApplyReplacements(container.WorkingDir, stringReplacements) + + //Use ApplyArrayReplacements here, as additional commands may be added via an array parameter. + var newCommand []string + for _, c := range container.Command { + newCommand = append(newCommand, templating.ApplyArrayReplacements(c, stringReplacements, arrayReplacements)...) } + container.Command = newCommand + for iv, v := range container.VolumeMounts { - container.VolumeMounts[iv].Name = templating.ApplyReplacements(v.Name, replacements) - container.VolumeMounts[iv].MountPath = templating.ApplyReplacements(v.MountPath, replacements) - container.VolumeMounts[iv].SubPath = templating.ApplyReplacements(v.SubPath, replacements) + container.VolumeMounts[iv].Name = templating.ApplyReplacements(v.Name, stringReplacements) + container.VolumeMounts[iv].MountPath = templating.ApplyReplacements(v.MountPath, stringReplacements) + container.VolumeMounts[iv].SubPath = templating.ApplyReplacements(v.SubPath, stringReplacements) } } diff --git a/pkg/reconciler/v1alpha1/taskrun/resources/apply_test.go b/pkg/reconciler/v1alpha1/taskrun/resources/apply_test.go index b2013038dc3..dacf12901f4 100644 --- a/pkg/reconciler/v1alpha1/taskrun/resources/apply_test.go +++ b/pkg/reconciler/v1alpha1/taskrun/resources/apply_test.go @@ -14,13 +14,15 @@ See the License for the specific language governing permissions and limitations under the License. */ -package resources +package resources_test import ( "testing" "github.com/google/go-cmp/cmp" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" + "github.com/tektoncd/pipeline/pkg/reconciler/v1alpha1/taskrun/resources" + "github.com/tektoncd/pipeline/test/builder" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -140,13 +142,162 @@ var gcsTaskSpec = &v1alpha1.TaskSpec{ }}, } +var arrayParamTaskSpec = &v1alpha1.TaskSpec{ + Steps: []corev1.Container{{ + Name: "simple-image", + Image: "some-image", + }, { + Name: "image-with-args-specified", + Image: "some-other-image", + Command: []string{"echo"}, + Args: []string{"first", "second", "${inputs.params.array-param}", "last"}, + }}, +} + +var arrayAndStringParamTaskSpec = &v1alpha1.TaskSpec{ + Steps: []corev1.Container{{ + Name: "simple-image", + Image: "some-image", + }, { + Name: "image-with-args-specified", + Image: "some-other-image", + Command: []string{"echo"}, + Args: []string{"${inputs.params.normal-param}", "second", "${inputs.params.array-param}", "last"}, + }}, +} + +var multipleArrayParamsTaskSpec = &v1alpha1.TaskSpec{ + Steps: []corev1.Container{{ + Name: "simple-image", + Image: "some-image", + }, { + Name: "image-with-args-specified", + Image: "some-other-image", + Command: []string{"cmd", "${inputs.params.another-array-param}"}, + Args: []string{"first", "second", "${inputs.params.array-param}", "last"}, + }}, +} + +var multipleArrayAndStringsParamsTaskSpec = &v1alpha1.TaskSpec{ + Steps: []corev1.Container{{ + Name: "simple-image", + Image: "image-${inputs.params.string-param2}", + }, { + Name: "image-with-args-specified", + Image: "some-other-image", + Command: []string{"cmd", "${inputs.params.array-param1}"}, + Args: []string{"${inputs.params.array-param2}", "second", "${inputs.params.array-param1}", "${inputs.params.string-param1}", "last"}, + }}, +} + var paramTaskRun = &v1alpha1.TaskRun{ Spec: v1alpha1.TaskRunSpec{ Inputs: v1alpha1.TaskRunInputs{ - Params: []v1alpha1.Param{ + Params: []v1alpha1.ArrayOrStringParam{ { Name: "myimage", - Value: "bar", + Value: *builder.ArrayOrString("bar"), + }, + }, + }, + }, +} + +var arrayTaskRun0Elements = &v1alpha1.TaskRun{ + Spec: v1alpha1.TaskRunSpec{ + Inputs: v1alpha1.TaskRunInputs{ + Params: []v1alpha1.ArrayOrStringParam{ + { + Name: "array-param", + Value: v1alpha1.ArrayOrString{ + Type: v1alpha1.ParamTypeArray, + ArrayVal: []string{}, + }, + }, + }, + }, + }, +} + +var arrayTaskRun1Elements = &v1alpha1.TaskRun{ + Spec: v1alpha1.TaskRunSpec{ + Inputs: v1alpha1.TaskRunInputs{ + Params: []v1alpha1.ArrayOrStringParam{ + { + Name: "array-param", + Value: *builder.ArrayOrString("foo"), + }, + }, + }, + }, +} + +var arrayTaskRun3Elements = &v1alpha1.TaskRun{ + Spec: v1alpha1.TaskRunSpec{ + Inputs: v1alpha1.TaskRunInputs{ + Params: []v1alpha1.ArrayOrStringParam{ + { + Name: "array-param", + Value: *builder.ArrayOrString("foo", "bar", "third"), + }, + }, + }, + }, +} + +var arrayTaskRunMultipleArrays = &v1alpha1.TaskRun{ + Spec: v1alpha1.TaskRunSpec{ + Inputs: v1alpha1.TaskRunInputs{ + Params: []v1alpha1.ArrayOrStringParam{ + { + Name: "array-param", + Value: *builder.ArrayOrString("foo", "bar", "third"), + }, + { + Name: "another-array-param", + Value: *builder.ArrayOrString("part1", "part2"), + }, + }, + }, + }, +} + +var arrayTaskRunWith1StringParam = &v1alpha1.TaskRun{ + Spec: v1alpha1.TaskRunSpec{ + Inputs: v1alpha1.TaskRunInputs{ + Params: []v1alpha1.ArrayOrStringParam{ + { + Name: "array-param", + Value: *builder.ArrayOrString("middlefirst", "middlesecond"), + }, + { + Name: "normal-param", + Value: *builder.ArrayOrString("foo"), + }, + }, + }, + }, +} + +var arrayTaskRunMultipleArraysAndStrings = &v1alpha1.TaskRun{ + Spec: v1alpha1.TaskRunSpec{ + Inputs: v1alpha1.TaskRunInputs{ + Params: []v1alpha1.ArrayOrStringParam{ + { + Name: "array-param1", + Value: *builder.ArrayOrString("1-param1", "2-param1", "3-param1", "4-param1"), + }, + { + Name: "array-param2", + Value: *builder.ArrayOrString("1-param2", "2-param2", "2-param3"), + }, + { + Name: "string-param1", + Value: *builder.ArrayOrString("foo"), + }, + { + Name: "string-param2", + Value: *builder.ArrayOrString("bar"), }, }, }, @@ -241,6 +392,78 @@ func TestApplyParameters(t *testing.T) { want: applyMutation(simpleTaskSpec, func(spec *v1alpha1.TaskSpec) { spec.Steps[0].Image = "bar" }), + }, { + name: "array parameter with 0 elements", + args: args{ + ts: arrayParamTaskSpec, + tr: arrayTaskRun0Elements, + }, + want: applyMutation(arrayParamTaskSpec, func(spec *v1alpha1.TaskSpec) { + spec.Steps[1].Args = []string{"first", "second", "last"} + }), + }, { + name: "array parameter with 1 element", + args: args{ + ts: arrayParamTaskSpec, + tr: arrayTaskRun1Elements, + }, + want: applyMutation(arrayParamTaskSpec, func(spec *v1alpha1.TaskSpec) { + spec.Steps[1].Args = []string{"first", "second", "foo", "last"} + }), + }, { + name: "array parameter with 3 elements", + args: args{ + ts: arrayParamTaskSpec, + tr: arrayTaskRun3Elements, + }, + want: applyMutation(arrayParamTaskSpec, func(spec *v1alpha1.TaskSpec) { + spec.Steps[1].Args = []string{"first", "second", "foo", "bar", "third", "last"} + }), + }, { + name: "multiple arrays", + args: args{ + ts: multipleArrayParamsTaskSpec, + tr: arrayTaskRunMultipleArrays, + }, + want: applyMutation(multipleArrayParamsTaskSpec, func(spec *v1alpha1.TaskSpec) { + spec.Steps[1].Command = []string{"cmd", "part1", "part2"} + spec.Steps[1].Args = []string{"first", "second", "foo", "bar", "third", "last"} + }), + }, { + name: "array and normal string parameter", + args: args{ + ts: arrayAndStringParamTaskSpec, + tr: arrayTaskRunWith1StringParam, + }, + want: applyMutation(arrayAndStringParamTaskSpec, func(spec *v1alpha1.TaskSpec) { + spec.Steps[1].Args = []string{"foo", "second", "middlefirst", "middlesecond", "last"} + }), + }, { + name: "several arrays and strings", + args: args{ + ts: multipleArrayAndStringsParamsTaskSpec, + tr: arrayTaskRunMultipleArraysAndStrings, + }, + want: applyMutation(multipleArrayAndStringsParamsTaskSpec, func(spec *v1alpha1.TaskSpec) { + spec.Steps[0].Image = "image-bar" + spec.Steps[1].Command = []string{"cmd", "1-param1", "2-param1", "3-param1", "4-param1"} + spec.Steps[1].Args = []string{"1-param2", "2-param2", "2-param3", "second", "1-param1", "2-param1", "3-param1", "4-param1", "foo", "last"} + }), + }, { + name: "default array parameter", + args: args{ + ts: arrayParamTaskSpec, + tr: &v1alpha1.TaskRun{}, + dp: []v1alpha1.ParamSpec{ + { + Name: "array-param", + Default: builder.ArrayOrString("defaulted", "value!"), + }, + }, + }, + want: applyMutation(arrayParamTaskSpec, func(spec *v1alpha1.TaskSpec) { + spec.Steps[1].Args = []string{"first", "second", "defaulted", "value!", "last"} + }), }, { name: "volume mount parameter", args: args{ @@ -248,9 +471,9 @@ func TestApplyParameters(t *testing.T) { tr: &v1alpha1.TaskRun{ Spec: v1alpha1.TaskRunSpec{ Inputs: v1alpha1.TaskRunInputs{ - Params: []v1alpha1.Param{{ + Params: []v1alpha1.ArrayOrStringParam{{ Name: "FOO", - Value: "world", + Value: *builder.ArrayOrString("world"), }}, }, }, @@ -269,9 +492,9 @@ func TestApplyParameters(t *testing.T) { tr: &v1alpha1.TaskRun{ Spec: v1alpha1.TaskRunSpec{ Inputs: v1alpha1.TaskRunInputs{ - Params: []v1alpha1.Param{{ + Params: []v1alpha1.ArrayOrStringParam{{ Name: "FOO", - Value: "world", + Value: *builder.ArrayOrString("world"), }}, }, }, @@ -297,9 +520,9 @@ func TestApplyParameters(t *testing.T) { tr: &v1alpha1.TaskRun{ Spec: v1alpha1.TaskRunSpec{ Inputs: v1alpha1.TaskRunInputs{ - Params: []v1alpha1.Param{{ + Params: []v1alpha1.ArrayOrStringParam{{ Name: "FOO", - Value: "BAR", + Value: *builder.ArrayOrString("BAR"), }}, }, }, @@ -307,7 +530,7 @@ func TestApplyParameters(t *testing.T) { dp: []v1alpha1.ParamSpec{ { Name: "myimage", - Default: "replaced-image-name", + Default: builder.ArrayOrString("replaced-image-name"), }, }, }, @@ -322,9 +545,9 @@ func TestApplyParameters(t *testing.T) { tr: &v1alpha1.TaskRun{ Spec: v1alpha1.TaskRunSpec{ Inputs: v1alpha1.TaskRunInputs{ - Params: []v1alpha1.Param{{ + Params: []v1alpha1.ArrayOrStringParam{{ Name: "FOO", - Value: "BAR", + Value: *builder.ArrayOrString("BAR"), }}, }, }, @@ -332,7 +555,7 @@ func TestApplyParameters(t *testing.T) { dp: []v1alpha1.ParamSpec{ { Name: "myimage", - Default: "replaced-image-name", + Default: builder.ArrayOrString("replaced-image-name"), }, }, }, @@ -348,7 +571,7 @@ func TestApplyParameters(t *testing.T) { dp: []v1alpha1.ParamSpec{ { Name: "myimage", - Default: "mydefault", + Default: builder.ArrayOrString("mydefault"), }, }, }, @@ -358,7 +581,7 @@ func TestApplyParameters(t *testing.T) { }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := ApplyParameters(tt.args.ts, tt.args.tr, tt.args.dp...) + got := resources.ApplyParameters(tt.args.ts, tt.args.tr, tt.args.dp...) if d := cmp.Diff(got, tt.want); d != "" { t.Errorf("ApplyParameters() got diff %s", d) } @@ -420,7 +643,7 @@ func TestApplyResources(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { setup() - got := ApplyResources(tt.args.ts, tt.args.r, tt.args.rStr) + got := resources.ApplyResources(tt.args.ts, tt.args.r, tt.args.rStr) if d := cmp.Diff(got, tt.want); d != "" { t.Errorf("ApplyResources() diff %s", d) } @@ -532,7 +755,7 @@ func TestVolumeReplacement(t *testing.T) { }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := ApplyReplacements(tt.ts, tt.repl) + got := resources.ApplyReplacements(tt.ts, tt.repl, map[string][]string{}) if d := cmp.Diff(got, tt.want); d != "" { t.Errorf("ApplyResources() diff %s", d) } diff --git a/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go b/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go index 709de0cfe4f..0d3f2d6b2db 100644 --- a/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go +++ b/pkg/reconciler/v1alpha1/taskrun/taskrun_test.go @@ -112,9 +112,9 @@ var ( templatedTask = tb.Task("test-task-with-templating", "foo", tb.TaskSpec( tb.TaskInputs( tb.InputsResource("workspace", v1alpha1.PipelineResourceTypeGit), - tb.InputsParam("myarg"), tb.InputsParam("myarghasdefault", tb.ParamDefault("dont see me")), - tb.InputsParam("myarghasdefault2", tb.ParamDefault("thedefault")), - tb.InputsParam("configmapname"), + tb.InputsParam("myarg", v1alpha1.ParamTypeString), tb.InputsParam("myarghasdefault", v1alpha1.ParamTypeString, tb.ParamDefault("dont see me")), + tb.InputsParam("myarghasdefault2", v1alpha1.ParamTypeString, tb.ParamDefault("thedefault")), + tb.InputsParam("configmapname", v1alpha1.ParamTypeString), ), tb.TaskOutputs(tb.OutputsResource("myimage", v1alpha1.PipelineResourceTypeImage)), tb.Step("mycontainer", "myimage", tb.Command("/mycmd"), tb.Args( @@ -280,7 +280,7 @@ func TestReconcile(t *testing.T) { tb.TaskRunTaskSpec( tb.TaskInputs( tb.InputsResource("workspace", v1alpha1.PipelineResourceTypeGit), - tb.InputsParam("myarg", tb.ParamDefault("mydefault")), + tb.InputsParam("myarg", v1alpha1.ParamTypeString, tb.ParamDefault("mydefault")), ), tb.Step("mycontainer", "myimage", tb.Command("/mycmd"), tb.Args("--my-arg=${inputs.params.myarg}"), diff --git a/pkg/reconciler/v1alpha1/taskrun/timeout_check_test.go b/pkg/reconciler/v1alpha1/taskrun/timeout_check_test.go index 55991e330a2..6187ddcb9ed 100644 --- a/pkg/reconciler/v1alpha1/taskrun/timeout_check_test.go +++ b/pkg/reconciler/v1alpha1/taskrun/timeout_check_test.go @@ -37,22 +37,19 @@ func TestCheckTimeout(t *testing.T) { name: "TaskRun not started", taskRun: tb.TaskRun("test-taskrun-not-started", "foo", tb.TaskRunSpec( tb.TaskRunTaskRef(simpleTask.Name), - ), tb.TaskRunStatus(tb.Condition(apis.Condition{ - }), tb.TaskRunStartTime(zeroTime))), + ), tb.TaskRunStatus(tb.Condition(apis.Condition{}), tb.TaskRunStartTime(zeroTime))), expectedStatus: false, }, { name: "TaskRun no timeout", taskRun: tb.TaskRun("test-taskrun-no-timeout", "foo", tb.TaskRunSpec( tb.TaskRunTaskRef(simpleTask.Name), tb.TaskRunTimeout(0), - ), tb.TaskRunStatus(tb.Condition(apis.Condition{ - }), tb.TaskRunStartTime(time.Now().Add(-15 * time.Hour)))), + ), tb.TaskRunStatus(tb.Condition(apis.Condition{}), tb.TaskRunStartTime(time.Now().Add(-15*time.Hour)))), expectedStatus: false, }, { name: "TaskRun timed out", taskRun: tb.TaskRun("test-taskrun-timeout", "foo", tb.TaskRunSpec( - tb.TaskRunTaskRef(simpleTask.Name), tb.TaskRunTimeout(10 * time.Second), - ), tb.TaskRunStatus(tb.Condition(apis.Condition{ - }), tb.TaskRunStartTime(time.Now().Add(-15 * time.Second)))), + tb.TaskRunTaskRef(simpleTask.Name), tb.TaskRunTimeout(10*time.Second), + ), tb.TaskRunStatus(tb.Condition(apis.Condition{}), tb.TaskRunStartTime(time.Now().Add(-15*time.Second)))), expectedStatus: true, }} diff --git a/pkg/reconciler/v1alpha1/taskrun/validate_resources.go b/pkg/reconciler/v1alpha1/taskrun/validate_resources.go index cce822f4be1..32ba9a7810f 100644 --- a/pkg/reconciler/v1alpha1/taskrun/validate_resources.go +++ b/pkg/reconciler/v1alpha1/taskrun/validate_resources.go @@ -63,12 +63,14 @@ func validateResources(requiredResources []v1alpha1.TaskResource, providedResour return nil } -func validateParams(inputs *v1alpha1.Inputs, params []v1alpha1.Param) error { - neededParams := []string{} +func validateParams(inputs *v1alpha1.Inputs, params []v1alpha1.ArrayOrStringParam) error { + var neededParams []string + paramTypes := make(map[string]v1alpha1.ParamType) if inputs != nil { neededParams = make([]string, 0, len(inputs.Params)) for _, inputResourceParam := range inputs.Params { neededParams = append(neededParams, inputResourceParam.Name) + paramTypes[inputResourceParam.Name] = inputResourceParam.Type } } providedParams := make([]string, 0, len(params)) @@ -76,10 +78,10 @@ func validateParams(inputs *v1alpha1.Inputs, params []v1alpha1.Param) error { providedParams = append(providedParams, param.Name) } missingParams := list.DiffLeft(neededParams, providedParams) - missingParamsNoDefaults := []string{} + var missingParamsNoDefaults []string for _, param := range missingParams { for _, inputResourceParam := range inputs.Params { - if inputResourceParam.Name == param && inputResourceParam.Default == "" { + if inputResourceParam.Name == param && inputResourceParam.Default == nil { missingParamsNoDefaults = append(missingParamsNoDefaults, param) } } @@ -91,11 +93,24 @@ func validateParams(inputs *v1alpha1.Inputs, params []v1alpha1.Param) error { if len(extraParams) != 0 { return xerrors.Errorf("didn't need these params but they were provided anyway: %s", extraParams) } + + // Now that we have checked against missing/extra params, make sure each param's actual type matches + // the user-specified type. + var wrongTypeParamNames []string + for _, param := range params { + if param.Value.Type != paramTypes[param.Name] { + wrongTypeParamNames = append(wrongTypeParamNames, param.Name) + } + } + if len(wrongTypeParamNames) != 0 { + return xerrors.Errorf("param types don't match the user-specified type: %s", wrongTypeParamNames) + } + return nil } // ValidateResolvedTaskResources validates task inputs, params and output matches taskrun -func ValidateResolvedTaskResources(params []v1alpha1.Param, rtr *resources.ResolvedTaskResources) error { +func ValidateResolvedTaskResources(params []v1alpha1.ArrayOrStringParam, rtr *resources.ResolvedTaskResources) error { if err := validateParams(rtr.TaskSpec.Inputs, params); err != nil { return xerrors.Errorf("invalid input params: %w", err) } diff --git a/pkg/reconciler/v1alpha1/taskrun/validate_resources_test.go b/pkg/reconciler/v1alpha1/taskrun/validate_resources_test.go index 4efd4d6c090..091141a879c 100644 --- a/pkg/reconciler/v1alpha1/taskrun/validate_resources_test.go +++ b/pkg/reconciler/v1alpha1/taskrun/validate_resources_test.go @@ -39,7 +39,7 @@ func TestValidateResolvedTaskResources_ValidResources(t *testing.T) { tb.ResolvedTaskResourcesOutputs("resource-to-provide", tb.PipelineResource("example-image", "bar", tb.PipelineResourceSpec(v1alpha1.PipelineResourceTypeImage)), )) - if err := taskrun.ValidateResolvedTaskResources([]v1alpha1.Param{}, rtr); err != nil { + if err := taskrun.ValidateResolvedTaskResources([]v1alpha1.ArrayOrStringParam{}, rtr); err != nil { t.Fatalf("Did not expect to see error when validating valid resolved TaskRun but saw %v", err) } } @@ -47,14 +47,14 @@ func TestValidateResolvedTaskResources_ValidResources(t *testing.T) { func TestValidateResolvedTaskResources_ValidParams(t *testing.T) { rtr := tb.ResolvedTaskResources(tb.ResolvedTaskResourcesTaskSpec( tb.Step("mystep", "myimage", tb.Command("mycmd")), - tb.TaskInputs(tb.InputsParam("foo"), tb.InputsParam("bar")), + tb.TaskInputs(tb.InputsParam("foo", v1alpha1.ParamTypeString), tb.InputsParam("bar", v1alpha1.ParamTypeString)), )) - p := []v1alpha1.Param{{ + p := []v1alpha1.ArrayOrStringParam{{ Name: "foo", - Value: "somethinggood", + Value: *tb.ArrayOrString("somethinggood"), }, { Name: "bar", - Value: "somethinggood", + Value: *tb.ArrayOrString("somethinggood"), }} if err := taskrun.ValidateResolvedTaskResources(p, rtr); err != nil { t.Fatalf("Did not expect to see error when validating TaskRun with correct params but saw %v", err) @@ -65,29 +65,29 @@ func TestValidateResolvedTaskResources_InvalidParams(t *testing.T) { tcs := []struct { name string rtr *resources.ResolvedTaskResources - params []v1alpha1.Param + params []v1alpha1.ArrayOrStringParam }{{ name: "missing-params", rtr: tb.ResolvedTaskResources(tb.ResolvedTaskResourcesTaskSpec( tb.Step("mystep", "myimage", tb.Command("mycmd")), - tb.TaskInputs(tb.InputsParam("foo")), + tb.TaskInputs(tb.InputsParam("foo", v1alpha1.ParamTypeString)), )), - params: []v1alpha1.Param{{ + params: []v1alpha1.ArrayOrStringParam{{ Name: "foobar", - Value: "somethingfun", + Value: *tb.ArrayOrString("somethingfun"), }}, }, { name: "missing-params", rtr: tb.ResolvedTaskResources(tb.ResolvedTaskResourcesTaskSpec( tb.Step("mystep", "myimage", tb.Command("mycmd")), - tb.TaskInputs(tb.InputsParam("foo")), + tb.TaskInputs(tb.InputsParam("foo", v1alpha1.ParamTypeString)), )), - params: []v1alpha1.Param{{ + params: []v1alpha1.ArrayOrStringParam{{ Name: "foo", - Value: "i am a real param", + Value: *tb.ArrayOrString("i am a real param"), }, { Name: "extra", - Value: "i am an extra param", + Value: *tb.ArrayOrString("i am an extra param"), }}, }} for _, tc := range tcs { @@ -169,7 +169,7 @@ func TestValidateResolvedTaskResources_InvalidResources(t *testing.T) { for _, tc := range tcs { t.Run(tc.name, func(t *testing.T) { - if err := taskrun.ValidateResolvedTaskResources([]v1alpha1.Param{}, tc.rtr); err == nil { + if err := taskrun.ValidateResolvedTaskResources([]v1alpha1.ArrayOrStringParam{}, tc.rtr); err == nil { t.Errorf("Expected to see error when validating invalid resolved TaskRun but saw none") } }) diff --git a/pkg/status/taskrunpod.go b/pkg/status/taskrunpod.go index 162f1f99584..4a956294e67 100644 --- a/pkg/status/taskrunpod.go +++ b/pkg/status/taskrunpod.go @@ -64,8 +64,8 @@ func updateCompletedTaskRun(taskRun *v1alpha1.TaskRun, pod *corev1.Pod) { }) } else { taskRun.Status.SetCondition(&apis.Condition{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionTrue, + Type: apis.ConditionSucceeded, + Status: corev1.ConditionTrue, Reason: ReasonSucceeded, Message: "All Steps have completed executing", }) @@ -78,9 +78,9 @@ func updateIncompleteTaskRun(taskRun *v1alpha1.TaskRun, pod *corev1.Pod) { switch pod.Status.Phase { case corev1.PodRunning: taskRun.Status.SetCondition(&apis.Condition{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionUnknown, - Reason: ReasonBuilding, + Type: apis.ConditionSucceeded, + Status: corev1.ConditionUnknown, + Reason: ReasonBuilding, Message: "Not all Steps in the Task have finished executing", }) case corev1.PodPending: diff --git a/pkg/status/taskrunpod_test.go b/pkg/status/taskrunpod_test.go index 3218a0bc7d2..2d5518a839b 100644 --- a/pkg/status/taskrunpod_test.go +++ b/pkg/status/taskrunpod_test.go @@ -28,15 +28,15 @@ func TestUpdateStatusFromPod(t *testing.T) { Message: ReasonRunning, } conditionTrue := apis.Condition{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionTrue, + Type: apis.ConditionSucceeded, + Status: corev1.ConditionTrue, Reason: ReasonSucceeded, Message: "All Steps have completed executing", } conditionBuilding := apis.Condition{ - Type: apis.ConditionSucceeded, - Status: corev1.ConditionUnknown, - Reason: ReasonBuilding, + Type: apis.ConditionSucceeded, + Status: corev1.ConditionUnknown, + Reason: ReasonBuilding, Message: "Not all Steps in the Task have finished executing", } for _, c := range []struct { diff --git a/pkg/templating/templating.go b/pkg/templating/templating.go index aad09e73b7f..abf192d0c67 100644 --- a/pkg/templating/templating.go +++ b/pkg/templating/templating.go @@ -40,6 +40,51 @@ func ValidateVariable(name, value, prefix, contextPrefix, locationName, path str return nil } +// Verifies that variables matching the relevant string expressions do not reference any of the names present in vars. +func ValidateVariableProhibited(name, value, prefix, contextPrefix, locationName, path string, vars map[string]struct{}) *apis.FieldError { + if vs, present := extractVariablesFromString(value, contextPrefix+prefix); present { + for _, v := range vs { + if _, ok := vars[v]; ok { + return &apis.FieldError{ + Message: fmt.Sprintf("variable type invalid in %q for %s %s", value, locationName, name), + Paths: []string{path + "." + name}, + } + } + } + } + return nil +} + +// Verifies that variables matching the relevant string expressions are completely isolated if present. +func ValidateVariableIsolated(name, value, prefix, contextPrefix, locationName, path string, vars map[string]struct{}) *apis.FieldError { + if vs, present := extractVariablesFromString(value, contextPrefix+prefix); present { + firstMatch, _ := extractExpressionFromString(value, contextPrefix+prefix) + for _, v := range vs { + if _, ok := vars[v]; ok { + if len(value) != len(firstMatch) { + return &apis.FieldError{ + Message: fmt.Sprintf("variable is not properly isolated in %q for %s %s", value, locationName, name), + Paths: []string{path + "." + name}, + } + } + } + } + } + return nil +} + +// Extract a the first full string expressions found (e.g "${input.params.foo}"). Return +// "" and false if nothing is found. +func extractExpressionFromString(s, prefix string) (string, bool) { + pattern := fmt.Sprintf("\\$({%s.(?P%s)})", prefix, parameterSubstitution) + re := regexp.MustCompile(pattern) + match := re.FindStringSubmatch(s) + if match == nil { + return "", false + } + return match[0], true +} + func extractVariablesFromString(s, prefix string) ([]string, bool) { pattern := fmt.Sprintf("\\$({%s.(?P%s)})", prefix, parameterSubstitution) re := regexp.MustCompile(pattern) @@ -72,3 +117,21 @@ func ApplyReplacements(in string, replacements map[string]string) string { } return in } + +// Take an input string, and output an array of strings related to possible arrayReplacements. If there aren't any +// areas where the input can be split up via arrayReplacements, then just return an array with a single element, +// which is ApplyReplacements(in, replacements). +func ApplyArrayReplacements(in string, stringReplacements map[string]string, arrayReplacements map[string][]string) []string { + for k, v := range arrayReplacements { + stringToReplace := fmt.Sprintf("${%s}", k) + + // If the input string matches a replacement's key (without padding characters), return the corresponding array. + // Note that the webhook should prevent all instances where this could evaluate to false. + if (strings.Count(in, stringToReplace) == 1) && len(in) == len(stringToReplace) { + return v + } + } + + // Otherwise return a size-1 array containing the input string with standard stringReplacements applied. + return []string{ApplyReplacements(in, stringReplacements)} +} diff --git a/pkg/templating/templating_test.go b/pkg/templating/templating_test.go index 5a5caa135b1..82362dc4853 100644 --- a/pkg/templating/templating_test.go +++ b/pkg/templating/templating_test.go @@ -126,3 +126,122 @@ func TestValidateVariables(t *testing.T) { }) } } + +func TestApplyReplacements(t *testing.T) { + type args struct { + input string + replacements map[string]string + } + tests := []struct { + name string + args args + expectedOutput string + }{ + { + name: "no replacements requested", + args: args{ + input: "this is a string", + replacements: map[string]string{}, + }, + expectedOutput: "this is a string", + }, + { + name: "single replacement requested", + args: args{ + input: "this is ${a} string", + replacements: map[string]string{"a": "not a"}, + }, + expectedOutput: "this is not a string", + }, + { + name: "single replacement requested multiple matches", + args: args{ + input: "this ${is} a string ${is} a string", + replacements: map[string]string{"is": "a"}, + }, + expectedOutput: "this a a string a a string", + }, + { + name: "multiple replacements requested", + args: args{ + input: "this ${is} a ${string} ${is} a ${string}", + replacements: map[string]string{"is": "a", "string": "sstring"}, + }, + expectedOutput: "this a a sstring a a sstring", + }, + { + name: "multiple replacements requested nothing replaced", + args: args{ + input: "this is a string", + replacements: map[string]string{"this": "a", "evxasdd": "string"}, + }, + expectedOutput: "this is a string", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + actualOutput := templating.ApplyReplacements(tt.args.input, tt.args.replacements) + if d := cmp.Diff(actualOutput, tt.expectedOutput); d != "" { + t.Errorf("ApplyReplacements() output did not match expected value %s", d) + } + }) + } +} + +func TestApplyArrayReplacements(t *testing.T) { + type args struct { + input string + stringReplacements map[string]string + arrayReplacements map[string][]string + } + tests := []struct { + name string + args args + expectedOutput []string + }{ + { + name: "no replacements requested", + args: args{ + input: "this is a string", + stringReplacements: map[string]string{}, + arrayReplacements: map[string][]string{}, + }, + expectedOutput: []string{"this is a string"}, + }, + { + name: "multiple replacements requested nothing replaced", + args: args{ + input: "this is a string", + stringReplacements: map[string]string{"key": "replacement", "anotherkey": "foo"}, + arrayReplacements: map[string][]string{"key2": {"replacement", "a"}, "key3": {"1", "2"}}, + }, + expectedOutput: []string{"this is a string"}, + }, + { + name: "multiple replacements only string replacement possible", + args: args{ + input: "${string}rep${lacement}${string}", + stringReplacements: map[string]string{"string": "word", "lacement": "lacements"}, + arrayReplacements: map[string][]string{"ace": {"replacement", "a"}, "string": {"1", "2"}}, + }, + expectedOutput: []string{"wordreplacementsword"}, + }, + { + name: "array replacement", + args: args{ + input: "${match}", + stringReplacements: map[string]string{"string": "word", "lacement": "lacements"}, + arrayReplacements: map[string][]string{"ace": {"replacement", "a"}, "match": {"1", "2"}}, + }, + expectedOutput: []string{"1", "2"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + actualOutput := templating.ApplyArrayReplacements(tt.args.input, tt.args.stringReplacements, tt.args.arrayReplacements) + if d := cmp.Diff(actualOutput, tt.expectedOutput); d != "" { + t.Errorf("ApplyArrayReplacements() output did not match expected value %s", d) + } + }) + } +} diff --git a/test/builder/examples_test.go b/test/builder/examples_test.go index 68dcbd2280c..5b237b74557 100644 --- a/test/builder/examples_test.go +++ b/test/builder/examples_test.go @@ -80,7 +80,7 @@ func ExampleTaskRun() { tb.TaskRunTaskSpec( tb.TaskInputs( tb.InputsResource("workspace", v1alpha1.PipelineResourceTypeGit), - tb.InputsParam("myarg", tb.ParamDefault("mydefault")), + tb.InputsParam("myarg", v1alpha1.ParamTypeString, tb.ParamDefault("mydefault")), ), tb.Step("mycontainer", "myimage", tb.Command("/mycmd"), tb.Args("--my-arg=${inputs.params.myarg}"), diff --git a/test/builder/param.go b/test/builder/param.go new file mode 100644 index 00000000000..59da14f29aa --- /dev/null +++ b/test/builder/param.go @@ -0,0 +1,32 @@ +/* +Copyright 2019 The Tekton Authors +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package builder + +import "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" + +// ArrayOrString creates an ArrayOrString of type ParamTypeString or ParamTypeArray, based on +// how many inputs are given (>1 input will create an array, not string). +func ArrayOrString(value string, additionalValues ...string) *v1alpha1.ArrayOrString { + if len(additionalValues) > 0 { + additionalValues = append([]string{value}, additionalValues...) + return &v1alpha1.ArrayOrString{ + Type: v1alpha1.ParamTypeArray, + ArrayVal: additionalValues, + } + } + return &v1alpha1.ArrayOrString{ + Type: v1alpha1.ParamTypeString, + StringVal: value, + } +} diff --git a/test/builder/param_test.go b/test/builder/param_test.go new file mode 100644 index 00000000000..2d120c9989c --- /dev/null +++ b/test/builder/param_test.go @@ -0,0 +1,44 @@ +/* +Copyright 2019 The Tekton Authors +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + http://www.apache.org/licenses/LICENSE-2.0 +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package builder_test + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1" + "github.com/tektoncd/pipeline/test/builder" +) + +func TestGenerateString(t *testing.T) { + value := builder.ArrayOrString("somestring") + expectedValue := &v1alpha1.ArrayOrString{ + Type: v1alpha1.ParamTypeString, + StringVal: "somestring", + } + if d := cmp.Diff(expectedValue, value); d != "" { + t.Fatalf("ArrayOrString diff -want, +got: %v", d) + } +} + +func TestGenerateArray(t *testing.T) { + value := builder.ArrayOrString("some", "array", "elements") + expectedValue := &v1alpha1.ArrayOrString{ + Type: v1alpha1.ParamTypeArray, + ArrayVal: []string{"some", "array", "elements"}, + } + if d := cmp.Diff(expectedValue, value); d != "" { + t.Fatalf("ArrayOrString diff -want, +got: %v", d) + } +} diff --git a/test/builder/pipeline.go b/test/builder/pipeline.go index 8d667d335e2..bcc04c3189f 100644 --- a/test/builder/pipeline.go +++ b/test/builder/pipeline.go @@ -111,11 +111,11 @@ func PipelineDeclaredResource(name string, t v1alpha1.PipelineResourceType) Pipe } } -// ParamSpec adds a param, with specified name, to the Spec. +// ParamSpec adds a param, with specified name and type, to the Spec. // Any number of ParamSpec modifiers can be passed to transform it. -func PipelineParam(name string, ops ...PipelineParamOp) PipelineSpecOp { +func PipelineParam(name string, pt v1alpha1.ParamType, ops ...PipelineParamOp) PipelineSpecOp { return func(ps *v1alpha1.PipelineSpec) { - pp := &v1alpha1.ParamSpec{Name: name} + pp := &v1alpha1.ParamSpec{Name: name, Type: pt} for _, op := range ops { op(pp) } @@ -131,9 +131,10 @@ func PipelineParamDescription(desc string) PipelineParamOp { } // PipelineParamDefault sets the default value to the ParamSpec. -func PipelineParamDefault(value string) PipelineParamOp { +func PipelineParamDefault(value string, additionalValues ...string) PipelineParamOp { + arrayOrString := ArrayOrString(value, additionalValues...) return func(pp *v1alpha1.ParamSpec) { - pp.Default = value + pp.Default = arrayOrString } } @@ -176,11 +177,12 @@ func PipelineTaskRefKind(kind v1alpha1.TaskKind) PipelineTaskOp { } // PipelineTaskParam adds a Param, with specified name and value, to the PipelineTask. -func PipelineTaskParam(name, value string) PipelineTaskOp { +func PipelineTaskParam(name string, value string, additionalValues ...string) PipelineTaskOp { + arrayOrString := ArrayOrString(value, additionalValues...) return func(pt *v1alpha1.PipelineTask) { - pt.Params = append(pt.Params, v1alpha1.Param{ + pt.Params = append(pt.Params, v1alpha1.ArrayOrStringParam{ Name: name, - Value: value, + Value: *arrayOrString, }) } } @@ -324,11 +326,12 @@ func PipelineRunServiceAccountTask(taskName, sa string) PipelineRunSpecOp { } // PipelineRunParam add a param, with specified name and value, to the PipelineRunSpec. -func PipelineRunParam(name, value string) PipelineRunSpecOp { +func PipelineRunParam(name string, value string, additionalValues ...string) PipelineRunSpecOp { + arrayOrString := ArrayOrString(value, additionalValues...) return func(prs *v1alpha1.PipelineRunSpec) { - prs.Params = append(prs.Params, v1alpha1.Param{ + prs.Params = append(prs.Params, v1alpha1.ArrayOrStringParam{ Name: name, - Value: value, + Value: *arrayOrString, }) } } diff --git a/test/builder/pipeline_test.go b/test/builder/pipeline_test.go index c24d1a9a0a0..0fbbf5f8435 100644 --- a/test/builder/pipeline_test.go +++ b/test/builder/pipeline_test.go @@ -31,9 +31,10 @@ func TestPipeline(t *testing.T) { pipeline := tb.Pipeline("tomatoes", "foo", tb.PipelineSpec( tb.PipelineDeclaredResource("my-only-git-resource", "git"), tb.PipelineDeclaredResource("my-only-image-resource", "image"), - tb.PipelineParam("first-param", tb.PipelineParamDefault("default-value"), tb.PipelineParamDescription("default description")), + tb.PipelineParam("first-param", v1alpha1.ParamTypeString, tb.PipelineParamDefault("default-value"), tb.PipelineParamDescription("default description")), tb.PipelineTask("foo", "banana", - tb.PipelineTaskParam("name", "value"), + tb.PipelineTaskParam("stringparam", "value"), + tb.PipelineTaskParam("arrayparam", "array", "value"), ), tb.PipelineTask("bar", "chocolate", tb.PipelineTaskRefKind(v1alpha1.ClusterTaskKind), @@ -61,13 +62,20 @@ func TestPipeline(t *testing.T) { }}, Params: []v1alpha1.ParamSpec{{ Name: "first-param", - Default: "default-value", + Type: v1alpha1.ParamTypeString, + Default: tb.ArrayOrString("default-value"), Description: "default description", }}, Tasks: []v1alpha1.PipelineTask{{ Name: "foo", TaskRef: v1alpha1.TaskRef{Name: "banana"}, - Params: []v1alpha1.Param{{Name: "name", Value: "value"}}, + Params: []v1alpha1.ArrayOrStringParam{{ + Name: "stringparam", + Value: *tb.ArrayOrString("value"), + }, { + Name: "arrayparam", + Value: *tb.ArrayOrString("array", "value"), + }}, }, { Name: "bar", TaskRef: v1alpha1.TaskRef{Name: "chocolate", Kind: v1alpha1.ClusterTaskKind}, @@ -100,7 +108,8 @@ func TestPipelineRun(t *testing.T) { pipelineRun := tb.PipelineRun("pear", "foo", tb.PipelineRunSpec( "tomatoes", tb.PipelineRunServiceAccount("sa"), - tb.PipelineRunParam("first-param", "first-value"), + tb.PipelineRunParam("first-param-string", "first-value"), + tb.PipelineRunParam("second-param-array", "some", "array"), tb.PipelineRunTimeout(1*time.Hour), tb.PipelineRunResourceBinding("some-resource", tb.PipelineResourceBindingRef("my-special-resource")), tb.PipelineRunServiceAccountTask("foo", "sa-2"), @@ -121,9 +130,12 @@ func TestPipelineRun(t *testing.T) { PipelineRef: v1alpha1.PipelineRef{Name: "tomatoes"}, ServiceAccount: "sa", ServiceAccounts: []v1alpha1.PipelineRunSpecServiceAccount{{TaskName: "foo", ServiceAccount: "sa-2"}}, - Params: []v1alpha1.Param{{ - Name: "first-param", - Value: "first-value", + Params: []v1alpha1.ArrayOrStringParam{{ + Name: "first-param-string", + Value: *tb.ArrayOrString("first-value"), + }, { + Name: "second-param-array", + Value: *tb.ArrayOrString("some", "array"), }}, Timeout: &metav1.Duration{Duration: 1 * time.Hour}, Resources: []v1alpha1.PipelineResourceBinding{{ diff --git a/test/builder/task.go b/test/builder/task.go index c2e3b6e576a..c6e22e1d8bd 100644 --- a/test/builder/task.go +++ b/test/builder/task.go @@ -251,9 +251,9 @@ func OutputsResource(name string, resourceType v1alpha1.PipelineResourceType) Ou // InputsParam adds a param, with specified name, to the Inputs. // Any number of ParamSpec modifier can be passed to transform it. -func InputsParam(name string, ops ...TaskParamOp) InputsOp { +func InputsParam(name string, pt v1alpha1.ParamType, ops ...TaskParamOp) InputsOp { return func(i *v1alpha1.Inputs) { - tp := &v1alpha1.ParamSpec{Name: name} + tp := &v1alpha1.ParamSpec{Name: name, Type: pt} for _, op := range ops { op(tp) } @@ -269,9 +269,10 @@ func ParamDescription(desc string) TaskParamOp { } // ParamDefault sets the default value to the ParamSpec. -func ParamDefault(value string) TaskParamOp { +func ParamDefault(value string, additionalValues ...string) TaskParamOp { + arrayOrString := ArrayOrString(value, additionalValues...) return func(tp *v1alpha1.ParamSpec) { - tp.Default = value + tp.Default = arrayOrString } } @@ -509,11 +510,12 @@ func TaskRunInputs(ops ...TaskRunInputsOp) TaskRunSpecOp { } // TaskRunInputsParam add a param, with specified name and value, to the TaskRunInputs. -func TaskRunInputsParam(name, value string) TaskRunInputsOp { +func TaskRunInputsParam(name, value string, additionalValues ...string) TaskRunInputsOp { + arrayOrString := ArrayOrString(value, additionalValues...) return func(i *v1alpha1.TaskRunInputs) { - i.Params = append(i.Params, v1alpha1.Param{ + i.Params = append(i.Params, v1alpha1.ArrayOrStringParam{ Name: name, - Value: value, + Value: *arrayOrString, }) } } diff --git a/test/builder/task_test.go b/test/builder/task_test.go index 3e6cf8caf2e..8af3a496a68 100644 --- a/test/builder/task_test.go +++ b/test/builder/task_test.go @@ -41,7 +41,8 @@ func TestTask(t *testing.T) { task := tb.Task("test-task", "foo", tb.TaskSpec( tb.TaskInputs( tb.InputsResource("workspace", v1alpha1.PipelineResourceTypeGit, tb.ResourceTargetPath("/foo/bar")), - tb.InputsParam("param", tb.ParamDescription("mydesc"), tb.ParamDefault("default")), + tb.InputsParam("param", v1alpha1.ParamTypeString, tb.ParamDescription("mydesc"), tb.ParamDefault("default")), + tb.InputsParam("array-param", v1alpha1.ParamTypeString, tb.ParamDescription("desc"), tb.ParamDefault("array", "values")), ), tb.TaskOutputs(tb.OutputsResource("myotherimage", v1alpha1.PipelineResourceTypeImage)), tb.Step("mycontainer", "myimage", tb.Command("/mycmd"), tb.Args( @@ -73,8 +74,17 @@ func TestTask(t *testing.T) { Type: v1alpha1.PipelineResourceTypeGit, TargetPath: "/foo/bar", }}, - Params: []v1alpha1.ParamSpec{{Name: "param", Description: "mydesc", Default: "default"}}, - }, + Params: []v1alpha1.ParamSpec{{ + Name: "param", + Type: v1alpha1.ParamTypeString, + Description: "mydesc", + Default: tb.ArrayOrString("default"), + }, { + Name: "array-param", + Type: v1alpha1.ParamTypeString, + Description: "desc", + Default: tb.ArrayOrString("array", "values"), + }}}, Outputs: &v1alpha1.Outputs{ Resources: []v1alpha1.TaskResource{{ Name: "myotherimage", @@ -153,6 +163,7 @@ func TestTaskRunWithTaskRef(t *testing.T) { tb.TaskResourceBindingResourceSpec(&v1alpha1.PipelineResourceSpec{Type: v1alpha1.PipelineResourceTypeCluster}), ), tb.TaskRunInputsParam("iparam", "ivalue"), + tb.TaskRunInputsParam("arrayparam", "array", "values"), ), tb.TaskRunOutputs( tb.TaskRunOutputsResource(gitResource.Name, @@ -193,7 +204,13 @@ func TestTaskRunWithTaskRef(t *testing.T) { ResourceSpec: &v1alpha1.PipelineResourceSpec{Type: v1alpha1.PipelineResourceType("cluster")}, Paths: []string{"source-folder"}, }}, - Params: []v1alpha1.Param{{Name: "iparam", Value: "ivalue"}}, + Params: []v1alpha1.ArrayOrStringParam{{ + Name: "iparam", + Value: *tb.ArrayOrString("ivalue"), + }, { + Name: "arrayparam", + Value: *tb.ArrayOrString("array", "values"), + }}, }, Outputs: v1alpha1.TaskRunOutputs{ Resources: []v1alpha1.TaskResourceBinding{{ diff --git a/test/dag_test.go b/test/dag_test.go index 1b3f86b05da..7ac7c336e2f 100644 --- a/test/dag_test.go +++ b/test/dag_test.go @@ -49,7 +49,7 @@ func TestDAGPipelineRun(t *testing.T) { echoTask := tb.Task("echo-task", namespace, tb.TaskSpec( tb.TaskInputs( tb.InputsResource("repo", v1alpha1.PipelineResourceTypeGit), - tb.InputsParam("text", tb.ParamDescription("The text that should be echoed")), + tb.InputsParam("text", v1alpha1.ParamTypeString, tb.ParamDescription("The text that should be echoed")), ), tb.TaskOutputs(tb.OutputsResource("repo", v1alpha1.PipelineResourceTypeGit)), tb.Step("echo-text", "busybox", tb.Command("echo"), tb.Args("${inputs.params.text}")), diff --git a/test/helm_task_test.go b/test/helm_task_test.go index 07832476712..1ce1af60c61 100644 --- a/test/helm_task_test.go +++ b/test/helm_task_test.go @@ -168,8 +168,8 @@ func getHelmDeployTask(namespace string) *v1alpha1.Task { tb.TaskInputs( tb.InputsResource("gitsource", v1alpha1.PipelineResourceTypeGit), tb.InputsResource("image", v1alpha1.PipelineResourceTypeImage), - tb.InputsParam("pathToHelmCharts", tb.ParamDescription("Path to the helm charts")), - tb.InputsParam("chartname", tb.ParamDefault("")), + tb.InputsParam("pathToHelmCharts", v1alpha1.ParamTypeString, tb.ParamDescription("Path to the helm charts")), + tb.InputsParam("chartname", v1alpha1.ParamTypeString, tb.ParamDefault("")), ), tb.Step("helm-init", "alpine/helm:2.14.0", tb.Args("init", "--wait")), tb.Step("helm-deploy", "alpine/helm:2.14.0", tb.Args( @@ -187,7 +187,7 @@ func getHelmDeployPipeline(namespace string) *v1alpha1.Pipeline { return tb.Pipeline(helmDeployPipelineName, namespace, tb.PipelineSpec( tb.PipelineDeclaredResource("git-repo", "git"), tb.PipelineDeclaredResource("the-image", "image"), - tb.PipelineParam("chartname"), + tb.PipelineParam("chartname", v1alpha1.ParamTypeString), tb.PipelineTask("push-image", createImageTaskName, tb.PipelineTaskInputResource("gitsource", "git-repo"), tb.PipelineTaskOutputResource("builtimage", "the-image"),