From af2901130db3a55775b4622cdf8fb3d4acac8130 Mon Sep 17 00:00:00 2001 From: Paul Banks Date: Fri, 13 Jul 2018 22:38:35 +0100 Subject: [PATCH] Implement missing HTTP host to ConsulResolver func for Connect SDK. --- connect/resolver.go | 93 ++++++++++++++++++++++++++++++++++ connect/resolver_test.go | 107 +++++++++++++++++++++++++++++++++++++++ connect/service.go | 9 ++-- connect/service_test.go | 25 +++++++++ 4 files changed, 230 insertions(+), 4 deletions(-) diff --git a/connect/resolver.go b/connect/resolver.go index 2923b81f33..67f876fb80 100644 --- a/connect/resolver.go +++ b/connect/resolver.go @@ -4,6 +4,7 @@ import ( "context" "fmt" "math/rand" + "strings" "sync" "github.com/hashicorp/consul/agent/connect" @@ -202,3 +203,95 @@ func (cr *ConsulResolver) queryOptions(ctx context.Context) *api.QueryOptions { } return q.WithContext(ctx) } + +// ConsulResolverFromAddrFunc returns a function for constructing ConsulResolver +// from a consul DNS formatted hostname (e.g. foo.service.consul or +// foo.query.consul). +// +// Note, the returned ConsulResolver resolves the query via regular agent HTTP +// discovery API. DNS is not needed or used for discovery, only the hostname +// format re-used for consistency. +func ConsulResolverFromAddrFunc(client *api.Client) func(addr string) (Resolver, error) { + // Capture client dependency + return func(addr string) (Resolver, error) { + // Http clients might provide hostname and port + host := strings.ToLower(stripPort(addr)) + + // For now we force use of `.consul` TLD regardless of the configured domain + // on the cluster. That's because we don't know that domain here and it + // would be really complicated to discover it inline here. We do however + // need to be able to distingush a hostname with the optional datacenter + // segment which we can't do unambiguously if we allow arbitrary trailing + // domains. + domain := ".consul" + if !strings.HasSuffix(host, domain) { + return nil, fmt.Errorf("invalid Consul DNS domain: note Connect SDK " + + "currently requires use of .consul domain even if cluster is " + + "configured with a different domain.") + } + + // Remove the domain suffix + host = host[0 : len(host)-len(domain)] + + parts := strings.Split(host, ".") + numParts := len(parts) + + r := &ConsulResolver{ + Client: client, + Namespace: "default", + } + + // Note that 3 segments may be a valid DNS name like + // ..service.consul but not one we support, it might also be + // .service..consul which we do want to support so we + // have to figure out if the last segment is supported keyword and if not + // check if the supported keyword is further up... + + // To simplify logic for now, we must match one of the following (not domain + // is stripped): + // .[service|query] + // .[service|query]. + if numParts < 2 || numParts > 3 || !supportedTypeLabel(parts[1]) { + return nil, fmt.Errorf("unsupported Consul DNS domain: must be either " + + ".service[.].consul or " + + ".query[.].consul") + } + + if numParts == 3 { + // Must be datacenter case + r.Datacenter = parts[2] + } + + // By know we must have a supported query type which means at least 2 + // elements with first 2 being name, and type respectively. + r.Name = parts[0] + switch parts[1] { + case "service": + r.Type = ConsulResolverTypeService + case "query": + r.Type = ConsulResolverTypePreparedQuery + default: + // This should never happen (tm) unless the supportedTypeLabel + // implementation is changed and this switch isn't. + return nil, fmt.Errorf("invalid discovery type") + } + + return r, nil + } +} + +func supportedTypeLabel(label string) bool { + return label == "service" || label == "query" +} + +// stripPort copied from net/url/url.go +func stripPort(hostport string) string { + colon := strings.IndexByte(hostport, ':') + if colon == -1 { + return hostport + } + if i := strings.IndexByte(hostport, ']'); i != -1 { + return strings.TrimPrefix(hostport[:i], "[") + } + return hostport[:colon] +} diff --git a/connect/resolver_test.go b/connect/resolver_test.go index 6a1ca983be..5f319eb770 100644 --- a/connect/resolver_test.go +++ b/connect/resolver_test.go @@ -216,3 +216,110 @@ func TestConsulResolver_Resolve(t *testing.T) { }) } } + +func TestConsulResolverFromAddrFunc(t *testing.T) { + // Don't need an actual instance since we don't do the service discovery but + // we do want to assert the client is pass through correctly. + client, err := api.NewClient(api.DefaultConfig()) + require.NoError(t, err) + + tests := []struct { + name string + addr string + want Resolver + wantErr string + }{ + { + name: "service", + addr: "foo.service.consul", + want: &ConsulResolver{ + Client: client, + Namespace: "default", + Name: "foo", + Type: ConsulResolverTypeService, + }, + }, + { + name: "query", + addr: "foo.query.consul", + want: &ConsulResolver{ + Client: client, + Namespace: "default", + Name: "foo", + Type: ConsulResolverTypePreparedQuery, + }, + }, + { + name: "service with dc", + addr: "foo.service.dc2.consul", + want: &ConsulResolver{ + Client: client, + Datacenter: "dc2", + Namespace: "default", + Name: "foo", + Type: ConsulResolverTypeService, + }, + }, + { + name: "query with dc", + addr: "foo.query.dc2.consul", + want: &ConsulResolver{ + Client: client, + Datacenter: "dc2", + Namespace: "default", + Name: "foo", + Type: ConsulResolverTypePreparedQuery, + }, + }, + { + name: "invalid host:port", + addr: "%%%", + wantErr: "invalid Consul DNS domain", + }, + { + name: "custom domain", + addr: "foo.service.my-consul.com", + wantErr: "invalid Consul DNS domain", + }, + { + name: "unsupported query type", + addr: "foo.connect.consul", + wantErr: "unsupported Consul DNS domain", + }, + { + name: "unsupported query type and datacenter", + addr: "foo.connect.dc1.consul", + wantErr: "unsupported Consul DNS domain", + }, + { + name: "unsupported query type and datacenter", + addr: "foo.connect.dc1.consul", + wantErr: "unsupported Consul DNS domain", + }, + { + name: "unsupported tag filter", + addr: "tag1.foo.service.consul", + wantErr: "unsupported Consul DNS domain", + }, + { + name: "unsupported tag filter with DC", + addr: "tag1.foo.service.dc1.consul", + wantErr: "unsupported Consul DNS domain", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require := require.New(t) + + fn := ConsulResolverFromAddrFunc(client) + got, gotErr := fn(tt.addr) + if tt.wantErr != "" { + require.Error(gotErr) + require.Contains(gotErr.Error(), tt.wantErr) + } else { + require.NoError(gotErr) + require.Equal(tt.want, got) + } + }) + } +} diff --git a/connect/service.go b/connect/service.go index 462f1fb472..7bdaf3c60f 100644 --- a/connect/service.go +++ b/connect/service.go @@ -69,10 +69,11 @@ func NewService(serviceName string, client *api.Client) (*Service, error) { func NewServiceWithLogger(serviceName string, client *api.Client, logger *log.Logger) (*Service, error) { s := &Service{ - service: serviceName, - client: client, - logger: logger, - tlsCfg: newDynamicTLSConfig(defaultTLSConfig()), + service: serviceName, + client: client, + logger: logger, + tlsCfg: newDynamicTLSConfig(defaultTLSConfig()), + httpResolverFromAddr: ConsulResolverFromAddrFunc(client), } // Set up root and leaf watches diff --git a/connect/service_test.go b/connect/service_test.go index b18bf4bb41..ce3b7c7a72 100644 --- a/connect/service_test.go +++ b/connect/service_test.go @@ -252,3 +252,28 @@ func TestService_HTTPClient(t *testing.T) { } }) } + +func TestService_HasDefaultHTTPResolverFromAddr(t *testing.T) { + + client, err := api.NewClient(api.DefaultConfig()) + require.NoError(t, err) + + s, err := NewService("foo", client) + require.NoError(t, err) + + // Sanity check this is actually set in constructor since we always override + // it in tests. Full tests of the resolver func are in resolver_test.go + require.NotNil(t, s.httpResolverFromAddr) + + fn := s.httpResolverFromAddr + + expected := &ConsulResolver{ + Client: client, + Namespace: "default", + Name: "foo", + Type: ConsulResolverTypeService, + } + got, err := fn("foo.service.consul") + require.NoError(t, err) + require.Equal(t, expected, got) +}