From c6958c9d5c7fd3c7f44685bc7b4d773cc383ecc0 Mon Sep 17 00:00:00 2001 From: Chitrang Patel Date: Fri, 3 Nov 2023 14:38:42 -0400 Subject: [PATCH] Add support for params between Step and StepActions Following the previous [PR](https://github.com/tektoncd/pipeline/pull/7317), which introduced Params to the `StepAction` CRD, this PR integrates `param` usage between `Steps` and `StepActions`. This completes support for params in `StepActions`. This work is part of issue https://github.com/tektoncd/pipeline/issues/7259. --- .../pipelineruns/alpha/stepaction-params.yaml | 67 ++++ .../v1/taskruns/alpha/stepaction-params.yaml | 75 ++++ pkg/reconciler/pipelinerun/resources/apply.go | 6 +- pkg/reconciler/taskrun/resources/apply.go | 146 ++++++- .../taskrun/resources/apply_test.go | 56 +++ pkg/reconciler/taskrun/resources/taskref.go | 2 +- pkg/reconciler/taskrun/resources/taskspec.go | 7 + .../taskrun/resources/taskspec_test.go | 364 +++++++++++++++++- .../taskrun/resources/validate_params.go | 38 ++ pkg/reconciler/taskrun/taskrun_test.go | 191 +++++++++ 10 files changed, 929 insertions(+), 23 deletions(-) create mode 100644 examples/v1/pipelineruns/alpha/stepaction-params.yaml create mode 100644 examples/v1/taskruns/alpha/stepaction-params.yaml diff --git a/examples/v1/pipelineruns/alpha/stepaction-params.yaml b/examples/v1/pipelineruns/alpha/stepaction-params.yaml new file mode 100644 index 00000000000..860d1483e91 --- /dev/null +++ b/examples/v1/pipelineruns/alpha/stepaction-params.yaml @@ -0,0 +1,67 @@ +apiVersion: tekton.dev/v1alpha1 +kind: StepAction +metadata: + name: step-action +spec: + params: + - name: string-param + default: "a string param" + - name: array-param + type: array + default: + - an + - array + - param + - name: object-param + type: object + properties: + key1: + type: string + key2: + type: string + key3: + type: string + default: + key1: "step-action default key1" + key2: "step-action default key2" + key3: "step-action default key3" + image: bash:3.2 + args: [ + "echo", + "$(params.array-param[*])", + "$(params.string-param)", + "$(params.object-param.key1)", + "$(params.object-param.key2)" + ] +--- +apiVersion: tekton.dev/v1 +kind: PipelineRun +metadata: + name: step-action-pipeline-run-propagated +spec: + params: + - name: stringparam + value: "pipelinerun stringparam" + - name: arrayparam + value: + - "pipelinerun" + - "array" + - "param" + - name: objectparam + value: + key2: "pipelinerun key2" + PipelineSpec: + tasks: + - name: run-action + taskSpec: + steps: + - name: action-runner + ref: + name: step-action + params: + - name: string-param + value: $(params.stringparam) + - name: array-param + value: $(params.arrayparam[*]) + - name: object-param + value: $(params.objectparam[*]) diff --git a/examples/v1/taskruns/alpha/stepaction-params.yaml b/examples/v1/taskruns/alpha/stepaction-params.yaml new file mode 100644 index 00000000000..b17eb8da8b1 --- /dev/null +++ b/examples/v1/taskruns/alpha/stepaction-params.yaml @@ -0,0 +1,75 @@ +apiVersion: tekton.dev/v1alpha1 +kind: StepAction +metadata: + name: step-action +spec: + params: + - name: string-param + default: "a string param" + - name: array-param + type: array + default: + - an + - array + - param + - name: object-param + type: object + properties: + key1: + type: string + key2: + type: string + key3: + type: string + default: + key1: "step-action default key1" + key2: "step-action default key2" + key3: "step-action default key3" + image: bash:3.2 + args: [ + "echo", + "$(params.array-param[*])", + "$(params.string-param)", + "$(params.object-param.key1)", + "$(params.object-param.key2)", + "$(params.object-param.key3)" + ] +--- +apiVersion: tekton.dev/v1 +kind: TaskRun +metadata: + name: step-action-run +spec: + params: + - name: stringparam + value: "taskrun stringparam" + - name: arrayparam + value: + - "taskrun" + - "array" + - "param" + - name: objectparam + value: + key2: "taksrun key2" + TaskSpec: + params: + - name: objectparam + properties: + key1: + type: string + key2: + type: string + default: + key1: "taskspec default key1" + key2: "taskspec default key2" + steps: + - name: action-runner + ref: + name: step-action + params: + - name: string-param + value: $(params.stringparam) + - name: array-param + value: $(params.arrayparam[*]) + - name: object-param + value: $(params.objectparam[*]) diff --git a/pkg/reconciler/pipelinerun/resources/apply.go b/pkg/reconciler/pipelinerun/resources/apply.go index 39210369f9c..5ed091dd11f 100644 --- a/pkg/reconciler/pipelinerun/resources/apply.go +++ b/pkg/reconciler/pipelinerun/resources/apply.go @@ -367,9 +367,9 @@ func propagateParams(t v1.PipelineTask, stringReplacements map[string]string, ar } } } - t.TaskSpec.TaskSpec = *resources.ApplyReplacements(&t.TaskSpec.TaskSpec, stringReplacementsDup, arrayReplacementsDup) + t.TaskSpec.TaskSpec = *resources.ApplyReplacements(&t.TaskSpec.TaskSpec, stringReplacementsDup, arrayReplacementsDup, objectReplacementsDup) } else { - t.TaskSpec.TaskSpec = *resources.ApplyReplacements(&t.TaskSpec.TaskSpec, stringReplacements, arrayReplacements) + t.TaskSpec.TaskSpec = *resources.ApplyReplacements(&t.TaskSpec.TaskSpec, stringReplacements, arrayReplacements, objectReplacements) } return t } @@ -395,7 +395,7 @@ func PropagateResults(rpt *ResolvedPipelineTask, runStates PipelineRunState) { } } } - rpt.ResolvedTask.TaskSpec = resources.ApplyReplacements(rpt.ResolvedTask.TaskSpec, stringReplacements, arrayReplacements) + rpt.ResolvedTask.TaskSpec = resources.ApplyReplacements(rpt.ResolvedTask.TaskSpec, stringReplacements, arrayReplacements, map[string]map[string]string{}) } // ApplyTaskResultsToPipelineResults applies the results of completed TasksRuns and Runs to a Pipeline's diff --git a/pkg/reconciler/taskrun/resources/apply.go b/pkg/reconciler/taskrun/resources/apply.go index f99ac510e80..9c3273dc3be 100644 --- a/pkg/reconciler/taskrun/resources/apply.go +++ b/pkg/reconciler/taskrun/resources/apply.go @@ -46,14 +46,74 @@ var ( } ) -// ApplyParameters applies the params from a TaskRun.Input.Parameters to a TaskSpec -func ApplyParameters(ctx context.Context, spec *v1.TaskSpec, tr *v1.TaskRun, defaults ...v1.ParamSpec) *v1.TaskSpec { +// applyStepActionParameters applies the params from a TaskRun.Input.Parameters to a TaskSpec +func applyStepActionParameters(ctx context.Context, step *v1.Step, spec *v1.TaskSpec, tr *v1.TaskRun, stepParams v1.Params, defaults []v1.ParamSpec) *v1.Step { + + if stepParams != nil { + stringR, arrayR, objectR := getTaskParameters(ctx, spec, tr, spec.Params...) + stepParams = stepParams.ReplaceVariables(stringR, arrayR, objectR) + } + stringReplacements := map[string]string{} + arrayReplacements := map[string][]string{} + objectReplacements := map[string]map[string]string{} + + // Set all the default stringReplacements + for _, p := range defaults { + if p.Default != nil { + switch p.Default.Type { + case v1.ParamTypeArray: + for _, pattern := range paramPatterns { + for i := 0; i < len(p.Default.ArrayVal); i++ { + stringReplacements[fmt.Sprintf(pattern+"[%d]", p.Name, i)] = p.Default.ArrayVal[i] + } + arrayReplacements[fmt.Sprintf(pattern, p.Name)] = p.Default.ArrayVal + } + case v1.ParamTypeObject: + for _, pattern := range paramPatterns { + objectReplacements[fmt.Sprintf(pattern, p.Name)] = p.Default.ObjectVal + } + for k, v := range p.Default.ObjectVal { + stringReplacements[fmt.Sprintf(objectIndividualVariablePattern, p.Name, k)] = v + } + case v1.ParamTypeString: + fallthrough + default: + for _, pattern := range paramPatterns { + stringReplacements[fmt.Sprintf(pattern, p.Name)] = p.Default.StringVal + } + } + } + } + + // Set and overwrite params with the ones from the Step + stepStrings, stepArrays, stepObjects := paramsFromStep(ctx, stepParams) + for k, v := range stepStrings { + stringReplacements[k] = v + } + for k, v := range stepArrays { + arrayReplacements[k] = v + } + for k, v := range stepObjects { + for key, val := range v { + if objectReplacements != nil && objectReplacements[k] != nil { + objectReplacements[k][key] = val + } + } + } + + container.ApplyStepReplacements(step, stringReplacements, arrayReplacements) + return step +} + +// getTaskParameters applies the params from a TaskRun.Input.Parameters to a TaskSpec +func getTaskParameters(ctx context.Context, 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. // stringReplacements is used for standard single-string stringReplacements, while arrayReplacements contains arrays // that need to be further processed. stringReplacements := map[string]string{} arrayReplacements := map[string][]string{} + objectReplacements := map[string]map[string]string{} // Set all the default stringReplacements for _, p := range defaults { @@ -67,6 +127,9 @@ func ApplyParameters(ctx context.Context, spec *v1.TaskSpec, tr *v1.TaskRun, def arrayReplacements[fmt.Sprintf(pattern, p.Name)] = p.Default.ArrayVal } case v1.ParamTypeObject: + for _, pattern := range paramPatterns { + objectReplacements[fmt.Sprintf(pattern, p.Name)] = p.Default.ObjectVal + } for k, v := range p.Default.ObjectVal { stringReplacements[fmt.Sprintf(objectIndividualVariablePattern, p.Name, k)] = v } @@ -80,22 +143,73 @@ func ApplyParameters(ctx context.Context, spec *v1.TaskSpec, tr *v1.TaskRun, def } } // Set and overwrite params with the ones from the TaskRun - trStrings, trArrays := paramsFromTaskRun(ctx, tr) + trStrings, trArrays, trObjects := paramsFromTaskRun(ctx, tr) for k, v := range trStrings { stringReplacements[k] = v } for k, v := range trArrays { arrayReplacements[k] = v } + for k, v := range trObjects { + for key, val := range v { + if objectReplacements != nil { + if objectReplacements[k] != nil { + objectReplacements[k][key] = val + } else { + objectReplacements[k] = v + } + } + } + } + return stringReplacements, arrayReplacements, objectReplacements +} - return ApplyReplacements(spec, stringReplacements, arrayReplacements) +// ApplyParameters applies the params from a TaskRun.Input.Parameters to a TaskSpec +func ApplyParameters(ctx context.Context, spec *v1.TaskSpec, tr *v1.TaskRun, defaults ...v1.ParamSpec) *v1.TaskSpec { + stringReplacements, arrayReplacements, objectReplacements := getTaskParameters(ctx, spec, tr, defaults...) + return ApplyReplacements(spec, stringReplacements, arrayReplacements, objectReplacements) +} + +func paramsFromStep(ctx context.Context, stepParams v1.Params) (map[string]string, map[string][]string, map[string]map[string]string) { + // stringReplacements is used for standard single-string stringReplacements, while arrayReplacements contains arrays + // that need to be further processed. + stringReplacements := map[string]string{} + arrayReplacements := map[string][]string{} + objectReplacements := map[string]map[string]string{} + + for _, p := range stepParams { + switch p.Value.Type { + case v1.ParamTypeArray: + for _, pattern := range paramPatterns { + for i := 0; i < len(p.Value.ArrayVal); i++ { + stringReplacements[fmt.Sprintf(pattern+"[%d]", p.Name, i)] = p.Value.ArrayVal[i] + } + arrayReplacements[fmt.Sprintf(pattern, p.Name)] = p.Value.ArrayVal + } + case v1.ParamTypeObject: + for _, pattern := range paramPatterns { + objectReplacements[fmt.Sprintf(pattern, p.Name)] = p.Value.ObjectVal + } + for k, v := range p.Value.ObjectVal { + stringReplacements[fmt.Sprintf(objectIndividualVariablePattern, p.Name, k)] = v + } + case v1.ParamTypeString: + fallthrough + default: + for _, pattern := range paramPatterns { + stringReplacements[fmt.Sprintf(pattern, p.Name)] = p.Value.StringVal + } + } + } + return stringReplacements, arrayReplacements, objectReplacements } -func paramsFromTaskRun(ctx context.Context, tr *v1.TaskRun) (map[string]string, map[string][]string) { +func paramsFromTaskRun(ctx context.Context, tr *v1.TaskRun) (map[string]string, map[string][]string, map[string]map[string]string) { // stringReplacements is used for standard single-string stringReplacements, while arrayReplacements contains arrays // that need to be further processed. stringReplacements := map[string]string{} arrayReplacements := map[string][]string{} + objectReplacements := map[string]map[string]string{} for _, p := range tr.Spec.Params { switch p.Value.Type { @@ -107,6 +221,9 @@ func paramsFromTaskRun(ctx context.Context, tr *v1.TaskRun) (map[string]string, arrayReplacements[fmt.Sprintf(pattern, p.Name)] = p.Value.ArrayVal } case v1.ParamTypeObject: + for _, pattern := range paramPatterns { + objectReplacements[fmt.Sprintf(pattern, p.Name)] = p.Value.ObjectVal + } for k, v := range p.Value.ObjectVal { stringReplacements[fmt.Sprintf(objectIndividualVariablePattern, p.Name, k)] = v } @@ -119,7 +236,7 @@ func paramsFromTaskRun(ctx context.Context, tr *v1.TaskRun) (map[string]string, } } - return stringReplacements, arrayReplacements + return stringReplacements, arrayReplacements, objectReplacements } func getContextReplacements(taskName string, tr *v1.TaskRun) map[string]string { @@ -135,7 +252,7 @@ func getContextReplacements(taskName string, tr *v1.TaskRun) map[string]string { // ApplyContexts applies the substitution from $(context.(taskRun|task).*) with the specified values. // Uses "" as a default if a value is not available. func ApplyContexts(spec *v1.TaskSpec, taskName string, tr *v1.TaskRun) *v1.TaskSpec { - return ApplyReplacements(spec, getContextReplacements(taskName, tr), map[string][]string{}) + return ApplyReplacements(spec, getContextReplacements(taskName, tr), map[string][]string{}, map[string]map[string]string{}) } // ApplyWorkspaces applies the substitution from paths that the workspaces in declarations mounted to, the @@ -170,7 +287,7 @@ func ApplyWorkspaces(ctx context.Context, spec *v1.TaskSpec, declarations []v1.W stringReplacements[fmt.Sprintf("workspaces.%s.claim", binding.Name)] = "" } } - return ApplyReplacements(spec, stringReplacements, map[string][]string{}) + return ApplyReplacements(spec, stringReplacements, map[string][]string{}, map[string]map[string]string{}) } // applyWorkspaceMountPath accepts a workspace path variable of the form $(workspaces.foo.path) and replaces @@ -204,7 +321,7 @@ func applyWorkspaceMountPath(variable string, spec *v1.TaskSpec, declaration v1. // Replace any remaining instances of the workspace path variable, which should fall // back to the mount path specified in the declaration. stringReplacements[variable] = defaultMountPath - return ApplyReplacements(spec, stringReplacements, emptyArrayReplacements) + return ApplyReplacements(spec, stringReplacements, emptyArrayReplacements, map[string]map[string]string{}) } // ApplyTaskResults applies the substitution from values in results which are referenced in spec as subitems @@ -223,7 +340,7 @@ func ApplyTaskResults(spec *v1.TaskSpec) *v1.TaskSpec { stringReplacements[fmt.Sprintf(pattern, result.Name)] = filepath.Join(pipeline.DefaultResultPath, result.Name) } } - return ApplyReplacements(spec, stringReplacements, map[string][]string{}) + return ApplyReplacements(spec, stringReplacements, map[string][]string{}, map[string]map[string]string{}) } // ApplyStepExitCodePath replaces the occurrences of exitCode path with the absolute tekton internal path @@ -235,7 +352,7 @@ func ApplyStepExitCodePath(spec *v1.TaskSpec) *v1.TaskSpec { stringReplacements[fmt.Sprintf("steps.%s.exitCode.path", pod.StepName(step.Name, i))] = filepath.Join(pipeline.StepsDir, pod.StepName(step.Name, i), "exitCode") } - return ApplyReplacements(spec, stringReplacements, map[string][]string{}) + return ApplyReplacements(spec, stringReplacements, map[string][]string{}, map[string]map[string]string{}) } // ApplyCredentialsPath applies a substitution of the key $(credentials.path) with the path that credentials @@ -244,16 +361,19 @@ func ApplyCredentialsPath(spec *v1.TaskSpec, path string) *v1.TaskSpec { stringReplacements := map[string]string{ "credentials.path": path, } - return ApplyReplacements(spec, stringReplacements, map[string][]string{}) + return ApplyReplacements(spec, stringReplacements, map[string][]string{}, map[string]map[string]string{}) } // ApplyReplacements replaces placeholders for declared parameters with the specified replacements. -func ApplyReplacements(spec *v1.TaskSpec, stringReplacements map[string]string, arrayReplacements map[string][]string) *v1.TaskSpec { +func ApplyReplacements(spec *v1.TaskSpec, stringReplacements map[string]string, arrayReplacements map[string][]string, objectReplacements map[string]map[string]string) *v1.TaskSpec { spec = spec.DeepCopy() // Apply variable expansion to steps fields. steps := spec.Steps for i := range steps { + if steps[i].Params != nil { + steps[i].Params = steps[i].Params.ReplaceVariables(stringReplacements, arrayReplacements, objectReplacements) + } container.ApplyStepReplacements(&steps[i], stringReplacements, arrayReplacements) } diff --git a/pkg/reconciler/taskrun/resources/apply_test.go b/pkg/reconciler/taskrun/resources/apply_test.go index 625041411a6..61bd533afdf 100644 --- a/pkg/reconciler/taskrun/resources/apply_test.go +++ b/pkg/reconciler/taskrun/resources/apply_test.go @@ -175,6 +175,26 @@ var ( }}, } + stepParamTaskSpec = &v1.TaskSpec{ + Params: v1.ParamSpecs{{ + Name: "myObject", + Default: &v1.ParamValue{ + Type: v1.ParamTypeObject, + ObjectVal: map[string]string{"key1": "key1"}, + }, + }}, + Steps: []v1.Step{{ + Name: "foo", + Ref: &v1.Ref{ + Name: "stepAction", + }, + Params: v1.Params{{ + Name: "myObject", + Value: *v1.NewStructuredValues("$(params.myObject[*])"), + }}, + }}, + } + // a taskspec for testing object var in all places i.e. Sidecars, StepTemplate, Steps and Volumns objectParamTaskSpec = &v1.TaskSpec{ Sidecars: []v1.Sidecar{{ @@ -933,6 +953,42 @@ func TestApplyObjectParameters(t *testing.T) { } } +func TestApplyStepParameters(t *testing.T) { + // define the taskrun to test values provided by taskrun can overwrite the values provided in spec's default + tr := &v1.TaskRun{ + Spec: v1.TaskRunSpec{ + Params: []v1.Param{{ + Name: "myObject", + Value: *v1.NewObject(map[string]string{ + "key1": "taskrun-value-for-key1", + }), + }}, + TaskSpec: stepParamTaskSpec, + }, + } + dp := []v1.ParamSpec{{ + Name: "myObject", + Default: &v1.ParamValue{ + Type: v1.ParamTypeObject, + ObjectVal: map[string]string{"key1": "key1"}, + }, + }} + + want := applyMutation(stepParamTaskSpec, func(spec *v1.TaskSpec) { + spec.Steps[0].Params = []v1.Param{{ + Name: "myObject", + Value: v1.ParamValue{ + Type: v1.ParamTypeObject, + ObjectVal: map[string]string{"key1": "taskrun-value-for-key1"}, + }, + }} + }) + got := resources.ApplyParameters(context.Background(), stepParamTaskSpec, tr, dp...) + if d := cmp.Diff(want, got); d != "" { + t.Errorf("ApplyParameters() got diff %s", diff.PrintWantGot(d)) + } +} + func TestApplyWorkspaces(t *testing.T) { names.TestingSeed() ts := &v1.TaskSpec{ diff --git a/pkg/reconciler/taskrun/resources/taskref.go b/pkg/reconciler/taskrun/resources/taskref.go index 32a687e3f7e..5eb88c9ff42 100644 --- a/pkg/reconciler/taskrun/resources/taskref.go +++ b/pkg/reconciler/taskrun/resources/taskref.go @@ -97,7 +97,7 @@ func GetTaskFunc(ctx context.Context, k8s kubernetes.Interface, tekton clientset return func(ctx context.Context, name string) (*v1.Task, *v1.RefSource, *trustedresources.VerificationResult, error) { var replacedParams v1.Params if ownerAsTR, ok := owner.(*v1.TaskRun); ok { - stringReplacements, arrayReplacements := paramsFromTaskRun(ctx, ownerAsTR) + stringReplacements, arrayReplacements, _ := paramsFromTaskRun(ctx, ownerAsTR) for k, v := range getContextReplacements("", ownerAsTR) { stringReplacements[k] = v } diff --git a/pkg/reconciler/taskrun/resources/taskspec.go b/pkg/reconciler/taskrun/resources/taskspec.go index 66c19f7dde6..1643bf84256 100644 --- a/pkg/reconciler/taskrun/resources/taskspec.go +++ b/pkg/reconciler/taskrun/resources/taskspec.go @@ -112,6 +112,8 @@ func GetStepActionsData(ctx context.Context, taskSpec v1.TaskSpec, taskRun *v1.T return nil, err } stepActionSpec := stepAction.StepActionSpec() + stepActionSpec.SetDefaults(ctx) + s.Image = stepActionSpec.Image if len(stepActionSpec.Command) > 0 { s.Command = stepActionSpec.Command @@ -125,6 +127,11 @@ func GetStepActionsData(ctx context.Context, taskSpec v1.TaskSpec, taskRun *v1.T if stepActionSpec.Env != nil { s.Env = stepActionSpec.Env } + if err := validateStepHasStepActionParameters(ctx, s.Params, stepActionSpec.Params); err != nil { + return nil, err + } + s = applyStepActionParameters(ctx, s, &taskSpec, taskRun, s.Params, stepActionSpec.Params) + s.Params = nil s.Ref = nil steps = append(steps, *s) } else { diff --git a/pkg/reconciler/taskrun/resources/taskspec_test.go b/pkg/reconciler/taskrun/resources/taskspec_test.go index 47a9dd8239c..1f254bfe53b 100644 --- a/pkg/reconciler/taskrun/resources/taskspec_test.go +++ b/pkg/reconciler/taskrun/resources/taskspec_test.go @@ -454,6 +454,292 @@ func TestGetStepActionsData(t *testing.T) { Image: "foo", Command: []string{"ls"}, }}, + }, { + name: "params propagated from taskrun", + tr: &v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mytaskrun", + Namespace: "default", + }, + Spec: v1.TaskRunSpec{ + Params: v1.Params{{ + Name: "stringparam", + Value: v1.ParamValue{ + Type: v1.ParamTypeString, + StringVal: "taskrun string param", + }, + }, { + Name: "arrayparam", + Value: v1.ParamValue{ + Type: v1.ParamTypeArray, + ArrayVal: []string{"taskrun", "array", "param"}, + }, + }, { + Name: "objectparam", + Value: v1.ParamValue{ + Type: v1.ParamTypeObject, + ObjectVal: map[string]string{"key": "taskrun object param"}, + }, + }}, + TaskSpec: &v1.TaskSpec{ + Steps: []v1.Step{{ + Ref: &v1.Ref{ + Name: "stepAction", + }, + Params: v1.Params{{ + Name: "string-param", + Value: *v1.NewStructuredValues("$(params.stringparam)"), + }, { + Name: "array-param", + Value: *v1.NewStructuredValues("$(params.arrayparam[*])"), + }, { + Name: "object-param", + Value: *v1.NewStructuredValues("$(params.objectparam[*])"), + }}, + }}, + }, + }, + }, + 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)"}, + 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{{ + Image: "myimage", + Args: []string{"taskrun string param", "taskrun", "array", "taskrun", "array", "param", "taskrun object param"}, + }}, + }, { + name: "params propagated from taskspec", + tr: &v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mytaskrun", + Namespace: "default", + }, + Spec: v1.TaskRunSpec{ + TaskSpec: &v1.TaskSpec{ + Params: v1.ParamSpecs{{ + Name: "stringparam", + Default: &v1.ParamValue{ + Type: v1.ParamTypeString, + StringVal: "taskspec string param", + }, + }, { + Name: "arrayparam", + Default: &v1.ParamValue{ + Type: v1.ParamTypeArray, + ArrayVal: []string{"taskspec", "array", "param"}, + }, + }, { + Name: "objectparam", + Default: &v1.ParamValue{ + Type: v1.ParamTypeObject, + ObjectVal: map[string]string{"key": "taskspec object param"}, + }, + }}, + Steps: []v1.Step{{ + Ref: &v1.Ref{ + Name: "stepAction", + }, + Params: v1.Params{{ + Name: "string-param", + Value: *v1.NewStructuredValues("$(params.stringparam)"), + }, { + Name: "array-param", + Value: *v1.NewStructuredValues("$(params.arrayparam[*])"), + }, { + Name: "object-param", + Value: *v1.NewStructuredValues("$(params.objectparam[*])"), + }}, + }}, + }, + }, + }, + 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)"}, + 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{{ + Image: "myimage", + Args: []string{"taskspec string param", "taskspec", "array", "taskspec", "array", "param", "taskspec object param"}, + }}, + }, { + name: "params from step action defaults", + tr: &v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mytaskrun", + Namespace: "default", + }, + Spec: v1.TaskRunSpec{ + TaskSpec: &v1.TaskSpec{ + Steps: []v1.Step{{ + Ref: &v1.Ref{ + Name: "stepAction", + }, + }}, + }, + }, + }, + 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)"}, + Params: v1.ParamSpecs{{ + Name: "string-param", + Type: v1.ParamTypeString, + Default: &v1.ParamValue{ + Type: v1.ParamTypeString, + StringVal: "step action string param", + }, + }, { + Name: "array-param", + Type: v1.ParamTypeArray, + Default: &v1.ParamValue{ + Type: v1.ParamTypeArray, + ArrayVal: []string{"step action", "array", "param"}, + }, + }, { + Name: "object-param", + Type: v1.ParamTypeObject, + Properties: map[string]v1.PropertySpec{"key": {Type: "string"}}, + Default: &v1.ParamValue{ + Type: v1.ParamTypeObject, + ObjectVal: map[string]string{"key": "step action object param"}, + }, + }}, + }, + }, + want: []v1.Step{{ + Image: "myimage", + Args: []string{"step action string param", "step action", "array", "step action", "array", "param", "step action object param"}, + }}, + }, { + name: "params propagated partially from taskrun taskspec and stepaction", + tr: &v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mytaskrun", + Namespace: "default", + }, + Spec: v1.TaskRunSpec{ + Params: v1.Params{{ + Name: "stringparam", + Value: v1.ParamValue{ + Type: v1.ParamTypeString, + StringVal: "taskrun string param", + }, + }, { + Name: "objectparam", + Value: v1.ParamValue{ + Type: v1.ParamTypeObject, + ObjectVal: map[string]string{"key": "taskrun key"}, + }, + }}, + TaskSpec: &v1.TaskSpec{ + Params: v1.ParamSpecs{{ + Name: "arrayparam", + Default: &v1.ParamValue{ + Type: v1.ParamTypeArray, + ArrayVal: []string{"taskspec", "array", "param"}, + }, + }, { + Name: "objectparam", + Default: &v1.ParamValue{ + Type: v1.ParamTypeObject, + ObjectVal: map[string]string{"key": "key1", "key2": "taskspec key2"}, + }, + }}, + Steps: []v1.Step{{ + Ref: &v1.Ref{ + Name: "stepAction", + }, + Params: v1.Params{{ + Name: "string-param", + Value: *v1.NewStructuredValues("$(params.stringparam)"), + }, { + Name: "array-param", + Value: *v1.NewStructuredValues("$(params.arrayparam[*])"), + }, { + Name: "object-param", + Value: *v1.NewStructuredValues("$(params.objectparam[*])"), + }}, + }}, + }, + }, + }, + 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)", "$(params.object-param.key2)", "$(params.object-param.key3)"}, + Params: v1.ParamSpecs{{ + Name: "string-param", + Type: v1.ParamTypeString, + Default: &v1.ParamValue{ + Type: v1.ParamTypeString, + StringVal: "step action string param", + }, + }, { + Name: "array-param", + Type: v1.ParamTypeArray, + Default: &v1.ParamValue{ + Type: v1.ParamTypeArray, + ArrayVal: []string{"step action", "array", "param"}, + }, + }, { + Name: "object-param", + Type: v1.ParamTypeObject, + Properties: map[string]v1.PropertySpec{"key": {Type: "string"}}, + Default: &v1.ParamValue{ + Type: v1.ParamTypeObject, + ObjectVal: map[string]string{"key": "step action key1", "key2": "step action key2", "key3": "step action key3"}, + }, + }}, + }, + }, + want: []v1.Step{{ + Image: "myimage", + Args: []string{"taskrun string param", "taskspec", "array", "taskspec", "array", "param", "taskrun key", "taskspec key2", "step action key3"}, + }}, }} for _, tt := range tests { ctx := context.Background() @@ -461,7 +747,7 @@ func TestGetStepActionsData(t *testing.T) { got, err := resources.GetStepActionsData(ctx, *tt.tr.Spec.TaskSpec, tt.tr, tektonclient, nil, nil) if err != nil { - t.Errorf("Did not expect an error but got : %s", err) + t.Fatalf("Did not expect an error but got : %s", err) } if d := cmp.Diff(tt.want, got); d != "" { t.Errorf("the taskSpec did not match what was expected diff: %s", diff.PrintWantGot(d)) @@ -471,10 +757,10 @@ func TestGetStepActionsData(t *testing.T) { func TestGetStepActionsData_Error(t *testing.T) { tests := []struct { - name string - tr *v1.TaskRun - stepActionFunc func(ctx context.Context, ref *v1.Ref) (*v1alpha1.StepAction, *v1.RefSource, error) - expectedError error + name string + tr *v1.TaskRun + stepAction *v1alpha1.StepAction + expectedError error }{{ name: "namespace missing error", tr: &v1.TaskRun{ @@ -491,10 +777,76 @@ func TestGetStepActionsData_Error(t *testing.T) { }, }, }, + stepAction: &v1alpha1.StepAction{}, expectedError: fmt.Errorf("must specify namespace to resolve reference to step action stepActionError"), + }, { + name: "params missing", + tr: &v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mytaskrun", + Namespace: "default", + }, + Spec: v1.TaskRunSpec{ + TaskSpec: &v1.TaskSpec{ + Steps: []v1.Step{{ + Ref: &v1.Ref{ + Name: "stepaction", + }, + }}, + }, + }, + }, + stepAction: &v1alpha1.StepAction{ + ObjectMeta: metav1.ObjectMeta{ + Name: "stepaction", + Namespace: "default", + }, + Spec: v1alpha1.StepActionSpec{ + Image: "myimage", + Params: v1.ParamSpecs{{ + Name: "string-param", + Type: v1.ParamTypeString, + }}, + }, + }, + expectedError: fmt.Errorf("non-existent params in Step: [string-param]"), + }, { + name: "params extra", + tr: &v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mytaskrun", + Namespace: "default", + }, + Spec: v1.TaskRunSpec{ + TaskSpec: &v1.TaskSpec{ + Steps: []v1.Step{{ + Ref: &v1.Ref{ + Name: "stepaction", + }, + Params: v1.Params{{ + Name: "string-param", + Value: *v1.NewStructuredValues("$(params.stringparam)"), + }}, + }}, + }, + }, + }, + stepAction: &v1alpha1.StepAction{ + ObjectMeta: metav1.ObjectMeta{ + Name: "stepaction", + Namespace: "default", + }, + Spec: v1alpha1.StepActionSpec{ + Image: "myimage", + }, + }, + expectedError: fmt.Errorf("extra params passed by Step to StepAction: [string-param]"), }} for _, tt := range tests { - _, err := resources.GetStepActionsData(context.Background(), *tt.tr.Spec.TaskSpec, tt.tr, nil, nil, nil) + ctx := context.Background() + tektonclient := fake.NewSimpleClientset(tt.stepAction) + + _, err := resources.GetStepActionsData(ctx, *tt.tr.Spec.TaskSpec, tt.tr, tektonclient, nil, nil) if err == nil { t.Fatalf("Expected to get an error but did not find any.") } diff --git a/pkg/reconciler/taskrun/resources/validate_params.go b/pkg/reconciler/taskrun/resources/validate_params.go index 345fe724164..6a75ee001dd 100644 --- a/pkg/reconciler/taskrun/resources/validate_params.go +++ b/pkg/reconciler/taskrun/resources/validate_params.go @@ -1,6 +1,7 @@ package resources import ( + "context" "fmt" v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" @@ -45,3 +46,40 @@ func ValidateOutOfBoundArrayParams(declarations v1.ParamSpecs, params v1.Params, } return nil } + +func validateStepHasStepActionParameters(ctx context.Context, stepParams v1.Params, stepActionDefaults []v1.ParamSpec) error { + stepActionParams := sets.String{} + requiredStepActionParams := []string{} + for _, sa := range stepActionDefaults { + stepActionParams.Insert(sa.Name) + if sa.Default == nil { + requiredStepActionParams = append(requiredStepActionParams, sa.Name) + } + } + + stepProvidedParams := sets.String{} + extra := []string{} + for _, sp := range stepParams { + stepProvidedParams.Insert(sp.Name) + if !stepActionParams.Has(sp.Name) { + // Extra parameter that is not needed + extra = append(extra, sp.Name) + } + } + if len(extra) > 0 { + return fmt.Errorf("extra params passed by Step to StepAction: %v", extra) + } + + missing := []string{} + + for _, requiredParam := range requiredStepActionParams { + if !stepProvidedParams.Has(requiredParam) { + // Missing required param + missing = append(missing, requiredParam) + } + } + if len(missing) > 0 { + return fmt.Errorf("non-existent params in Step: %v", missing) + } + return nil +} diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index 9275b02dc6f..a0c0c9f1dd5 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -3052,6 +3052,197 @@ spec: } } +func TestStepActionRefParams(t *testing.T) { + tests := []struct { + name string + taskRun *v1.TaskRun + stepAction *v1alpha1.StepAction + want []v1.Step + }{{ + name: "params propagated from taskrun", + taskRun: parse.MustParseV1TaskRun(t, ` +metadata: + name: taskrun-with-string-params + namespace: foo +spec: + params: + - name: stringparam + value: "taskrun string param" + - name: arrayparam + value: ["taskrun", "array", "param"] + - name: objectparam + value: + key: taskrun object param + taskSpec: + steps: + - ref: + name: stepAction + name: step1 + params: + - name: string-param + value: $(params.stringparam) + - name: array-param + value: $(params.arrayparam[*]) + - name: object-param + value: $(params.objectparam[*]) +`), + stepAction: parse.MustParseV1alpha1StepAction(t, ` +metadata: + name: stepAction + namespace: foo +spec: + params: + - name: string-param + - name: array-param + type: array + - name: object-param + type: object + properties: + key: + type: string + image: myImage + command: ["echo"] + args: ["$(params.string-param)", "$(params.array-param[0])", "$(params.array-param[1])", "$(params.array-param[*])", "$(params.object-param.key)"] +`), + want: []v1.Step{{ + Image: "myImage", + Command: []string{"echo"}, + Args: []string{"taskrun string param", "taskrun", "array", "taskrun", "array", "param", "taskrun object param"}, + Name: "step1", + }}, + }, { + name: "params from taskspec", + taskRun: parse.MustParseV1TaskRun(t, ` +metadata: + name: taskrun-with-string-params + namespace: foo +spec: + taskSpec: + params: + - name: stringparam + default: "taskspec string param" + - name: arrayparam + default: ["taskspec", "array", "param"] + - name: objectparam + properties: + key: + type: string + default: + key: taskspec object param + steps: + - ref: + name: stepAction + name: step1 + params: + - name: string-param + value: $(params.stringparam) + - name: array-param + value: $(params.arrayparam[*]) + - name: object-param + value: $(params.objectparam[*]) +`), + stepAction: parse.MustParseV1alpha1StepAction(t, ` +metadata: + name: stepAction + namespace: foo +spec: + params: + - name: string-param + - name: array-param + type: array + - name: object-param + type: object + properties: + key: + type: string + image: myImage + command: ["echo"] + args: ["$(params.string-param)", "$(params.array-param[0])", "$(params.array-param[1])", "$(params.array-param[*])", "$(params.object-param.key)"] +`), + want: []v1.Step{{ + Image: "myImage", + Command: []string{"echo"}, + Args: []string{"taskspec string param", "taskspec", "array", "taskspec", "array", "param", "taskspec object param"}, + Name: "step1", + }}, + }, { + name: "params from step action defaults", + taskRun: parse.MustParseV1TaskRun(t, ` +metadata: + name: taskrun-with-string-params + namespace: foo +spec: + taskSpec: + steps: + - ref: + name: stepAction + name: step1 +`), + stepAction: parse.MustParseV1alpha1StepAction(t, ` +metadata: + name: stepAction + namespace: foo +spec: + params: + - name: string-param + type: string + default: "stepaction string param" + - name: array-param + type: array + default: + - stepaction + - array + - param + - name: object-param + type: object + properties: + key: + type: string + default: + key: "stepaction object param" + results: + - name: result + image: myImage + command: ["echo"] + args: ["$(params.string-param)", "$(params.array-param[0])", "$(params.array-param[1])", "$(params.array-param[*])", "$(params.object-param.key)"] +`), + want: []v1.Step{{ + Image: "myImage", + Command: []string{"echo"}, + Args: []string{"stepaction string param", "stepaction", "array", "stepaction", "array", "param", "stepaction object param"}, + Name: "step1", + }}, + }} + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + d := test.Data{ + TaskRuns: []*v1.TaskRun{tt.taskRun}, + StepActions: []*v1alpha1.StepAction{tt.stepAction}, + ConfigMaps: []*corev1.ConfigMap{ + { + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()}, + Data: map[string]string{ + "enable-step-actions": "true", + }, + }, + }, + } + testAssets, cancel := getTaskRunController(t, d) + defer cancel() + createServiceAccount(t, testAssets, "default", tt.taskRun.Namespace) + c := testAssets.Controller + if err := c.Reconciler.Reconcile(testAssets.Ctx, getRunName(tt.taskRun)); err == nil { + t.Fatalf("Could not reconcile the taskrun: %v", err) + } + getTaskRun, _ := testAssets.Clients.Pipeline.TektonV1().TaskRuns(tt.taskRun.Namespace).Get(testAssets.Ctx, tt.taskRun.Name, metav1.GetOptions{}) + got := getTaskRun.Status.TaskSpec.Steps + if c := cmp.Diff(tt.want, got); c != "" { + t.Errorf("TestStepActionRefParams errored with: %s", diff.PrintWantGot(c)) + } + }) + } +} + func TestExpandMountPath(t *testing.T) { expectedMountPath := "/temppath/replaced" expectedReplacedArgs := fmt.Sprintf("replacedArgs - %s", expectedMountPath)