sprig expression that works in 3.3.9 doesn't work in 3.4.0

[tooptoop4]

Pre-requisites

• I have double-checked my configuration
• I can confirm the issues exists when I tested with :latest
• I'd like to contribute the fix myself (see contributing guide)

What happened/what you expected to happen?

a param to my workflow is a payload like the json in these 12 lines:
https://github.com/LeidosLabs/holeshot/blob/b3aa04a84059e3a4c07d42ff9a46cc37f443eb3d/ingest-service/src/test/resources/sample-notification.json#L1-L12

my expression below worked fine in 3.3.9 but in 3.4.0 gives error

                workflowMetadata:
                  labelsFrom:
                    s3-folder:
                      expression: "{{= sprig.quote(sprig.regexReplaceAll('^[-_.]*',sprig.regexReplaceAll('[-_.]*$',sprig.substr(0,63,sprig.regexReplaceAll('[^-A-Za-z0-9_.]',sprig.regexReplaceAll('/([^/]*)$',sprig.regexFind('landing/([^,]+),', workflow.parameters.message),''),'')),''),'')) }}"

Error: failed to evaluate label "s3-folder" expression "{{= sprig.quote(sprig.regexReplaceAll('^[-_.]*',sprig.regexReplaceAll('[-_.]*$',sprig.substr(0,63,sprig.regexReplaceAll('[^-A-Za-z0-9_.]',sprig.regexReplaceAll('/([^/]*)$',sprig.regexFind('landing/([^,]+),', workflow.parameters.message),''),'')),''),'')) }}": a map key must be a quoted string, a number, a identifier, or an expression enclosed in parentheses (unexpected token Bracket("{")) (1:2) | {{= sprig.quote(sprig.regexReplaceAll('^[-_.]*',sprig.regexReplaceAll('[-_.]*$',sprig.substr(0,63,sprig.regexReplaceAll('[^-A-Za-z0-9_.]',sprig.regexReplaceAll('/([^/]*)$',sprig.regexFind('landing/([^,]+),', workflow.parameters.message),''),'')),''),'')) }} | .^

Version

3.4.0

Paste a small workflow that reproduces the issue. We must be able to run the workflow; don't enter a workflows that uses private images.

n/a

Logs from the workflow controller

kubectl logs -n argo deploy/workflow-controller | grep ${workflow}

time="2022-09-20T00:57:22.742Z" level=info msg="Processing workflow" namespace=auth workflow=redacted
time="2022-09-20T00:57:22.749Z" level=info msg="Updated phase  -> Error" namespace=auth workflow=redacted
time="2022-09-20T00:57:22.749Z" level=info msg="Updated message  -> failed to evaluate label \"s3-folder\" expression \"{{= sprig.quote(sprig.regexReplaceAll('^[-_.]*',sprig.regexReplaceAll('[-_.]*$',sprig.substr(0,63,sprig.regexReplaceAll('[^-A-Za-z0-9_.]',sprig.regexReplaceAll('/([^/]*)$',sprig.regexFind('landing/([^,]+),', workflow.parameters.message),''),'')),''),'')) }}\": a map key must be a quoted string, a number, a identifier, or an expression enclosed in parentheses (unexpected token Bracket(\"{\")) (1:2)\n | {{= sprig.quote(sprig.regexReplaceAll('^[-_.]*',sprig.regexReplaceAll('[-_.]*$',sprig.substr(0,63,sprig.regexReplaceAll('[^-A-Za-z0-9_.]',sprig.regexReplaceAll('/([^/]*)$',sprig.regexFind('landing/([^,]+),', workflow.parameters.message),''),'')),''),'')) }}\n | .^" namespace=auth workflow=redacted
time="2022-09-20T00:57:22.749Z" level=info msg="Marking workflow completed" namespace=auth workflow=redacted
time="2022-09-20T00:57:22.749Z" level=info msg="Marking workflow as pending archiving" namespace=auth workflow=redacted
time="2022-09-20T00:57:22.749Z" level=error msg="Unable to set ExecWorkflow" error="failed to evaluate label \"s3-folder\" expression \"{{= sprig.quote(sprig.regexReplaceAll('^[-_.]*',sprig.regexReplaceAll('[-_.]*$',sprig.substr(0,63,sprig.regexReplaceAll('[^-A-Za-z0-9_.]',sprig.regexReplaceAll('/([^/]*)$',sprig.regexFind('landing/([^,]+),', workflow.parameters.message),''),'')),''),'')) }}\": a map key must be a quoted string, a number, a identifier, or an expression enclosed in parentheses (unexpected token Bracket(\"{\")) (1:2)\n | {{= sprig.quote(sprig.regexReplaceAll('^[-_.]*',sprig.regexReplaceAll('[-_.]*$',sprig.substr(0,63,sprig.regexReplaceAll('[^-A-Za-z0-9_.]',sprig.regexReplaceAll('/([^/]*)$',sprig.regexFind('landing/([^,]+),', workflow.parameters.message),''),'')),''),'')) }}\n | .^" namespace=auth workflow=redacted
time="2022-09-20T00:57:22.749Z" level=info msg="Checking daemoned children of " namespace=auth workflow=redacted
time="2022-09-20T00:57:22.749Z" level=info msg="Workflow to be dehydrated" Workflow Size=44708

Logs from in your workflow's wait container

kubectl logs -c wait -l workflows.argoproj.io/workflow=${workflow},workflow.argoproj.io/phase!=Succeeded

[tooptoop4]

https://github.com/argoproj/argo-workflows/blame/v3.4.0/workflow/controller/operator.go#L505 is the error being thrown

i wonder if https://github.com/argoproj/argo-workflows/pull/8907/files#diff-63ebd7c535e623fd05481826190cb78c1c7160ea8aadb801a4e44eb57621c54d / ede1a39#diff-f321d4af83fcf8311dc80c0d50c59ac4c240f761206e7bb652709870eb9feb43 is related @juliev0 ?

[juliev0]

https://github.com/argoproj/argo-workflows/blame/v3.4.0/workflow/controller/operator.go#L505 is the error being thrown

i wonder if https://github.com/argoproj/argo-workflows/pull/8907/files#diff-63ebd7c535e623fd05481826190cb78c1c7160ea8aadb801a4e44eb57621c54d / ede1a39#diff-f321d4af83fcf8311dc80c0d50c59ac4c240f761206e7bb652709870eb9feb43 is related @juliev0 ?

Not sure, but I assigned it to myself so I can take a look.

[juliev0]

@tooptoop4 since your regex example is finding some text like "landing/" I thought I might see that in your example payload, but it isn't in there, right? Do you by chance have a payload that better matches the nested regex expressions?

[tooptoop4]

@juliev0 in the json payload u can change po_235248_pan_0000000.ntf to landing/mysch/mytbl/po_235248_pan_0000000.ntf

[juliev0]

@juliev0 in the json payload u can change po_235248_pan_0000000.ntf to landing/mysch/mytbl/po_235248_pan_0000000.ntf

okay, thanks!

[juliev0]

I can see that the commit you referenced does in fact break this. And that's because in operator.go, substituteGlobalVariables() is now being called after updateWorkflowMetadata(). I need to do more investigation to figure out the right order for these functions to be called. I believe in that commit that the order got changed, but for a different reason.