Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TEP-0075]Pipeline results support object #5088

Merged
merged 1 commit into from
Jul 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion docs/pipelines.md
Original file line number Diff line number Diff line change
Expand Up @@ -1001,7 +1001,20 @@ results:

For an end-to-end example, see [`Results` in a `PipelineRun`](../examples/v1beta1/pipelineruns/pipelinerun-results.yaml).

Array results is supported as alpha feature, see [`Array Results` in a `PipelineRun`](../examples/v1beta1/pipelineruns/alpha/pipelinerun-array-results.yaml).
Array and object results is supported as alpha feature, see [`Array Results` in a `PipelineRun`](../examples/v1beta1/pipelineruns/alpha/pipeline-emitting-results.yaml).

```yaml
results:
- name: object-results
type: object
description: whole object
value: $(tasks.task2.results.object-results[*])
- name: object-element
type: string
description: object element
value: $(tasks.task2.results.object-results.foo)
```


A `Pipeline Result` is not emitted if any of the following are true:
- A `PipelineTask` referenced by the `Pipeline Result` failed. The `PipelineRun` will also
Expand Down
40 changes: 0 additions & 40 deletions examples/v1beta1/pipelineruns/alpha/pipeline-array-results.yaml

This file was deleted.

61 changes: 61 additions & 0 deletions examples/v1beta1/pipelineruns/alpha/pipeline-emitting-results.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
apiVersion: tekton.dev/v1beta1
kind: PipelineRun
metadata:
name: pipelinerun-array-indexing-results
spec:
pipelineSpec:
tasks:
- name: task1
taskSpec:
results:
- name: array-results
type: array
description: The array results
steps:
- name: write-array
image: bash:latest
script: |
#!/usr/bin/env bash
echo -n "[\"1\",\"2\",\"3\"]" | tee $(results.array-results.path)
- name: task2
taskSpec:
results:
- name: object-results
type: object
description: The object results
properties:
foo: {
type: string
}
hello: {
type: string
}
steps:
- name: write-array
image: bash:latest
script: |
#!/usr/bin/env bash
echo -n "{\"foo\":\"bar\",\"hello\":\"world\"}" | tee $(results.object-results.path)
results:
- name: array-results
type: array
description: whole array
value: $(tasks.task1.results.array-results[*])
- name: array-indexing-results
Yongxuanzhang marked this conversation as resolved.
Show resolved Hide resolved
type: string
description: array element
value: $(tasks.task1.results.array-results[1])
- name: object-results
type: object
description: whole object
value: $(tasks.task2.results.object-results[*])
- name: object-results-from-array-indexing-and-object-elements
type: object
description: whole object
value:
key1: $(tasks.task1.results.array-results[1])
key2: $(tasks.task2.results.object-results.hello)
- name: object-element
type: string
description: object element
value: $(tasks.task2.results.object-results.foo)
7 changes: 1 addition & 6 deletions pkg/apis/pipeline/v1beta1/param_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ func (arrayOrString *ArrayOrString) applyOrCorrect(stringReplacements map[string

// trim the head "$(" and the tail ")" or "[*])"
// i.e. get "params.name" from "$(params.name)" or "$(params.name[*])"
trimedStringVal := StripStarVarSubExpression(stringVal)
trimedStringVal := substitution.StripStarVarSubExpression(stringVal)

// if the stringVal is a reference to a string param
if _, ok := stringReplacements[trimedStringVal]; ok {
Expand All @@ -250,11 +250,6 @@ func (arrayOrString *ArrayOrString) applyOrCorrect(stringReplacements map[string
}
}

// StripStarVarSubExpression strips "$(target[*])"" to get "target"
func StripStarVarSubExpression(s string) string {
return strings.TrimSuffix(strings.TrimSuffix(strings.TrimPrefix(s, "$("), ")"), "[*]")
}

// NewArrayOrString creates an ArrayOrString of type ParamTypeString or ParamTypeArray, based on
// how many inputs are given (>1 input will create an array, not string).
func NewArrayOrString(value string, values ...string) *ArrayOrString {
Expand Down
3 changes: 3 additions & 0 deletions pkg/apis/pipeline/v1beta1/resultref.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.ObjectVal {
allExpressions = append(allExpressions, validateString(v)...)
}
return allExpressions, len(allExpressions) != 0
}

Expand Down
38 changes: 38 additions & 0 deletions pkg/apis/pipeline/v1beta1/resultref_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -718,3 +718,41 @@ func TestParseResultName(t *testing.T) {
})
}
}

