Merge pull request #842 from narayanan/annotation-with-colon

Fix for #818 - Added support for quoted values
kubernetes-sigs · Mar 5, 2019 · a6f6514 · a6f6514
2 parents 31ee38b + e666630
commit a6f6514
Show file tree

Hide file tree

Showing 2 changed files with 91 additions and 25 deletions.
diff --git a/pkg/commands/edit/add/addmetadata.go b/pkg/commands/edit/add/addmetadata.go
@@ -130,17 +130,18 @@ 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 {
-			return nil, o.makeError(input, "too many colons")
-		}
-		if len(kv) > 1 {
-			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
 		}
 	}
 	return result, nil
@@ -171,5 +172,14 @@ 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 {
+	if len(s) >= 2 {
+		if s[0] == '"' && s[len(s)-1] == '"' {
+			return s[1 : len(s)-1]
+		}
+	}
+	return s
 }
diff --git 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)
@@ -113,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)" {
-		t.Errorf("incorrect error: %v", err.Error())
+	v.VerifyCall()
+	if err != nil {
+		t.Errorf("unexpected error: %v", err.Error())
 	}
 }
 
@@ -223,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)" {
-		t.Errorf("incorrect error: %v", err.Error())
+	v.VerifyCall()
+	if err != nil {
+		t.Errorf("unexpected error: %v", err.Error())
 	}
 }
 
@@ -271,3 +294,36 @@ 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\",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 {
+		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\",:f:g"
+
+	_, err := o.convertToMap(args)
+	if err == nil {
+		t.Errorf("expected an error")
+	}
+	if err.Error() != "invalid annotation: ':f:g' (need k:v pair where v may be quoted)" {
+		t.Errorf("incorrect error: %v", err.Error())
+	}
+}