Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement missing HTTP host to ConsulResolver func for Connect SDK. #4392

Merged
merged 3 commits into from
Jul 16, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 93 additions & 0 deletions connect/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"fmt"
"math/rand"
"strings"
"sync"

"github.com/hashicorp/consul/agent/connect"
Expand Down Expand Up @@ -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]
}
107 changes: 107 additions & 0 deletions connect/resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
}
9 changes: 5 additions & 4 deletions connect/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
25 changes: 25 additions & 0 deletions connect/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
27 changes: 23 additions & 4 deletions website/source/docs/connect/native/go.html.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down