From d07e74fd126a5c21fee99774ee3e4545b95be80b Mon Sep 17 00:00:00 2001 From: Eli Zucker Date: Tue, 16 Jul 2019 00:13:59 -0400 Subject: [PATCH] Expand parameters to support the Array type. Instead of just strings, add support for users to supply parameters in the form of an array. Also include appropriate type validation and defaulting to support backwards-compatibility. Fixes #207. The size of a user-supplied array is not specified anywhere in the corresponding ParamSpec, which is useful for defining tasks that require a dynamic number of strings used as distinct elements in an array. For instance, one might use an array parameter in an image-building task that feeds in a dynamic number of flags via the args field for a particular step. The implementation is defined such that an Array parameter can only be used if the replacement string is completely isolated and within a field that is an array of strings (currently eligible fields are 'command' and 'args'). For instance, the webhook will prevent an array parameter from being used in the 'image' field. Similarly, an array parameter used in a composite string, like 'value=${array}', is invalid and will be caught. The decision was made to completely prevent any use of array parameters as strings because it clutters Tekton with unnecessary functionality and is naturally extensible beyond the intended scope. For instance, if the behavior was defined such that array parameters used in the context of strings were automatically converted by separating them with a space, then that relatively arbitrary implementation could be questioned and eventually lead to needing to specify a custom delimiter (like commas), and so on. Another implementation detail to note is that the ArrayOrString type was necessary, in combination with implementing json.Marshaller, to support backwards-compatibility with strings. This is based off IntOrString from kubernetes/apimachinery, and allows the value-type to be immediately parsed and decided through the webhook. Lastly, a possibly unintuitive design decision is that if an EMPTY array parameter properly replaces an array element, then that string in the original array will be completely removed. --- pkg/apis/pipeline/v1alpha1/param_types.go | 93 ++++++- .../pipeline/v1alpha1/param_types_test.go | 189 +++++++++++++ .../pipeline/v1alpha1/pipeline_defaults.go | 3 + pkg/apis/pipeline/v1alpha1/pipeline_types.go | 2 +- .../pipeline/v1alpha1/pipeline_validation.go | 60 +++- .../v1alpha1/pipeline_validation_test.go | 58 +++- .../pipeline/v1alpha1/pipelinerun_types.go | 2 +- pkg/apis/pipeline/v1alpha1/task_defaults.go | 9 + pkg/apis/pipeline/v1alpha1/task_validation.go | 91 +++++- .../pipeline/v1alpha1/task_validation_test.go | 260 +++++++++++++++++- pkg/apis/pipeline/v1alpha1/taskrun_types.go | 2 +- .../pipeline/v1alpha1/taskrun_validation.go | 2 +- .../v1alpha1/taskrun_validation_test.go | 12 +- .../v1alpha1/zz_generated.deepcopy.go | 69 ++++- .../v1alpha1/pipelinerun/pipelinerun.go | 19 ++ .../v1alpha1/pipelinerun/pipelinerun_test.go | 20 +- .../v1alpha1/pipelinerun/resources/apply.go | 30 +- .../pipelinerun/resources/apply_test.go | 110 +++++--- .../pipelinerun/resources/validate_params.go | 47 ++++ .../resources/validate_params_test.go | 123 +++++++++ .../v1alpha1/taskrun/resources/apply.go | 92 ++++--- .../v1alpha1/taskrun/resources/apply_test.go | 257 +++++++++++++++-- .../v1alpha1/taskrun/taskrun_test.go | 8 +- .../v1alpha1/taskrun/timeout_check_test.go | 11 +- .../v1alpha1/taskrun/validate_resources.go | 25 +- .../taskrun/validate_resources_test.go | 28 +- pkg/status/taskrunpod.go | 10 +- pkg/status/taskrunpod_test.go | 10 +- pkg/templating/templating.go | 63 +++++ pkg/templating/templating_test.go | 119 ++++++++ test/builder/examples_test.go | 2 +- test/builder/param.go | 32 +++ test/builder/param_test.go | 44 +++ test/builder/pipeline.go | 25 +- test/builder/pipeline_test.go | 28 +- test/builder/task.go | 16 +- test/builder/task_test.go | 25 +- test/dag_test.go | 2 +- test/helm_task_test.go | 6 +- 39 files changed, 1789 insertions(+), 215 deletions(-) create mode 100644 pkg/apis/pipeline/v1alpha1/param_types_test.go create mode 100644 pkg/reconciler/v1alpha1/pipelinerun/resources/validate_params.go create mode 100644 pkg/reconciler/v1alpha1/pipelinerun/resources/validate_params_test.go create mode 100644 test/builder/param.go create mode 100644 test/builder/param_test.go 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"),