add index validation

pkg/apis/pipeline/v1beta1/resultref.go

@@ -19,13 +19,15 @@ package v1beta1
 import (
 	"fmt"
 	"regexp"
+	"strconv"
 	"strings"
 )
 
 // ResultRef is a type that represents a reference to a task run result
 type ResultRef struct {
 	PipelineTask string `json:"pipelineTask"`
 	Result       string `json:"result"`
+	ResultsIndex int    `json:"resultsIndex"`
 }
 
 const (
@@ -37,30 +39,31 @@ const (
 	// TODO(#2462) use one regex across all substitutions
 	// variableSubstitutionFormat matches format like $result.resultname, $result.resultname[int] and $result.resultname[*]
 	variableSubstitutionFormat = `\$\([_a-zA-Z0-9.-]+(\.[_a-zA-Z0-9.-]+)*(\[([0-9])*\*?\])?\)`
-	// excludeArrayIndexing will replace all `[int]` and `[*]` for parseExpression to extract result name
-	excludeArrayIndexing = `\[([0-9])*\*?\]`
+	// arrayIndexing will match all `[int]` and `[*]` for parseExpression
+	arrayIndexing = `\[([0-9])*\*?\]`
 	// ResultNameFormat Constant used to define the the regex Result.Name should follow
 	ResultNameFormat = `^([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$`
 )
 
 var variableSubstitutionRegex = regexp.MustCompile(variableSubstitutionFormat)
 var resultNameFormatRegex = regexp.MustCompile(ResultNameFormat)
-var excludeArrayIndexingRegex = regexp.MustCompile(excludeArrayIndexing)
+var arrayIndexingRegex = regexp.MustCompile(arrayIndexing)
 
 // NewResultRefs extracts all ResultReferences from a param or a pipeline result.
 // If the ResultReference can be extracted, they are returned. Expressions which are not
 // results are ignored.
 func NewResultRefs(expressions []string) []*ResultRef {
 	var resultRefs []*ResultRef
 	for _, expression := range expressions {
-		pipelineTask, result, err := parseExpression(expression)
+		pipelineTask, result, index, err := parseExpression(expression)
 		// If the expression isn't a result but is some other expression,
 		// parseExpression will return an error, in which case we just skip that expression,
 		// since although it's not a result ref, it might be some other kind of reference
 		if err == nil {
 			resultRefs = append(resultRefs, &ResultRef{
 				PipelineTask: pipelineTask,
 				Result:       result,
+				ResultsIndex: index,
 			})
 		}
 	}
@@ -126,13 +129,19 @@ func stripVarSubExpression(expression string) string {
 	return strings.TrimSuffix(strings.TrimPrefix(expression, "$("), ")")
 }
 
-func parseExpression(substitutionExpression string) (string, string, error) {
+func parseExpression(substitutionExpression string) (string, string, int, error) {
 	subExpressions := strings.Split(substitutionExpression, ".")
 	if len(subExpressions) != 4 || subExpressions[0] != ResultTaskPart || subExpressions[2] != ResultResultPart {
-		return "", "", fmt.Errorf("Must be of the form %q", resultExpressionFormat)
+		return "", "", 0, fmt.Errorf("Must be of the form %q", resultExpressionFormat)
 	}
-	subExpressions[3] = excludeArrayIndexingRegex.ReplaceAllString(subExpressions[3], "")
-	return subExpressions[1], subExpressions[3], nil
+
+	stringIdx := strings.TrimSuffix(strings.TrimPrefix(arrayIndexingRegex.FindString(subExpressions[3]), "["), "]")
+	subExpressions[3] = arrayIndexingRegex.ReplaceAllString(subExpressions[3], "")
+	if stringIdx != "" {
+		intIdx, _ := strconv.Atoi(stringIdx)
+		return subExpressions[1], subExpressions[3], intIdx, nil
+	}
+	return subExpressions[1], subExpressions[3], 0, nil
 }
 
 // PipelineTaskResultRefs walks all the places a result reference can be used

pkg/apis/pipeline/v1beta1/resultref_test.go

@@ -41,6 +41,27 @@ func TestNewResultReference(t *testing.T) {
 			PipelineTask: "sumTask",
 			Result:       "sumResult",
 		}},
+	},{
+		name: "refer whole array result",
+		param: v1beta1.Param{
+			Name:  "param",
+			Value: *v1beta1.NewArrayOrString("$(tasks.sumTask.results.sumResult[*])"),
+		},
+		want: []*v1beta1.ResultRef{{
+			PipelineTask: "sumTask",
+			Result:       "sumResult",
+		}},
+	},{
+		name: "refer array indexing result",
+		param: v1beta1.Param{
+			Name:  "param",
+			Value: *v1beta1.NewArrayOrString("$(tasks.sumTask.results.sumResult[1])"),
+		},
+		want: []*v1beta1.ResultRef{{
+			PipelineTask: "sumTask",
+			Result:       "sumResult",
+			ResultsIndex: 1,
+		}},
 	}, {
 		name: "substitution within string",
 		param: v1beta1.Param{

pkg/reconciler/pipelinerun/resources/apply_test.go

@@ -431,7 +431,38 @@ func TestApplyTaskResults_MinimalExpression(t *testing.T) {
 				}},
 			},
 		}},
