From 2f7a3456d3b81bde6c8e120075c33f4562136f69 Mon Sep 17 00:00:00 2001 From: Saravanan Balasubramanian Date: Thu, 22 Jul 2021 22:34:07 -0700 Subject: [PATCH 1/6] fix(controller): Randomaly expr expression fail to resolve Signed-off-by: Saravanan Balasubramanian --- workflow/common/common.go | 2 +- workflow/controller/operator_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/workflow/common/common.go b/workflow/common/common.go index 4587fca89a77..369e2b0437ab 100644 --- a/workflow/common/common.go +++ b/workflow/common/common.go @@ -139,7 +139,7 @@ const ( // GlobalVarWorkflowDuration is the current duration of this workflow GlobalVarWorkflowDuration = "workflow.duration" // GlobalVarWorkflowParameters is a JSON string containing all workflow parameters - GlobalVarWorkflowParameters = "workflow.parameters" + GlobalVarWorkflowParameters = "workflow.parameters.json" // GlobalVarWorkflowCronScheduleTime is the scheduled timestamp of a Workflow started by a CronWorkflow GlobalVarWorkflowCronScheduleTime = "workflow.scheduledTime" diff --git a/workflow/controller/operator_test.go b/workflow/controller/operator_test.go index 43010e234402..0ad514fd2c28 100644 --- a/workflow/controller/operator_test.go +++ b/workflow/controller/operator_test.go @@ -1975,7 +1975,7 @@ spec: arguments: parameters: - name: json - value: "Workflow: {{workflow.parameters}}. Template: {{inputs.parameters}}" + value: "Workflow: {{workflow.parameters.json}}. Template: {{inputs.parameters}}" - name: whalesay inputs: From 98e1bdef5317175aa1634b6a525b05dcf69888f6 Mon Sep 17 00:00:00 2001 From: Saravanan Balasubramanian Date: Fri, 23 Jul 2021 09:11:10 -0700 Subject: [PATCH 2/6] fixed test Signed-off-by: Saravanan Balasubramanian --- docs/variables.md | 2 +- workflow/controller/operator_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/variables.md b/docs/variables.md index f42fdf51b626..e48600cefe52 100644 --- a/docs/variables.md +++ b/docs/variables.md @@ -228,7 +228,7 @@ 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.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 | diff --git a/workflow/controller/operator_test.go b/workflow/controller/operator_test.go index 0ad514fd2c28..09ef1fc993e7 100644 --- a/workflow/controller/operator_test.go +++ b/workflow/controller/operator_test.go @@ -318,7 +318,7 @@ func TestGlobalParams(t *testing.T) { assert.Contains(t, woc.globalParams, "workflow.duration") assert.Contains(t, woc.globalParams, "workflow.name") assert.Contains(t, woc.globalParams, "workflow.namespace") - assert.Contains(t, woc.globalParams, "workflow.parameters") + assert.Contains(t, woc.globalParams, "workflow.parameters.json") assert.Contains(t, woc.globalParams, "workflow.serviceAccountName") assert.Contains(t, woc.globalParams, "workflow.uid") From 7669578bb21595a023b0e39da81fd823aafe8fe4 Mon Sep 17 00:00:00 2001 From: Saravanan Balasubramanian Date: Sun, 25 Jul 2021 11:48:17 -0700 Subject: [PATCH 3/6] refactor fix Signed-off-by: Saravanan Balasubramanian --- docs/variables.md | 2 +- workflow/common/common.go | 2 +- workflow/validate/validate.go | 13 +++++++++++++ 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/docs/variables.md b/docs/variables.md index e48600cefe52..f42fdf51b626 100644 --- a/docs/variables.md +++ b/docs/variables.md @@ -228,7 +228,7 @@ 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.json` | All input parameters to the workflow as a JSON string | +| `workflow.parameters` | 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 | diff --git a/workflow/common/common.go b/workflow/common/common.go index 369e2b0437ab..4587fca89a77 100644 --- a/workflow/common/common.go +++ b/workflow/common/common.go @@ -139,7 +139,7 @@ const ( // GlobalVarWorkflowDuration is the current duration of this workflow GlobalVarWorkflowDuration = "workflow.duration" // GlobalVarWorkflowParameters is a JSON string containing all workflow parameters - GlobalVarWorkflowParameters = "workflow.parameters.json" + GlobalVarWorkflowParameters = "workflow.parameters" // GlobalVarWorkflowCronScheduleTime is the scheduled timestamp of a Workflow started by a CronWorkflow GlobalVarWorkflowCronScheduleTime = "workflow.scheduledTime" diff --git a/workflow/validate/validate.go b/workflow/validate/validate.go index f6641c2f0f10..5079a00f947a 100644 --- a/workflow/validate/validate.go +++ b/workflow/validate/validate.go @@ -768,6 +768,11 @@ func (ctx *templateValidationCtx) validateSteps(scope map[string]interface{}, tm if err != nil { 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) } @@ -1126,6 +1131,10 @@ 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 { @@ -1174,6 +1183,10 @@ 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 db57714f2183522309ad285d0062941719b94310 Mon Sep 17 00:00:00 2001 From: Saravanan Balasubramanian Date: Mon, 26 Jul 2021 09:36:39 -0700 Subject: [PATCH 4/6] fixed test Signed-off-by: Saravanan Balasubramanian --- workflow/validate/validate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/workflow/validate/validate.go b/workflow/validate/validate.go index 5079a00f947a..e1774d208cae 100644 --- a/workflow/validate/validate.go +++ b/workflow/validate/validate.go @@ -1132,7 +1132,7 @@ func (d *dagValidationContext) GetTaskFinishedAtTime(taskName string) time.Time } func validateWhenExpression(when string) bool { - return strings.HasPrefix(when, "{{=") + return !strings.HasPrefix(when, "{{=") } func (ctx *templateValidationCtx) validateDAG(scope map[string]interface{}, tmplCtx *templateresolution.Context, tmpl *wfv1.Template) error { From c2c4b056def03ffd9738bb2188b8ffb094c4012b Mon Sep 17 00:00:00 2001 From: Saravanan Balasubramanian Date: Mon, 26 Jul 2021 10:04:25 -0700 Subject: [PATCH 5/6] updated Signed-off-by: Saravanan Balasubramanian --- workflow/controller/operator_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/workflow/controller/operator_test.go b/workflow/controller/operator_test.go index 09ef1fc993e7..0ad514fd2c28 100644 --- a/workflow/controller/operator_test.go +++ b/workflow/controller/operator_test.go @@ -318,7 +318,7 @@ func TestGlobalParams(t *testing.T) { assert.Contains(t, woc.globalParams, "workflow.duration") assert.Contains(t, woc.globalParams, "workflow.name") assert.Contains(t, woc.globalParams, "workflow.namespace") - assert.Contains(t, woc.globalParams, "workflow.parameters.json") + assert.Contains(t, woc.globalParams, "workflow.parameters") assert.Contains(t, woc.globalParams, "workflow.serviceAccountName") assert.Contains(t, woc.globalParams, "workflow.uid") From 5005db4dbb4d078a08a563a6263cb008f7978e29 Mon Sep 17 00:00:00 2001 From: Saravanan Balasubramanian Date: Mon, 26 Jul 2021 10:30:24 -0700 Subject: [PATCH 6/6] update Signed-off-by: Saravanan Balasubramanian --- workflow/controller/operator_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/workflow/controller/operator_test.go b/workflow/controller/operator_test.go index 0ad514fd2c28..43010e234402 100644 --- a/workflow/controller/operator_test.go +++ b/workflow/controller/operator_test.go @@ -1975,7 +1975,7 @@ spec: arguments: parameters: - name: json - value: "Workflow: {{workflow.parameters.json}}. Template: {{inputs.parameters}}" + value: "Workflow: {{workflow.parameters}}. Template: {{inputs.parameters}}" - name: whalesay inputs: