Skip to content

Commit

Permalink
fix: a WorkflowTemplate doesn't need to define workflow-level input p… (
Browse files Browse the repository at this point in the history 
#9762)

* fix: a WorkflowTemplate doesn't need to define workflow-level input parameters

Signed-off-by: Julie Vogelman <[email protected]>

* fix: accidental removal of code

Signed-off-by: Julie Vogelman <[email protected]>

* fix: accidental removal of code

Signed-off-by: Julie Vogelman <[email protected]>

Signed-off-by: Julie Vogelman <[email protected]>
  • Loading branch information
@juliev0
juliev0 authored Oct 10, 2022
1 parent b12b5f9 commit f1bab89
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 25 deletions.
50 changes: 26 additions & 24 deletions workflow/validate/validate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,14 +215,14 @@ func ValidateWorkflow(wftmplGetter templateresolution.WorkflowTemplateNamespaced
if hasWorkflowTemplateRef {
tmpl = &wfv1.WorkflowStep{TemplateRef: wfTmplRef}
}
_, err = ctx.validateTemplateHolder(tmpl, tmplCtx, args)
_, err = ctx.validateTemplateHolder(tmpl, tmplCtx, args, opts.WorkflowTemplateValidation)
if err != nil {
return err
}
}
if wf.Spec.OnExit != "" {
ctx.globalParams[common.GlobalVarWorkflowFailures] = placeholderGenerator.NextPlaceholder()
_, err = ctx.validateTemplateHolder(&wfv1.WorkflowStep{Template: wf.Spec.OnExit}, tmplCtx, &wf.Spec.Arguments)
_, err = ctx.validateTemplateHolder(&wfv1.WorkflowStep{Template: wf.Spec.OnExit}, tmplCtx, &wf.Spec.Arguments, opts.WorkflowTemplateValidation)
if err != nil {
return err
}
Expand All @@ -237,7 +237,7 @@ func ValidateWorkflow(wftmplGetter templateresolution.WorkflowTemplateNamespaced

// Check if all templates can be resolved.
for _, template := range wf.Spec.Templates {
_, err := ctx.validateTemplateHolder(&wfv1.WorkflowStep{Template: template.Name}, tmplCtx, &FakeArguments{})
_, err := ctx.validateTemplateHolder(&wfv1.WorkflowStep{Template: template.Name}, tmplCtx, &FakeArguments{}, opts.WorkflowTemplateValidation)
if err != nil {
return errors.Errorf(errors.CodeBadRequest, "templates.%s %s", template.Name, err.Error())
}
Expand Down Expand Up @@ -341,7 +341,7 @@ func (ctx *templateValidationCtx) validateInitContainers(containers []wfv1.UserC
return nil
}

func (ctx *templateValidationCtx) validateTemplate(tmpl *wfv1.Template, tmplCtx *templateresolution.Context, args wfv1.ArgumentsProvider) error {
func (ctx *templateValidationCtx) validateTemplate(tmpl *wfv1.Template, tmplCtx *templateresolution.Context, args wfv1.ArgumentsProvider, workflowTemplateValidation bool) error {
if err := validateTemplateType(tmpl); err != nil {
return err
}
Expand Down Expand Up @@ -413,16 +413,16 @@ func (ctx *templateValidationCtx) validateTemplate(tmpl *wfv1.Template, tmplCtx
}
switch newTmpl.GetType() {
case wfv1.TemplateTypeSteps:
err = ctx.validateSteps(scope, tmplCtx, newTmpl)
err = ctx.validateSteps(scope, tmplCtx, newTmpl, workflowTemplateValidation)
case wfv1.TemplateTypeDAG:
err = ctx.validateDAG(scope, tmplCtx, newTmpl)
err = ctx.validateDAG(scope, tmplCtx, newTmpl, workflowTemplateValidation)
default:
err = ctx.validateLeaf(scope, newTmpl)
err = ctx.validateLeaf(scope, newTmpl, workflowTemplateValidation)
}
if err != nil {
return err
}
err = validateOutputs(scope, ctx.globalParams, newTmpl)
err = validateOutputs(scope, ctx.globalParams, newTmpl, workflowTemplateValidation)
if err != nil {
return err
}
Expand Down Expand Up @@ -453,7 +453,7 @@ func (ctx *templateValidationCtx) validateTemplate(tmpl *wfv1.Template, tmplCtx
}

// validateTemplateHolder validates a template holder and returns the validated template.
func (ctx *templateValidationCtx) validateTemplateHolder(tmplHolder wfv1.TemplateReferenceHolder, tmplCtx *templateresolution.Context, args wfv1.ArgumentsProvider) (*wfv1.Template, error) {
func (ctx *templateValidationCtx) validateTemplateHolder(tmplHolder wfv1.TemplateReferenceHolder, tmplCtx *templateresolution.Context, args wfv1.ArgumentsProvider, workflowTemplateValidation bool) (*wfv1.Template, error) {
tmplRef := tmplHolder.GetTemplateRef()
tmplName := tmplHolder.GetTemplateName()
if tmplRef != nil {
Expand Down Expand Up @@ -498,7 +498,7 @@ func (ctx *templateValidationCtx) validateTemplateHolder(tmplHolder wfv1.Templat
}
}

return resolvedTmpl, ctx.validateTemplate(resolvedTmpl, tmplCtx, args)
return resolvedTmpl, ctx.validateTemplate(resolvedTmpl, tmplCtx, args, workflowTemplateValidation)
}

// validateTemplateType validates that only one template type is defined
Expand Down Expand Up @@ -580,7 +580,7 @@ func validateArtifactLocation(errPrefix string, art wfv1.ArtifactLocation) error
}

// resolveAllVariables is a helper to ensure all {{variables}} are resolvable from current scope
func resolveAllVariables(scope map[string]interface{}, globalParams map[string]string, tmplStr string) error {
func resolveAllVariables(scope map[string]interface{}, globalParams map[string]string, tmplStr string, workflowTemplateValidation bool) error {
_, allowAllItemRefs := scope[anyItemMagicValue] // 'item.*' is a magic placeholder value set by addItemsToScope
_, allowAllWorkflowOutputParameterRefs := scope[anyWorkflowOutputParameterMagicValue]
_, allowAllWorkflowOutputArtifactRefs := scope[anyWorkflowOutputArtifactMagicValue]
Expand All @@ -607,6 +607,8 @@ func resolveAllVariables(scope map[string]interface{}, globalParams map[string]s
} 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, "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 All @@ -632,12 +634,12 @@ func validateNonLeaf(tmpl *wfv1.Template) error {
return nil
}

func (ctx *templateValidationCtx) validateLeaf(scope map[string]interface{}, tmpl *wfv1.Template) error {
func (ctx *templateValidationCtx) validateLeaf(scope map[string]interface{}, tmpl *wfv1.Template, workflowTemplateValidation bool) error {
tmplBytes, err := json.Marshal(tmpl)
if err != nil {
return errors.InternalWrapError(err)
}
err = resolveAllVariables(scope, ctx.globalParams, string(tmplBytes))
err = resolveAllVariables(scope, ctx.globalParams, string(tmplBytes), workflowTemplateValidation)
if err != nil {
return errors.Errorf(errors.CodeBadRequest, "templates.%s: %s", tmpl.Name, err.Error())
}
Expand Down Expand Up @@ -790,7 +792,7 @@ func validateArgumentsValues(prefix string, arguments wfv1.Arguments, allowEmpty
return nil
}

func (ctx *templateValidationCtx) validateSteps(scope map[string]interface{}, tmplCtx *templateresolution.Context, tmpl *wfv1.Template) error {
func (ctx *templateValidationCtx) validateSteps(scope map[string]interface{}, tmplCtx *templateresolution.Context, tmpl *wfv1.Template, workflowTemplateValidation bool) error {
err := validateNonLeaf(tmpl)
if err != nil {
return err
Expand Down Expand Up @@ -820,7 +822,7 @@ func (ctx *templateValidationCtx) validateSteps(scope map[string]interface{}, tm
if err != nil {
return err
}
resolvedTmpl, err := ctx.validateTemplateHolder(&step, tmplCtx, &FakeArguments{})
resolvedTmpl, err := ctx.validateTemplateHolder(&step, tmplCtx, &FakeArguments{}, workflowTemplateValidation)
if err != nil {
return errors.Errorf(errors.CodeBadRequest, "templates.%s.steps[%d].%s %s", tmpl.Name, i, step.Name, err.Error())
}
Expand All @@ -839,7 +841,7 @@ func (ctx *templateValidationCtx) validateSteps(scope map[string]interface{}, tm
if err != nil {
return errors.InternalWrapError(err)
}
err = resolveAllVariables(scope, ctx.globalParams, string(stepBytes))
err = resolveAllVariables(scope, ctx.globalParams, string(stepBytes), workflowTemplateValidation)
if err != nil {
return errors.Errorf(errors.CodeBadRequest, "templates.%s.steps %s", tmpl.Name, err.Error())
}
Expand All @@ -850,7 +852,7 @@ func (ctx *templateValidationCtx) validateSteps(scope map[string]interface{}, tm
ctx.addOutputsToScope(resolvedTmpl, fmt.Sprintf("steps.%s", step.Name), scope, aggregate, false)

// Validate the template again with actual arguments.
_, err = ctx.validateTemplateHolder(&step, tmplCtx, &step.Arguments)
_, err = ctx.validateTemplateHolder(&step, tmplCtx, &step.Arguments, workflowTemplateValidation)
if err != nil {
return errors.Errorf(errors.CodeBadRequest, "templates.%s.steps[%d].%s %s", tmpl.Name, i, step.Name, err.Error())
}
Expand Down Expand Up @@ -959,7 +961,7 @@ func (ctx *templateValidationCtx) addOutputsToScope(tmpl *wfv1.Template, prefix
}
}

func validateOutputs(scope map[string]interface{}, globalParams map[string]string, tmpl *wfv1.Template) error {
func validateOutputs(scope map[string]interface{}, globalParams map[string]string, tmpl *wfv1.Template, workflowTemplateValidation bool) error {
err := validateWorkflowFieldNames(tmpl.Outputs.Parameters)
if err != nil {
return errors.Errorf(errors.CodeBadRequest, "templates.%s.outputs.parameters %s", tmpl.Name, err.Error())
Expand All @@ -972,7 +974,7 @@ func validateOutputs(scope map[string]interface{}, globalParams map[string]strin
if err != nil {
return errors.InternalWrapError(err)
}
err = resolveAllVariables(scope, globalParams, string(outputBytes))
err = resolveAllVariables(scope, globalParams, string(outputBytes), workflowTemplateValidation)
if err != nil {
return errors.Errorf(errors.CodeBadRequest, "templates.%s.outputs %s", tmpl.Name, err.Error())
}
Expand Down Expand Up @@ -1154,7 +1156,7 @@ func validateWhenExpression(when string) bool {
return !strings.HasPrefix(when, "{{=")
}

func (ctx *templateValidationCtx) validateDAG(scope map[string]interface{}, tmplCtx *templateresolution.Context, tmpl *wfv1.Template) error {
func (ctx *templateValidationCtx) validateDAG(scope map[string]interface{}, tmplCtx *templateresolution.Context, tmpl *wfv1.Template, workflowTemplateValidation bool) error {
err := validateNonLeaf(tmpl)
if err != nil {
return err
Expand Down Expand Up @@ -1206,7 +1208,7 @@ func (ctx *templateValidationCtx) validateDAG(scope map[string]interface{}, tmpl
return errors.Errorf(errors.CodeBadRequest, "templates.%s when doesn't support 'expr' expression '{{='. 'When' expression is only support govaluate format {{", tmpl.Name)
}

resolvedTmpl, err := ctx.validateTemplateHolder(&task, tmplCtx, &FakeArguments{})
resolvedTmpl, err := ctx.validateTemplateHolder(&task, tmplCtx, &FakeArguments{}, workflowTemplateValidation)
if err != nil {
return errors.Errorf(errors.CodeBadRequest, "templates.%s.tasks.%s %s", tmpl.Name, task.Name, err.Error())
}
Expand Down Expand Up @@ -1239,7 +1241,7 @@ func (ctx *templateValidationCtx) validateDAG(scope map[string]interface{}, tmpl
if err = verifyNoCycles(tmpl, dagValidationCtx); err != nil {
return err
}
err = resolveAllVariables(scope, ctx.globalParams, tmpl.DAG.Target)
err = resolveAllVariables(scope, ctx.globalParams, tmpl.DAG.Target, workflowTemplateValidation)
if err != nil {
return errors.Errorf(errors.CodeBadRequest, "templates.%s.targets %s", tmpl.Name, err.Error())
}
Expand Down Expand Up @@ -1285,7 +1287,7 @@ func (ctx *templateValidationCtx) validateDAG(scope map[string]interface{}, tmpl
if err != nil {
return errors.Errorf(errors.CodeBadRequest, "templates.%s.tasks.%s %s", tmpl.Name, task.Name, err.Error())
}
err = resolveAllVariables(taskScope, ctx.globalParams, string(taskBytes))
err = resolveAllVariables(taskScope, ctx.globalParams, string(taskBytes), workflowTemplateValidation)
if err != nil {
return errors.Errorf(errors.CodeBadRequest, "templates.%s.tasks.%s %s", tmpl.Name, task.Name, err.Error())
}
Expand All @@ -1298,7 +1300,7 @@ func (ctx *templateValidationCtx) validateDAG(scope map[string]interface{}, tmpl
return errors.Errorf(errors.CodeBadRequest, "templates.%s.tasks.%s %s", tmpl.Name, task.Name, err.Error())
}
// Validate the template again with actual arguments.
_, err = ctx.validateTemplateHolder(&task, tmplCtx, &task.Arguments)
_, err = ctx.validateTemplateHolder(&task, tmplCtx, &task.Arguments, workflowTemplateValidation)
if err != nil {
return errors.Errorf(errors.CodeBadRequest, "templates.%s.tasks.%s %s", tmpl.Name, task.Name, err.Error())
}
Expand Down
19 changes: 18 additions & 1 deletion workflow/validate/validate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1518,6 +1518,24 @@ func TestWorkflowTemplate(t *testing.T) {
assert.NoError(t, err)
}

var templateWithGlobalParams = `
apiVersion: argoproj.io/v1alpha1
kind: WorkflowTemplate
metadata:
name: template-ref-target
spec:
templates:
- name: A
container:
image: alpine:latest
command: [echo, "{{workflow.parameters.something}}"]
`

func TestWorkflowTemplateWithGlobalParams(t *testing.T) {
err := validateWorkflowTemplate(templateWithGlobalParams, ValidateOpts{})
assert.NoError(t, err)
}

var templateRefNestedTarget = `
apiVersion: argoproj.io/v1alpha1
kind: WorkflowTemplate
Expand Down Expand Up @@ -2493,7 +2511,6 @@ spec:
- echo
- '{{inputs.parameters.message}}'
`

var workflowTeamplateWithEnumValuesWithoutValue = `
apiVersion: argoproj.io/v1alpha1
kind: WorkflowTemplate
Expand Down

0 comments on commit f1bab89

Please sign in to comment.