Merge pull request #1996 from kelseyhightower/fix-dns-discovery-hostnames

etcdmain: resolve DNS hostnames for client and peer URLs

etcdmain/config.go

@@ -17,6 +17,7 @@
 package etcdmain
 
 import (
+	"errors"
 	"flag"
 	"fmt"
 	"log"
@@ -27,6 +28,7 @@ import (
 	"github.com/coreos/etcd/etcdserver"
 	"github.com/coreos/etcd/pkg/cors"
 	"github.com/coreos/etcd/pkg/flags"
+	"github.com/coreos/etcd/pkg/netutil"
 	"github.com/coreos/etcd/pkg/transport"
 	"github.com/coreos/etcd/version"
 )
@@ -242,9 +244,17 @@ func (cfg *config) Parse(arguments []string) error {
 		return err
 	}
 
+	if err := cfg.resolveUrls(); err != nil {
+		return errors.New("cannot resolve DNS hostnames.")
+	}
+
 	return nil
 }
 
+func (cfg *config) resolveUrls() error {
+	return netutil.ResolveTCPAddrs(cfg.lpurls, cfg.apurls, cfg.lcurls, cfg.acurls)
+}
+
 func (cfg config) isNewCluster() bool          { return cfg.clusterState.String() == clusterStateFlagNew }
 func (cfg config) isProxy() bool               { return cfg.proxy.String() != proxyFlagOff }
 func (cfg config) isReadonlyProxy() bool       { return cfg.proxy.String() == proxyFlagReadonly }

pkg/netutil/netutil.go

