Skip to content

Commit

Permalink
Support escaped commas in service tags for connect (#1532)
Browse files Browse the repository at this point in the history
* support escaped commas in service tag annotations
  • Loading branch information
kschoche authored Sep 27, 2022
1 parent d4c3b7d commit c220618
Show file tree
Hide file tree
Showing 7 changed files with 118 additions and 101 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,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)

Expand Down
53 changes: 2 additions & 51 deletions control-plane/catalog/to-consul/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
44 changes: 0 additions & 44 deletions control-plane/catalog/to-consul/resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
5 changes: 3 additions & 2 deletions control-plane/connect-inject/endpoints_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions control-plane/connect-inject/endpoints_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down
56 changes: 56 additions & 0 deletions control-plane/helper/parsetags/parsetags.go
Original file line number Diff line number Diff line change
@@ -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
}
51 changes: 51 additions & 0 deletions control-plane/helper/parsetags/parsetags_test.go
Original file line number Diff line number Diff line change
@@ -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))
}
}

0 comments on commit c220618

Please sign in to comment.