From b6e671318a446f129740ce790f53425d65e436f3 Mon Sep 17 00:00:00 2001 From: gaganapplatix <33636454+gaganapplatix@users.noreply.github.com> Date: Tue, 12 Dec 2017 11:07:42 -0800 Subject: [PATCH] Implementation for allowing access to global parameters in workflow (#571) * Implementation for allowing access to global parameters in workflow without need to define them in inputs or passing them around * Fix a debug change back to original form * Change error string as requested --- examples/global-parameters-complex.yaml | 49 ++++++++++++++++++++++ examples/global-parameters.yaml | 20 +++++++++ workflow/common/common.go | 3 ++ workflow/common/util.go | 38 +++++++++++++---- workflow/common/validate.go | 24 +++++++++-- workflow/common/validate_test.go | 54 +++++++++++++++++++++++++ workflow/controller/operator.go | 6 +-- 7 files changed, 180 insertions(+), 14 deletions(-) create mode 100644 examples/global-parameters-complex.yaml create mode 100644 examples/global-parameters.yaml diff --git a/examples/global-parameters-complex.yaml b/examples/global-parameters-complex.yaml new file mode 100644 index 000000000000..545ba400efc3 --- /dev/null +++ b/examples/global-parameters-complex.yaml @@ -0,0 +1,49 @@ +apiVersion: argoproj.io/v1alpha1 +kind: Workflow +metadata: + generateName: global-parameters-complex- +spec: + entrypoint: test-workflow + # Parameters can be passed/overridden via the argo CLI. + # To override the printed message, run `argo submit` with the -p option: + # $ argo submit examples/arguments-parameters.yaml -p message="goodbye world" + arguments: + parameters: + - name: message1 + value: hello world + - name: message2 + value: foo bar + + templates: + - name: test-workflow + inputs: + parameters: + - name: message1 + - name: message-internal + value: "{{workflow.parameters.message1}}" + steps: + - - name: step1 + template: whalesay2 + arguments: + parameters: + - name: message1 + value: world hello + - name: message2 + value: "{{inputs.parameters.message1}}" + - name: message3 + value: "{{workflow.parameters.message2}}" + - name: message4 + value: "{{inputs.parameters.message-internal}}" + + + - name: whalesay2 + inputs: + parameters: + - name: message1 + - name: message2 + - name: message3 + - name: message4 + container: + image: docker/whalesay:latest + command: [cowsay] + args: ["Global 1: {{workflow.parameters.message1}} Input 1: {{inputs.parameters.message1}} Input 2/Steps Input 1/Global 1: {{inputs.parameters.message2}} Input 3/Global 2: {{inputs.parameters.message3}} Input4/Steps Input 2 internal/Global 1: {{inputs.parameters.message4}}"] diff --git a/examples/global-parameters.yaml b/examples/global-parameters.yaml new file mode 100644 index 000000000000..179a61b7e35e --- /dev/null +++ b/examples/global-parameters.yaml @@ -0,0 +1,20 @@ +apiVersion: argoproj.io/v1alpha1 +kind: Workflow +metadata: + generateName: global-parameters- +spec: + entrypoint: whalesay1 + # Parameters can be passed/overridden via the argo CLI. + # To override the printed message, run `argo submit` with the -p option: + # $ argo submit examples/arguments-parameters.yaml -p message="goodbye world" + arguments: + parameters: + - name: message + value: hello world + + templates: + - name: whalesay1 + container: + image: docker/whalesay:latest + command: [cowsay] + args: ["{{workflow.parameters.message}}"] diff --git a/workflow/common/common.go b/workflow/common/common.go index f1754ff0a0d2..b3bcaafd0f36 100644 --- a/workflow/common/common.go +++ b/workflow/common/common.go @@ -22,6 +22,9 @@ const ( // Content encoding is expected to be YAML. WorkflowControllerConfigMapKey = "config" + // Workflow Global Parameter Reference Prefix string in yaml + WorkflowGlobalParameterPrefixString = "workflow.parameters." + // Container names used in the workflow pod MainContainerName = "main" InitContainerName = "init" diff --git a/workflow/common/util.go b/workflow/common/util.go index 5d59d12be81a..90e92798b774 100644 --- a/workflow/common/util.go +++ b/workflow/common/util.go @@ -115,7 +115,8 @@ func DefaultConfigMapName(controllerName string) string { // ProcessArgs sets in the inputs, the values either passed via arguments, or the hardwired values // It also substitutes parameters in the template from the arguments -func ProcessArgs(tmpl *wfv1.Template, args wfv1.Arguments, validateOnly bool) (*wfv1.Template, error) { +// It will also substitue Global Workflow Parameters referenced in template. +func ProcessArgs(tmpl *wfv1.Template, args wfv1.Arguments, wfGlobalParams []wfv1.Parameter, validateOnly bool) (*wfv1.Template, error) { // For each input parameter: // 1) check if was supplied as argument. if so use the supplied value from arg // 2) if not, use default value. @@ -137,7 +138,7 @@ func ProcessArgs(tmpl *wfv1.Template, args wfv1.Arguments, validateOnly bool) (* } tmpl.Inputs.Parameters[i] = inParam } - tmpl, err := substituteParams(tmpl) + tmpl, err := substituteParams(tmpl, wfGlobalParams) if err != nil { return nil, err } @@ -166,20 +167,40 @@ func ProcessArgs(tmpl *wfv1.Template, args wfv1.Arguments, validateOnly bool) (* } // substituteParams returns a new copy of the template with all input parameters substituted -func substituteParams(tmpl *wfv1.Template) (*wfv1.Template, error) { +func substituteParams(tmpl *wfv1.Template, wfGlobalParams []wfv1.Parameter) (*wfv1.Template, error) { tmplBytes, err := json.Marshal(tmpl) if err != nil { return nil, errors.InternalWrapError(err) } + globalReplaceMap := make(map[string]string) + for _, param := range wfGlobalParams { + if param.Value == nil { + return nil, errors.InternalErrorf("workflow.spec.arguments.parameters.%s had no value", param.Name) + } + globalReplaceMap[WorkflowGlobalParameterPrefixString+param.Name] = *param.Value + } + // First replace globals then replace inputs because globals could be referenced in the + // inputs. Note globals cannot be unresolved + fstTmpl := fasttemplate.New(string(tmplBytes), "{{", "}}") + globalReplacedTmplStr, err := Replace(fstTmpl, globalReplaceMap, false, WorkflowGlobalParameterPrefixString) + if err != nil { + return nil, err + } + var globalReplacedTmpl wfv1.Template + err = json.Unmarshal([]byte(globalReplacedTmplStr), &globalReplacedTmpl) + if err != nil { + return nil, errors.InternalWrapError(err) + } + // Now replace the rest of substitutions (the ones that can be made) in the template replaceMap := make(map[string]string) - for _, inParam := range tmpl.Inputs.Parameters { + for _, inParam := range globalReplacedTmpl.Inputs.Parameters { if inParam.Value == nil { return nil, errors.InternalErrorf("inputs.parameters.%s had no value", inParam.Name) } replaceMap["inputs.parameters."+inParam.Name] = *inParam.Value } - fstTmpl := fasttemplate.New(string(tmplBytes), "{{", "}}") - s, err := Replace(fstTmpl, replaceMap, true) + fstTmpl = fasttemplate.New(globalReplacedTmplStr, "{{", "}}") + s, err := Replace(fstTmpl, replaceMap, true, "") if err != nil { return nil, err } @@ -194,9 +215,12 @@ func substituteParams(tmpl *wfv1.Template) (*wfv1.Template, error) { // Replace executes basic string substitution of a template with replacement values. // allowUnresolved indicates whether or not it is acceptable to have unresolved variables // remaining in the substituted template. -func Replace(fstTmpl *fasttemplate.Template, replaceMap map[string]string, allowUnresolved bool) (string, error) { +func Replace(fstTmpl *fasttemplate.Template, replaceMap map[string]string, allowUnresolved bool, prefixFilter string) (string, error) { var unresolvedErr error replacedTmpl := fstTmpl.ExecuteFuncString(func(w io.Writer, tag string) (int, error) { + if !strings.HasPrefix(tag, prefixFilter) { + return w.Write([]byte(fmt.Sprintf("{{%s}}", tag))) + } replacement, ok := replaceMap[tag] if !ok { if allowUnresolved { diff --git a/workflow/common/validate.go b/workflow/common/validate.go index 5f9dac9932d0..7232a41bb131 100644 --- a/workflow/common/validate.go +++ b/workflow/common/validate.go @@ -39,10 +39,11 @@ func ValidateWorkflow(wf *wfv1.Workflow) error { if entryTmpl == nil { return errors.Errorf(errors.CodeBadRequest, "spec.entrypoint template '%s' undefined", ctx.wf.Spec.Entrypoint) } - return ctx.validateTemplate(entryTmpl, ctx.wf.Spec.Arguments) + + return ctx.validateTemplate(entryTmpl, ctx.wf.Spec.Arguments, ctx.wf.Spec.Arguments.Parameters) } -func (ctx *wfValidationCtx) validateTemplate(tmpl *wfv1.Template, args wfv1.Arguments) error { +func (ctx *wfValidationCtx) validateTemplate(tmpl *wfv1.Template, args wfv1.Arguments, wfGlobalParameters []wfv1.Parameter) error { _, ok := ctx.results[tmpl.Name] if ok { // we already processed this template @@ -52,7 +53,7 @@ func (ctx *wfValidationCtx) validateTemplate(tmpl *wfv1.Template, args wfv1.Argu errors.Errorf(errors.CodeBadRequest, "template names are required") } ctx.results[tmpl.Name] = validationResult{} - _, err := ProcessArgs(tmpl, args, true) + _, err := ProcessArgs(tmpl, args, wfGlobalParameters, true) if err != nil { return err } @@ -60,6 +61,10 @@ func (ctx *wfValidationCtx) validateTemplate(tmpl *wfv1.Template, args wfv1.Argu if err != nil { return err } + err = validateWFGlobalParams(tmpl, wfGlobalParameters, scope) + if err != nil { + return err + } if tmpl.Steps == nil { err = validateLeaf(scope, tmpl) } else { @@ -114,6 +119,17 @@ func validateInputs(tmpl *wfv1.Template) (map[string]interface{}, error) { return scope, nil } +func validateWFGlobalParams(tmpl *wfv1.Template, wfGlobalParams []wfv1.Parameter, scope map[string]interface{}) error { + err := VerifyUniqueNonEmptyNames(wfGlobalParams) + if err != nil { + return errors.Errorf(errors.CodeBadRequest, "Workflow spec.arguments.parameters%s", err.Error()) + } + for _, param := range wfGlobalParams { + scope[WorkflowGlobalParameterPrefixString+param.Name] = true + } + return nil +} + func validateArtifactLocation(errPrefix string, art wfv1.Artifact) error { if art.Git != nil { if art.Git.Repo == "" { @@ -185,7 +201,7 @@ func (ctx *wfValidationCtx) validateSteps(scope map[string]interface{}, tmpl *wf if childTmpl == nil { return errors.Errorf(errors.CodeBadRequest, "template '%s' steps[%d].%s.template '%s' undefined", tmpl.Name, i, step.Name, step.Template) } - err = ctx.validateTemplate(childTmpl, step.Arguments) + err = ctx.validateTemplate(childTmpl, step.Arguments, ctx.wf.Spec.Arguments.Parameters) if err != nil { return err } diff --git a/workflow/common/validate_test.go b/workflow/common/validate_test.go index 812e163cbad1..f0b83282b74e 100644 --- a/workflow/common/validate_test.go +++ b/workflow/common/validate_test.go @@ -184,3 +184,57 @@ func TestUnsatisfiedParam(t *testing.T) { assert.Contains(t, err.Error(), "not supplied") } } + +var globalParam = ` +apiVersion: argoproj.io/v1alpha1 +kind: Workflow +metadata: + generateName: global-parameters-complex- +spec: + entrypoint: test-workflow + arguments: + parameters: + - name: message1 + value: hello world + - name: message2 + value: foo bar + + templates: + - name: test-workflow + inputs: + parameters: + - name: message1 + - name: message-internal + value: "{{workflow.parameters.message1}}" + steps: + - - name: step1 + template: whalesay + arguments: + parameters: + - name: message1 + value: world hello + - name: message2 + value: "{{inputs.parameters.message1}}" + - name: message3 + value: "{{workflow.parameters.message2}}" + - name: message4 + value: "{{inputs.parameters.message-internal}}" + + + - name: whalesay + inputs: + parameters: + - name: message1 + - name: message2 + - name: message3 + - name: message4 + container: + image: docker/whalesay:latest + command: [cowsay] + args: ["Global 1: {{workflow.parameters.message1}} Input 1: {{inputs.parameters.message1}} Input 2/Steps Input 1/Global 1: {{inputs.parameters.message2}} Input 3/Global 2: {{inputs.parameters.message3}} Input4/Steps Input 2 internal/Global 1: {{inputs.parameters.message4}}"] +` + +func TestGlobalParam(t *testing.T) { + err := validate(globalParam) + assert.Nil(t, err) +} diff --git a/workflow/controller/operator.go b/workflow/controller/operator.go index 58b4cbc90841..92a55fa03126 100644 --- a/workflow/controller/operator.go +++ b/workflow/controller/operator.go @@ -590,7 +590,7 @@ func (woc *wfOperationCtx) executeTemplate(templateName string, args wfv1.Argume return err } - tmpl, err := common.ProcessArgs(tmpl, args, false) + tmpl, err := common.ProcessArgs(tmpl, args, woc.wf.Spec.Arguments.Parameters, false) if err != nil { woc.markNodeError(nodeName, err) return err @@ -918,7 +918,7 @@ func (woc *wfOperationCtx) resolveReferences(stepGroup []wfv1.WorkflowStep, scop } } fstTmpl := fasttemplate.New(string(stepBytes), "{{", "}}") - newStepStr, err := common.Replace(fstTmpl, replaceMap, true) + newStepStr, err := common.Replace(fstTmpl, replaceMap, true, "") if err != nil { return nil, err } @@ -1015,7 +1015,7 @@ func (woc *wfOperationCtx) expandStep(step wfv1.WorkflowStep) ([]wfv1.WorkflowSt default: return nil, errors.Errorf(errors.CodeBadRequest, "withItems[%d] expected string, number, or map. received: %s", i, val) } - newStepStr, err := common.Replace(fstTmpl, replaceMap, false) + newStepStr, err := common.Replace(fstTmpl, replaceMap, false, "") if err != nil { return nil, err }