[TEP-0075]Validate Pipeline results array index

This is part of work in TEP-0075. Previous to this commit, we have added support for pipeline array results indexing. This commit adds the validation to check if it is out of bound.
tektoncd · Jul 14, 2022 · 6c3c47e · 6c3c47e
1 parent 0cd5e2e
commit 6c3c47e
Show file tree

Hide file tree

Showing 6 changed files with 81 additions and 13 deletions.
diff --git a/docs/pipelines.md b/docs/pipelines.md
@@ -1005,6 +1005,14 @@ Array and object results is supported as alpha feature, see [`Array Results` in
 
 ```yaml
     results:
+      - name: array-results
+        type: array
+        description: whole array
+        value: $(tasks.task1.results.array-results[*])
+      - name: array-indexing-results
+        type: string
+        description: array element
+        value: $(tasks.task1.results.array-results[1])
       - name: object-results
         type: object
         description: whole object

diff --git a/examples/v1beta1/pipelineruns/alpha/pipeline-emitting-results.yaml b/examples/v1beta1/pipelineruns/alpha/pipeline-emitting-results.yaml
@@ -41,6 +41,10 @@ spec:
         type: array
         description: whole array
         value: $(tasks.task1.results.array-results[*])
+      - name: array-results-from-array-indexing-and-object-elements
+        type: array
+        description: whole array
+        value: ["$(tasks.task1.results.array-results[0])", "$(tasks.task2.results.object-results.foo)"]
       - name: array-indexing-results
         type: string
         description: array element

diff --git a/pkg/apis/pipeline/v1beta1/resultref.go b/pkg/apis/pipeline/v1beta1/resultref.go
@@ -128,6 +128,9 @@ func GetVarSubstitutionExpressionsForParam(param Param) ([]string, bool) {
 // GetVarSubstitutionExpressionsForPipelineResult extracts all the value between "$(" and ")"" for a pipeline result
 func GetVarSubstitutionExpressionsForPipelineResult(result PipelineResult) ([]string, bool) {
 	allExpressions := validateString(result.Value.StringVal)
+	for _, v := range result.Value.ArrayVal {
+		allExpressions = append(allExpressions, validateString(v)...)
+	}
 	for _, v := range result.Value.ObjectVal {
 		allExpressions = append(allExpressions, validateString(v)...)
 	}

diff --git a/pkg/apis/pipeline/v1beta1/resultref_test.go b/pkg/apis/pipeline/v1beta1/resultref_test.go
@@ -732,20 +732,27 @@ func TestGetVarSubstitutionExpressionsForPipelineResult(t *testing.T) {
 			Value: *v1beta1.NewArrayOrString("$(tasks.task1.results.result1) and $(tasks.task2.results.result2)"),
 		},
 		want: []string{"tasks.task1.results.result1", "tasks.task2.results.result2"},
-	},
-		{
-			name: "get object result expressions",
-			result: v1beta1.PipelineResult{
-				Name: "object result",
-				Type: v1beta1.ResultsTypeString,
-				Value: *v1beta1.NewObject(map[string]string{
-					"key1": "$(tasks.task1.results.result1)",
-					"key2": "$(tasks.task2.results.result2) and another one $(tasks.task3.results.result3)",
-					"key3": "no ref here",
-				}),
-			},
-			want: []string{"tasks.task1.results.result1", "tasks.task2.results.result2", "tasks.task3.results.result3"},
+	}, {
+		name: "get array result expressions",
+		result: v1beta1.PipelineResult{
+			Name:  "array result",
+			Type:  v1beta1.ResultsTypeString,
+			Value: *v1beta1.NewArrayOrString("$(tasks.task1.results.result1)", "$(tasks.task2.results.result2)"),
+		},
+		want: []string{"tasks.task1.results.result1", "tasks.task2.results.result2"},
+	}, {
+		name: "get object result expressions",
+		result: v1beta1.PipelineResult{
+			Name: "object result",
+			Type: v1beta1.ResultsTypeString,
+			Value: *v1beta1.NewObject(map[string]string{
+				"key1": "$(tasks.task1.results.result1)",
+				"key2": "$(tasks.task2.results.result2) and another one $(tasks.task3.results.result3)",
+				"key3": "no ref here",
+			}),
 		},
+		want: []string{"tasks.task1.results.result1", "tasks.task2.results.result2", "tasks.task3.results.result3"},
+	},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

diff --git a/pkg/reconciler/pipelinerun/resources/apply.go b/pkg/reconciler/pipelinerun/resources/apply.go
@@ -303,6 +303,9 @@ func ApplyTaskResultsToPipelineResults(
 							intIdx, _ := strconv.Atoi(stringIdx)
 							if intIdx < len(resultValue.ArrayVal) {
 								stringReplacements[variable] = resultValue.ArrayVal[intIdx]
+							} else {
+								invalidPipelineResults = append(invalidPipelineResults, pipelineResult.Name)
+								validPipelineResult = false
 							}
 						} else {
 							arrayReplacements[substitution.StripStarVarSubExpression(variable)] = resultValue.ArrayVal

diff --git a/pkg/reconciler/pipelinerun/resources/apply_test.go b/pkg/reconciler/pipelinerun/resources/apply_test.go
@@ -2079,6 +2079,22 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) {
 			},
 		},
 		expectedResults: nil,
+	}, {
+		description: "array-index-out-of-bound",
+		results: []v1beta1.PipelineResult{{
+			Name:  "pipeline-result-1",
+			Value: *v1beta1.NewArrayOrString("$(tasks.pt1.results.foo[4])"),
+		}},
+		taskResults: map[string][]v1beta1.TaskRunResult{
+			"pt1": {
+				{
+					Name:  "foo",
+					Value: *v1beta1.NewArrayOrString("do", "rae", "mi"),
+				},
+			},
+		},
+		expectedResults: nil,
+		expectedError:   fmt.Errorf("invalid pipelineresults [pipeline-result-1], the referred results don't exist"),
 	}, {
 		description: "object-reference-not-exist",
 		results: []v1beta1.PipelineResult{{
@@ -2191,6 +2207,33 @@ func TestApplyTaskResultsToPipelineResults(t *testing.T) {
 				"pkey2": "rae",
 			}),
 		}},
+	}, {
+		description: "array-results-from-array-indexing-and-object-element",
+		results: []v1beta1.PipelineResult{{
+			Name:  "pipeline-result-1",
+			Value: *v1beta1.NewArrayOrString("$(tasks.pt1.results.foo.key1)", "$(tasks.pt2.results.bar[1])"),
+		}},
+		taskResults: map[string][]v1beta1.TaskRunResult{
+			"pt1": {
+				{
+					Name: "foo",
+					Value: *v1beta1.NewObject(map[string]string{
+						"key1": "val1",
+						"key2": "val2",
+					}),
+				},
+			},
+			"pt2": {
+				{
+					Name:  "bar",
+					Value: *v1beta1.NewArrayOrString("do", "rae", "mi"),
+				},
+			},
+		},
+		expectedResults: []v1beta1.PipelineRunResult{{
+			Name:  "pipeline-result-1",
+			Value: *v1beta1.NewArrayOrString("val1", "rae"),
+		}},
 	}, {
 		description: "apply-object-element",
 		results: []v1beta1.PipelineResult{{