From 64b50ad7a17e1c1bf50d57abcfbfb1da752b25e9 Mon Sep 17 00:00:00 2001 From: Nick Cabatoff Date: Tue, 25 Jul 2023 16:26:54 -0400 Subject: [PATCH] Support mixed case consul service tags on consul storage engine (#6483) * 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 Co-authored-by: Peter Wilson --- changelog/6483.txt | 3 ++ .../consul/consul_service_registration.go | 6 ++- .../consul_service_registration_test.go | 43 +++++++++++++++++++ .../service-registration/consul.mdx | 4 +- .../docs/upgrading/upgrade-to-1.15.x.mdx | 34 +++++++++++++++ website/data/docs-nav-data.json | 7 ++- 6 files changed, 93 insertions(+), 4 deletions(-) create mode 100644 changelog/6483.txt create mode 100644 website/content/docs/upgrading/upgrade-to-1.15.x.mdx diff --git a/changelog/6483.txt b/changelog/6483.txt new file mode 100644 index 000000000000..2f0dbed1fdc8 --- /dev/null +++ b/changelog/6483.txt @@ -0,0 +1,3 @@ +```release-note:bug +storage/consul: Consul service registration tags are now case-sensitive. +``` \ No newline at end of file diff --git a/serviceregistration/consul/consul_service_registration.go b/serviceregistration/consul/consul_service_registration.go index a49ab4ff63f7..d34a77260173 100644 --- a/serviceregistration/consul/consul_service_registration.go +++ b/serviceregistration/consul/consul_service_registration.go @@ -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 @@ -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, diff --git a/serviceregistration/consul/consul_service_registration_test.go b/serviceregistration/consul/consul_service_registration_test.go index 0ced651e0242..af4d2980de8f 100644 --- a/serviceregistration/consul/consul_service_registration_test.go +++ b/serviceregistration/consul/consul_service_registration_test.go @@ -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" @@ -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) + }) + } +} diff --git a/website/content/docs/configuration/service-registration/consul.mdx b/website/content/docs/configuration/service-registration/consul.mdx index ea7db6f59036..bd73ce7e598e 100644 --- a/website/content/docs/configuration/service-registration/consul.mdx +++ b/website/content/docs/configuration/service-registration/consul.mdx @@ -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 diff --git a/website/content/docs/upgrading/upgrade-to-1.15.x.mdx b/website/content/docs/upgrading/upgrade-to-1.15.x.mdx new file mode 100644 index 000000000000..07128970f943 --- /dev/null +++ b/website/content/docs/upgrading/upgrade-to-1.15.x.mdx @@ -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 + + + diff --git a/website/data/docs-nav-data.json b/website/data/docs-nav-data.json index 66d59811c1dd..9e1b1cfe151a 100644 --- a/website/data/docs-nav-data.json +++ b/website/data/docs-nav-data.json @@ -127,7 +127,7 @@ } ] }, - + { "title": "Token Authentication", "path": "internals/token" @@ -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"