Support ConfigMaps from Application definition (helm apps)

[evrardjp]

Is your feature request related to a problem? Please describe. Defining an helm application (app of apps pattern for example) requires to set parameters, one by one, using strings.

If the helm value to override is complex it's easy to make a mistake.

Describe the solution you'd like I think it would be nice to have a way to load the value files to pass to an helm chart in an Application using a configmap, instead of relying on setting parameters, one by one.

Have you thought about contributing yourself?

I would like to do it myself, but I need guidance, never wrote go software nor worked with swagger.

I suppose the first thing to do would be to modify the swagger.json, then run make codegen, then create the appropriate logic somewhere in the generated code. On that point, I guess I could compare how the configmap for argoconfig is read and reimplement something similar. I will probably require help for testing too.

[iampluque]

Actually that would be great. And not only using a config map but also using a block

so instead of

apiVersion: argoproj.io/v1alpha1
kind: Application
spec:
  source:
    helm:
      releaseName: cert-manager      
      valueFiles:
        - values.yaml      
      parameters:
      - name: param1
        value: value1
      - name: param2
        value: value2
      - name: param3
        value: value3

would become

apiVersion: argoproj.io/v1alpha1
kind: Application
spec:
  source:
    helm:
      releaseName: cert-manager      
      valueFiles:
        - values.yaml     
        - override.yaml: |
           param1: value1
           param2: value2
           param3: value3

[alexec]

Is this a core Helm feature? Rather than just Argo CD?

[iampluque]

Is this a core Helm feature? Rather than just Argo CD?

I Dont think, Argo is responsible calling helm with values files and custom params, but actually values files must be located in the same source as the helm chart.

This is problematic when you have multiple clients. In my case, application crd is defined by client (different git repos) and I must override a lot of values in the value files coming from the application crd source. That’s why in our case in-line block would be useful.

In the case of inline block, Argo would be responsible of storing temporary values in the inline block to a temp file and then call helm in the format helm template . --values values.yaml --values override.yaml as per my application crd example

[alexec]

Ah - I get it. You want to have different values for every app without having multiple values files?

[iampluque]

Yeah kind of. At least, value files juste not be in the same got repo than the helm chart.

The helm chart default value file is one thing and can be committed in the same git than the helm chart, but value files representing each client custom properties must be stored along application crd with configmap or within application crd with inline block

[alexec]

ok, I think this is a good issue, perhaps we should get community contribution?

[evrardjp]

I am fine with stepping up. I even have installed slack to communicate with the team for guidance ;)

[iampluque]

@evrardjp are you planning to to work only on the ConfigMap side or on the inline-block as well?

[evrardjp]

I don't mind working on both, but happy to have some help. I thought it would be wise to have a different keyword than valueFiles for this, though.

[iampluque]

I can try to help but that would be my first Go app, my first k8s collaboration, my first everything in the open source world :D

[evrardjp]

There is a start for everything : )

[jessesuen]

Agree with the feature request to allow an in-line block for the contents of values.yaml. I believe it's been proposed before, but I couldn't find the issue.

That said, the proposed syntax is not possible, since you will not be able to mix a literal string with a map in the valuesFiles list. Though it's actually possible to do this in YAML, it's problematic for the backing golang datastructure. Thus we would need to have a separate field to store the literal values. Something like:

apiVersion: argoproj.io/v1alpha1
kind: Application
spec:
  source:
    helm:
      valueFiles:
        - staging.yaml     
      valuesLiteral: |
           param1: value1
           param2: value2
           param3: value3

This would translate to the command:

helm template . -f staging.yaml -f /tmp/valuesLiteral.yaml

Also, my proposed syntax would not handle if someone wanted to do:

helm template . -f /tmp/valuesLiteral.yaml -f staging.yaml

Finally, I didn't think there's a use case to support multiple valuesLiteral fields, which is why valuesLiteral is a string instead of a string array.

[jessesuen]

If we want greatest flexibility (i.e. support multiple valuesLiteral, and support interwoven literals to existing values.yaml in git), it would probably need to look like:

apiVersion: argoproj.io/v1alpha1
kind: Application
spec:
  source:
    helm:
      valueFiles:
      - override.yaml   
      - staging.yaml   
      valuesLiteral:
      - name: override.yaml
        value: |
           param1: value1
           param2: value2
           param3: value3

