support referencing array params in when expression values

Array params are referenced using $(params.arrayParam[*]), when expressions values is a list of string and was designed to resolve an array parameter. But the implementation was limiting the array param usage in the values. Adding support for the array params in when expressions.
tektoncd · Jul 7, 2021 · 232ec0e · 232ec0e
1 parent c0fae23
commit 232ec0e
Show file tree

Hide file tree

Showing 8 changed files with 130 additions and 29 deletions.
diff --git a/docs/pipelines.md b/docs/pipelines.md
@@ -451,6 +451,15 @@ tasks:
         values: ["true"]
     taskRef:
       name: lint-source
+---
+tasks:
+  - name: deploy-in-blue
+    when:
+      - input: "blue"
+        operator: in
+        values: ["$(params.deployments[*])"]
+    taskRef:
+      name: deployment
 ```
 
 For an end-to-end example, see [PipelineRun with WhenExpressions](../examples/v1beta1/pipelineruns/pipelinerun-with-when-expressions.yaml).

diff --git a/examples/v1beta1/pipelineruns/pipelinerun-with-when-expressions.yaml b/examples/v1beta1/pipelineruns/pipelinerun-with-when-expressions.yaml
@@ -9,6 +9,9 @@ spec:
       - name: path
         type: string
         description: The path of the file to be created
+      - name: branches
+        type: array
+        description: The list of branch names
     workspaces:
       - name: source
         description: |
@@ -67,6 +70,16 @@ spec:
             - name: echo
               image: ubuntu
               script: 'echo file exists'
+      - name: sample-task-with-array-values
+        when:
+          - input: "main"
+            operator: in
+            values: ["$(params.branches[*])"]
+        taskSpec:
+          steps:
+            - name: echo
+              image: alpine
+              script: 'echo hello'
       - name: task-should-be-skipped-1
         when:
           - input: "$(tasks.check-file.results.exists)" # when expression using task result, evaluates to false
@@ -99,6 +112,16 @@ spec:
             - name: echo
               image: ubuntu
               script: exit 1
+      - name: task-should-be-skipped-4 # task with when expression using array parameter, evaluates to false
+        when:
+          - input: "master"
+            operator: in
+            values: ["$(params.branches[*])"]
+        taskSpec:
+          steps:
+            - name: echo
+              image: alpine
+              script: exit 1
     finally:
       - name: finally-task-should-be-skipped-1 # when expression using execution status, evaluates to false
         when:
@@ -162,6 +185,10 @@ spec:
   params:
     - name: path
       value: README.md
+    - name: branches
+      value:
+        - main
+        - hotfix
   workspaces:
     - name: source
       volumeClaimTemplate:

diff --git a/pkg/apis/pipeline/v1beta1/param_types.go b/pkg/apis/pipeline/v1beta1/param_types.go
@@ -148,22 +148,22 @@ func NewArrayOrString(value string, values ...string) *ArrayOrString {
 func validatePipelineParametersVariablesInTaskParameters(params []Param, prefix string, paramNames sets.String, arrayParamNames sets.String) (errs *apis.FieldError) {
 	for _, param := range params {
 		if param.Value.Type == ParamTypeString {
-			errs = errs.Also(validateStringVariableInTaskParameters(param.Value.StringVal, prefix, paramNames, arrayParamNames).ViaFieldKey("params", param.Name))
+			errs = errs.Also(validateStringVariable(param.Value.StringVal, prefix, paramNames, arrayParamNames).ViaFieldKey("params", param.Name))
 		} else {
 			for idx, arrayElement := range param.Value.ArrayVal {
-				errs = errs.Also(validateArrayVariableInTaskParameters(arrayElement, prefix, paramNames, arrayParamNames).ViaFieldIndex("value", idx).ViaFieldKey("params", param.Name))
+				errs = errs.Also(validateArrayVariable(arrayElement, prefix, paramNames, arrayParamNames).ViaFieldIndex("value", idx).ViaFieldKey("params", param.Name))
 			}
 		}
 	}
 	return errs
 }
 
-func validateStringVariableInTaskParameters(value, prefix string, stringVars sets.String, arrayVars sets.String) *apis.FieldError {
+func validateStringVariable(value, prefix string, stringVars sets.String, arrayVars sets.String) *apis.FieldError {
 	errs := substitution.ValidateVariableP(value, prefix, stringVars)
 	return errs.Also(substitution.ValidateVariableProhibitedP(value, prefix, arrayVars))
 }
 
-func validateArrayVariableInTaskParameters(value, prefix string, stringVars sets.String, arrayVars sets.String) *apis.FieldError {
+func validateArrayVariable(value, prefix string, stringVars sets.String, arrayVars sets.String) *apis.FieldError {
 	errs := substitution.ValidateVariableP(value, prefix, stringVars)
 	return errs.Also(substitution.ValidateVariableIsolatedP(value, prefix, arrayVars))
 }
diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go b/pkg/apis/pipeline/v1beta1/pipeline_validation_test.go
@@ -1261,6 +1261,22 @@ func TestValidatePipelineParameterVariables_Success(t *testing.T) {
 				Values:   []string{"$(params.foo-is-baz)"},
 			}},
 		}},
+	}, {
+		name: "valid string parameter variables in input, array reference in values in when expression",
+		params: []ParamSpec{{
+			Name: "baz", Type: ParamTypeString,
+		}, {
+			Name: "foo", Type: ParamTypeArray, Default: &ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"anarray", "elements"}},
+		}},
+		tasks: []PipelineTask{{
+			Name:    "bar",
+			TaskRef: &TaskRef{Name: "bar-task"},
+			WhenExpressions: []WhenExpression{{
+				Input:    "$(params.baz)",
+				Operator: selection.In,
+				Values:   []string{"$(params.foo[*])"},
+			}},
+		}},
 	}, {
 		name: "valid array parameter variables",
 		params: []ParamSpec{{
@@ -1380,22 +1396,22 @@ func TestValidatePipelineParameterVariables_Failure(t *testing.T) {
 			Paths:   []string{"[0].when[0].input"},
 		},
 	}, {
-		name: "invalid string parameter variables in when expression, array reference in values",
+		name: "Invalid array parameter variable in when expression, array reference in input with array notation [*]",
 		params: []ParamSpec{{
 			Name: "foo", Type: ParamTypeArray, Default: &ArrayOrString{Type: ParamTypeArray, ArrayVal: []string{"anarray", "elements"}},
 		}},
 		tasks: []PipelineTask{{
 			Name:    "bar",
 			TaskRef: &TaskRef{Name: "bar-task"},
 			WhenExpressions: []WhenExpression{{
-				Input:    "bax",
+				Input:    "$(params.foo)[*]",
 				Operator: selection.In,
-				Values:   []string{"$(params.foo)"},
+				Values:   []string{"$(params.foo[*])"},
 			}},
 		}},
 		expectedError: apis.FieldError{
-			Message: `variable type invalid in "$(params.foo)"`,
-			Paths:   []string{"[0].when[0].values"},
+			Message: `variable type invalid in "$(params.foo)[*]"`,
+			Paths:   []string{"[0].when[0].input"},
 		},
 	}, {
 		name: "invalid pipeline task with a parameter combined with missing param from the param declarations",

diff --git a/pkg/apis/pipeline/v1beta1/when_types.go b/pkg/apis/pipeline/v1beta1/when_types.go
@@ -17,6 +17,8 @@ limitations under the License.
 package v1beta1
 
 import (
+	"strings"
+
 	"github.com/tektoncd/pipeline/pkg/substitution"
 	"k8s.io/apimachinery/pkg/selection"
 )
@@ -59,12 +61,19 @@ func (we *WhenExpression) hasVariable() bool {
 	return false
 }
 
-func (we *WhenExpression) applyReplacements(replacements map[string]string) WhenExpression {
+func (we *WhenExpression) applyReplacements(replacements map[string]string, arrayReplacements map[string][]string) WhenExpression {
 	replacedInput := substitution.ApplyReplacements(we.Input, replacements)
 
 	var replacedValues []string
 	for _, val := range we.Values {
-		replacedValues = append(replacedValues, substitution.ApplyReplacements(val, replacements))
+		// arrayReplacements holds a list of array parameters with a pattern - params.arrayParam1
+		// array params are referenced using $(params.arrayParam1[*])
+		// check if the param exist in the arrayReplacements to replace it with a list of values
+		if _, ok := arrayReplacements[strings.TrimSuffix(strings.TrimPrefix(val, "$("), "[*])")]; ok {
+			replacedValues = append(replacedValues, substitution.ApplyArrayReplacements(val, replacements, arrayReplacements)...)
+		} else {
+			replacedValues = append(replacedValues, substitution.ApplyReplacements(val, replacements))
+		}
 	}
 
 	return WhenExpression{Input: replacedInput, Operator: we.Operator, Values: replacedValues}
@@ -109,10 +118,10 @@ func (wes WhenExpressions) HaveVariables() bool {
 
 // ReplaceWhenExpressionsVariables interpolates variables, such as Parameters and Results, in
 // the Input and Values.
-func (wes WhenExpressions) ReplaceWhenExpressionsVariables(replacements map[string]string) WhenExpressions {
+func (wes WhenExpressions) ReplaceWhenExpressionsVariables(replacements map[string]string, arrayReplacements map[string][]string) WhenExpressions {
 	replaced := wes
 	for i := range wes {
-		replaced[i] = wes[i].applyReplacements(replacements)
+		replaced[i] = wes[i].applyReplacements(replacements, arrayReplacements)
 	}
 	return replaced
 }
diff --git a/pkg/apis/pipeline/v1beta1/when_types_test.go b/pkg/apis/pipeline/v1beta1/when_types_test.go
@@ -261,7 +261,7 @@ func TestReplaceWhenExpressionsVariables(t *testing.T) {
 	}}
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			got := tc.whenExpressions.ReplaceWhenExpressionsVariables(tc.replacements)
+			got := tc.whenExpressions.ReplaceWhenExpressionsVariables(tc.replacements, nil)
 			if d := cmp.Diff(tc.expected, got); d != "" {
 				t.Errorf("Error evaluating When Expressions in test case %s", diff.PrintWantGot(d))
 			}
@@ -271,10 +271,11 @@ func TestReplaceWhenExpressionsVariables(t *testing.T) {
 
 func TestApplyReplacements(t *testing.T) {
 	tests := []struct {
-		name         string
-		original     *WhenExpression
-		replacements map[string]string
-		expected     *WhenExpression
+		name              string
+		original          *WhenExpression
+		replacements      map[string]string
+		arrayReplacements map[string][]string
+		expected          *WhenExpression
 	}{{
 		name: "replace parameters variables",
 		original: &WhenExpression{
@@ -307,10 +308,47 @@ func TestApplyReplacements(t *testing.T) {
 			Operator: selection.In,
 			Values:   []string{"barfoo"},
 		},
+	}, {
+		name: "replace array params",
+		original: &WhenExpression{
+			Input:    "$(params.path)",
+			Operator: selection.In,
+			Values:   []string{"$(params.branches[*])"},
+		},
+		replacements: map[string]string{
+			"params.path": "readme.md",
+		},
+		arrayReplacements: map[string][]string{
+			"params.branches": {"dev", "stage"},
+		},
+		expected: &WhenExpression{
+			Input:    "readme.md",
+			Operator: selection.In,
+			Values:   []string{"dev", "stage"},
+		},
+	}, {
+		name: "replace string and array params",
+		original: &WhenExpression{
+			Input:    "$(params.path)",
+			Operator: selection.In,
+			Values:   []string{"$(params.branches[*])", "$(params.files[*])"},
+		},
+		replacements: map[string]string{
+			"params.path": "readme.md",
+		},
+		arrayReplacements: map[string][]string{
+			"params.branches": {"dev", "stage"},
+			"params.files":    {"readme.md", "test.go"},
+		},
+		expected: &WhenExpression{
+			Input:    "readme.md",
+			Operator: selection.In,
+			Values:   []string{"dev", "stage", "readme.md", "test.go"},
+		},
 	}}
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
-			got := tc.original.applyReplacements(tc.replacements)
+			got := tc.original.applyReplacements(tc.replacements, tc.arrayReplacements)
 			if d := cmp.Diff(tc.expected, &got); d != "" {
 				t.Errorf("Error applying replacements for When Expressions: %s", diff.PrintWantGot(d))
 			}

diff --git a/pkg/apis/pipeline/v1beta1/when_validation.go b/pkg/apis/pipeline/v1beta1/when_validation.go
@@ -20,7 +20,6 @@ import (
 	"fmt"
 	"strings"
 
-	"github.com/tektoncd/pipeline/pkg/substitution"
 	"k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/selection"
 	"k8s.io/apimachinery/pkg/util/sets"
@@ -79,12 +78,15 @@ func (wes WhenExpressions) validatePipelineParametersVariables(prefix string, pa
 	for idx, we := range wes {
 		errs = errs.Also(validateStringVariable(we.Input, prefix, paramNames, arrayParamNames).ViaField("input").ViaFieldIndex("when", idx))
 		for _, val := range we.Values {
-			errs = errs.Also(validateStringVariable(val, prefix, paramNames, arrayParamNames).ViaField("values").ViaFieldIndex("when", idx))
+			// one of the values could be a reference to an array param, such as, $(params.foo[*])
+			// extract the variable name from the pattern $(params.foo[*]), if the variable name matches with one of the array params
+			// validate the param as an array variable otherwise, validate it as a string variable
+			if arrayParamNames.Has(strings.TrimSuffix(strings.TrimPrefix(val, "$("+prefix+"."), "[*])")) {
+				errs = errs.Also(validateArrayVariable(val, prefix, paramNames, arrayParamNames).ViaField("values").ViaFieldIndex("when", idx))
+			} else {
+				errs = errs.Also(validateStringVariable(val, prefix, paramNames, arrayParamNames).ViaField("values").ViaFieldIndex("when", idx))
+			}
 		}
 	}
 	return errs
 }
-func validateStringVariable(value, prefix string, stringVars sets.String, arrayVars sets.String) *apis.FieldError {
-	errs := substitution.ValidateVariableP(value, prefix, stringVars)
-	return errs.Also(substitution.ValidateVariableProhibitedP(value, prefix, arrayVars))
-}
diff --git a/pkg/reconciler/pipelinerun/resources/apply.go b/pkg/reconciler/pipelinerun/resources/apply.go
@@ -93,7 +93,7 @@ func ApplyTaskResults(targets PipelineRunState, resolvedResultRefs ResolvedResul
 		if resolvedPipelineRunTask.PipelineTask != nil {
 			pipelineTask := resolvedPipelineRunTask.PipelineTask.DeepCopy()
 			pipelineTask.Params = replaceParamValues(pipelineTask.Params, stringReplacements, nil)
-			pipelineTask.WhenExpressions = pipelineTask.WhenExpressions.ReplaceWhenExpressionsVariables(stringReplacements)
+			pipelineTask.WhenExpressions = pipelineTask.WhenExpressions.ReplaceWhenExpressionsVariables(stringReplacements, nil)
 			resolvedPipelineRunTask.PipelineTask = pipelineTask
 		}
 	}
@@ -105,7 +105,7 @@ func ApplyPipelineTaskStateContext(state PipelineRunState, replacements map[stri
 		if resolvedPipelineRunTask.PipelineTask != nil {
 			pipelineTask := resolvedPipelineRunTask.PipelineTask.DeepCopy()
 			pipelineTask.Params = replaceParamValues(pipelineTask.Params, replacements, nil)
-			pipelineTask.WhenExpressions = pipelineTask.WhenExpressions.ReplaceWhenExpressionsVariables(replacements)
+			pipelineTask.WhenExpressions = pipelineTask.WhenExpressions.ReplaceWhenExpressionsVariables(replacements, nil)
 			resolvedPipelineRunTask.PipelineTask = pipelineTask
 		}
 	}
@@ -137,12 +137,12 @@ func ApplyReplacements(p *v1beta1.PipelineSpec, replacements map[string]string,
 			c := p.Tasks[i].Conditions[j]
 			c.Params = replaceParamValues(c.Params, replacements, arrayReplacements)
 		}
-		p.Tasks[i].WhenExpressions = p.Tasks[i].WhenExpressions.ReplaceWhenExpressionsVariables(replacements)
+		p.Tasks[i].WhenExpressions = p.Tasks[i].WhenExpressions.ReplaceWhenExpressionsVariables(replacements, arrayReplacements)
 	}
 
 	for i := range p.Finally {
 		p.Finally[i].Params = replaceParamValues(p.Finally[i].Params, replacements, arrayReplacements)
-		p.Finally[i].WhenExpressions = p.Finally[i].WhenExpressions.ReplaceWhenExpressionsVariables(replacements)
+		p.Finally[i].WhenExpressions = p.Finally[i].WhenExpressions.ReplaceWhenExpressionsVariables(replacements, arrayReplacements)
 	}
 
 	return p