Release values inheritance #726

Vince-Chenal · 2023-03-06T13:55:01Z

Vince-Chenal
Mar 6, 2023

Context

Hello,

We have started experimenting with helmfile in our company and intend to use it on a fairly large scale, and we think that for some of our more complex use cases, we have found a possible usability improvement to reduce duplication.
It appears others have also expressed a similar need (here or here) in the past, and we’d like to know if our approach makes sense for helmfile and ensure that we have a full understanding of the impact of our proposed changes.

Problem

We understand that today the layering mechanism allows for reusing previously defined values.

Example:

environments:
  default:
    values:
    - value1: somevalue
---
environments:
  default:
    values:
    - value2: {{ .Values.value1 }}
---
releases:
  - name: my-release
    chart: my-chart
    values:
      - finalValue: {{ .Values.value2 }}

Some of our more complex use cases require many layers, which makes the corresponding helmfiles quite verbose and unwieldy. We therefore would like your opinion on the proposed improvement, which we believe to be fairly intuitive and concise.

Proposition

While generating the temporary values files (here) it would be feasible to load the rendered yamls in memory (in RenderedValues) so that we can inherit the "previously defined values".

More complex example:

helmfile.yaml

environments:
  dev:
    values:
    - defaults.yaml
  production:
    values:
    - defaults.yaml
    - production-values.yaml
---
releases:
  - name: my-release
    chart: my-chart
    values:
      - common.yaml.gotmpl
      - common-{{ .Environment.Name }}.yaml.gotmpl
      - values.yaml.gotmpl
    missingFileHandler: Debug

defaults.yaml

projectName: foo
region: bar
domain: .io

production-values.yaml

domain: .com

common.yaml.gotmpl

application_url: {{ .Values.projectName }}-{{ .Values.environment }}.{{ .Values.region }}.{{ .Values.domain }}

common-production.yaml.gotmpl

application_url: {{ .Values.projectName }}{{ .Values.domain }}

values.yaml.gotmpl

conf: |
    config {
      field1 = 1
    }

    url = {{ .Values.application_url }}

The goal here is to have some defaults that define the common way for building the application URL and allow overriding it on a per-environment logic. We could generalize this logic per project, per env, per region...

Here's what we would get depending on the environment:
dev:

❯ helmfile template --environment dev .
Building dependency release=my-release, chart=my-chart
Templating release=my-release, chart=my-chart
---
# Source: my-chart/templates/configmap.yaml
apiVersion: v1
kind: ConfigMap
metadata:
  name: my-cm
data:
  foo.json: |
    config {
      field1 = 1
    }
    url = foo-dev.bar.io

production:

# Source: my-chart/templates/configmap.yaml
apiVersion: v1
kind: ConfigMap
metadata:
  name: my-cm
data:
  foo.json: |
    config {
      field1 = 1
    }
    url = foo.com

Implementation

We have provided a demonstration implementation in this PR (we don’t expect it to be mergeable as-is) and we’d be happy to discuss some more about it.

If it makes sense, a similar could be also be used for the environments blocks.

mumoshu · 2023-03-06T23:29:30Z

mumoshu
Mar 6, 2023
Maintainer

@Vince-Chenal Hey! Thanks a lot for this detailed write-up. It did help me understand the context of this feature request.

To be sure, could you clarify this to be about adding a way to pass inputs to a release values template file values.yaml.gotmpl, where the inputs are some values generated from environment values- Is that correct?

7 replies

mumoshu Mar 8, 2023
Maintainer

    - append:
        from: all
        to: values

could be instead

    - append:
        values:
        - output: all

so that it is consistent with how we refer to output in render.with.

Vince-Chenal Mar 9, 2023
Author

Hello,

To give a bit more context, we are migrating our usecases from Terraform to Helmfile and we are struggling to represent some of them with Helmfile. In Terraform we use a lot of locals which can depend on each other. Terraform builds a dependency graph behind the scenes that allows cross-referencing without defining dependencies explicitly.

To sum up, we do think that helmfile's flexibility could be improved so that we can add similar cross-referencing and I thought that this tiny semantic change (taking advantage of the inherent ordering of a list) would be a big added value at low cost.

Regarding your proposition, in my opinion that approach would add a lot of additional complexity to helmfile. While reading your proposition I thought about a possible compromise that could help limit the ambiguity: how about only introducing this semantic change in the environments's values listing ?

I just did the test and the code change is even simpler: https://github.com/helmfile/helmfile/compare/main...ContentSquare:helmfile:add-environments-values-inheritance?expand=1

This way it would only add more flexibility in the environments definition and not in releases rendering.
And in particular, it doesn't break the mentioned "any values.yaml and values.yaml.gotmpl files can depend on environment values" contract :)

We could even flag that behaviour behind a different syntax for explicitness, for example by specifying .Environment..layeredValues[] instead of .Environment..values[] in order to ensure it's visible to anyone reviewing a helmfile without having seen it prior.

What do you think?

mumoshu Mar 12, 2023
Maintainer

@Vince-Chenal Something like "layedValues", a new field for a new feature, would be a viable option. Thank you for the suggestion!

I'm not sure if "layered" is the right term here though- Environment values and release values are already "layered" in the sense that they are rendered and merged in the order of definition.
Do you have any other suggestions regarding the name?
Does "pipelinedValues" make sense to you?

Vince-Chenal Mar 13, 2023
Author

@mumoshu
Ok for pipelinedValues if you really prefer to isolate the behaviour.

Do you think it would be that risky to integrate this behaviour in environments values as is ? I don't see usecases where it could break anything actually, unless i'm missing something?

Vince-Chenal Mar 19, 2023
Author

@mumoshu @yxxhero
Any ideas about the previous comment?

Here's the related PR:
https://github.com/helmfile/helmfile/pull/741/files

yxxhero · 2023-03-09T03:11:38Z

yxxhero
Mar 9, 2023
Collaborator

In my opinion. it makes helmfile become more complex.

1 reply

mumoshu Mar 9, 2023
Maintainer

True! At the same time, I'd like to prevent folks from easily exploiting the combination of exec, loadYAML, and merging in the yaml template to achieve the same result 😂

Perhaps it might be ok if we could come up with a simple-enonugh and generalized new feature, instead of making an existing feature more complex(which is a maintenance nightmare)?

Uh oh!

Uh oh!

Release values inheritance #726

Uh oh!

Uh oh!

Vince-Chenal Mar 6, 2023

Context

Problem

Proposition

More complex example:

Implementation

Replies: 2 comments · 8 replies

Uh oh!

mumoshu Mar 6, 2023 Maintainer

Uh oh!

mumoshu Mar 8, 2023 Maintainer

Uh oh!

Vince-Chenal Mar 9, 2023 Author

Uh oh!

Uh oh!

mumoshu Mar 12, 2023 Maintainer

Uh oh!

Vince-Chenal Mar 13, 2023 Author

Uh oh!

Vince-Chenal Mar 19, 2023 Author

Uh oh!

yxxhero Mar 9, 2023 Collaborator

Uh oh!

Uh oh!

mumoshu Mar 9, 2023 Maintainer

Vince-Chenal
Mar 6, 2023

Replies: 2 comments 8 replies

mumoshu
Mar 6, 2023
Maintainer

mumoshu Mar 8, 2023
Maintainer

Vince-Chenal Mar 9, 2023
Author

mumoshu Mar 12, 2023
Maintainer

Vince-Chenal Mar 13, 2023
Author

Vince-Chenal Mar 19, 2023
Author

yxxhero
Mar 9, 2023
Collaborator

mumoshu Mar 9, 2023
Maintainer