NOTE the use of name/value is intentional over using the filename as the key, in order to follow K8s API guidelines.

[evrardjp]

I propose we split this in issue in two. One to focus on giving the whole series of "Literal" and one to use the configMap.

I am proposing a split because they will technically be implemented differently.

On top of that I think valuesLiteral's value would be just a YAML string, isn't it? So it's very close to the "set" features we already have, just a different format.

[jessesuen]

I propose we split this in issue in two. One to focus on giving the whole series of "Literal" and one to use the configMap.

Yes they would be entirely separate features. That said, I don't really agree that we should support getting the contents of a values.yaml from a ConfigMap, as it adds an unnecessary layer of complexity in the code. Manifest rendering would suddenly involve extra lookups of kubernetes objects, which the repo-server currently has no privileges to do (intentionally for security reasons).

I'd be happy to convert this issue to supporting only the string literals for values in the app spec.

[iampluque]

I'd be happy to convert this issue to supporting only the string literals for values in the app spec.

the inline block should be for sure another feature but I still support @evrardjp issue as ConfigMap are specifically intend to store configurations. It might be harder to implement, but it still interessting.

On my point of view, valueFiles/parameters properties starts to be the less useful as values located in the source repo are not intended to be used as is.

[evrardjp]

Yes they would be entirely separate features. That said, I don't really agree that we should support getting the contents of a values.yaml from a ConfigMap, as it adds an unnecessary layer of complexity in the code. Manifest rendering would suddenly involve extra lookups of kubernetes objects, which the repo-server currently has no privileges to do (intentionally for security reasons).

Do you have an architecture graph I could see? That's something I miss to have a full understanding of the scope of the change. At first sight, it doesn't seem that different to pass arbitrary data from the Application content (valuesLiteral) vs the configmap. Would it be possible to have resolution before data is fetched in repo-server, and data then sent the same way as valuesLiteral?

[evrardjp]

I suppose it's not that far away from https://github.com/argoproj/argo-cd/issues/1786 , which makes me think there is a interest of users, but we would need to tighten the security story.

[evrardjp]

I'd be happy to convert this issue to supporting only the string literals for values in the app spec.

the inline block should be for sure another feature but I still support @evrardjp issue as ConfigMap are specifically intend to store configurations. It might be harder to implement, but it still interessting.

On my point of view, valueFiles/parameters properties starts to be the less useful as values located in the source repo are not intended to be used as is.

We can work in an iterative matter, work on something we all agree first (the inline file), then do the ConfigMap later.

[alexec]

The literal block is implemented in #2057. Can we close this please?

[evrardjp]

The literal block allows to close #1930. This is a different item, and I don't mind tackling this based on learnings from #2057 .

[evrardjp]

@pluqueTheLuxe with the values from a literal block merged, it is now possible to use the App of Apps pattern to rely on local files outside the remote git repo used for app deploy. Assuming you template the "App" to deploy using helm, you can now pass the content of a file into the block using the helm "File" templating.

I think it's good enough for many use cases, including mine. Opinion?

[iampluque]

Thats probably enough for now. agree

[alexec]

I think we should keep this issue open, as it is a valid feature, but maybe prioritise it.

[dcherman]

Hi,

Just a heads up, I'm starting to tackle this using the same strategy as the helm-operator from WeaveWorks. Both secrets and configmaps would be supported.

Just to keep updates/discussion in one place, I'm putting all details in #1786

[stale[bot]]

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[OmerKahani]

Hi, is there any update on this? what is blocking this from been merged?

[jcrsilva]

As a possible workaround to this, is there any way of customizing the argocd build environment?

[jcrsilva]

I was able to find a workaround for this, leaving it here for posterity. For reference, my use case for this is to allow argocd to manage itself and the cluster in which it is installed (App of App pattern) but having external metadata ingested directly from the cluster. In my case, this metadata is in a configmap managed by terraform, and provides info like in what datacenter the cluster is and what external domain it is reachable from.

You can mount a configMap as a volume onto the repo server. The official helm chart already exposes the required config as a value:

  repoServer:
    volumes:
      - name: environment-metadata
        configMap:
          name: environment-metadata

    volumeMounts:
      - name: environment-metadata
        mountPath: /environment-metadata

Since this content never changes, you can change the configmap and argo will not consider itself out of sync.

Although the UI doesn't allow you to, you can reference absolute paths in helm's valueFiles, so the following works:

  source:
    helm:
      valueFiles:
      - /environment-metadata/environment-metadata.json

