Proposal to introduce dynamic application parameters to Argo CD

Signed-off-by: jannfis [email protected]

Note on DCO:

If the DCO action in the integration test fails, one or more of your commits are not signed off. Please click on the Details link next to the DCO action for instructions on how to resolve this.

Checklist:

[ ] Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
[ ] The title of the PR states what changed and the related issues number (used for the release note).
[ ] I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
[ ] I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
[ ] Does this PR require documentation updates?
[ ] I've updated documentation as required by this PR.
[ ] Optional. My organization is added to USERS.md.
[ ] I have signed off all my commits as required by DCO
[ ] I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
[ ] My build is green (troubleshooting builds).

Jan 19 '23 18:01 jannfis

Codecov Report

Base: 47.38% // Head: 47.38% // No change to project coverage :thumbsup:

Coverage data is based on head (117770b) compared to base (584428e). Patch has no changes to coverable lines.

Additional details and impacted files

@@           Coverage Diff           @@
##           master   #12050   +/-   ##
=======================================
  Coverage   47.38%   47.38%           
=======================================
  Files         245      245           
  Lines       41670    41670           
=======================================
  Hits        19745    19745           
  Misses      19937    19937           
  Partials     1988     1988

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

Jan 19 '23 18:01 codecov[bot]

@jannfis how would you feel about moving dynamicParameters outside the helm block and then communicating the parameters into the helm block via the already-existing "build environment" mechanism?

    dynamicParameters:
      - name: bar
        resourceRef:
          kind: ConfigMap
          group: ""
          name: some-cm
          namespace: some-ns # if omitted, use target namespace
          path: .data.somekey # if omitted, set param to true if resource exists or to false if resource does not exist
    helm:
      parameters:
        - name: bar
          value: "$ENV_BAR"
    kustomize:
      images:
      - image=image:$ENV_BAR
    plugin:
      env:
      - name: BAR
        value: "$ENV_BAR"

That would make this feature usable for all source types.

EDIT: come to think of it, we might want to move it outside source so that the params are available to all sources in a multi-source app. Alternatively, we could create a new resourceRef source type, and communicate parameters to other sources via mechanisms similar to the "external values file" mechanism in multi-source apps. I have no preference between those two options.

May 26 '23 20:05 crenshaw-dev

Should vanilla manifests be considered as part of this proposal or this is something that would work just with Helm and Kustomize by leveraging their internal parameters capabilities? If so, how variable interpolation would work with vanilla manifests?

Jun 01 '23 16:06 leoluz

Thanks for your comments and input folks!

Should vanilla manifests be considered as part of this proposal

I would tend to answer this question as "no", as this would expand the scope of the change significantly. I'm not generally opposed to be able to parameterize vanilla manifests. However, once (if) we decide we want vanilla manifests to be parameterized, then I see no reason to not scope dynamic parameters to vanilla manifests as well.

how would you feel about moving dynamicParameters outside the helm block we might want to move it outside source so that the params are available to all sources in a multi-source app

The reason the dynamicParameters are in the helm block is to have then in the same scope/level as where existing static parameters are today. I'm not strongly opinionated about the actual location of the configuration, but I think that a parameter should affect a source, and not all sources or the application itself. For example, if you render two Helm charts they may want to have different value for the same parameter.

I'm also not sure that I like values being exposed as extrapolated strings, it feels a little too complicated to me and string operations can be expensive and error-prone (e.g. another spot for a typo to be made).

I think the other source types (when they support dynamic parameterization) should have some kind of first-class means of defining parameters, and then get dynamic parameters as first class citizens, too.

As an alternative idea, how about implementing the dynamic portion directly in existing mechanisms for source parameterization, for example:

spec:
  source:
    helm:
      parameters:
         name: bar
         type: dynamic # defaults to static
         resourceRef:
           ...
    plugin:
      env:
      - name: bar
        type: dynamic # defaults to static
        resourceRef:
          ...
      parameters:
      - name: bar
        type: dynamic
        resourceRef: # data type of parameter determined by referenced field
         ...
    directory:
      jsonnet:
        extVars:
        - name: bar
          type: dynamic
          resourceRef:
          ....

For kustomize, there's limited use for this though, because everything in the kustomize spec is a string.

Jun 04 '23 19:06 jannfis

This is a pivotal feature for adoption of Argo into our organization, particularly around secret management.

Sep 26 '23 15:09 des1redState

This is a pivotal feature for adoption of Argo into our organization, particularly around secret management.

I agree. I was using a custom plugin and a shell script to read/replace from a k8s secret but having this embedded in upstream ArgoCD would be a game changer!

Sep 27 '23 08:09 sitilge

Hi, thank for this suggestion, @jannfis! I see that this is an architectural discussion PR. Is there any on-going work for this that I missed? I am interested in following the WIP of this feature. This can make our secrets management much easier. I have spent almost a week trying to figure out a good approach to manage our secret Helm values, but haven't found any solution that made me feel that it is "clean" ^^". Referencing them from a secret would be on of the best we can do, IMHO.

@crenshaw-dev, may I ask you if there are any plans in the near future for this? Just for me to be able to prioritize my work.

I see that this is not part of 2.9 (which will be released in less than a week), but can this be planned for 2.10?