@@ -0,0 +1,57 @@
+/*
+   Copyright 2014 CoreOS, Inc.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package netutil
+
+import (
+	"log"
+	"net"
+	"net/url"
+)
+
+var (
+	// indirection for testing
+	resolveTCPAddr = net.ResolveTCPAddr
+)
+
+// ResolveTCPAddrs is a convenience wrapper for net.ResolveTCPAddr.
+// ResolveTCPAddrs resolves all DNS hostnames in-place for the given set of
+// url.URLs.
+func ResolveTCPAddrs(urls ...[]url.URL) error {
+	for _, us := range urls {
+		for i, u := range us {
+			host, _, err := net.SplitHostPort(u.Host)
+			if err != nil {
+				log.Printf("netutil: Could not parse url %s during tcp resolving.", u.Host)
+				return err
+			}
+			if host == "localhost" {
+				continue
+			}
+			if net.ParseIP(host) != nil {
+				continue
+			}
+			tcpAddr, err := resolveTCPAddr("tcp", u.Host)
+			if err != nil {
+				log.Printf("netutil: Could not resolve host: %s", u.Host)
+				return err
+			}
+			log.Printf("netutil: Resolving %s to %s", u.Host, tcpAddr.String())
+			us[i].Host = tcpAddr.String()
+		}
+	}
+	return nil
+}

pkg/netutil/netutil_test.go

@@ -0,0 +1,140 @@
+/*
+   Copyright 2014 CoreOS, Inc.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package netutil
+
+import (
+	"errors"
+	"net"
+	"net/url"
+	"reflect"
+	"strconv"
+	"testing"
+)
+
+func TestResolveTCPAddrs(t *testing.T) {
+	defer func() { resolveTCPAddr = net.ResolveTCPAddr }()
+	tests := []struct {
+		urls     [][]url.URL
+		expected [][]url.URL
+		hostMap  map[string]string
+		hasError bool
+	}{
+		{
+			urls: [][]url.URL{
+				[]url.URL{
+					url.URL{Scheme: "http", Host: "127.0.0.1:4001"},
+					url.URL{Scheme: "http", Host: "127.0.0.1:2379"},
+				},
+				[]url.URL{
+					url.URL{Scheme: "http", Host: "127.0.0.1:7001"},
+					url.URL{Scheme: "http", Host: "127.0.0.1:2380"},
+				},
+			},
+			expected: [][]url.URL{
+				[]url.URL{
+					url.URL{Scheme: "http", Host: "127.0.0.1:4001"},
+					url.URL{Scheme: "http", Host: "127.0.0.1:2379"},
+				},
+				[]url.URL{
+					url.URL{Scheme: "http", Host: "127.0.0.1:7001"},
+					url.URL{Scheme: "http", Host: "127.0.0.1:2380"},
+				},
+			},
+		},
+		{
+			urls: [][]url.URL{
+				[]url.URL{
+					url.URL{Scheme: "http", Host: "infra0.example.com:4001"},
+					url.URL{Scheme: "http", Host: "infra0.example.com:2379"},
+				},
+				[]url.URL{
+					url.URL{Scheme: "http", Host: "infra0.example.com:7001"},
+					url.URL{Scheme: "http", Host: "infra0.example.com:2380"},
+				},
+			},
+			expected: [][]url.URL{
+				[]url.URL{
+					url.URL{Scheme: "http", Host: "10.0.1.10:4001"},
+					url.URL{Scheme: "http", Host: "10.0.1.10:2379"},
+				},
+				[]url.URL{
+					url.URL{Scheme: "http", Host: "10.0.1.10:7001"},
+					url.URL{Scheme: "http", Host: "10.0.1.10:2380"},
+				},
+			},
+			hostMap: map[string]string{
+				"infra0.example.com": "10.0.1.10",
+			},
+			hasError: false,
+		},
+		{
+			urls: [][]url.URL{
+				[]url.URL{
+					url.URL{Scheme: "http", Host: "infra0.example.com:4001"},
+					url.URL{Scheme: "http", Host: "infra0.example.com:2379"},
+				},
+				[]url.URL{
+					url.URL{Scheme: "http", Host: "infra0.example.com:7001"},
+					url.URL{Scheme: "http", Host: "infra0.example.com:2380"},
+				},
+			},
+			hostMap: map[string]string{
+				"infra0.example.com": "",
+			},
+			hasError: true,
+		},
+		{
+			urls: [][]url.URL{
+				[]url.URL{
+					url.URL{Scheme: "http", Host: "ssh://infra0.example.com:4001"},
+					url.URL{Scheme: "http", Host: "ssh://infra0.example.com:2379"},
+				},
+				[]url.URL{
+					url.URL{Scheme: "http", Host: "ssh://infra0.example.com:7001"},
+					url.URL{Scheme: "http", Host: "ssh://infra0.example.com:2380"},
+				},
+			},
+			hasError: true,
+		},
+	}
+	for _, tt := range tests {
+		resolveTCPAddr = func(network, addr string) (*net.TCPAddr, error) {
+			host, port, err := net.SplitHostPort(addr)
+			if err != nil {
+				return nil, err
+			}
+			if tt.hostMap[host] == "" {
+				return nil, errors.New("cannot resolve host.")
+			}
+			i, err := strconv.Atoi(port)
+			if err != nil {
+				return nil, err
+			}
+			return &net.TCPAddr{IP: net.ParseIP(tt.hostMap[host]), Port: i, Zone: ""}, nil
+		}
+		err := ResolveTCPAddrs(tt.urls...)
+		if tt.hasError {
+			if err == nil {
+				t.Errorf("expected error")
+			}
+			continue
+		}
+		if !reflect.DeepEqual(tt.urls, tt.expected) {
+			t.Errorf("expected: %v, got %v", tt.expected, tt.urls)
+		}
+	}
+}

test

@@ -15,7 +15,7 @@ COVER=${COVER:-"-cover"}
 source ./build
 
 # Hack: gofmt ./ will recursively check the .git directory. So use *.go for gofmt.
-TESTABLE_AND_FORMATTABLE="client discovery error etcdctl/command etcdmain etcdserver etcdserver/etcdhttp etcdserver/etcdhttp/httptypes etcdserver/etcdserverpb integration migrate pkg/fileutil pkg/flags pkg/ioutils pkg/types pkg/transport pkg/wait proxy raft rafthttp snap store wal"
+TESTABLE_AND_FORMATTABLE="client discovery error etcdctl/command etcdmain etcdserver etcdserver/etcdhttp etcdserver/etcdhttp/httptypes etcdserver/etcdserverpb integration migrate pkg/fileutil pkg/flags pkg/ioutils pkg/netutil pkg/types pkg/transport pkg/wait proxy raft rafthttp snap store wal"
 FORMATTABLE="$TESTABLE_AND_FORMATTABLE *.go etcdctl/"
 
 # user has not provided PKG override