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

WorkflowStep when condition behavior changes after v3.1 #6314

Closed
tczhao opened this issue Jul 9, 2021 · 3 comments
Closed

WorkflowStep when condition behavior changes after v3.1 #6314

tczhao opened this issue Jul 9, 2021 · 3 comments
Labels
solution/workaround There's a workaround, might not be great, but exists type/bug type/regression Regression from previous behavior (a specific type of bug)
Milestone

Comments

@tczhao
Copy link
Member

tczhao commented Jul 9, 2021

Summary

What happened/what you expected to happen?

With #5115, the when condition is now evaluated as an expr

The below template works prior to v3.1

apiVersion: argoproj.io/v1alpha1
kind: Workflow
metadata:
  generateName: conditional-
  namespace: default
spec:
  entrypoint: conditional-example
  arguments:
    parameters:
    - name: should-print
      value: "2021-01-01-test"

  templates:
  - name: conditional-example
    inputs:
      parameters:
      - name: should-print
    steps:
    - - name: print-hello
        template: whalesay
        when: "{{inputs.parameters.should-print}} != 2021-01-01"

  - name: whalesay
    container:
      image: docker/whalesay:latest
      command: [sh, -c]
      args: ["cowsay hello"]

However, with the expression update, if a value starts with number, it gets interpreted as numeric token

Currently, I put a single quote around to make it work.

when: "'{{inputs.parameters.should-print}}' != '2021-01-01'"

Maybe we need to document this or update the code to make it align with the old behavior?


Message from the maintainers:

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

@tczhao tczhao added the type/bug label Jul 9, 2021
@sarabala1979
Copy link
Member

Expression evaluation is refactored to support Goevaluate and expr v3.x onwards. It expects ' for string comparison.

@sarabala1979 sarabala1979 added this to the v3.1 milestone Jul 9, 2021
@alexec alexec added the type/regression Regression from previous behavior (a specific type of bug) label Jul 12, 2021
@sarabala1979 sarabala1979 added the solution/workaround There's a workaround, might not be great, but exists label Jul 13, 2021
@sarabala1979
Copy link
Member

@alexec I don't think we can fix this. fasttemplate will consider all left and right is a string. But after changed govaluate, it is typed strict evaluator. User has to wrap the ' if the value start with number or date.

@alexec alexec closed this as completed in 10c0fa5 Jul 22, 2021
@alexec
Copy link
Contributor

alexec commented Jul 22, 2021

I've documented this is the upgrading guide.

@alexec alexec reopened this Jul 22, 2021
uturunku1 pushed a commit to newrelic-forks/argo-workflows that referenced this issue Jul 22, 2021
Signed-off-by: Alex Collins <alex_collins@intuit.com>
Signed-off-by: uturunku1 <luces.huayhuaca@gmail.com>
@alexec alexec closed this as completed in 96b165e Jul 26, 2021
This was referenced Jul 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
solution/workaround There's a workaround, might not be great, but exists type/bug type/regression Regression from previous behavior (a specific type of bug)
Projects
None yet
Development

No branches or pull requests

3 participants