Skip to content
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

Deep merge of values files #1620

Closed
thomastaylor312 opened this issue Dec 1, 2016 · 3 comments
Closed

Deep merge of values files #1620

thomastaylor312 opened this issue Dec 1, 2016 · 3 comments
Milestone

Comments

@thomastaylor312
Copy link
Contributor

Helm should be able to deep merge multiple values files. This is helpful when you want to pass different variables for different environments (like whether or not to use a PVC) As it stands right now, it just overwrites the previous variable.

Example

Given these two values files for an example postgresql subchart

secrets.yaml

postgresql:
  postgresPassword: "super-secret-pass"

dev_environment.yaml

postgresql:
  persistence:
    enabled: false

Current Behavior
When using -f secrets.yaml -f dev_environment.yaml with helm install, the variables available would look like this:

postgresql:
  persistence:
    enabled: false

If we swap the flags: -f dev_environment.yaml -f secrets.yaml, the variables look like:

postgresql:
  postgresPassword: "super-secret-pass"

Desired Behavior
When using -f secrets.yaml -f dev_environment.yaml with helm install, the variables available would look like this:

postgresql:
  postgresPassword: "super-secret-pass"
  persistence:
    enabled: false

Is there another way to get this behavior or would this need to be added?

@thomastaylor312
Copy link
Contributor Author

Side note: I know that this would make the ordering of the files passed with the -f flag important, because if there are variables with the same name, one would overwrite the others.

@technosophos
Copy link
Member

I am not sure Cobra will allow us to repeat flags. But if we can figure out a way around that, I think we could just pass files through the existing -f/--set merge logic.

We might be able to change -f to accept a []string, which (IIRC) is expressed on CLI as -f foo.yaml,bar.yaml.

@technosophos technosophos added this to the 2.2.0-Triage milestone Dec 1, 2016
@thomastaylor312
Copy link
Contributor Author

@technosophos: Thanks for the quick response. It looks like it does (from the godoc example):

// Set is the method to set the flag value, part of the flag.Value interface.
// Set's argument is a string to be parsed to set the flag.
// It's a comma-separated list, so we split it.
func (i *interval) Set(value string) error {
    // If we wanted to allow the flag to be set multiple times,
    // accumulating values, we would delete this if statement.
    // That would permit usages such as
    //	-deltaT 10s -deltaT 15s
    // and other combinations.
    if len(*i) > 0 {
        return errors.New("interval flag already set")
    }
    for _, dt := range strings.Split(value, ",") {
        duration, err := time.ParseDuration(dt)
        if err != nil {
            return err
        }
        *i = append(*i, duration)
    }
    return nil
}

thomastaylor312 pushed a commit to thomastaylor312/helm that referenced this issue Dec 7, 2016
You can now specify the `-f` flag multiple times to include multiple
values files. The priority will be from left to right in the order
specified.

Closes helm#1620
thomastaylor312 pushed a commit to thomastaylor312/helm that referenced this issue Dec 7, 2016
You can now specify the `-f` flag multiple times to include multiple
values files. The priority will be from left to right in the order
specified.

Closes helm#1620
thomastaylor312 pushed a commit to thomastaylor312/helm that referenced this issue Dec 7, 2016
You can now specify the `-f` flag multiple times to include multiple
values files. The priority will be from left to right in the order
specified.

Closes helm#1620
thomastaylor312 pushed a commit to thomastaylor312/helm that referenced this issue Dec 8, 2016
You can now specify the `-f` flag multiple times to include multiple
values files. The priority will be given to the last (right-most)
file specified.

Closes helm#1620
thomastaylor312 pushed a commit to thomastaylor312/helm that referenced this issue Dec 9, 2016
You can now specify the `-f` flag multiple times to include multiple
values files. The priority will be given to the last (right-most)
file specified.

Closes helm#1620
@technosophos technosophos modified the milestones: 2.1.0, 2.3.0-Triage Dec 13, 2016
blampe added a commit to pulumi/pulumi-kubernetes that referenced this issue Apr 24, 2024
Stop relying on `yaml.Unmarshal` to merge `ValuesYaml` as it cannot do a
deep merge. Instead we Unmarshal the values in a temporary map and use
the `mergeMap` function that already exists to merge.

With this the behavior of the provider is aligned with the deep merge
made by Helm helm/helm#1620

Fixes #2958 .

---------

Co-authored-by: Bryce Lampe <bryce@pulumi.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants