diff --git a/pkg/reconciler/taskrun/resources/apply.go b/pkg/reconciler/taskrun/resources/apply.go index 2b40716b409..e6036faf832 100644 --- a/pkg/reconciler/taskrun/resources/apply.go +++ b/pkg/reconciler/taskrun/resources/apply.go @@ -21,6 +21,7 @@ import ( "fmt" "path/filepath" "regexp" + "sort" "strconv" "strings" @@ -50,6 +51,14 @@ var ( "inputs.params.%s", } + substitutionToParamNamePatterns = []string{ + `^params\.(\w+)$`, + `^params\["([^"]+)"\]$`, + `^params\['([^']+)'\]$`, + // FIXME(vdemeester) Remove that with deprecating v1beta1 + `^inputs\.params\.(\w+)$`, + } + paramIndexRegexPatterns = []string{ `\$\(params.%s\[([0-9]*)*\*?\]\)`, `\$\(params\[%q\]\[([0-9]*)*\*?\]\)`, @@ -58,7 +67,7 @@ var ( ) // applyStepActionParameters applies the params from the Task and the underlying Step to the referenced StepAction. -func applyStepActionParameters(step *v1.Step, spec *v1.TaskSpec, tr *v1.TaskRun, stepParams v1.Params, defaults []v1.ParamSpec) *v1.Step { +func applyStepActionParameters(step *v1.Step, spec *v1.TaskSpec, tr *v1.TaskRun, stepParams v1.Params, defaults []v1.ParamSpec) (*v1.Step, error) { if stepParams != nil { stringR, arrayR, objectR := getTaskParameters(spec, tr, spec.Params...) stepParams = stepParams.ReplaceVariables(stringR, arrayR, objectR) @@ -79,8 +88,45 @@ func applyStepActionParameters(step *v1.Step, spec *v1.TaskSpec, tr *v1.TaskRun, for k, v := range stepResultReplacements { stringReplacements[k] = v } + + // Check if there are duplicate keys in the replacements + // If the same key is present in both stringReplacements and arrayReplacements, it means + // that the default value and the passed value have different types. + err := checkForDuplicateKeys(stringReplacements, arrayReplacements) + if err != nil { + return nil, err + } + container.ApplyStepReplacements(step, stringReplacements, arrayReplacements) - return step + return step, nil +} + +// checkForDuplicateKeys checks if there are duplicate keys in the replacements +func checkForDuplicateKeys(stringReplacements map[string]string, arrayReplacements map[string][]string) error { + keys := make([]string, 0, len(stringReplacements)) + for k := range stringReplacements { + keys = append(keys, k) + } + sort.Strings(keys) + for _, k := range keys { + if _, ok := arrayReplacements[k]; ok { + paramName := paramNameFromReplacementKey(k) + return fmt.Errorf("invalid parameter substitution: %s. Please check the types of the default value and the passed value", paramName) + } + } + return nil +} + +// paramNameFromReplacementKey returns the param name from the replacement key in best effort +func paramNameFromReplacementKey(key string) string { + for _, regexPattern := range substitutionToParamNamePatterns { + re := regexp.MustCompile(regexPattern) + if matches := re.FindStringSubmatch(key); matches != nil { + return matches[1] + } + } + // If no match is found, return the key + return key } // findArrayIndexParamUsage finds the array index in a string using array param substitution diff --git a/pkg/reconciler/taskrun/resources/taskspec.go b/pkg/reconciler/taskrun/resources/taskspec.go index f8f856da4ff..33275935010 100644 --- a/pkg/reconciler/taskrun/resources/taskspec.go +++ b/pkg/reconciler/taskrun/resources/taskspec.go @@ -118,7 +118,11 @@ func GetStepActionsData(ctx context.Context, taskSpec v1.TaskSpec, taskRun *v1.T if err := validateStepHasStepActionParameters(s.Params, stepActionSpec.Params); err != nil { return nil, err } - stepFromStepAction = applyStepActionParameters(stepFromStepAction, &taskSpec, taskRun, s.Params, stepActionSpec.Params) + + stepFromStepAction, err = applyStepActionParameters(stepFromStepAction, &taskSpec, taskRun, s.Params, stepActionSpec.Params) + if err != nil { + return nil, err + } s.Image = stepFromStepAction.Image s.SecurityContext = stepFromStepAction.SecurityContext diff --git a/pkg/reconciler/taskrun/resources/taskspec_test.go b/pkg/reconciler/taskrun/resources/taskspec_test.go index 0c53fff2330..5bf7716eceb 100644 --- a/pkg/reconciler/taskrun/resources/taskspec_test.go +++ b/pkg/reconciler/taskrun/resources/taskspec_test.go @@ -961,6 +961,54 @@ func TestGetStepActionsData(t *testing.T) { Value: "$(steps.inlined-step.results.result1)", }}, }}, + }, { + name: "param types are matching", + tr: &v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mytaskrun", + Namespace: "default", + }, + Spec: v1.TaskRunSpec{ + TaskSpec: &v1.TaskSpec{ + Steps: []v1.Step{{ + Name: "test", + Ref: &v1.Ref{Name: "stepAction"}, + Params: v1.Params{{ + Name: "commands", + Value: v1.ParamValue{ + Type: v1.ParamTypeArray, + ArrayVal: []string{"Hello, I am of type list"}, + }, + }}, + }}, + }, + }, + }, + stepAction: &v1beta1.StepAction{ + ObjectMeta: metav1.ObjectMeta{ + Name: "stepAction", + Namespace: "default", + }, + Spec: v1beta1.StepActionSpec{ + Image: "myimage", + Args: []string{"$(params.commands)"}, + Script: "echo $@", + Params: v1.ParamSpecs{{ + Name: "commands", + Type: v1.ParamTypeArray, + Default: &v1.ParamValue{ + Type: v1.ParamTypeArray, + ArrayVal: []string{"Hello, I am the default value"}, + }, + }}, + }, + }, + want: []v1.Step{{ + Name: "test", + Image: "myimage", + Args: []string{"Hello, I am of type list"}, + Script: "echo $@", + }}, }} for _, tt := range tests { ctx := context.Background() @@ -1062,6 +1110,46 @@ func TestGetStepActionsData_Error(t *testing.T) { }, }, expectedError: errors.New("extra params passed by Step to StepAction: [string-param]"), + }, { + name: "param types not matching", + tr: &v1.TaskRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "mytaskrun", + Namespace: "default", + }, + Spec: v1.TaskRunSpec{ + TaskSpec: &v1.TaskSpec{ + Steps: []v1.Step{{ + Name: "test", + Ref: &v1.Ref{Name: "stepAction"}, + Params: v1.Params{{ + Name: "commands", + Value: *v1.NewStructuredValues("Hello, I am of type string"), + }}, + }}, + }, + }, + }, + stepAction: &v1beta1.StepAction{ + ObjectMeta: metav1.ObjectMeta{ + Name: "stepAction", + Namespace: "default", + }, + Spec: v1beta1.StepActionSpec{ + Image: "myimage", + Args: []string{"$(params.commands)"}, + Script: "echo $@", + Params: v1.ParamSpecs{{ + Name: "commands", + Type: v1.ParamTypeArray, + Default: &v1.ParamValue{ + Type: v1.ParamTypeArray, + ArrayVal: []string{"Hello, I am the default value"}, + }, + }}, + }, + }, + expectedError: errors.New("invalid parameter substitution: commands. Please check the types of the default value and the passed value"), }} for _, tt := range tests { ctx := context.Background()