Support escaped commas in service tags for connect

CHANGELOG.md

@@ -13,6 +13,8 @@ BUG FIXES:
 IMPROVEMENTS:
 * Helm:
   * API Gateway: Set primary datacenter flag when deploying controller into secondary datacenter with federation enabled [[GH-1511](https://github.com/hashicorp/consul-k8s/pull/1511)]
+* Control-plane:
+  * Support escaped commas in service tag annotations for pods which use `consul.hashicorp.com/connect-service-tags` or `consul.hashicorp.com/service-tags`. [[GH-1532](https://github.com/hashicorp/consul-k8s/pull/1532)]
 
 ## 0.48.0 (September 01, 2022)
 

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

@@ -9,6 +9,7 @@ import (
 
 	mapset "github.com/deckarep/golang-set"
 	"github.com/hashicorp/consul-k8s/control-plane/helper/controller"
+	"github.com/hashicorp/consul-k8s/control-plane/helper/parsetags"
 	"github.com/hashicorp/consul-k8s/control-plane/namespaces"
 	consulapi "github.com/hashicorp/consul/api"
 	"github.com/hashicorp/go-hclog"
@@ -438,7 +439,7 @@ func (t *ServiceResource) generateRegistrations(key string) {
 
 	// Parse any additional tags
 	if rawTags, ok := svc.Annotations[annotationServiceTags]; ok {
-		baseService.Tags = append(baseService.Tags, parseTags(rawTags)...)
+		baseService.Tags = append(baseService.Tags, parsetags.ParseTags(rawTags)...)
 	}
 
 	// Parse any additional meta
@@ -787,53 +788,3 @@ 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

@@ -1490,50 +1490,6 @@ 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{

control-plane/connect-inject/endpoints_controller.go

@@ -13,6 +13,7 @@ import (
 	mapset "github.com/deckarep/golang-set"
 	"github.com/go-logr/logr"
 	"github.com/hashicorp/consul-k8s/control-plane/consul"
+	"github.com/hashicorp/consul-k8s/control-plane/helper/parsetags"
 	"github.com/hashicorp/consul-k8s/control-plane/namespaces"
 	"github.com/hashicorp/consul/api"
 	"github.com/hashicorp/go-multierror"
@@ -1213,11 +1214,11 @@ func isLabeledIgnore(labels map[string]string) bool {
 func consulTags(pod corev1.Pod) []string {
 	var tags []string
 	if raw, ok := pod.Annotations[annotationTags]; ok && raw != "" {
-		tags = strings.Split(raw, ",")
+		tags = append(tags, parsetags.ParseTags(raw)...)
 	}
 	// Get the tags from the deprecated tags annotation and combine.
 	if raw, ok := pod.Annotations[annotationConnectTags]; ok && raw != "" {
-		tags = append(tags, strings.Split(raw, ",")...)
+		tags = append(tags, parsetags.ParseTags(raw)...)
 	}
 
 	var interpolatedTags []string

control-plane/connect-inject/endpoints_controller_test.go

@@ -1522,8 +1522,8 @@ func TestReconcileCreateEndpoint(t *testing.T) {
 				pod1.Annotations[fmt.Sprintf("%sname", annotationMeta)] = "abc"
 				pod1.Annotations[fmt.Sprintf("%sversion", annotationMeta)] = "2"
 				pod1.Annotations[fmt.Sprintf("%spod_name", annotationMeta)] = "$POD_NAME"
-				pod1.Annotations[annotationTags] = "abc,123,$POD_NAME"
-				pod1.Annotations[annotationConnectTags] = "def,456,$POD_NAME"
+				pod1.Annotations[annotationTags] = "abc\\,123,$POD_NAME"
+				pod1.Annotations[annotationConnectTags] = "def\\,456,$POD_NAME"
 				pod1.Annotations[annotationUpstreams] = "upstream1:1234"
 				pod1.Annotations[annotationEnableMetrics] = "true"
 				pod1.Annotations[annotationPrometheusScrapePort] = "12345"
@@ -1567,7 +1567,7 @@ func TestReconcileCreateEndpoint(t *testing.T) {
 						MetaKeyKubeNS:          "default",
 						MetaKeyManagedBy:       managedByValue,
 					},
-					ServiceTags: []string{"abc", "123", "pod1", "def", "456", "pod1"},
+					ServiceTags: []string{"abc,123", "pod1", "def,456", "pod1"},
 				},
 			},
 			expectedProxySvcInstances: []*api.CatalogService{
@@ -1601,7 +1601,7 @@ func TestReconcileCreateEndpoint(t *testing.T) {
 						MetaKeyKubeNS:          "default",
 						MetaKeyManagedBy:       managedByValue,
 					},
-					ServiceTags: []string{"abc", "123", "pod1", "def", "456", "pod1"},
+					ServiceTags: []string{"abc,123", "pod1", "def,456", "pod1"},
 				},
 			},
 			expectedAgentHealthChecks: []*api.AgentCheck{

control-plane/helper/parsetags/parsetags.go

@@ -0,0 +1,56 @@
+package parsetags
+
+import (
+	"fmt"
+	"strings"
+)
+
+// 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/helper/parsetags/parsetags_test.go

@@ -0,0 +1,51 @@
+package parsetags
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+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))
+	}
+}