From ea3d5e68db0aa21cadc7619d4258f02fe35e10df Mon Sep 17 00:00:00 2001 From: Narayanan Singaram Date: Sat, 2 Mar 2019 12:23:55 -0800 Subject: [PATCH 1/3] Fix for #818 - Added support for quoted values --- pkg/commands/edit/add/addmetadata.go | 21 ++++++-- pkg/commands/edit/add/addmetadata_test.go | 63 ++++++++++++++++++++++- 2 files changed, 79 insertions(+), 5 deletions(-) diff --git a/pkg/commands/edit/add/addmetadata.go b/pkg/commands/edit/add/addmetadata.go index 633c7566a..f39bc3bb1 100644 --- a/pkg/commands/edit/add/addmetadata.go +++ b/pkg/commands/edit/add/addmetadata.go @@ -135,13 +135,28 @@ func (o *addMetadataOptions) convertToMap(arg string) (map[string]string, error) return nil, o.makeError(input, "empty key") } if len(kv) > 2 { - return nil, o.makeError(input, "too many colons") - } - if len(kv) > 1 { + // more than one colon found + // check if value is quoted + qc := strings.Index(input, ":\"") + if qc >= 1 && input[len(input)-1:] == "\"" { + //value is quoted + result[kv[0]] = input[qc+1:] + } else { + // value is not quoted, return error + return nil, o.makeError(input, "too many colons, quote the values") + } + } else if len(kv) == 2 { result[kv[0]] = kv[1] } else { result[kv[0]] = "" } + + // remove quotes if value is quoted + if len(result[kv[0]]) > 0 && + result[kv[0]][:1] == "\"" && + result[kv[0]][len(result[kv[0]])-1:] == "\"" { + result[kv[0]] = result[kv[0]][1 : len(result[kv[0]])-1] + } } return result, nil } diff --git a/pkg/commands/edit/add/addmetadata_test.go b/pkg/commands/edit/add/addmetadata_test.go index dce543396..8d1041ec3 100644 --- a/pkg/commands/edit/add/addmetadata_test.go +++ b/pkg/commands/edit/add/addmetadata_test.go @@ -17,6 +17,7 @@ limitations under the License. package add import ( + "reflect" "testing" "sigs.k8s.io/kustomize/pkg/commands/kustfile" @@ -103,6 +104,32 @@ func TestAddAnnotationManyArgs(t *testing.T) { } } +func TestAddAnnotationValueQuoted(t *testing.T) { + fakeFS := fs.MakeFakeFS() + fakeFS.WriteTestKustomization() + v := validators.MakeHappyMapValidator(t) + cmd := newCmdAddAnnotation(fakeFS, v.Validator) + args := []string{"k1:\"v1\""} + err := cmd.RunE(cmd, args) + v.VerifyCall() + if err != nil { + t.Errorf("unexpected error: %v", err.Error()) + } +} + +func TestAddAnnotationValueWithColon(t *testing.T) { + fakeFS := fs.MakeFakeFS() + fakeFS.WriteTestKustomization() + v := validators.MakeHappyMapValidator(t) + cmd := newCmdAddAnnotation(fakeFS, v.Validator) + args := []string{"k1:\"v1:v2\""} + err := cmd.RunE(cmd, args) + v.VerifyCall() + if err != nil { + t.Errorf("unexpected error: %v", err.Error()) + } +} + func TestAddAnnotationNoKey(t *testing.T) { fakeFS := fs.MakeFakeFS() v := validators.MakeHappyMapValidator(t) @@ -128,7 +155,7 @@ func TestAddAnnotationTooManyColons(t *testing.T) { if err == nil { t.Errorf("expected an error") } - if err.Error() != "invalid annotation: key:v1:v2 (too many colons)" { + if err.Error() != "invalid annotation: key:v1:v2 (too many colons, quote the values)" { t.Errorf("incorrect error: %v", err.Error()) } } @@ -238,7 +265,7 @@ func TestAddLabelTooManyColons(t *testing.T) { if err == nil { t.Errorf("expected an error") } - if err.Error() != "invalid label: key:v1:v2 (too many colons)" { + if err.Error() != "invalid label: key:v1:v2 (too many colons, quote the values)" { t.Errorf("incorrect error: %v", err.Error()) } } @@ -271,3 +298,35 @@ func TestAddLabelMultipleArgs(t *testing.T) { t.Errorf("incorrect error: %v", err.Error()) } } + +func TestConvertToMap(t *testing.T) { + var o addMetadataOptions + args := "a:b,c:\"d\",e:\"f:g\"" + expected := make(map[string]string) + expected["a"] = "b" + expected["c"] = "d" + expected["e"] = "f:g" + + result, err := o.convertToMap(args) + 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 TestConvertToMapError(t *testing.T) { + var o addMetadataOptions + args := "a:b,c:\"d\",e:f:g" + + _, err := o.convertToMap(args) + if err == nil { + t.Errorf("expected an error") + } + if err.Error() != "invalid annotation: e:f:g (too many colons, quote the values)" { + t.Errorf("incorrect error: %v", err.Error()) + } +} From ed2ad860c69738dc7e2fe876e16f57236ba6790c Mon Sep 17 00:00:00 2001 From: Narayanan Singaram Date: Mon, 4 Mar 2019 13:19:18 -0800 Subject: [PATCH 2/3] Move trim quotes logic to separate function --- pkg/commands/edit/add/addmetadata.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/pkg/commands/edit/add/addmetadata.go b/pkg/commands/edit/add/addmetadata.go index f39bc3bb1..cbb3bc8a7 100644 --- a/pkg/commands/edit/add/addmetadata.go +++ b/pkg/commands/edit/add/addmetadata.go @@ -152,11 +152,7 @@ func (o *addMetadataOptions) convertToMap(arg string) (map[string]string, error) } // remove quotes if value is quoted - if len(result[kv[0]]) > 0 && - result[kv[0]][:1] == "\"" && - result[kv[0]][len(result[kv[0]])-1:] == "\"" { - result[kv[0]] = result[kv[0]][1 : len(result[kv[0]])-1] - } + result[kv[0]] = trimQuotes(result[kv[0]]) } return result, nil } @@ -188,3 +184,12 @@ func (o *addMetadataOptions) writeToMap(m map[string]string, kind kindOfAdd) err func (o *addMetadataOptions) makeError(input string, message string) error { return fmt.Errorf("invalid %s: %s (%s)", o.kind, input, message) } + +func trimQuotes(s string) string { + if len(s) >= 2 { + if s[0] == '"' && s[len(s)-1] == '"' { + return s[1 : len(s)-1] + } + } + return s +} From e666630d36be79d3beef8d150fd4f6f48bb310de Mon Sep 17 00:00:00 2001 From: Narayanan Singaram Date: Mon, 4 Mar 2019 13:50:49 -0800 Subject: [PATCH 3/3] Simplify map conversion logic --- pkg/commands/edit/add/addmetadata.go | 34 ++++++++--------------- pkg/commands/edit/add/addmetadata_test.go | 31 ++++++++++----------- 2 files changed, 26 insertions(+), 39 deletions(-) diff --git a/pkg/commands/edit/add/addmetadata.go b/pkg/commands/edit/add/addmetadata.go index cbb3bc8a7..d9104f35d 100644 --- a/pkg/commands/edit/add/addmetadata.go +++ b/pkg/commands/edit/add/addmetadata.go @@ -130,29 +130,19 @@ func (o *addMetadataOptions) convertToMap(arg string) (map[string]string, error) result := make(map[string]string) inputs := strings.Split(arg, ",") for _, input := range inputs { - kv := strings.Split(input, ":") - if len(kv[0]) < 1 { - return nil, o.makeError(input, "empty key") - } - if len(kv) > 2 { - // more than one colon found - // check if value is quoted - qc := strings.Index(input, ":\"") - if qc >= 1 && input[len(input)-1:] == "\"" { - //value is quoted - result[kv[0]] = input[qc+1:] - } else { - // value is not quoted, return error - return nil, o.makeError(input, "too many colons, quote the values") - } - } else if len(kv) == 2 { - result[kv[0]] = kv[1] + c := strings.Index(input, ":") + if c == 0 { + // key is not passed + return nil, o.makeError(input, "need k:v pair where v may be quoted") + } else if c < 0 { + // only key passed + result[input] = "" } else { - result[kv[0]] = "" + // both key and value passed + key := input[:c] + value := trimQuotes(input[c+1:]) + result[key] = value } - - // remove quotes if value is quoted - result[kv[0]] = trimQuotes(result[kv[0]]) } return result, nil } @@ -182,7 +172,7 @@ func (o *addMetadataOptions) writeToMap(m map[string]string, kind kindOfAdd) err } func (o *addMetadataOptions) makeError(input string, message string) error { - return fmt.Errorf("invalid %s: %s (%s)", o.kind, input, message) + return fmt.Errorf("invalid %s: '%s' (%s)", o.kind, input, message) } func trimQuotes(s string) string { diff --git a/pkg/commands/edit/add/addmetadata_test.go b/pkg/commands/edit/add/addmetadata_test.go index 8d1041ec3..6c7f0743a 100644 --- a/pkg/commands/edit/add/addmetadata_test.go +++ b/pkg/commands/edit/add/addmetadata_test.go @@ -140,23 +140,21 @@ func TestAddAnnotationNoKey(t *testing.T) { if err == nil { t.Errorf("expected an error") } - if err.Error() != "invalid annotation: :nokey (empty key)" { + if err.Error() != "invalid annotation: ':nokey' (need k:v pair where v may be quoted)" { t.Errorf("incorrect error: %v", err.Error()) } } func TestAddAnnotationTooManyColons(t *testing.T) { fakeFS := fs.MakeFakeFS() + fakeFS.WriteTestKustomization() v := validators.MakeHappyMapValidator(t) cmd := newCmdAddAnnotation(fakeFS, v.Validator) args := []string{"key:v1:v2"} err := cmd.RunE(cmd, args) - v.VerifyNoCall() - if err == nil { - t.Errorf("expected an error") - } - if err.Error() != "invalid annotation: key:v1:v2 (too many colons, quote the values)" { - t.Errorf("incorrect error: %v", err.Error()) + v.VerifyCall() + if err != nil { + t.Errorf("unexpected error: %v", err.Error()) } } @@ -250,23 +248,21 @@ func TestAddLabelNoKey(t *testing.T) { if err == nil { t.Errorf("expected an error") } - if err.Error() != "invalid label: :nokey (empty key)" { + if err.Error() != "invalid label: ':nokey' (need k:v pair where v may be quoted)" { t.Errorf("incorrect error: %v", err.Error()) } } func TestAddLabelTooManyColons(t *testing.T) { fakeFS := fs.MakeFakeFS() + fakeFS.WriteTestKustomization() v := validators.MakeHappyMapValidator(t) cmd := newCmdAddLabel(fakeFS, v.Validator) args := []string{"key:v1:v2"} err := cmd.RunE(cmd, args) - v.VerifyNoCall() - if err == nil { - t.Errorf("expected an error") - } - if err.Error() != "invalid label: key:v1:v2 (too many colons, quote the values)" { - t.Errorf("incorrect error: %v", err.Error()) + v.VerifyCall() + if err != nil { + t.Errorf("unexpected error: %v", err.Error()) } } @@ -301,11 +297,12 @@ func TestAddLabelMultipleArgs(t *testing.T) { func TestConvertToMap(t *testing.T) { var o addMetadataOptions - args := "a:b,c:\"d\",e:\"f:g\"" + args := "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 := o.convertToMap(args) if err != nil { @@ -320,13 +317,13 @@ func TestConvertToMap(t *testing.T) { func TestConvertToMapError(t *testing.T) { var o addMetadataOptions - args := "a:b,c:\"d\",e:f:g" + args := "a:b,c:\"d\",:f:g" _, err := o.convertToMap(args) if err == nil { t.Errorf("expected an error") } - if err.Error() != "invalid annotation: e:f:g (too many colons, quote the values)" { + if err.Error() != "invalid annotation: ':f:g' (need k:v pair where v may be quoted)" { t.Errorf("incorrect error: %v", err.Error()) } }