From 7a814fce63d5bb9c6070bffd68ba0f78280832c3 Mon Sep 17 00:00:00 2001
From: Sean Chittenden <sean@chittenden.org>
Date: Fri, 1 Apr 2016 10:20:45 -0700
Subject: [PATCH] Print a helpful message re: duplicate addresses

IP sockets provide nice endpoints where the kernel will fail to bind and will error out saying socket already in use.  UNIX sockets, however, don't enjoy this nice property when cleaning up stale sockets on listen.  Given the number of addresses in Consul, provide operators with a helpful message that indicates the source of the reused address.

Before this fix, it was possible for the HTTP socket to unlink the RPC socket, leading to confusing blocked behavior when running commands like `consul info`.

```
% cat tmp.config.json
{
    "addresses": {
	"http": "unix:///tmp/.consul.sock",
	"rpc": "unix:///tmp/.consul.sock"
    },
    "unix_sockets": {
	"mode": "0700"
    }
}
% consul agent -config-file tmp.config.json -advertise=127.0.0.1 -data-dir=/tmp/
==> All listening endpoints must be unique: HTTP address already configured for RPC
Exit 1
```
---
 command/agent/command.go     | 67 ++++++++++++++++++++++++++++++++++++
 command/agent/config_test.go | 36 +++++++++++++++++++
 2 files changed, 103 insertions(+)

diff --git a/command/agent/command.go b/command/agent/command.go
index 86db5eb739..ecc46b083f 100644
--- a/command/agent/command.go
+++ b/command/agent/command.go
@@ -192,6 +192,12 @@ func (c *Command) readConfig() *Config {
 		return nil
 	}
 
+	// Ensure all endpoints are unique
+	if ok, err := config.verifyUniqueListeners(); !ok {
+		c.Ui.Error(fmt.Sprintf("All listening endpoints must be unique: %s", err))
+		return nil
+	}
+
 	// Check the data dir for signs of an un-migrated Consul 0.5.x or older
 	// server. Consul refuses to start if this is present to protect a server
 	// with existing data from starting on a fresh data set.
@@ -301,6 +307,67 @@ func (c *Command) readConfig() *Config {
 	return config
 }
 
+// verifyUniqueListeners checks to see if an address was used more than once in
+// the config
+func (config *Config) verifyUniqueListeners() (bool, error) {
+	type key struct {
+		host string
+		port int
+	}
+	const numUniqueAddrs = 7
+	m := make(map[key]string, numUniqueAddrs)
+
+	testFunc := func(k key, descr string) (bool, error) {
+		if k.host == "" {
+			k.host = "0.0.0.0"
+		} else if strings.HasPrefix(k.host, "unix") {
+			// Don't compare ports on unix sockets
+			k.port = 0
+		}
+		if k.host == "0.0.0.0" && k.port <= 0 {
+			return true, nil
+		}
+
+		v, ok := m[k]
+		if ok {
+			return false, fmt.Errorf("%s address already configured for %s", descr, v)
+		}
+		m[k] = descr
+
+		return true, nil
+	}
+
+	if ok, err := testFunc(key{config.Addresses.RPC, config.Ports.RPC}, "RPC"); !ok {
+		return false, err
+	}
+
+	if ok, err := testFunc(key{config.Addresses.DNS, config.Ports.DNS}, "DNS"); !ok {
+		return false, err
+	}
+
+	if ok, err := testFunc(key{config.Addresses.HTTP, config.Ports.HTTP}, "HTTP"); !ok {
+		return false, err
+	}
+
+	if ok, err := testFunc(key{config.Addresses.HTTPS, config.Ports.HTTPS}, "HTTPS"); !ok {
+		return false, err
+	}
+
+	if ok, err := testFunc(key{config.AdvertiseAddr, config.Ports.Server}, "Server RPC"); !ok {
+		return false, err
+	}
+
+	if ok, err := testFunc(key{config.AdvertiseAddr, config.Ports.SerfLan}, "Serf LAN"); !ok {
+		return false, err
+	}
+
+	if ok, err := testFunc(key{config.AdvertiseAddr, config.Ports.SerfWan}, "Serf WAN"); !ok {
+		return false, err
+	}
+
+	return true, nil
+}
+
 // setupLoggers is used to setup the logGate, logWriter, and our logOutput
 func (c *Command) setupLoggers(config *Config) (*GatedWriter, *logWriter, io.Writer) {
 	// Setup logging. First create the gated log writer, which will
diff --git a/command/agent/config_test.go b/command/agent/config_test.go
index 72cae23a5d..28b51c73f6 100644
--- a/command/agent/config_test.go
+++ b/command/agent/config_test.go
@@ -989,6 +989,42 @@ func TestDecodeConfig_Services(t *testing.T) {
 	}
 }
 
+func TestDecodeConfig_verifyUniqueListeners(t *testing.T) {
+	tests := []struct {
+		name string
+		cfg  string
+		pass bool
+	}{
+		{
+			"http_rpc1",
+			`{"addresses": {"http": "0.0.0.0", "rpc": "127.0.0.1"}, "ports": {"rpc": 8000, "dns": 8000}}`,
+			true,
+		},
+		{
+			"http_rpc IP identical",
+			`{"addresses": {"http": "0.0.0.0", "rpc": "0.0.0.0"}, "ports": {"rpc": 8000, "dns": 8000}}`,
+			false,
+		},
+		{
+			"http_rpc unix identical (diff ports)",
+			`{"addresses": {"http": "unix:///tmp/.consul.sock", "rpc": "unix:///tmp/.consul.sock"}, "ports": {"rpc": 8000, "dns": 8001}}`,
+			false,
+		},
+	}
+
+	for _, test := range tests {
+		config, err := DecodeConfig(bytes.NewReader([]byte(test.cfg)))
+		if err != nil {
+			t.Fatalf("err: %s %s", test.name, err)
+		}
+
+		ok, err := config.verifyUniqueListeners()
+		if ok != test.pass {
+			t.Errorf("err: %s should have %v: %v: %v", test.name, test.pass, test.cfg, err)
+		}
+	}
+}
+
 func TestDecodeConfig_Checks(t *testing.T) {
 	input := `{
 		"checks": [