func TestGetVarSubstitutionExpressionsForPipelineResult(t *testing.T) {
tests := []struct {
name string
result v1beta1.PipelineResult
want []string
}{{
name: "get string result expressions",
result: v1beta1.PipelineResult{
Name: "string result",
Type: v1beta1.ResultsTypeString,
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"},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
get, _ := v1beta1.GetVarSubstitutionExpressionsForPipelineResult(tt.result)
if d := cmp.Diff(tt.want, get); d != "" {
t.Error(diff.PrintWantGot(d))
}
})
}
}
6 changes: 5 additions & 1 deletion pkg/reconciler/pipelinerun/pipelinerun.go
Original file line number Diff line number Diff line change
Expand Up @@ -612,8 +612,12 @@ func (c *Reconciler) reconcile(ctx context.Context, pr *v1beta1.PipelineRun, get

pr.Status.SkippedTasks = pipelineRunFacts.GetSkippedTasks()
if after.Status == corev1.ConditionTrue || after.Status == corev1.ConditionFalse {
pr.Status.PipelineResults = resources.ApplyTaskResultsToPipelineResults(pipelineSpec.Results,
pr.Status.PipelineResults, err = resources.ApplyTaskResultsToPipelineResults(pipelineSpec.Results,
pipelineRunFacts.State.GetTaskRunsResults(), pipelineRunFacts.State.GetRunsResults())
if err != nil {
pr.Status.MarkFailed(ReasonFailedValidation, err.Error())
return err
}
}

logger.Infof("PipelineRun %s status is being set to %s", pr.Name, after)
Expand Down
66 changes: 56 additions & 10 deletions pkg/reconciler/pipelinerun/resources/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,14 @@ import (
"github.com/tektoncd/pipeline/pkg/substitution"
)

const (
// resultsParseNumber is the value of how many parts we split from result reference. e.g. tasks.<taskName>.results.<objectResultName>
resultsParseNumber = 4
// objectElementResultsParseNumber is the value of how many parts we split from
// object attribute result reference. e.g. tasks.<taskName>.results.<objectResultName>.<individualAttribute>
objectElementResultsParseNumber = 5
)

// ApplyParameters applies the params from a PipelineRun.Params to a PipelineSpec.
func ApplyParameters(ctx context.Context, p *v1beta1.PipelineSpec, pr *v1beta1.PipelineRun) *v1beta1.PipelineSpec {
// This assumes that the PipelineRun inputs have been validated against what the Pipeline requests.
Expand Down Expand Up @@ -251,13 +259,17 @@ func replaceParamValues(params []v1beta1.Param, stringReplacements map[string]st
func ApplyTaskResultsToPipelineResults(
results []v1beta1.PipelineResult,
taskRunResults map[string][]v1beta1.TaskRunResult,
customTaskResults map[string][]v1alpha1.RunResult) []v1beta1.PipelineRunResult {

customTaskResults map[string][]v1alpha1.RunResult) ([]v1beta1.PipelineRunResult, error) {
var runResults []v1beta1.PipelineRunResult
var invalidPipelineResults []string
stringReplacements := map[string]string{}
arrayReplacements := map[string][]string{}
objectReplacements := map[string]map[string]string{}
for _, pipelineResult := range results {
variablesInPipelineResult, _ := v1beta1.GetVarSubstitutionExpressionsForPipelineResult(pipelineResult)
if len(variablesInPipelineResult) == 0 {
continue
}
validPipelineResult := true
for _, variable := range variablesInPipelineResult {
if _, isMemoized := stringReplacements[variable]; isMemoized {
Expand All @@ -266,10 +278,20 @@ func ApplyTaskResultsToPipelineResults(
if _, isMemoized := arrayReplacements[variable]; isMemoized {
continue
}
// TODO(#4723): Need to consider object case.
// e.g.: tasks.taskname.results.resultname.objectkey
if _, isMemoized := objectReplacements[variable]; isMemoized {
continue
}
variableParts := strings.Split(variable, ".")
if len(variableParts) == 4 && variableParts[0] == "tasks" && variableParts[2] == "results" {
if variableParts[0] != v1beta1.ResultTaskPart || variableParts[2] != v1beta1.ResultResultPart {
validPipelineResult = false
invalidPipelineResults = append(invalidPipelineResults, pipelineResult.Name)
continue
}
switch len(variableParts) {
// For string result: tasks.<taskName>.results.<objectResultName>
// For array result: tasks.<taskName>.results.<objectResultName>[*], tasks.<taskName>.results.<objectResultName>[i]
// For object result: tasks.<taskName>.results.<objectResultName>[*],
case resultsParseNumber:
taskName, resultName := variableParts[1], variableParts[3]
resultName, stringIdx := v1beta1.ParseResultName(resultName)
if resultValue := taskResultValue(taskName, resultName, taskRunResults); resultValue != nil {
Expand All @@ -279,31 +301,55 @@ func ApplyTaskResultsToPipelineResults(
case v1beta1.ParamTypeArray:
if stringIdx != "*" {
intIdx, _ := strconv.Atoi(stringIdx)
stringReplacements[variable] = resultValue.ArrayVal[intIdx]
if intIdx < len(resultValue.ArrayVal) {
stringReplacements[variable] = resultValue.ArrayVal[intIdx]
}
} else {
arrayReplacements[v1beta1.StripStarVarSubExpression(variable)] = resultValue.ArrayVal
arrayReplacements[substitution.StripStarVarSubExpression(variable)] = resultValue.ArrayVal
}
case v1beta1.ParamTypeObject:
objectReplacements[substitution.StripStarVarSubExpression(variable)] = resultValue.ObjectVal
}
} else if resultValue := runResultValue(taskName, resultName, customTaskResults); resultValue != nil {
stringReplacements[variable] = *resultValue
} else {
// referred array index out of bound
invalidPipelineResults = append(invalidPipelineResults, pipelineResult.Name)
validPipelineResult = false
}
} else {
// For object type result: tasks.<taskName>.results.<objectResultName>.<individualAttribute>
case objectElementResultsParseNumber:
taskName, resultName, objectKey := variableParts[1], variableParts[3], variableParts[4]
resultName, _ = v1beta1.ParseResultName(resultName)
if resultValue := taskResultValue(taskName, resultName, taskRunResults); resultValue != nil {
if _, ok := resultValue.ObjectVal[objectKey]; ok {
stringReplacements[variable] = resultValue.ObjectVal[objectKey]
} else {
// referred object key is not existent
invalidPipelineResults = append(invalidPipelineResults, pipelineResult.Name)
validPipelineResult = false
}
}
default:
invalidPipelineResults = append(invalidPipelineResults, pipelineResult.Name)
validPipelineResult = false
}
}
if validPipelineResult {
finalValue := pipelineResult.Value
finalValue.ApplyReplacements(stringReplacements, arrayReplacements, nil)
finalValue.ApplyReplacements(stringReplacements, arrayReplacements, objectReplacements)
runResults = append(runResults, v1beta1.PipelineRunResult{
Name: pipelineResult.Name,
Value: finalValue,
})
}
}

return runResults
if len(invalidPipelineResults) > 0 {
return runResults, fmt.Errorf("invalid pipelineresults %v, the referred results don't exist", invalidPipelineResults)
}

return runResults, nil
Comment on lines +348 to +352
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a different behaviour, it may be easier to do like this, during the apply we can check if the results reference is valid.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like something that would be good to split out into a separate PR

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the part of the validation, I remember we also need validation for this PR?
Or should I split the validation into another PR?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh I see, that's fine-- also I think it's generally OK to add validation in an earlier PR (as opposed to a later one)

}

// taskResultValue returns the result value for a given pipeline task name and result name in a map of TaskRunResults for
Expand Down
Loading