forked from tektoncd/pipeline
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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 tektoncd#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.
- Loading branch information
Showing
39 changed files
with
1,789 additions
and
215 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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)) | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.