diff --git a/CHANGELOG.md b/CHANGELOG.md index 5da5573269..aedc614254 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/control-plane/catalog/to-consul/resource.go b/control-plane/catalog/to-consul/resource.go index dd170e4990..5188f2ccb5 100644 --- a/control-plane/catalog/to-consul/resource.go +++ b/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 +} diff --git a/control-plane/catalog/to-consul/resource_test.go b/control-plane/catalog/to-consul/resource_test.go index 461f87bd0c..6bc99eb5f7 100644 --- a/control-plane/catalog/to-consul/resource_test.go +++ b/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{