-	}, {
+	},  {
+		name: "Test array indexing result substitution out of bound - params",
+		resolvedResultRefs: ResolvedResultRefs{{
+			Value: *v1beta1.NewArrayOrString("arrayResultValueOne", "arrayResultValueTwo"),
+			ResultReference: v1beta1.ResultRef{
+				PipelineTask: "aTask",
+				Result:       "a.Result",
+			},
+			FromTaskRun: "aTaskRun",
+		}},
+		targets: PipelineRunState{{
+			PipelineTask: &v1beta1.PipelineTask{
+				Name:    "bTask",
+				TaskRef: &v1beta1.TaskRef{Name: "bTask"},
+				Params: []v1beta1.Param{{
+					Name:  "bParam",
+					Value: *v1beta1.NewArrayOrString(`$(tasks.aTask.results["a.Result"][3])`),
+				}},
+			},
+		}},
+		want: PipelineRunState{{
+			PipelineTask: &v1beta1.PipelineTask{
+				Name:    "bTask",
+				TaskRef: &v1beta1.TaskRef{Name: "bTask"},
+				Params: []v1beta1.Param{{
+					Name:  "bParam",
+					// index validation is done in ResolveResultRefs() before ApplyTaskResults()
+					Value: *v1beta1.NewArrayOrString(`$(tasks.aTask.results["a.Result"][3])`),
+				}},
+			},
+		}},
+	},{
 		name: "Test result substitution on minimal variable substitution expression - when expressions",
 		resolvedResultRefs: ResolvedResultRefs{{
 			Value: *v1beta1.NewArrayOrString("aResultValue"),

pkg/reconciler/pipelinerun/resources/pipelinerunresolution.go

@@ -306,8 +306,10 @@ func (t *ResolvedPipelineRunTask) skipBecauseResultReferencesAreMissing(facts *P
 	if t.checkParentsDone(facts) && t.hasResultReferences() {
 		resolvedResultRefs, pt, err := ResolveResultRefs(facts.State, PipelineRunState{t})
 		rprt := facts.State.ToMap()[pt]
-		if err != nil && (t.IsFinalTask(facts) || rprt.Skip(facts).SkippingReason == v1beta1.WhenExpressionsSkip) {
-			return true
+		if rprt != nil {
+			if err != nil && (t.IsFinalTask(facts) || rprt.Skip(facts).SkippingReason == v1beta1.WhenExpressionsSkip) {
+				return true
+			}
 		}
 		ApplyTaskResults(PipelineRunState{t}, resolvedResultRefs)
 		facts.ResetSkippedCache()

pkg/reconciler/pipelinerun/resources/resultrefresolution.go

@@ -43,7 +43,7 @@ func ResolveResultRef(pipelineRunState PipelineRunState, target *ResolvedPipelin
 	if err != nil {
 		return nil, pt, err
 	}
-	return removeDup(resolvedResultRefs), "", nil
+	return validateArrayResultsIndex(removeDup(resolvedResultRefs))
 }
 
 // ResolveResultRefs resolves any ResultReference that are found in the target ResolvedPipelineRunTask
@@ -56,7 +56,19 @@ func ResolveResultRefs(pipelineRunState PipelineRunState, targets PipelineRunSta
 		}
 		allResolvedResultRefs = append(allResolvedResultRefs, resolvedResultRefs...)
 	}
-	return removeDup(allResolvedResultRefs), "", nil
+	return validateArrayResultsIndex(removeDup(allResolvedResultRefs))
+}
+
+// validateArrayResultsIndex checks if the result array indexing reference is out of bound of the array size
+func validateArrayResultsIndex(allResolvedResultRefs ResolvedResultRefs) (ResolvedResultRefs, string, error) {
+	for _, r := range allResolvedResultRefs {
+		if r.Value.Type == v1beta1.ParamTypeArray {
+			if r.ResultReference.ResultsIndex >= len(r.Value.ArrayVal) {
+				return nil, "", fmt.Errorf("Array Result Index %d for Task %s Result %s is out of bound of size %d", r.ResultReference.ResultsIndex, r.ResultReference.PipelineTask, r.ResultReference.Result, len(r.Value.ArrayVal))
+			}
+		}
+	}
+	return allResolvedResultRefs, "", nil
 }
 
 // extractResultRefs resolves any ResultReference that are found in param or pipeline result

pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go

@@ -120,6 +120,58 @@ var pipelineRunState = PipelineRunState{{
 			Value: *v1beta1.NewArrayOrString("$(tasks.aCustomPipelineTask.results.aResult)"),
 		}},
 	},
+}, {
+	TaskRunName: "cTaskRun",
+	TaskRun: &v1beta1.TaskRun{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "cTaskRun",
+		},
+		Status: v1beta1.TaskRunStatus{
+			Status: duckv1beta1.Status{
+				Conditions: duckv1beta1.Conditions{successCondition},
+			},
+			TaskRunStatusFields: v1beta1.TaskRunStatusFields{
+				TaskRunResults: []v1beta1.TaskRunResult{{
+					Name:  "cResult",
+					Value: *v1beta1.NewArrayOrString("arrayResultOne", "arrayResultTwo"),
+				}},
+			},
+		},
+	},
+	PipelineTask: &v1beta1.PipelineTask{
+		Name:    "cTask",
+		TaskRef: &v1beta1.TaskRef{Name: "cTask"},
+		Params: []v1beta1.Param{{
+			Name:  "cParam",
+			Value: *v1beta1.NewArrayOrString("$(tasks.cTask.results.cResult[1])"),
+		}},
+	},
+}, {
+	TaskRunName: "dTaskRun",
+	TaskRun: &v1beta1.TaskRun{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "dTaskRun",
+		},
+		Status: v1beta1.TaskRunStatus{
+			Status: duckv1beta1.Status{
+				Conditions: duckv1beta1.Conditions{successCondition},
+			},
+			TaskRunStatusFields: v1beta1.TaskRunStatusFields{
+				TaskRunResults: []v1beta1.TaskRunResult{{
+					Name:  "dResult",
+					Value: *v1beta1.NewArrayOrString("arrayResultOne", "arrayResultTwo"),
+				}},
+			},
+		},
+	},
+	PipelineTask: &v1beta1.PipelineTask{
+		Name:    "dTask",
+		TaskRef: &v1beta1.TaskRef{Name: "dTask"},
+		Params: []v1beta1.Param{{
+			Name:  "dParam",
+			Value: *v1beta1.NewArrayOrString("$(tasks.dTask.results.dResult[3])"),
+		}},
+	},
 }}
 
 func TestTaskParamResolver_ResolveResultRefs(t *testing.T) {
@@ -479,6 +531,30 @@ func TestResolveResultRefs(t *testing.T) {
 			FromTaskRun: "aTaskRun",
 		}},
 		wantErr: false,
+	}, {
+		name:             "Test successful array result references resolution - params",
+		pipelineRunState: pipelineRunState,
+		targets: PipelineRunState{
+			pipelineRunState[7],
+		},
+		want: ResolvedResultRefs{{
+			Value: *v1beta1.NewArrayOrString("arrayResultOne", "arrayResultTwo"),
+			ResultReference: v1beta1.ResultRef{
+				PipelineTask: "cTask",
+				Result:       "cResult",
+				ResultsIndex: 1,
+			},
+			FromTaskRun: "cTaskRun",
+		}},
+		wantErr: false,
+	}, {
+		name:             "Test unsuccessful result references resolution - params",
+		pipelineRunState: pipelineRunState,
+		targets: PipelineRunState{
+			pipelineRunState[8],
+		},
+		want:    nil,
+		wantErr: true,
 	}, {
 		name:             "Test successful result references resolution - when expressions",
 		pipelineRunState: pipelineRunState,