From d217d36c23307e9463184d228e590959dea2b742 Mon Sep 17 00:00:00 2001 From: Chitrang Patel Date: Wed, 15 Nov 2023 11:56:56 -0500 Subject: [PATCH] Passing StepResults between Steps --- .../alpha/stepaction-passing-results.yaml | 34 +++++ pkg/apis/pipeline/v1/pipeline_validation.go | 2 +- pkg/apis/pipeline/v1/resultref.go | 69 +++++++--- pkg/entrypoint/entrypointer.go | 120 ++++++++++++++++++ pkg/pod/entrypoint.go | 4 +- pkg/pod/status.go | 4 +- pkg/reconciler/taskrun/resources/apply.go | 85 +++++++++++++ 7 files changed, 295 insertions(+), 23 deletions(-) create mode 100644 examples/v1/taskruns/alpha/stepaction-passing-results.yaml diff --git a/examples/v1/taskruns/alpha/stepaction-passing-results.yaml b/examples/v1/taskruns/alpha/stepaction-passing-results.yaml new file mode 100644 index 00000000000..63f4d759e01 --- /dev/null +++ b/examples/v1/taskruns/alpha/stepaction-passing-results.yaml @@ -0,0 +1,34 @@ +apiVersion: tekton.dev/v1alpha1 +kind: StepAction +metadata: + name: step-action +spec: + params: + - name: param1 + type: array + image: bash:3.2 + command: ["echo"] + args: [ + "$(params.param1[*])", + ] +--- +apiVersion: tekton.dev/v1 +kind: TaskRun +metadata: + name: step-action-run +spec: + TaskSpec: + steps: + - name: inline-step + results: + - name: result1 + type: array + image: alpine + script: | + echo -n "[\"image1\", \"image2\", \"image3\"]" | tee $(step.results.result1.path) + - name: action-runner + ref: + name: step-action + params: + - name: param1 + value: $(steps.inline-step.results.result1[*]) diff --git a/pkg/apis/pipeline/v1/pipeline_validation.go b/pkg/apis/pipeline/v1/pipeline_validation.go index 2dc3b884f7d..b7c51e7fcd0 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1/pipeline_validation.go @@ -684,7 +684,7 @@ func taskContainsResult(resultExpression string, pipelineTaskNames sets.String, for _, expression := range split { if expression != "" { value := stripVarSubExpression("$" + expression) - pipelineTaskName, _, _, _, err := parseExpression(value) + pipelineTaskName, _, _, _, _, err := parseExpression(value) if err != nil { return false diff --git a/pkg/apis/pipeline/v1/resultref.go b/pkg/apis/pipeline/v1/resultref.go index 2de6bb80804..a46c727f983 100644 --- a/pkg/apis/pipeline/v1/resultref.go +++ b/pkg/apis/pipeline/v1/resultref.go @@ -37,6 +37,8 @@ const ( // 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..results.." + // ResultStepPart Constant used to define the "steps" part of a step result reference + ResultStepPart = "steps" // ResultTaskPart Constant used to define the "tasks" part of a pipeline result reference ResultTaskPart = "tasks" // ResultFinallyPart Constant used to define the "finally" part of a pipeline result reference @@ -69,9 +71,9 @@ var arrayIndexingRegex = regexp.MustCompile(arrayIndexing) func NewResultRefs(expressions []string) []*ResultRef { var resultRefs []*ResultRef for _, expression := range expressions { - pipelineTask, result, index, property, err := parseExpression(expression) + pipelineTask, result, index, property, _, err := parseTaskExpression(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, + // parseTaskExpression 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{ @@ -105,6 +107,13 @@ func looksLikeResultRef(expression string) bool { return len(subExpressions) >= 4 && (subExpressions[0] == ResultTaskPart || subExpressions[0] == ResultFinallyPart) && subExpressions[2] == ResultResultPart } +// looksLikeStepResultRef attempts to check if the given string looks like it contains any +// step result references. Returns true if it does, false otherwise +func looksLikeStepResultRef(expression string) bool { + subExpressions := strings.Split(expression, ".") + return len(subExpressions) >= 4 && subExpressions[0] == ResultStepPart && subExpressions[2] == ResultResultPart +} + func validateString(value string) []string { expressions := VariableSubstitutionRegex.FindAllString(value, -1) if expressions == nil { @@ -121,38 +130,62 @@ func stripVarSubExpression(expression string) string { return strings.TrimSuffix(strings.TrimPrefix(expression, "$("), ")") } -// parseExpression parses "task name", "result name", "array index" (iff it's an array result) and "object key name" (iff it's an object result) +// parseExpression parses "task name" or "step 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 +// - Input: steps.myStep.results.aStringResult +// - Output: "myStep", "aStringResult", -1, "", nil // Valid Example 2: // - Input: tasks.myTask.results.anObjectResult.key1 // - Output: "myTask", "anObjectResult", 0, "key1", nil +// - Input: steps.myStep.results.anObjectResult.key1 +// - Output: "myStep", "anObjectResult", 0, "key1", nil // Valid Example 3: // - Input: tasks.myTask.results.anArrayResult[1] // - Output: "myTask", "anArrayResult", 1, "", nil +// - Input: steps.myStep.results.anArrayResult[1] +// - Output: "myStep", "anArrayResult", 1, "", nil // Invalid Example 1: // - Input: tasks.myTask.results.resultName.foo.bar // - Output: "", "", 0, "", error +// - Input: steps.myStep.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) { - 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 != "" { - intIdx, _ := strconv.Atoi(stringIdx) - return subExpressions[1], resultName, intIdx, "", nil +func parseExpression(substitutionExpression string) (string, string, int, string, ParamType, error) { + subExpressions := strings.Split(substitutionExpression, ".") + // For string result: tasks..results. or steps..results. + // For array result: tasks..results.[index] or steps..results.[index] + if len(subExpressions) == 4 { + resultName, stringIdx := ParseResultName(subExpressions[3]) + if stringIdx != "" { + if stringIdx == "*" { + return subExpressions[1], resultName, -1, "", ParamTypeArray, nil } - return subExpressions[1], resultName, 0, "", nil - } else if len(subExpressions) == 5 { - // For object type result: tasks..results.. - return subExpressions[1], subExpressions[3], 0, subExpressions[4], nil + intIdx, _ := strconv.Atoi(stringIdx) + return subExpressions[1], resultName, intIdx, "", ParamTypeArray, nil } + return subExpressions[1], resultName, 0, "", ParamTypeString, nil + } else if len(subExpressions) == 5 { + // For object type result: tasks..results.. + // or steps..results.. + return subExpressions[1], subExpressions[3], 0, subExpressions[4], ParamTypeObject, nil + } + return "", "", 0, "", ParamTypeString, fmt.Errorf("must be one of the form 1). %q; 2). %q", resultExpressionFormat, objectResultExpressionFormat) +} + +func parseTaskExpression(substitutionExpression string) (string, string, int, string, ParamType, error) { + if looksLikeResultRef(substitutionExpression) { + return parseExpression(substitutionExpression) + } + return "", "", 0, "", ParamTypeString, fmt.Errorf("must be one of the form 1). %q; 2). %q", resultExpressionFormat, objectResultExpressionFormat) +} + +func ParseStepExpression(substitutionExpression string) (string, string, int, string, ParamType, error) { + if looksLikeStepResultRef(substitutionExpression) { + return parseExpression(substitutionExpression) } - return "", "", 0, "", fmt.Errorf("must be one of the form 1). %q; 2). %q", resultExpressionFormat, objectResultExpressionFormat) + return "", "", 0, "", ParamTypeString, 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/entrypoint/entrypointer.go b/pkg/entrypoint/entrypointer.go index c90b6d0960c..f0ec0cfe470 100644 --- a/pkg/entrypoint/entrypointer.go +++ b/pkg/entrypoint/entrypointer.go @@ -24,6 +24,7 @@ import ( "os" "os/exec" "path/filepath" + "regexp" "strconv" "strings" "syscall" @@ -31,6 +32,7 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline" + v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "github.com/tektoncd/pipeline/pkg/pod" "github.com/tektoncd/pipeline/pkg/result" "github.com/tektoncd/pipeline/pkg/spire" @@ -182,6 +184,9 @@ func (e Entrypointer) Go() error { ctx := context.Background() var cancel context.CancelFunc if err == nil { + if err := e.applyStepResultSubstitutions(); err != nil { + logger.Error("Error while substituting step results: ", err) + } ctx, cancel = context.WithCancel(ctx) if e.Timeout != nil && *e.Timeout > time.Duration(0) { ctx, cancel = context.WithTimeout(ctx, *e.Timeout) @@ -336,3 +341,118 @@ func (e Entrypointer) waitingCancellation(ctx context.Context, cancel context.Ca cancel() return nil } + +// loadStepResult reads the step result file and returns the string, array or object result value. +func loadStepResult(stepName string, resultName string) (v1.ResultValue, error) { + v := v1.ResultValue{} + fileContents, err := os.ReadFile(getStepResultPath(pod.GetContainerName(stepName), resultName)) + if err != nil { + return v, err + } + err = v.UnmarshalJSON(fileContents) + if err != nil { + return v, err + } + return v, nil +} + +// getStepResultPath gets the path to the step result +func getStepResultPath(stepName string, resultName string) string { + return filepath.Join(pipeline.StepsDir, stepName, "results", resultName) +} + +func findReplacement(s string) (string, []string, error) { + value := strings.TrimSuffix(strings.TrimPrefix(s, "$("), ")") + stepName, resultName, arrayIdx, objectKey, paramType, err := v1.ParseStepExpression(value) + if err != nil { + return "", nil, err + } + result, err := loadStepResult(stepName, resultName) + if err != nil { + return "", nil, err + } + replaceWithArray := []string{} + replaceWithString := "" + if paramType == v1.ParamTypeObject && objectKey != "" { + replaceWithString = result.ObjectVal[objectKey] + } else if paramType == v1.ParamTypeArray { + if arrayIdx == -1 { + replaceWithArray = append(replaceWithArray, result.ArrayVal...) + } else { + replaceWithString = result.ArrayVal[arrayIdx] + } + } else { + replaceWithString = result.StringVal + } + return replaceWithString, replaceWithArray, nil +} + +// applyStepResultSubstitutions applies the runtime step result substitutions in env. +func (e *Entrypointer) applyStepResultSubstitutions() error { + pattern := `\$\(steps\..*\.results\..*\)` + stepResultRegex := regexp.MustCompile(pattern) + // env + for _, e := range os.Environ() { + pair := strings.SplitN(e, "=", 2) + matches := stepResultRegex.FindAllStringSubmatch(pair[1], -1) + for _, m := range matches { + replaceWith, _, err := findReplacement(m[0]) + if err != nil { + return err + } + os.Setenv(pair[0], strings.ReplaceAll(pair[1], m[0], replaceWith)) + } + } + // script + if strings.HasPrefix(e.Command[0], "/tekton/scripts") { + fileContentBytes, err := os.ReadFile(e.Command[0]) + if err != nil { + return err + } + fileContents := string(fileContentBytes) + matches := stepResultRegex.FindAllStringSubmatch(fileContents, -1) + if len(matches) > 0 { + for _, m := range matches { + replaceWith, _, err := findReplacement(m[0]) + if err != nil { + return err + } + fileContents = strings.ReplaceAll(fileContents, m[0], replaceWith) + } + // copy the modified contents to a different file. + newFilePath := filepath.Join(e.StepMetadataDir, "script") + err := os.WriteFile(newFilePath, []byte(fileContents), 0755) + if err != nil { + return err + } + // set the command to execute the new file. + e.Command[0] = newFilePath + } + } + // command + args + newCommand := []string{} + for _, c := range e.Command { + matches := stepResultRegex.FindAllStringSubmatch(c, -1) + if len(matches) > 0 { + for _, m := range matches { + replaceWithString, replaceWithArray, err := findReplacement(m[0]) + if err != nil { + return err + } + // if replacing with an array + if len(replaceWithArray) > 1 { + // append with the array items + newCommand = append(newCommand, replaceWithArray...) + } else { + // append with replaced string + c = strings.ReplaceAll(c, m[0], replaceWithString) + newCommand = append(newCommand, c) + } + } + } else { + newCommand = append(newCommand, c) + } + } + e.Command = newCommand + return nil +} diff --git a/pkg/pod/entrypoint.go b/pkg/pod/entrypoint.go index 72096adb72b..e6235f7ade3 100644 --- a/pkg/pod/entrypoint.go +++ b/pkg/pod/entrypoint.go @@ -353,11 +353,11 @@ func TrimSidecarPrefix(name string) string { return strings.TrimPrefix(name, sid // returns "step-unnamed-" if not specified func StepName(name string, i int) string { if name != "" { - return getContainerName(name) + return GetContainerName(name) } return fmt.Sprintf("%sunnamed-%d", stepPrefix, i) } -func getContainerName(name string) string { +func GetContainerName(name string) string { return fmt.Sprintf("%s%s", stepPrefix, name) } diff --git a/pkg/pod/status.go b/pkg/pod/status.go index 5fa850fb8ca..cecba308dc1 100644 --- a/pkg/pod/status.go +++ b/pkg/pod/status.go @@ -247,7 +247,7 @@ func setTaskRunStatusBasedOnStepStatus(ctx context.Context, logger *zap.SugaredL stepResults := []v1.StepResult{} if ts != nil { for _, step := range ts.Steps { - if getContainerName(step.Name) == s.Name { + if GetContainerName(step.Name) == s.Name { stepResults = append(stepResults, step.Results...) } } @@ -359,7 +359,7 @@ func findStepResultsFetchedByTask(containerName string, specResults []v1.TaskRes return nil, err } // Only look at named results - referencing unnamed steps is unsupported. - if getContainerName(sName) == containerName { + if GetContainerName(sName) == containerName { neededStepResults[resultName] = r.Name } } diff --git a/pkg/reconciler/taskrun/resources/apply.go b/pkg/reconciler/taskrun/resources/apply.go index 40fac760cc2..7acd8b83595 100644 --- a/pkg/reconciler/taskrun/resources/apply.go +++ b/pkg/reconciler/taskrun/resources/apply.go @@ -20,7 +20,9 @@ import ( "context" "fmt" "path/filepath" + "regexp" "strconv" + "strings" "github.com/tektoncd/pipeline/pkg/apis/pipeline" v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" @@ -44,6 +46,12 @@ var ( // FIXME(vdemeester) Remove that with deprecating v1beta1 "inputs.params.%s", } + + paramIndexRegexPatterns = []string{ + `\$\(params.%s\[([0-9]*)*\*?\]\)`, + `\$\(params[%q]\[([0-9]*)*\*?\]\)`, + `\$\(params['%s']\[([0-9]*)*\*?\]\)`, + } ) // applyStepActionParameters applies the params from the Task and the underlying Step to the referenced StepAction. @@ -64,10 +72,87 @@ func applyStepActionParameters(step *v1.Step, spec *v1.TaskSpec, tr *v1.TaskRun, arrayReplacements[k] = v } + stepResultReplacements, _ := replacementsFromStepResults(step, stepParams, defaults) + for k, v := range stepResultReplacements { + stringReplacements[k] = v + } container.ApplyStepReplacements(step, stringReplacements, arrayReplacements) return step } +// findArrayIndexParamUsage finds the array index in a string using array param substitution +func findArrayIndexParamUsage(s string, paramName string, stepName string, resultName string, stringReplacements map[string]string) map[string]string { + for _, pattern := range paramIndexRegexPatterns { + arrayIndexingRegex := regexp.MustCompile(fmt.Sprintf(pattern, paramName)) + matches := arrayIndexingRegex.FindAllStringSubmatch(s, -1) + for _, match := range matches { + if len(match) == 2 { + key := strings.TrimSuffix(strings.TrimPrefix(match[0], "$("), ")") + if match[1] != "" { + stringReplacements[key] = fmt.Sprintf("$(steps.%s.results.%s[%s])", stepName, resultName, match[1]) + } + } + } + } + return stringReplacements +} + +// replacementsArrayIdxStepResults looks for Step Result array usage with index in the Step's command, args, env and script. +func replacementsArrayIdxStepResults(step *v1.Step, paramName string, stepName string, resultName string) map[string]string { + stringReplacements := map[string]string{} + for _, c := range step.Command { + stringReplacements = findArrayIndexParamUsage(c, paramName, stepName, resultName, stringReplacements) + } + for _, a := range step.Args { + stringReplacements = findArrayIndexParamUsage(a, paramName, stepName, resultName, stringReplacements) + } + stringReplacements = findArrayIndexParamUsage(step.Script, paramName, stepName, resultName, stringReplacements) + for _, e := range step.Env { + stringReplacements = findArrayIndexParamUsage(e.Value, paramName, stepName, resultName, stringReplacements) + } + return stringReplacements +} + +// replacementsFromStepResults generates string replacements for params whose values is a variable substitution of a step result. +func replacementsFromStepResults(step *v1.Step, stepParams v1.Params, defaults []v1.ParamSpec) (map[string]string, error) { + stringReplacements := map[string]string{} + for _, sp := range stepParams { + if sp.Value.StringVal != "" { + // $(params.p1) --> $(steps.step1.results.foo) (normal substitution) + value := strings.TrimSuffix(strings.TrimPrefix(sp.Value.StringVal, "$("), ")") + stepName, resultName, _, _, _, err := v1.ParseStepExpression(value) + if err != nil { + return nil, err + } + for _, d := range defaults { + if d.Name == sp.Name { + switch d.Type { + case v1.ParamTypeObject: + for k := range d.Properties { + stringReplacements[fmt.Sprintf("params.%s.%s", d.Name, k)] = fmt.Sprintf("$(steps.%s.results.%s.%s)", stepName, resultName, k) + } + case v1.ParamTypeArray: + // $(params.p1[*]) --> $(steps.step1.results.foo) + for _, pattern := range paramPatterns { + stringReplacements[fmt.Sprintf(pattern+"[*]", d.Name)] = fmt.Sprintf("$(steps.%s.results.%s[*])", stepName, resultName) + } + // $(params.p1[idx]) --> $(steps.step1.results.foo[idx]) + for k, v := range replacementsArrayIdxStepResults(step, d.Name, stepName, resultName) { + stringReplacements[k] = v + } + // This is handled by normal param substitution. + // $(params.p1.key) --> $(steps.step1.results.foo) + case v1.ParamTypeString: + // Since String is the default, This is handled by normal param substitution. + default: + } + } + } + } + } + return stringReplacements, nil +} + // getTaskParameters gets the string, array and object parameter variable replacements needed in the Task func getTaskParameters(spec *v1.TaskSpec, tr *v1.TaskRun, defaults ...v1.ParamSpec) (map[string]string, map[string][]string, map[string]map[string]string) { // This assumes that the TaskRun inputs have been validated against what the Task requests.