diff --git a/examples/v1/pipelineruns/beta/7392-regression.yaml b/examples/v1/pipelineruns/beta/7392-regression.yaml new file mode 100644 index 00000000000..c15369ffdc0 --- /dev/null +++ b/examples/v1/pipelineruns/beta/7392-regression.yaml @@ -0,0 +1,51 @@ +apiVersion: tekton.dev/v1 +kind: Task +metadata: + name: task-with-array-result +spec: + params: + - name: arrayVal1 + description: "description3" + type: array + default: [] + steps: + - name: step1 + image: bash:latest + args: + - --images + - $(params.arrayVal1[*]) + script: | + #!/usr/bin/env bash + for arg in "$@"; do + echo $arg + done + - name: step2 + image: bash:latest + script: | + #!/usr/bin/env bash + echo -n '[]' | tee $(results.resultArray.path) + results: + - name: resultArray + description: "description4" + type: array + +--- +apiVersion: tekton.dev/v1 +kind: PipelineRun +metadata: + name: pipelinerun-array-result +spec: + pipelineSpec: + tasks: + - name: task-1 + taskRef: + name: task-with-array-result + params: + - name: arrayVal1 + value: [] + - name: task-2 + taskRef: + name: task-with-array-result + params: + - name: arrayVal1 + value: $(tasks.task-1.results.resultArray) diff --git a/pkg/apis/pipeline/v1/openapi_generated.go b/pkg/apis/pipeline/v1/openapi_generated.go index fe5d925166f..f3de88d9659 100644 --- a/pkg/apis/pipeline/v1/openapi_generated.go +++ b/pkg/apis/pipeline/v1/openapi_generated.go @@ -2328,9 +2328,8 @@ func schema_pkg_apis_pipeline_v1_ResultRef(ref common.ReferenceCallback) common. }, "resultsIndex": { SchemaProps: spec.SchemaProps{ - Default: 0, - Type: []string{"integer"}, - Format: "int32", + Type: []string{"integer"}, + Format: "int32", }, }, "property": { diff --git a/pkg/apis/pipeline/v1/resultref.go b/pkg/apis/pipeline/v1/resultref.go index 2de6bb80804..3948581f6d6 100644 --- a/pkg/apis/pipeline/v1/resultref.go +++ b/pkg/apis/pipeline/v1/resultref.go @@ -27,7 +27,7 @@ import ( type ResultRef struct { PipelineTask string `json:"pipelineTask"` Result string `json:"result"` - ResultsIndex int `json:"resultsIndex"` + ResultsIndex *int `json:"resultsIndex"` Property string `json:"property"` } @@ -122,37 +122,40 @@ func stripVarSubExpression(expression string) string { } // 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: +// 1. Reference string result // - Input: tasks.myTask.results.aStringResult -// - Output: "myTask", "aStringResult", -1, "", nil -// Valid Example 2: +// - Output: "myTask", "aStringResult", nil, "", nil +// 2. Reference Object value with key: // - Input: tasks.myTask.results.anObjectResult.key1 -// - Output: "myTask", "anObjectResult", 0, "key1", nil -// Valid Example 3: +// - Output: "myTask", "anObjectResult", nil, "key1", nil +// 3. Reference array elements with array indexing : // - Input: tasks.myTask.results.anArrayResult[1] // - Output: "myTask", "anArrayResult", 1, "", nil -// Invalid Example 1: +// 4. Referencing whole array or object result: +// - Input: tasks.myTask.results.Result[*] +// - Output: "myTask", "Result", nil, "", nil +// Invalid Case: // - Input: tasks.myTask.results.resultName.foo.bar -// - Output: "", "", 0, "", error +// - Output: "", "", nil, "", error // TODO: may use regex for each type to handle possible reference formats -func parseExpression(substitutionExpression string) (string, string, int, string, error) { +func parseExpression(substitutionExpression string) (string, string, *int, string, error) { if looksLikeResultRef(substitutionExpression) { subExpressions := strings.Split(substitutionExpression, ".") // For string result: tasks..results. // For array result: tasks..results.[index] if len(subExpressions) == 4 { resultName, stringIdx := ParseResultName(subExpressions[3]) - if stringIdx != "" { + if stringIdx != "" && stringIdx != "*" { intIdx, _ := strconv.Atoi(stringIdx) - return subExpressions[1], resultName, intIdx, "", nil + return subExpressions[1], resultName, &intIdx, "", nil } - return subExpressions[1], resultName, 0, "", nil + return subExpressions[1], resultName, nil, "", nil } else if len(subExpressions) == 5 { // For object type result: tasks..results.. - return subExpressions[1], subExpressions[3], 0, subExpressions[4], nil + return subExpressions[1], subExpressions[3], nil, subExpressions[4], nil } } - return "", "", 0, "", fmt.Errorf("must be one of the form 1). %q; 2). %q", resultExpressionFormat, objectResultExpressionFormat) + return "", "", nil, "", fmt.Errorf("must be one of the form 1). %q; 2). %q", resultExpressionFormat, objectResultExpressionFormat) } // ParseResultName parse the input string to extract resultName and result index. diff --git a/pkg/apis/pipeline/v1/resultref_test.go b/pkg/apis/pipeline/v1/resultref_test.go index 0ec0c37a79c..22f0dba5f3d 100644 --- a/pkg/apis/pipeline/v1/resultref_test.go +++ b/pkg/apis/pipeline/v1/resultref_test.go @@ -28,6 +28,7 @@ import ( ) func TestNewResultReference(t *testing.T) { + idx := 1 for _, tt := range []struct { name string param v1.Param @@ -43,7 +44,7 @@ func TestNewResultReference(t *testing.T) { Result: "sumResult", }}, }, { - name: "refer whole array result", + name: "reference whole array result", param: v1.Param{ Name: "param", Value: *v1.NewStructuredValues("$(tasks.sumTask.results.sumResult[*])"), @@ -51,6 +52,7 @@ func TestNewResultReference(t *testing.T) { want: []*v1.ResultRef{{ PipelineTask: "sumTask", Result: "sumResult", + ResultsIndex: nil, }}, }, { name: "Test valid expression with single object result property", @@ -72,7 +74,7 @@ func TestNewResultReference(t *testing.T) { want: []*v1.ResultRef{{ PipelineTask: "sumTask", Result: "sumResult", - ResultsIndex: 1, + ResultsIndex: &idx, }}, }, { name: "Test valid expression with multiple object result properties", diff --git a/pkg/apis/pipeline/v1/swagger.json b/pkg/apis/pipeline/v1/swagger.json index 8093c7e8389..1264eb28913 100644 --- a/pkg/apis/pipeline/v1/swagger.json +++ b/pkg/apis/pipeline/v1/swagger.json @@ -1176,8 +1176,7 @@ }, "resultsIndex": { "type": "integer", - "format": "int32", - "default": 0 + "format": "int32" } } }, diff --git a/pkg/apis/pipeline/v1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1/zz_generated.deepcopy.go index 40fe4ba804d..db26c7d4c5e 100644 --- a/pkg/apis/pipeline/v1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1/zz_generated.deepcopy.go @@ -1079,6 +1079,11 @@ func (in *ResolverRef) DeepCopy() *ResolverRef { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ResultRef) DeepCopyInto(out *ResultRef) { *out = *in + if in.ResultsIndex != nil { + in, out := &in.ResultsIndex, &out.ResultsIndex + *out = new(int) + **out = **in + } return } diff --git a/pkg/apis/pipeline/v1beta1/openapi_generated.go b/pkg/apis/pipeline/v1beta1/openapi_generated.go index d9eaff0ecc4..1646119cb51 100644 --- a/pkg/apis/pipeline/v1beta1/openapi_generated.go +++ b/pkg/apis/pipeline/v1beta1/openapi_generated.go @@ -3197,9 +3197,8 @@ func schema_pkg_apis_pipeline_v1beta1_ResultRef(ref common.ReferenceCallback) co }, "resultsIndex": { SchemaProps: spec.SchemaProps{ - Default: 0, - Type: []string{"integer"}, - Format: "int32", + Type: []string{"integer"}, + Format: "int32", }, }, "property": { diff --git a/pkg/apis/pipeline/v1beta1/resultref.go b/pkg/apis/pipeline/v1beta1/resultref.go index 43ad32036f1..7c99dc0c034 100644 --- a/pkg/apis/pipeline/v1beta1/resultref.go +++ b/pkg/apis/pipeline/v1beta1/resultref.go @@ -27,7 +27,7 @@ import ( type ResultRef struct { PipelineTask string `json:"pipelineTask"` Result string `json:"result"` - ResultsIndex int `json:"resultsIndex"` + ResultsIndex *int `json:"resultsIndex"` Property string `json:"property"` } @@ -157,38 +157,40 @@ func stripVarSubExpression(expression string) string { } // 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: +// 1. Reference string result // - Input: tasks.myTask.results.aStringResult -// - Output: "myTask", "aStringResult", -1, "", nil -// Valid Example 2: +// - Output: "myTask", "aStringResult", nil, "", nil +// 2. Reference Object value with key: // - Input: tasks.myTask.results.anObjectResult.key1 -// - Output: "myTask", "anObjectResult", 0, "key1", nil -// Valid Example 3: +// - Output: "myTask", "anObjectResult", nil, "key1", nil +// 3. Reference array elements with array indexing : // - Input: tasks.myTask.results.anArrayResult[1] // - Output: "myTask", "anArrayResult", 1, "", nil -// Invalid Example 1: +// 4. Referencing whole array or object result: +// - Input: tasks.myTask.results.Result[*] +// - Output: "myTask", "Result", nil, "", nil +// Invalid Case: // - Input: tasks.myTask.results.resultName.foo.bar -// - Output: "", "", 0, "", error +// - Output: "", "", nil, "", error // TODO: may use regex for each type to handle possible reference formats -func parseExpression(substitutionExpression string) (string, string, int, string, error) { +func parseExpression(substitutionExpression string) (string, string, *int, string, error) { if looksLikeResultRef(substitutionExpression) { subExpressions := strings.Split(substitutionExpression, ".") // For string result: tasks..results. // For array result: tasks..results.[index] if len(subExpressions) == 4 { resultName, stringIdx := ParseResultName(subExpressions[3]) - if stringIdx != "" { + if stringIdx != "" && stringIdx != "*" { intIdx, _ := strconv.Atoi(stringIdx) - return subExpressions[1], resultName, intIdx, "", nil + return subExpressions[1], resultName, &intIdx, "", nil } - return subExpressions[1], resultName, 0, "", nil + return subExpressions[1], resultName, nil, "", nil } else if len(subExpressions) == 5 { // For object type result: tasks..results.. - return subExpressions[1], subExpressions[3], 0, subExpressions[4], nil + return subExpressions[1], subExpressions[3], nil, subExpressions[4], nil } } - - return "", "", 0, "", fmt.Errorf("must be one of the form 1). %q; 2). %q", resultExpressionFormat, objectResultExpressionFormat) + return "", "", nil, "", fmt.Errorf("must be one of the form 1). %q; 2). %q", resultExpressionFormat, objectResultExpressionFormat) } // ParseResultName parse the input string to extract resultName and result index. diff --git a/pkg/apis/pipeline/v1beta1/resultref_test.go b/pkg/apis/pipeline/v1beta1/resultref_test.go index 42e881ed5b2..6e775582733 100644 --- a/pkg/apis/pipeline/v1beta1/resultref_test.go +++ b/pkg/apis/pipeline/v1beta1/resultref_test.go @@ -27,6 +27,7 @@ import ( ) func TestNewResultReference(t *testing.T) { + idx1 := 1 for _, tt := range []struct { name string param v1beta1.Param @@ -42,7 +43,7 @@ func TestNewResultReference(t *testing.T) { Result: "sumResult", }}, }, { - name: "refer whole array result", + name: "reference whole array/object result", param: v1beta1.Param{ Name: "param", Value: *v1beta1.NewStructuredValues("$(tasks.sumTask.results.sumResult[*])"), @@ -50,6 +51,7 @@ func TestNewResultReference(t *testing.T) { want: []*v1beta1.ResultRef{{ PipelineTask: "sumTask", Result: "sumResult", + ResultsIndex: nil, }}, }, { name: "Test valid expression with single object result property", @@ -71,7 +73,7 @@ func TestNewResultReference(t *testing.T) { want: []*v1beta1.ResultRef{{ PipelineTask: "sumTask", Result: "sumResult", - ResultsIndex: 1, + ResultsIndex: &idx1, }}, }, { name: "Test valid expression with multiple object result properties", diff --git a/pkg/apis/pipeline/v1beta1/swagger.json b/pkg/apis/pipeline/v1beta1/swagger.json index 63c4af3d15b..62be890e599 100644 --- a/pkg/apis/pipeline/v1beta1/swagger.json +++ b/pkg/apis/pipeline/v1beta1/swagger.json @@ -1780,8 +1780,7 @@ }, "resultsIndex": { "type": "integer", - "format": "int32", - "default": 0 + "format": "int32" } } }, diff --git a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go index 2dd4fd8edb2..68cce2d2643 100644 --- a/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/pipeline/v1beta1/zz_generated.deepcopy.go @@ -1527,6 +1527,11 @@ func (in *ResolverRef) DeepCopy() *ResolverRef { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ResultRef) DeepCopyInto(out *ResultRef) { *out = *in + if in.ResultsIndex != nil { + in, out := &in.ResultsIndex, &out.ResultsIndex + *out = new(int) + **out = **in + } return } diff --git a/pkg/reconciler/pipelinerun/resources/resultrefresolution.go b/pkg/reconciler/pipelinerun/resources/resultrefresolution.go index 103d587101c..2172a8f8eae 100644 --- a/pkg/reconciler/pipelinerun/resources/resultrefresolution.go +++ b/pkg/reconciler/pipelinerun/resources/resultrefresolution.go @@ -70,8 +70,8 @@ func ResolveResultRefs(pipelineRunState PipelineRunState, targets PipelineRunSta func validateArrayResultsIndex(allResolvedResultRefs ResolvedResultRefs) error { for _, r := range allResolvedResultRefs { if r.Value.Type == v1.ParamTypeArray { - if r.ResultReference.ResultsIndex >= len(r.Value.ArrayVal) { - return 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)) + if r.ResultReference.ResultsIndex != nil && *r.ResultReference.ResultsIndex >= len(r.Value.ArrayVal) { + return 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)) } } } diff --git a/pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go b/pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go index 657805c7e23..ef18e506b13 100644 --- a/pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go +++ b/pkg/reconciler/pipelinerun/resources/resultrefresolution_test.go @@ -41,6 +41,8 @@ var ( Type: apis.ConditionSucceeded, Status: corev1.ConditionFalse, } + idx1 = 1 + idx2 = 2 ) var pipelineRunState = PipelineRunState{{ @@ -375,7 +377,7 @@ func TestResolveResultRefs(t *testing.T) { ResultReference: v1.ResultRef{ PipelineTask: "cTask", Result: "cResult", - ResultsIndex: 1, + ResultsIndex: &idx1, }, FromTaskRun: "cTaskRun", }}, @@ -391,7 +393,6 @@ func TestResolveResultRefs(t *testing.T) { ResultReference: v1.ResultRef{ PipelineTask: "cTask", Result: "cResult", - ResultsIndex: 0, }, FromTaskRun: "cTaskRun", }, { @@ -399,7 +400,6 @@ func TestResolveResultRefs(t *testing.T) { ResultReference: v1.ResultRef{ PipelineTask: "dTask", Result: "dResult", - ResultsIndex: 0, }, FromTaskRun: "dTaskRun", }}, @@ -753,7 +753,21 @@ func TestValidateArrayResultsIndex(t *testing.T) { ResultReference: v1.ResultRef{ PipelineTask: "aTask", Result: "aResult", - ResultsIndex: 1, + ResultsIndex: nil, + }, + FromTaskRun: "aTaskRun", + }}, + }, { + name: "Reference an Empty Array", + refs: ResolvedResultRefs{{ + Value: v1.ResultValue{ + Type: "array", + ArrayVal: []string{}, + }, + ResultReference: v1.ResultRef{ + PipelineTask: "aTask", + Result: "aResult", + ResultsIndex: &idx1, }, FromTaskRun: "aTaskRun", }}, @@ -768,7 +782,7 @@ func TestValidateArrayResultsIndex(t *testing.T) { ResultReference: v1.ResultRef{ PipelineTask: "aTask", Result: "aResult", - ResultsIndex: 1, + ResultsIndex: &idx1, }, FromTaskRun: "aTaskRun", }}, @@ -778,16 +792,16 @@ func TestValidateArrayResultsIndex(t *testing.T) { refs: ResolvedResultRefs{{ Value: v1.ResultValue{ Type: "array", - ArrayVal: []string{"a", "b", "c"}, + ArrayVal: []string{"a", "b"}, }, ResultReference: v1.ResultRef{ PipelineTask: "aTask", Result: "aResult", - ResultsIndex: 3, + ResultsIndex: &idx2, }, FromTaskRun: "aTaskRun", }}, - wantErr: "array Result Index 3 for Task aTask Result aResult is out of bound of size 3", + wantErr: "array Result Index 2 for Task aTask Result aResult is out of bound of size 2", }, { name: "In Bounds and Out of Bounds Array", refs: ResolvedResultRefs{{ @@ -798,22 +812,22 @@ func TestValidateArrayResultsIndex(t *testing.T) { ResultReference: v1.ResultRef{ PipelineTask: "aTask", Result: "aResult", - ResultsIndex: 1, + ResultsIndex: &idx1, }, FromTaskRun: "aTaskRun", }, { Value: v1.ResultValue{ Type: "array", - ArrayVal: []string{"a", "b", "c"}, + ArrayVal: []string{"a", "b"}, }, ResultReference: v1.ResultRef{ PipelineTask: "aTask", Result: "aResult", - ResultsIndex: 3, + ResultsIndex: &idx2, }, FromTaskRun: "aTaskRun", }}, - wantErr: "array Result Index 3 for Task aTask Result aResult is out of bound of size 3", + wantErr: "array Result Index 2 for Task aTask Result aResult is out of bound of size 2", }} { t.Run(tt.name, func(t *testing.T) { err := validateArrayResultsIndex(tt.refs)