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 #9752

Closed
wants to merge 67 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
67 commits
Select commit Hold shift + click to select a range
e703d0b
chore(deps): bump docker/login-action from 1 to 2 (#1)
dependabot[bot] May 9, 2022
559e049
chore(deps): bump docker/setup-buildx-action from 1 to 2 (#3)
dependabot[bot] May 9, 2022
21f02d2
chore(deps): bump docker/setup-qemu-action from 1 to 2 (#4)
dependabot[bot] May 9, 2022
a18d932
chore(deps): bump github.com/minio/minio-go/v7 from 7.0.24 to 7.0.26 …
dependabot[bot] May 9, 2022
f8a26d5
chore(deps): bump google.golang.org/api from 0.75.0 to 0.78.0 (#6)
dependabot[bot] May 9, 2022
baaa822
chore(deps): bump github.com/argoproj/pkg from 0.12.0 to 0.13.2 (#7)
dependabot[bot] May 9, 2022
34255e6
chore(deps): bump google.golang.org/api from 0.78.0 to 0.79.0 (#8)
dependabot[bot] May 10, 2022
a66ff32
chore(deps): bump github.com/coreos/go-oidc/v3 from 3.1.0 to 3.2.0 (#9)
dependabot[bot] May 11, 2022
a7dc5d3
chore(deps): bump github.com/aliyun/aliyun-oss-go-sdk (#10)
dependabot[bot] May 13, 2022
d83da66
chore(deps): bump github.com/prometheus/client_golang (#11)
dependabot[bot] May 13, 2022
25e6d08
chore(deps): bump google.golang.org/api from 0.79.0 to 0.80.0 (#12)
dependabot[bot] May 17, 2022
77d1d99
chore(deps): bump cloud.google.com/go/storage from 1.22.0 to 1.22.1 (…
dependabot[bot] May 19, 2022
2b45c8f
chore(deps): bump google.golang.org/api from 0.80.0 to 0.81.0 (#14)
dependabot[bot] May 24, 2022
d77801d
chore(deps): bump github.com/aliyun/aliyun-oss-go-sdk (#15)
dependabot[bot] May 25, 2022
93735ed
chore(deps): bump github.com/minio/minio-go/v7 from 7.0.26 to 7.0.27 …
dependabot[bot] May 26, 2022
755e7ce
chore(deps): bump github.com/spf13/viper from 1.11.0 to 1.12.0 (#17)
dependabot[bot] May 26, 2022
d3c2b07
chore(deps): bump google.golang.org/api from 0.81.0 to 0.82.0 (#18)
dependabot[bot] Jun 2, 2022
f4560b2
chore(deps): bump google.golang.org/api from 0.82.0 to 0.83.0 (#19)
dependabot[bot] Jun 7, 2022
cf48825
chore(deps): bump github.com/stretchr/testify from 1.7.1 to 1.7.2 (#20)
dependabot[bot] Jun 7, 2022
0a56094
chore(deps): bump actions/setup-python from 3 to 4 (#21)
dependabot[bot] Jun 8, 2022
bc897f2
chore(deps): bump github.com/minio/minio-go/v7 from 7.0.27 to 7.0.28 …
dependabot[bot] Jun 13, 2022
8b119db
chore(deps): bump google.golang.org/api from 0.83.0 to 0.84.0 (#23)
dependabot[bot] Jun 14, 2022
f271e36
chore(deps): bump github.com/argoproj/pkg from 0.13.2 to 0.13.3 (#24)
dependabot[bot] Jun 17, 2022
f95fb4c
chore(deps): bump github.com/prometheus/common from 0.34.0 to 0.35.0 …
dependabot[bot] Jun 20, 2022
1bc3dc1
chore(deps): bump github.com/minio/minio-go/v7 from 7.0.28 to 7.0.29 …
dependabot[bot] Jun 20, 2022
51a03f1
chore(deps): bump google.golang.org/api from 0.84.0 to 0.85.0 (#27)
dependabot[bot] Jun 21, 2022
4996c01
chore(deps): bump github.com/stretchr/testify from 1.7.2 to 1.7.4 (#29)
dependabot[bot] Jun 23, 2022
7d8e6d2
chore(deps): bump github.com/argoproj/pkg from 0.13.3 to 0.13.6 (#30)
dependabot[bot] Jun 23, 2022
5996ff7
chore(deps): bump cloud.google.com/go/storage from 1.22.1 to 1.23.0 (…
dependabot[bot] Jun 23, 2022
e6f88aa
chore(deps): bump github.com/stretchr/testify from 1.7.4 to 1.7.5 (#32)
dependabot[bot] Jun 24, 2022
d4fbeac
chore(deps): bump google.golang.org/api from 0.85.0 to 0.86.0 (#33)
dependabot[bot] Jun 28, 2022
0e1b600
chore(deps): bump github.com/minio/minio-go/v7 from 7.0.29 to 7.0.30 …
dependabot[bot] Jun 28, 2022
b095170
chore(deps): bump github.com/stretchr/testify from 1.7.5 to 1.8.0 (#36)
dependabot[bot] Jun 30, 2022
bd66a3a
chore(deps): bump dependabot/fetch-metadata from 1.3.1 to 1.3.3 (#37)
dependabot[bot] Jul 1, 2022
483ecff
chore(deps): bump github.com/minio/minio-go/v7 from 7.0.30 to 7.0.31 …
dependabot[bot] Jul 6, 2022
74a07d8
chore(deps): bump github.com/prometheus/common from 0.35.0 to 0.36.0 …
dependabot[bot] Jul 11, 2022
526e14b
chore(deps): bump google.golang.org/api from 0.86.0 to 0.87.0 (#40)
dependabot[bot] Jul 12, 2022
e84003a
chore(deps): bump github.com/prometheus/common from 0.36.0 to 0.37.0 …
dependabot[bot] Jul 14, 2022
9a6edfb
chore(deps): bump github.com/sirupsen/logrus from 1.8.1 to 1.9.0 (#42)
dependabot[bot] Jul 19, 2022
56b7dc0
chore(deps): bump google.golang.org/api from 0.87.0 to 0.88.0 (#43)
dependabot[bot] Jul 20, 2022
a47fe88
chore(deps): bump cloud.google.com/go/storage from 1.23.0 to 1.24.0 (…
dependabot[bot] Jul 20, 2022
1869eee
chore(deps): bump github.com/minio/minio-go/v7 from 7.0.31 to 7.0.32 …
dependabot[bot] Jul 25, 2022
a4d7b5d
chore(deps): bump google.golang.org/api from 0.88.0 to 0.89.0 (#46)
dependabot[bot] Jul 26, 2022
5424bd0
chore(deps): bump google.golang.org/api from 0.89.0 to 0.90.0 (#47)
dependabot[bot] Jul 29, 2022
bc524b4
chore(deps): bump github.com/minio/minio-go/v7 from 7.0.32 to 7.0.33 …
dependabot[bot] Aug 1, 2022
76195aa
chore(deps): bump google.golang.org/api from 0.90.0 to 0.91.0 (#49)
dependabot[bot] Aug 3, 2022
12f51c1
chore(deps): bump github.com/minio/minio-go/v7 from 7.0.33 to 7.0.34 …
dependabot[bot] Aug 3, 2022
f5e68a1
chore(deps): bump github.com/tidwall/gjson from 1.14.1 to 1.14.2 (#51)
dependabot[bot] Aug 5, 2022
f1ca8b3
chore(deps): bump github.com/prometheus/client_golang (#52)
dependabot[bot] Aug 8, 2022
c3b2276
chore(deps): bump google.golang.org/api from 0.91.0 to 0.92.0 (#53)
dependabot[bot] Aug 10, 2022
4f72f89
chore(deps): bump cloud.google.com/go/storage from 1.24.0 to 1.25.0 (…
dependabot[bot] Aug 11, 2022
0bbfd78
chore(deps): bump google.golang.org/api from 0.92.0 to 0.93.0 (#55)
dependabot[bot] Aug 16, 2022
680b877
chore(deps): bump github.com/argoproj-labs/argo-dataflow (#56)
dependabot[bot] Aug 16, 2022
d397eaf
chore(deps): bump github.com/tidwall/gjson from 1.14.2 to 1.14.3 (#57)
dependabot[bot] Aug 18, 2022
3b88b6f
chore(deps): bump github.com/aliyun/aliyun-oss-go-sdk (#58)
dependabot[bot] Aug 22, 2022
7a19a66
chore(deps): bump google.golang.org/api from 0.93.0 to 0.94.0 (#59)
dependabot[bot] Aug 23, 2022
6cfdc26
chore(deps): bump cloud.google.com/go/storage from 1.25.0 to 1.26.0 (…
dependabot[bot] Aug 29, 2022
88f2e1a
chore: put latest master of workflows onto my fork
juliev0 Sep 2, 2022
0cbcbfa
chore: update my fork with latest master
juliev0 Sep 22, 2022
d3c51c4
chore: updating to latest master
juliev0 Sep 30, 2022
c4d7ded
fix: remove error check for 'when' expressions using 'expr' and take …
juliev0 Oct 5, 2022
22a7451
chore: add test
juliev0 Oct 5, 2022
c32854b
fix: remove validation error for when expressions using expr
juliev0 Oct 5, 2022
493ccb0
chore: e2e
juliev0 Oct 5, 2022
9eef3c3
fix: empty commit
juliev0 Oct 5, 2022
1974e86
chore: comment
juliev0 Oct 6, 2022
8add8c9
chore: remove temporary print line
juliev0 Oct 6, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 21 additions & 15 deletions examples/conditionals.yaml
Original file line number Diff line number Diff line change
@@ -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:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed the dash from the name since expr expressions which include dash need to be formatted with brackets due to the special character

- - 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
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

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("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(`
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 @@ -822,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)
}
Expand Down Expand Up @@ -1147,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 {
Expand Down Expand Up @@ -1199,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())
Expand Down