From 8bdd14561d77301d663cf1b5d1dbc679d6a79ef4 Mon Sep 17 00:00:00 2001 From: Chitrang Patel Date: Wed, 15 Nov 2023 11:56:56 -0500 Subject: [PATCH] cleanup debug example passes tests added result ref coverage up entrypointer tests added --- .../alpha/stepaction-passing-results.yaml | 66 +++++ pkg/apis/pipeline/v1/pipeline_validation.go | 2 +- pkg/apis/pipeline/v1/resultref.go | 53 +++- pkg/apis/pipeline/v1/resultref_test.go | 121 +++++++++ pkg/entrypoint/entrypointer.go | 152 +++++++++++ pkg/entrypoint/entrypointer_test.go | 246 ++++++++++++++++++ pkg/pod/entrypoint.go | 4 +- pkg/pod/status.go | 4 +- pkg/reconciler/taskrun/resources/apply.go | 85 ++++++ .../taskrun/resources/taskspec_test.go | 92 +++++++ 10 files changed, 809 insertions(+), 16 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..18a2e7214a5 --- /dev/null +++ b/examples/v1/taskruns/alpha/stepaction-passing-results.yaml @@ -0,0 +1,66 @@ +apiVersion: tekton.dev/v1alpha1 +kind: StepAction +metadata: + name: step-action +spec: + params: + - name: param1 + type: array + - name: param2 + type: string + - name: param3 + type: object + properties: + IMAGE_URL: + type: string + IMAGE_DIGEST: + type: string + image: bash:3.2 + env: + - name: STRINGPARAM + value: $(params.param2) + args: [ + "$(params.param1[*])", + "$(params.param1[0])", + "$(params.param3.IMAGE_URL)", + "$(params.param3.IMAGE_DIGEST)", + ] + script: | + echo "$@" + echo "$STRINGPARAM" +--- +apiVersion: tekton.dev/v1 +kind: TaskRun +metadata: + name: step-action-run +spec: + TaskSpec: + steps: + - name: inline-step + results: + - name: result1 + type: array + - name: result2 + type: string + - name: result3 + type: object + properties: + IMAGE_URL: + type: string + IMAGE_DIGEST: + type: string + image: alpine + script: | + echo -n "[\"image1\", \"image2\", \"image3\"]" | tee $(step.results.result1.path) + echo -n "foo" | tee $(step.results.result2.path) + echo -n "{\"IMAGE_URL\":\"ar.com\", \"IMAGE_DIGEST\":\"sha234\"}" | tee $(step.results.result3.path) + - name: action-runner + ref: + name: step-action + params: + - name: param1 + value: $(steps.inline-step.results.result1[*]) + - name: param2 + value: $(steps.inline-step.results.result2) + - name: param3 + value: $(steps.inline-step.results.result3[*]) 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 3948581f6d6..86e664da707 100644 --- a/pkg/apis/pipeline/v1/resultref.go +++ b/pkg/apis/pipeline/v1/resultref.go @@ -32,11 +32,15 @@ type ResultRef struct { } const ( - resultExpressionFormat = "tasks..results." + resultExpressionFormat = "tasks..results." + stepResultExpressionFormat = "steps..results." // Result expressions of the form . 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..results.." + objectResultExpressionFormat = "tasks..results.." + objectStepResultExpressionFormat = "steps..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 +73,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 +109,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 { @@ -138,24 +149,44 @@ func stripVarSubExpression(expression string) string { // - Input: tasks.myTask.results.resultName.foo.bar // - Output: "", "", nil, "", 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) { +func parseExpression(substitutionExpression string) (string, string, *int, string, ParamType, error) { + if looksLikeResultRef(substitutionExpression) || looksLikeStepResultRef(substitutionExpression) { subExpressions := strings.Split(substitutionExpression, ".") // For string result: tasks..results. + // For string step result: steps..results. // For array result: tasks..results.[index] + // For array step result: steps..results.[index] if len(subExpressions) == 4 { resultName, stringIdx := ParseResultName(subExpressions[3]) - if stringIdx != "" && stringIdx != "*" { + if stringIdx != "" { + if stringIdx == "*" { + return subExpressions[1], resultName, nil, "", ParamTypeArray, nil + } intIdx, _ := strconv.Atoi(stringIdx) - return subExpressions[1], resultName, &intIdx, "", nil + return subExpressions[1], resultName, &intIdx, "", ParamTypeArray, nil } - return subExpressions[1], resultName, nil, "", nil + return subExpressions[1], resultName, nil, "", ParamTypeString, nil } else if len(subExpressions) == 5 { // For object type result: tasks..results.. - return subExpressions[1], subExpressions[3], nil, subExpressions[4], nil + // For object type step result: steps..results.. + return subExpressions[1], subExpressions[3], nil, subExpressions[4], ParamTypeObject, nil } } - return "", "", nil, "", fmt.Errorf("must be one of the form 1). %q; 2). %q", resultExpressionFormat, objectResultExpressionFormat) + return "", "", nil, "", ParamTypeString, fmt.Errorf("must be one of the form 1). %q; 2). %q; 3). %q; 4). %q", resultExpressionFormat, objectResultExpressionFormat, stepResultExpressionFormat, objectStepResultExpressionFormat) +} + +func parseTaskExpression(substitutionExpression string) (string, string, *int, string, ParamType, error) { + if looksLikeResultRef(substitutionExpression) { + return parseExpression(substitutionExpression) + } + return "", "", nil, "", 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 "", "", nil, "", ParamTypeString, fmt.Errorf("must be one of the form 1). %q; 2). %q", stepResultExpressionFormat, objectStepResultExpressionFormat) } // 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 22f0dba5f3d..ff9a6c69293 100644 --- a/pkg/apis/pipeline/v1/resultref_test.go +++ b/pkg/apis/pipeline/v1/resultref_test.go @@ -17,6 +17,7 @@ limitations under the License. package v1_test import ( + "fmt" "testing" "github.com/google/go-cmp/cmp" @@ -295,6 +296,126 @@ func TestHasResultReference(t *testing.T) { } } +func TestParseStepExpression(t *testing.T) { + for _, tt := range []struct { + name string + param v1.Param + wantStep string + wantResult string + wantIdx int + wantAttribute string + wantParamType v1.ParamType + wantError error + }{{ + name: "a string step result ref", + param: v1.Param{ + Name: "param", + Value: *v1.NewStructuredValues("$(steps.sumSteps.results.sumResult)"), + }, + wantStep: "sumSteps", + wantResult: "sumResult", + wantIdx: -1, + wantAttribute: "", + wantParamType: v1.ParamTypeString, + wantError: nil, + }, { + name: "an array step result ref with idx", + param: v1.Param{ + Name: "param", + Value: *v1.NewStructuredValues("$(steps.sumSteps.results.sumResult[1000])"), + }, + wantStep: "sumSteps", + wantResult: "sumResult", + wantIdx: 1000, + wantAttribute: "", + wantParamType: v1.ParamTypeArray, + wantError: nil, + }, { + name: "an array step result ref with [*]", + param: v1.Param{ + Name: "param", + Value: *v1.NewStructuredValues("$(steps.sumSteps.results.sumResult[*])"), + }, + wantStep: "sumSteps", + wantResult: "sumResult", + wantIdx: -1, + wantAttribute: "", + wantParamType: v1.ParamTypeArray, + wantError: nil, + }, { + name: "an object step result ref with attribute", + param: v1.Param{ + Name: "param", + Value: *v1.NewStructuredValues("$(steps.sumSteps.results.sumResult.sum)"), + }, + wantStep: "sumSteps", + wantResult: "sumResult", + wantIdx: -1, + wantAttribute: "sum", + wantParamType: v1.ParamTypeObject, + wantError: nil, + }, { + name: "an invalid step result ref", + param: v1.Param{ + Name: "param", + Value: *v1.NewStructuredValues("$(steps.sumSteps.results.sumResult.foo.bar)"), + }, + wantStep: "", + wantResult: "", + wantIdx: -1, + wantAttribute: "", + wantParamType: v1.ParamTypeString, + wantError: fmt.Errorf("must be one of the form 1). \"tasks..results.\"; 2). \"tasks..results..\"; 3). \"steps..results.\"; 4). \"steps..results..\""), + }, { + name: "not a step result ref", + param: v1.Param{ + Name: "param", + Value: *v1.NewStructuredValues("$(tasks.sumSteps.results.sumResult.foo.bar)"), + }, + wantStep: "", + wantResult: "", + wantIdx: -1, + wantAttribute: "", + wantParamType: v1.ParamTypeString, + wantError: fmt.Errorf("must be one of the form 1). \"steps..results.\"; 2). \"steps..results..\""), + }} { + t.Run(tt.name, func(t *testing.T) { + expressions, _ := tt.param.GetVarSubstitutionExpressions() + gotStep, gotResult, gotIdx, gotAttribute, gotParamType, gotError := v1.ParseStepExpression(expressions[0]) + if gotStep != tt.wantStep { + t.Errorf("ParseStepExpression() = %v, want %v", gotStep, tt.wantStep) + } + if gotResult != tt.wantResult { + t.Errorf("ParseStepExpression() = %v, want %v", gotResult, tt.wantResult) + } + if tt.wantIdx == -1 { + if gotIdx != nil { + t.Errorf("ParseStepExpression() = %v, want nil", gotIdx) + } + } else { + if *gotIdx != tt.wantIdx { + t.Errorf("ParseStepExpression() = %v, want %v", *gotIdx, tt.wantIdx) + } + } + if gotAttribute != tt.wantAttribute { + t.Errorf("ParseStepExpression() = %v, want %v", gotAttribute, tt.wantAttribute) + } + if gotParamType != tt.wantParamType { + t.Errorf("ParseStepExpression() = %v, want %v", gotParamType, tt.wantParamType) + } + if tt.wantError == nil { + if gotError != nil { + t.Errorf("ParseStepExpression() = %v, want nil", gotError) + } + } else { + if gotError.Error() != tt.wantError.Error() { + t.Errorf("ParseStepExpression() = \n%v\n, want \n%v", gotError.Error(), tt.wantError.Error()) + } + } + }) + } +} + func TestLooksLikeResultRef(t *testing.T) { for _, tt := range []struct { name string diff --git a/pkg/entrypoint/entrypointer.go b/pkg/entrypoint/entrypointer.go index c90b6d0960c..325d9b6b8d0 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,8 @@ 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/names" "github.com/tektoncd/pipeline/pkg/pod" "github.com/tektoncd/pipeline/pkg/result" "github.com/tektoncd/pipeline/pkg/spire" @@ -182,6 +185,10 @@ func (e Entrypointer) Go() error { ctx := context.Background() var cancel context.CancelFunc if err == nil { + newScriptPath := filepath.Join(e.StepMetadataDir, names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("script")) + if err := e.applyStepResultSubstitutions(pipeline.StepsDir, newScriptPath, "/tekton/scripts"); 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 +343,148 @@ 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(stepDir string, stepName string, resultName string) (v1.ResultValue, error) { + v := v1.ResultValue{} + fp := getStepResultPath(stepDir, pod.GetContainerName(stepName), resultName) + fileContents, err := os.ReadFile(fp) + 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(stepDir string, stepName string, resultName string) string { + return filepath.Join(stepDir, stepName, "results", resultName) +} + +// findReplacement looks for any usage of step results in an input string. +// If found, it loads the results from the previous steps and provides the replacement value. +func findReplacement(stepDir string, 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(stepDir, 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 != nil { + replaceWithString = result.ArrayVal[*arrayIdx] + } else { + replaceWithArray = append(replaceWithArray, result.ArrayVal...) + } + } else { + replaceWithString = result.StringVal + } + return replaceWithString, replaceWithArray, nil +} + +// replaceCommandAndArgs performs replacements for step results in environment variables. +func replaceEnv(stepDir string, stepResultRegex *regexp.Regexp) error { + for _, e := range os.Environ() { + pair := strings.SplitN(e, "=", 2) + matches := stepResultRegex.FindAllStringSubmatch(pair[1], -1) + for _, m := range matches { + replaceWith, _, err := findReplacement(stepDir, m[0]) + if err != nil { + return err + } + os.Setenv(pair[0], strings.ReplaceAll(pair[1], m[0], replaceWith)) + } + } + return nil +} + +// replaceScript performs replacements for step results in the script file. +// It reads the original script file, performs the replacements and writes a new file +// since the original script location is unknown. +// It then updates the command to execute the new file. +func replaceScript(e *Entrypointer, stepDir string, scriptPrefix string, newFilePath string, stepResultRegex *regexp.Regexp) error { + if len(e.Command) > 0 && strings.HasPrefix(e.Command[0], scriptPrefix) { + 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(stepDir, m[0]) + if err != nil { + return err + } + fileContents = strings.ReplaceAll(fileContents, m[0], replaceWith) + } + // copy the modified contents to a different file. + err := os.WriteFile(newFilePath, []byte(fileContents), 0755) + if err != nil { + return err + } + // set the command to execute the new file. + e.Command[0] = newFilePath + } + } + return nil +} + +// replaceCommandAndArgs performs replacements for step results in e.Command +func replaceCommandAndArgs(command []string, stepDir string, stepResultRegex *regexp.Regexp) ([]string, error) { + newCommand := []string{} + for _, c := range command { + matches := stepResultRegex.FindAllStringSubmatch(c, -1) + if len(matches) > 0 { + for _, m := range matches { + replaceWithString, replaceWithArray, err := findReplacement(stepDir, m[0]) + if err != nil { + return nil, 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) + } + } + return newCommand, nil +} + +// applyStepResultSubstitutions applies the runtime step result substitutions in env, script and command. +func (e *Entrypointer) applyStepResultSubstitutions(stepDir, newFilePath, scriptPrefix string) error { + pattern := `\$\(steps\..*\.results\..*\)` + stepResultRegex := regexp.MustCompile(pattern) + // env + if err := replaceEnv(stepDir, stepResultRegex); err != nil { + return err + } + // script + if err := replaceScript(e, stepDir, scriptPrefix, newFilePath, stepResultRegex); err != nil { + return err + } + // command + args + newCommand, err := replaceCommandAndArgs(e.Command, stepDir, stepResultRegex) + if err != nil { + return err + } + e.Command = newCommand + return nil +} diff --git a/pkg/entrypoint/entrypointer_test.go b/pkg/entrypoint/entrypointer_test.go index 127d70b992d..cf3c1913907 100644 --- a/pkg/entrypoint/entrypointer_test.go +++ b/pkg/entrypoint/entrypointer_test.go @@ -21,6 +21,7 @@ import ( "encoding/json" "errors" "fmt" + "log" "os" "os/exec" "path" @@ -725,6 +726,251 @@ func TestEntrypointerStopOnCancel(t *testing.T) { } } +func TestApplyStepResultSubstitutions_Env(t *testing.T) { + testCases := []struct { + name string + stepName string + resultName string + result string + envValue string + want string + wantErr bool + }{{ + name: "string param", + stepName: "foo", + resultName: "res", + result: "Hello", + envValue: "$(steps.foo.results.res)", + want: "Hello", + wantErr: false, + }, { + name: "array param", + stepName: "foo", + resultName: "res", + result: "[\"Hello\",\"World\"]", + envValue: "$(steps.foo.results.res[1])", + want: "World", + wantErr: false, + }, { + name: "object param", + stepName: "foo", + resultName: "res", + result: "{\"hello\":\"World\"}", + envValue: "$(steps.foo.results.res.hello)", + want: "World", + wantErr: false, + }, { + name: "bad-result-format", + stepName: "foo", + resultName: "res", + result: "{\"hello\":\"World\"}", + envValue: "echo $(steps.foo.results.res.hello.bar)", + want: "echo $(steps.foo.results.res.hello.bar)", + wantErr: true, + }} + stepDir := createTmpDir(t, "env-steps") + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + resultPath := filepath.Join(stepDir, pod.GetContainerName(tc.stepName), "results") + err := os.MkdirAll(resultPath, 0750) + if err != nil { + log.Fatal(err) + } + resultFile := filepath.Join(resultPath, tc.resultName) + err = os.WriteFile(resultFile, []byte(tc.result), 0666) + if err != nil { + log.Fatal(err) + } + os.Setenv("FOO", tc.envValue) + defer os.Unsetenv("FOO") + e := Entrypointer{ + Command: []string{}, + } + err = e.applyStepResultSubstitutions(stepDir, "", "no-such-path") + if tc.wantErr == false && err != nil { + t.Fatalf("Did not expect and error but got: %v", err) + } else if tc.wantErr == true && err == nil { + t.Fatalf("Expected and error but did not get any.") + } + got := os.Getenv("FOO") + if got != tc.want { + t.Errorf("applyStepResultSubstitutions(): got %v; want %v", got, tc.want) + } + }) + } +} + +func TestApplyStepResultSubstitutions_Command(t *testing.T) { + testCases := []struct { + name string + stepName string + resultName string + result string + command []string + want []string + wantErr bool + }{{ + name: "string param", + stepName: "foo", + resultName: "res1", + result: "Hello", + command: []string{"$(steps.foo.results.res1)"}, + want: []string{"Hello"}, + wantErr: false, + }, { + name: "array param", + stepName: "foo", + resultName: "res", + result: "[\"Hello\",\"World\"]", + command: []string{"$(steps.foo.results.res[1])"}, + want: []string{"World"}, + wantErr: false, + }, { + name: "array param no index", + stepName: "foo", + resultName: "res", + result: "[\"Hello\",\"World\"]", + command: []string{"start", "$(steps.foo.results.res[*])", "stop"}, + want: []string{"start", "Hello", "World", "stop"}, + wantErr: false, + }, { + name: "object param", + stepName: "foo", + resultName: "res", + result: "{\"hello\":\"World\"}", + command: []string{"$(steps.foo.results.res.hello)"}, + want: []string{"World"}, + wantErr: false, + }, { + name: "bad-result-format", + stepName: "foo", + resultName: "res", + result: "{\"hello\":\"World\"}", + command: []string{"echo $(steps.foo.results.res.hello.bar)"}, + want: []string{"echo $(steps.foo.results.res.hello.bar)"}, + wantErr: true, + }} + stepDir := createTmpDir(t, "command-steps") + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + resultPath := filepath.Join(stepDir, pod.GetContainerName(tc.stepName), "results") + err := os.MkdirAll(resultPath, 0750) + if err != nil { + log.Fatal(err) + } + resultFile := filepath.Join(resultPath, tc.resultName) + err = os.WriteFile(resultFile, []byte(tc.result), 0666) + if err != nil { + log.Fatal(err) + } + e := Entrypointer{ + Command: tc.command, + } + err = e.applyStepResultSubstitutions(stepDir, "", "no-such-path") + if tc.wantErr == false && err != nil { + t.Fatalf("Did not expect and error but got: %v", err) + } else if tc.wantErr == true && err == nil { + t.Fatalf("Expected and error but did not get any.") + } + got := e.Command + if d := cmp.Diff(tc.want, got); d != "" { + t.Errorf("Entrypointer error diff %s", diff.PrintWantGot(d)) + } + }) + } +} + +func TestApplyStepResultSubstitutions_Script(t *testing.T) { + testCases := []struct { + name string + stepName string + resultName string + result string + script string + want string + wantErr bool + }{{ + name: "string param", + stepName: "foo", + resultName: "res", + result: "Hello", + script: "echo $(steps.foo.results.res)", + want: "echo Hello", + wantErr: false, + }, { + name: "array param", + stepName: "foo", + resultName: "res", + result: "[\"Hello\",\"World\"]", + script: "echo $(steps.foo.results.res[1])", + want: "echo World", + wantErr: false, + }, { + name: "object param", + stepName: "foo", + resultName: "res", + result: "{\"hello\":\"World\"}", + script: "echo $(steps.foo.results.res.hello)", + want: "echo World", + wantErr: false, + }, { + name: "non-existent-result", + stepName: "foo", + resultName: "res", + result: "{\"hello\":\"World\"}", + script: "echo $(steps.foo.results.does-not-exist.hello)", + want: "echo World", + wantErr: true, + }, { + name: "bad-result-format", + stepName: "foo", + resultName: "res", + result: "{\"hello\":\"World\"}", + script: "echo $(steps.foo.results.res.hello.bar)", + want: "echo World", + wantErr: true, + }} + stepDir := createTmpDir(t, "script-steps") + scriptDir := createTmpDir(t, "scripts") + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + resultPath := filepath.Join(stepDir, pod.GetContainerName(tc.stepName), "results") + err := os.MkdirAll(resultPath, 0750) + if err != nil { + log.Fatal(err) + } + resultFile := filepath.Join(resultPath, tc.resultName) + err = os.WriteFile(resultFile, []byte(tc.result), 0666) + if err != nil { + log.Fatal(err) + } + scriptFile := filepath.Join(scriptDir, "script") + err = os.WriteFile(scriptFile, []byte(tc.script), 0666) + if err != nil { + log.Fatal(err) + } + e := Entrypointer{ + Command: []string{scriptFile}, + } + newScript := filepath.Join(scriptDir, "new-script") + err = e.applyStepResultSubstitutions(stepDir, newScript, scriptDir) + if tc.wantErr == false && err != nil { + t.Fatalf("Did not expect and error but got: %v", err) + } else if tc.wantErr == true && err == nil { + t.Fatalf("Expected and error but did not get any.") + } + fileContents, err := os.ReadFile(newScript) + if err != nil { + log.Fatal(err) + } + got := string(fileContents) + if got != tc.want { + t.Errorf("applyStepResultSubstitutions(): got %v; want %v", got, tc.want) + } + }) + } +} + func TestIsContextDeadlineError(t *testing.T) { ctxErr := ContextError(context.DeadlineExceeded.Error()) if !IsContextDeadlineError(ctxErr) { 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 a6a842395ab..2a5351763ac 100644 --- a/pkg/pod/status.go +++ b/pkg/pod/status.go @@ -248,7 +248,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...) } } @@ -360,7 +360,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..3564e60a01b 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. diff --git a/pkg/reconciler/taskrun/resources/taskspec_test.go b/pkg/reconciler/taskrun/resources/taskspec_test.go index 7e1d90b69c4..1e8d8ac603e 100644 --- a/pkg/reconciler/taskrun/resources/taskspec_test.go +++ b/pkg/reconciler/taskrun/resources/taskspec_test.go @@ -817,6 +817,98 @@ func TestGetStepActionsData(t *testing.T) { Image: "myimage", Args: []string{"taskrun string param", "taskspec", "array", "taskspec", "array", "param", "taskrun key", "taskspec key2", "step action key3"}, }}, + }, { + name: "propagating step result substitution strings into step actions", + tr: &v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mytaskrun", + Namespace: "default", + }, + Spec: v1.TaskRunSpec{ + TaskSpec: &v1.TaskSpec{ + Steps: []v1.Step{{ + Name: "inlined-step", + Image: "ubuntu", + Results: []v1.StepResult{{ + Name: "result1", + }, { + Name: "result2", + Type: v1.ResultsTypeArray, + }, { + Name: "result3", + Type: v1.ResultsTypeObject, + }}, + }, { + Ref: &v1.Ref{ + Name: "stepAction", + }, + Params: v1.Params{{ + Name: "string-param", + Value: *v1.NewStructuredValues("$(steps.inlined-step.results.result1)"), + }, { + Name: "array-param", + Value: *v1.NewStructuredValues("$(steps.inlined-step.results.result2[*])"), + }, { + Name: "object-param", + Value: *v1.NewStructuredValues("$(steps.inlined-step.results.result3[*])"), + }}, + }}, + }, + }, + }, + stepAction: &v1alpha1.StepAction{ + ObjectMeta: metav1.ObjectMeta{ + Name: "stepAction", + Namespace: "default", + }, + Spec: v1alpha1.StepActionSpec{ + Image: "myimage", + Args: []string{"$(params.string-param)", "$(params.array-param[0])", "$(params.array-param[1])", "$(params.array-param[*])", "$(params.object-param.key)"}, + Command: []string{"$(params[\"string-param\"])", "$(params[\"array-param\"][0])"}, + Env: []corev1.EnvVar{{ + Name: "env1", + Value: "$(params['array-param'][0])", + }, { + Name: "env2", + Value: "$(params['string-param'])", + }}, + Params: v1.ParamSpecs{{ + Name: "string-param", + Type: v1.ParamTypeString, + }, { + Name: "array-param", + Type: v1.ParamTypeArray, + }, { + Name: "object-param", + Type: v1.ParamTypeObject, + Properties: map[string]v1.PropertySpec{"key": {Type: "string"}}, + }}, + }, + }, + want: []v1.Step{{ + Name: "inlined-step", + Image: "ubuntu", + Results: []v1.StepResult{{ + Name: "result1", + }, { + Name: "result2", + Type: v1.ResultsTypeArray, + }, { + Name: "result3", + Type: v1.ResultsTypeObject, + }}, + }, { + Image: "myimage", + Args: []string{"$(steps.inlined-step.results.result1)", "$(steps.inlined-step.results.result2[0])", "$(steps.inlined-step.results.result2[1])", "$(steps.inlined-step.results.result2[*])", "$(steps.inlined-step.results.result3.key)"}, + Command: []string{"$(steps.inlined-step.results.result1)", "$(steps.inlined-step.results.result2[0])"}, + Env: []corev1.EnvVar{{ + Name: "env1", + Value: "$(steps.inlined-step.results.result2[0])", + }, { + Name: "env2", + Value: "$(steps.inlined-step.results.result1)", + }}, + }}, }} for _, tt := range tests { ctx := context.Background()