Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: enable when expressions to use expr; add new json variables to avoid expr conflicts #9761

Merged
merged 11 commits into from
Oct 11, 2022
5 changes: 4 additions & 1 deletion docs/variables.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.<NAME>` | 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.<NAME>` | Global parameter in the workflow |
| `workflow.outputs.artifacts.<NAME>` | Global artifact in the workflow |
| `workflow.annotations.<NAME>` | Workflow annotations |
| `workflow.annotations.json` | all Workflow annotations as a JSON string |
| `workflow.labels.<NAME>` | 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.<STRFTIMECHAR>` | Creation time-stamp formatted with a [`strftime`](http://strftime.org) format character. |
| `workflow.creationTimestamp.RFC3339` | Creation time-stamp formatted with in RFC 3339. |
Expand Down
18 changes: 12 additions & 6 deletions examples/conditionals.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,26 @@ spec:
arguments:
parameters:
- name: should-print
value: "false"
value: "true"

templates:
- name: conditional-example
inputs:
parameters:
- name: should-print
steps:
- - name: print-hello
template: whalesay
when: "{{inputs.parameters.should-print}} == true"
- - name: print-hello-govaluate
template: argosay
when: "{{inputs.parameters.should-print}} == true" # govaluate form
- name: print-hello-expr
template: argosay
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

- name: whalesay
- name: argosay
container:
image: docker/whalesay:latest
image: argoproj/argosay:v1
command: [sh, -c]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

had to change the whalesay image to argosay image since apparently whalesay isn't supposed to be used for e2e tests

args: ["cowsay hello"]
38 changes: 38 additions & 0 deletions test/e2e/functional_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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("print-hello-govaluate"), func(t *testing.T, n *wfv1.NodeStatus, p *apiv1.Pod) {
assert.NotEqual(t, wfv1.NodeTypeSkipped, n.Type)
}).
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("print-hello-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(`
Expand Down
26 changes: 26 additions & 0 deletions test/e2e/testdata/json-variables.yaml
Original file line number Diff line number Diff line change
@@ -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')}}"
12 changes: 9 additions & 3 deletions workflow/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
3 changes: 3 additions & 0 deletions workflow/controller/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
15 changes: 3 additions & 12 deletions workflow/validate/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -825,10 +828,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)
}
Expand Down Expand Up @@ -1150,10 +1149,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 {
Expand Down Expand Up @@ -1202,10 +1197,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())
Expand Down