From 07c25608540171f190d211be1a03c05ed139bab0 Mon Sep 17 00:00:00 2001 From: Ruin09 Date: Sat, 16 Sep 2023 09:29:08 +0900 Subject: [PATCH] fix: Fixed workflow template skip whitespaced parameters. Fixes #11767 (#11781) --- workflow/validate/validate.go | 30 ++++++++++++++++-------------- workflow/validate/validate_test.go | 23 +++++++++++++++++++++++ 2 files changed, 39 insertions(+), 14 deletions(-) diff --git a/workflow/validate/validate.go b/workflow/validate/validate.go index bdba06413b6b..24fbf9cf902c 100644 --- a/workflow/validate/validate.go +++ b/workflow/validate/validate.go @@ -654,30 +654,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) diff --git a/workflow/validate/validate_test.go b/workflow/validate/validate_test.go index d8f92e26a3ea..230d52967e1a 100644 --- a/workflow/validate/validate_test.go +++ b/workflow/validate/validate_test.go @@ -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 }}") + } +}