From beb91cf5d9fc7c3d50f7e70612d630c4c5428d42 Mon Sep 17 00:00:00 2001 From: Hans Hasselberg Date: Wed, 7 Aug 2019 10:39:44 +0200 Subject: [PATCH 1/4] back to using server-port --- agent/consul/auto_encrypt.go | 32 +++++++++----------------------- 1 file changed, 9 insertions(+), 23 deletions(-) diff --git a/agent/consul/auto_encrypt.go b/agent/consul/auto_encrypt.go index 3acf15a61498..0c1fdf788c70 100644 --- a/agent/consul/auto_encrypt.go +++ b/agent/consul/auto_encrypt.go @@ -4,7 +4,6 @@ import ( "fmt" "log" "net" - "strconv" "strings" "time" @@ -19,7 +18,7 @@ const ( retryJitterWindow = 30 * time.Second ) -func (c *Client) RequestAutoEncryptCerts(servers []string, defaultPort int, token string, interruptCh chan struct{}) (*structs.SignedResponse, string, error) { +func (c *Client) RequestAutoEncryptCerts(servers []string, port int, token string, interruptCh chan struct{}) (*structs.SignedResponse, string, error) { errFn := func(err error) (*structs.SignedResponse, string, error) { return nil, "", err } @@ -82,7 +81,7 @@ func (c *Client) RequestAutoEncryptCerts(servers []string, defaultPort int, toke // Translate host to net.TCPAddr to make life easier for // RPCInsecure. for _, s := range servers { - ips, port, err := resolveAddr(s, defaultPort, c.logger) + ips, err := resolveAddr(s, c.logger) if err != nil { c.logger.Printf("[WARN] agent: AutoEncrypt resolveAddr failed: %v", err) continue @@ -116,27 +115,14 @@ func (c *Client) RequestAutoEncryptCerts(servers []string, defaultPort int, toke // resolveAddr is used to resolve the host into IPs, port, and error. // If no port is given, use the default -func resolveAddr(rawHost string, defaultPort int, logger *log.Logger) ([]net.IP, int, error) { - host, splitPort, err := net.SplitHostPort(rawHost) +func resolveAddr(rawHost string, logger *log.Logger) ([]net.IP, error) { + host, _, err := net.SplitHostPort(rawHost) if err != nil && err.Error() != fmt.Sprintf("address %s: missing port in address", rawHost) { - return nil, defaultPort, err - } - - // SplitHostPort returns empty host and splitPort on missingPort err, - // so those are set to defaults - var port int - if err != nil { - host = rawHost - port = defaultPort - } else { - port, err = strconv.Atoi(splitPort) - if err != nil { - port = defaultPort - } + return nil, err } if ip := net.ParseIP(host); ip != nil { - return []net.IP{ip}, port, nil + return []net.IP{ip}, nil } // First try TCP so we have the best chance for the largest list of @@ -145,7 +131,7 @@ func resolveAddr(rawHost string, defaultPort int, logger *log.Logger) ([]net.IP, if ips, err := tcpLookupIP(host, logger); err != nil { logger.Printf("[DEBUG] agent: TCP-first lookup failed for '%s', falling back to UDP: %s", host, err) } else if len(ips) > 0 { - return ips, port, nil + return ips, nil } // If TCP didn't yield anything then use the normal Go resolver which @@ -153,9 +139,9 @@ func resolveAddr(rawHost string, defaultPort int, logger *log.Logger) ([]net.IP, // indicates it was truncated. ips, err := net.LookupIP(host) if err != nil { - return nil, port, err + return nil, err } - return ips, port, nil + return ips, nil } // tcpLookupIP is a helper to initiate a TCP-based DNS lookup for the given host. From 8f7a35ea367844dbfe3b4e6f6688acf8e28fd779 Mon Sep 17 00:00:00 2001 From: Hans Hasselberg Date: Thu, 22 Aug 2019 11:26:50 +0200 Subject: [PATCH 2/4] remove ports --- agent/consul/auto_encrypt.go | 3 +-- agent/consul/auto_encrypt_test.go | 17 +++-------------- 2 files changed, 4 insertions(+), 16 deletions(-) diff --git a/agent/consul/auto_encrypt.go b/agent/consul/auto_encrypt.go index 0c1fdf788c70..2d48a2679edb 100644 --- a/agent/consul/auto_encrypt.go +++ b/agent/consul/auto_encrypt.go @@ -113,8 +113,7 @@ func (c *Client) RequestAutoEncryptCerts(servers []string, port int, token strin } } -// resolveAddr is used to resolve the host into IPs, port, and error. -// If no port is given, use the default +// resolveAddr is used to resolve the host into IPs and error. func resolveAddr(rawHost string, logger *log.Logger) ([]net.IP, error) { host, _, err := net.SplitHostPort(rawHost) if err != nil && err.Error() != fmt.Sprintf("address %s: missing port in address", rawHost) { diff --git a/agent/consul/auto_encrypt_test.go b/agent/consul/auto_encrypt_test.go index 2a4daa012bcd..f0a19371fd99 100644 --- a/agent/consul/auto_encrypt_test.go +++ b/agent/consul/auto_encrypt_test.go @@ -10,71 +10,60 @@ import ( func TestAutoEncrypt_resolveAddr(t *testing.T) { type args struct { - rawHost string - defaultPort int - logger *log.Logger + rawHost string + logger *log.Logger } tests := []struct { name string args args ips []net.IP - port int wantErr bool }{ { name: "host without port", args: args{ "127.0.0.1", - 8300, log.New(os.Stderr, "", log.LstdFlags), }, ips: []net.IP{net.IPv4(127, 0, 0, 1)}, - port: 8300, wantErr: false, }, { name: "host with port", args: args{ "127.0.0.1:1234", - 8300, log.New(os.Stderr, "", log.LstdFlags), }, ips: []net.IP{net.IPv4(127, 0, 0, 1)}, - port: 1234, wantErr: false, }, { name: "host with broken port", args: args{ "127.0.0.1:xyz", - 8300, log.New(os.Stderr, "", log.LstdFlags), }, ips: []net.IP{net.IPv4(127, 0, 0, 1)}, - port: 8300, wantErr: false, }, { name: "not an address", args: args{ "abc", - 8300, log.New(os.Stderr, "", log.LstdFlags), }, ips: nil, - port: 8300, wantErr: true, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - ips, port, err := resolveAddr(tt.args.rawHost, tt.args.defaultPort, tt.args.logger) + ips, err := resolveAddr(tt.args.rawHost, tt.args.logger) if (err != nil) != tt.wantErr { t.Errorf("resolveAddr error: %v, wantErr: %v", err, tt.wantErr) return } require.Equal(t, tt.ips, ips) - require.Equal(t, tt.port, port) }) } } From 8c4f161728d054cc1a4c311c83309389f52e4f3d Mon Sep 17 00:00:00 2001 From: Hans Hasselberg Date: Thu, 22 Aug 2019 16:08:22 +0200 Subject: [PATCH 3/4] test coverage --- agent/consul/auto_encrypt.go | 12 ++++++++++-- agent/consul/auto_encrypt_test.go | 10 ++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/agent/consul/auto_encrypt.go b/agent/consul/auto_encrypt.go index 2d48a2679edb..0016922624de 100644 --- a/agent/consul/auto_encrypt.go +++ b/agent/consul/auto_encrypt.go @@ -113,11 +113,19 @@ func (c *Client) RequestAutoEncryptCerts(servers []string, port int, token strin } } +func missingPortError(host string, err error) bool { + return err != nil && err.Error() == fmt.Sprintf("address %s: missing port in address", host) +} + // resolveAddr is used to resolve the host into IPs and error. func resolveAddr(rawHost string, logger *log.Logger) ([]net.IP, error) { host, _, err := net.SplitHostPort(rawHost) - if err != nil && err.Error() != fmt.Sprintf("address %s: missing port in address", rawHost) { - return nil, err + if err != nil { + if missingPortError(rawHost, err) { + host = rawHost + } else { + return nil, err + } } if ip := net.ParseIP(host); ip != nil { diff --git a/agent/consul/auto_encrypt_test.go b/agent/consul/auto_encrypt_test.go index f0a19371fd99..d27b2f94898e 100644 --- a/agent/consul/auto_encrypt_test.go +++ b/agent/consul/auto_encrypt_test.go @@ -67,3 +67,13 @@ func TestAutoEncrypt_resolveAddr(t *testing.T) { }) } } + +func TestAutoEncrypt_missingPortError(t *testing.T) { + host := "127.0.0.1" + _, _, err := net.SplitHostPort(host) + require.True(t, missingPortError(host, err)) + + host = "127.0.0.1:1234" + _, _, err = net.SplitHostPort(host) + require.False(t, missingPortError(host, err)) +} From 77e7d1764ed664590b3b54c6bcddf08738526254 Mon Sep 17 00:00:00 2001 From: Hans Hasselberg Date: Thu, 22 Aug 2019 16:12:23 +0200 Subject: [PATCH 4/4] explanation in the comments --- agent/consul/auto_encrypt.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/agent/consul/auto_encrypt.go b/agent/consul/auto_encrypt.go index 0016922624de..21c5c149cc30 100644 --- a/agent/consul/auto_encrypt.go +++ b/agent/consul/auto_encrypt.go @@ -121,6 +121,9 @@ func missingPortError(host string, err error) bool { func resolveAddr(rawHost string, logger *log.Logger) ([]net.IP, error) { host, _, err := net.SplitHostPort(rawHost) if err != nil { + // In case we encounter this error, we proceed with the + // rawHost. This is fine since -start-join and -retry-join + // take only hosts anyways and this is an expected case. if missingPortError(rawHost, err) { host = rawHost } else {