helmChartInflationGenerator: Support multiple helm values files

[jkroepke]

Is your feature request related to a problem? Please describe.

I'm. using the helmChartInflationGenerator to combine multiple helm charts into one ArgoCD release

In my case, I have a ArgoCD Application, that contains:

• kube-prometheus-stack
• oauth-proxy
• Additional manifests like SealedSecrets

This the kube-prometheus-stack is a heavy value file, I would like to create a values.yaml file to the base and extend the values as needed for specific environments. This pattern works out of the box with helm by define multiple value files.

Currently, the helmChartInflationGenerator only allows to define one valueFile + valueInline.

Describe the solution you'd like

Support multiple value files inside the helmChartInflationGenerator.

Helm does support this by just running helm template -f base/values.yaml -f stages/values.yaml

Describe alternatives you've considered

Using a combination of valueFile and valuesInline, but this is going the be messy.

Additional context

[pbecotte]

I don't know if this actually goes far enough. Say we have this

• base
  --- kustomization.yaml

helmChartInflationGenerator:
   values: values.yaml
patches:
 # some patches that I need for this chart

--- values.yaml

• overlays
  ---- prod
  ------ prod.values.yaml
  ------ kustomization.yaml

? what goes here... 

The problem is I could extract the helmChartInflationGenerator block into each overlay...it'd be ugly, but doable. Now by default though, even if values accepted two files, I couldn't refer to the values file in base anyway (could of course override the restrictor). I also can't use "patches" in the base, since the objects won't exist until the overlay runs. I could make base a component, but that's not really what components are for.

It feels like what I really want to do is patch the base kustomization.yaml before it gets executed (to add the extra values file or the inline one), or patch the values.yaml file before the generator runs, but I can't think of any way that works considering the way kustomize handles resource resolution.

Am just going to have to drop base altogether and do a lot of copy/pasta for the project I'm working on, but this would be a very powerful way to deploy projects from third party helm charts.

[KnVerey]

/assign @natasha41575

[khrisrichardson]

@KnVerey does it sound reasonable that this may be a class of problem that becomes more tractable once the plugin composition effort you have proposed lands (e.g. - a HelmValuesMerge mutator that doesn't yet exist populates the valuesInline field of the HelmChartInflationGenerator, which is somehow late bound to a variant as opposed to being clumsily bound to a base where its evaluated too early in the cycle for any notion of a Helm values overlay to be effective or predictable if for example there were conflicting values in the Helm base or overlay, such that were the resultant merged data structure not passed to the Helm templating engine as a unit would yield non-deterministic results)? If not, would you deem that a worthy use case to consider for plugin composition? Thanks

[jkroepke]

Please do not implement an own yaml merge logic. It may incompatible with the helm values merge, since helm supports some custom merge strategies. Implement an own logic would result in new bug reports from time to time.

I recommend to add an additional property valueFiles to the existing plugin helmChartInflationGenerator. valueFiles should accept a list or array of file paths. When executing the helm template command, pass all value files to the helm command helm template . -f file1 -f file2. For future, deprecate valueFile and remove it in kustomize v5.

Additionally, in case kustomize would pass the file path to helm, it would enable the support for downloader plugins.

I understand, downloading value files from remote location can result in non-deterministic results. But there are plugins like helm-secrets which using the plugin downloader mechanic to decrypt encrypted files which can be live in the same repository as the kustomization.yaml itself.

[khrisrichardson]

Hi @jkroepke. I didn't mean to discount your original request. I can see the utility in permitting multiple values files, but the mention of a HelmValuesMerger was meant to be illustrative, not prescriptive, and could easily take the form of coalescing a list of values files and/or merging valuesInline.

The main point I was trying to drive home is how @pbecotte's assertion regarding values in both bases and overlays illustrates the exceptional nature of the helmChartInflationGenerator, which does not allow for neat layered encapsulation like other mutators, but instead would need to operate in a lazily evaluated manner to address his comment. If plugin composition were to take this use case into consideration, the burden on the end user could be minimized.

I have attempted to merge valuesInline with partial success by presenting the HelmChartInflationGenerator in the base as a resource and using PatchTransformer in overlays to merge the valuesInline fields. The effort could have been streamlined if multiple values files were supported instead. The problem with this approach was that patching the HelmChartInflationGenerator resource from the base in an overlay, did not make it any more amenable for consumption by the overlay, since the base also used the generator by design and GVKN conflicts ensued in the overlay. Trying to use mutators as resources and patching them I think showcases one of the reasons Katrina wishes to make plugins more first class.

Such is the type of problem one encounters when dealing with templating. They should write a tool or two that composes bases and overlays to produce variants, so we can stop this templating madness. :)

[pbecotte]

You'll see that I wound up with the same solution...using patches on the valuesInline field to pass transformer and generator configuration up the stack. I opened a ticket related to this as well.

I had the same thought about the helm values resource and actually cloned this repo to take a shot at implementing. I quickly realized I don't follow the code base well enough, but have some observations.

1. The values file is relative to the LAST kustomize root, same for chart home. Makes it very interesting composing complicated overlays.
2. I wanted to have patches for the helm chart objects in the base- in order to work the patches will have to be lazy evaluated as well as the chart generator (otherwise the resources won't exist)
3. I wanted to add values.yaml, resources, and patches in components. Again, none of the transformers should run before the chart generator. Except ones that target the chart generator itself.

Allowing me to specify multiple values files would let me hack it together more easily though, by just including the generator in the last kustomize and pulling in extra values files instead of overlays. So it may be less correct...but would definitely be easier to implement

[natasha41575]

/triage accepted

[pbecotte]

I just reread my comment and realized- what I was trying to say is "multiple values files doesn't help with the hard problems - but its an easy change that will at least help with SOME problems"

[natasha41575]

We will be supporting this in our next iteration of the helm generator.

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[jkroepke]

unstable

[k8s-triage-robot]

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

[chrisduong]

/remove-lifecycle rotten

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue or PR as fresh with /remove-lifecycle stale
• Mark this issue or PR as rotten with /lifecycle rotten
• Close this issue or PR with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[jkroepke]

/remove-lifecycle stale

[wmiller112]

Not sure if this fits here exactly, but I came across this and a few similar issues trying to work out how I could patch Helm values in either valuesInline of valuesFile. Seems its not directly supported, but I did find a workaround that doesn't involve too much repetition. This demo repo shows how you can point the generator to another kustomize directory that patches a HelmChartInflationGenerator as a resource.

[aaronborden-rivian]

👋 I am absolutely in favor of implementing this feature and ask that this Helm chart example be clarified as part of this work.

The Helm chart inflation example seems to be advocating against this feature. Instead, it promotes inflating the chart manually and committing that to the repository and not rely on using remote third-party configuration at all.

Maybe we can remove this document?