Skip to content

Commit

Permalink
fix step action datatype
Browse files Browse the repository at this point in the history
  • Loading branch information
samagana committed Jul 6, 2024
1 parent d345e99 commit 1476477
Show file tree
Hide file tree
Showing 3 changed files with 93 additions and 3 deletions.
50 changes: 48 additions & 2 deletions pkg/reconciler/taskrun/resources/apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"fmt"
"path/filepath"
"regexp"
"sort"
"strconv"
"strings"

Expand Down Expand Up @@ -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]*)*\*?\]\)`,
Expand All @@ -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)
Expand All @@ -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
Expand Down
6 changes: 5 additions & 1 deletion pkg/reconciler/taskrun/resources/taskspec.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
40 changes: 40 additions & 0 deletions pkg/reconciler/taskrun/resources/taskspec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1062,6 +1062,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()
Expand Down

0 comments on commit 1476477

Please sign in to comment.