diff --git a/docs/stepactions.md b/docs/stepactions.md index 834b8e70490..8dc3ed91c5b 100644 --- a/docs/stepactions.md +++ b/docs/stepactions.md @@ -181,7 +181,7 @@ spec: date | tee $(step.results.current-date-human-readable.path) ``` -`Results` from the above `StepAction` can be [fetched by the `Task`](#fetching-emitted-results-from-stepactions) in another `StepAction` via `$(steps..results.)`. +`Results` from the above `StepAction` can be [fetched by the `Task`](#fetching-emitted-results-from-stepactions) or in [another `Step/StepAction`](#passing-step-results-between-steps) via `$(steps..results.)`. #### Fetching Emitted Results from StepActions @@ -238,6 +238,64 @@ spec: name: kaniko-step-action ``` +#### Passing Results between Steps + +`StepResults` (i.e. results written to `$(step.results..path)`, NOT `$(results..path)`) can be shared with following steps via replacement variable `$(steps..results.)`. + +Pipeline supports two new types of results and parameters: array `[]string` and object `map[string]string`. +Array and Object result is a beta feature and can be enabled by setting `enable-api-fields` to `alpha` or `beta`. + +| Result Type | Parameter Type | Specification | `enable-api-fields` | +|-------------|----------------|--------------------------------------------------|---------------------| +| string | string | `$(steps..results.)` | stable | +| array | array | `$(steps..results.[*])` | alpha or beta | +| array | string | `$(steps..results.[i])` | alpha or beta | +| object | string | `$(tasks..results..key)` | alpha or beta | + +**Note:** Whole Array `Results` (using star notation) cannot be referred in `script` and `env`. + +The example below shows how you could pass `step results` from a `step` into following steps, in this case, into a `StepAction`. + +```yaml +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[*]) +``` + +**Note:** `Step Results` can only be referenced in a `Step's/StepAction's` `env`, `command` and `args`. Referencing in any other field will throw an error. + ### Declaring WorkingDir You can declare `workingDir` in a `StepAction`: @@ -463,9 +521,3 @@ spec: ``` The default resolver type can be configured by the `default-resolver-type` field in the `config-defaults` ConfigMap (`alpha` feature). See [additional-configs.md](./additional-configs.md) for details. - -## Known Limitations - -### Cannot pass Step Results between Steps - -It's not currently possible to pass results produced by a `Step` into following `Steps`. We are working on this feature and it will be made available soon. 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..e02a771c746 --- /dev/null +++ b/examples/v1/taskruns/alpha/stepaction-passing-results.yaml @@ -0,0 +1,93 @@ +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: | + if [[ $1 != "image1" ]]; then + echo "Want: image1, Got: $1" + exit 1 + fi + if [[ $2 != "image2" ]]; then + echo "Want: image2, Got: $2" + exit 1 + fi + if [[ $3 != "image3" ]]; then + echo "Want: image3, Got: $3" + exit 1 + fi + if [[ $4 != "image1" ]]; then + echo "Want: image1, Got: $4" + exit 1 + fi + if [[ $5 != "ar.com" ]]; then + echo "Want: ar.com, Got: $5" + exit 1 + fi + if [[ $6 != "sha234" ]]; then + echo "Want: sha234, Got: $6" + exit 1 + fi + if [[ ${STRINGPARAM} != "foo" ]]; then + echo "Want: foo, Got: ${STRINGPARAM}" + exit 1 + fi +--- +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) + cat /tekton/scripts/* + - 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 9c000c673e8..c6d7d0ee9de 100644 --- a/pkg/apis/pipeline/v1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1/pipeline_validation.go @@ -23,6 +23,7 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/validate" + "github.com/tektoncd/pipeline/pkg/internal/resultref" "github.com/tektoncd/pipeline/pkg/reconciler/pipeline/dag" "github.com/tektoncd/pipeline/pkg/substitution" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" @@ -643,7 +644,7 @@ func validatePipelineResults(results []PipelineResult, tasks []PipelineTask, fin "value").ViaFieldIndex("results", idx)) } - expressions = filter(expressions, looksLikeResultRef) + expressions = filter(expressions, resultref.LooksLikeResultRef) resultRefs := NewResultRefs(expressions) if len(expressions) != len(resultRefs) { errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("expected all of the expressions %v to be result expressions but only %v were", expressions, resultRefs), @@ -678,16 +679,16 @@ func taskContainsResult(resultExpression string, pipelineTaskNames sets.String, for _, expression := range split { if expression != "" { value := stripVarSubExpression("$" + expression) - pipelineTaskName, _, _, _, err := parseExpression(value) + pr, err := resultref.ParseTaskExpression(value) if err != nil { return false } - if strings.HasPrefix(value, "tasks") && !pipelineTaskNames.Has(pipelineTaskName) { + if strings.HasPrefix(value, "tasks") && !pipelineTaskNames.Has(pr.ResourceName) { return false } - if strings.HasPrefix(value, "finally") && !pipelineFinallyTaskNames.Has(pipelineTaskName) { + if strings.HasPrefix(value, "finally") && !pipelineFinallyTaskNames.Has(pr.ResourceName) { return false } } diff --git a/pkg/apis/pipeline/v1/pipelinerun_validation.go b/pkg/apis/pipeline/v1/pipelinerun_validation.go index 859fe311ff4..84698b06644 100644 --- a/pkg/apis/pipeline/v1/pipelinerun_validation.go +++ b/pkg/apis/pipeline/v1/pipelinerun_validation.go @@ -23,6 +23,7 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/validate" + "github.com/tektoncd/pipeline/pkg/internal/resultref" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "knative.dev/pkg/apis" @@ -128,7 +129,7 @@ func (ps *PipelineRunSpec) validatePipelineRunParameters(ctx context.Context) (e expressions, ok := param.GetVarSubstitutionExpressions() if ok { if LooksLikeContainsResultRefs(expressions) { - expressions = filter(expressions, looksLikeResultRef) + expressions = filter(expressions, resultref.LooksLikeResultRef) resultRefs := NewResultRefs(expressions) if len(resultRefs) > 0 { errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("cannot use result expressions in %v as PipelineRun parameter values", expressions), diff --git a/pkg/apis/pipeline/v1/resultref.go b/pkg/apis/pipeline/v1/resultref.go index 3948581f6d6..18609ec62c1 100644 --- a/pkg/apis/pipeline/v1/resultref.go +++ b/pkg/apis/pipeline/v1/resultref.go @@ -17,10 +17,10 @@ limitations under the License. package v1 import ( - "fmt" "regexp" - "strconv" "strings" + + "github.com/tektoncd/pipeline/pkg/internal/resultref" ) // ResultRef is a type that represents a reference to a task run result @@ -32,25 +32,21 @@ type ResultRef struct { } const ( - resultExpressionFormat = "tasks..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.." // ResultTaskPart Constant used to define the "tasks" part of a pipeline result reference - ResultTaskPart = "tasks" + // retained because of backwards compatibility + ResultTaskPart = resultref.ResultTaskPart // ResultFinallyPart Constant used to define the "finally" part of a pipeline result reference - ResultFinallyPart = "finally" + // retained because of backwards compatibility + ResultFinallyPart = resultref.ResultFinallyPart // ResultResultPart Constant used to define the "results" part of a pipeline result reference - ResultResultPart = "results" + // retained because of backwards compatibility + ResultResultPart = resultref.ResultResultPart // TODO(#2462) use one regex across all substitutions // variableSubstitutionFormat matches format like $result.resultname, $result.resultname[int] and $result.resultname[*] variableSubstitutionFormat = `\$\([_a-zA-Z0-9.-]+(\.[_a-zA-Z0-9.-]+)*(\[([0-9]+|\*)\])?\)` // exactVariableSubstitutionFormat matches strings that only contain a single reference to result or param variables, but nothing else // i.e. `$(result.resultname)` is a match, but `foo $(result.resultname)` is not. exactVariableSubstitutionFormat = `^\$\([_a-zA-Z0-9.-]+(\.[_a-zA-Z0-9.-]+)*(\[([0-9]+|\*)\])?\)$` - // arrayIndexing will match all `[int]` and `[*]` for parseExpression - arrayIndexing = `\[([0-9])*\*?\]` // ResultNameFormat Constant used to define the regex Result.Name should follow ResultNameFormat = `^([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$` ) @@ -60,25 +56,22 @@ var VariableSubstitutionRegex = regexp.MustCompile(variableSubstitutionFormat) var exactVariableSubstitutionRegex = regexp.MustCompile(exactVariableSubstitutionFormat) var resultNameFormatRegex = regexp.MustCompile(ResultNameFormat) -// arrayIndexingRegex is used to match `[int]` and `[*]` -var arrayIndexingRegex = regexp.MustCompile(arrayIndexing) - // NewResultRefs extracts all ResultReferences from a param or a pipeline result. // If the ResultReference can be extracted, they are returned. Expressions which are not // results are ignored. func NewResultRefs(expressions []string) []*ResultRef { var resultRefs []*ResultRef for _, expression := range expressions { - pipelineTask, result, index, property, err := parseExpression(expression) + pr, err := resultref.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{ - PipelineTask: pipelineTask, - Result: result, - ResultsIndex: index, - Property: property, + PipelineTask: pr.ResourceName, + Result: pr.ResultName, + ResultsIndex: pr.ArrayIdx, + Property: pr.ObjectKey, }) } } @@ -91,20 +84,13 @@ func NewResultRefs(expressions []string) []*ResultRef { // performing strict validation func LooksLikeContainsResultRefs(expressions []string) bool { for _, expression := range expressions { - if looksLikeResultRef(expression) { + if resultref.LooksLikeResultRef(expression) { return true } } return false } -// looksLikeResultRef attempts to check if the given string looks like it contains any -// result references. Returns true if it does, false otherwise -func looksLikeResultRef(expression string) bool { - subExpressions := strings.Split(expression, ".") - return len(subExpressions) >= 4 && (subExpressions[0] == ResultTaskPart || subExpressions[0] == ResultFinallyPart) && subExpressions[2] == ResultResultPart -} - func validateString(value string) []string { expressions := VariableSubstitutionRegex.FindAllString(value, -1) if expressions == nil { @@ -121,43 +107,6 @@ 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) -// 1. Reference string result -// - Input: tasks.myTask.results.aStringResult -// - Output: "myTask", "aStringResult", nil, "", nil -// 2. Reference Object value with key: -// - Input: tasks.myTask.results.anObjectResult.key1 -// - Output: "myTask", "anObjectResult", nil, "key1", nil -// 3. Reference array elements with array indexing : -// - Input: tasks.myTask.results.anArrayResult[1] -// - Output: "myTask", "anArrayResult", 1, "", nil -// 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: "", "", 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) { - 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 != "" && stringIdx != "*" { - intIdx, _ := strconv.Atoi(stringIdx) - return subExpressions[1], resultName, &intIdx, "", nil - } - return subExpressions[1], resultName, nil, "", nil - } else if len(subExpressions) == 5 { - // For object type result: tasks..results.. - return subExpressions[1], subExpressions[3], nil, subExpressions[4], nil - } - } - 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. // Array indexing: // Input: anArrayResult[1] @@ -165,10 +114,9 @@ func parseExpression(substitutionExpression string) (string, string, *int, strin // Array star reference: // Input: anArrayResult[*] // Output: anArrayResult, "*" +// retained for backwards compatibility func ParseResultName(resultName string) (string, string) { - stringIdx := strings.TrimSuffix(strings.TrimPrefix(arrayIndexingRegex.FindString(resultName), "["), "]") - resultName = arrayIndexingRegex.ReplaceAllString(resultName, "") - return resultName, stringIdx + return resultref.ParseResultName(resultName) } // PipelineTaskResultRefs walks all the places a result reference can be used diff --git a/pkg/apis/pipeline/v1/task_validation.go b/pkg/apis/pipeline/v1/task_validation.go index 23f7178645e..8fadf0940e4 100644 --- a/pkg/apis/pipeline/v1/task_validation.go +++ b/pkg/apis/pipeline/v1/task_validation.go @@ -28,6 +28,7 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline" "github.com/tektoncd/pipeline/pkg/apis/validate" + "github.com/tektoncd/pipeline/pkg/internal/resultref" "github.com/tektoncd/pipeline/pkg/substitution" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" corev1 "k8s.io/api/core/v1" @@ -316,6 +317,44 @@ func isCreateOrUpdateAndDiverged(ctx context.Context, s Step) bool { return false } +func errorIfStepResultReferenceinField(value, fieldName string) (errs *apis.FieldError) { + matches := resultref.StepResultRegex.FindAllStringSubmatch(value, -1) + if len(matches) > 0 { + errs = errs.Also(&apis.FieldError{ + Message: "stepResult substitutions are only allowed in env, command and args. Found usage in", + Paths: []string{fieldName}, + }) + } + return errs +} + +func validateStepResultReference(s Step) (errs *apis.FieldError) { + errs = errs.Also(errorIfStepResultReferenceinField(s.Name, "name")) + errs = errs.Also(errorIfStepResultReferenceinField(s.Image, "image")) + errs = errs.Also(errorIfStepResultReferenceinField(s.Script, "script")) + errs = errs.Also(errorIfStepResultReferenceinField(string(s.ImagePullPolicy), "imagePullPoliicy")) + errs = errs.Also(errorIfStepResultReferenceinField(s.WorkingDir, "workingDir")) + for _, e := range s.EnvFrom { + errs = errs.Also(errorIfStepResultReferenceinField(e.Prefix, "envFrom.prefix")) + if e.ConfigMapRef != nil { + errs = errs.Also(errorIfStepResultReferenceinField(e.ConfigMapRef.LocalObjectReference.Name, "envFrom.configMapRef")) + } + if e.SecretRef != nil { + errs = errs.Also(errorIfStepResultReferenceinField(e.SecretRef.LocalObjectReference.Name, "envFrom.secretRef")) + } + } + for _, v := range s.VolumeMounts { + errs = errs.Also(errorIfStepResultReferenceinField(v.Name, "volumeMounts.name")) + errs = errs.Also(errorIfStepResultReferenceinField(v.MountPath, "volumeMounts.mountPath")) + errs = errs.Also(errorIfStepResultReferenceinField(v.SubPath, "volumeMounts.subPath")) + } + for _, v := range s.VolumeDevices { + errs = errs.Also(errorIfStepResultReferenceinField(v.Name, "volumeDevices.name")) + errs = errs.Also(errorIfStepResultReferenceinField(v.DevicePath, "volumeDevices.devicePath")) + } + return errs +} + func validateStep(ctx context.Context, s Step, names sets.String) (errs *apis.FieldError) { if s.Ref != nil { if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableStepActions && isCreateOrUpdateAndDiverged(ctx, s) { @@ -453,6 +492,10 @@ func validateStep(ctx context.Context, s Step, names sets.String) (errs *apis.Fi if s.StderrConfig != nil { errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "step stderr stream support", config.AlphaAPIFields).ViaField("stderrconfig")) } + + // Validate usage of step result reference. + // Referencing previous step's results are only allowed in `env`, `command` and `args`. + errs = errs.Also(validateStepResultReference(s)) return errs } diff --git a/pkg/apis/pipeline/v1/task_validation_test.go b/pkg/apis/pipeline/v1/task_validation_test.go index 27a830faa54..098947b6fc4 100644 --- a/pkg/apis/pipeline/v1/task_validation_test.go +++ b/pkg/apis/pipeline/v1/task_validation_test.go @@ -1628,6 +1628,113 @@ func TestTaskSpecValidateErrorWithStepActionRef(t *testing.T) { } } +func TestTaskSpecValidateErrorWithStepResultRef(t *testing.T) { + tests := []struct { + name string + Steps []v1.Step + expectedError apis.FieldError + }{{ + name: "Cannot reference step results in image", + Steps: []v1.Step{{ + Image: "$(steps.prevStep.results.resultName)", + }}, + expectedError: apis.FieldError{ + Message: "stepResult substitutions are only allowed in env, command and args. Found usage in", + Paths: []string{"steps[0].image"}, + }, + }, { + name: "Cannot reference step results in script", + Steps: []v1.Step{{ + Image: "my-img", + Script: "echo $(steps.prevStep.results.resultName)", + }}, + expectedError: apis.FieldError{ + Message: "stepResult substitutions are only allowed in env, command and args. Found usage in", + Paths: []string{"steps[0].script"}, + }, + }, { + name: "Cannot reference step results in workingDir", + Steps: []v1.Step{{ + Image: "my-img", + WorkingDir: "$(steps.prevStep.results.resultName)", + }}, + expectedError: apis.FieldError{ + Message: "stepResult substitutions are only allowed in env, command and args. Found usage in", + Paths: []string{"steps[0].workingDir"}, + }, + }, { + name: "Cannot reference step results in envFrom", + Steps: []v1.Step{{ + Image: "my-img", + EnvFrom: []corev1.EnvFromSource{{ + Prefix: "$(steps.prevStep.results.resultName)", + ConfigMapRef: &corev1.ConfigMapEnvSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "$(steps.prevStep.results.resultName)", + }, + }, + SecretRef: &corev1.SecretEnvSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "$(steps.prevStep.results.resultName)", + }, + }, + }}, + }}, + expectedError: apis.FieldError{ + Message: "stepResult substitutions are only allowed in env, command and args. Found usage in", + Paths: []string{"steps[0].envFrom.configMapRef", "steps[0].envFrom.prefix", "steps[0].envFrom.secretRef"}, + }, + }, { + name: "Cannot reference step results in VolumeMounts", + Steps: []v1.Step{{ + Image: "my-img", + VolumeMounts: []corev1.VolumeMount{{ + Name: "$(steps.prevStep.results.resultName)", + MountPath: "$(steps.prevStep.results.resultName)", + SubPath: "$(steps.prevStep.results.resultName)", + }}, + }}, + expectedError: apis.FieldError{ + Message: "stepResult substitutions are only allowed in env, command and args. Found usage in", + Paths: []string{"steps[0].volumeMounts.name", "steps[0].volumeMounts.mountPath", "steps[0].volumeMounts.subPath"}, + }, + }, { + name: "Cannot reference step results in VolumeDevices", + Steps: []v1.Step{{ + Image: "my-img", + VolumeDevices: []corev1.VolumeDevice{{ + Name: "$(steps.prevStep.results.resultName)", + DevicePath: "$(steps.prevStep.results.resultName)", + }}, + }}, + expectedError: apis.FieldError{ + Message: "stepResult substitutions are only allowed in env, command and args. Found usage in", + Paths: []string{"steps[0].volumeDevices.name", "steps[0].volumeDevices.devicePath"}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ts := v1.TaskSpec{ + Steps: tt.Steps, + } + ctx := config.ToContext(context.Background(), &config.Config{ + FeatureFlags: &config.FeatureFlags{ + EnableStepActions: true, + }, + }) + ctx = apis.WithinCreate(ctx) + ts.SetDefaults(ctx) + err := ts.Validate(ctx) + if err == nil { + t.Fatalf("Expected an error, got nothing for %v", ts) + } + if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" { + t.Errorf("TaskSpec.Validate() errors diff %s", diff.PrintWantGot(d)) + } + }) + } +} func TestTaskSpecValidateErrorSidecars(t *testing.T) { tests := []struct { name string diff --git a/pkg/apis/pipeline/v1beta1/pipeline_validation.go b/pkg/apis/pipeline/v1beta1/pipeline_validation.go index 69d39e09546..a84f79e79a7 100644 --- a/pkg/apis/pipeline/v1beta1/pipeline_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipeline_validation.go @@ -24,6 +24,7 @@ import ( "github.com/google/go-containerregistry/pkg/name" "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/validate" + "github.com/tektoncd/pipeline/pkg/internal/resultref" "github.com/tektoncd/pipeline/pkg/reconciler/pipeline/dag" "github.com/tektoncd/pipeline/pkg/substitution" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" @@ -648,7 +649,7 @@ func validatePipelineResults(results []PipelineResult, tasks []PipelineTask, fin "value").ViaFieldIndex("results", idx)) } - expressions = filter(expressions, looksLikeResultRef) + expressions = filter(expressions, resultref.LooksLikeResultRef) resultRefs := NewResultRefs(expressions) if len(expressions) != len(resultRefs) { errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("expected all of the expressions %v to be result expressions but only %v were", expressions, resultRefs), @@ -683,16 +684,15 @@ func taskContainsResult(resultExpression string, pipelineTaskNames sets.String, for _, expression := range split { if expression != "" { value := stripVarSubExpression("$" + expression) - pipelineTaskName, _, _, _, err := parseExpression(value) - + pr, err := resultref.ParseTaskExpression(value) if err != nil { return false } - if strings.HasPrefix(value, "tasks") && !pipelineTaskNames.Has(pipelineTaskName) { + if strings.HasPrefix(value, "tasks") && !pipelineTaskNames.Has(pr.ResourceName) { return false } - if strings.HasPrefix(value, "finally") && !pipelineFinallyTaskNames.Has(pipelineTaskName) { + if strings.HasPrefix(value, "finally") && !pipelineFinallyTaskNames.Has(pr.ResourceName) { return false } } diff --git a/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go b/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go index 56d44138805..10799ed1732 100644 --- a/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go +++ b/pkg/apis/pipeline/v1beta1/pipelinerun_validation.go @@ -23,6 +23,7 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/validate" + "github.com/tektoncd/pipeline/pkg/internal/resultref" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" @@ -149,7 +150,7 @@ func (ps *PipelineRunSpec) validatePipelineRunParameters(ctx context.Context) (e expressions, ok := GetVarSubstitutionExpressionsForParam(param) if ok { if LooksLikeContainsResultRefs(expressions) { - expressions = filter(expressions, looksLikeResultRef) + expressions = filter(expressions, resultref.LooksLikeResultRef) resultRefs := NewResultRefs(expressions) if len(resultRefs) > 0 { errs = errs.Also(apis.ErrInvalidValue(fmt.Sprintf("cannot use result expressions in %v as PipelineRun parameter values", expressions), diff --git a/pkg/apis/pipeline/v1beta1/resultref.go b/pkg/apis/pipeline/v1beta1/resultref.go index 7c99dc0c034..5fb4658334c 100644 --- a/pkg/apis/pipeline/v1beta1/resultref.go +++ b/pkg/apis/pipeline/v1beta1/resultref.go @@ -17,10 +17,10 @@ limitations under the License. package v1beta1 import ( - "fmt" "regexp" - "strconv" "strings" + + "github.com/tektoncd/pipeline/pkg/internal/resultref" ) // ResultRef is a type that represents a reference to a task run result @@ -32,25 +32,21 @@ type ResultRef struct { } const ( - resultExpressionFormat = "tasks..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.." // ResultTaskPart Constant used to define the "tasks" part of a pipeline result reference - ResultTaskPart = "tasks" + // retained because of backwards compatibility + ResultTaskPart = resultref.ResultTaskPart // ResultFinallyPart Constant used to define the "finally" part of a pipeline result reference - ResultFinallyPart = "finally" + // retained because of backwards compatibility + ResultFinallyPart = resultref.ResultFinallyPart // ResultResultPart Constant used to define the "results" part of a pipeline result reference - ResultResultPart = "results" + // retained because of backwards compatibility + ResultResultPart = resultref.ResultResultPart // TODO(#2462) use one regex across all substitutions // variableSubstitutionFormat matches format like $result.resultname, $result.resultname[int] and $result.resultname[*] variableSubstitutionFormat = `\$\([_a-zA-Z0-9.-]+(\.[_a-zA-Z0-9.-]+)*(\[([0-9]+|\*)\])?\)` // exactVariableSubstitutionFormat matches strings that only contain a single reference to result or param variables, but nothing else // i.e. `$(result.resultname)` is a match, but `foo $(result.resultname)` is not. exactVariableSubstitutionFormat = `^\$\([_a-zA-Z0-9.-]+(\.[_a-zA-Z0-9.-]+)*(\[([0-9]+|\*)\])?\)$` - // arrayIndexing will match all `[int]` and `[*]` for parseExpression - arrayIndexing = `\[([0-9])*\*?\]` // ResultNameFormat Constant used to define the regex Result.Name should follow ResultNameFormat = `^([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9]$` ) @@ -60,25 +56,22 @@ var VariableSubstitutionRegex = regexp.MustCompile(variableSubstitutionFormat) var exactVariableSubstitutionRegex = regexp.MustCompile(exactVariableSubstitutionFormat) var resultNameFormatRegex = regexp.MustCompile(ResultNameFormat) -// arrayIndexingRegex is used to match `[int]` and `[*]` -var arrayIndexingRegex = regexp.MustCompile(arrayIndexing) - // NewResultRefs extracts all ResultReferences from a param or a pipeline result. // If the ResultReference can be extracted, they are returned. Expressions which are not // results are ignored. func NewResultRefs(expressions []string) []*ResultRef { var resultRefs []*ResultRef for _, expression := range expressions { - pipelineTask, result, index, property, err := parseExpression(expression) + pr, err := resultref.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, // since although it's not a result ref, it might be some other kind of reference if err == nil { resultRefs = append(resultRefs, &ResultRef{ - PipelineTask: pipelineTask, - Result: result, - ResultsIndex: index, - Property: property, + PipelineTask: pr.ResourceName, + Result: pr.ResultName, + ResultsIndex: pr.ArrayIdx, + Property: pr.ObjectKey, }) } } @@ -91,20 +84,13 @@ func NewResultRefs(expressions []string) []*ResultRef { // performing strict validation func LooksLikeContainsResultRefs(expressions []string) bool { for _, expression := range expressions { - if looksLikeResultRef(expression) { + if resultref.LooksLikeResultRef(expression) { return true } } return false } -// looksLikeResultRef attempts to check if the given string looks like it contains any -// result references. Returns true if it does, false otherwise -func looksLikeResultRef(expression string) bool { - subExpressions := strings.Split(expression, ".") - return len(subExpressions) >= 4 && (subExpressions[0] == ResultTaskPart || subExpressions[0] == ResultFinallyPart) && subExpressions[2] == ResultResultPart -} - // GetVarSubstitutionExpressionsForParam extracts all the value between "$(" and ")"" for a parameter func GetVarSubstitutionExpressionsForParam(param Param) ([]string, bool) { var allExpressions []string @@ -156,43 +142,6 @@ 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) -// 1. Reference string result -// - Input: tasks.myTask.results.aStringResult -// - Output: "myTask", "aStringResult", nil, "", nil -// 2. Reference Object value with key: -// - Input: tasks.myTask.results.anObjectResult.key1 -// - Output: "myTask", "anObjectResult", nil, "key1", nil -// 3. Reference array elements with array indexing : -// - Input: tasks.myTask.results.anArrayResult[1] -// - Output: "myTask", "anArrayResult", 1, "", nil -// 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: "", "", 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) { - 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 != "" && stringIdx != "*" { - intIdx, _ := strconv.Atoi(stringIdx) - return subExpressions[1], resultName, &intIdx, "", nil - } - return subExpressions[1], resultName, nil, "", nil - } else if len(subExpressions) == 5 { - // For object type result: tasks..results.. - return subExpressions[1], subExpressions[3], nil, subExpressions[4], nil - } - } - 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. // Array indexing: // Input: anArrayResult[1] @@ -201,9 +150,7 @@ func parseExpression(substitutionExpression string) (string, string, *int, strin // Input: anArrayResult[*] // Output: anArrayResult, "*" func ParseResultName(resultName string) (string, string) { - stringIdx := strings.TrimSuffix(strings.TrimPrefix(arrayIndexingRegex.FindString(resultName), "["), "]") - resultName = arrayIndexingRegex.ReplaceAllString(resultName, "") - return resultName, stringIdx + return resultref.ParseResultName(resultName) } // PipelineTaskResultRefs walks all the places a result reference can be used diff --git a/pkg/apis/pipeline/v1beta1/task_validation.go b/pkg/apis/pipeline/v1beta1/task_validation.go index c9a5e5a9ec9..80a4bf7f47b 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation.go +++ b/pkg/apis/pipeline/v1beta1/task_validation.go @@ -29,6 +29,7 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/pipeline" v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "github.com/tektoncd/pipeline/pkg/apis/validate" + "github.com/tektoncd/pipeline/pkg/internal/resultref" "github.com/tektoncd/pipeline/pkg/substitution" admissionregistrationv1 "k8s.io/api/admissionregistration/v1" corev1 "k8s.io/api/core/v1" @@ -305,6 +306,44 @@ func isCreateOrUpdateAndDiverged(ctx context.Context, s Step) bool { return false } +func errorIfStepResultReferenceinField(value, fieldName string) (errs *apis.FieldError) { + matches := resultref.StepResultRegex.FindAllStringSubmatch(value, -1) + if len(matches) > 0 { + errs = errs.Also(&apis.FieldError{ + Message: "stepResult substitutions are only allowed in env, command and args. Found usage in", + Paths: []string{fieldName}, + }) + } + return errs +} + +func validateStepResultReference(s Step) (errs *apis.FieldError) { + errs = errs.Also(errorIfStepResultReferenceinField(s.Name, "name")) + errs = errs.Also(errorIfStepResultReferenceinField(s.Image, "image")) + errs = errs.Also(errorIfStepResultReferenceinField(s.Script, "script")) + errs = errs.Also(errorIfStepResultReferenceinField(string(s.ImagePullPolicy), "imagePullPoliicy")) + errs = errs.Also(errorIfStepResultReferenceinField(s.WorkingDir, "workingDir")) + for _, e := range s.EnvFrom { + errs = errs.Also(errorIfStepResultReferenceinField(e.Prefix, "envFrom.prefix")) + if e.ConfigMapRef != nil { + errs = errs.Also(errorIfStepResultReferenceinField(e.ConfigMapRef.LocalObjectReference.Name, "envFrom.configMapRef")) + } + if e.SecretRef != nil { + errs = errs.Also(errorIfStepResultReferenceinField(e.SecretRef.LocalObjectReference.Name, "envFrom.secretRef")) + } + } + for _, v := range s.VolumeMounts { + errs = errs.Also(errorIfStepResultReferenceinField(v.Name, "volumeMounts.name")) + errs = errs.Also(errorIfStepResultReferenceinField(v.MountPath, "volumeMounts.mountPath")) + errs = errs.Also(errorIfStepResultReferenceinField(v.SubPath, "volumeMounts.subPath")) + } + for _, v := range s.VolumeDevices { + errs = errs.Also(errorIfStepResultReferenceinField(v.Name, "volumeDevices.name")) + errs = errs.Also(errorIfStepResultReferenceinField(v.DevicePath, "volumeDevices.devicePath")) + } + return errs +} + func validateStep(ctx context.Context, s Step, names sets.String) (errs *apis.FieldError) { if s.Ref != nil { if !config.FromContextOrDefaults(ctx).FeatureFlags.EnableStepActions && isCreateOrUpdateAndDiverged(ctx, s) { @@ -442,6 +481,10 @@ func validateStep(ctx context.Context, s Step, names sets.String) (errs *apis.Fi if s.StderrConfig != nil { errs = errs.Also(config.ValidateEnabledAPIFields(ctx, "step stderr stream support", config.AlphaAPIFields).ViaField("stderrconfig")) } + + // Validate usage of step result reference. + // Referencing previous step's results are only allowed in `env`, `command` and `args`. + errs = errs.Also(validateStepResultReference(s)) return errs } diff --git a/pkg/apis/pipeline/v1beta1/task_validation_test.go b/pkg/apis/pipeline/v1beta1/task_validation_test.go index bee77c71b69..90b8e117720 100644 --- a/pkg/apis/pipeline/v1beta1/task_validation_test.go +++ b/pkg/apis/pipeline/v1beta1/task_validation_test.go @@ -1639,6 +1639,114 @@ func TestTaskSpecValidateErrorWithStepActionRef(t *testing.T) { } } +func TestTaskSpecValidateErrorWithStepResultRef(t *testing.T) { + tests := []struct { + name string + Steps []v1beta1.Step + expectedError apis.FieldError + }{{ + name: "Cannot reference step results in image", + Steps: []v1beta1.Step{{ + Image: "$(steps.prevStep.results.resultName)", + }}, + expectedError: apis.FieldError{ + Message: "stepResult substitutions are only allowed in env, command and args. Found usage in", + Paths: []string{"steps[0].image"}, + }, + }, { + name: "Cannot reference step results in script", + Steps: []v1beta1.Step{{ + Image: "my-img", + Script: "echo $(steps.prevStep.results.resultName)", + }}, + expectedError: apis.FieldError{ + Message: "stepResult substitutions are only allowed in env, command and args. Found usage in", + Paths: []string{"steps[0].script"}, + }, + }, { + name: "Cannot reference step results in workingDir", + Steps: []v1beta1.Step{{ + Image: "my-img", + WorkingDir: "$(steps.prevStep.results.resultName)", + }}, + expectedError: apis.FieldError{ + Message: "stepResult substitutions are only allowed in env, command and args. Found usage in", + Paths: []string{"steps[0].workingDir"}, + }, + }, { + name: "Cannot reference step results in envFrom", + Steps: []v1beta1.Step{{ + Image: "my-img", + EnvFrom: []corev1.EnvFromSource{{ + Prefix: "$(steps.prevStep.results.resultName)", + ConfigMapRef: &corev1.ConfigMapEnvSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "$(steps.prevStep.results.resultName)", + }, + }, + SecretRef: &corev1.SecretEnvSource{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: "$(steps.prevStep.results.resultName)", + }, + }, + }}, + }}, + expectedError: apis.FieldError{ + Message: "stepResult substitutions are only allowed in env, command and args. Found usage in", + Paths: []string{"steps[0].envFrom.configMapRef", "steps[0].envFrom.prefix", "steps[0].envFrom.secretRef"}, + }, + }, { + name: "Cannot reference step results in VolumeMounts", + Steps: []v1beta1.Step{{ + Image: "my-img", + VolumeMounts: []corev1.VolumeMount{{ + Name: "$(steps.prevStep.results.resultName)", + MountPath: "$(steps.prevStep.results.resultName)", + SubPath: "$(steps.prevStep.results.resultName)", + }}, + }}, + expectedError: apis.FieldError{ + Message: "stepResult substitutions are only allowed in env, command and args. Found usage in", + Paths: []string{"steps[0].volumeMounts.name", "steps[0].volumeMounts.mountPath", "steps[0].volumeMounts.subPath"}, + }, + }, { + name: "Cannot reference step results in VolumeDevices", + Steps: []v1beta1.Step{{ + Image: "my-img", + VolumeDevices: []corev1.VolumeDevice{{ + Name: "$(steps.prevStep.results.resultName)", + DevicePath: "$(steps.prevStep.results.resultName)", + }}, + }}, + expectedError: apis.FieldError{ + Message: "stepResult substitutions are only allowed in env, command and args. Found usage in", + Paths: []string{"steps[0].volumeDevices.name", "steps[0].volumeDevices.devicePath"}, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ts := v1beta1.TaskSpec{ + Steps: tt.Steps, + } + ctx := config.ToContext(context.Background(), &config.Config{ + FeatureFlags: &config.FeatureFlags{ + EnableStepActions: true, + }, + }) + ctx = apis.WithinCreate(ctx) + ts.SetDefaults(ctx) + err := ts.Validate(ctx) + if err == nil { + t.Fatalf("Expected an error, got nothing for %v", ts) + } + if d := cmp.Diff(tt.expectedError.Error(), err.Error(), cmpopts.IgnoreUnexported(apis.FieldError{})); d != "" { + t.Errorf("TaskSpec.Validate() errors diff %s", diff.PrintWantGot(d)) + } + }) + } +} + func TestTaskSpecValidateErrorSidecarName(t *testing.T) { tests := []struct { name string diff --git a/pkg/entrypoint/entrypointer.go b/pkg/entrypoint/entrypointer.go index c90b6d0960c..d27d3554682 100644 --- a/pkg/entrypoint/entrypointer.go +++ b/pkg/entrypoint/entrypointer.go @@ -31,6 +31,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/internal/resultref" "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(pipeline.StepsDir); 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,115 @@ 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, "$("), ")") + pr, err := resultref.ParseStepExpression(value) + if err != nil { + return "", nil, err + } + result, err := loadStepResult(stepDir, pr.ResourceName, pr.ResultName) + if err != nil { + return "", nil, err + } + replaceWithArray := []string{} + replaceWithString := "" + + switch pr.ResultType { + case "object": + if pr.ObjectKey != "" { + replaceWithString = result.ObjectVal[pr.ObjectKey] + } + case "array": + if pr.ArrayIdx != nil { + replaceWithString = result.ArrayVal[*pr.ArrayIdx] + } else { + replaceWithArray = append(replaceWithArray, result.ArrayVal...) + } + // "string" + default: + replaceWithString = result.StringVal + } + return replaceWithString, replaceWithArray, nil +} + +// replaceCommandAndArgs performs replacements for step results in environment variables. +func replaceEnv(stepDir string) error { + for _, e := range os.Environ() { + pair := strings.SplitN(e, "=", 2) + matches := resultref.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 +} + +// replaceCommandAndArgs performs replacements for step results in e.Command +func replaceCommandAndArgs(command []string, stepDir string) ([]string, error) { + newCommand := []string{} + for _, c := range command { + matches := resultref.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, args and command. +func (e *Entrypointer) applyStepResultSubstitutions(stepDir string) error { + // env + if err := replaceEnv(stepDir); err != nil { + return err + } + // command + args + newCommand, err := replaceCommandAndArgs(e.Command, stepDir) + 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..159a809fa64 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,159 @@ 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) + } + t.Setenv("FOO", tc.envValue) + e := Entrypointer{ + Command: []string{}, + } + err = e.applyStepResultSubstitutions(stepDir) + 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) + 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 TestIsContextDeadlineError(t *testing.T) { ctxErr := ContextError(context.DeadlineExceeded.Error()) if !IsContextDeadlineError(ctxErr) { diff --git a/pkg/internal/resultref/resultref.go b/pkg/internal/resultref/resultref.go new file mode 100644 index 00000000000..b42d0055a4d --- /dev/null +++ b/pkg/internal/resultref/resultref.go @@ -0,0 +1,173 @@ +/* +Copyright 2023 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package resultref + +import ( + "fmt" + "regexp" + "strconv" + "strings" +) + +const ( + 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.." + 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 + ResultFinallyPart = "finally" + // ResultResultPart Constant used to define the "results" part of a pipeline result reference + ResultResultPart = "results" + + // arrayIndexing will match all `[int]` and `[*]` for parseExpression + arrayIndexing = `\[([0-9])*\*?\]` + stepResultUsagePattern = `\$\(steps\..*\.results\..*\)` +) + +// arrayIndexingRegex is used to match `[int]` and `[*]` +var arrayIndexingRegex = regexp.MustCompile(arrayIndexing) + +// StepResultRegex compiles the regex pattern for the usage of step results. +var StepResultRegex = regexp.MustCompile(stepResultUsagePattern) + +// LooksLikeResultRef attempts to check if the given string looks like it contains any +// result references. Returns true if it does, false otherwise +func LooksLikeResultRef(expression string) bool { + subExpressions := strings.Split(expression, ".") + 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 +} + +// ParsedResult captures the task/step name, result name, type, +// array idx (in case of array result) and +// object key (in case of an object result). +// This is generated by parsing expressions that use +// $(tasks.taskName.results.resultName...) or $(steps.stepName.results.resultName...) +type ParsedResult struct { + ResourceName string + ResultName string + ResultType string + ArrayIdx *int + ObjectKey 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) +// 1. Reference string result +// - Input: tasks.myTask.results.aStringResult +// - Output: "myTask", "aStringResult", nil, "", nil +// 2. Reference Object value with key: +// - Input: tasks.myTask.results.anObjectResult.key1 +// - Output: "myTask", "anObjectResult", nil, "key1", nil +// 3. Reference array elements with array indexing : +// - Input: tasks.myTask.results.anArrayResult[1] +// - Output: "myTask", "anArrayResult", 1, "", nil +// 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: "", "", nil, "", error +// TODO: may use regex for each type to handle possible reference formats +func parseExpression(substitutionExpression string) (ParsedResult, 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 != "" { + if stringIdx == "*" { + pr := ParsedResult{ + ResourceName: subExpressions[1], + ResultName: resultName, + ResultType: "array", + } + return pr, nil + } + intIdx, _ := strconv.Atoi(stringIdx) + pr := ParsedResult{ + ResourceName: subExpressions[1], + ResultName: resultName, + ResultType: "array", + ArrayIdx: &intIdx, + } + return pr, nil + } + pr := ParsedResult{ + ResourceName: subExpressions[1], + ResultName: resultName, + ResultType: "string", + } + return pr, nil + } else if len(subExpressions) == 5 { + // For object type result: tasks..results.. + // For object type step result: steps..results.. + pr := ParsedResult{ + ResourceName: subExpressions[1], + ResultName: subExpressions[3], + ResultType: "object", + ObjectKey: subExpressions[4], + } + return pr, nil + } + } + return ParsedResult{}, fmt.Errorf("must be one of the form 1). %q; 2). %q; 3). %q; 4). %q", resultExpressionFormat, objectResultExpressionFormat, stepResultExpressionFormat, objectStepResultExpressionFormat) +} + +// ParseTaskExpression parses the input string and searches for the use of task result usage. +func ParseTaskExpression(substitutionExpression string) (ParsedResult, error) { + if LooksLikeResultRef(substitutionExpression) { + return parseExpression(substitutionExpression) + } + return ParsedResult{}, fmt.Errorf("must be one of the form 1). %q; 2). %q", resultExpressionFormat, objectResultExpressionFormat) +} + +// ParseStepExpression parses the input string and searches for the use of step result usage. +func ParseStepExpression(substitutionExpression string) (ParsedResult, error) { + if looksLikeStepResultRef(substitutionExpression) { + return parseExpression(substitutionExpression) + } + return ParsedResult{}, 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. +// Array indexing: +// Input: anArrayResult[1] +// Output: anArrayResult, "1" +// Array star reference: +// Input: anArrayResult[*] +// Output: anArrayResult, "*" +func ParseResultName(resultName string) (string, string) { + stringIdx := strings.TrimSuffix(strings.TrimPrefix(arrayIndexingRegex.FindString(resultName), "["), "]") + resultName = arrayIndexingRegex.ReplaceAllString(resultName, "") + return resultName, stringIdx +} diff --git a/pkg/internal/resultref/resultref_test.go b/pkg/internal/resultref/resultref_test.go new file mode 100644 index 00000000000..2d8ab653949 --- /dev/null +++ b/pkg/internal/resultref/resultref_test.go @@ -0,0 +1,239 @@ +/* +Copyright 2023 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package resultref_test + +import ( + "fmt" + "testing" + + "github.com/google/go-cmp/cmp" + v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" + "github.com/tektoncd/pipeline/pkg/internal/resultref" + "github.com/tektoncd/pipeline/test/diff" +) + +func TestParseStepExpression(t *testing.T) { + idx := 1000 + for _, tt := range []struct { + name string + param v1.Param + wantParsedResult resultref.ParsedResult + wantError error + }{{ + name: "a string step result ref", + param: v1.Param{ + Name: "param", + Value: *v1.NewStructuredValues("$(steps.sumSteps.results.sumResult)"), + }, + wantParsedResult: resultref.ParsedResult{ + ResourceName: "sumSteps", + ResultName: "sumResult", + ResultType: "string", + }, + wantError: nil, + }, { + name: "an array step result ref with idx", + param: v1.Param{ + Name: "param", + Value: *v1.NewStructuredValues(fmt.Sprintf("$(steps.sumSteps.results.sumResult[%v])", idx)), + }, + wantParsedResult: resultref.ParsedResult{ + ResourceName: "sumSteps", + ResultName: "sumResult", + ResultType: "array", + ArrayIdx: &idx, + }, + wantError: nil, + }, { + name: "an array step result ref with [*]", + param: v1.Param{ + Name: "param", + Value: *v1.NewStructuredValues("$(steps.sumSteps.results.sumResult[*])"), + }, + wantParsedResult: resultref.ParsedResult{ + ResourceName: "sumSteps", + ResultName: "sumResult", + ResultType: "array", + }, + wantError: nil, + }, { + name: "an object step result ref with attribute", + param: v1.Param{ + Name: "param", + Value: *v1.NewStructuredValues("$(steps.sumSteps.results.sumResult.sum)"), + }, + wantParsedResult: resultref.ParsedResult{ + ResourceName: "sumSteps", + ResultName: "sumResult", + ResultType: "object", + ObjectKey: "sum", + }, + wantError: nil, + }, { + name: "an invalid step result ref", + param: v1.Param{ + Name: "param", + Value: *v1.NewStructuredValues("$(steps.sumSteps.results.sumResult.foo.bar)"), + }, + wantParsedResult: resultref.ParsedResult{}, + 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)"), + }, + wantParsedResult: resultref.ParsedResult{}, + 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() + gotParsedResult, gotError := resultref.ParseStepExpression(expressions[0]) + if d := cmp.Diff(tt.wantParsedResult, gotParsedResult); d != "" { + t.Error(diff.PrintWantGot(d)) + } + 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 TestParseTaskExpression(t *testing.T) { + idx := 1000 + for _, tt := range []struct { + name string + param v1.Param + wantParsedResult resultref.ParsedResult + wantError error + }{{ + name: "a string task result ref", + param: v1.Param{ + Name: "param", + Value: *v1.NewStructuredValues("$(tasks.sumTasks.results.sumResult)"), + }, + wantParsedResult: resultref.ParsedResult{ + ResourceName: "sumTasks", + ResultName: "sumResult", + ResultType: "string", + }, + wantError: nil, + }, { + name: "an array task result ref with idx", + param: v1.Param{ + Name: "param", + Value: *v1.NewStructuredValues(fmt.Sprintf("$(tasks.sumTasks.results.sumResult[%v])", idx)), + }, + wantParsedResult: resultref.ParsedResult{ + ResourceName: "sumTasks", + ResultName: "sumResult", + ResultType: "array", + ArrayIdx: &idx, + }, + wantError: nil, + }, { + name: "an array task result ref with [*]", + param: v1.Param{ + Name: "param", + Value: *v1.NewStructuredValues("$(tasks.sumTasks.results.sumResult[*])"), + }, + wantParsedResult: resultref.ParsedResult{ + ResourceName: "sumTasks", + ResultName: "sumResult", + ResultType: "array", + }, + wantError: nil, + }, { + name: "an object task result ref with attribute", + param: v1.Param{ + Name: "param", + Value: *v1.NewStructuredValues("$(tasks.sumTasks.results.sumResult.sum)"), + }, + wantParsedResult: resultref.ParsedResult{ + ResourceName: "sumTasks", + ResultName: "sumResult", + ResultType: "object", + ObjectKey: "sum", + }, + wantError: nil, + }, { + name: "an invalid task result ref", + param: v1.Param{ + Name: "param", + Value: *v1.NewStructuredValues("$(tasks.sumTasks.results.sumResult.foo.bar)"), + }, + wantParsedResult: resultref.ParsedResult{}, + wantError: fmt.Errorf("must be one of the form 1). \"tasks..results.\"; 2). \"tasks..results..\"; 3). \"steps..results.\"; 4). \"steps..results..\""), + }, { + name: "not a task result ref", + param: v1.Param{ + Name: "param", + Value: *v1.NewStructuredValues("$(nottasks.sumTasks.results.sumResult.foo.bar)"), + }, + wantParsedResult: resultref.ParsedResult{}, + wantError: fmt.Errorf("must be one of the form 1). \"tasks..results.\"; 2). \"tasks..results..\""), + }} { + t.Run(tt.name, func(t *testing.T) { + expressions, _ := tt.param.GetVarSubstitutionExpressions() + gotParsedResult, gotError := resultref.ParseTaskExpression(expressions[0]) + if d := cmp.Diff(tt.wantParsedResult, gotParsedResult); d != "" { + t.Error(diff.PrintWantGot(d)) + } + 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 TestParseResultName(t *testing.T) { + tests := []struct { + name string + input string + want []string + }{{ + name: "array indexing", + input: "anArrayResult[1]", + want: []string{"anArrayResult", "1"}, + }, + { + name: "array star reference", + input: "anArrayResult[*]", + want: []string{"anArrayResult", "*"}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + resultName, idx := v1.ParseResultName(tt.input) + if d := cmp.Diff(tt.want, []string{resultName, idx}); d != "" { + t.Error(diff.PrintWantGot(d)) + } + }) + } +} diff --git a/pkg/pod/entrypoint.go b/pkg/pod/entrypoint.go index 72096adb72b..f8c73d7491b 100644 --- a/pkg/pod/entrypoint.go +++ b/pkg/pod/entrypoint.go @@ -353,11 +353,12 @@ 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 { +// GetContainerName prefixes the input name with "step-" +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 5fa635907ae..2283246b95e 100644 --- a/pkg/reconciler/taskrun/resources/apply.go +++ b/pkg/reconciler/taskrun/resources/apply.go @@ -20,11 +20,14 @@ import ( "context" "fmt" "path/filepath" + "regexp" "strconv" + "strings" "github.com/tektoncd/pipeline/pkg/apis/pipeline" v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" "github.com/tektoncd/pipeline/pkg/container" + "github.com/tektoncd/pipeline/pkg/internal/resultref" "github.com/tektoncd/pipeline/pkg/pod" "github.com/tektoncd/pipeline/pkg/substitution" corev1 "k8s.io/api/core/v1" @@ -44,6 +47,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 +73,86 @@ 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) + } + 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, "$("), ")") + pr, err := resultref.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)", pr.ResourceName, pr.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[*])", pr.ResourceName, pr.ResultName) + } + // $(params.p1[idx]) --> $(steps.step1.results.foo[idx]) + for k, v := range replacementsArrayIdxStepResults(step, d.Name, pr.ResourceName, pr.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()