Skip to content

Commit

Permalink
fix: enable when expressions to use expr; add new json variables to a…
Browse files Browse the repository at this point in the history
…void expr conflicts (#9761)

Signed-off-by: Julie Vogelman <[email protected]>
  • Loading branch information
juliev0 authored Oct 11, 2022
1 parent 0fc883a commit aa59b43
Show file tree
Hide file tree
Showing 7 changed files with 96 additions and 22 deletions.
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]
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 @@ -177,12 +177,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.ValueFrom != nil && param.ValueFrom.ConfigMapKeyRef != nil {
Expand Down Expand Up @@ -609,12 +610,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
16 changes: 4 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 @@ -827,10 +830,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 @@ -1152,10 +1151,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, workflowTemplateValidation bool) error {
err := validateNonLeaf(tmpl)
if err != nil {
Expand Down Expand Up @@ -1204,11 +1199,8 @@ 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{}, workflowTemplateValidation)

if err != nil {
return errors.Errorf(errors.CodeBadRequest, "templates.%s.tasks.%s %s", tmpl.Name, task.Name, err.Error())
}
Expand Down

0 comments on commit aa59b43

Please sign in to comment.