[TEP-0075] Validate task result variable of object type

pkg/apis/pipeline/v1beta1/pipeline_validation_test.go

@@ -1102,6 +1102,10 @@ func TestValidatePipelineResults_Success(t *testing.T) {
 		Name:        "my-pipeline-result",
 		Description: "this is my pipeline result",
 		Value:       "$(tasks.a-task.results.output)",
+	}, {
+		Name:        "my-pipeline-object-result",
+		Description: "this is my pipeline result",
+		Value:       "$(tasks.a-task.results.gitrepo.commit)",
 	}}
 	if err := validatePipelineResults(results); err != nil {
 		t.Errorf("Pipeline.validatePipelineResults() returned error for valid pipeline: %s: %v", desc, err)
@@ -1118,10 +1122,10 @@ func TestValidatePipelineResults_Failure(t *testing.T) {
 		results: []PipelineResult{{
 			Name:        "my-pipeline-result",
 			Description: "this is my pipeline result",
-			Value:       "$(tasks.a-task.results.output.output)",
+			Value:       "$(tasks.a-task.results.output.key1.extra)",
 		}},
 		expectedError: apis.FieldError{
-			Message: `invalid value: expected all of the expressions [tasks.a-task.results.output.output] to be result expressions but only [] were`,
+			Message: `invalid value: expected all of the expressions [tasks.a-task.results.output.key1.extra] to be result expressions but only [] were`,
 			Paths:   []string{"results[0].value"},
 		},
 	}, {

pkg/apis/pipeline/v1beta1/resultref.go

@@ -28,10 +28,15 @@ type ResultRef struct {
 	PipelineTask string `json:"pipelineTask"`
 	Result       string `json:"result"`
 	ResultsIndex int    `json:"resultsIndex"`
+	Property     string `json:"property"`
 }
 
 const (
 	resultExpressionFormat = "tasks.<taskName>.results.<resultName>"
+	// Result expressions of the form <resultName>.<attribute> will be treated as object results.
+	// If a string result name contains a dot, brackets should be used to differentiate it from an object result.
+	// https://github.com/tektoncd/community/blob/main/teps/0075-object-param-and-result-types.md#collisions-with-builtin-variable-replacement
+	objectResultExpressionFormat = "tasks.<taskName>.results.<objectResultName>.<individualAttribute>"
 	// ResultTaskPart Constant used to define the "tasks" part of a pipeline result reference
 	ResultTaskPart = "tasks"
 	// ResultResultPart Constant used to define the "results" part of a pipeline result reference
@@ -55,7 +60,7 @@ var arrayIndexingRegex = regexp.MustCompile(arrayIndexing)
 func NewResultRefs(expressions []string) []*ResultRef {
 	var resultRefs []*ResultRef
 	for _, expression := range expressions {
-		pipelineTask, result, index, err := parseExpression(expression)
+		pipelineTask, result, index, property, 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
@@ -64,6 +69,7 @@ func NewResultRefs(expressions []string) []*ResultRef {
 				PipelineTask: pipelineTask,
 				Result:       result,
 				ResultsIndex: index,
+				Property:     property,
 			})
 		}
 	}
@@ -101,6 +107,11 @@ func GetVarSubstitutionExpressionsForParam(param Param) ([]string, bool) {
 	case ParamTypeString:
 		// string type
 		allExpressions = append(allExpressions, validateString(param.Value.StringVal)...)
+	case ParamTypeObject:
+		// object type
+		for _, value := range param.Value.ObjectVal {
+			allExpressions = append(allExpressions, validateString(value)...)
+		}
 	default:
 		return nil, false
 	}
@@ -129,19 +140,41 @@ func stripVarSubExpression(expression string) string {
 	return strings.TrimSuffix(strings.TrimPrefix(expression, "$("), ")")
 }
 
-func parseExpression(substitutionExpression string) (string, string, int, error) {
+// parseExpression parses "task name", "result name", "array index" (iff it's an array result) and "object key name" (iff it's an object result)
+// Valid Example 1:
+// - Input: tasks.myTask.results.aStringResult
+// - Output: "myTask", "aStringResult", -1, "", nil
+// Valid Example 2:
+// - Input: tasks.myTask.results.anObjectResult.key1
+// - Output: "myTask", "anObjectResult", 0, "key1", nil
+// Valid Example 3:
+// - Input: tasks.myTask.results.anArrayResult[1]
+// - Output: "myTask", "anArrayResult", 1, "", nil
+// Invalid Example 1:
+// - Input: tasks.myTask.results.resultName.foo.bar
+// - Output: "", "", 0, "", error
+// TODO: may use regex for each type to handle possible reference formats
+func parseExpression(substitutionExpression string) (string, string, int, string, error) {
 	subExpressions := strings.Split(substitutionExpression, ".")
-	if len(subExpressions) != 4 || subExpressions[0] != ResultTaskPart || subExpressions[2] != ResultResultPart {
-		return "", "", 0, fmt.Errorf("Must be of the form %q", resultExpressionFormat)
+
+	// For string result: tasks.<taskName>.results.<stringResultName>
+	// For array result: tasks.<taskName>.results.<arrayResultName>[index]
+	if len(subExpressions) == 4 && subExpressions[0] == ResultTaskPart && subExpressions[2] == ResultResultPart {
+		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
 	}
 
-	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
+	// For object type result: tasks.<taskName>.results.<objectResultName>.<individualAttribute>
+	if len(subExpressions) == 5 && subExpressions[0] == ResultTaskPart && subExpressions[2] == ResultResultPart {
+		return subExpressions[1], subExpressions[3], 0, subExpressions[4], nil
 	}
-	return subExpressions[1], subExpressions[3], 0, nil
+
+	return "", "", 0, "", fmt.Errorf("Must be one of the form 1). %q; 2). %q", resultExpressionFormat, objectResultExpressionFormat)
 }
 
 // PipelineTaskResultRefs walks all the places a result reference can be used

pkg/apis/pipeline/v1beta1/resultref_test.go

@@ -51,6 +51,17 @@ func TestNewResultReference(t *testing.T) {
 			PipelineTask: "sumTask",
 			Result:       "sumResult",
 		}},
+	}, {
+		name: "Test valid expression with single object result property",
+		param: v1beta1.Param{
+			Name:  "param",
+			Value: *v1beta1.NewArrayOrString("$(tasks.sumTask.results.sumResult.key1)"),
+		},
+		want: []*v1beta1.ResultRef{{
+			PipelineTask: "sumTask",
+			Result:       "sumResult",
+			Property:     "key1",
+		}},
 	}, {
 		name: "refer array indexing result",
 		param: v1beta1.Param{
@@ -62,6 +73,21 @@ func TestNewResultReference(t *testing.T) {
 			Result:       "sumResult",
 			ResultsIndex: 1,
 		}},
+	}, {
+		name: "Test valid expression with multiple object result properties",
+		param: v1beta1.Param{
+			Name:  "param",
+			Value: *v1beta1.NewArrayOrString("$(tasks.sumTask.results.imageresult.digest), and another one $(tasks.sumTask.results.imageresult.tag)"),
+		},
+		want: []*v1beta1.ResultRef{{
+			PipelineTask: "sumTask",
+			Result:       "imageresult",
+			Property:     "digest",
+		}, {
+			PipelineTask: "sumTask",
+			Result:       "imageresult",
+			Property:     "tag",
+		}},
 	}, {
 		name: "substitution within string",
 		param: v1beta1.Param{
@@ -76,27 +102,35 @@ func TestNewResultReference(t *testing.T) {
 		name: "multiple substitution",
 		param: v1beta1.Param{
 			Name:  "param",
-			Value: *v1beta1.NewArrayOrString("$(tasks.sumTask1.results.sumResult) and another $(tasks.sumTask2.results.sumResult)"),
+			Value: *v1beta1.NewArrayOrString("$(tasks.sumTask1.results.sumResult) and another $(tasks.sumTask2.results.sumResult), last one $(tasks.sumTask3.results.sumResult.key1)"),
 		},
 		want: []*v1beta1.ResultRef{{
 			PipelineTask: "sumTask1",
 			Result:       "sumResult",
 		}, {
 			PipelineTask: "sumTask2",
 			Result:       "sumResult",
+		}, {
+			PipelineTask: "sumTask3",
+			Result:       "sumResult",
+			Property:     "key1",
 		}},
 	}, {
 		name: "multiple substitution with param",
 		param: v1beta1.Param{
 			Name:  "param",
-			Value: *v1beta1.NewArrayOrString("$(params.param) $(tasks.sumTask1.results.sumResult) and another $(tasks.sumTask2.results.sumResult)"),
+			Value: *v1beta1.NewArrayOrString("$(params.param) $(tasks.sumTask1.results.sumResult) and another $(tasks.sumTask2.results.sumResult), last one $(tasks.sumTask3.results.sumResult.key1)"),
 		},
 		want: []*v1beta1.ResultRef{{
 			PipelineTask: "sumTask1",
 			Result:       "sumResult",
 		}, {
 			PipelineTask: "sumTask2",
 			Result:       "sumResult",
+		}, {
+			PipelineTask: "sumTask3",
+			Result:       "sumResult",
+			Property:     "key1",
 		}},
 	}, {
 		name: "first separator typo",
@@ -112,6 +146,13 @@ func TestNewResultReference(t *testing.T) {
 			Value: *v1beta1.NewArrayOrString("$(tasks.sumTasks.result.sumResult)"),
 		},
 		want: nil,
+	}, {
+		name: "more than 5 dot-separated components",
+		param: v1beta1.Param{
+			Name:  "param",
+			Value: *v1beta1.NewArrayOrString("$(tasks.sumTasks.result.sumResult.key.extra)"),
+		},
+		want: nil,
 	}, {
 		name: "param substitution shouldn't be considered result ref",
 		param: v1beta1.Param{
@@ -209,6 +250,26 @@ func TestHasResultReference(t *testing.T) {
 			PipelineTask: "sumTask2",
 			Result:       "sumResult2",
 		}},
+	}, {
+		name: "Test valid expression in object",
+		param: v1beta1.Param{
+			Name: "param",
+			Value: *v1beta1.NewObject(map[string]string{
+				"key1": "$(tasks.sumTask1.results.sumResult1)",
+				"key2": "$(tasks.sumTask2.results.sumResult2) and another one $(tasks.sumTask3.results.sumResult3)",
+				"key3": "no ref here",
+			}),
+		},
+		wantRef: []*v1beta1.ResultRef{{
+			PipelineTask: "sumTask1",
+			Result:       "sumResult1",
+		}, {
+			PipelineTask: "sumTask2",
+			Result:       "sumResult2",
+		}, {
+			PipelineTask: "sumTask3",
+			Result:       "sumResult3",
+		}},
 	}} {
 		t.Run(tt.name, func(t *testing.T) {
 			expressions, ok := v1beta1.GetVarSubstitutionExpressionsForParam(tt.param)

pkg/apis/pipeline/v1beta1/swagger.json

@@ -1525,13 +1525,18 @@
       "required": [
         "pipelineTask",
         "result",
-        "resultsIndex"
+        "resultsIndex",
+        "property"
       ],
       "properties": {
         "pipelineTask": {
           "type": "string",
           "default": ""
         },
+        "property": {
+          "type": "string",
+          "default": ""
+        },
         "result": {
           "type": "string",
           "default": ""