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

Better support for govaluate expressions #4153

Closed
christophschnabl opened this issue Sep 29, 2020 · 7 comments
Closed

Better support for govaluate expressions #4153

christophschnabl opened this issue Sep 29, 2020 · 7 comments
Labels
area/templating Templating with `{{...}}` solution/workaround There's a workaround, might not be great, but exists type/feature Feature request

Comments

@christophschnabl
Copy link

christophschnabl commented Sep 29, 2020

Summary

I found out that Argo is using govaluate to parse when conditional expressions. I played around with it and turns out most of govaluate works out of the box. However, the regex comparison operator seems to be very flaky with all the yaml parsing (json mapping) etc stuff. I compiled a list of some expressions and their respective outcome, below

expression in yaml output
when: "a11 =~ a1\\d" when 'a11 =~ a1\d
when: "a11 =~ a1\\d+" Invalid 'when' expression 'a11 =~ a1\d+': Unexpected end of expression
when: "a11 =~ a1\\\\d" works
"a112 =~ a\\\\d{3}" 'Invalid 'when' expression 'a112 =~ a\d{3}': Invalid token: '{' (hint: try wrapping the affected expression in quotes ("))'`
when: "'a112 =~ a\\\\d{3}'" Expected boolean evaluation for ''a112 =~ a\\d{3}''. Got a112 =~ a\d{3}
when: "\"a112 =~ a\\\\d{3}\"" expected boolean evaluation for '"a112 =~ a\\d{3}"'. Got a112 =~ a\d{3}

Use Cases

When would you use this?

I would use that quite often to evaluate expression in a regex like style.


Message from the maintainers:

Impacted by this bug? Give it a 👍. We prioritise the issues with the most 👍.

@christophschnabl christophschnabl added the type/feature Feature request label Sep 29, 2020
@simster7
Copy link
Member

We use https://github.com/antonmedv/expr for evaluation in depends: logic and have been considering switching the when: logic to it as well. However, since the issue here seems to be related to the yaml itself, I'm not sure if making the switch would fix this particular issue. Maybe you can test that for us?

Meanwhile, as a workaround I can suggest you add a new container step right before the relevant when: step and perform the regex in said container. The container can then output true or false and then you can use its result on the field like so when: {{steps.regex.outputs.result}}. Does this make sense?

@simster7 simster7 added the solution/workaround There's a workaround, might not be great, but exists label Sep 29, 2020
@christophschnabl
Copy link
Author

Yeah, will test it and add my findings in this ticket.

Ad) workaround - yes this would be another option. However being able to use regexp would make the when statement ever so elegant and powerful.

@multinegsix
Copy link

multinegsix commented Apr 21, 2021

I found that if you wrap the left and right expressions with single quotes respectively, the error will go away. For example, for

when: "a11 =~ a1\\d+"

the working version would be

when: "'a11' =~ 'a1\\d+'"

@scravy
Copy link
Contributor

scravy commented Sep 29, 2021

I came here as I was confused as to whether govaluate or expr is used in when:

To me it is very confusing to find when: use one library, but depends: and {{# use another one.

@zxpower
Copy link

zxpower commented Oct 20, 2021

Looks like govaluate isn't doing it's job correctly...

Our use case:

  • we have DAG task that runs multiple commands and outputs something like this:
Activated service account credentials for: [[email protected]]
true
  • next task uses this output in it's when: clause like this:
when: "'{{tasks.taskname.outputs.result}}' =~ 'false'"

It was working fine while we used docker executor, but fails with invalid character 'A' looking for beginning of value when using pns or k8sapi executors which creates issues for us to migrate to newer version of GKE as v1.19.x is the last to support Docker.

Argo version: v3.1.13

@alexec
Copy link
Contributor

alexec commented Feb 7, 2022

We're moving away from govaluate. Closing issue.

@alexec alexec closed this as completed Feb 7, 2022
@alexec alexec added the area/templating Templating with `{{...}}` label Feb 7, 2022
@agilgur5
Copy link
Contributor

See #7831 and related issues for moving away from govaluate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/templating Templating with `{{...}}` solution/workaround There's a workaround, might not be great, but exists type/feature Feature request
Projects
None yet
Development

No branches or pull requests

7 participants