Skip to content

Commit

Permalink
fix: Fixed workflow template skip whitespaced parameters. Fixes #11767 (
Browse files Browse the repository at this point in the history
  • Loading branch information
shmruin authored and terrytangyuan committed Oct 19, 2023
1 parent a08d73a commit 71ad0a2
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 14 deletions.
30 changes: 16 additions & 14 deletions workflow/validate/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -651,30 +651,32 @@ func resolveAllVariables(scope map[string]interface{}, globalParams map[string]s
_, allowAllWorkflowOutputParameterRefs := scope[anyWorkflowOutputParameterMagicValue]
_, allowAllWorkflowOutputArtifactRefs := scope[anyWorkflowOutputArtifactMagicValue]
return template.Validate(tmplStr, func(tag string) error {
// Trim the tag to check the validations
trimmedTag := strings.TrimSpace(tag)
// Skip the custom variable references
if !checkValidWorkflowVariablePrefix(tag) {
if !checkValidWorkflowVariablePrefix(trimmedTag) {
return nil
}
_, ok := scope[tag]
_, isGlobal := globalParams[tag]
_, ok := scope[trimmedTag]
_, isGlobal := globalParams[trimmedTag]
if !ok && !isGlobal {
if (tag == "item" || strings.HasPrefix(tag, "item.")) && allowAllItemRefs {
if (trimmedTag == "item" || strings.HasPrefix(trimmedTag, "item.")) && allowAllItemRefs {
// we are *probably* referencing a undetermined item using withParam
// NOTE: this is far from foolproof.
} else if strings.HasPrefix(tag, "workflow.outputs.parameters.") && allowAllWorkflowOutputParameterRefs {
} else if strings.HasPrefix(trimmedTag, "workflow.outputs.parameters.") && allowAllWorkflowOutputParameterRefs {
// Allow runtime resolution of workflow output parameter names
} else if strings.HasPrefix(tag, "workflow.outputs.artifacts.") && allowAllWorkflowOutputArtifactRefs {
} else if strings.HasPrefix(trimmedTag, "workflow.outputs.artifacts.") && allowAllWorkflowOutputArtifactRefs {
// Allow runtime resolution of workflow output artifact names
} else if strings.HasPrefix(tag, "outputs.") {
} else if strings.HasPrefix(trimmedTag, "outputs.") {
// We are self referencing for metric emission, allow it.
} else if strings.HasPrefix(tag, common.GlobalVarWorkflowCreationTimestamp) {
} else if strings.HasPrefix(tag, common.GlobalVarWorkflowCronScheduleTime) {
} else if strings.HasPrefix(trimmedTag, common.GlobalVarWorkflowCreationTimestamp) {
} else if strings.HasPrefix(trimmedTag, common.GlobalVarWorkflowCronScheduleTime) {
// Allow runtime resolution for "scheduledTime" which will pass from CronWorkflow
} else if strings.HasPrefix(tag, common.GlobalVarWorkflowDuration) {
} else if strings.HasPrefix(tag, "tasks.name") {
} else if strings.HasPrefix(tag, "steps.name") {
} else if strings.HasPrefix(tag, "node.name") {
} else if strings.HasPrefix(tag, "workflow.parameters") && workflowTemplateValidation {
} else if strings.HasPrefix(trimmedTag, common.GlobalVarWorkflowDuration) {
} else if strings.HasPrefix(trimmedTag, "tasks.name") {
} else if strings.HasPrefix(trimmedTag, "steps.name") {
} else if strings.HasPrefix(trimmedTag, "node.name") {
} else if strings.HasPrefix(trimmedTag, "workflow.parameters") && workflowTemplateValidation {
// If we are simply validating a WorkflowTemplate in isolation, some of the parameters may come from the Workflow that uses it
} else {
return fmt.Errorf("failed to resolve {{%s}}", tag)
Expand Down
23 changes: 23 additions & 0 deletions workflow/validate/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3319,3 +3319,26 @@ func TestSubstituteGlobalVariablesLabelsAnnotations(t *testing.T) {
})
}
}

var spacedParameterWorkflowTemplate = `
apiVersion: argoproj.io/v1alpha1
kind: WorkflowTemplate
metadata:
generateName: hello-world-
spec:
entrypoint: helloworld
templates:
- name: helloworld
container:
image: "alpine:3.18"
command: ["echo", "{{ workflow.thisdoesnotexist }}"]
`

func TestShouldCheckValidationToSpacedParameters(t *testing.T) {
err := validate(spacedParameterWorkflowTemplate)
// Do not allow leading or trailing spaces in parameters
if assert.NotNil(t, err) {
assert.Contains(t, err.Error(), "failed to resolve {{ workflow.thisdoesnotexist }}")
}
}

0 comments on commit 71ad0a2

Please sign in to comment.