diff --git a/CHANGELOG.md b/CHANGELOG.md index 6bb0faae72..5112097916 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/control-plane/catalog/to-consul/resource.go b/control-plane/catalog/to-consul/resource.go index 03de1d26bb..239e2f5db9 100644 --- a/control-plane/catalog/to-consul/resource.go +++ b/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 -} diff --git a/control-plane/catalog/to-consul/resource_test.go b/control-plane/catalog/to-consul/resource_test.go index 1a4203725a..28335dea27 100644 --- a/control-plane/catalog/to-consul/resource_test.go +++ b/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{ diff --git a/control-plane/connect-inject/endpoints_controller.go b/control-plane/connect-inject/endpoints_controller.go index 04c8e70a26..9b37ca3bcf 100644 --- a/control-plane/connect-inject/endpoints_controller.go +++ b/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 diff --git a/control-plane/connect-inject/endpoints_controller_test.go b/control-plane/connect-inject/endpoints_controller_test.go index 19cda9a206..abbad4b2a6 100644 --- a/control-plane/connect-inject/endpoints_controller_test.go +++ b/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{ diff --git a/control-plane/helper/parsetags/parsetags.go b/control-plane/helper/parsetags/parsetags.go new file mode 100644 index 0000000000..e9d9625338 --- /dev/null +++ b/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 +} diff --git a/control-plane/helper/parsetags/parsetags_test.go b/control-plane/helper/parsetags/parsetags_test.go new file mode 100644 index 0000000000..f403a2f711 --- /dev/null +++ b/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)) + } +}