luxolus
/
open-consul
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

Ensure Consul is IPv6 compliant (#5468)

This commit is contained in:
Pierre Souchay 2019-06-04 16:02:38 +02:00 committed by Matt Keeler
parent 923448f00e
commit 1da1825056
10 changed files with 207 additions and 115 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
agent/agent.go
View File

@ -2537,11 +2537,14 @@ func (a *Agent) addProxyLocked(proxy *structs.ConnectManagedProxy, persist, From
chkAddr := a.resolveProxyCheckAddress(proxyCfg)
chkTypes := []*structs.CheckType{}
if chkAddr != "" {
bindPort, ok := proxyCfg["bind_port"].(int)
if !ok {
return fmt.Errorf("Cannot convert bind_port=%v to an int for creating TCP Check for address %s", proxyCfg["bind_port"], chkAddr)
}
chkTypes = []*structs.CheckType{
&structs.CheckType{
Name: "Connect Proxy Listening",
TCP: fmt.Sprintf("%s:%d", chkAddr,
proxyCfg["bind_port"]),
Name: "Connect Proxy Listening",
TCP: ipaddr.FormatAddressPort(chkAddr, bindPort),
Interval: 10 * time.Second,
},
}

229
agent/agent_endpoint_test.go
View File

@ -2479,17 +2479,26 @@ func TestAgent_RegisterService(t *testing.T) {
func TestAgent_RegisterService_TranslateKeys(t *testing.T) {
t.Parallel()
a := NewTestAgent(t, t.Name(), `
tests := []struct {
ip string
expectedTCPCheckStart string
}{
{"127.0.0.1", "127.0.0.1:"}, // private network address
{"::1", "[::1]:"}, // shared address space
}
for _, tt := range tests {
t.Run(tt.ip, func(t *testing.T) {
a := NewTestAgent(t, t.Name(), `
connect {
proxy {
allow_managed_api_registration = true
}
}
`)
defer a.Shutdown()
testrpc.WaitForTestAgent(t, a.RPC, "dc1")
defer a.Shutdown()
testrpc.WaitForTestAgent(t, a.RPC, "dc1")
json := `
json := `
{
"name":"test",
"port":8000,
@ -2499,14 +2508,14 @@ func TestAgent_RegisterService_TranslateKeys(t *testing.T) {
"enable_tag_override": "meta is 'opaque' so should not get translated"
},
"kind": "connect-proxy",` +
// Note the uppercase P is important here - it ensures translation works
// correctly in case-insensitive way. Without it this test can pass even
// when translation is broken for other valid inputs.
`"Proxy": {
// Note the uppercase P is important here - it ensures translation works
// correctly in case-insensitive way. Without it this test can pass even
// when translation is broken for other valid inputs.
`"Proxy": {
"destination_service_name": "web",
"destination_service_id": "web",
"local_service_port": 1234,
"local_service_address": "127.0.0.1",
"local_service_address": "` + tt.ip + `",
"config": {
"destination_type": "proxy.config is 'opaque' so should not get translated"
},
@ -2515,7 +2524,7 @@ func TestAgent_RegisterService_TranslateKeys(t *testing.T) {
"destination_type": "service",
"destination_namespace": "default",
"destination_name": "db",
"local_bind_address": "127.0.0.1",
"local_bind_address": "` + tt.ip + `",
"local_bind_port": 1234,
"config": {
"destination_type": "proxy.upstreams.config is 'opaque' so should not get translated"
@ -2534,7 +2543,7 @@ func TestAgent_RegisterService_TranslateKeys(t *testing.T) {
"destination_type": "service",
"destination_namespace": "default",
"destination_name": "db",
"local_bind_address": "127.0.0.1",
"local_bind_address": "` + tt.ip + `",
"local_bind_port": 1234,
"config": {
"destination_type": "connect.proxy.upstreams.config is 'opaque' so should not get translated"
@ -2555,13 +2564,13 @@ func TestAgent_RegisterService_TranslateKeys(t *testing.T) {
"destination_service_name": "test",
"destination_service_id": "test",
"local_service_port": 4321,
"local_service_address": "127.0.0.1",
"local_service_address": "` + tt.ip + `",
"upstreams": [
{
"destination_type": "service",
"destination_namespace": "default",
"destination_name": "db",
"local_bind_address": "127.0.0.1",
"local_bind_address": "` + tt.ip + `",
"local_bind_port": 1234,
"config": {
"destination_type": "sidecar_service.proxy.upstreams.config is 'opaque' so should not get translated"
@ -2575,109 +2584,121 @@ func TestAgent_RegisterService_TranslateKeys(t *testing.T) {
"passing": 16
}
}`
req, _ := http.NewRequest("PUT", "/v1/agent/service/register", strings.NewReader(json))
req, _ := http.NewRequest("PUT", "/v1/agent/service/register", strings.NewReader(json))
rr := httptest.NewRecorder()
obj, err := a.srv.AgentRegisterService(rr, req)
require.NoError(t, err)
require.Nil(t, obj)
require.Equal(t, 200, rr.Code, "body: %s", rr.Body)
rr := httptest.NewRecorder()
obj, err := a.srv.AgentRegisterService(rr, req)
require.NoError(t, err)
require.Nil(t, obj)
require.Equal(t, 200, rr.Code, "body: %s", rr.Body)
svc := &structs.NodeService{
ID: "test",
Service: "test",
Meta: map[string]string{
"some": "meta",
"enable_tag_override": "meta is 'opaque' so should not get translated",
},
Port: 8000,
EnableTagOverride: true,
Weights: &structs.Weights{Passing: 16, Warning: 0},
Kind: structs.ServiceKindConnectProxy,
Proxy: structs.ConnectProxyConfig{
DestinationServiceName: "web",
DestinationServiceID: "web",
LocalServiceAddress: "127.0.0.1",
LocalServicePort: 1234,
Config: map[string]interface{}{
"destination_type": "proxy.config is 'opaque' so should not get translated",
},
Upstreams: structs.Upstreams{
{
DestinationType: structs.UpstreamDestTypeService,
DestinationName: "db",
DestinationNamespace: "default",
LocalBindAddress: "127.0.0.1",
LocalBindPort: 1234,
svc := &structs.NodeService{
ID: "test",
Service: "test",
Meta: map[string]string{
"some": "meta",
"enable_tag_override": "meta is 'opaque' so should not get translated",
},
Port: 8000,
EnableTagOverride: true,
Weights: &structs.Weights{Passing: 16, Warning: 0},
Kind: structs.ServiceKindConnectProxy,
Proxy: structs.ConnectProxyConfig{
DestinationServiceName: "web",
DestinationServiceID: "web",
LocalServiceAddress: tt.ip,
LocalServicePort: 1234,
Config: map[string]interface{}{
"destination_type": "proxy.upstreams.config is 'opaque' so should not get translated",
"destination_type": "proxy.config is 'opaque' so should not get translated",
},
},
},
},
Connect: structs.ServiceConnect{
Proxy: &structs.ServiceDefinitionConnectProxy{
ExecMode: "script",
Config: map[string]interface{}{
"destination_type": "connect.proxy.config is 'opaque' so should not get translated",
},
Upstreams: structs.Upstreams{
{
DestinationType: structs.UpstreamDestTypeService,
DestinationName: "db",
DestinationNamespace: "default",
LocalBindAddress: "127.0.0.1",
LocalBindPort: 1234,
Config: map[string]interface{}{
"destination_type": "connect.proxy.upstreams.config is 'opaque' so should not get translated",
Upstreams: structs.Upstreams{
{
DestinationType: structs.UpstreamDestTypeService,
DestinationName: "db",
DestinationNamespace: "default",
LocalBindAddress: tt.ip,
LocalBindPort: 1234,
Config: map[string]interface{}{
"destination_type": "proxy.upstreams.config is 'opaque' so should not get translated",
},
},
},
},
},
// The sidecar service is nilled since it is only config sugar and
// shouldn't be represented in state. We assert that the translations
// there worked by inspecting the registered sidecar below.
SidecarService: nil,
},
}
Connect: structs.ServiceConnect{
Proxy: &structs.ServiceDefinitionConnectProxy{
ExecMode: "script",
Config: map[string]interface{}{
"destination_type": "connect.proxy.config is 'opaque' so should not get translated",
},
Upstreams: structs.Upstreams{
{
DestinationType: structs.UpstreamDestTypeService,
DestinationName: "db",
DestinationNamespace: "default",
LocalBindAddress: tt.ip,
LocalBindPort: 1234,
Config: map[string]interface{}{
"destination_type": "connect.proxy.upstreams.config is 'opaque' so should not get translated",
},
},
},
},
// The sidecar service is nilled since it is only config sugar and
// shouldn't be represented in state. We assert that the translations
// there worked by inspecting the registered sidecar below.
SidecarService: nil,
},
}
got := a.State.Service("test")
require.Equal(t, svc, got)
got := a.State.Service("test")
require.Equal(t, svc, got)
sidecarSvc := &structs.NodeService{
Kind: structs.ServiceKindConnectProxy,
ID: "test-sidecar-proxy",
Service: "test-proxy",
Meta: map[string]string{
"some": "meta",
"enable_tag_override": "sidecar_service.meta is 'opaque' so should not get translated",
},
Port: 8001,
EnableTagOverride: true,
Weights: &structs.Weights{Passing: 1, Warning: 1},
LocallyRegisteredAsSidecar: true,
Proxy: structs.ConnectProxyConfig{
DestinationServiceName: "test",
DestinationServiceID: "test",
LocalServiceAddress: "127.0.0.1",
LocalServicePort: 4321,
Upstreams: structs.Upstreams{
{
DestinationType: structs.UpstreamDestTypeService,
DestinationName: "db",
DestinationNamespace: "default",
LocalBindAddress: "127.0.0.1",
LocalBindPort: 1234,
Config: map[string]interface{}{
"destination_type": "sidecar_service.proxy.upstreams.config is 'opaque' so should not get translated",
sidecarSvc := &structs.NodeService{
Kind: structs.ServiceKindConnectProxy,
ID: "test-sidecar-proxy",
Service: "test-proxy",
Meta: map[string]string{
"some": "meta",
"enable_tag_override": "sidecar_service.meta is 'opaque' so should not get translated",
},
Port: 8001,
EnableTagOverride: true,
Weights: &structs.Weights{Passing: 1, Warning: 1},
LocallyRegisteredAsSidecar: true,
Proxy: structs.ConnectProxyConfig{
DestinationServiceName: "test",
DestinationServiceID: "test",
LocalServiceAddress: tt.ip,
LocalServicePort: 4321,
Upstreams: structs.Upstreams{
{
DestinationType: structs.UpstreamDestTypeService,
DestinationName: "db",
DestinationNamespace: "default",
LocalBindAddress: tt.ip,
LocalBindPort: 1234,
Config: map[string]interface{}{
"destination_type": "sidecar_service.proxy.upstreams.config is 'opaque' so should not get translated",
},
},
},
},
},
},
}
gotSidecar := a.State.Service("test-sidecar-proxy")
hasNoCorrectTCPCheck := true
for _, v := range a.checkTCPs {
if strings.HasPrefix(v.TCP, tt.expectedTCPCheckStart) {
hasNoCorrectTCPCheck = false
break
}
fmt.Println("TCP Check:= ", v)
}
if hasNoCorrectTCPCheck {
t.Fatalf("Did not find the expected TCP Healtcheck '%s' in %#v ", tt.expectedTCPCheckStart, a.checkTCPs)
}
require.Equal(t, sidecarSvc, gotSidecar)
})
}
gotSidecar := a.State.Service("test-sidecar-proxy")
require.Equal(t, sidecarSvc, gotSidecar)
}
func TestAgent_RegisterService_ACLDeny(t *testing.T) {

3
agent/dns.go
View File

@ -19,6 +19,7 @@ import (
"github.com/hashicorp/consul/agent/consul"
"github.com/hashicorp/consul/agent/structs"
"github.com/hashicorp/consul/api"
"github.com/hashicorp/consul/ipaddr"
"github.com/hashicorp/consul/lib"
"github.com/miekg/dns"
)
@ -264,7 +265,7 @@ func recursorAddr(recursor string) (string, error) {
START:
_, _, err := net.SplitHostPort(recursor)
if ae, ok := err.(*net.AddrError); ok && ae.Err == "missing port in address" {
recursor = fmt.Sprintf("%s:%d", recursor, 53)
recursor = ipaddr.FormatAddressPort(recursor, 53)
goto START
}
if err != nil {

4
agent/sidecar_service.go
View File

@ -4,6 +4,8 @@ import (
"fmt"
"time"
"github.com/hashicorp/consul/ipaddr"
"github.com/hashicorp/consul/agent/structs"
)
@ -171,7 +173,7 @@ func (a *Agent) sidecarServiceFromNodeService(ns *structs.NodeService, token str
Name: "Connect Sidecar Listening",
// Default to localhost rather than agent/service public IP. The checks
// can always be overridden if a non-loopback IP is needed.
TCP: fmt.Sprintf("127.0.0.1:%d", sidecar.Port),
TCP: ipaddr.FormatAddressPort(sidecar.Proxy.LocalServiceAddress, sidecar.Port),
Interval: 10 * time.Second,
},
&structs.CheckType{

3
connect/proxy/config.go
View File

@ -10,6 +10,7 @@ import (
"github.com/hashicorp/consul/api"
"github.com/hashicorp/consul/api/watch"
"github.com/hashicorp/consul/connect"
"github.com/hashicorp/consul/ipaddr"
"github.com/hashicorp/consul/lib"
)
@ -242,7 +243,7 @@ func (w *AgentConfigWatcher) handler(blockVal watch.BlockingParamVal,
}
cfg.PublicListener.BindAddress = resp.Address
cfg.PublicListener.BindPort = resp.Port
cfg.PublicListener.LocalServiceAddress = fmt.Sprintf("%s:%d",
cfg.PublicListener.LocalServiceAddress = ipaddr.FormatAddressPort(
resp.Proxy.LocalServiceAddress, resp.Proxy.LocalServicePort)
cfg.PublicListener.applyDefaults()

6
connect/proxy/listener.go
View File

@ -4,7 +4,6 @@ import (
"context"
"crypto/tls"
"errors"
"fmt"
"log"
"net"
"sync"
@ -14,6 +13,7 @@ import (
metrics "github.com/armon/go-metrics"
"github.com/hashicorp/consul/api"
"github.com/hashicorp/consul/connect"
"github.com/hashicorp/consul/ipaddr"
)
const (
@ -58,7 +58,7 @@ type Listener struct {
// connections and proxy them to the configured local application over TCP.
func NewPublicListener(svc *connect.Service, cfg PublicListenerConfig,
logger *log.Logger) *Listener {
bindAddr := fmt.Sprintf("%s:%d", cfg.BindAddress, cfg.BindPort)
bindAddr := ipaddr.FormatAddressPort(cfg.BindAddress, cfg.BindPort)
return &Listener{
Service: svc,
listenFunc: func() (net.Listener, error) {
@ -96,7 +96,7 @@ func NewUpstreamListener(svc *connect.Service, client *api.Client,
func newUpstreamListenerWithResolver(svc *connect.Service, cfg UpstreamConfig,
resolverFunc func(UpstreamConfig) (connect.Resolver, error),
logger *log.Logger) *Listener {
bindAddr := fmt.Sprintf("%s:%d", cfg.LocalBindAddress, cfg.LocalBindPort)
bindAddr := ipaddr.FormatAddressPort(cfg.LocalBindAddress, cfg.LocalBindPort)
return &Listener{
Service: svc,
listenFunc: func() (net.Listener, error) {

3
connect/proxy/listener_test.go
View File

@ -16,6 +16,7 @@ import (
agConnect "github.com/hashicorp/consul/agent/connect"
"github.com/hashicorp/consul/connect"
"github.com/hashicorp/consul/ipaddr"
"github.com/hashicorp/consul/sdk/freeport"
)
@ -204,7 +205,7 @@ func TestUpstreamListener(t *testing.T) {
// Proxy and fake remote service are running, play the part of the app
// connecting to a remote connect service over TCP.
conn, err := net.Dial("tcp",
fmt.Sprintf("%s:%d", cfg.LocalBindAddress, cfg.LocalBindPort))
ipaddr.FormatAddressPort(cfg.LocalBindAddress, cfg.LocalBindPort))
require.NoError(t, err)
TestEchoConn(t, conn, "")

3
connect/resolver.go
View File

@ -8,6 +8,7 @@ import (
"github.com/hashicorp/consul/agent/connect"
"github.com/hashicorp/consul/api"
"github.com/hashicorp/consul/ipaddr"
)
// Resolver is the interface implemented by a service discovery mechanism to get
@ -156,7 +157,7 @@ func (cr *ConsulResolver) resolveServiceEntry(entry *api.ServiceEntry) (string,
Service: service,
}
return fmt.Sprintf("%s:%d", addr, port), certURI, nil
return ipaddr.FormatAddressPort(addr, port), certURI, nil
}
func (cr *ConsulResolver) queryOptions(ctx context.Context) *api.QueryOptions {

6
ipaddr/ipaddr.go
View File

@ -4,8 +4,14 @@ import (
"fmt"
"net"
"reflect"
"strconv"
)
// FormatAddressPort Helper for net.JoinHostPort that takes int for port
func FormatAddressPort(address string, port int) string {
return net.JoinHostPort(address, strconv.Itoa(port))
}
// IsAny checks if the given ip address is an IPv4 or IPv6 ANY address. ip
// can be either a *net.IP or a string. It panics on another type.
func IsAny(ip interface{}) bool {

56
ipaddr/ipaddr_test.go Normal file
View File

@ -0,0 +1,56 @@
package ipaddr
import (
"fmt"
"testing"
)
func TestIsIPv6(t *testing.T) {
tests := []struct {
ip string
ipv6 bool
}{
// IPv4 private addresses
{"10.0.0.1", false}, // private network address
{"100.64.0.1", false}, // shared address space
{"172.16.0.1", false}, // private network address
{"192.168.0.1", false}, // private network address
{"192.0.0.1", false}, // IANA address
{"192.0.2.1", false}, // documentation address
{"127.0.0.1", false}, // loopback address
{"169.254.0.1", false}, // link local address
// IPv4 public addresses
{"1.2.3.4", false},
// IPv6 private addresses
{"::1", true}, // loopback address
{"fe80::1", true}, // link local address
{"fc00::1", true}, // unique local address
{"fec0::1", true}, // site local address
{"2001:db8::1", true}, // documentation address
// IPv6 public addresses
{"2004:db6::1", true},
// hostname
{"example.com", false},
{"localhost", false},
{"1.257.0.1", false},
}
for _, tt := range tests {
t.Run(tt.ip, func(t *testing.T) {
port := 1234
formated := FormatAddressPort(tt.ip, port)
if tt.ipv6 {
if fmt.Sprintf("[%s]:%d", tt.ip, port) != formated {
t.Fatalf("Wrong format %s for %s", formated, tt.ip)
}
} else {
if fmt.Sprintf("%s:%d", tt.ip, port) != formated {
t.Fatalf("Wrong format %s for %s", formated, tt.ip)
}
}
})
}
}