Support commas in tag annotation.

CHANGELOG.md

@@ -14,6 +14,7 @@ IMPROVEMENTS:
 * Control Plane
   * Support the value `$POD_NAME` for the annotation `consul.hashicorp.com/service-meta-*` that will now be interpolated and set to the pod's name in the service's metadata. [[GH-982](https://github.com/hashicorp/consul-k8s/pull/982)]
   * Allow managing Consul sidecar resources via annotations. [[GH-956](https://github.com/hashicorp/consul-k8s/pull/956)]
+  * Support using a backslash to escape commas in `consul.hashicorp.com/service-tags` annotation. [[GH-983](https://github.com/hashicorp/consul-k8s/pull/983)]
 
 BUG FIXES:
 * Helm

control-plane/catalog/to-consul/resource.go

@@ -434,10 +434,8 @@ func (t *ServiceResource) generateRegistrations(key string) {
 	}
 
 	// Parse any additional tags
-	if tags, ok := svc.Annotations[annotationServiceTags]; ok {
-		for _, t := range strings.Split(tags, ",") {
-			baseService.Tags = append(baseService.Tags, strings.TrimSpace(t))
-		}
+	if rawTags, ok := svc.Annotations[annotationServiceTags]; ok {
+		baseService.Tags = append(baseService.Tags, parseTags(rawTags)...)
 	}
 
 	// Parse any additional meta
@@ -773,3 +771,53 @@ func (t *ServiceResource) addPrefixAndK8SNamespace(name, namespace string) strin
 
 	return name
 }
+
+// parseTags parses the tags annotation into a slice of tags.
+// Tags are split on commas (except for escaped commas "\,").
+func parseTags(tagsAnno string) []string {
+
+	// This algorithm parses the tagsAnno string into a slice of strings.
+	// Ideally we'd just split on commas but since Consul tags support commas,
+	// we allow users to escape commas so they're included in the tag, e.g.
+	// the annotation "tag\,with\,commas,tag2" will become the tags:
+	// ["tag,with,commas", "tag2"].
+
+	var tags []string
+	// nextTag is built up char by char until we see a comma. Then we
+	// append it to tags.
+	var nextTag string
+
+	for _, runeChar := range tagsAnno {
+		runeStr := fmt.Sprintf("%c", runeChar)
+
+		// Not a comma, just append to nextTag.
+		if runeStr != "," {
+			nextTag += runeStr
+			continue
+		}
+
+		// Reached a comma but there's nothing in nextTag,
+		// skip. (e.g. "a,,b" => ["a", "b"])
+		if len(nextTag) == 0 {
+			continue
+		}
+
+		// Check if the comma was escaped comma, e.g. "a\,b".
+		if string(nextTag[len(nextTag)-1]) == `\` {
+			// Replace the backslash with a comma.
+			nextTag = nextTag[0:len(nextTag)-1] + ","
+			continue
+		}
+
+		// Non-escaped comma. We're ready to push nextTag onto tags and reset nextTag.
+		tags = append(tags, strings.TrimSpace(nextTag))
+		nextTag = ""
+	}
+
+	// We're done the loop but nextTag still contains the last tag.
+	if len(nextTag) > 0 {
+		tags = append(tags, strings.TrimSpace(nextTag))
+	}
+
+	return tags
+}

control-plane/catalog/to-consul/resource_test.go

@@ -566,7 +566,7 @@ func TestServiceResource_lbAnnotatedTags(t *testing.T) {
 
 	// Insert an LB service
 	svc := lbService("foo", metav1.NamespaceDefault, "1.2.3.4")
-	svc.Annotations[annotationServiceTags] = "one, two,three"
+	svc.Annotations[annotationServiceTags] = `one, leadingwhitespace,trailingwhitespace ,\,leadingcomma,trailingcomma\,,middle\,comma,,`
 	_, err := client.CoreV1().Services(metav1.NamespaceDefault).Create(context.Background(), svc, metav1.CreateOptions{})
 	require.NoError(t, err)
 
@@ -576,7 +576,7 @@ func TestServiceResource_lbAnnotatedTags(t *testing.T) {
 		defer syncer.Unlock()
 		actual := syncer.Registrations
 		require.Len(r, actual, 1)
-		require.Equal(r, []string{"k8s", "one", "two", "three"}, actual[0].Service.Tags)
+		require.Equal(r, []string{"k8s", "one", "leadingwhitespace", "trailingwhitespace", ",leadingcomma", "trailingcomma,", "middle,comma"}, actual[0].Service.Tags)
 	})
 }
 
@@ -1453,6 +1453,50 @@ func TestServiceResource_MirroredPrefixNamespace(t *testing.T) {
 	})
 }
 
+func TestParseTags(t *testing.T) {
+	cases := []struct {
+		tagsAnno string
+		exp      []string
+	}{
+		{
+			"tag",
+			[]string{"tag"},
+		},
+		{
+			",,removes,,empty,elems,,",
+			[]string{"removes", "empty", "elems"},
+		},
+		{
+			"removes , white  ,space ",
+			[]string{"removes", "white", "space"},
+		},
+		{
+			`\,leading,comma`,
+			[]string{",leading", "comma"},
+		},
+		{
+			`trailing,comma\,`,
+			[]string{"trailing", "comma,"},
+		},
+		{
+			`mid\,dle,com\,ma`,
+			[]string{"mid,dle", "com,ma"},
+		},
+		{
+			`\,\,multi\,\,,\,com\,\,ma`,
+			[]string{",,multi,,", ",com,,ma"},
+		},
+		{
+			`  every\,\,   ,  thing  `,
+			[]string{"every,,", "thing"},
+		},
+	}
+
+	for _, c := range cases {
+		require.Equal(t, c.exp, parseTags(c.tagsAnno))
+	}
+}
+
 // lbService returns a Kubernetes service of type LoadBalancer.
 func lbService(name, namespace, lbIP string) *apiv1.Service {
 	return &apiv1.Service{