From 84cd418296a1aedbdc5ac7b2bc032cf957e077e9 Mon Sep 17 00:00:00 2001 From: Julie Vogelman Date: Thu, 6 Oct 2022 13:55:19 -0700 Subject: [PATCH 01/10] fix: enable when expressions to use expr; add new json variables to avoid expr conflicts Signed-off-by: Julie Vogelman --- examples/conditionals.yaml | 36 ++++++++++++++----------- test/e2e/functional_test.go | 38 +++++++++++++++++++++++++++ test/e2e/testdata/json-variables.yaml | 26 ++++++++++++++++++ workflow/common/common.go | 12 ++++++--- workflow/controller/operator.go | 3 +++ workflow/validate/validate.go | 18 +++---------- 6 files changed, 100 insertions(+), 33 deletions(-) create mode 100644 test/e2e/testdata/json-variables.yaml diff --git a/examples/conditionals.yaml b/examples/conditionals.yaml index 02bd44a9538c..9ace0538e691 100644 --- a/examples/conditionals.yaml +++ b/examples/conditionals.yaml @@ -1,35 +1,41 @@ # Conditionals provide a way to affect the control flow of a # workflow at runtime, depending on parameters. In this example -# the 'print-hello' template may or may not be executed depending -# on the input parameter, 'should-print'. When submitted with: +# the 'printHello' template may or may not be executed depending +# on the input parameter, 'shouldPrint'. When submitted with: # argo submit examples/conditionals.yaml -# the step will be skipped since 'should-print' will evaluate false. +# the step will be skipped since 'shouldPrint' will evaluate false. # When submitted with: -# argo submit examples/conditionals.yaml -p should-print=true -# the step will be executed since 'should-print' will evaluate true. +# argo submit examples/conditionals.yaml -p shouldPrint=true +# the step will be executed since 'shouldPrint' will evaluate true. apiVersion: argoproj.io/v1alpha1 kind: Workflow metadata: generateName: conditional- spec: - entrypoint: conditional-example + entrypoint: conditionalExample arguments: parameters: - - name: should-print - value: "false" + - name: shouldPrint + value: "true" templates: - - name: conditional-example + - name: conditionalExample inputs: parameters: - - name: should-print + - name: shouldPrint steps: - - - name: print-hello - template: whalesay - when: "{{inputs.parameters.should-print}} == true" + - - name: printHello-govaluate + template: argosay + when: "{{inputs.parameters.shouldPrint}} == true" # govaluate form + - name: printHello-expr + template: argosay + when: "{{= inputs.parameters.shouldPrint == 'true'}}" # expr form + - name: printHello-expr-json + template: argosay + when: "{{=jsonpath(workflow.parameters.json, '$[0].value') == 'true'}}" # expr form - - name: whalesay + - name: argosay container: - image: docker/whalesay:latest + image: argoproj/argosay:v1 command: [sh, -c] args: ["cowsay hello"] diff --git a/test/e2e/functional_test.go b/test/e2e/functional_test.go index defac6d78af7..35e8c5e89d8f 100644 --- a/test/e2e/functional_test.go +++ b/test/e2e/functional_test.go @@ -89,6 +89,44 @@ spec: }) } +func (s *FunctionalSuite) TestWhenExpressions() { + s.Given(). + Workflow("@functional/conditionals.yaml"). + When(). + SubmitWorkflow(). + WaitForWorkflow(fixtures.ToBeSucceeded, 2*time.Minute). + Then(). + ExpectWorkflowNode(wfv1.NodeWithDisplayName("printHello-govaluate"), func(t *testing.T, n *wfv1.NodeStatus, p *apiv1.Pod) { + assert.NotEqual(t, wfv1.NodeTypeSkipped, n.Type) + }). + ExpectWorkflowNode(wfv1.NodeWithDisplayName("printHello-expr"), func(t *testing.T, n *wfv1.NodeStatus, p *apiv1.Pod) { + assert.NotEqual(t, wfv1.NodeTypeSkipped, n.Type) + }). + ExpectWorkflowNode(wfv1.NodeWithDisplayName("printHello-expr-json"), func(t *testing.T, n *wfv1.NodeStatus, p *apiv1.Pod) { + assert.NotEqual(t, wfv1.NodeTypeSkipped, n.Type) + }) +} + +func (s *FunctionalSuite) TestJSONVariables() { + + s.Given(). + Workflow("@testdata/json-variables.yaml"). + When(). + SubmitWorkflow(). + WaitForWorkflow(). + Then(). + ExpectWorkflowNode(wfv1.SucceededPodNode, func(t *testing.T, n *wfv1.NodeStatus, p *apiv1.Pod) { + for _, c := range p.Spec.Containers { + if c.Name == "main" { + assert.Equal(t, 3, len(c.Args)) + assert.Equal(t, "myLabelValue", c.Args[0]) + assert.Equal(t, "myAnnotationValue", c.Args[1]) + assert.Equal(t, "myParamValue", c.Args[2]) + } + } + }) +} + func (s *FunctionalSuite) TestWorkflowTTL() { s.Given(). Workflow(` diff --git a/test/e2e/testdata/json-variables.yaml b/test/e2e/testdata/json-variables.yaml new file mode 100644 index 000000000000..1312aed84422 --- /dev/null +++ b/test/e2e/testdata/json-variables.yaml @@ -0,0 +1,26 @@ +apiVersion: argoproj.io/v1alpha1 +kind: Workflow + +metadata: + generateName: json-variables- + + labels: + myLabel: myLabelValue + annotations: + myAnnotation: myAnnotationValue +spec: + entrypoint: argosay1 + arguments: + parameters: + - name: myParam + value: myParamValue + + templates: + - name: argosay1 + container: + image: argoproj/argosay:v1 + command: [echo] + args: + - "{{=jsonpath(workflow.labels.json, '$.myLabel')}}" + - "{{=jsonpath(workflow.annotations.json, '$.myAnnotation')}}" + - "{{=jsonpath(workflow.parameters.json, '$[0].value')}}" \ No newline at end of file diff --git a/workflow/common/common.go b/workflow/common/common.go index 9647e162e2a2..f4c16ec5ad52 100644 --- a/workflow/common/common.go +++ b/workflow/common/common.go @@ -175,12 +175,18 @@ const ( GlobalVarWorkflowFailures = "workflow.failures" // GlobalVarWorkflowDuration is the current duration of this workflow GlobalVarWorkflowDuration = "workflow.duration" - // GlobalVarWorkflowAnnotations is a JSON string containing all workflow annotations + // GlobalVarWorkflowAnnotations is a JSON string containing all workflow annotations - which will be deprecated in favor of GlobalVarWorkflowAnnotationsJSON GlobalVarWorkflowAnnotations = "workflow.annotations" - // GlobalVarWorkflowLabels is a JSON string containing all workflow labels + // GlobalVarWorkflowAnnotationsJSON is a JSON string containing all workflow annotations + GlobalVarWorkflowAnnotationsJSON = "workflow.annotations.json" + // GlobalVarWorkflowLabels is a JSON string containing all workflow labels - which will be deprecated in favor of GlobalVarWorkflowLabelsJSON GlobalVarWorkflowLabels = "workflow.labels" - // GlobalVarWorkflowParameters is a JSON string containing all workflow parameters + // GlobalVarWorkflowLabelsJSON is a JSON string containing all workflow labels + GlobalVarWorkflowLabelsJSON = "workflow.labels.json" + // GlobalVarWorkflowParameters is a JSON string containing all workflow parameters - which will be deprecated in favor of GlobalVarWorkflowParametersJSON GlobalVarWorkflowParameters = "workflow.parameters" + // GlobalVarWorkflowParametersJSON is a JSON string containing all workflow parameters + GlobalVarWorkflowParametersJSON = "workflow.parameters.json" // GlobalVarWorkflowCronScheduleTime is the scheduled timestamp of a Workflow started by a CronWorkflow GlobalVarWorkflowCronScheduleTime = "workflow.scheduledTime" diff --git a/workflow/controller/operator.go b/workflow/controller/operator.go index f8722a46d118..d3d24276e047 100644 --- a/workflow/controller/operator.go +++ b/workflow/controller/operator.go @@ -570,6 +570,7 @@ func (woc *wfOperationCtx) setGlobalParameters(executionParameters wfv1.Argument if workflowParameters, err := json.Marshal(woc.execWf.Spec.Arguments.Parameters); err == nil { woc.globalParams[common.GlobalVarWorkflowParameters] = string(workflowParameters) + woc.globalParams[common.GlobalVarWorkflowParametersJSON] = string(workflowParameters) } for _, param := range executionParameters.Parameters { if param.Value == nil && param.ValueFrom == nil { @@ -605,12 +606,14 @@ func (woc *wfOperationCtx) setGlobalParameters(executionParameters wfv1.Argument if workflowAnnotations, err := json.Marshal(woc.wf.ObjectMeta.Annotations); err == nil { woc.globalParams[common.GlobalVarWorkflowAnnotations] = string(workflowAnnotations) + woc.globalParams[common.GlobalVarWorkflowAnnotationsJSON] = string(workflowAnnotations) } for k, v := range woc.wf.ObjectMeta.Annotations { woc.globalParams["workflow.annotations."+k] = v } if workflowLabels, err := json.Marshal(woc.wf.ObjectMeta.Labels); err == nil { woc.globalParams[common.GlobalVarWorkflowLabels] = string(workflowLabels) + woc.globalParams[common.GlobalVarWorkflowLabelsJSON] = string(workflowLabels) } for k, v := range woc.wf.ObjectMeta.Labels { // if the Label will get overridden by a LabelsFrom expression later, don't set it now diff --git a/workflow/validate/validate.go b/workflow/validate/validate.go index 6276e0806762..983272b111b7 100644 --- a/workflow/validate/validate.go +++ b/workflow/validate/validate.go @@ -161,6 +161,7 @@ func ValidateWorkflow(wftmplGetter templateresolution.WorkflowTemplateNamespaced } if len(wfArgs.Parameters) > 0 { ctx.globalParams[common.GlobalVarWorkflowParameters] = placeholderGenerator.NextPlaceholder() + ctx.globalParams[common.GlobalVarWorkflowParametersJSON] = placeholderGenerator.NextPlaceholder() } for _, param := range wfArgs.Parameters { @@ -190,11 +191,13 @@ func ValidateWorkflow(wftmplGetter templateresolution.WorkflowTemplateNamespaced ctx.globalParams["workflow.annotations."+k] = placeholderGenerator.NextPlaceholder() } ctx.globalParams[common.GlobalVarWorkflowAnnotations] = placeholderGenerator.NextPlaceholder() + ctx.globalParams[common.GlobalVarWorkflowAnnotationsJSON] = placeholderGenerator.NextPlaceholder() for k := range mergedLabels { ctx.globalParams["workflow.labels."+k] = placeholderGenerator.NextPlaceholder() } ctx.globalParams[common.GlobalVarWorkflowLabels] = placeholderGenerator.NextPlaceholder() + ctx.globalParams[common.GlobalVarWorkflowLabelsJSON] = placeholderGenerator.NextPlaceholder() if wf.Spec.Priority != nil { ctx.globalParams[common.GlobalVarWorkflowPriority] = strconv.Itoa(int(*wf.Spec.Priority)) @@ -764,9 +767,6 @@ func validateArgumentsValues(prefix string, arguments wfv1.Arguments, allowEmpty if len(param.Enum) == 0 { return errors.Errorf(errors.CodeBadRequest, "%s%s.enum should contain at least one value", prefix, param.Name) } - if param.Value == nil { - return errors.Errorf(errors.CodeBadRequest, "%s%s.value is required", prefix, param.Name) - } valueSpecifiedInEnumList := false for _, enum := range param.Enum { if enum == *param.Value { @@ -825,10 +825,6 @@ func (ctx *templateValidationCtx) validateSteps(scope map[string]interface{}, tm return errors.Errorf(errors.CodeBadRequest, "templates.%s.steps[%d].%s %s", tmpl.Name, i, step.Name, err.Error()) } - if !validateWhenExpression(step.When) { - return errors.Errorf(errors.CodeBadRequest, "templates.%s when expression doesn't support 'expr' format '{{='. 'When' expression is only support govaluate format {{", tmpl.Name) - } - if step.HasExitHook() { ctx.addOutputsToScope(resolvedTmpl, fmt.Sprintf("steps.%s", step.Name), scope, false, false) } @@ -1150,10 +1146,6 @@ func (d *dagValidationContext) GetTaskFinishedAtTime(taskName string) time.Time return time.Now() } -func validateWhenExpression(when string) bool { - return !strings.HasPrefix(when, "{{=") -} - func (ctx *templateValidationCtx) validateDAG(scope map[string]interface{}, tmplCtx *templateresolution.Context, tmpl *wfv1.Template) error { err := validateNonLeaf(tmpl) if err != nil { @@ -1202,10 +1194,6 @@ func (ctx *templateValidationCtx) validateDAG(scope map[string]interface{}, tmpl return errors.Errorf(errors.CodeBadRequest, "templates.%s cannot use 'continueOn' when using 'depends'. Instead use 'dep-task.Failed'/'dep-task.Errored'", tmpl.Name) } - if !validateWhenExpression(task.When) { - 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{}) if err != nil { return errors.Errorf(errors.CodeBadRequest, "templates.%s.tasks.%s %s", tmpl.Name, task.Name, err.Error()) From 2ddc6b2ba73e960d16c33e7a4614cd096f1fb951 Mon Sep 17 00:00:00 2001 From: Julie Vogelman Date: Thu, 6 Oct 2022 14:13:59 -0700 Subject: [PATCH 02/10] fix: empty commit Signed-off-by: Julie Vogelman From 8258a153bac33b290efe8f7f63b5e47b81090e08 Mon Sep 17 00:00:00 2001 From: Julie Vogelman Date: Thu, 6 Oct 2022 14:40:31 -0700 Subject: [PATCH 03/10] fix: accidental removal of code Signed-off-by: Julie Vogelman --- workflow/validate/validate.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/workflow/validate/validate.go b/workflow/validate/validate.go index 983272b111b7..32facd79a958 100644 --- a/workflow/validate/validate.go +++ b/workflow/validate/validate.go @@ -767,6 +767,9 @@ func validateArgumentsValues(prefix string, arguments wfv1.Arguments, allowEmpty if len(param.Enum) == 0 { return errors.Errorf(errors.CodeBadRequest, "%s%s.enum should contain at least one value", prefix, param.Name) } + if param.Value == nil { + return errors.Errorf(errors.CodeBadRequest, "%s%s.value is required", prefix, param.Name) + } valueSpecifiedInEnumList := false for _, enum := range param.Enum { if enum == *param.Value { From 719bfbcbbe71a47f236eb8ca9da6667a2f6f2e2d Mon Sep 17 00:00:00 2001 From: Julie Vogelman Date: Fri, 7 Oct 2022 10:19:45 -0700 Subject: [PATCH 04/10] fix: show example which requires the use of dashes Signed-off-by: Julie Vogelman --- examples/conditionals.yaml | 28 ++++++++++++++-------------- test/e2e/functional_test.go | 6 +++--- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/examples/conditionals.yaml b/examples/conditionals.yaml index 9ace0538e691..e203db601a5a 100644 --- a/examples/conditionals.yaml +++ b/examples/conditionals.yaml @@ -1,36 +1,36 @@ # Conditionals provide a way to affect the control flow of a # workflow at runtime, depending on parameters. In this example -# the 'printHello' template may or may not be executed depending -# on the input parameter, 'shouldPrint'. When submitted with: +# the 'print-hello' template may or may not be executed depending +# on the input parameter, 'should-print'. When submitted with: # argo submit examples/conditionals.yaml -# the step will be skipped since 'shouldPrint' will evaluate false. +# the step will be skipped since 'should-print' will evaluate false. # When submitted with: -# argo submit examples/conditionals.yaml -p shouldPrint=true -# the step will be executed since 'shouldPrint' will evaluate true. +# argo submit examples/conditionals.yaml -p should-print=true +# the step will be executed since 'should-print' will evaluate true. apiVersion: argoproj.io/v1alpha1 kind: Workflow metadata: generateName: conditional- spec: - entrypoint: conditionalExample + entrypoint: conditional-example arguments: parameters: - - name: shouldPrint + - name: should-print value: "true" templates: - - name: conditionalExample + - name: conditional-example inputs: parameters: - - name: shouldPrint + - name: should-print steps: - - - name: printHello-govaluate + - - name: print-hello-govaluate template: argosay - when: "{{inputs.parameters.shouldPrint}} == true" # govaluate form - - name: printHello-expr + when: "{{inputs.parameters.should-print}} == true" # govaluate form + - name: print-hello-expr template: argosay - when: "{{= inputs.parameters.shouldPrint == 'true'}}" # expr form - - name: printHello-expr-json + when: "{{= inputs.parameters[\"should-print\"] == 'true'}}" # expr form; note: a dash in the name requires the use of brackets and quotes for expr to handle + - name: print-hello-expr-json template: argosay when: "{{=jsonpath(workflow.parameters.json, '$[0].value') == 'true'}}" # expr form diff --git a/test/e2e/functional_test.go b/test/e2e/functional_test.go index 35e8c5e89d8f..e2fb62231d43 100644 --- a/test/e2e/functional_test.go +++ b/test/e2e/functional_test.go @@ -96,13 +96,13 @@ func (s *FunctionalSuite) TestWhenExpressions() { SubmitWorkflow(). WaitForWorkflow(fixtures.ToBeSucceeded, 2*time.Minute). Then(). - ExpectWorkflowNode(wfv1.NodeWithDisplayName("printHello-govaluate"), func(t *testing.T, n *wfv1.NodeStatus, p *apiv1.Pod) { + ExpectWorkflowNode(wfv1.NodeWithDisplayName("print-hello-govaluate"), func(t *testing.T, n *wfv1.NodeStatus, p *apiv1.Pod) { assert.NotEqual(t, wfv1.NodeTypeSkipped, n.Type) }). - ExpectWorkflowNode(wfv1.NodeWithDisplayName("printHello-expr"), func(t *testing.T, n *wfv1.NodeStatus, p *apiv1.Pod) { + ExpectWorkflowNode(wfv1.NodeWithDisplayName("print-hello-expr"), func(t *testing.T, n *wfv1.NodeStatus, p *apiv1.Pod) { assert.NotEqual(t, wfv1.NodeTypeSkipped, n.Type) }). - ExpectWorkflowNode(wfv1.NodeWithDisplayName("printHello-expr-json"), func(t *testing.T, n *wfv1.NodeStatus, p *apiv1.Pod) { + ExpectWorkflowNode(wfv1.NodeWithDisplayName("print-hello-expr-json"), func(t *testing.T, n *wfv1.NodeStatus, p *apiv1.Pod) { assert.NotEqual(t, wfv1.NodeTypeSkipped, n.Type) }) } From ec1bc670a6e7e0c0ae0d88d2df40197471f1a5dd Mon Sep 17 00:00:00 2001 From: Julie Vogelman Date: Fri, 7 Oct 2022 18:27:05 -0700 Subject: [PATCH 05/10] docs: new variables exposed in the documentation Signed-off-by: Julie Vogelman --- docs/variables.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/docs/variables.md b/docs/variables.md index da5892488aa8..f1234e0faff0 100644 --- a/docs/variables.md +++ b/docs/variables.md @@ -246,11 +246,14 @@ For `Template`-level metrics: | `workflow.serviceAccountName` | Workflow service account name | | `workflow.uid` | Workflow UID. Useful for setting ownership reference to a resource, or a unique artifact location | | `workflow.parameters.` | Input parameter to the workflow | -| `workflow.parameters` | All input parameters to the workflow as a JSON string | +| `workflow.parameters` | All input parameters to the workflow as a JSON string (this is deprecated in favor of `workflow.parameters.json` as this doesn't work with expression tags and that does) | +| `workflow.parameters.json` | All input parameters to the workflow as a JSON string | | `workflow.outputs.parameters.` | Global parameter in the workflow | | `workflow.outputs.artifacts.` | Global artifact in the workflow | | `workflow.annotations.` | Workflow annotations | +| `workflow.annotations.json` | all Workflow annotations as a JSON string | | `workflow.labels.` | Workflow labels | +| `workflow.labels.json` | all Workflow labels as a JSON string | | `workflow.creationTimestamp` | Workflow creation time-stamp formatted in RFC 3339 (e.g. `2018-08-23T05:42:49Z`) | | `workflow.creationTimestamp.` | Creation time-stamp formatted with a [`strftime`](http://strftime.org) format character. | | `workflow.creationTimestamp.RFC3339` | Creation time-stamp formatted with in RFC 3339. | From 8fed36e0968ba944257774eac805cc248cf0c98d Mon Sep 17 00:00:00 2001 From: Julie Vogelman Date: Fri, 7 Oct 2022 18:45:22 -0700 Subject: [PATCH 06/10] fix: empty commit Signed-off-by: Julie Vogelman From 8d9515f5ea1601c8d0f6450b280ac39c2cafa8bd Mon Sep 17 00:00:00 2001 From: Julie Vogelman Date: Sat, 8 Oct 2022 11:47:11 -0700 Subject: [PATCH 07/10] fix: empty commit Signed-off-by: Julie Vogelman From 9f8978e3e2c387dbab058a8c8703686088a381d6 Mon Sep 17 00:00:00 2001 From: Julie Vogelman Date: Tue, 11 Oct 2022 11:46:17 -0700 Subject: [PATCH 08/10] fix: empty commit Signed-off-by: Julie Vogelman From 4e1a9484c8032c4d62f5d47bf440851badf9696b Mon Sep 17 00:00:00 2001 From: Julie Vogelman Date: Tue, 11 Oct 2022 11:48:10 -0700 Subject: [PATCH 09/10] fix: merge conflict Signed-off-by: Julie Vogelman --- workflow/validate/validate.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/workflow/validate/validate.go b/workflow/validate/validate.go index 9eb5e38bff3f..f525339970c0 100644 --- a/workflow/validate/validate.go +++ b/workflow/validate/validate.go @@ -1151,15 +1151,7 @@ func (d *dagValidationContext) GetTaskFinishedAtTime(taskName string) time.Time return time.Now() } -<<<<<<< HEAD -func (ctx *templateValidationCtx) validateDAG(scope map[string]interface{}, tmplCtx *templateresolution.Context, tmpl *wfv1.Template) error { -======= -func validateWhenExpression(when string) bool { - return !strings.HasPrefix(when, "{{=") -} - func (ctx *templateValidationCtx) validateDAG(scope map[string]interface{}, tmplCtx *templateresolution.Context, tmpl *wfv1.Template, workflowTemplateValidation bool) error { ->>>>>>> upstream/master err := validateNonLeaf(tmpl) if err != nil { return err From b9ebb06ace66e40e25dc2f1752a4e87bc01710a1 Mon Sep 17 00:00:00 2001 From: Julie Vogelman Date: Tue, 11 Oct 2022 12:00:08 -0700 Subject: [PATCH 10/10] fix: empty commit Signed-off-by: Julie Vogelman