mirror of
				https://github.com/optim-enterprises-bv/vault.git
				synced 2025-10-30 18:17:55 +00:00 
			
		
		
		
	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 <ncabatoff@hashicorp.com> Co-authored-by: Peter Wilson <peter.wilson@hashicorp.com>
This commit is contained in:
		
							
								
								
									
										3
									
								
								changelog/6483.txt
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										3
									
								
								changelog/6483.txt
									
									
									
									
									
										Normal file
									
								
							| @@ -0,0 +1,3 @@ | |||||||
|  | ```release-note:bug | ||||||
|  | storage/consul: Consul service registration tags are now case-sensitive. | ||||||
|  | ``` | ||||||
| @@ -88,6 +88,10 @@ type serviceRegistration struct { | |||||||
|  |  | ||||||
| // NewConsulServiceRegistration constructs a Consul-based ServiceRegistration. | // NewConsulServiceRegistration constructs a Consul-based ServiceRegistration. | ||||||
| func NewServiceRegistration(conf map[string]string, logger log.Logger, state sr.State) (sr.ServiceRegistration, error) { | 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 | 	// Allow admins to disable consul integration | ||||||
| 	disableReg, ok := conf["disable_registration"] | 	disableReg, ok := conf["disable_registration"] | ||||||
| 	var disableRegistration bool | 	var disableRegistration bool | ||||||
| @@ -168,7 +172,7 @@ func NewServiceRegistration(conf map[string]string, logger log.Logger, state sr. | |||||||
|  |  | ||||||
| 		logger:              logger, | 		logger:              logger, | ||||||
| 		serviceName:         service, | 		serviceName:         service, | ||||||
| 		serviceTags:         strutil.ParseDedupLowercaseAndSortStrings(tags, ","), | 		serviceTags:         strutil.ParseDedupAndSortStrings(tags, ","), | ||||||
| 		serviceAddress:      serviceAddr, | 		serviceAddress:      serviceAddr, | ||||||
| 		checkTimeout:        checkTimeout, | 		checkTimeout:        checkTimeout, | ||||||
| 		disableRegistration: disableRegistration, | 		disableRegistration: disableRegistration, | ||||||
|   | |||||||
| @@ -10,6 +10,8 @@ import ( | |||||||
| 	"testing" | 	"testing" | ||||||
| 	"time" | 	"time" | ||||||
|  |  | ||||||
|  | 	"github.com/stretchr/testify/require" | ||||||
|  |  | ||||||
| 	"github.com/go-test/deep" | 	"github.com/go-test/deep" | ||||||
| 	"github.com/hashicorp/consul/api" | 	"github.com/hashicorp/consul/api" | ||||||
| 	log "github.com/hashicorp/go-hclog" | 	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) | ||||||
|  | 		}) | ||||||
|  | 	} | ||||||
|  | } | ||||||
|   | |||||||
| @@ -82,8 +82,8 @@ at Consul's service discovery layer. | |||||||
| - `service` `(string: "vault")` – Specifies the name of the service to register | - `service` `(string: "vault")` – Specifies the name of the service to register | ||||||
|   in Consul. |   in Consul. | ||||||
|  |  | ||||||
| - `service_tags` `(string: "")` – Specifies a comma-separated list of tags to | - `service_tags` `(string: "")` – Specifies a comma-separated list of case-sensitive | ||||||
|   attach to the service registration in Consul. |   tags to attach to the service registration in Consul. | ||||||
|  |  | ||||||
| - `service_address` `(string: nil)` – Specifies a service-specific address to | - `service_address` `(string: nil)` – Specifies a service-specific address to | ||||||
|   set on the service registration in Consul. If unset, Vault will use what it |   set on the service registration in Consul. If unset, Vault will use what it | ||||||
|   | |||||||
							
								
								
									
										34
									
								
								website/content/docs/upgrading/upgrade-to-1.15.x.mdx
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										34
									
								
								website/content/docs/upgrading/upgrade-to-1.15.x.mdx
									
									
									
									
									
										Normal file
									
								
							| @@ -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 | ||||||
|  |  | ||||||
|  |  | ||||||
|  |  | ||||||
| @@ -1945,6 +1945,11 @@ | |||||||
|         "title": "Upgrade Plugins", |         "title": "Upgrade Plugins", | ||||||
|         "path": "upgrading/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", |         "title": "Upgrade to 1.14.x", | ||||||
|         "path": "upgrading/upgrade-to-1.14.x" |         "path": "upgrading/upgrade-to-1.14.x" | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 Nick Cabatoff
					Nick Cabatoff