Argo: Add support for alternate variable-reference syntax

[danxmoran]

Summary

(This would be a significant change, so I understand if it's not really feasible)

Argo currently supports syntax like this for referencing variables:

{{some.variable.reference}}

It would be a significant UX improvement for my team if we could use something other than {{ }} as the syntax.

Motivation

I have two main motivations for asking.

First, my team uses Helm to publish & share utility WorkflowTemplate definitions, as well as to inject top-level inputs into CronWorkflow specs on deployment. Helm uses the same syntax for its own variable substitution, so you have to jump through some hoops to "hide" the Argo variables from the rendering engine. There are a few ways to get around it, but we've landed on:

{{ "{{some.variable.reference}}" }}

It's not the worst thing ever, but it's definitely not ideal. It's really easy to forget the double-bracketing, or misplace a bracket, or quote in the wrong place, etc. It also adds another layer of cognitive load to reading templates (is this a Helm variable, or an Argo variable?).

Second, whenever the YAML parser in kubectl / helm (and potentially argo, I can't remember) sees a leading { in a value, it assumes that the value is an object specified using JSON syntax. When the second character is another {, it fails parsing. This means that any stand-alone Argo variable reference has to be quoted, but references embedded in the middle of some other string don't need to be. This adds to all the other inconsistencies that come with writing YAML, as you end up with statements like:

arguments:
  parameters:
    - name: input-one
      value: '{{some.bare.variable}}' # Very tempting to remove these quotes
    - name: input-two
      value: gs://{{some.prefixed.variable}}/

It gets even more "fun" when you mix it with our Helm hack:

arguments:
  parameters:
    - name: input-one
      value: '{{ "{{some.bare.variable}}" }}' # Or {{ "{{some.bare.variable}}" | quote }}
    - name: input-two
      value: gs://{{ "{{some.prefixed.variable}}" }}/ # Or {{ "gs://{{some.prefixed.variable}}/" }}

None of these are show-stopping problems, but they do introduce a lot of small code-style decisions that bog us down / confuse newcomers to the code.

Proposal

I'm not familiar enough with the codebase to have a deep proposal (but I'd be willing to dig in and implement this if there's interest!). I do see some high-level design questions that would need to be addressed first:

• Should the controller hard-code a second option for variable references? Or should the choice be exposed as config, so users can do whatever they want?
• If hard-coding a 2nd choice, what should it be?
  
  
  
  • [[ ]] would make Helm happy, but it'd have similar issues with the YAML parser.
  
  
  • << >> and (( )) feel "safe", but I haven't thought about it too deeply.
  
  
• If exposing the choice as config, where should that config go?
  
  
  
  • Exposing it in the controller's config map feels least complex, but it could make reusing templates from different sources trickier (sorry I can't use your template, it uses (()) but I'm running with <<>>)
  
  
  • Exposing it as an annotation on workflows / templates / cron workflows would help with the reusability, but it feels like that'd be a lot more complicated for the controller / server to deal with.
  
  

---

Message from the maintainers:

If you wish to see this enhancement implemented please add a 👍 reaction to this issue! We often sort issues this way to know what to prioritize.

[danxmoran]

(Just thinking out loud): Instead of an annotation, could this be implemented using new spec fields? With default values of "{{" and "}}"

[smitthakkar96]

I would like to work on this if the maintainers agree to make this change. It looks really ugly when used with helm

[simster7]

I've given some thought to this and see some blockers before introducing this in the controller:

1. It seems like an abstraction violation for us to implement a solution in the controller for a problem which is really external to it (collisions with Helm).

2. I can foresee some issues with WorkflowTemplates. We agree that it's not a good idea to define the syntax on the controller-level, as it would be impossible to run Workflows with different syntaxes.

   However, I also see an issue in defining this in either a Workflow annotation or the Workflow Spec: if a Workflow references a single template withing an external WorkflowTemplate with a different syntax, how should it be resolved? We would have to consider the syntax preferences of the WorkflowTemplate and the Workflow.

   This seems _doable_, but in my opinion it would be more of a maintenance burden than it would be useful.

3. Most importantly perhaps is a reason pointed out by @jessesuen. We rely on the {{...}} syntax for a technical reason as well: since it is impossible to have a valid JSON with the {{ token, we can exploit this to replace Argo variables by converting Argo objects to JSON, doing the replace, and converting back. This might not hold true for other tokens, such as [[.

From here I can see at least two paths forward:

1. Implement this as a convenience feature in a step outside of the controller, either as a utility arguments to argo submit (something like argo submit --var-open="<<" --var-close=">>"). However, this would only be useful when using argo submit.

2. Take an official position that we do not want to support this behavior and would leave it to our users to script around it. A user in Slack (@brabster) posted a promising workaround:

I think I've found a neat solution specifically for the Helm usecase. Change the {{ and }} to whatever values you want that don't conflict with anything and use a post-processor to sed them back

eg. template

inputs:
  parameters:
    - name: fetch-url
    - name: local-dump-path
      value: {{ .Values.foo }}
 outputs:
   parameters:
     - name: local-dump-path
       value: "{-inputs.parameters.local-dump-path-}"

then helm install foobar helm-chart --post-renderer ./helm-chart/post-render-argo.sh

where post-render-argo.sh is

#!/bin/bash
set -e
sed 's/{-/{{/g' | sed 's/-}/}}/g' <&0

[brabster]

I'm pretty happy with my workaround, not seen any issues so far. I have a more detailed walkthough up at https://tempered.works/posts/2020-05-23-helm-charts-for-argo-workflows with a linked Github example repo if helpful.

If you do decide to implement, on point (3) I think it would be reasonable to require new delimiters to not be valid JSON syntax and to expect users to ensure that is so.

[mac9416]

We've found that the initial reaction to {{`{{}}`}} is confusion. But after that, it's okay.

If we did find we needed it, I think the workaround would be sufficient.

[simster7]

Thanks guys, closing this issue.