argoproj / argo-cd

Declarative Continuous Deployment for Kubernetes
https://argo-cd.readthedocs.io
Apache License 2.0
17.8k stars 5.43k forks source link

Support additional arguments for plugins #5510

Open msmsimondean opened 3 years ago

msmsimondean commented 3 years ago

Summary

I use Helm together with hiera-eyaml. This means I have to use Helm with Argo CD via a plugin. I also want to be able to use Helm's --set and --set-string arguments.

If I was using Argo CD's built in support for Helm, I'd just be able to do something like this:

https://github.com/argoproj/argo-cd/blob/master/docs/operator-manual/application.yaml#L15

  source:
    helm:
      parameters:
      - name: "nginx-ingress.controller.service.annotations.external-dns\\.alpha\\.kubernetes\\.io/hostname"
        value: mydomain.example.com
      - name: "ingress.annotations.kubernetes\\.io/tls-acme"
        value: "true"
        forceString: true

As I'm using a plugin instead of the builtin Helm support, I'm not able to pass arguments to helm. Instead I have to do something like this:

  source:
    plugin:
      name: eyaml-helm-with-parameters
      env:
        - name: HELM_VALUE_some_key
          value: "{{ .Values.someKey }}"

My custom plugin then looks like this:

    configManagementPlugins: |
      - name: eyaml-helm-with-env-vars
        init:
          command: ["/bin/sh", "-c"]
          args: ["helm dependency build"]
        generate:
          command: ["/bin/sh", "-c"]
          args: ["eyaml decrypt -f $VALUES_FILE --pkcs7-public-key=some-public-key.pem --pkcs7-private-key=some-private-key.pem > ${VALUES_FILE%.eyaml}.yaml && helm template . -f ${VALUES_FILE%.eyaml}.yaml `printenv | grep HELM_VALUE_ | sed 's/^HELM_VALUE_//' | sed -e ':loop' -e 's/^\([a-zA-Z0-9.]*\)_/\1./' -e 't loop' | tr '\n' ',' | sed 's/^/-set /' | sed 's/,$//'`"]

As you can see, the bash command is quite horrible 😢

Also it's not possible to pass complex Helm keys like nginx-ingress.controller.service.annotations.external-dns\\.alpha\\.kubernetes\\.io/hostname

Motivation

It would be good to be able to either:

  1. pass additional arguments to a plugin from an Argo CD application or
  2. be able to configure a plugin as supporting the same arguments as Helm (or any other built-in template tool)

Proposal

Proposal 1 below seems like the easiest and simplest of the two proposals.

Proposal 1

Support custom arguments for plugins. Here's an example of how this could be used:

  source:
    plugin:
      name: custom-helm-based-plugin
      args:
      - "--set"
      - "nginx-ingress.controller.service.annotations.external-dns\\.alpha\\.kubernetes\\.io/hostname=mydomain.example.com"
      - "--set-string"
      - "ingress.annotations.kubernetes\\.io/tls-acme=true"

Argo CD would pass the args as additional arguments when executing the plugin's generate command.

An alternative name for args would be generateArgs.

Proposal 2

Configure plugins as supporting the same command line arguments as one of the built-in template tools. Here's an example of how this could be used:

  source:
    plugin:
      name: custom-helm-based-plugin
      parameters:
      - name: "nginx-ingress.controller.service.annotations.external-dns\\.alpha\\.kubernetes\\.io/hostname"
        value: mydomain.example.com
      - name: "ingress.annotations.kubernetes\\.io/tls-acme"
        value: "true"
        forceString: true
    configManagementPlugins: |
      - name: custom-helm-based-plugin
        compatibleWith: helm
        init:
          command: ["/bin/sh", "-c"]
          args: ["helm dependency build"]
        generate:
          command: ["/bin/sh", "-c"]
          args: ["eyaml decrypt -f $VALUES_FILE --pkcs7-public-key=some-public-key.pem --pkcs7-private-key=some-private-key.pem > ${VALUES_FILE%.eyaml}.yaml && helm template . -f ${VALUES_FILE%.eyaml}.yaml"]

compatibleWith would tell Argo CD to support Helm parameters with this plugin.

jannfis commented 3 years ago

We have an item to improve config management plugins on our roadmap, so this request should be discussed in that context.

msmsimondean commented 3 years ago

Thanks for the info @jannfis. Based on the entry in the roadmap, would a PR for proposal 1 be welcome or is it too early for that?

jannfis commented 3 years ago

To be honest, I think it would make more sense to collect some more requirements from the community before actually steering into some direction with changes regarding to current custom plugin behavior and setup.

But you are more than welcome to participate in the process of gathering and defining requirements, and of course also in the implementation of those at a later point :). To get things rolling, you might want to consider bringing this topic up on the agenda of our monthly community meeting and discuss it with us (you can find the upcoming time slots in the Argo Project community calendar).

msmsimondean commented 3 years ago

Thanks @jannfis. What's the process for adding something to the agenda for the community meeting? I see http://bit.ly/argo-cd-cmty-mtng hasn't got the March meeting listed yet and doesn't mention a person to contact for adding something to the agenda. Thanks!

msmsimondean commented 3 years ago

Hi @jannfis. A colleague suggested a much simpler approach to solving this. I've not tried on a real cluster yet, but it should work. You pass all the Helm params as a single environment variable (well one env var for all normal params and a second env var for force string params) rather than every Helm param having its own env var. It also avoids the Helm param names being in the env var name (they're in the env var value instead) which avoids the issue of POSIX/Linux not supporting "." in env var names.

Here's the plugin config:

    configManagementPlugins: |
      - name: eyaml-helm-with-env-vars
        init:
          command: ["/bin/sh", "-c"]
          args: ["helm dependency build"]
        generate:
          command: ["/bin/bash", "-c"]
          args: ["eyaml decrypt -f $VALUES_FILE --pkcs7-public-key=some-public-key.pem --pkcs7-private-key=some-private-key.pem > ${VALUES_FILE%.eyaml}.yaml && helm template . -f ${VALUES_FILE%.eyaml}.yaml --set ${MSMG_HELM_PARAMS@Q} --set-string ${MSMG_HELM_STRING_PARAMS@Q}"]

It's a LOT simpler than my original approach and doesn't require a change in Argo CD itself. You have to (I think) use 'bash' rather than sh as the ${variable@Q} syntax is Bash specific.

Here's an example of using it:

  source:
    plugin:
      name: eyaml-helm-with-parameters
      env:
        - name: MSMG_HELM_PARAMS
          value: "key1={{ .Values.value1 }},key2={{ .Values.value2 }}"
        - name: MSMG_HELM_STRING_PARAMS
          value: "key3={{ .Values.value3 }},key4={{ .Values.value4 }}"
jannfis commented 3 years ago

Thanks @jannfis. What's the process for adding something to the agenda for the community meeting? I see http://bit.ly/argo-cd-cmty-mtng hasn't got the March meeting listed yet and doesn't mention a person to contact for adding something to the agenda. Thanks!

Oops. Thanks for the reminder. Since I'm no editor myself, I made an edit suggestions to add the upcoming date along with some note on how to put things to the agenda (which would also involve suggestion mode). :)

jannfis commented 3 years ago

As for your example above, it looks interesting. I wonder whether that will work if one of the expanded values from actually contains a comma? Never took a look at that particular bash string feature yet.