Merge pull request #4392 from hashicorp/connect-sdk-http

Implement missing HTTP host to ConsulResolver func for Connect SDK.

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
+		// <tag>.<service>.service.consul but not one we support, it might also be
+		// <service>.service.<datacenter>.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):
+		//  <name>.[service|query]
+		//  <name>.[service|query].<dc>
+		if numParts < 2 || numParts > 3 || !supportedTypeLabel(parts[1]) {
+			return nil, fmt.Errorf("unsupported Consul DNS domain: must be either " +
+				"<name>.service[.<datacenter>].consul or " +
+				"<name>.query[.<datacenter>].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]
+}

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)
+			}
+		})
+	}
+}

connect/service.go

@@ -73,6 +73,7 @@ func NewServiceWithLogger(serviceName string, client *api.Client,
 		client:               client,
 		logger:               logger,
 		tlsCfg:               newDynamicTLSConfig(defaultTLSConfig()),
+		httpResolverFromAddr: ConsulResolverFromAddrFunc(client),
 	}
 
 	// Set up root and leaf watches

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)
+}

website/source/docs/connect/native/go.html.md

@@ -157,14 +157,33 @@ The HTTP client configuration automatically sends the correct client
 certificate, verifies the server certificate, and manages background
 goroutines for updating our certificates as necessary.
 
--> **Important:** The HTTP client _requires_ the hostname is a Consul
-DNS name. Static IP addresses and external DNS cannot be used with the
-HTTP client. For these values, please use `svc.Dial` directly.
-
 If the application already uses a manually constructed `*http.Client`,
 the `svc.HTTPDialTLS` function can be used to configure the
 `http.Transport.DialTLS` field to achieve equivalent behavior.
 
+### Hostname Requirements
+
+The hostname used in the request URL is used to identify the logical service
+discovery mechanism for the target. **It's not actually resolved via DNS** but
+used as a logical identifier for a Consul service discovery mechanism. It has
+the following specific limitations:
+
+ * The scheme must be `https://`.
+ * It must be a Consul DNS name in one of the following forms:
+   * `<name>.service[.<datacenter>].consul` to discover a healthy service
+     instance for a given service.
+   * `<name>.query[.<datacenter>].consul` to discover an instance via
+     [Prepared Query](/api/query.html).
+ * The top-level domain _must_ be `.consul` even if your cluster has a custom
+   `domain` configured for it's DNS interface. This might be relaxed in the
+   future.
+ * Tag filters for services are not currently supported (i.e.
+   `tag1.web.service.consul`) however the same behaviour can be achieved using a
+   prepared query.
+ * External DNS names, raw IP addresses and so on will cause an error and should
+   be fetched using a separate `HTTPClient`.
+
+
 ## Raw TLS Connection
 
 For a raw `net.Conn` TLS connection, the `svc.Dial` function can be used.