[phajduk]

@jcrsilva I think it's not possible anymore due to https://www.zdnet.com/article/argo-cd-releases-patch-for-0-day-vulnerability/

[kagemusha-sgs]

One possible solution would be to use a recursive Helm Template function:

{{- /*
  Put this in Application templates/_helpers.tpl

  Recursive function, callable with `include`function

  Generates compliant YAML for ArgoCD Application Helm parameters override
  that contain iterable values (e.g. `slice` or `map`), not just flat strings

  Name/value pairs are printed unindented,
  you need to pipe the results with `indent` accordingly

  Example call:

    helm:
      parameters:
        {{- include "recStruct" (list .Values.myvariable $ "myvariable") | indent 8 }}

*/}}
{{- define "recStruct" }}

  {{- /*
    Expect three params
    1: The variable to print
    2: The context of the yaml that includes the function
       (in order to be able to resolve templated variables with `tpl` function)
    3: The path of the resulting top level element
  */}}

  {{- $v := index . 0 }}
  {{- $context := index . 1 }}
  {{- $k := index . 2 }}

  {{- /*
    Test first the type of element.
    Target three conditions:
    - slice (e.g. python list) or map (e.g. python dict)
    - string
    - boolean
  */}}
  {{- if or (kindIs "map" $v) (kindIs "slice" $v) }}
    {{- /*
      Set a boolean value, based on the type of iterable
    */}}
    {{- $ismap := (kindIs "map" $v) }}

    {{- if empty $v }}
      {{- /*
        If variable is empty, stop processing
      */}}
- name: {{ printf "%v" $k }}
  value: ""
    {{- else }}
      {{- /*
        Iterate over the iterable `$v`, obtaining key/value pairs `$k2` and `$v2`
      */}}
      {{- range $k2, $v2 := $v }}
        {{- /*
          Construct the element path as a YAML array or object,
          e.g. with brackets or with a dot, depending on the type of iterable,
          using the key `k2` returned by the iterator
        */}}
        {{- $el_path := ternary (printf "%v.%v" $k $k2)  (printf "%v[%v]" $k $k2) $ismap -}}

        {{- /*
          Test if the value `$v2` returned by the iterator is itself an iterable type
        */}}
        {{- if or (kindIs "map" $v2) (kindIs "slice" $v2) }}

          {{- /*
            If the value $`v2` is iterable, call the function recursively,
            passing the following params:
            1: `$v2` variable
            2: original context, inherited from initial call
            3: element path computed above
          */}}
          {{- include "recStruct" (list $v2 $context $el_path) }}
        {{- else }}
          {{- if kindIs "string" $v2 }}
            {{- /*
              If the `$v2` value is a string, and contains a templated variable:
              use the `tpl` function to replace it with its actual content
            */}}
            {{- if and (contains "{{" $v2) (contains "}}" $v2) }}
- name: {{ $el_path }}
  value: {{ tpl $v2 $context | quote }}
            {{- else }}
              {{- /*
                It is a string, and it does not contain any variable
              */}}
- name: {{ $el_path }}
  value: {{ $v2 | quote }}
            {{- end }}
          {{- else }}
            {{- /*
              It is a boolean
            */}}
- name: {{ $el_path }}
  value: {{ $v2 | quote }}
          {{- end }}
        {{- end }}
      {{- end }}
    {{- end }}
  {{- else }}
    {{- if kindIs "string" $v }}
      {{- /*
        If the value is a string, and contains a templated variable:
        use the `tpl` function to replace it with its actual content
      */}}
      {{- if and (contains "{{" $v) (contains "}}" $v) }}
- name: {{ printf "%v" $k }}
  value: {{ tpl $v $context | quote }}
      {{- else }}
        {{- /*
          It is a string, and it does not contain any variable
        */}}
- name: {{ printf "%v" $k }}
  value: {{ $v | quote }}
      {{- end }}
    {{- else }}
      {{- /*
        It is a boolean
      */}}
- name: {{ printf "%v" $k }}
  value: {{ $v | quote }}
    {{- end }}
  {{- end }}
{{- end }}

[andrii-korotkov-verkada]

Similar to https://github.com/argoproj/argo-cd/issues/1786#issuecomment-2478709959. We need some way to detect when application needs to be updated, as its manifest doesn't change when CM changes.