From b214fa7d5aa51d7c2ae306ec15115bf1c044fed8 Mon Sep 17 00:00:00 2001 From: Donny Xia Date: Tue, 6 Oct 2020 11:49:39 -0700 Subject: [PATCH] remove ',' separator in edit add label/annotation --- .../internal/commands/edit/add/addmetadata.go | 9 ++-- .../commands/edit/add/addmetadata_test.go | 42 +++++++++++++------ kustomize/internal/commands/util/util.go | 10 ++++- kustomize/internal/commands/util/util_test.go | 19 +++++++++ 4 files changed, 60 insertions(+), 20 deletions(-) diff --git a/kustomize/internal/commands/edit/add/addmetadata.go b/kustomize/internal/commands/edit/add/addmetadata.go index 4eb2d4c51..6157782fa 100644 --- a/kustomize/internal/commands/edit/add/addmetadata.go +++ b/kustomize/internal/commands/edit/add/addmetadata.go @@ -50,7 +50,7 @@ func newCmdAddAnnotation(fSys filesys.FileSystem, v func(map[string]string) erro Short: "Adds one or more commonAnnotations to " + konfig.DefaultKustomizationFileName(), Example: ` - add annotation {annotationKey1:annotationValue1},{annotationKey2:annotationValue2}`, + add annotation {annotationKey1:annotationValue1} {annotationKey2:annotationValue2}`, RunE: func(cmd *cobra.Command, args []string) error { return o.runE(args, fSys, o.addAnnotations) }, @@ -71,7 +71,7 @@ func newCmdAddLabel(fSys filesys.FileSystem, v func(map[string]string) error) *c Short: "Adds one or more commonLabels to " + konfig.DefaultKustomizationFileName(), Example: ` - add label {labelKey1:labelValue1},{labelKey2:labelValue2}`, + add label {labelKey1:labelValue1} {labelKey2:labelValue2}`, RunE: func(cmd *cobra.Command, args []string) error { return o.runE(args, fSys, o.addLabels) }, @@ -108,10 +108,7 @@ func (o *addMetadataOptions) validateAndParse(args []string) error { if len(args) < 1 { return fmt.Errorf("must specify %s", o.kind) } - if len(args) > 1 { - return fmt.Errorf("%ss must be comma-separated, with no spaces", o.kind) - } - m, err := util.ConvertToMap(args[0], o.kind.String()) + m, err := util.ConvertSliceToMap(args, o.kind.String()) if err != nil { return err } diff --git a/kustomize/internal/commands/edit/add/addmetadata_test.go b/kustomize/internal/commands/edit/add/addmetadata_test.go index 691095b11..ae0dbe6b8 100644 --- a/kustomize/internal/commands/edit/add/addmetadata_test.go +++ b/kustomize/internal/commands/edit/add/addmetadata_test.go @@ -4,6 +4,7 @@ package add import ( + "strings" "testing" "sigs.k8s.io/kustomize/api/filesys" @@ -117,6 +118,29 @@ func TestAddAnnotationValueWithColon(t *testing.T) { } } +func TestAddAnnotationValueWithComma(t *testing.T) { + fSys := filesys.MakeFsInMemory() + testutils_test.WriteTestKustomization(fSys) + v := valtest_test.MakeHappyMapValidator(t) + cmd := newCmdAddAnnotation(fSys, v.Validator) + value := "{\"k1\":\"v1\",\"k2\":\"v2\"}" + args := []string{"test:" + value} + err := cmd.RunE(cmd, args) + v.VerifyCall() + if err != nil { + t.Errorf("unexpected error: %v", err.Error()) + } + b, err := fSys.ReadFile("/kustomization.yaml") + if err != nil { + t.Errorf("unexpected error: %v", err.Error()) + } + if !strings.Contains(string(b), value) { + t.Errorf( + "Modified file doesn't contain expected string.\nExpected string:\n%s\nActual:\n%s", + value, b) + } +} + func TestAddAnnotationNoKey(t *testing.T) { fSys := filesys.MakeFsInMemory() v := valtest_test.MakeHappyMapValidator(t) @@ -165,12 +189,9 @@ func TestAddAnnotationMultipleArgs(t *testing.T) { cmd := newCmdAddAnnotation(fSys, v.Validator) args := []string{"this:annotation", "has:spaces"} err := cmd.RunE(cmd, args) - v.VerifyNoCall() - if err == nil { - t.Errorf("expected an error") - } - if err.Error() != "annotations must be comma-separated, with no spaces" { - t.Errorf("incorrect error: %v", err.Error()) + v.VerifyCall() + if err != nil { + t.Errorf("unexpected error: %v", err.Error()) } } @@ -307,12 +328,9 @@ func TestAddLabelMultipleArgs(t *testing.T) { cmd := newCmdAddLabel(fSys, v.Validator) args := []string{"this:input", "has:spaces"} err := cmd.RunE(cmd, args) - v.VerifyNoCall() - if err == nil { - t.Errorf("expected an error") - } - if err.Error() != "labels must be comma-separated, with no spaces" { - t.Errorf("incorrect error: %v", err.Error()) + v.VerifyCall() + if err != nil { + t.Errorf("unexpected error: %v", err.Error()) } } diff --git a/kustomize/internal/commands/util/util.go b/kustomize/internal/commands/util/util.go index cdbe62d6f..a9c837e65 100644 --- a/kustomize/internal/commands/util/util.go +++ b/kustomize/internal/commands/util/util.go @@ -56,14 +56,20 @@ func GlobPatternsWithLoader(fSys filesys.FileSystem, ldr ifc.Loader, patterns [] return result, nil } -// ConvertToMap converts a slice of strings in the form of -// `key:value` into a map. +// ConvertToMap converts a string in the form of `key:value,key:value,...` into a map. func ConvertToMap(input string, kind string) (map[string]string, error) { result := make(map[string]string) if input == "" { return result, nil } inputs := strings.Split(input, ",") + return ConvertSliceToMap(inputs, kind) +} + +// ConvertSliceToMap converts a slice of strings in the form of +// `key:value` into a map. +func ConvertSliceToMap(inputs []string, kind string) (map[string]string, error) { + result := make(map[string]string) for _, input := range inputs { c := strings.Index(input, ":") if c == 0 { diff --git a/kustomize/internal/commands/util/util_test.go b/kustomize/internal/commands/util/util_test.go index 4daea1a0d..d39c38f57 100644 --- a/kustomize/internal/commands/util/util_test.go +++ b/kustomize/internal/commands/util/util_test.go @@ -40,6 +40,25 @@ func TestConvertToMapError(t *testing.T) { } } +func TestConvertSliceToMap(t *testing.T) { + args := []string{"a:b", "c:\"d\"", "e:\"f:g\"", "g:h:k"} + expected := make(map[string]string) + expected["a"] = "b" + expected["c"] = "d" + expected["e"] = "f:g" + expected["g"] = "h:k" + + result, err := ConvertSliceToMap(args, "annotation") + if err != nil { + t.Errorf("unexpected error: %v", err.Error()) + } + + eq := reflect.DeepEqual(expected, result) + if !eq { + t.Errorf("Converted map does not match expected, expected: %v, result: %v\n", expected, result) + } +} + func TestGlobPatternsWithLoaderRemoteFile(t *testing.T) { fSys := filesys.MakeFsInMemory() fSys.Create("test.yml")