From 68ee30b48cd0e34f2404f7ea56f491b34158edee Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Thu, 1 Aug 2019 11:04:36 -0400 Subject: [PATCH 1/3] cmd/*,pkg/*: move ValueOptions to cmd package and decouple from SDK Signed-off-by: Joe Lanford --- cmd/helm/install.go | 111 ++++++++++++++++++++++++++++--- cmd/helm/lint.go | 8 ++- cmd/helm/package.go | 8 ++- cmd/helm/template.go | 5 +- cmd/helm/upgrade.go | 11 ++-- pkg/action/install.go | 130 ++----------------------------------- pkg/action/install_test.go | 118 +++++++++------------------------ pkg/action/lint.go | 6 +- pkg/action/package.go | 6 +- pkg/action/upgrade.go | 32 ++++----- pkg/action/upgrade_test.go | 15 ++--- 11 files changed, 184 insertions(+), 266 deletions(-) diff --git a/cmd/helm/install.go b/cmd/helm/install.go index bfbe329cf..5ef9ba25d 100644 --- a/cmd/helm/install.go +++ b/cmd/helm/install.go @@ -18,19 +18,26 @@ package main import ( "io" + "io/ioutil" + "net/url" + "os" + "strings" "time" "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/spf13/pflag" + "sigs.k8s.io/yaml" "helm.sh/helm/cmd/helm/require" "helm.sh/helm/pkg/action" "helm.sh/helm/pkg/chart" "helm.sh/helm/pkg/chart/loader" + "helm.sh/helm/pkg/cli" "helm.sh/helm/pkg/downloader" "helm.sh/helm/pkg/getter" "helm.sh/helm/pkg/release" + "helm.sh/helm/pkg/strvals" ) const installDesc = ` @@ -97,6 +104,7 @@ charts in a repository, use 'helm search'. func newInstallCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { client := action.NewInstall(cfg) + valueOpts := &ValueOptions{} cmd := &cobra.Command{ Use: "install [NAME] [CHART]", @@ -104,7 +112,7 @@ func newInstallCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { Long: installDesc, Args: require.MinimumNArgs(1), RunE: func(_ *cobra.Command, args []string) error { - rel, err := runInstall(args, client, out) + rel, err := runInstall(args, client, valueOpts, out) if err != nil { return err } @@ -113,12 +121,12 @@ func newInstallCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { }, } - addInstallFlags(cmd.Flags(), client) + addInstallFlags(cmd.Flags(), client, valueOpts) return cmd } -func addInstallFlags(f *pflag.FlagSet, client *action.Install) { +func addInstallFlags(f *pflag.FlagSet, client *action.Install, valueOpts *ValueOptions) { f.BoolVar(&client.DryRun, "dry-run", false, "simulate an install") f.BoolVar(&client.DisableHooks, "no-hooks", false, "prevent hooks from running during install") f.BoolVar(&client.Replace, "replace", false, "re-use the given name, even if that name is already used. This is unsafe in production") @@ -129,11 +137,11 @@ func addInstallFlags(f *pflag.FlagSet, client *action.Install) { f.BoolVar(&client.Devel, "devel", false, "use development versions, too. Equivalent to version '>0.0.0-0'. If --version is set, this is ignored.") f.BoolVar(&client.DependencyUpdate, "dependency-update", false, "run helm dependency update before installing the chart") f.BoolVar(&client.Atomic, "atomic", false, "if set, installation process purges chart on fail. The --wait flag will be set automatically if --atomic is used") - addValueOptionsFlags(f, &client.ValueOptions) + addValueOptionsFlags(f, valueOpts) addChartPathOptionsFlags(f, &client.ChartPathOptions) } -func addValueOptionsFlags(f *pflag.FlagSet, v *action.ValueOptions) { +func addValueOptionsFlags(f *pflag.FlagSet, v *ValueOptions) { f.StringSliceVarP(&v.ValueFiles, "values", "f", []string{}, "specify values in a YAML file or a URL(can specify multiple)") f.StringArrayVar(&v.Values, "set", []string{}, "set values on the command line (can specify multiple or separate values with commas: key1=val1,key2=val2)") f.StringArrayVar(&v.StringValues, "set-string", []string{}, "set STRING values on the command line (can specify multiple or separate values with commas: key1=val1,key2=val2)") @@ -151,7 +159,7 @@ func addChartPathOptionsFlags(f *pflag.FlagSet, c *action.ChartPathOptions) { f.StringVar(&c.CaFile, "ca-file", "", "verify certificates of HTTPS-enabled servers using this CA bundle") } -func runInstall(args []string, client *action.Install, out io.Writer) (*release.Release, error) { +func runInstall(args []string, client *action.Install, valueOpts *ValueOptions, out io.Writer) (*release.Release, error) { debug("Original chart version: %q", client.Version) if client.Version == "" && client.Devel { debug("setting version to >0.0.0-0") @@ -171,7 +179,8 @@ func runInstall(args []string, client *action.Install, out io.Writer) (*release. debug("CHART PATH: %s\n", cp) - if err := client.ValueOptions.MergeValues(settings); err != nil { + vals, err := valueOpts.MergeValues(settings) + if err != nil { return nil, err } @@ -210,7 +219,7 @@ func runInstall(args []string, client *action.Install, out io.Writer) (*release. } client.Namespace = getNamespace() - return client.Run(chartRequested) + return client.Run(chartRequested, vals) } // isChartInstallable validates if a chart can be installed @@ -223,3 +232,89 @@ func isChartInstallable(ch *chart.Chart) (bool, error) { } return false, errors.Errorf("%s charts are not installable", ch.Metadata.Type) } + +type ValueOptions struct { + ValueFiles []string + StringValues []string + Values []string +} + +// MergeValues merges values from files specified via -f/--values and +// directly via --set or --set-string, marshaling them to YAML +func (v *ValueOptions) MergeValues(settings cli.EnvSettings) (map[string]interface{}, error) { + base := map[string]interface{}{} + + // User specified a values files via -f/--values + for _, filePath := range v.ValueFiles { + currentMap := map[string]interface{}{} + + bytes, err := readFile(filePath, settings) + if err != nil { + return nil, err + } + + if err := yaml.Unmarshal(bytes, ¤tMap); err != nil { + return nil, errors.Wrapf(err, "failed to parse %s", filePath) + } + // Merge with the previous map + base = mergeMaps(base, currentMap) + } + + // User specified a value via --set + for _, value := range v.Values { + if err := strvals.ParseInto(value, base); err != nil { + return nil, errors.Wrap(err, "failed parsing --set data") + } + } + + // User specified a value via --set-string + for _, value := range v.StringValues { + if err := strvals.ParseIntoString(value, base); err != nil { + return nil, errors.Wrap(err, "failed parsing --set-string data") + } + } + + return base, nil +} + +func mergeMaps(a, b map[string]interface{}) map[string]interface{} { + out := make(map[string]interface{}, len(a)) + for k, v := range a { + out[k] = v + } + for k, v := range b { + if v, ok := v.(map[string]interface{}); ok { + if bv, ok := out[k]; ok { + if bv, ok := bv.(map[string]interface{}); ok { + out[k] = mergeMaps(bv, v) + continue + } + } + } + out[k] = v + } + return out +} + +// readFile load a file from stdin, the local directory, or a remote file with a url. +func readFile(filePath string, settings cli.EnvSettings) ([]byte, error) { + if strings.TrimSpace(filePath) == "-" { + return ioutil.ReadAll(os.Stdin) + } + u, _ := url.Parse(filePath) + p := getter.All(settings) + + // FIXME: maybe someone handle other protocols like ftp. + getterConstructor, err := p.ByScheme(u.Scheme) + + if err != nil { + return ioutil.ReadFile(filePath) + } + + getter, err := getterConstructor(getter.WithURL(filePath)) + if err != nil { + return []byte{}, err + } + data, err := getter.Get(filePath) + return data.Bytes(), err +} diff --git a/cmd/helm/lint.go b/cmd/helm/lint.go index 65797e67b..cf8802668 100644 --- a/cmd/helm/lint.go +++ b/cmd/helm/lint.go @@ -38,6 +38,7 @@ or recommendation, it will emit [WARNING] messages. func newLintCmd(out io.Writer) *cobra.Command { client := action.NewLint() + valueOpts := &ValueOptions{} cmd := &cobra.Command{ Use: "lint PATH", @@ -49,10 +50,11 @@ func newLintCmd(out io.Writer) *cobra.Command { paths = args } client.Namespace = getNamespace() - if err := client.ValueOptions.MergeValues(settings); err != nil { + vals, err := valueOpts.MergeValues(settings) + if err != nil { return err } - result := client.Run(paths) + result := client.Run(paths, vals) var message strings.Builder fmt.Fprintf(&message, "%d chart(s) linted, %d chart(s) failed\n", result.TotalChartsLinted, len(result.Errors)) for _, err := range result.Errors { @@ -72,7 +74,7 @@ func newLintCmd(out io.Writer) *cobra.Command { f := cmd.Flags() f.BoolVar(&client.Strict, "strict", false, "fail on lint warnings") - addValueOptionsFlags(f, &client.ValueOptions) + addValueOptionsFlags(f, valueOpts) return cmd } diff --git a/cmd/helm/package.go b/cmd/helm/package.go index c33164a0a..f73ee9916 100644 --- a/cmd/helm/package.go +++ b/cmd/helm/package.go @@ -43,6 +43,7 @@ Versioned chart archives are used by Helm package repositories. func newPackageCmd(out io.Writer) *cobra.Command { client := action.NewPackage() + valueOpts := &ValueOptions{} cmd := &cobra.Command{ Use: "package [CHART_PATH] [...]", @@ -60,7 +61,8 @@ func newPackageCmd(out io.Writer) *cobra.Command { return errors.New("--keyring is required for signing a package") } } - if err := client.ValueOptions.MergeValues(settings); err != nil { + vals, err := valueOpts.MergeValues(settings) + if err != nil { return err } @@ -84,7 +86,7 @@ func newPackageCmd(out io.Writer) *cobra.Command { return err } } - p, err := client.Run(path) + p, err := client.Run(path, vals) if err != nil { return err } @@ -102,7 +104,7 @@ func newPackageCmd(out io.Writer) *cobra.Command { f.StringVar(&client.AppVersion, "app-version", "", "set the appVersion on the chart to this version") f.StringVarP(&client.Destination, "destination", "d", ".", "location to write the chart.") f.BoolVarP(&client.DependencyUpdate, "dependency-update", "u", false, `update dependencies from "Chart.yaml" to dir "charts/" before packaging`) - addValueOptionsFlags(f, &client.ValueOptions) + addValueOptionsFlags(f, valueOpts) return cmd } diff --git a/cmd/helm/template.go b/cmd/helm/template.go index 9fcbc2b10..47462e28a 100644 --- a/cmd/helm/template.go +++ b/cmd/helm/template.go @@ -39,6 +39,7 @@ is done. func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { var validate bool client := action.NewInstall(cfg) + valueOpts := &ValueOptions{} cmd := &cobra.Command{ Use: "template [NAME] [CHART]", @@ -50,7 +51,7 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { client.ReleaseName = "RELEASE-NAME" client.Replace = true // Skip the name check client.ClientOnly = !validate - rel, err := runInstall(args, client, out) + rel, err := runInstall(args, client, valueOpts, out) if err != nil { return err } @@ -60,7 +61,7 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { } f := cmd.Flags() - addInstallFlags(f, client) + addInstallFlags(f, client, valueOpts) f.StringVar(&client.OutputDir, "output-dir", "", "writes the executed templates to files in output-dir instead of stdout") f.BoolVar(&validate, "validate", false, "establish a connection to Kubernetes for schema validation") diff --git a/cmd/helm/upgrade.go b/cmd/helm/upgrade.go index 537660efd..9256a34f6 100644 --- a/cmd/helm/upgrade.go +++ b/cmd/helm/upgrade.go @@ -57,6 +57,7 @@ set for a key called 'foo', the 'newbar' value would take precedence: func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { client := action.NewUpgrade(cfg) + valueOpts := &ValueOptions{} cmd := &cobra.Command{ Use: "upgrade [RELEASE] [CHART]", @@ -71,7 +72,8 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { client.Version = ">0.0.0-0" } - if err := client.ValueOptions.MergeValues(settings); err != nil { + vals, err := valueOpts.MergeValues(settings) + if err != nil { return err } @@ -89,7 +91,6 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { fmt.Fprintf(out, "Release %q does not exist. Installing it now.\n", args[0]) instClient := action.NewInstall(cfg) instClient.ChartPathOptions = client.ChartPathOptions - instClient.ValueOptions = client.ValueOptions instClient.DryRun = client.DryRun instClient.DisableHooks = client.DisableHooks instClient.Timeout = client.Timeout @@ -98,7 +99,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { instClient.Namespace = client.Namespace instClient.Atomic = client.Atomic - _, err := runInstall(args, instClient, out) + _, err := runInstall(args, instClient, valueOpts, out) return err } } @@ -114,7 +115,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { } } - resp, err := client.Run(args[0], ch) + resp, err := client.Run(args[0], ch, vals) if err != nil { return errors.Wrap(err, "UPGRADE FAILED") } @@ -151,7 +152,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { f.BoolVar(&client.Atomic, "atomic", false, "if set, upgrade process rolls back changes made in case of failed upgrade. The --wait flag will be set automatically if --atomic is used") f.IntVar(&client.MaxHistory, "history-max", 0, "limit the maximum number of revisions saved per release. Use 0 for no limit.") addChartPathOptionsFlags(f, &client.ChartPathOptions) - addValueOptionsFlags(f, &client.ValueOptions) + addValueOptionsFlags(f, valueOpts) return cmd } diff --git a/pkg/action/install.go b/pkg/action/install.go index cab48c5ad..dd9b9349c 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -20,7 +20,6 @@ import ( "bytes" "fmt" "io/ioutil" - "net/url" "os" "path" "path/filepath" @@ -30,7 +29,6 @@ import ( "github.com/Masterminds/sprig" "github.com/pkg/errors" - "sigs.k8s.io/yaml" "helm.sh/helm/pkg/chart" "helm.sh/helm/pkg/chartutil" @@ -45,7 +43,6 @@ import ( "helm.sh/helm/pkg/repo" "helm.sh/helm/pkg/storage" "helm.sh/helm/pkg/storage/driver" - "helm.sh/helm/pkg/strvals" "helm.sh/helm/pkg/version" ) @@ -69,7 +66,6 @@ type Install struct { cfg *Configuration ChartPathOptions - ValueOptions ClientOnly bool DryRun bool @@ -87,13 +83,6 @@ type Install struct { Atomic bool } -type ValueOptions struct { - ValueFiles []string - StringValues []string - Values []string - rawValues map[string]interface{} -} - type ChartPathOptions struct { CaFile string // --ca-file CertFile string // --cert-file @@ -116,7 +105,7 @@ func NewInstall(cfg *Configuration) *Install { // Run executes the installation // // If DryRun is set to true, this will prepare the release, but not install it -func (i *Install) Run(chrt *chart.Chart) (*release.Release, error) { +func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release.Release, error) { if err := i.availableName(); err != nil { return nil, err } @@ -129,7 +118,7 @@ func (i *Install) Run(chrt *chart.Chart) (*release.Release, error) { i.cfg.Releases = storage.Init(driver.NewMemory()) } - if err := chartutil.ProcessDependencies(chrt, i.rawValues); err != nil { + if err := chartutil.ProcessDependencies(chrt, vals); err != nil { return nil, err } @@ -147,12 +136,12 @@ func (i *Install) Run(chrt *chart.Chart) (*release.Release, error) { Namespace: i.Namespace, IsInstall: true, } - valuesToRender, err := chartutil.ToRenderValues(chrt, i.rawValues, options, caps) + valuesToRender, err := chartutil.ToRenderValues(chrt, vals, options, caps) if err != nil { return nil, err } - rel := i.createRelease(chrt, i.rawValues) + rel := i.createRelease(chrt, vals) var manifestDoc *bytes.Buffer rel.Hooks, manifestDoc, rel.Info.Notes, err = i.cfg.renderResources(chrt, valuesToRender, i.OutputDir) // Even for errors, attach this if available @@ -641,114 +630,3 @@ func (c *ChartPathOptions) LocateChart(name string, settings cli.EnvSettings) (s return filename, errors.Errorf("failed to download %q (hint: running `helm repo update` may help)", name) } - -// MergeValues merges values from files specified via -f/--values and -// directly via --set or --set-string, marshaling them to YAML -func (v *ValueOptions) MergeValues(settings cli.EnvSettings) error { - base := map[string]interface{}{} - - // User specified a values files via -f/--values - for _, filePath := range v.ValueFiles { - currentMap := map[string]interface{}{} - - bytes, err := readFile(filePath, settings) - if err != nil { - return err - } - - if err := yaml.Unmarshal(bytes, ¤tMap); err != nil { - return errors.Wrapf(err, "failed to parse %s", filePath) - } - // Merge with the previous map - base = mergeMaps(base, currentMap) - } - - // User specified a value via --set - for _, value := range v.Values { - if err := strvals.ParseInto(value, base); err != nil { - return errors.Wrap(err, "failed parsing --set data") - } - } - - // User specified a value via --set-string - for _, value := range v.StringValues { - if err := strvals.ParseIntoString(value, base); err != nil { - return errors.Wrap(err, "failed parsing --set-string data") - } - } - - v.rawValues = base - return nil -} - -func NewValueOptions(values map[string]interface{}) ValueOptions { - return ValueOptions{ - rawValues: values, - } -} - -// mergeValues merges source and destination map, preferring values from the source map -func mergeValues(dest, src map[string]interface{}) map[string]interface{} { - out := make(map[string]interface{}) - for k, v := range dest { - out[k] = v - } - for k, v := range src { - if _, ok := out[k]; !ok { - // If the key doesn't exist already, then just set the key to that value - } else if nextMap, ok := v.(map[string]interface{}); !ok { - // If it isn't another map, overwrite the value - } else if destMap, isMap := out[k].(map[string]interface{}); !isMap { - // Edge case: If the key exists in the destination, but isn't a map - // If the source map has a map for this key, prefer it - } else { - // If we got to this point, it is a map in both, so merge them - out[k] = mergeValues(destMap, nextMap) - continue - } - out[k] = v - } - return out -} - -func mergeMaps(a, b map[string]interface{}) map[string]interface{} { - out := make(map[string]interface{}, len(a)) - for k, v := range a { - out[k] = v - } - for k, v := range b { - if v, ok := v.(map[string]interface{}); ok { - if bv, ok := out[k]; ok { - if bv, ok := bv.(map[string]interface{}); ok { - out[k] = mergeMaps(bv, v) - continue - } - } - } - out[k] = v - } - return out -} - -// readFile load a file from stdin, the local directory, or a remote file with a url. -func readFile(filePath string, settings cli.EnvSettings) ([]byte, error) { - if strings.TrimSpace(filePath) == "-" { - return ioutil.ReadAll(os.Stdin) - } - u, _ := url.Parse(filePath) - p := getter.All(settings) - - // FIXME: maybe someone handle other protocols like ftp. - getterConstructor, err := p.ByScheme(u.Scheme) - - if err != nil { - return ioutil.ReadFile(filePath) - } - - getter, err := getterConstructor(getter.WithURL(filePath)) - if err != nil { - return []byte{}, err - } - data, err := getter.Get(filePath) - return data.Bytes(), err -} diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index af8b28ee5..77b70baa0 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -22,7 +22,6 @@ import ( "log" "os" "path/filepath" - "reflect" "regexp" "testing" @@ -53,8 +52,8 @@ func installAction(t *testing.T) *Install { func TestInstallRelease(t *testing.T) { is := assert.New(t) instAction := installAction(t) - instAction.rawValues = map[string]interface{}{} - res, err := instAction.Run(buildChart()) + vals := map[string]interface{}{} + res, err := instAction.Run(buildChart(), vals) if err != nil { t.Fatalf("Failed install: %s", err) } @@ -79,7 +78,7 @@ func TestInstallReleaseClientOnly(t *testing.T) { is := assert.New(t) instAction := installAction(t) instAction.ClientOnly = true - instAction.Run(buildChart()) // disregard output + instAction.Run(buildChart(), nil) // disregard output is.Equal(instAction.cfg.Capabilities, chartutil.DefaultCapabilities) is.Equal(instAction.cfg.KubeClient, &kubefake.PrintingKubeClient{Out: ioutil.Discard}) @@ -88,8 +87,8 @@ func TestInstallReleaseClientOnly(t *testing.T) { func TestInstallRelease_NoName(t *testing.T) { instAction := installAction(t) instAction.ReleaseName = "" - instAction.rawValues = map[string]interface{}{} - _, err := instAction.Run(buildChart()) + vals := map[string]interface{}{} + _, err := instAction.Run(buildChart(), vals) if err == nil { t.Fatal("expected failure when no name is specified") } @@ -100,8 +99,8 @@ func TestInstallRelease_WithNotes(t *testing.T) { is := assert.New(t) instAction := installAction(t) instAction.ReleaseName = "with-notes" - instAction.rawValues = map[string]interface{}{} - res, err := instAction.Run(buildChart(withNotes("note here"))) + vals := map[string]interface{}{} + res, err := instAction.Run(buildChart(withNotes("note here")), vals) if err != nil { t.Fatalf("Failed install: %s", err) } @@ -127,8 +126,8 @@ func TestInstallRelease_WithNotesRendered(t *testing.T) { is := assert.New(t) instAction := installAction(t) instAction.ReleaseName = "with-notes" - instAction.rawValues = map[string]interface{}{} - res, err := instAction.Run(buildChart(withNotes("got-{{.Release.Name}}"))) + vals := map[string]interface{}{} + res, err := instAction.Run(buildChart(withNotes("got-{{.Release.Name}}")), vals) if err != nil { t.Fatalf("Failed install: %s", err) } @@ -146,8 +145,8 @@ func TestInstallRelease_WithChartAndDependencyNotes(t *testing.T) { is := assert.New(t) instAction := installAction(t) instAction.ReleaseName = "with-notes" - instAction.rawValues = map[string]interface{}{} - res, err := instAction.Run(buildChart(withNotes("parent"), withDependency(withNotes("child")))) + vals := map[string]interface{}{} + res, err := instAction.Run(buildChart(withNotes("parent"), withDependency(withNotes("child"))), vals) if err != nil { t.Fatalf("Failed install: %s", err) } @@ -163,8 +162,8 @@ func TestInstallRelease_DryRun(t *testing.T) { is := assert.New(t) instAction := installAction(t) instAction.DryRun = true - instAction.rawValues = map[string]interface{}{} - res, err := instAction.Run(buildChart(withSampleTemplates())) + vals := map[string]interface{}{} + res, err := instAction.Run(buildChart(withSampleTemplates()), vals) if err != nil { t.Fatalf("Failed install: %s", err) } @@ -189,8 +188,8 @@ func TestInstallRelease_NoHooks(t *testing.T) { instAction.ReleaseName = "no-hooks" instAction.cfg.Releases.Create(releaseStub()) - instAction.rawValues = map[string]interface{}{} - res, err := instAction.Run(buildChart()) + vals := map[string]interface{}{} + res, err := instAction.Run(buildChart(), vals) if err != nil { t.Fatalf("Failed install: %s", err) } @@ -206,8 +205,8 @@ func TestInstallRelease_FailedHooks(t *testing.T) { failer.WatchUntilReadyError = fmt.Errorf("Failed watch") instAction.cfg.KubeClient = failer - instAction.rawValues = map[string]interface{}{} - res, err := instAction.Run(buildChart()) + vals := map[string]interface{}{} + res, err := instAction.Run(buildChart(), vals) is.Error(err) is.Contains(res.Info.Description, "failed post-install") is.Equal(res.Info.Status, release.StatusFailed) @@ -223,8 +222,8 @@ func TestInstallRelease_ReplaceRelease(t *testing.T) { instAction.cfg.Releases.Create(rel) instAction.ReleaseName = rel.Name - instAction.rawValues = map[string]interface{}{} - res, err := instAction.Run(buildChart()) + vals := map[string]interface{}{} + res, err := instAction.Run(buildChart(), vals) is.NoError(err) // This should have been auto-incremented @@ -239,14 +238,14 @@ func TestInstallRelease_ReplaceRelease(t *testing.T) { func TestInstallRelease_KubeVersion(t *testing.T) { is := assert.New(t) instAction := installAction(t) - instAction.rawValues = map[string]interface{}{} - _, err := instAction.Run(buildChart(withKube(">=0.0.0"))) + vals := map[string]interface{}{} + _, err := instAction.Run(buildChart(withKube(">=0.0.0")), vals) is.NoError(err) // This should fail for a few hundred years instAction.ReleaseName = "should-fail" - instAction.rawValues = map[string]interface{}{} - _, err = instAction.Run(buildChart(withKube(">=99.0.0"))) + vals = map[string]interface{}{} + _, err = instAction.Run(buildChart(withKube(">=99.0.0")), vals) is.Error(err) is.Contains(err.Error(), "chart requires kubernetesVersion") } @@ -259,9 +258,9 @@ func TestInstallRelease_Wait(t *testing.T) { failer.WaitError = fmt.Errorf("I timed out") instAction.cfg.KubeClient = failer instAction.Wait = true - instAction.rawValues = map[string]interface{}{} + vals := map[string]interface{}{} - res, err := instAction.Run(buildChart()) + res, err := instAction.Run(buildChart(), vals) is.Error(err) is.Contains(res.Info.Description, "I timed out") is.Equal(res.Info.Status, release.StatusFailed) @@ -277,9 +276,9 @@ func TestInstallRelease_Atomic(t *testing.T) { failer.WaitError = fmt.Errorf("I timed out") instAction.cfg.KubeClient = failer instAction.Atomic = true - instAction.rawValues = map[string]interface{}{} + vals := map[string]interface{}{} - res, err := instAction.Run(buildChart()) + res, err := instAction.Run(buildChart(), vals) is.Error(err) is.Contains(err.Error(), "I timed out") is.Contains(err.Error(), "atomic") @@ -298,9 +297,9 @@ func TestInstallRelease_Atomic(t *testing.T) { failer.DeleteError = fmt.Errorf("uninstall fail") instAction.cfg.KubeClient = failer instAction.Atomic = true - instAction.rawValues = map[string]interface{}{} + vals := map[string]interface{}{} - _, err := instAction.Run(buildChart()) + _, err := instAction.Run(buildChart(), vals) is.Error(err) is.Contains(err.Error(), "I timed out") is.Contains(err.Error(), "uninstall fail") @@ -377,65 +376,10 @@ func TestNameTemplate(t *testing.T) { } } -func TestMergeValues(t *testing.T) { - nestedMap := map[string]interface{}{ - "foo": "bar", - "baz": map[string]string{ - "cool": "stuff", - }, - } - anotherNestedMap := map[string]interface{}{ - "foo": "bar", - "baz": map[string]string{ - "cool": "things", - "awesome": "stuff", - }, - } - flatMap := map[string]interface{}{ - "foo": "bar", - "baz": "stuff", - } - anotherFlatMap := map[string]interface{}{ - "testing": "fun", - } - - testMap := mergeValues(flatMap, nestedMap) - equal := reflect.DeepEqual(testMap, nestedMap) - if !equal { - t.Errorf("Expected a nested map to overwrite a flat value. Expected: %v, got %v", nestedMap, testMap) - } - - testMap = mergeValues(nestedMap, flatMap) - equal = reflect.DeepEqual(testMap, flatMap) - if !equal { - t.Errorf("Expected a flat value to overwrite a map. Expected: %v, got %v", flatMap, testMap) - } - - testMap = mergeValues(nestedMap, anotherNestedMap) - equal = reflect.DeepEqual(testMap, anotherNestedMap) - if !equal { - t.Errorf("Expected a nested map to overwrite another nested map. Expected: %v, got %v", anotherNestedMap, testMap) - } - - testMap = mergeValues(anotherFlatMap, anotherNestedMap) - expectedMap := map[string]interface{}{ - "testing": "fun", - "foo": "bar", - "baz": map[string]string{ - "cool": "things", - "awesome": "stuff", - }, - } - equal = reflect.DeepEqual(testMap, expectedMap) - if !equal { - t.Errorf("Expected a map with different keys to merge properly with another map. Expected: %v, got %v", expectedMap, testMap) - } -} - func TestInstallReleaseOutputDir(t *testing.T) { is := assert.New(t) instAction := installAction(t) - instAction.rawValues = map[string]interface{}{} + vals := map[string]interface{}{} dir, err := ioutil.TempDir("", "output-dir") if err != nil { @@ -445,7 +389,7 @@ func TestInstallReleaseOutputDir(t *testing.T) { instAction.OutputDir = dir - _, err = instAction.Run(buildChart(withSampleTemplates(), withMultipleManifestTemplate())) + _, err = instAction.Run(buildChart(withSampleTemplates(), withMultipleManifestTemplate()), vals) if err != nil { t.Fatalf("Failed install: %s", err) } diff --git a/pkg/action/lint.go b/pkg/action/lint.go index 5ec870570..d7ca8cd03 100644 --- a/pkg/action/lint.go +++ b/pkg/action/lint.go @@ -35,8 +35,6 @@ var errLintNoChart = errors.New("no chart found for linting (missing Chart.yaml) // // It provides the implementation of 'helm lint'. type Lint struct { - ValueOptions - Strict bool Namespace string } @@ -53,7 +51,7 @@ func NewLint() *Lint { } // Run executes 'helm Lint' against the given chart. -func (l *Lint) Run(paths []string) *LintResult { +func (l *Lint) Run(paths []string, vals map[string]interface{}) *LintResult { lowestTolerance := support.ErrorSev if l.Strict { lowestTolerance = support.WarningSev @@ -61,7 +59,7 @@ func (l *Lint) Run(paths []string) *LintResult { result := &LintResult{} for _, path := range paths { - if linter, err := lintChart(path, l.ValueOptions.rawValues, l.Namespace, l.Strict); err != nil { + if linter, err := lintChart(path, vals, l.Namespace, l.Strict); err != nil { if err == errLintNoChart { result.Errors = append(result.Errors, err) } diff --git a/pkg/action/package.go b/pkg/action/package.go index c3208b44b..5deb15a5f 100644 --- a/pkg/action/package.go +++ b/pkg/action/package.go @@ -36,8 +36,6 @@ import ( // // It provides the implementation of 'helm package'. type Package struct { - ValueOptions - Sign bool Key string Keyring string @@ -53,13 +51,13 @@ func NewPackage() *Package { } // Run executes 'helm package' against the given chart and returns the path to the packaged chart. -func (p *Package) Run(path string) (string, error) { +func (p *Package) Run(path string, vals map[string]interface{}) (string, error) { ch, err := loader.LoadDir(path) if err != nil { return "", err } - combinedVals, err := chartutil.CoalesceValues(ch, p.ValueOptions.rawValues) + combinedVals, err := chartutil.CoalesceValues(ch, vals) if err != nil { return "", err } diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 6189429fd..918c55619 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -39,7 +39,6 @@ type Upgrade struct { cfg *Configuration ChartPathOptions - ValueOptions Install bool Devel bool @@ -66,8 +65,8 @@ func NewUpgrade(cfg *Configuration) *Upgrade { } // Run executes the upgrade on the given release. -func (u *Upgrade) Run(name string, chart *chart.Chart) (*release.Release, error) { - if err := chartutil.ProcessDependencies(chart, u.rawValues); err != nil { +func (u *Upgrade) Run(name string, chart *chart.Chart, vals map[string]interface{}) (*release.Release, error) { + if err := chartutil.ProcessDependencies(chart, vals); err != nil { return nil, err } @@ -79,7 +78,7 @@ func (u *Upgrade) Run(name string, chart *chart.Chart) (*release.Release, error) return nil, errors.Errorf("release name is invalid: %s", name) } u.cfg.Log("preparing upgrade for %s", name) - currentRelease, upgradedRelease, err := u.prepareUpgrade(name, chart) + currentRelease, upgradedRelease, err := u.prepareUpgrade(name, chart, vals) if err != nil { return nil, err } @@ -122,7 +121,7 @@ func validateReleaseName(releaseName string) error { } // prepareUpgrade builds an upgraded release for an upgrade operation. -func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart) (*release.Release, *release.Release, error) { +func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart, vals map[string]interface{}) (*release.Release, *release.Release, error) { if chart == nil { return nil, nil, errMissingChart } @@ -134,7 +133,8 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart) (*release.Rele } // determine if values will be reused - if err := u.reuseValues(chart, currentRelease); err != nil { + vals, err = u.reuseValues(chart, currentRelease, vals) + if err != nil { return nil, nil, err } @@ -158,7 +158,7 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart) (*release.Rele if err != nil { return nil, nil, err } - valuesToRender, err := chartutil.ToRenderValues(chart, u.rawValues, options, caps) + valuesToRender, err := chartutil.ToRenderValues(chart, vals, options, caps) if err != nil { return nil, nil, err } @@ -173,7 +173,7 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart) (*release.Rele Name: name, Namespace: currentRelease.Namespace, Chart: chart, - Config: u.rawValues, + Config: vals, Info: &release.Info{ FirstDeployed: currentRelease.Info.FirstDeployed, LastDeployed: Timestamper(), @@ -310,11 +310,11 @@ func (u *Upgrade) failRelease(rel *release.Release, err error) (*release.Release // // This is skipped if the u.ResetValues flag is set, in which case the // request values are not altered. -func (u *Upgrade) reuseValues(chart *chart.Chart, current *release.Release) error { +func (u *Upgrade) reuseValues(chart *chart.Chart, current *release.Release, newVals map[string]interface{}) (map[string]interface{}, error) { if u.ResetValues { // If ResetValues is set, we completely ignore current.Config. u.cfg.Log("resetting values to the chart's original version") - return nil + return newVals, nil } // If the ReuseValues flag is set, we always copy the old values over the new config's values. @@ -324,21 +324,21 @@ func (u *Upgrade) reuseValues(chart *chart.Chart, current *release.Release) erro // We have to regenerate the old coalesced values: oldVals, err := chartutil.CoalesceValues(current.Chart, current.Config) if err != nil { - return errors.Wrap(err, "failed to rebuild old values") + return nil, errors.Wrap(err, "failed to rebuild old values") } - u.rawValues = chartutil.CoalesceTables(u.rawValues, current.Config) + newVals = chartutil.CoalesceTables(newVals, current.Config) chart.Values = oldVals - return nil + return newVals, nil } - if len(u.rawValues) == 0 && len(current.Config) > 0 { + if len(newVals) == 0 && len(current.Config) > 0 { u.cfg.Log("copying values from %s (v%d) to new release.", current.Name, current.Version) - u.rawValues = current.Config + newVals = current.Config } - return nil + return newVals, nil } func validateManifest(c kube.Interface, manifest []byte) error { diff --git a/pkg/action/upgrade_test.go b/pkg/action/upgrade_test.go index 5e1913c15..81004e907 100644 --- a/pkg/action/upgrade_test.go +++ b/pkg/action/upgrade_test.go @@ -49,9 +49,9 @@ func TestUpgradeRelease_Wait(t *testing.T) { failer.WaitError = fmt.Errorf("I timed out") upAction.cfg.KubeClient = failer upAction.Wait = true - upAction.rawValues = map[string]interface{}{} + vals := map[string]interface{}{} - res, err := upAction.Run(rel.Name, buildChart()) + res, err := upAction.Run(rel.Name, buildChart(), vals) req.Error(err) is.Contains(res.Info.Description, "I timed out") is.Equal(res.Info.Status, release.StatusFailed) @@ -74,9 +74,9 @@ func TestUpgradeRelease_Atomic(t *testing.T) { failer.WatchUntilReadyError = fmt.Errorf("arming key removed") upAction.cfg.KubeClient = failer upAction.Atomic = true - upAction.rawValues = map[string]interface{}{} + vals := map[string]interface{}{} - res, err := upAction.Run(rel.Name, buildChart()) + res, err := upAction.Run(rel.Name, buildChart(), vals) req.Error(err) is.Contains(err.Error(), "arming key removed") is.Contains(err.Error(), "atomic") @@ -99,9 +99,9 @@ func TestUpgradeRelease_Atomic(t *testing.T) { failer.UpdateError = fmt.Errorf("update fail") upAction.cfg.KubeClient = failer upAction.Atomic = true - upAction.rawValues = map[string]interface{}{} + vals := map[string]interface{}{} - _, err := upAction.Run(rel.Name, buildChart()) + _, err := upAction.Run(rel.Name, buildChart(), vals) req.Error(err) is.Contains(err.Error(), "update fail") is.Contains(err.Error(), "an error occurred while rolling back the release") @@ -141,8 +141,7 @@ func TestUpgradeRelease_ReuseValues(t *testing.T) { upAction.ReuseValues = true // setting newValues and upgrading - upAction.rawValues = newValues - res, err := upAction.Run(rel.Name, buildChart()) + res, err := upAction.Run(rel.Name, buildChart(), newValues) is.NoError(err) // Now make sure it is actually upgraded From 8a4b70b1e34dbbf51c567ec4deb2b713ce0a4169 Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Thu, 1 Aug 2019 17:40:52 -0400 Subject: [PATCH 2/3] review: move ValueOptions to SDK Signed-off-by: Joe Lanford --- cmd/helm/install.go | 102 ++------------------------------- cmd/helm/lint.go | 3 +- cmd/helm/package.go | 3 +- cmd/helm/template.go | 3 +- cmd/helm/upgrade.go | 3 +- pkg/cli/values/options.go | 117 ++++++++++++++++++++++++++++++++++++++ 6 files changed, 130 insertions(+), 101 deletions(-) create mode 100644 pkg/cli/values/options.go diff --git a/cmd/helm/install.go b/cmd/helm/install.go index 5ef9ba25d..da6e13b14 100644 --- a/cmd/helm/install.go +++ b/cmd/helm/install.go @@ -18,26 +18,20 @@ package main import ( "io" - "io/ioutil" - "net/url" - "os" - "strings" "time" "github.com/pkg/errors" "github.com/spf13/cobra" "github.com/spf13/pflag" - "sigs.k8s.io/yaml" "helm.sh/helm/cmd/helm/require" "helm.sh/helm/pkg/action" "helm.sh/helm/pkg/chart" "helm.sh/helm/pkg/chart/loader" - "helm.sh/helm/pkg/cli" + "helm.sh/helm/pkg/cli/values" "helm.sh/helm/pkg/downloader" "helm.sh/helm/pkg/getter" "helm.sh/helm/pkg/release" - "helm.sh/helm/pkg/strvals" ) const installDesc = ` @@ -104,7 +98,7 @@ charts in a repository, use 'helm search'. func newInstallCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { client := action.NewInstall(cfg) - valueOpts := &ValueOptions{} + valueOpts := &values.Options{} cmd := &cobra.Command{ Use: "install [NAME] [CHART]", @@ -126,7 +120,7 @@ func newInstallCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { return cmd } -func addInstallFlags(f *pflag.FlagSet, client *action.Install, valueOpts *ValueOptions) { +func addInstallFlags(f *pflag.FlagSet, client *action.Install, valueOpts *values.Options) { f.BoolVar(&client.DryRun, "dry-run", false, "simulate an install") f.BoolVar(&client.DisableHooks, "no-hooks", false, "prevent hooks from running during install") f.BoolVar(&client.Replace, "replace", false, "re-use the given name, even if that name is already used. This is unsafe in production") @@ -141,7 +135,7 @@ func addInstallFlags(f *pflag.FlagSet, client *action.Install, valueOpts *ValueO addChartPathOptionsFlags(f, &client.ChartPathOptions) } -func addValueOptionsFlags(f *pflag.FlagSet, v *ValueOptions) { +func addValueOptionsFlags(f *pflag.FlagSet, v *values.Options) { f.StringSliceVarP(&v.ValueFiles, "values", "f", []string{}, "specify values in a YAML file or a URL(can specify multiple)") f.StringArrayVar(&v.Values, "set", []string{}, "set values on the command line (can specify multiple or separate values with commas: key1=val1,key2=val2)") f.StringArrayVar(&v.StringValues, "set-string", []string{}, "set STRING values on the command line (can specify multiple or separate values with commas: key1=val1,key2=val2)") @@ -159,7 +153,7 @@ func addChartPathOptionsFlags(f *pflag.FlagSet, c *action.ChartPathOptions) { f.StringVar(&c.CaFile, "ca-file", "", "verify certificates of HTTPS-enabled servers using this CA bundle") } -func runInstall(args []string, client *action.Install, valueOpts *ValueOptions, out io.Writer) (*release.Release, error) { +func runInstall(args []string, client *action.Install, valueOpts *values.Options, out io.Writer) (*release.Release, error) { debug("Original chart version: %q", client.Version) if client.Version == "" && client.Devel { debug("setting version to >0.0.0-0") @@ -232,89 +226,3 @@ func isChartInstallable(ch *chart.Chart) (bool, error) { } return false, errors.Errorf("%s charts are not installable", ch.Metadata.Type) } - -type ValueOptions struct { - ValueFiles []string - StringValues []string - Values []string -} - -// MergeValues merges values from files specified via -f/--values and -// directly via --set or --set-string, marshaling them to YAML -func (v *ValueOptions) MergeValues(settings cli.EnvSettings) (map[string]interface{}, error) { - base := map[string]interface{}{} - - // User specified a values files via -f/--values - for _, filePath := range v.ValueFiles { - currentMap := map[string]interface{}{} - - bytes, err := readFile(filePath, settings) - if err != nil { - return nil, err - } - - if err := yaml.Unmarshal(bytes, ¤tMap); err != nil { - return nil, errors.Wrapf(err, "failed to parse %s", filePath) - } - // Merge with the previous map - base = mergeMaps(base, currentMap) - } - - // User specified a value via --set - for _, value := range v.Values { - if err := strvals.ParseInto(value, base); err != nil { - return nil, errors.Wrap(err, "failed parsing --set data") - } - } - - // User specified a value via --set-string - for _, value := range v.StringValues { - if err := strvals.ParseIntoString(value, base); err != nil { - return nil, errors.Wrap(err, "failed parsing --set-string data") - } - } - - return base, nil -} - -func mergeMaps(a, b map[string]interface{}) map[string]interface{} { - out := make(map[string]interface{}, len(a)) - for k, v := range a { - out[k] = v - } - for k, v := range b { - if v, ok := v.(map[string]interface{}); ok { - if bv, ok := out[k]; ok { - if bv, ok := bv.(map[string]interface{}); ok { - out[k] = mergeMaps(bv, v) - continue - } - } - } - out[k] = v - } - return out -} - -// readFile load a file from stdin, the local directory, or a remote file with a url. -func readFile(filePath string, settings cli.EnvSettings) ([]byte, error) { - if strings.TrimSpace(filePath) == "-" { - return ioutil.ReadAll(os.Stdin) - } - u, _ := url.Parse(filePath) - p := getter.All(settings) - - // FIXME: maybe someone handle other protocols like ftp. - getterConstructor, err := p.ByScheme(u.Scheme) - - if err != nil { - return ioutil.ReadFile(filePath) - } - - getter, err := getterConstructor(getter.WithURL(filePath)) - if err != nil { - return []byte{}, err - } - data, err := getter.Get(filePath) - return data.Bytes(), err -} diff --git a/cmd/helm/lint.go b/cmd/helm/lint.go index cf8802668..bab1ebb7c 100644 --- a/cmd/helm/lint.go +++ b/cmd/helm/lint.go @@ -25,6 +25,7 @@ import ( "github.com/spf13/cobra" "helm.sh/helm/pkg/action" + "helm.sh/helm/pkg/cli/values" ) var longLintHelp = ` @@ -38,7 +39,7 @@ or recommendation, it will emit [WARNING] messages. func newLintCmd(out io.Writer) *cobra.Command { client := action.NewLint() - valueOpts := &ValueOptions{} + valueOpts := &values.Options{} cmd := &cobra.Command{ Use: "lint PATH", diff --git a/cmd/helm/package.go b/cmd/helm/package.go index f73ee9916..48fcace1f 100644 --- a/cmd/helm/package.go +++ b/cmd/helm/package.go @@ -26,6 +26,7 @@ import ( "github.com/spf13/cobra" "helm.sh/helm/pkg/action" + "helm.sh/helm/pkg/cli/values" "helm.sh/helm/pkg/downloader" "helm.sh/helm/pkg/getter" ) @@ -43,7 +44,7 @@ Versioned chart archives are used by Helm package repositories. func newPackageCmd(out io.Writer) *cobra.Command { client := action.NewPackage() - valueOpts := &ValueOptions{} + valueOpts := &values.Options{} cmd := &cobra.Command{ Use: "package [CHART_PATH] [...]", diff --git a/cmd/helm/template.go b/cmd/helm/template.go index 47462e28a..babfe0eac 100644 --- a/cmd/helm/template.go +++ b/cmd/helm/template.go @@ -25,6 +25,7 @@ import ( "helm.sh/helm/cmd/helm/require" "helm.sh/helm/pkg/action" + "helm.sh/helm/pkg/cli/values" ) const templateDesc = ` @@ -39,7 +40,7 @@ is done. func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { var validate bool client := action.NewInstall(cfg) - valueOpts := &ValueOptions{} + valueOpts := &values.Options{} cmd := &cobra.Command{ Use: "template [NAME] [CHART]", diff --git a/cmd/helm/upgrade.go b/cmd/helm/upgrade.go index 9256a34f6..87d279950 100644 --- a/cmd/helm/upgrade.go +++ b/cmd/helm/upgrade.go @@ -27,6 +27,7 @@ import ( "helm.sh/helm/cmd/helm/require" "helm.sh/helm/pkg/action" "helm.sh/helm/pkg/chart/loader" + "helm.sh/helm/pkg/cli/values" "helm.sh/helm/pkg/storage/driver" ) @@ -57,7 +58,7 @@ set for a key called 'foo', the 'newbar' value would take precedence: func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { client := action.NewUpgrade(cfg) - valueOpts := &ValueOptions{} + valueOpts := &values.Options{} cmd := &cobra.Command{ Use: "upgrade [RELEASE] [CHART]", diff --git a/pkg/cli/values/options.go b/pkg/cli/values/options.go new file mode 100644 index 000000000..86e83ab76 --- /dev/null +++ b/pkg/cli/values/options.go @@ -0,0 +1,117 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package values + +import ( + "io/ioutil" + "net/url" + "os" + "strings" + + "github.com/pkg/errors" + "sigs.k8s.io/yaml" + + "helm.sh/helm/pkg/cli" + "helm.sh/helm/pkg/getter" + "helm.sh/helm/pkg/strvals" +) + +type Options struct { + ValueFiles []string + StringValues []string + Values []string +} + +// MergeValues merges values from files specified via -f/--values and +// directly via --set or --set-string, marshaling them to YAML +func (opts *Options) MergeValues(settings cli.EnvSettings) (map[string]interface{}, error) { + base := map[string]interface{}{} + + // User specified a values files via -f/--values + for _, filePath := range opts.ValueFiles { + currentMap := map[string]interface{}{} + + bytes, err := readFile(filePath, settings) + if err != nil { + return nil, err + } + + if err := yaml.Unmarshal(bytes, ¤tMap); err != nil { + return nil, errors.Wrapf(err, "failed to parse %s", filePath) + } + // Merge with the previous map + base = mergeMaps(base, currentMap) + } + + // User specified a value via --set + for _, value := range opts.Values { + if err := strvals.ParseInto(value, base); err != nil { + return nil, errors.Wrap(err, "failed parsing --set data") + } + } + + // User specified a value via --set-string + for _, value := range opts.StringValues { + if err := strvals.ParseIntoString(value, base); err != nil { + return nil, errors.Wrap(err, "failed parsing --set-string data") + } + } + + return base, nil +} + +func mergeMaps(a, b map[string]interface{}) map[string]interface{} { + out := make(map[string]interface{}, len(a)) + for k, v := range a { + out[k] = v + } + for k, v := range b { + if v, ok := v.(map[string]interface{}); ok { + if bv, ok := out[k]; ok { + if bv, ok := bv.(map[string]interface{}); ok { + out[k] = mergeMaps(bv, v) + continue + } + } + } + out[k] = v + } + return out +} + +// readFile load a file from stdin, the local directory, or a remote file with a url. +func readFile(filePath string, settings cli.EnvSettings) ([]byte, error) { + if strings.TrimSpace(filePath) == "-" { + return ioutil.ReadAll(os.Stdin) + } + u, _ := url.Parse(filePath) + p := getter.All(settings) + + // FIXME: maybe someone handle other protocols like ftp. + getterConstructor, err := p.ByScheme(u.Scheme) + + if err != nil { + return ioutil.ReadFile(filePath) + } + + getter, err := getterConstructor(getter.WithURL(filePath)) + if err != nil { + return []byte{}, err + } + data, err := getter.Get(filePath) + return data.Bytes(), err +} From 45f697b5075584e07efa2393b20062f0155dde18 Mon Sep 17 00:00:00 2001 From: Joe Lanford Date: Mon, 5 Aug 2019 22:58:51 -0400 Subject: [PATCH 3/3] pkg/cli/values/options_test.go: re-add MergeValues test with mergeMaps Signed-off-by: Joe Lanford --- pkg/cli/values/options_test.go | 77 ++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) create mode 100644 pkg/cli/values/options_test.go diff --git a/pkg/cli/values/options_test.go b/pkg/cli/values/options_test.go new file mode 100644 index 000000000..d988274bf --- /dev/null +++ b/pkg/cli/values/options_test.go @@ -0,0 +1,77 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package values + +import ( + "reflect" + "testing" +) + +func TestMergeValues(t *testing.T) { + nestedMap := map[string]interface{}{ + "foo": "bar", + "baz": map[string]string{ + "cool": "stuff", + }, + } + anotherNestedMap := map[string]interface{}{ + "foo": "bar", + "baz": map[string]string{ + "cool": "things", + "awesome": "stuff", + }, + } + flatMap := map[string]interface{}{ + "foo": "bar", + "baz": "stuff", + } + anotherFlatMap := map[string]interface{}{ + "testing": "fun", + } + + testMap := mergeMaps(flatMap, nestedMap) + equal := reflect.DeepEqual(testMap, nestedMap) + if !equal { + t.Errorf("Expected a nested map to overwrite a flat value. Expected: %v, got %v", nestedMap, testMap) + } + + testMap = mergeMaps(nestedMap, flatMap) + equal = reflect.DeepEqual(testMap, flatMap) + if !equal { + t.Errorf("Expected a flat value to overwrite a map. Expected: %v, got %v", flatMap, testMap) + } + + testMap = mergeMaps(nestedMap, anotherNestedMap) + equal = reflect.DeepEqual(testMap, anotherNestedMap) + if !equal { + t.Errorf("Expected a nested map to overwrite another nested map. Expected: %v, got %v", anotherNestedMap, testMap) + } + + testMap = mergeMaps(anotherFlatMap, anotherNestedMap) + expectedMap := map[string]interface{}{ + "testing": "fun", + "foo": "bar", + "baz": map[string]string{ + "cool": "things", + "awesome": "stuff", + }, + } + equal = reflect.DeepEqual(testMap, expectedMap) + if !equal { + t.Errorf("Expected a map with different keys to merge properly with another map. Expected: %v, got %v", expectedMap, testMap) + } +}