Ensure a socket is created for permissions adjustment tests and fix some

items pointed out in the code review

This code is copyright 2014 Akamai Technologies, Inc. <opensource@akamai.com>

api/api.go

@@ -249,7 +249,7 @@ func (r *request) toHTTP() (*http.Request, error) {
 	req.Host = r.url.Host
 
 	// Setup auth
-	if err == nil && r.config.HttpAuth != nil {
+	if r.config.HttpAuth != nil {
 		req.SetBasicAuth(r.config.HttpAuth.Username, r.config.HttpAuth.Password)
 	}
 

api/api_test.go

@@ -20,7 +20,7 @@ type testServer struct {
 	configFile string
 }
 
-type TestPortConfig struct {
+type testPortConfig struct {
 	DNS     int `json:"dns,omitempty"`
 	HTTP    int `json:"http,omitempty"`
 	RPC     int `json:"rpc,omitempty"`
@@ -29,31 +29,33 @@ type TestPortConfig struct {
 	Server  int `json:"server,omitempty"`
 }
 
-type TestAddressConfig struct {
+type testAddressConfig struct {
 	HTTP string `json:"http,omitempty"`
 }
 
-type TestServerConfig struct {
+type testServerConfig struct {
 	Bootstrap bool               `json:"bootstrap,omitempty"`
 	Server    bool               `json:"server,omitempty"`
 	DataDir   string             `json:"data_dir,omitempty"`
 	LogLevel  string             `json:"log_level,omitempty"`
-	Addresses *TestAddressConfig `json:"addresses,omitempty"`
+	Addresses *testAddressConfig `json:"addresses,omitempty"`
-	Ports     TestPortConfig     `json:"ports,omitempty"`
+	Ports     testPortConfig     `json:"ports,omitempty"`
 }
 
-var consulConfig = &TestServerConfig{
+func defaultConfig() *testServerConfig {
-	Bootstrap: true,
+	return &testServerConfig{
-	Server:    true,
+		Bootstrap: true,
-	LogLevel:  "debug",
+		Server:    true,
-	Ports: TestPortConfig{
+		LogLevel:  "debug",
-		DNS:     19000,
+		Ports: testPortConfig{
-		HTTP:    18800,
+			DNS:     19000,
-		RPC:     18600,
+			HTTP:    18800,
-		SerfLan: 18200,
+			RPC:     18600,
-		SerfWan: 18400,
+			SerfLan: 18200,
-		Server:  18000,
+			SerfWan: 18400,
-	},
+			Server:  18000,
+		},
+	}
 }
 
 func (s *testServer) stop() {
@@ -67,10 +69,10 @@ func (s *testServer) stop() {
 }
 
 func newTestServer(t *testing.T) *testServer {
-	return newTestServerWithConfig(t, func(c *TestServerConfig) {})
+	return newTestServerWithConfig(t, func(c *testServerConfig) {})
 }
 
-func newTestServerWithConfig(t *testing.T, cb func(c *TestServerConfig)) *testServer {
+func newTestServerWithConfig(t *testing.T, cb func(c *testServerConfig)) *testServer {
 	if path, err := exec.LookPath("consul"); err != nil || path == "" {
 		t.Log("consul not found on $PATH, skipping")
 		t.SkipNow()
@@ -93,6 +95,7 @@ func newTestServerWithConfig(t *testing.T, cb func(c *TestServerConfig)) *testSe
 		t.Fatalf("err: %s", err)
 	}
 
+	consulConfig := defaultConfig()
 	consulConfig.DataDir = dataDir
 
 	cb(consulConfig)
@@ -125,10 +128,10 @@ func newTestServerWithConfig(t *testing.T, cb func(c *TestServerConfig)) *testSe
 func makeClient(t *testing.T) (*Client, *testServer) {
 	return makeClientWithConfig(t, func(c *Config) {
 		c.Address = "127.0.0.1:18800"
-	}, func(c *TestServerConfig) {})
+	}, func(c *testServerConfig) {})
 }
 
-func makeClientWithConfig(t *testing.T, clientConfig func(c *Config), serverConfig func(c *TestServerConfig)) (*Client, *testServer) {
+func makeClientWithConfig(t *testing.T, clientConfig func(c *Config), serverConfig func(c *testServerConfig)) (*Client, *testServer) {
 	server := newTestServerWithConfig(t, serverConfig)
 	conf := DefaultConfig()
 	clientConfig(conf)

api/status_test.go

@@ -38,14 +38,14 @@ func TestStatusLeaderUnix(t *testing.T) {
 		c.Address = socket
 	}
 
-	serverConfig := func(c *TestServerConfig) {
+	serverConfig := func(c *testServerConfig) {
 		user, err := user.Current()
 		if err != nil {
 			t.Fatal("Could not get current user")
 		}
 
 		if c.Addresses == nil {
-			c.Addresses = &TestAddressConfig{}
+			c.Addresses = &testAddressConfig{}
 		}
 		c.Addresses.HTTP = socket + ";" + user.Uid + ";" + user.Gid + ";640"
 	}

command/agent/config.go

@@ -384,15 +384,14 @@ func populateUnixSocket(addr string) (*UnixSocket, error) {
 	} else {
 		userVal, err = user.Lookup(splitAddr[1])
 	}
-
 	if err != nil {
 		return nil, fmt.Errorf("Invalid user given for Unix socket ownership: %v", splitAddr[1])
+	}
+
+	if uid64, err := strconv.ParseInt(userVal.Uid, 10, 32); err != nil {
+		return nil, fmt.Errorf("Failed to parse given user ID of %v into integer", userVal.Uid)
 	} else {
-		if uid64, err := strconv.ParseInt(userVal.Uid, 10, 32); err != nil {
+		ret.Uid = int(uid64)
-			return nil, fmt.Errorf("Failed to parse given user ID of %v into integer", userVal.Uid)
-		} else {
-			ret.Uid = int(uid64)
-		}
 	}
 
 	// Go doesn't currently have a way to look up gid from group name,

command/agent/config_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"encoding/base64"
 	"io/ioutil"
+	"net"
 	"os"
 	"os/user"
 	"path/filepath"
@@ -1078,12 +1079,12 @@ func TestUnixSockets(t *testing.T) {
 
 	usr, err := user.Current()
 	if err != nil {
-		t.Fatal("Could not get current user")
+		t.Fatal("Could not get current user: ", err)
 	}
 
 	tempdir, err := ioutil.TempDir("", "consul-test-")
 	if err != nil {
-		t.Fatal("Could not create a working directory")
+		t.Fatal("Could not create a working directory: ", err)
 	}
 
 	type SocketTestData struct {
@@ -1124,12 +1125,6 @@ func TestUnixSockets(t *testing.T) {
 		t.Fatal("Did not error on invalid username")
 	}
 
-	std.Uid = "999999"
-	_, err = testUnixSocketPopulation(std)
-	if err == nil {
-		t.Fatal("Did not error on invalid uid")
-	}
-
 	std.Uid = usr.Username
 	std.Gid = "foinfphawepofhewof"
 	_, err = testUnixSocketPopulation(std)
@@ -1137,13 +1132,6 @@ func TestUnixSockets(t *testing.T) {
 		t.Fatal("Did not error on invalid group (a name, must be gid)")
 	}
 
-	std.Uid = usr.Uid
-	std.Gid = "999999"
-	_, err = testUnixSocketPopulation(std)
-	if err == nil {
-		t.Fatal("Did not error on invalid uid")
-	}
-
 	std.Gid = usr.Gid
 	std.Mode = "999"
 	_, err = testUnixSocketPopulation(std)
@@ -1155,17 +1143,37 @@ func TestUnixSockets(t *testing.T) {
 	std.Mode = "640"
 	_, err = testUnixSocketPopulation(std)
 	if err != nil {
-		t.Fatal("Unix socket test failed for no obvious reason (using username)")
+		t.Fatal("Unix socket test failed (using username): ", err)
 	}
 
 	std.Uid = usr.Uid
-	_, err = testUnixSocketPopulation(std)
+	sock, err := testUnixSocketPopulation(std)
 	if err != nil {
-		t.Fatal("Unix socket test failed for no obvious reason (using uid)")
+		t.Fatal("Unix socket test failed (using uid): ", err)
 	}
 
+	addr := &net.UnixAddr{Name: sock.Path, Net: "unix"}
+	_, err = net.Listen(addr.Network(), addr.String())
+	if err != nil {
+		t.Fatal("Error creating socket for futher tests: ", err)
+	}
+
+	std.Uid = "-999999"
+	err = testUnixSocketPermissions(std)
+	if err == nil {
+		t.Fatal("Did not error on invalid uid")
+	}
+
+	std.Uid = usr.Uid
+	std.Gid = "-999999"
+	err = testUnixSocketPermissions(std)
+	if err == nil {
+		t.Fatal("Did not error on invalid uid")
+	}
+
+	std.Gid = usr.Gid
 	err = testUnixSocketPermissions(std)
 	if err != nil {
-		t.Fatal("Adjusting socket permissions failed for no obvious reason")
+		t.Fatal("Adjusting socket permissions failed: ", err)
 	}
 }

command/agent/rpc_client_test.go

@@ -223,12 +223,12 @@ func TestRPCClientStatsUnix(t *testing.T) {
 
 	tempdir, err := ioutil.TempDir("", "consul-test-")
 	if err != nil {
-		t.Fatal("Could not create a working directory")
+		t.Fatal("Could not create a working directory: ", err)
 	}
 
 	user, err := user.Current()
 	if err != nil {
-		t.Fatal("Could not get current user")
+		t.Fatal("Could not get current user: ", err)
 	}
 
 	cb := func(c *Config) {