From b1239a69d1df411ec0134c63f4a18b5c639977ad Mon Sep 17 00:00:00 2001 From: Bhargav Date: Tue, 1 Oct 2024 04:10:39 +0530 Subject: [PATCH] feat(values): Add Support for Values Directory New CLI flag --values-directory (or) -d for loading directory name which has the values YAML files. This flag will be used by the following commands: 1. Install 2. Lint 3. Package 4. Upgrade The values directory(s) (or the values inside the values YAML files inside the specified directory(s)) are processed first. i.e., values from other input types (--values, --set-json, --set, --set-string, --set-file, --set-literal) can all overwrite values from directories if matching keys. Fixes #10416 Signed-off-by: Bhargav Ravuri --- cmd/helm/flags.go | 1 + pkg/cli/values/options.go | 77 +++++++-- pkg/cli/values/options_test.go | 146 +++++++++++++++++- .../values.d/non-power-users.yaml | 7 + .../values.d/power-users.yaml | 5 + .../testdata/multi-level-values-dir/ship.yaml | 1 + .../values.d/extras/crew.yaml | 1 + .../values.d/extras/ship.yaml | 1 + .../values.d/non-power-users.yaml | 7 + .../values.d/power-users.yaml | 5 + 10 files changed, 240 insertions(+), 11 deletions(-) create mode 100644 pkg/cli/values/testdata/chart-with-values-dir/values.d/non-power-users.yaml create mode 100644 pkg/cli/values/testdata/chart-with-values-dir/values.d/power-users.yaml create mode 100644 pkg/cli/values/testdata/multi-level-values-dir/ship.yaml create mode 100644 pkg/cli/values/testdata/multi-level-values-dir/values.d/extras/crew.yaml create mode 100644 pkg/cli/values/testdata/multi-level-values-dir/values.d/extras/ship.yaml create mode 100644 pkg/cli/values/testdata/multi-level-values-dir/values.d/non-power-users.yaml create mode 100644 pkg/cli/values/testdata/multi-level-values-dir/values.d/power-users.yaml diff --git a/cmd/helm/flags.go b/cmd/helm/flags.go index 62e9f90fa6b..12deabbd0e5 100644 --- a/cmd/helm/flags.go +++ b/cmd/helm/flags.go @@ -44,6 +44,7 @@ const ( 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.StringSliceVarP(&v.ValuesDirectories, "values-directory", "d", []string{}, "specify values directory to recursively read for value's YAML files. Note: The YAML files in the directory are read in the lexical order. (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)") f.StringArrayVar(&v.FileValues, "set-file", []string{}, "set values from respective files specified via the command line (can specify multiple or separate values with commas: key1=path1,key2=path2)") diff --git a/pkg/cli/values/options.go b/pkg/cli/values/options.go index 06631cd33d9..a9c972d3cb8 100644 --- a/pkg/cli/values/options.go +++ b/pkg/cli/values/options.go @@ -18,8 +18,10 @@ package values import ( "io" + "io/fs" "net/url" "os" + "path/filepath" "strings" "github.com/pkg/errors" @@ -31,21 +33,48 @@ import ( // Options captures the different ways to specify values type Options struct { - ValueFiles []string // -f/--values - StringValues []string // --set-string - Values []string // --set - FileValues []string // --set-file - JSONValues []string // --set-json - LiteralValues []string // --set-literal + ValueFiles []string // -f/--values + ValuesDirectories []string // -d/--values-directory + StringValues []string // --set-string + Values []string // --set + FileValues []string // --set-file + JSONValues []string // --set-json + LiteralValues []string // --set-literal } -// MergeValues merges values from files specified via -f/--values and directly -// via --set-json, --set, --set-string, or --set-file, marshaling them to YAML +// MergeValues merges values specified via any of the following flags, and marshals them to YAML: +// 1. -d/--values-directory - from values file(s) in the directory(s) +// 2. -f/--values - from values file(s) or URL +// 3. --set-json - from input JSON +// 4. --set - from input key-value pairs +// 5. --set-string - from input key-value pairs, with string values, always +// 6. --set-file - from files +// 7. --set-literal - from input string literal +// +// The precedence order of inputs are 1 to 7, where 1 gets evaluated first and 7 last. i.e., If key1="val1" in inputs +// from --values-directory, and key1="val2" in --values, the second overwrites the first and the final value of key1 +// is "val2". Similarly values from --set-json are replaced from that of --values, and so on. func (opts *Options) MergeValues(p getter.Providers) (map[string]interface{}, error) { base := map[string]interface{}{} - // User specified a values files via -f/--values - for _, filePath := range opts.ValueFiles { + var valuesFiles []string + + // User specified directory(s) via -d/--values-directory + for _, dir := range opts.ValuesDirectories { + // Recursive list of YAML files in input values directory + files, err := recursiveListOfFilesInDir(dir, `.yaml`) + if err != nil { + // Error already wrapped + return nil, err + } + + valuesFiles = append(valuesFiles, files...) + } + + // User specified values files via -f/--values + valuesFiles = append(valuesFiles, opts.ValueFiles...) + + for _, filePath := range valuesFiles { currentMap := map[string]interface{}{} bytes, err := readFile(filePath, p) @@ -145,3 +174,31 @@ func readFile(filePath string, p getter.Providers) ([]byte, error) { } return data.Bytes(), err } + +// recursiveListOfFilesInDir lists the directory recursively, i.e., files in all nested directories. +// The list can be filtered by file extension. If no extension is specified, it returns all files. +// +// Result format: [/, ..., // ...] +func recursiveListOfFilesInDir(directory, extension string) ([]string, error) { + var files []string + + // Traverse through the directory, recursively + err := filepath.WalkDir(directory, func(path string, file fs.DirEntry, err error) error { + // Check if accessing the file failed + if err != nil { + return errors.Wrapf(err, "failed to read info of file %q", path) + } + + // When the file has the required extension, or when extension is not specified + if !file.IsDir() && (extension == "" || filepath.Ext(path) == extension) { + files = append(files, path) + } + + return nil + }) + if err != nil { + return nil, errors.Wrapf(err, "failed to recursively list files in directory %q", directory) + } + + return files, nil +} diff --git a/pkg/cli/values/options_test.go b/pkg/cli/values/options_test.go index 54124c0fa42..aac4e12f224 100644 --- a/pkg/cli/values/options_test.go +++ b/pkg/cli/values/options_test.go @@ -23,7 +23,7 @@ import ( "helm.sh/helm/v3/pkg/getter" ) -func TestMergeValues(t *testing.T) { +func Test_mergeMaps(t *testing.T) { nestedMap := map[string]interface{}{ "foo": "bar", "baz": map[string]string{ @@ -86,3 +86,147 @@ func TestReadFile(t *testing.T) { t.Errorf("Expected error when has special strings") } } + +func TestOptions_MergeValues(t *testing.T) { + const ( + crewNameKey = `crew` + shipNameKey = `ship` + powerUserskey = `power-users` + nonPowerUsersKey = `non-power-users` + strawHatsCrew = `Straw Hat Pirates` + strawHatsShip1 = `Going Merry` + strawHatsShip2 = `Thousand Sunny` + ) + + var ( + powerUsersVal = []interface{}{ + "Luffy", + "Chopper", + "Robin", + "Brook", + } + nonPowerUsersVal = []interface{}{ + "Zoro", + "Nami", + "Ussop", + "Sanji", + "Franky", + "Jinbei", + } + ) + + type args struct { + p getter.Providers + } + tests := []struct { + name string + opts Options + args args + want map[string]interface{} + wantErr bool + }{ + { + name: "--values-directory with single level", + opts: Options{ + ValueFiles: []string{}, + ValuesDirectories: []string{ + "testdata/chart-with-values-dir/values.d", + }, + StringValues: []string{}, + Values: []string{}, + FileValues: []string{}, + JSONValues: []string{}, + }, + args: args{ + p: []getter.Provider{}, + }, + want: map[string]interface{}{ + powerUserskey: powerUsersVal, + nonPowerUsersKey: nonPowerUsersVal, + }, + wantErr: false, + }, + { + name: "--values-directory with nested directories", + opts: Options{ + ValueFiles: []string{}, + ValuesDirectories: []string{ + "testdata/multi-level-values-dir/values.d", + }, + StringValues: []string{}, + Values: []string{}, + FileValues: []string{}, + JSONValues: []string{}, + }, + args: args{ + p: []getter.Provider{}, + }, + want: map[string]interface{}{ + crewNameKey: strawHatsCrew, + shipNameKey: strawHatsShip1, + powerUserskey: powerUsersVal, + nonPowerUsersKey: nonPowerUsersVal, + }, + wantErr: false, + }, + { + name: "--values-directory value overwritten by --values", + opts: Options{ + ValueFiles: []string{ + "testdata/multi-level-values-dir/ship.yaml", + }, + ValuesDirectories: []string{ + "testdata/multi-level-values-dir/values.d", + }, + StringValues: []string{}, + Values: []string{}, + FileValues: []string{}, + JSONValues: []string{}, + }, + args: args{ + p: []getter.Provider{}, + }, + want: map[string]interface{}{ + crewNameKey: strawHatsCrew, + shipNameKey: strawHatsShip2, // This is the value overwritten by values file "ship.yaml" + powerUserskey: powerUsersVal, + nonPowerUsersKey: nonPowerUsersVal, + }, + wantErr: false, + }, + { + name: "--values-directory with missing directory", + opts: Options{ + ValueFiles: []string{}, + ValuesDirectories: []string{ + "testdata/chart-with-values-dir/non-existing/", + }, + StringValues: []string{}, + Values: []string{}, + FileValues: []string{}, + JSONValues: []string{}, + }, + args: args{ + p: []getter.Provider{}, + }, + want: map[string]interface{}(nil), + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := tt.opts.MergeValues(tt.args.p) + + if (err != nil) != tt.wantErr { + t.Errorf("Options.MergeValues() error = %v, wantErr %v", err, tt.wantErr) + + return + } + + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("Expected result from MergeValues() = %v, got %v", tt.want, got) + } + }) + } +} diff --git a/pkg/cli/values/testdata/chart-with-values-dir/values.d/non-power-users.yaml b/pkg/cli/values/testdata/chart-with-values-dir/values.d/non-power-users.yaml new file mode 100644 index 00000000000..b1be381b570 --- /dev/null +++ b/pkg/cli/values/testdata/chart-with-values-dir/values.d/non-power-users.yaml @@ -0,0 +1,7 @@ +non-power-users: + - Zoro + - Nami + - Ussop + - Sanji + - Franky + - Jinbei diff --git a/pkg/cli/values/testdata/chart-with-values-dir/values.d/power-users.yaml b/pkg/cli/values/testdata/chart-with-values-dir/values.d/power-users.yaml new file mode 100644 index 00000000000..05d9cda9167 --- /dev/null +++ b/pkg/cli/values/testdata/chart-with-values-dir/values.d/power-users.yaml @@ -0,0 +1,5 @@ +power-users: + - Luffy + - Chopper + - Robin + - Brook diff --git a/pkg/cli/values/testdata/multi-level-values-dir/ship.yaml b/pkg/cli/values/testdata/multi-level-values-dir/ship.yaml new file mode 100644 index 00000000000..41603bb1aef --- /dev/null +++ b/pkg/cli/values/testdata/multi-level-values-dir/ship.yaml @@ -0,0 +1 @@ +ship: Thousand Sunny diff --git a/pkg/cli/values/testdata/multi-level-values-dir/values.d/extras/crew.yaml b/pkg/cli/values/testdata/multi-level-values-dir/values.d/extras/crew.yaml new file mode 100644 index 00000000000..8266463b105 --- /dev/null +++ b/pkg/cli/values/testdata/multi-level-values-dir/values.d/extras/crew.yaml @@ -0,0 +1 @@ +crew: Straw Hat Pirates diff --git a/pkg/cli/values/testdata/multi-level-values-dir/values.d/extras/ship.yaml b/pkg/cli/values/testdata/multi-level-values-dir/values.d/extras/ship.yaml new file mode 100644 index 00000000000..659919c7d1a --- /dev/null +++ b/pkg/cli/values/testdata/multi-level-values-dir/values.d/extras/ship.yaml @@ -0,0 +1 @@ +ship: Going Merry diff --git a/pkg/cli/values/testdata/multi-level-values-dir/values.d/non-power-users.yaml b/pkg/cli/values/testdata/multi-level-values-dir/values.d/non-power-users.yaml new file mode 100644 index 00000000000..b1be381b570 --- /dev/null +++ b/pkg/cli/values/testdata/multi-level-values-dir/values.d/non-power-users.yaml @@ -0,0 +1,7 @@ +non-power-users: + - Zoro + - Nami + - Ussop + - Sanji + - Franky + - Jinbei diff --git a/pkg/cli/values/testdata/multi-level-values-dir/values.d/power-users.yaml b/pkg/cli/values/testdata/multi-level-values-dir/values.d/power-users.yaml new file mode 100644 index 00000000000..05d9cda9167 --- /dev/null +++ b/pkg/cli/values/testdata/multi-level-values-dir/values.d/power-users.yaml @@ -0,0 +1,5 @@ +power-users: + - Luffy + - Chopper + - Robin + - Brook