From d74e03a6a6f4ba6d5c5743cbf90b3cf06f754ab2 Mon Sep 17 00:00:00 2001 From: Cari Date: Fri, 18 Feb 2022 12:15:25 -0800 Subject: [PATCH 01/10] Add test for library ref with dv file flags --- pkg/cmd/template/cmd_data_values_file_test.go | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/pkg/cmd/template/cmd_data_values_file_test.go b/pkg/cmd/template/cmd_data_values_file_test.go index 9ab260b9..d725e792 100644 --- a/pkg/cmd/template/cmd_data_values_file_test.go +++ b/pkg/cmd/template/cmd_data_values_file_test.go @@ -118,6 +118,69 @@ array: assert.Equal(t, expectedYAMLTplData, string(file.Bytes())) } +func TestDataValuesFileFlagsWithLibRefPath(t *testing.T) { + yamlTplData := []byte(` +#@ load("@ytt:data", "data") +#@ load("@ytt:library", "library") +#@ load("@ytt:template", "template") +--- +fromRoot: #@ data.values + +#@ lib = library.get("lib") +--- #@ template.replace(lib.eval())`) + + libYamlTplData := []byte(`#@ load("@ytt:data", "data") +--- +fromLibrary: #@ data.values`) + + dvs1 := []byte(`val1: 1`) + + dvs2 := []byte(`val2: 2`) + + dvs3 := []byte(`3`) + + expectedYAMLTplData := `fromRoot: + val1: 1 +fromLibrary: + val2: 2 + val3: 3 +` + + filesToProcess := files.NewSortedFiles([]*files.File{ + files.MustNewFileFromSource(files.NewBytesSource("tpl.yml", yamlTplData)), + files.MustNewFileFromSource(files.NewBytesSource("_ytt_lib/lib/tpl.yml", libYamlTplData)), + }) + + ui := ui.NewTTY(false) + opts := cmdtpl.NewOptions() + + opts.DataValuesFlags = cmdtpl.DataValuesFlags{ + FromFiles: []string{"c:\\User\\user\\dvs1.yml", "@lib:dvs2.yml"}, + KVsFromFiles: []string{"val3=@lib:c:\\User\\user\\dvs3.yml"}, + ReadFileFunc: func(path string) ([]byte, error) { + switch path { + case "c:\\User\\user\\dvs1.yml": + return dvs1, nil + case "dvs2.yml": + return dvs2, nil + case "c:\\User\\user\\dvs3.yml": + return dvs3, nil + default: + return nil, fmt.Errorf("Unknown file '%s'", path) + } + }, + } + + out := opts.RunWithFiles(cmdtpl.Input{Files: filesToProcess}, ui) + require.NoError(t, out.Err) + require.Len(t, out.Files, 1, "unexpected number of output files") + + file := out.Files[0] + + assert.Equal(t, "tpl.yml", file.RelativePath()) + assert.Equal(t, expectedYAMLTplData, string(file.Bytes())) +} + func TestDataValuesWithDataValuesFileFlagsForbiddenComment(t *testing.T) { yamlTplData := []byte(` #@ load("@ytt:data", "data") From 8a29e23aec7a4138227bb5822af673ccc5a7a621 Mon Sep 17 00:00:00 2001 From: Cari Date: Sat, 5 Mar 2022 20:26:35 -0800 Subject: [PATCH 02/10] Use `@` to detect library reference --- pkg/cmd/template/cmd_data_values_file_test.go | 16 +++++++++++----- pkg/cmd/template/data_values_flags.go | 11 +++++++++++ 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/template/cmd_data_values_file_test.go b/pkg/cmd/template/cmd_data_values_file_test.go index d725e792..4239f74d 100644 --- a/pkg/cmd/template/cmd_data_values_file_test.go +++ b/pkg/cmd/template/cmd_data_values_file_test.go @@ -137,13 +137,17 @@ fromLibrary: #@ data.values`) dvs2 := []byte(`val2: 2`) - dvs3 := []byte(`3`) + dvs3 := []byte(`val3: 3`) + + dvs4 := []byte(`4`) expectedYAMLTplData := `fromRoot: val1: 1 -fromLibrary: +--- +fromLibrary: val2: 2 val3: 3 + val4: "4" ` filesToProcess := files.NewSortedFiles([]*files.File{ @@ -155,16 +159,18 @@ fromLibrary: opts := cmdtpl.NewOptions() opts.DataValuesFlags = cmdtpl.DataValuesFlags{ - FromFiles: []string{"c:\\User\\user\\dvs1.yml", "@lib:dvs2.yml"}, - KVsFromFiles: []string{"val3=@lib:c:\\User\\user\\dvs3.yml"}, + FromFiles: []string{"c:\\User\\user\\dvs1.yml", "@lib:dvs2.yml", "@lib:D:\\User\\user\\dvs3.yml"}, + KVsFromFiles: []string{"@lib:val4=c:\\User\\user\\dvs4.yml"}, ReadFileFunc: func(path string) ([]byte, error) { switch path { case "c:\\User\\user\\dvs1.yml": return dvs1, nil case "dvs2.yml": return dvs2, nil - case "c:\\User\\user\\dvs3.yml": + case "D:\\User\\user\\dvs3.yml": return dvs3, nil + case "c:\\User\\user\\dvs4.yml": + return dvs4, nil default: return nil, fmt.Errorf("Unknown file '%s'", path) } diff --git a/pkg/cmd/template/data_values_flags.go b/pkg/cmd/template/data_values_flags.go index e57013ee..4943683f 100644 --- a/pkg/cmd/template/data_values_flags.go +++ b/pkg/cmd/template/data_values_flags.go @@ -15,6 +15,7 @@ import ( "github.com/vmware-tanzu/carvel-ytt/pkg/files" "github.com/vmware-tanzu/carvel-ytt/pkg/template" "github.com/vmware-tanzu/carvel-ytt/pkg/workspace/datavalues" + "github.com/vmware-tanzu/carvel-ytt/pkg/workspace/ref" "github.com/vmware-tanzu/carvel-ytt/pkg/yamlmeta" yttoverlay "github.com/vmware-tanzu/carvel-ytt/pkg/yttlibrary/overlay" ) @@ -289,7 +290,17 @@ func (DataValuesFlags) libraryRefAndKey(key string) (string, string, error) { if len(keyPieces[0]) == 0 { return "", "", fmt.Errorf("Expected library ref to not be empty") } + if !strings.HasPrefix(keyPieces[0], ref.LibrarySep) { + // the libraryKeySep is part of the path + return "", key, nil + } return keyPieces[0], keyPieces[1], nil + case 3: + if strings.HasPrefix(keyPieces[0], ref.LibrarySep) { + // the second libraryKeySep is part of the path + return keyPieces[0], keyPieces[1] + libraryKeySep + keyPieces[2], nil + } + return "", "", fmt.Errorf("Expected at most one library-key separator '%s' in '%s'", libraryKeySep, key) default: return "", "", fmt.Errorf("Expected at most one library-key separator '%s' in '%s'", libraryKeySep, key) From 9612ef3bc1221826272a2f8fc578cd382121e0a0 Mon Sep 17 00:00:00 2001 From: Cari Date: Sat, 5 Mar 2022 20:54:01 -0800 Subject: [PATCH 03/10] Simplify switch --- pkg/cmd/template/data_values_flags.go | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/template/data_values_flags.go b/pkg/cmd/template/data_values_flags.go index 4943683f..f3cc7236 100644 --- a/pkg/cmd/template/data_values_flags.go +++ b/pkg/cmd/template/data_values_flags.go @@ -290,21 +290,18 @@ func (DataValuesFlags) libraryRefAndKey(key string) (string, string, error) { if len(keyPieces[0]) == 0 { return "", "", fmt.Errorf("Expected library ref to not be empty") } - if !strings.HasPrefix(keyPieces[0], ref.LibrarySep) { - // the libraryKeySep is part of the path - return "", key, nil + if strings.HasPrefix(keyPieces[0], ref.LibrarySep) { + return keyPieces[0], keyPieces[1], nil } - return keyPieces[0], keyPieces[1], nil + // the LibrarySep is part of the path + return "", key, nil case 3: if strings.HasPrefix(keyPieces[0], ref.LibrarySep) { - // the second libraryKeySep is part of the path + // the second LibrarySep is part of the path return keyPieces[0], keyPieces[1] + libraryKeySep + keyPieces[2], nil } - return "", "", fmt.Errorf("Expected at most one library-key separator '%s' in '%s'", libraryKeySep, key) - - default: - return "", "", fmt.Errorf("Expected at most one library-key separator '%s' in '%s'", libraryKeySep, key) } + return "", "", fmt.Errorf("Expected at most one library-key separator '%s' in '%s'", libraryKeySep, key) } func (s *DataValuesFlags) buildOverlay(keyPieces []string, value interface{}, desc string, line string) *yamlmeta.Document { From e53d582b3bb9e821e43ad849ac12f90668114429 Mon Sep 17 00:00:00 2001 From: Cari Date: Tue, 15 Mar 2022 00:29:53 -0700 Subject: [PATCH 04/10] Allow ":" in data values flags - Move test --- pkg/cmd/template/cmd_data_values_file_test.go | 69 ------------------- pkg/cmd/template/cmd_data_values_test.go | 53 +++++++++++--- pkg/cmd/template/data_values_flags.go | 41 +++++------ 3 files changed, 63 insertions(+), 100 deletions(-) diff --git a/pkg/cmd/template/cmd_data_values_file_test.go b/pkg/cmd/template/cmd_data_values_file_test.go index 4239f74d..9ab260b9 100644 --- a/pkg/cmd/template/cmd_data_values_file_test.go +++ b/pkg/cmd/template/cmd_data_values_file_test.go @@ -118,75 +118,6 @@ array: assert.Equal(t, expectedYAMLTplData, string(file.Bytes())) } -func TestDataValuesFileFlagsWithLibRefPath(t *testing.T) { - yamlTplData := []byte(` -#@ load("@ytt:data", "data") -#@ load("@ytt:library", "library") -#@ load("@ytt:template", "template") ---- -fromRoot: #@ data.values - -#@ lib = library.get("lib") ---- #@ template.replace(lib.eval())`) - - libYamlTplData := []byte(`#@ load("@ytt:data", "data") ---- -fromLibrary: #@ data.values`) - - dvs1 := []byte(`val1: 1`) - - dvs2 := []byte(`val2: 2`) - - dvs3 := []byte(`val3: 3`) - - dvs4 := []byte(`4`) - - expectedYAMLTplData := `fromRoot: - val1: 1 ---- -fromLibrary: - val2: 2 - val3: 3 - val4: "4" -` - - filesToProcess := files.NewSortedFiles([]*files.File{ - files.MustNewFileFromSource(files.NewBytesSource("tpl.yml", yamlTplData)), - files.MustNewFileFromSource(files.NewBytesSource("_ytt_lib/lib/tpl.yml", libYamlTplData)), - }) - - ui := ui.NewTTY(false) - opts := cmdtpl.NewOptions() - - opts.DataValuesFlags = cmdtpl.DataValuesFlags{ - FromFiles: []string{"c:\\User\\user\\dvs1.yml", "@lib:dvs2.yml", "@lib:D:\\User\\user\\dvs3.yml"}, - KVsFromFiles: []string{"@lib:val4=c:\\User\\user\\dvs4.yml"}, - ReadFileFunc: func(path string) ([]byte, error) { - switch path { - case "c:\\User\\user\\dvs1.yml": - return dvs1, nil - case "dvs2.yml": - return dvs2, nil - case "D:\\User\\user\\dvs3.yml": - return dvs3, nil - case "c:\\User\\user\\dvs4.yml": - return dvs4, nil - default: - return nil, fmt.Errorf("Unknown file '%s'", path) - } - }, - } - - out := opts.RunWithFiles(cmdtpl.Input{Files: filesToProcess}, ui) - require.NoError(t, out.Err) - require.Len(t, out.Files, 1, "unexpected number of output files") - - file := out.Files[0] - - assert.Equal(t, "tpl.yml", file.RelativePath()) - assert.Equal(t, expectedYAMLTplData, string(file.Bytes())) -} - func TestDataValuesWithDataValuesFileFlagsForbiddenComment(t *testing.T) { yamlTplData := []byte(` #@ load("@ytt:data", "data") diff --git a/pkg/cmd/template/cmd_data_values_test.go b/pkg/cmd/template/cmd_data_values_test.go index 0e113c0f..5d763815 100644 --- a/pkg/cmd/template/cmd_data_values_test.go +++ b/pkg/cmd/template/cmd_data_values_test.go @@ -4,6 +4,7 @@ package template_test import ( + "fmt" "testing" "github.com/stretchr/testify/assert" @@ -183,7 +184,9 @@ func TestDataValuesWithLibraryAttachedFlags(t *testing.T) { tplBytes := []byte(` #@ load("@ytt:library", "library") #@ load("@ytt:template", "template") +#@ load("@ytt:data", "data") +root: #@ data.values --- #@ template.replace(library.get("lib", alias="inst1").eval())`) libTplBytes := []byte(` @@ -191,31 +194,48 @@ func TestDataValuesWithLibraryAttachedFlags(t *testing.T) { #@ load("@ytt:template", "template") #@ load("@ytt:data", "data") -lib-val: #@ data.values.lib_val +from_library: #@ data.values --- #@ template.replace(library.get("nested-lib").eval()) `) libValuesBytes := []byte(` #@data/values --- -lib_val: override-me +val0: override-me `) nestedLibTplBytes := []byte(` #@ load("@ytt:data", "data") -nested-lib-val: #@ data.values.nested_lib_val +from_nested_lib: #@ data.values `) nestedLibValuesBytes := []byte(` #@data/values --- -nested_lib_val: override-me +val1: override-me `) - expectedYAMLTplData := `lib-val: test + dvs2 := []byte(`val2: 2`) + + dvs3 := []byte(`val3: 3`) + + dvs4 := []byte(`val4: 4`) + + dvs6 := []byte(`6`) + + expectedYAMLTplData := `root: + val2: 2 --- -nested-lib-val: passes +from_library: + val0: 0 + val3: 3 + val4: 4 + val5: "5" + val6: "6" +--- +from_nested_lib: + val1: 1 ` filesToProcess := files.NewSortedFiles([]*files.File{ @@ -230,8 +250,25 @@ nested-lib-val: passes opts := cmdtpl.NewOptions() opts.DataValuesFlags = cmdtpl.DataValuesFlags{ - // TODO env and files? - KVsFromYAML: []string{"@~inst1:lib_val=test", "@~inst1@nested-lib:nested_lib_val=passes"}, + KVsFromYAML: []string{"@~inst1:val0=0", "@~inst1@nested-lib:val1=1"}, + FromFiles: []string{"c:\\User\\user\\dvs2.yml", "@~inst1:dvs3.yml", "@lib:D:\\User\\user\\dvs4.yml"}, + EnvFromStrings: []string{"@lib::DVS"}, + EnvironFunc: func() []string { return []string{"DVS_val5=5"} }, + KVsFromFiles: []string{"@lib:val6=c:\\User\\user\\dvs6.yml"}, + ReadFileFunc: func(path string) ([]byte, error) { + switch path { + case "c:\\User\\user\\dvs2.yml": + return dvs2, nil + case "dvs3.yml": + return dvs3, nil + case "D:\\User\\user\\dvs4.yml": + return dvs4, nil + case "c:\\User\\user\\dvs6.yml": + return dvs6, nil + default: + return nil, fmt.Errorf("Unknown file '%s'", path) + } + }, } out := opts.RunWithFiles(cmdtpl.Input{Files: filesToProcess}, ui) diff --git a/pkg/cmd/template/data_values_flags.go b/pkg/cmd/template/data_values_flags.go index f3cc7236..30dc5717 100644 --- a/pkg/cmd/template/data_values_flags.go +++ b/pkg/cmd/template/data_values_flags.go @@ -136,15 +136,15 @@ func (s *DataValuesFlags) AsOverlays(strict bool) ([]*datavalues.Envelope, []*da return overlayValues, libraryOverlays, nil } -func (s *DataValuesFlags) file(path string, strict bool) ([]*datavalues.Envelope, error) { - libRef, path, err := s.libraryRefAndKey(path) +func (s *DataValuesFlags) file(fullPath string, strict bool) ([]*datavalues.Envelope, error) { + libRef, path, err := s.libraryRefAndKey(fullPath) if err != nil { return nil, err } contents, err := s.readFile(path) if err != nil { - return nil, fmt.Errorf("Reading file '%s': %s", path, err) + return nil, fmt.Errorf("Unable to read file '%s' from flag value '%s': %s", path, err) } docSetOpts := yamlmeta.DocSetOpts{ @@ -275,33 +275,28 @@ func (s *DataValuesFlags) kvFile(kv string) (*datavalues.Envelope, error) { return datavalues.NewEnvelopeWithLibRef(overlay, libRef) } +// libraryRefAndKey separates a library reference and a key. +// A library reference starts with ref.LibrarySep, and ends with the first occurrence of libraryPathSep. func (DataValuesFlags) libraryRefAndKey(key string) (string, string, error) { const ( - libraryKeySep = ":" + libraryPathSep = ":" ) - keyPieces := strings.Split(key, libraryKeySep) - - switch len(keyPieces) { - case 1: - return "", key, nil - - case 2: - if len(keyPieces[0]) == 0 { - return "", "", fmt.Errorf("Expected library ref to not be empty") - } - if strings.HasPrefix(keyPieces[0], ref.LibrarySep) { + if strings.HasPrefix(key, ref.LibrarySep) { + keyPieces := strings.SplitN(key, libraryPathSep, 2) + switch len(keyPieces) { + case 1: + return "", key, nil + case 2: + if len(keyPieces[0]) == 1 { + return "", "", fmt.Errorf("Expected library ref to not be empty") + } return keyPieces[0], keyPieces[1], nil } - // the LibrarySep is part of the path - return "", key, nil - case 3: - if strings.HasPrefix(keyPieces[0], ref.LibrarySep) { - // the second LibrarySep is part of the path - return keyPieces[0], keyPieces[1] + libraryKeySep + keyPieces[2], nil - } } - return "", "", fmt.Errorf("Expected at most one library-key separator '%s' in '%s'", libraryKeySep, key) + + return "", key, nil + } func (s *DataValuesFlags) buildOverlay(keyPieces []string, value interface{}, desc string, line string) *yamlmeta.Document { From 6981a3f5d737a46d71b7867f9a16d802bd9b539e Mon Sep 17 00:00:00 2001 From: Cari Date: Tue, 15 Mar 2022 22:05:04 -0700 Subject: [PATCH 05/10] Only allow ':' in data value file flags - exclude env flags or key-value flags --- pkg/cmd/template/cmd_data_values_test.go | 6 ++--- pkg/cmd/template/data_values_flags.go | 30 ++++++++++++++---------- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/pkg/cmd/template/cmd_data_values_test.go b/pkg/cmd/template/cmd_data_values_test.go index 5d763815..9a5fd2bd 100644 --- a/pkg/cmd/template/cmd_data_values_test.go +++ b/pkg/cmd/template/cmd_data_values_test.go @@ -250,9 +250,9 @@ from_nested_lib: opts := cmdtpl.NewOptions() opts.DataValuesFlags = cmdtpl.DataValuesFlags{ - KVsFromYAML: []string{"@~inst1:val0=0", "@~inst1@nested-lib:val1=1"}, + KVsFromYAML: []string{"@~inst1:val0=0", "@~inst1@nested-lib:val1=1"}, FromFiles: []string{"c:\\User\\user\\dvs2.yml", "@~inst1:dvs3.yml", "@lib:D:\\User\\user\\dvs4.yml"}, - EnvFromStrings: []string{"@lib::DVS"}, + EnvFromStrings: []string{"@lib:DVS"}, EnvironFunc: func() []string { return []string{"DVS_val5=5"} }, KVsFromFiles: []string{"@lib:val6=c:\\User\\user\\dvs6.yml"}, ReadFileFunc: func(path string) ([]byte, error) { @@ -262,7 +262,7 @@ from_nested_lib: case "dvs3.yml": return dvs3, nil case "D:\\User\\user\\dvs4.yml": - return dvs4, nil + return dvs4, nil case "c:\\User\\user\\dvs6.yml": return dvs6, nil default: diff --git a/pkg/cmd/template/data_values_flags.go b/pkg/cmd/template/data_values_flags.go index 30dc5717..8c590c5d 100644 --- a/pkg/cmd/template/data_values_flags.go +++ b/pkg/cmd/template/data_values_flags.go @@ -21,8 +21,9 @@ import ( ) const ( - dvsKVSep = "=" - dvsMapKeySep = "." + dvsKVSep = "=" + dvsMapKeySep = "." + libraryKeySep = ":" ) type DataValuesFlags struct { @@ -189,7 +190,7 @@ func (s *DataValuesFlags) env(prefix string, src dataValuesFlagsSource) ([]*data envVars = s.EnvironFunc() } - libRef, keyPrefix, err := s.libraryRefAndKey(prefix) + libRef, keyPrefix, err := s.libraryRefAndKeyStrict(prefix) if err != nil { return nil, err } @@ -236,7 +237,7 @@ func (s *DataValuesFlags) kv(kv string, src dataValuesFlagsSource) (*datavalues. return nil, fmt.Errorf("Deserializing value for key '%s': %s", pieces[0], err) } - libRef, key, err := s.libraryRefAndKey(pieces[0]) + libRef, key, err := s.libraryRefAndKeyStrict(pieces[0]) if err != nil { return nil, err } @@ -274,16 +275,23 @@ func (s *DataValuesFlags) kvFile(kv string) (*datavalues.Envelope, error) { return datavalues.NewEnvelopeWithLibRef(overlay, libRef) } +func (DataValuesFlags) libraryRefAndKeyStrict(key string) (string, string, error) { + libRef, key, err := DataValuesFlags{}.libraryRefAndKey(key) + if err != nil { + return "", "", err + } + if len(strings.Split(key, libraryKeySep)) > 1 { + // error on a common syntax mistake + return "", "", fmt.Errorf("Expected at most one library-key separator '%s' in '%s'", libraryKeySep, key) + } + return libRef, key, nil +} // libraryRefAndKey separates a library reference and a key. -// A library reference starts with ref.LibrarySep, and ends with the first occurrence of libraryPathSep. +// A library reference starts with ref.LibrarySep, and ends with the first occurrence of libraryKeySep. func (DataValuesFlags) libraryRefAndKey(key string) (string, string, error) { - const ( - libraryPathSep = ":" - ) - if strings.HasPrefix(key, ref.LibrarySep) { - keyPieces := strings.SplitN(key, libraryPathSep, 2) + keyPieces := strings.SplitN(key, libraryKeySep, 2) switch len(keyPieces) { case 1: return "", key, nil @@ -294,9 +302,7 @@ func (DataValuesFlags) libraryRefAndKey(key string) (string, string, error) { return keyPieces[0], keyPieces[1], nil } } - return "", key, nil - } func (s *DataValuesFlags) buildOverlay(keyPieces []string, value interface{}, desc string, line string) *yamlmeta.Document { From b119264ff1bf0103a5639acf580b34302275eb4e Mon Sep 17 00:00:00 2001 From: Cari Date: Mon, 21 Mar 2022 18:27:11 -0700 Subject: [PATCH 06/10] Fix error message --- pkg/cmd/template/data_values_flags.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cmd/template/data_values_flags.go b/pkg/cmd/template/data_values_flags.go index 8c590c5d..d8e102ee 100644 --- a/pkg/cmd/template/data_values_flags.go +++ b/pkg/cmd/template/data_values_flags.go @@ -145,7 +145,7 @@ func (s *DataValuesFlags) file(fullPath string, strict bool) ([]*datavalues.Enve contents, err := s.readFile(path) if err != nil { - return nil, fmt.Errorf("Unable to read file '%s' from flag value '%s': %s", path, err) + return nil, fmt.Errorf("Reading file '%s': %s", path, err) } docSetOpts := yamlmeta.DocSetOpts{ From 85141602620d8ab8c022b75a1dbd2e2628777deb Mon Sep 17 00:00:00 2001 From: Cari Date: Tue, 22 Mar 2022 15:55:18 -0700 Subject: [PATCH 07/10] Rename method, add godocs --- pkg/cmd/template/data_values_flags.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/template/data_values_flags.go b/pkg/cmd/template/data_values_flags.go index d8e102ee..74618e79 100644 --- a/pkg/cmd/template/data_values_flags.go +++ b/pkg/cmd/template/data_values_flags.go @@ -190,7 +190,7 @@ func (s *DataValuesFlags) env(prefix string, src dataValuesFlagsSource) ([]*data envVars = s.EnvironFunc() } - libRef, keyPrefix, err := s.libraryRefAndKeyStrict(prefix) + libRef, keyPrefix, err := s.libraryRefAndKeyExactlyOneSep(prefix) if err != nil { return nil, err } @@ -237,7 +237,7 @@ func (s *DataValuesFlags) kv(kv string, src dataValuesFlagsSource) (*datavalues. return nil, fmt.Errorf("Deserializing value for key '%s': %s", pieces[0], err) } - libRef, key, err := s.libraryRefAndKeyStrict(pieces[0]) + libRef, key, err := s.libraryRefAndKeyExactlyOneSep(pieces[0]) if err != nil { return nil, err } @@ -275,7 +275,10 @@ func (s *DataValuesFlags) kvFile(kv string) (*datavalues.Envelope, error) { return datavalues.NewEnvelopeWithLibRef(overlay, libRef) } -func (DataValuesFlags) libraryRefAndKeyStrict(key string) (string, string, error) { + +// libraryRefAndKeyExactlyOneSep separates a library reference and a key and validates that no libraryKeySep exist in the key. +// libraryKeySep is disallowed in some data values flags. Should we reconsider allowing this? See issue #637. +func (DataValuesFlags) libraryRefAndKeyExactlyOneSep(key string) (string, string, error) { libRef, key, err := DataValuesFlags{}.libraryRefAndKey(key) if err != nil { return "", "", err From 2ad8d2207c2cbbdcf9464977540251a94cb04bf1 Mon Sep 17 00:00:00 2001 From: Cari Date: Wed, 23 Mar 2022 12:03:39 -0700 Subject: [PATCH 08/10] Only modify `--data-values-file` flag - rename method to clear distinction between method used for keys and method used for just string parsing --- pkg/cmd/template/data_values_flags.go | 32 +++++++++++++-------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/pkg/cmd/template/data_values_flags.go b/pkg/cmd/template/data_values_flags.go index 74618e79..62bc90a3 100644 --- a/pkg/cmd/template/data_values_flags.go +++ b/pkg/cmd/template/data_values_flags.go @@ -138,7 +138,7 @@ func (s *DataValuesFlags) AsOverlays(strict bool) ([]*datavalues.Envelope, []*da } func (s *DataValuesFlags) file(fullPath string, strict bool) ([]*datavalues.Envelope, error) { - libRef, path, err := s.libraryRefAndKey(fullPath) + libRef, path, err := s.libraryRefAndString(fullPath) if err != nil { return nil, err } @@ -190,7 +190,7 @@ func (s *DataValuesFlags) env(prefix string, src dataValuesFlagsSource) ([]*data envVars = s.EnvironFunc() } - libRef, keyPrefix, err := s.libraryRefAndKeyExactlyOneSep(prefix) + libRef, keyPrefix, err := s.libraryRefAndKey(prefix) if err != nil { return nil, err } @@ -237,7 +237,7 @@ func (s *DataValuesFlags) kv(kv string, src dataValuesFlagsSource) (*datavalues. return nil, fmt.Errorf("Deserializing value for key '%s': %s", pieces[0], err) } - libRef, key, err := s.libraryRefAndKeyExactlyOneSep(pieces[0]) + libRef, key, err := s.libraryRefAndKey(pieces[0]) if err != nil { return nil, err } @@ -276,10 +276,10 @@ func (s *DataValuesFlags) kvFile(kv string) (*datavalues.Envelope, error) { return datavalues.NewEnvelopeWithLibRef(overlay, libRef) } -// libraryRefAndKeyExactlyOneSep separates a library reference and a key and validates that no libraryKeySep exist in the key. -// libraryKeySep is disallowed in some data values flags. Should we reconsider allowing this? See issue #637. -func (DataValuesFlags) libraryRefAndKeyExactlyOneSep(key string) (string, string, error) { - libRef, key, err := DataValuesFlags{}.libraryRefAndKey(key) +// libraryRefAndKey separates a library reference and a key and validates that no libraryKeySep exist in the key. +// libraryKeySep is disallowed in data value flag keys. +func (DataValuesFlags) libraryRefAndKey(key string) (string, string, error) { + libRef, key, err := DataValuesFlags{}.libraryRefAndString(key) if err != nil { return "", "", err } @@ -290,22 +290,22 @@ func (DataValuesFlags) libraryRefAndKeyExactlyOneSep(key string) (string, string return libRef, key, nil } -// libraryRefAndKey separates a library reference and a key. +// libraryRefAndString separates a library reference and a string. // A library reference starts with ref.LibrarySep, and ends with the first occurrence of libraryKeySep. -func (DataValuesFlags) libraryRefAndKey(key string) (string, string, error) { - if strings.HasPrefix(key, ref.LibrarySep) { - keyPieces := strings.SplitN(key, libraryKeySep, 2) - switch len(keyPieces) { +func (DataValuesFlags) libraryRefAndString(str string) (string, string, error) { + if strings.HasPrefix(str, ref.LibrarySep) { + strPieces := strings.SplitN(str, libraryKeySep, 2) + switch len(strPieces) { case 1: - return "", key, nil + return "", str, nil case 2: - if len(keyPieces[0]) == 1 { + if len(strPieces[0]) == 1 { return "", "", fmt.Errorf("Expected library ref to not be empty") } - return keyPieces[0], keyPieces[1], nil + return strPieces[0], strPieces[1], nil } } - return "", key, nil + return "", str, nil } func (s *DataValuesFlags) buildOverlay(keyPieces []string, value interface{}, desc string, line string) *yamlmeta.Document { From 8b542b8ff001446f7c1e3ea1f5de3ba85da429d2 Mon Sep 17 00:00:00 2001 From: Cari Date: Wed, 23 Mar 2022 12:21:12 -0700 Subject: [PATCH 09/10] Add test for disallow `:` in `--data-value-yaml` --- pkg/cmd/template/cmd_data_values_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/pkg/cmd/template/cmd_data_values_test.go b/pkg/cmd/template/cmd_data_values_test.go index 9a5fd2bd..23ffe776 100644 --- a/pkg/cmd/template/cmd_data_values_test.go +++ b/pkg/cmd/template/cmd_data_values_test.go @@ -595,3 +595,21 @@ nested_val: nested_from_env assert.Equal(t, "tpl.yml", file.RelativePath()) assert.Equal(t, expectedYAMLTplData, string(file.Bytes())) } + +func TestDataValuesWithInvalidFlagsFail(t *testing.T) { + t.Run("when `--data-value-yaml` has a `:` in the key name", func(t *testing.T) { + + expectedErr := `Extracting data value from KV: Expected at most one library-key separator ':' in 'i:nt'` + + ui := ui.NewTTY(false) + opts := cmdtpl.NewOptions() + + opts.DataValuesFlags = cmdtpl.DataValuesFlags{ + KVsFromYAML: []string{"i:nt=124"}, + } + + out := opts.RunWithFiles(cmdtpl.Input{}, ui) + require.Errorf(t, out.Err, expectedErr) + require.Equal(t, expectedErr, out.Err.Error()) + }) +} From 85078b7c9010bbb0a65c2548f7216a6a1d2aebdd Mon Sep 17 00:00:00 2001 From: Cari Date: Mon, 28 Mar 2022 16:50:17 -0700 Subject: [PATCH 10/10] Rename function, and variable args --- pkg/cmd/template/data_values_flags.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/cmd/template/data_values_flags.go b/pkg/cmd/template/data_values_flags.go index 62bc90a3..f6c537ae 100644 --- a/pkg/cmd/template/data_values_flags.go +++ b/pkg/cmd/template/data_values_flags.go @@ -138,7 +138,7 @@ func (s *DataValuesFlags) AsOverlays(strict bool) ([]*datavalues.Envelope, []*da } func (s *DataValuesFlags) file(fullPath string, strict bool) ([]*datavalues.Envelope, error) { - libRef, path, err := s.libraryRefAndString(fullPath) + libRef, path, err := s.libraryRefAndRemainder(fullPath) if err != nil { return nil, err } @@ -278,8 +278,8 @@ func (s *DataValuesFlags) kvFile(kv string) (*datavalues.Envelope, error) { // libraryRefAndKey separates a library reference and a key and validates that no libraryKeySep exist in the key. // libraryKeySep is disallowed in data value flag keys. -func (DataValuesFlags) libraryRefAndKey(key string) (string, string, error) { - libRef, key, err := DataValuesFlags{}.libraryRefAndString(key) +func (DataValuesFlags) libraryRefAndKey(arg string) (string, string, error) { + libRef, key, err := DataValuesFlags{}.libraryRefAndRemainder(arg) if err != nil { return "", "", err } @@ -290,14 +290,14 @@ func (DataValuesFlags) libraryRefAndKey(key string) (string, string, error) { return libRef, key, nil } -// libraryRefAndString separates a library reference and a string. +// libraryRefAndRemainder separates a library reference prefix from the remainder of the string. // A library reference starts with ref.LibrarySep, and ends with the first occurrence of libraryKeySep. -func (DataValuesFlags) libraryRefAndString(str string) (string, string, error) { - if strings.HasPrefix(str, ref.LibrarySep) { - strPieces := strings.SplitN(str, libraryKeySep, 2) +func (DataValuesFlags) libraryRefAndRemainder(arg string) (string, string, error) { + if strings.HasPrefix(arg, ref.LibrarySep) { + strPieces := strings.SplitN(arg, libraryKeySep, 2) switch len(strPieces) { case 1: - return "", str, nil + return "", arg, nil case 2: if len(strPieces[0]) == 1 { return "", "", fmt.Errorf("Expected library ref to not be empty") @@ -305,7 +305,7 @@ func (DataValuesFlags) libraryRefAndString(str string) (string, string, error) { return strPieces[0], strPieces[1], nil } } - return "", str, nil + return "", arg, nil } func (s *DataValuesFlags) buildOverlay(keyPieces []string, value interface{}, desc string, line string) *yamlmeta.Document {