-
Notifications
You must be signed in to change notification settings - Fork 564
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: specify env values from the parent to the nested state #622
Conversation
Adds the `helmfiles[].environment.values` that accepts a mix of file pathes and inline dictes: ```yaml helmfiles: - path: path/to/nested/helmfile.yaml environment: values: - key1: val1 - values.yaml ``` The values files are loaded in the context of the parent state file. For example, in case the above state file is located at `/path/to/helmfile.yaml`, `values.yaml` is located at `/path/to/values.yaml` instead of `/path/to/nested/values.yaml`. Resolves #523
The latest commit broke any state files like the below to NOT pass env value overrides at all: ``` helmfiles: - path: nested/state.yaml environment: values: - overrides.yaml ``` This fixes the issue.
# dev.yaml
foo:
bar: 1 # helmfile.yaml
environments:
dev:
values:
- dev.yaml Then:
|
I am sorry I don't understand what this does ? |
ha I guess you meant this
|
I mean helmfile v0.68.0 introduces a Here is a more practical example:
|
@sgandon I've intentionally omitted the environment name from the config syntax, because requiring the env name to override makes it harder to "just override some values of the selected env". With the config syntax introduced via this change, you don't need to write something like the below to do that:
Instead you write:
|
@mumoshu I am not sure that I get it, to what environment the values |
I am not a fan of this, I wanted to mention. Perhaps I'm missing something, but this broke my existing setup.
|
@yurrriq Hey! Thanks for reporting. The only reason you don't love this feature is that it breaks your existing setup, right? This isn't intended to break anything. If it broke, it is a bug. Would you mind sharing your detailed setup incl. helmfile version, so that I can try to reproduce and see what's wrong? I had no prob with this:
|
@sgandon Hey! It overrides the values for the current environment (set via --environment, defaults to |
…-helmfiles) We added envvals overrides in the state file via #622 two days ago: ``` helmfiles: - name: sub.helmfile.yaml environment: values: - mykey: myvalue ``` This change removes the `environment` level in the above cofig, so that it looks like: ``` helmfiles: - name: sub.helmfile.yaml values: - mykey: myvalue `` This is an inevitable breaking change towards #361. But I wanted to break it earlier so that less folks are affected.` Ref #361 (comment)
…-helmfiles) (#635) We added envvals overrides in the state file via #622 two days ago: ``` helmfiles: - name: sub.helmfile.yaml environment: values: - mykey: myvalue ``` This change removes the `environment` level in the above cofig, so that it looks like: ``` helmfiles: - name: sub.helmfile.yaml values: - mykey: myvalue `` This is an inevitable breaking change towards #361. But I wanted to break it earlier so that less folks are affected.` Ref #361 (comment)
I don't really understand or have a use for this future, at least at this time, and it broke my old configs, which looked more like the following. ==> environments/baz.yaml <==
foo: FOO
==> helmfile.d/subhelmfile.yaml <==
{{ readFile "../include/environments.yaml" }}
==> include/environments.yaml <==
environments:
default: &default_environment
values:
- "../environments/{{ .Environment.Name }}.yaml"
baz: *default_environment This worked fine with helmfile 0.54.0 (and 0.64.1 IIRC) and broke when I switched to 0.68.1. |
Adds the
helmfiles[].environment.values
that accepts a mix of file pathes and inline dicts:The values files are loaded in the context of the parent state file. For example, in case the above state file is located at
/path/to/helmfile.yaml
,values.yaml
is located at/path/to/values.yaml
instead of/path/to/nested/values.yaml
.Resolves #523
I've also included the fix for #615 and the another fix that fixes the regression due to that. I've included all hese 3 commits into this one PR, because all of them correlate to each other(I think).