Skip to content

Commit

Permalink
Support mixed case consul service tags on consul storage engine (#6483)
Browse files Browse the repository at this point in the history
* When support for service tags was added, the only way we had to parse
and dedup a list of strings also forced them to be lowercase.  Now there's
another helper func that doesn't smash the case so use that instead.

* update Consul 'service_tag' documentation to include case sensitivity

* added upgrade guide for 1.15

* test for service tags

Co-authored-by: Nick Cabatoff <[email protected]>
Co-authored-by: Peter Wilson <[email protected]>
  • Loading branch information
ncabatoff and peteski22 authored Jul 25, 2023
1 parent 6ff7d38 commit 64b50ad
Show file tree
Hide file tree
Showing 6 changed files with 93 additions and 4 deletions.
3 changes: 3 additions & 0 deletions changelog/6483.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
storage/consul: Consul service registration tags are now case-sensitive.
```
6 changes: 5 additions & 1 deletion serviceregistration/consul/consul_service_registration.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ type serviceRegistration struct {

// NewConsulServiceRegistration constructs a Consul-based ServiceRegistration.
func NewServiceRegistration(conf map[string]string, logger log.Logger, state sr.State) (sr.ServiceRegistration, error) {
if logger == nil {
return nil, errors.New("logger is required")
}

// Allow admins to disable consul integration
disableReg, ok := conf["disable_registration"]
var disableRegistration bool
Expand Down Expand Up @@ -168,7 +172,7 @@ func NewServiceRegistration(conf map[string]string, logger log.Logger, state sr.

logger: logger,
serviceName: service,
serviceTags: strutil.ParseDedupLowercaseAndSortStrings(tags, ","),
serviceTags: strutil.ParseDedupAndSortStrings(tags, ","),
serviceAddress: serviceAddr,
checkTimeout: checkTimeout,
disableRegistration: disableRegistration,
Expand Down
43 changes: 43 additions & 0 deletions serviceregistration/consul/consul_service_registration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import (
"testing"
"time"

"github.com/stretchr/testify/require"

"github.com/go-test/deep"
"github.com/hashicorp/consul/api"
log "github.com/hashicorp/go-hclog"
Expand Down Expand Up @@ -563,3 +565,44 @@ func TestConsul_serviceID(t *testing.T) {
}
}
}

// TestConsul_NewServiceRegistration_serviceTags ensures that we do not modify
// the case of any 'service_tags' set by the config.
// We do expect tags to be sorted in lexicographic order (A-Z).
func TestConsul_NewServiceRegistration_serviceTags(t *testing.T) {
tests := map[string]struct {
Tags string
ExpectedTags []string
}{
"lowercase": {
Tags: "foo,bar",
ExpectedTags: []string{"bar", "foo"},
},
"uppercase": {
Tags: "FOO,BAR",
ExpectedTags: []string{"BAR", "FOO"},
},
"PascalCase": {
Tags: "FooBar, Feedface",
ExpectedTags: []string{"Feedface", "FooBar"},
},
}

for name, tc := range tests {
name := name
tc := tc
t.Run(name, func(t *testing.T) {
t.Parallel()

cfg := map[string]string{"service_tags": tc.Tags}
logger := logging.NewVaultLogger(log.Trace)
be, err := NewServiceRegistration(cfg, logger, sr.State{})
require.NoError(t, err)
require.NotNil(t, be)
c, ok := be.(*serviceRegistration)
require.True(t, ok)
require.NotNil(t, c)
require.Equal(t, tc.ExpectedTags, c.serviceTags)
})
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ at Consul's service discovery layer.
- `service` `(string: "vault")` – Specifies the name of the service to register
in Consul.

- `service_tags` `(string: "")` – Specifies a comma-separated list of tags to
attach to the service registration in Consul.
- `service_tags` `(string: "")` – Specifies a comma-separated list of case-sensitive
tags to attach to the service registration in Consul.

- `service_address` `(string: nil)` – Specifies a service-specific address to
set on the service registration in Consul. If unset, Vault will use what it
Expand Down
34 changes: 34 additions & 0 deletions website/content/docs/upgrading/upgrade-to-1.15.x.mdx
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
---
layout: docs
page_title: Upgrade to Vault 1.15.x - Guides
description: |-
Deprecations, important or breaking changes, and remediation recommendations
for anyone upgrading to 1.15.x from Vault 1.14.x.
---

# Overview

The Vault 1.15.x upgrade guide contains information on deprecations, important
or breaking changes, and remediation recommendations for anyone upgrading from
Vault 1.14. **Please read carefully**.

## Consul service registration

As of version 1.15, `service_tags` supplied to Vault for the purpose of [Consul
service registration](/vault/docs/configuration/service-registration/consul#service_tags)
will be **case-sensitive**.

In previous versions of Vault tags were converted to lowercase which led to issues,
for example when tags contained Traefik rules which use case-sensitive method names
such as `Host()`.

If you previously used Consul service registration tags ignoring case, or relied
on the lowercase tags created by Vault, then this change may cause unexpected behavior.

Please audit your Consul storage stanza to ensure that you either:

* Manually convert your `service_tags` to lowercase if required
* Ensure that any system that relies on the tags is aware of the new case-preserving behavior



7 changes: 6 additions & 1 deletion website/data/docs-nav-data.json
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@
}
]
},

{
"title": "Token Authentication",
"path": "internals/token"
Expand Down Expand Up @@ -1945,6 +1945,11 @@
"title": "Upgrade Plugins",
"path": "upgrading/plugins"
},
{
"title": "Upgrade to 1.15.x",
"path": "upgrading/upgrade-to-1.15.x",
"hidden": true
},
{
"title": "Upgrade to 1.14.x",
"path": "upgrading/upgrade-to-1.14.x"
Expand Down

0 comments on commit 64b50ad

Please sign in to comment.