From f88536f1927717f4f68c7785215b0462370969b8 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Tue, 13 Jan 2015 17:50:17 +0000 Subject: [PATCH] Remove unnecessary ClientListenerAddr function. Rework config test functions to be cleaner. Start of runtime tests. This code is copyright 2014 Akamai Technologies, Inc. --- command/agent/agent_test.go | 33 +++++++++++++++- command/agent/command.go | 8 ++-- command/agent/config.go | 25 +++++------- command/agent/config_test.go | 73 +++++++++++++++++++++++++++--------- 4 files changed, 100 insertions(+), 39 deletions(-) diff --git a/command/agent/agent_test.go b/command/agent/agent_test.go index 0add36702..91cb5c1ef 100644 --- a/command/agent/agent_test.go +++ b/command/agent/agent_test.go @@ -7,8 +7,10 @@ import ( "io" "io/ioutil" "os" + "os/user" "path/filepath" "reflect" + "runtime" "sync/atomic" "testing" "time" @@ -123,7 +125,7 @@ func TestAgentStartStop(t *testing.T) { } } -func TestAgent_RPCPing(t *testing.T) { +func TestAgent_RPCPingTCP(t *testing.T) { dir, agent := makeAgent(t, nextConfig()) defer os.RemoveAll(dir) defer agent.Shutdown() @@ -134,6 +136,35 @@ func TestAgent_RPCPing(t *testing.T) { } } +func TestAgent_RPCPingUnix(t *testing.T) { + if runtime.GOOS == "windows" { + t.SkipNow() + } + + nextConf := nextConfig() + + tempdir, err := ioutil.TempDir("", "consul-test-") + if err != nil { + t.Fatal("Could not create a working directory") + } + + user, err := user.Current() + if err != nil { + t.Fatal("Could not get current user") + } + + nextConf.Addresses.RPC = "unix://" + tempdir + "/unix-rpc-test.sock;" + user.Uid + ";" + user.Gid + ";640" + + dir, agent := makeAgent(t, nextConf) + defer os.RemoveAll(dir) + defer agent.Shutdown() + + var out struct{} + if err := agent.RPC("Status.Ping", struct{}{}, &out); err != nil { + t.Fatalf("err: %v", err) + } +} + func TestAgent_AddService(t *testing.T) { dir, agent := makeAgent(t, nextConfig()) defer os.RemoveAll(dir) diff --git a/command/agent/command.go b/command/agent/command.go index c7da28f4d..b9a82e19a 100644 --- a/command/agent/command.go +++ b/command/agent/command.go @@ -589,7 +589,7 @@ func (c *Command) Run(args []string) int { } // Get the new client http listener addr - httpAddr, err := config.ClientListenerAddr(config.Addresses.HTTP, config.Ports.HTTP) + httpAddr, err := config.ClientListener(config.Addresses.HTTP, config.Ports.HTTP) if err != nil { c.Ui.Error(fmt.Sprintf("Failed to determine HTTP address: %v", err)) } @@ -599,7 +599,7 @@ func (c *Command) Run(args []string) int { go func(wp *watch.WatchPlan) { wp.Handler = makeWatchHandler(logOutput, wp.Exempt["handler"]) wp.LogOutput = c.logOutput - if err := wp.Run(httpAddr); err != nil { + if err := wp.Run(httpAddr.String()); err != nil { c.Ui.Error(fmt.Sprintf("Error running watch: %v", err)) } }(wp) @@ -758,7 +758,7 @@ func (c *Command) handleReload(config *Config) *Config { } // Get the new client listener addr - httpAddr, err := newConf.ClientListenerAddr(config.Addresses.HTTP, config.Ports.HTTP) + httpAddr, err := newConf.ClientListener(config.Addresses.HTTP, config.Ports.HTTP) if err != nil { c.Ui.Error(fmt.Sprintf("Failed to determine HTTP address: %v", err)) } @@ -773,7 +773,7 @@ func (c *Command) handleReload(config *Config) *Config { go func(wp *watch.WatchPlan) { wp.Handler = makeWatchHandler(c.logOutput, wp.Exempt["handler"]) wp.LogOutput = c.logOutput - if err := wp.Run(httpAddr); err != nil { + if err := wp.Run(httpAddr.String()); err != nil { c.Ui.Error(fmt.Sprintf("Error running watch: %v", err)) } }(wp) diff --git a/command/agent/config.go b/command/agent/config.go index 4cd82390c..e20e78ac6 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -500,26 +500,19 @@ func (c *Config) ClientListener(override string, port int) (net.Addr, error) { if ip == nil { return nil, fmt.Errorf("Failed to parse IP: %v", addr) } + + if ip.IsUnspecified() { + ip = net.ParseIP("127.0.0.1") + } + + if ip == nil { + return nil, fmt.Errorf("Failed to parse IP 127.0.0.1") + } + return &net.TCPAddr{IP: ip, Port: port}, nil } } -// ClientListenerAddr is used to format an address for a -// port on a ClientAddr, handling the zero IP. -func (c *Config) ClientListenerAddr(override string, port int) (string, error) { - addr, err := c.ClientListener(override, port) - if err != nil { - return "", err - } - - if ipAddr, ok := addr.(*net.TCPAddr); ok { - if ipAddr.IP.IsUnspecified() { - ipAddr.IP = net.ParseIP("127.0.0.1") - } - } - return addr.String(), nil -} - // DecodeConfig reads the configuration from the given reader in JSON // format and decodes it into a proper Config structure. func DecodeConfig(r io.Reader) (*Config, error) { diff --git a/command/agent/config_test.go b/command/agent/config_test.go index 597bcebd4..244ea9f13 100644 --- a/command/agent/config_test.go +++ b/command/agent/config_test.go @@ -1076,17 +1076,7 @@ func TestUnixSockets(t *testing.T) { t.SkipNow() } - _, err := populateUnixSocket("tcp://abc123") - if err == nil { - t.Fatal("Should have rejected invalid scheme") - } - - _, err = populateUnixSocket("unix://x;y;z") - if err == nil { - t.Fatal("Should have rejected invalid number of parameters in Unix socket definition") - } - - user, err := user.Current() + usr, err := user.Current() if err != nil { t.Fatal("Could not get current user") } @@ -1096,39 +1086,86 @@ func TestUnixSockets(t *testing.T) { t.Fatal("Could not create a working directory") } - _, err = populateUnixSocket("unix://" + tempdir + "/unixtest.sock;osdfjo9ihf9h82;" + user.Gid + ";640") + type SocketTestData struct { + Path string + Uid string + Gid string + Mode string + } + + testUnixSocketPopulation := func(s SocketTestData) (*UnixSocket, error) { + return populateUnixSocket("unix://" + s.Path + ";" + s.Uid + ";" + s.Gid + ";" + s.Mode) + } + + testUnixSocketPermissions := func(s SocketTestData) error { + return adjustUnixSocketPermissions("unix://" + s.Path + ";" + s.Uid + ";" + s.Gid + ";" + s.Mode) + } + + _, err = populateUnixSocket("tcp://abc123") + if err == nil { + t.Fatal("Should have rejected invalid scheme") + } + + _, err = populateUnixSocket("unix://x;y;z") + if err == nil { + t.Fatal("Should have rejected invalid number of parameters in Unix socket definition") + } + + std := SocketTestData{ + Path: tempdir + "/unix-config-test.sock", + Uid: usr.Uid, + Gid: usr.Gid, + Mode: "640", + } + + std.Uid = "orasdfdsnfoinweroiu" + _, err = testUnixSocketPopulation(std) if err == nil { t.Fatal("Did not error on invalid username") } - _, err = populateUnixSocket("unix://" + tempdir + "/unixtest.sock;999999;" + user.Gid + ";640") + std.Uid = "999999" + _, err = testUnixSocketPopulation(std) if err == nil { t.Fatal("Did not error on invalid uid") } - _, err = populateUnixSocket("unix://" + tempdir + "/unixtest.sock;" + user.Username + ";foihafwereworg;" + ";640") + std.Uid = usr.Username + std.Gid = "foinfphawepofhewof" + _, err = testUnixSocketPopulation(std) if err == nil { t.Fatal("Did not error on invalid group (a name, must be gid)") } - _, err = populateUnixSocket("unix://" + tempdir + "/unixtest.sock;" + user.Username + ";999999;" + ";640") + std.Uid = usr.Uid + std.Gid = "999999" + _, err = testUnixSocketPopulation(std) if err == nil { t.Fatal("Did not error on invalid uid") } - _, err = populateUnixSocket("unix://" + tempdir + "/unixtest.sock;" + user.Username + ";" + user.Gid + ";999") + std.Gid = usr.Gid + std.Mode = "999" + _, err = testUnixSocketPopulation(std) if err == nil { t.Fatal("Did not error on invalid socket mode") } - _, err = populateUnixSocket("unix://" + tempdir + "/unixtest.sock;" + user.Username + ";" + user.Gid + ";640") + std.Uid = usr.Username + std.Mode = "640" + _, err = testUnixSocketPopulation(std) if err != nil { t.Fatal("Unix socket test failed for no obvious reason (using username)") } - _, err = populateUnixSocket("unix://" + tempdir + "/unixtest.sock;" + user.Uid + ";" + user.Gid + ";640") + std.Uid = usr.Uid + _, err = testUnixSocketPopulation(std) if err != nil { t.Fatal("Unix socket test failed for no obvious reason (using uid)") } + err = testUnixSocketPermissions(std) + if err != nil { + t.Fatal("Adjusting socket permissions failed for no obvious reason") + } }