From 25acd1534a0b5ebec4b1d5403c926d4acb92402a Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Thu, 27 Jul 2017 15:59:58 -0500 Subject: [PATCH 01/10] Move go-socketaddr template parsing into config package to make it happen before creating a new agent. Also removed redundant parsetemplate calls from agent.go. --- agent/agent.go | 111 +---------------------------------------------- agent/config.go | 94 +++++++++++++++++++++++++++++++++++++++ command/agent.go | 4 ++ 3 files changed, 99 insertions(+), 110 deletions(-) diff --git a/agent/agent.go b/agent/agent.go index 4ac1a4314e..ffd7b2272c 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -228,21 +228,9 @@ func New(c *Config) (*Agent, error) { httpAddrs: httpAddrs, tokens: new(token.Store), } - if err := a.resolveTmplAddrs(); err != nil { - return nil, err - } // Try to get an advertise address switch { - case a.config.AdvertiseAddr != "": - ipStr, err := parseSingleIPTemplate(a.config.AdvertiseAddr) - if err != nil { - return nil, fmt.Errorf("Advertise address resolution failed: %v", err) - } - if net.ParseIP(ipStr) == nil { - return nil, fmt.Errorf("Failed to parse advertise address: %v", ipStr) - } - a.config.AdvertiseAddr = ipStr case a.config.BindAddr != "" && !ipaddr.IsAny(a.config.BindAddr): a.config.AdvertiseAddr = a.config.BindAddr @@ -259,16 +247,7 @@ func New(c *Config) (*Agent, error) { } // Try to get an advertise address for the wan - if a.config.AdvertiseAddrWan != "" { - ipStr, err := parseSingleIPTemplate(a.config.AdvertiseAddrWan) - if err != nil { - return nil, fmt.Errorf("Advertise WAN address resolution failed: %v", err) - } - if net.ParseIP(ipStr) == nil { - return nil, fmt.Errorf("Failed to parse advertise address for WAN: %v", ipStr) - } - a.config.AdvertiseAddrWan = ipStr - } else { + if a.config.AdvertiseAddrWan == "" { a.config.AdvertiseAddrWan = a.config.AdvertiseAddr } @@ -839,94 +818,6 @@ func parseSingleIPTemplate(ipTmpl string) (string, error) { } } -// resolveTmplAddrs iterates over the myriad of addresses in the agent's config -// and performs go-sockaddr/template Parse on each known address in case the -// user specified a template config for any of their values. -func (a *Agent) resolveTmplAddrs() error { - if a.config.AdvertiseAddr != "" { - ipStr, err := parseSingleIPTemplate(a.config.AdvertiseAddr) - if err != nil { - return fmt.Errorf("Advertise address resolution failed: %v", err) - } - a.config.AdvertiseAddr = ipStr - } - - if a.config.Addresses.DNS != "" { - ipStr, err := parseSingleIPTemplate(a.config.Addresses.DNS) - if err != nil { - return fmt.Errorf("DNS address resolution failed: %v", err) - } - a.config.Addresses.DNS = ipStr - } - - if a.config.Addresses.HTTP != "" { - ipStr, err := parseSingleIPTemplate(a.config.Addresses.HTTP) - if err != nil { - return fmt.Errorf("HTTP address resolution failed: %v", err) - } - a.config.Addresses.HTTP = ipStr - } - - if a.config.Addresses.HTTPS != "" { - ipStr, err := parseSingleIPTemplate(a.config.Addresses.HTTPS) - if err != nil { - return fmt.Errorf("HTTPS address resolution failed: %v", err) - } - a.config.Addresses.HTTPS = ipStr - } - - if a.config.AdvertiseAddrWan != "" { - ipStr, err := parseSingleIPTemplate(a.config.AdvertiseAddrWan) - if err != nil { - return fmt.Errorf("Advertise WAN address resolution failed: %v", err) - } - a.config.AdvertiseAddrWan = ipStr - } - - if a.config.BindAddr != "" { - ipStr, err := parseSingleIPTemplate(a.config.BindAddr) - if err != nil { - return fmt.Errorf("Bind address resolution failed: %v", err) - } - a.config.BindAddr = ipStr - } - - if a.config.ClientAddr != "" { - ipStr, err := parseSingleIPTemplate(a.config.ClientAddr) - if err != nil { - return fmt.Errorf("Client address resolution failed: %v", err) - } - a.config.ClientAddr = ipStr - } - - if a.config.SerfLanBindAddr != "" { - ipStr, err := parseSingleIPTemplate(a.config.SerfLanBindAddr) - if err != nil { - return fmt.Errorf("Serf LAN Address resolution failed: %v", err) - } - a.config.SerfLanBindAddr = ipStr - } - - if a.config.SerfWanBindAddr != "" { - ipStr, err := parseSingleIPTemplate(a.config.SerfWanBindAddr) - if err != nil { - return fmt.Errorf("Serf WAN Address resolution failed: %v", err) - } - a.config.SerfWanBindAddr = ipStr - } - - // Parse all tagged addresses - for k, v := range a.config.TaggedAddresses { - ipStr, err := parseSingleIPTemplate(v) - if err != nil { - return fmt.Errorf("%s address resolution failed: %v", k, err) - } - a.config.TaggedAddresses[k] = ipStr - } - - return nil -} - // makeRandomID will generate a random UUID for a node. func (a *Agent) makeRandomID() (string, error) { id, err := uuid.GenerateUUID() diff --git a/agent/config.go b/agent/config.go index dd21cd6088..ae30c68e4b 100644 --- a/agent/config.go +++ b/agent/config.go @@ -2121,6 +2121,100 @@ func ReadConfigPaths(paths []string) (*Config, error) { return result, nil } +// ResolveTmplAddrs iterates over the myriad of addresses in the agent's config +// and performs go-sockaddr/template Parse on each known address in case the +// user specified a template config for any of their values. +func (c *Config) ResolveTmplAddrs() error { + if c.AdvertiseAddr != "" { + ipStr, err := parseSingleIPTemplate(c.AdvertiseAddr) + if err != nil { + return fmt.Errorf("Advertise address resolution failed: %v", err) + } + if net.ParseIP(ipStr) == nil { + return fmt.Errorf("Failed to parse advertise address: %v", ipStr) + } + c.AdvertiseAddr = ipStr + } + + if c.Addresses.DNS != "" { + ipStr, err := parseSingleIPTemplate(c.Addresses.DNS) + if err != nil { + return fmt.Errorf("DNS address resolution failed: %v", err) + } + c.Addresses.DNS = ipStr + } + + if c.Addresses.HTTP != "" { + ipStr, err := parseSingleIPTemplate(c.Addresses.HTTP) + if err != nil { + return fmt.Errorf("HTTP address resolution failed: %v", err) + } + c.Addresses.HTTP = ipStr + } + + if c.Addresses.HTTPS != "" { + ipStr, err := parseSingleIPTemplate(c.Addresses.HTTPS) + if err != nil { + return fmt.Errorf("HTTPS address resolution failed: %v", err) + } + c.Addresses.HTTPS = ipStr + } + + if c.AdvertiseAddrWan != "" { + ipStr, err := parseSingleIPTemplate(c.AdvertiseAddrWan) + if err != nil { + return fmt.Errorf("Advertise WAN address resolution failed: %v", err) + } + if net.ParseIP(ipStr) == nil { + return fmt.Errorf("Failed to parse Advertise WAN address: %v", ipStr) + } + c.AdvertiseAddrWan = ipStr + } + + if c.BindAddr != "" { + ipStr, err := parseSingleIPTemplate(c.BindAddr) + if err != nil { + return fmt.Errorf("Bind address resolution failed: %v", err) + } + c.BindAddr = ipStr + } + + if c.ClientAddr != "" { + ipStr, err := parseSingleIPTemplate(c.ClientAddr) + if err != nil { + return fmt.Errorf("Client address resolution failed: %v", err) + } + c.ClientAddr = ipStr + } + + if c.SerfLanBindAddr != "" { + ipStr, err := parseSingleIPTemplate(c.SerfLanBindAddr) + if err != nil { + return fmt.Errorf("Serf LAN Address resolution failed: %v", err) + } + c.SerfLanBindAddr = ipStr + } + + if c.SerfWanBindAddr != "" { + ipStr, err := parseSingleIPTemplate(c.SerfWanBindAddr) + if err != nil { + return fmt.Errorf("Serf WAN Address resolution failed: %v", err) + } + c.SerfWanBindAddr = ipStr + } + + // Parse all tagged addresses + for k, v := range c.TaggedAddresses { + ipStr, err := parseSingleIPTemplate(v) + if err != nil { + return fmt.Errorf("%s address resolution failed: %v", k, err) + } + c.TaggedAddresses[k] = ipStr + } + + return nil +} + // Implement the sort interface for dirEnts func (d dirEnts) Len() int { return len(d) diff --git a/command/agent.go b/command/agent.go index 0df8848b18..c5f43624e6 100644 --- a/command/agent.go +++ b/command/agent.go @@ -462,6 +462,10 @@ func (cmd *AgentCommand) readConfig() *agent.Config { cfg.Version = cmd.Version cfg.VersionPrerelease = cmd.VersionPrerelease + if err := cfg.ResolveTmplAddrs(); err != nil { + cmd.UI.Error(fmt.Sprintf("Failed to parse config: %v", err)) + return nil + } return cfg } From aa98aeb4b1730a56302e058334d4b35d51fe07d4 Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Thu, 27 Jul 2017 22:06:31 -0500 Subject: [PATCH 02/10] Moved handling advertise address to readConfig and out of the agent's constructor, plus unit test fixes --- agent/agent.go | 49 ------------------------------------- agent/agent_test.go | 1 + agent/config.go | 57 ++++++++++++++++++++++++++++++++++++++----- agent/local_test.go | 8 ++++-- agent/testagent.go | 1 + command/agent.go | 7 ++++++ command/agent_test.go | 6 +++++ 7 files changed, 72 insertions(+), 57 deletions(-) diff --git a/agent/agent.go b/agent/agent.go index ffd7b2272c..ea3c807a33 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -5,7 +5,6 @@ import ( "crypto/sha512" "crypto/tls" "encoding/json" - "errors" "fmt" "io" "io/ioutil" @@ -30,7 +29,6 @@ import ( "github.com/hashicorp/consul/logger" "github.com/hashicorp/consul/types" "github.com/hashicorp/consul/watch" - "github.com/hashicorp/go-sockaddr/template" "github.com/hashicorp/go-uuid" "github.com/hashicorp/raft" "github.com/hashicorp/serf/coordinate" @@ -229,34 +227,6 @@ func New(c *Config) (*Agent, error) { tokens: new(token.Store), } - // Try to get an advertise address - switch { - - case a.config.BindAddr != "" && !ipaddr.IsAny(a.config.BindAddr): - a.config.AdvertiseAddr = a.config.BindAddr - - default: - ip, err := consul.GetPrivateIP() - if ipaddr.IsAnyV6(a.config.BindAddr) { - ip, err = consul.GetPublicIPv6() - } - if err != nil { - return nil, fmt.Errorf("Failed to get advertise address: %v", err) - } - a.config.AdvertiseAddr = ip.String() - } - - // Try to get an advertise address for the wan - if a.config.AdvertiseAddrWan == "" { - a.config.AdvertiseAddrWan = a.config.AdvertiseAddr - } - - // Create the default set of tagged addresses. - a.config.TaggedAddresses = map[string]string{ - "lan": a.config.AdvertiseAddr, - "wan": a.config.AdvertiseAddrWan, - } - // Set up the initial state of the token store based on the config. a.tokens.UpdateUserToken(a.config.ACLToken) a.tokens.UpdateAgentToken(a.config.ACLAgentToken) @@ -799,25 +769,6 @@ func (a *Agent) consulConfig() (*consul.Config, error) { return base, nil } -// parseSingleIPTemplate is used as a helper function to parse out a single IP -// address from a config parameter. -func parseSingleIPTemplate(ipTmpl string) (string, error) { - out, err := template.Parse(ipTmpl) - if err != nil { - return "", fmt.Errorf("Unable to parse address template %q: %v", ipTmpl, err) - } - - ips := strings.Split(out, " ") - switch len(ips) { - case 0: - return "", errors.New("No addresses found, please configure one.") - case 1: - return ips[0], nil - default: - return "", fmt.Errorf("Multiple addresses found (%q), please configure one.", out) - } -} - // makeRandomID will generate a random UUID for a node. func (a *Agent) makeRandomID() (string, error) { id, err := uuid.GenerateUUID() diff --git a/agent/agent_test.go b/agent/agent_test.go index ab7463c644..e5cf02edcc 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -117,6 +117,7 @@ func TestAgent_CheckAdvertiseAddrsSettings(t *testing.T) { cfg.AdvertiseAddrs.SerfLan, _ = net.ResolveTCPAddr("tcp", "127.0.0.42:1233") cfg.AdvertiseAddrs.SerfWan, _ = net.ResolveTCPAddr("tcp", "127.0.0.43:1234") cfg.AdvertiseAddrs.RPC, _ = net.ResolveTCPAddr("tcp", "127.0.0.44:1235") + cfg.SetupTaggedAndAdvertiseAddrs() a := NewTestAgent(t.Name(), cfg) defer a.Shutdown() diff --git a/agent/config.go b/agent/config.go index ae30c68e4b..08341dd52e 100644 --- a/agent/config.go +++ b/agent/config.go @@ -16,10 +16,12 @@ import ( "github.com/hashicorp/consul/agent/consul" "github.com/hashicorp/consul/agent/consul/structs" + "github.com/hashicorp/consul/ipaddr" "github.com/hashicorp/consul/lib" "github.com/hashicorp/consul/tlsutil" "github.com/hashicorp/consul/types" "github.com/hashicorp/consul/watch" + "github.com/hashicorp/go-sockaddr/template" "github.com/mitchellh/mapstructure" ) @@ -2203,18 +2205,61 @@ func (c *Config) ResolveTmplAddrs() error { c.SerfWanBindAddr = ipStr } - // Parse all tagged addresses - for k, v := range c.TaggedAddresses { - ipStr, err := parseSingleIPTemplate(v) - if err != nil { - return fmt.Errorf("%s address resolution failed: %v", k, err) + return nil +} + +// Additional post processing of the configs to set tagged and advertise addresses +func (cfg *Config) SetupTaggedAndAdvertiseAddrs() error { + if cfg.AdvertiseAddr == "" { + switch { + + case cfg.BindAddr != "" && !ipaddr.IsAny(cfg.BindAddr): + cfg.AdvertiseAddr = cfg.BindAddr + + default: + ip, err := consul.GetPrivateIP() + if ipaddr.IsAnyV6(cfg.BindAddr) { + ip, err = consul.GetPublicIPv6() + } + if err != nil { + return fmt.Errorf("Failed to get advertise address: %v", err) + } + cfg.AdvertiseAddr = ip.String() } - c.TaggedAddresses[k] = ipStr } + // Try to get an advertise address for the wan + if cfg.AdvertiseAddrWan == "" { + cfg.AdvertiseAddrWan = cfg.AdvertiseAddr + } + + // Create the default set of tagged addresses. + cfg.TaggedAddresses = map[string]string{ + "lan": cfg.AdvertiseAddr, + "wan": cfg.AdvertiseAddrWan, + } return nil } +// parseSingleIPTemplate is used as a helper function to parse out a single IP +// address from a config parameter. +func parseSingleIPTemplate(ipTmpl string) (string, error) { + out, err := template.Parse(ipTmpl) + if err != nil { + return "", fmt.Errorf("Unable to parse address template %q: %v", ipTmpl, err) + } + + ips := strings.Split(out, " ") + switch len(ips) { + case 0: + return "", errors.New("No addresses found, please configure one.") + case 1: + return ips[0], nil + default: + return "", fmt.Errorf("Multiple addresses found (%q), please configure one.", out) + } +} + // Implement the sort interface for dirEnts func (d dirEnts) Len() int { return len(d) diff --git a/agent/local_test.go b/agent/local_test.go index 98406a3763..acd6bcc9d0 100644 --- a/agent/local_test.go +++ b/agent/local_test.go @@ -14,7 +14,9 @@ import ( func TestAgentAntiEntropy_Services(t *testing.T) { t.Parallel() - a := &TestAgent{Name: t.Name(), NoInitialSync: true} + cfg := TestConfig() + a := &TestAgent{Name: t.Name(), NoInitialSync: true, Config: cfg} + a.Start() defer a.Shutdown() @@ -670,8 +672,10 @@ func TestAgentAntiEntropy_Services_ACLDeny(t *testing.T) { func TestAgentAntiEntropy_Checks(t *testing.T) { t.Parallel() - a := &TestAgent{Name: t.Name(), NoInitialSync: true} + cfg := TestConfig() + a := &TestAgent{Name: t.Name(), NoInitialSync: true, Config: cfg} a.Start() + defer a.Shutdown() // Register info diff --git a/agent/testagent.go b/agent/testagent.go index cf5d98c4db..8269d7d335 100644 --- a/agent/testagent.go +++ b/agent/testagent.go @@ -334,6 +334,7 @@ func TestConfig() *Config { ccfg.CoordinateUpdatePeriod = 100 * time.Millisecond ccfg.ServerHealthInterval = 10 * time.Millisecond + cfg.SetupTaggedAndAdvertiseAddrs() return cfg } diff --git a/command/agent.go b/command/agent.go index c5f43624e6..c07a14b8ee 100644 --- a/command/agent.go +++ b/command/agent.go @@ -466,6 +466,13 @@ func (cmd *AgentCommand) readConfig() *agent.Config { cmd.UI.Error(fmt.Sprintf("Failed to parse config: %v", err)) return nil } + // More post processing of the config + if err := cfg.SetupTaggedAndAdvertiseAddrs(); err != nil { + cmd.UI.Error(fmt.Sprintf("Failed to set up tagged and advertise addresses: %v", err)) + return nil + } + // Try to get an advertise address if not set + return cfg } diff --git a/command/agent_test.go b/command/agent_test.go index 9f22f01646..b8a98508e4 100644 --- a/command/agent_test.go +++ b/command/agent_test.go @@ -176,6 +176,7 @@ func TestReadCliConfig(t *testing.T) { args: []string{ "-data-dir", tmpDir, "-node", `"a"`, + "-bind", "1.2.3.4", "-advertise-wan", "1.2.3.4", "-serf-wan-bind", "4.3.2.1", "-serf-lan-bind", "4.3.2.2", @@ -207,6 +208,7 @@ func TestReadCliConfig(t *testing.T) { "-data-dir", tmpDir, "-node-meta", "somekey:somevalue", "-node-meta", "otherkey:othervalue", + "-bind", "1.2.3.4", }, ShutdownCh: shutdownCh, BaseCommand: baseCommand(cli.NewMockUi()), @@ -229,6 +231,7 @@ func TestReadCliConfig(t *testing.T) { "-node", `"server1"`, "-server", "-data-dir", tmpDir, + "-bind", "1.2.3.4", }, ShutdownCh: shutdownCh, BaseCommand: baseCommand(ui), @@ -256,6 +259,7 @@ func TestReadCliConfig(t *testing.T) { args: []string{ "-data-dir", tmpDir, "-node", `"client"`, + "-bind", "1.2.3.4", }, ShutdownCh: shutdownCh, BaseCommand: baseCommand(ui), @@ -301,6 +305,7 @@ func TestAgent_HostBasedIDs(t *testing.T) { cmd := &AgentCommand{ args: []string{ "-data-dir", tmpDir, + "-bind", "127.0.0.1", }, BaseCommand: baseCommand(cli.NewMockUi()), } @@ -317,6 +322,7 @@ func TestAgent_HostBasedIDs(t *testing.T) { args: []string{ "-data-dir", tmpDir, "-disable-host-node-id=false", + "-bind", "127.0.0.1", }, BaseCommand: baseCommand(cli.NewMockUi()), } From 2fcdb35cbb8448a68a558962bbc392db634466b1 Mon Sep 17 00:00:00 2001 From: Frank Schroeder Date: Fri, 28 Jul 2017 12:20:49 +0200 Subject: [PATCH 03/10] config: refactor tmpl resolution fn --- agent/config.go | 97 ++++++++++++------------------------------------- 1 file changed, 23 insertions(+), 74 deletions(-) diff --git a/agent/config.go b/agent/config.go index 08341dd52e..51d689907b 100644 --- a/agent/config.go +++ b/agent/config.go @@ -2126,86 +2126,35 @@ func ReadConfigPaths(paths []string) (*Config, error) { // ResolveTmplAddrs iterates over the myriad of addresses in the agent's config // and performs go-sockaddr/template Parse on each known address in case the // user specified a template config for any of their values. -func (c *Config) ResolveTmplAddrs() error { - if c.AdvertiseAddr != "" { - ipStr, err := parseSingleIPTemplate(c.AdvertiseAddr) +func (c *Config) ResolveTmplAddrs() (err error) { + parse := func(addr *string, name string) { + if *addr == "" || err != nil { + return + } + var ip string + ip, err = parseSingleIPTemplate(*addr) if err != nil { - return fmt.Errorf("Advertise address resolution failed: %v", err) + err = fmt.Errorf("Resolution of %s failed: %v", name, err) + return } - if net.ParseIP(ipStr) == nil { - return fmt.Errorf("Failed to parse advertise address: %v", ipStr) + if net.ParseIP(ip) == nil { + err = fmt.Errorf("Failed to parse %s: %v", name, ip) + return } - c.AdvertiseAddr = ipStr + *addr = ip } - if c.Addresses.DNS != "" { - ipStr, err := parseSingleIPTemplate(c.Addresses.DNS) - if err != nil { - return fmt.Errorf("DNS address resolution failed: %v", err) - } - c.Addresses.DNS = ipStr - } + parse(&c.Addresses.DNS, "DNS address") + parse(&c.Addresses.HTTP, "HTTP address") + parse(&c.Addresses.HTTPS, "HTTPS address") + parse(&c.AdvertiseAddr, "Advertise address") + parse(&c.AdvertiseAddrWan, "Advertise WAN address") + parse(&c.BindAddr, "Bind address") + parse(&c.ClientAddr, "Client address") + parse(&c.SerfLanBindAddr, "Serf LAN address") + parse(&c.SerfWanBindAddr, "Serf WAN address") - if c.Addresses.HTTP != "" { - ipStr, err := parseSingleIPTemplate(c.Addresses.HTTP) - if err != nil { - return fmt.Errorf("HTTP address resolution failed: %v", err) - } - c.Addresses.HTTP = ipStr - } - - if c.Addresses.HTTPS != "" { - ipStr, err := parseSingleIPTemplate(c.Addresses.HTTPS) - if err != nil { - return fmt.Errorf("HTTPS address resolution failed: %v", err) - } - c.Addresses.HTTPS = ipStr - } - - if c.AdvertiseAddrWan != "" { - ipStr, err := parseSingleIPTemplate(c.AdvertiseAddrWan) - if err != nil { - return fmt.Errorf("Advertise WAN address resolution failed: %v", err) - } - if net.ParseIP(ipStr) == nil { - return fmt.Errorf("Failed to parse Advertise WAN address: %v", ipStr) - } - c.AdvertiseAddrWan = ipStr - } - - if c.BindAddr != "" { - ipStr, err := parseSingleIPTemplate(c.BindAddr) - if err != nil { - return fmt.Errorf("Bind address resolution failed: %v", err) - } - c.BindAddr = ipStr - } - - if c.ClientAddr != "" { - ipStr, err := parseSingleIPTemplate(c.ClientAddr) - if err != nil { - return fmt.Errorf("Client address resolution failed: %v", err) - } - c.ClientAddr = ipStr - } - - if c.SerfLanBindAddr != "" { - ipStr, err := parseSingleIPTemplate(c.SerfLanBindAddr) - if err != nil { - return fmt.Errorf("Serf LAN Address resolution failed: %v", err) - } - c.SerfLanBindAddr = ipStr - } - - if c.SerfWanBindAddr != "" { - ipStr, err := parseSingleIPTemplate(c.SerfWanBindAddr) - if err != nil { - return fmt.Errorf("Serf WAN Address resolution failed: %v", err) - } - c.SerfWanBindAddr = ipStr - } - - return nil + return } // Additional post processing of the configs to set tagged and advertise addresses From ac9602e798b2f4596605db0e9536413610eec198 Mon Sep 17 00:00:00 2001 From: Frank Schroeder Date: Fri, 28 Jul 2017 14:53:21 +0200 Subject: [PATCH 04/10] agent: unix sockets are not ip addrs --- agent/config.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/agent/config.go b/agent/config.go index 51d689907b..f9cdf0684c 100644 --- a/agent/config.go +++ b/agent/config.go @@ -2137,6 +2137,10 @@ func (c *Config) ResolveTmplAddrs() (err error) { err = fmt.Errorf("Resolution of %s failed: %v", name, err) return } + if strings.HasPrefix(ip, "unix://") { + *addr = ip + return + } if net.ParseIP(ip) == nil { err = fmt.Errorf("Failed to parse %s: %v", name, ip) return From b19b0621943b50cbe8299302f1cc39bdeba6c45b Mon Sep 17 00:00:00 2001 From: Frank Schroeder Date: Fri, 28 Jul 2017 15:40:22 +0200 Subject: [PATCH 05/10] add tests for go-sockaddr template parsing --- agent/config.go | 3 ++ agent/config_test.go | 91 +++++++++++++++++++++++++++++++++++--------- 2 files changed, 76 insertions(+), 18 deletions(-) diff --git a/agent/config.go b/agent/config.go index f9cdf0684c..4ee7b2df09 100644 --- a/agent/config.go +++ b/agent/config.go @@ -2148,6 +2148,9 @@ func (c *Config) ResolveTmplAddrs() (err error) { *addr = ip } + if c == nil { + return + } parse(&c.Addresses.DNS, "DNS address") parse(&c.Addresses.HTTP, "HTTP address") parse(&c.Addresses.HTTPS, "HTTPS address") diff --git a/agent/config_test.go b/agent/config_test.go index 9ad77ab4a1..eb51b8a06f 100644 --- a/agent/config_test.go +++ b/agent/config_test.go @@ -103,28 +103,56 @@ func TestDecodeConfig(t *testing.T) { c: &Config{ACLTTL: 2 * time.Second, ACLTTLRaw: "2s"}, }, { - in: `{"addresses":{"dns":"a"}}`, - c: &Config{Addresses: AddressConfig{DNS: "a"}}, + in: `{"addresses":{"dns":"1.2.3.4"}}`, + c: &Config{Addresses: AddressConfig{DNS: "1.2.3.4"}}, }, { - in: `{"addresses":{"http":"a"}}`, - c: &Config{Addresses: AddressConfig{HTTP: "a"}}, + in: `{"addresses":{"dns":"{{\"1.2.3.4\"}}"}}`, + c: &Config{Addresses: AddressConfig{DNS: "1.2.3.4"}}, }, { - in: `{"addresses":{"https":"a"}}`, - c: &Config{Addresses: AddressConfig{HTTPS: "a"}}, + in: `{"addresses":{"http":"1.2.3.4"}}`, + c: &Config{Addresses: AddressConfig{HTTP: "1.2.3.4"}}, + }, + { + in: `{"addresses":{"http":"unix:///var/foo/bar"}}`, + c: &Config{Addresses: AddressConfig{HTTP: "unix:///var/foo/bar"}}, + }, + { + in: `{"addresses":{"http":"{{\"1.2.3.4\"}}"}}`, + c: &Config{Addresses: AddressConfig{HTTP: "1.2.3.4"}}, + }, + { + in: `{"addresses":{"https":"1.2.3.4"}}`, + c: &Config{Addresses: AddressConfig{HTTPS: "1.2.3.4"}}, + }, + { + in: `{"addresses":{"https":"unix:///var/foo/bar"}}`, + c: &Config{Addresses: AddressConfig{HTTPS: "unix:///var/foo/bar"}}, + }, + { + in: `{"addresses":{"https":"{{\"1.2.3.4\"}}"}}`, + c: &Config{Addresses: AddressConfig{HTTPS: "1.2.3.4"}}, }, { in: `{"addresses":{"rpc":"a"}}`, c: &Config{Addresses: AddressConfig{RPC: "a"}}, }, { - in: `{"advertise_addr":"a"}`, - c: &Config{AdvertiseAddr: "a"}, + in: `{"advertise_addr":"1.2.3.4"}`, + c: &Config{AdvertiseAddr: "1.2.3.4"}, }, { - in: `{"advertise_addr_wan":"a"}`, - c: &Config{AdvertiseAddrWan: "a"}, + in: `{"advertise_addr":"{{\"1.2.3.4\"}}"}`, + c: &Config{AdvertiseAddr: "1.2.3.4"}, + }, + { + in: `{"advertise_addr_wan":"1.2.3.4"}`, + c: &Config{AdvertiseAddrWan: "1.2.3.4"}, + }, + { + in: `{"advertise_addr_wan":"{{\"1.2.3.4\"}}"}`, + c: &Config{AdvertiseAddrWan: "1.2.3.4"}, }, { in: `{"advertise_addrs":{"rpc":"1.2.3.4:5678"}}`, @@ -202,8 +230,12 @@ func TestDecodeConfig(t *testing.T) { c: &Config{Autopilot: Autopilot{CleanupDeadServers: Bool(true)}}, }, { - in: `{"bind_addr":"a"}`, - c: &Config{BindAddr: "a"}, + in: `{"bind_addr":"1.2.3.4"}`, + c: &Config{BindAddr: "1.2.3.4"}, + }, + { + in: `{"bind_addr":"{{\"1.2.3.4\"}}"}`, + c: &Config{BindAddr: "1.2.3.4"}, }, { in: `{"bootstrap":true}`, @@ -230,8 +262,12 @@ func TestDecodeConfig(t *testing.T) { c: &Config{CertFile: "a"}, }, { - in: `{"client_addr":"a"}`, - c: &Config{ClientAddr: "a"}, + in: `{"client_addr":"1.2.3.4"}`, + c: &Config{ClientAddr: "1.2.3.4"}, + }, + { + in: `{"client_addr":"{{\"1.2.3.4\"}}"}`, + c: &Config{ClientAddr: "1.2.3.4"}, }, { in: `{"data_dir":"a"}`, @@ -531,12 +567,28 @@ func TestDecodeConfig(t *testing.T) { c: &Config{RetryMaxAttemptsWan: 123}, }, { - in: `{"serf_lan_bind":"a"}`, - c: &Config{SerfLanBindAddr: "a"}, + in: `{"serf_lan_bind":"1.2.3.4"}`, + c: &Config{SerfLanBindAddr: "1.2.3.4"}, }, { - in: `{"serf_wan_bind":"a"}`, - c: &Config{SerfWanBindAddr: "a"}, + in: `{"serf_lan_bind":"unix:///var/foo/bar"}`, + c: &Config{SerfLanBindAddr: "unix:///var/foo/bar"}, + }, + { + in: `{"serf_lan_bind":"{{\"1.2.3.4\"}}"}`, + c: &Config{SerfLanBindAddr: "1.2.3.4"}, + }, + { + in: `{"serf_wan_bind":"1.2.3.4"}`, + c: &Config{SerfWanBindAddr: "1.2.3.4"}, + }, + { + in: `{"serf_wan_bind":"unix:///var/foo/bar"}`, + c: &Config{SerfWanBindAddr: "unix:///var/foo/bar"}, + }, + { + in: `{"serf_wan_bind":"{{\"1.2.3.4\"}}"}`, + c: &Config{SerfWanBindAddr: "1.2.3.4"}, }, { in: `{"server":true}`, @@ -1165,6 +1217,9 @@ func TestDecodeConfig(t *testing.T) { if got, want := err, tt.err; !reflect.DeepEqual(got, want) { t.Fatalf("got error %v want %v", got, want) } + if err := c.ResolveTmplAddrs(); err != nil { + t.Fatalf("got error %v on ResolveTmplAddrs", err) + } got, want := c, tt.c verify.Values(t, "", got, want) }) From 840749db7ec6f62d6eac1aaec2f4fa4d55dea4b3 Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Fri, 28 Jul 2017 10:40:43 -0500 Subject: [PATCH 06/10] Fix comments, and remove redundant TestConfig init from a couple of unit tests --- agent/config.go | 2 +- agent/local_test.go | 6 ++---- command/agent.go | 3 +-- 3 files changed, 4 insertions(+), 7 deletions(-) diff --git a/agent/config.go b/agent/config.go index 4ee7b2df09..8e29caab1d 100644 --- a/agent/config.go +++ b/agent/config.go @@ -2164,7 +2164,7 @@ func (c *Config) ResolveTmplAddrs() (err error) { return } -// Additional post processing of the configs to set tagged and advertise addresses +// SetupTaggedAndAdvertiseAddrs configures advertise addresses and sets up a map of tagged addresses func (cfg *Config) SetupTaggedAndAdvertiseAddrs() error { if cfg.AdvertiseAddr == "" { switch { diff --git a/agent/local_test.go b/agent/local_test.go index acd6bcc9d0..6556b182c9 100644 --- a/agent/local_test.go +++ b/agent/local_test.go @@ -14,8 +14,7 @@ import ( func TestAgentAntiEntropy_Services(t *testing.T) { t.Parallel() - cfg := TestConfig() - a := &TestAgent{Name: t.Name(), NoInitialSync: true, Config: cfg} + a := &TestAgent{Name: t.Name(), NoInitialSync: true} a.Start() defer a.Shutdown() @@ -672,8 +671,7 @@ func TestAgentAntiEntropy_Services_ACLDeny(t *testing.T) { func TestAgentAntiEntropy_Checks(t *testing.T) { t.Parallel() - cfg := TestConfig() - a := &TestAgent{Name: t.Name(), NoInitialSync: true, Config: cfg} + a := &TestAgent{Name: t.Name(), NoInitialSync: true} a.Start() defer a.Shutdown() diff --git a/command/agent.go b/command/agent.go index c07a14b8ee..bea22a6479 100644 --- a/command/agent.go +++ b/command/agent.go @@ -466,12 +466,11 @@ func (cmd *AgentCommand) readConfig() *agent.Config { cmd.UI.Error(fmt.Sprintf("Failed to parse config: %v", err)) return nil } - // More post processing of the config + if err := cfg.SetupTaggedAndAdvertiseAddrs(); err != nil { cmd.UI.Error(fmt.Sprintf("Failed to set up tagged and advertise addresses: %v", err)) return nil } - // Try to get an advertise address if not set return cfg } From 13c118ea516607d5823bcc495bf63ade2ed5c0d2 Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Fri, 28 Jul 2017 10:51:11 -0500 Subject: [PATCH 07/10] Removed extra newlines --- agent/local_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/agent/local_test.go b/agent/local_test.go index 6556b182c9..98406a3763 100644 --- a/agent/local_test.go +++ b/agent/local_test.go @@ -15,7 +15,6 @@ import ( func TestAgentAntiEntropy_Services(t *testing.T) { t.Parallel() a := &TestAgent{Name: t.Name(), NoInitialSync: true} - a.Start() defer a.Shutdown() @@ -673,7 +672,6 @@ func TestAgentAntiEntropy_Checks(t *testing.T) { t.Parallel() a := &TestAgent{Name: t.Name(), NoInitialSync: true} a.Start() - defer a.Shutdown() // Register info From 4cec55e8dbf42562a6e0d20297989fb6e51c4f4a Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Fri, 28 Jul 2017 15:01:32 -0500 Subject: [PATCH 08/10] Modify ResolveTmplAddrs to parse advertise IPs, added test cases that fail to parse correctly --- agent/config.go | 27 ++++++++++++--------------- agent/config_test.go | 37 +++++++++++++++++++++++++------------ 2 files changed, 37 insertions(+), 27 deletions(-) diff --git a/agent/config.go b/agent/config.go index 8e29caab1d..437e33641d 100644 --- a/agent/config.go +++ b/agent/config.go @@ -2127,7 +2127,7 @@ func ReadConfigPaths(paths []string) (*Config, error) { // and performs go-sockaddr/template Parse on each known address in case the // user specified a template config for any of their values. func (c *Config) ResolveTmplAddrs() (err error) { - parse := func(addr *string, name string) { + parse := func(addr *string, validateIP bool, name string) { if *addr == "" || err != nil { return } @@ -2137,11 +2137,8 @@ func (c *Config) ResolveTmplAddrs() (err error) { err = fmt.Errorf("Resolution of %s failed: %v", name, err) return } - if strings.HasPrefix(ip, "unix://") { - *addr = ip - return - } - if net.ParseIP(ip) == nil { + + if validateIP && net.ParseIP(ip) == nil { err = fmt.Errorf("Failed to parse %s: %v", name, ip) return } @@ -2151,15 +2148,15 @@ func (c *Config) ResolveTmplAddrs() (err error) { if c == nil { return } - parse(&c.Addresses.DNS, "DNS address") - parse(&c.Addresses.HTTP, "HTTP address") - parse(&c.Addresses.HTTPS, "HTTPS address") - parse(&c.AdvertiseAddr, "Advertise address") - parse(&c.AdvertiseAddrWan, "Advertise WAN address") - parse(&c.BindAddr, "Bind address") - parse(&c.ClientAddr, "Client address") - parse(&c.SerfLanBindAddr, "Serf LAN address") - parse(&c.SerfWanBindAddr, "Serf WAN address") + parse(&c.Addresses.DNS, false, "DNS address") + parse(&c.Addresses.HTTP, false, "HTTP address") + parse(&c.Addresses.HTTPS, false, "HTTPS address") + parse(&c.AdvertiseAddr, true, "Advertise address") + parse(&c.AdvertiseAddrWan, true, "Advertise WAN address") + parse(&c.BindAddr, false, "Bind address") + parse(&c.ClientAddr, false, "Client address") + parse(&c.SerfLanBindAddr, true, "Serf LAN address") + parse(&c.SerfWanBindAddr, true, "Serf WAN address") return } diff --git a/agent/config_test.go b/agent/config_test.go index eb51b8a06f..dca9d7cc3b 100644 --- a/agent/config_test.go +++ b/agent/config_test.go @@ -50,18 +50,28 @@ func TestConfigEncryptBytes(t *testing.T) { func TestDecodeConfig(t *testing.T) { tests := []struct { - desc string - in string - c *Config - err error + desc string + in string + c *Config + err error + parseTemplateErr error }{ // special flows { in: `{"bad": "no way jose"}`, err: errors.New("Config has invalid keys: bad"), }, - - // happy flows in alphabeical order + { + in: `{"advertise_addr":"unix:///path/to/file"}`, + parseTemplateErr: errors.New("Failed to parse Advertise address: unix:///path/to/file"), + c: &Config{AdvertiseAddr: "unix:///path/to/file"}, + }, + { + in: `{"advertise_addr_wan":"unix:///path/to/file"}`, + parseTemplateErr: errors.New("Failed to parse Advertise WAN address: unix:///path/to/file"), + c: &Config{AdvertiseAddrWan: "unix:///path/to/file"}, + }, + // happy flows in alphabetical order { in: `{"acl_agent_master_token":"a"}`, c: &Config{ACLAgentMasterToken: "a"}, @@ -571,8 +581,9 @@ func TestDecodeConfig(t *testing.T) { c: &Config{SerfLanBindAddr: "1.2.3.4"}, }, { - in: `{"serf_lan_bind":"unix:///var/foo/bar"}`, - c: &Config{SerfLanBindAddr: "unix:///var/foo/bar"}, + in: `{"serf_lan_bind":"unix:///var/foo/bar"}`, + c: &Config{SerfLanBindAddr: "unix:///var/foo/bar"}, + parseTemplateErr: errors.New("Failed to parse Serf LAN address: unix:///var/foo/bar"), }, { in: `{"serf_lan_bind":"{{\"1.2.3.4\"}}"}`, @@ -583,8 +594,9 @@ func TestDecodeConfig(t *testing.T) { c: &Config{SerfWanBindAddr: "1.2.3.4"}, }, { - in: `{"serf_wan_bind":"unix:///var/foo/bar"}`, - c: &Config{SerfWanBindAddr: "unix:///var/foo/bar"}, + in: `{"serf_wan_bind":"unix:///var/foo/bar"}`, + c: &Config{SerfWanBindAddr: "unix:///var/foo/bar"}, + parseTemplateErr: errors.New("Failed to parse Serf WAN address: unix:///var/foo/bar"), }, { in: `{"serf_wan_bind":"{{\"1.2.3.4\"}}"}`, @@ -1217,8 +1229,9 @@ func TestDecodeConfig(t *testing.T) { if got, want := err, tt.err; !reflect.DeepEqual(got, want) { t.Fatalf("got error %v want %v", got, want) } - if err := c.ResolveTmplAddrs(); err != nil { - t.Fatalf("got error %v on ResolveTmplAddrs", err) + err = c.ResolveTmplAddrs() + if got, want := err, tt.parseTemplateErr; !reflect.DeepEqual(got, want) { + t.Fatalf("got error %v on ResolveTmplAddrs, expected %v", err, want) } got, want := c, tt.c verify.Values(t, "", got, want) From 5aeab1463bece85363327a49a46e9f8d7cd29496 Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Fri, 28 Jul 2017 17:18:10 -0500 Subject: [PATCH 09/10] Validate unix sockets and ip addresses as needed, more test cases --- agent/config.go | 29 +++++++++++++++++------------ agent/config_test.go | 6 ++++++ 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/agent/config.go b/agent/config.go index 437e33641d..cc0958c85d 100644 --- a/agent/config.go +++ b/agent/config.go @@ -2127,7 +2127,7 @@ func ReadConfigPaths(paths []string) (*Config, error) { // and performs go-sockaddr/template Parse on each known address in case the // user specified a template config for any of their values. func (c *Config) ResolveTmplAddrs() (err error) { - parse := func(addr *string, validateIP bool, name string) { + parse := func(addr *string, socketAllowed bool, name string) { if *addr == "" || err != nil { return } @@ -2137,26 +2137,31 @@ func (c *Config) ResolveTmplAddrs() (err error) { err = fmt.Errorf("Resolution of %s failed: %v", name, err) return } - - if validateIP && net.ParseIP(ip) == nil { + ipAddr := net.ParseIP(ip) + if !socketAllowed && ipAddr == nil { err = fmt.Errorf("Failed to parse %s: %v", name, ip) return } + if socketAllowed && socketPath(ip) == "" && ipAddr == nil { + err = fmt.Errorf("Failed to parse %s, is not a valid IP address or socket: %v", name, ip) + return + } + *addr = ip } if c == nil { return } - parse(&c.Addresses.DNS, false, "DNS address") - parse(&c.Addresses.HTTP, false, "HTTP address") - parse(&c.Addresses.HTTPS, false, "HTTPS address") - parse(&c.AdvertiseAddr, true, "Advertise address") - parse(&c.AdvertiseAddrWan, true, "Advertise WAN address") - parse(&c.BindAddr, false, "Bind address") - parse(&c.ClientAddr, false, "Client address") - parse(&c.SerfLanBindAddr, true, "Serf LAN address") - parse(&c.SerfWanBindAddr, true, "Serf WAN address") + parse(&c.Addresses.DNS, true, "DNS address") + parse(&c.Addresses.HTTP, true, "HTTP address") + parse(&c.Addresses.HTTPS, true, "HTTPS address") + parse(&c.AdvertiseAddr, false, "Advertise address") + parse(&c.AdvertiseAddrWan, false, "Advertise WAN address") + parse(&c.BindAddr, true, "Bind address") + parse(&c.ClientAddr, true, "Client address") + parse(&c.SerfLanBindAddr, false, "Serf LAN address") + parse(&c.SerfWanBindAddr, false, "Serf WAN address") return } diff --git a/agent/config_test.go b/agent/config_test.go index dca9d7cc3b..13b3738c2a 100644 --- a/agent/config_test.go +++ b/agent/config_test.go @@ -71,6 +71,12 @@ func TestDecodeConfig(t *testing.T) { parseTemplateErr: errors.New("Failed to parse Advertise WAN address: unix:///path/to/file"), c: &Config{AdvertiseAddrWan: "unix:///path/to/file"}, }, + { + in: `{"addresses":{"http":"notunix://blah"}}`, + parseTemplateErr: errors.New("Failed to parse HTTP address, is not a valid IP address or socket: notunix://blah"), + c: &Config{Addresses: AddressConfig{HTTP: "notunix://blah"}}, + }, + // happy flows in alphabetical order { in: `{"acl_agent_master_token":"a"}`, From 2d84cd2330e116b7df0208d2421de2d425a5958e Mon Sep 17 00:00:00 2001 From: Preetha Appan Date: Fri, 28 Jul 2017 17:52:35 -0500 Subject: [PATCH 10/10] Tweaked parsing error message to quote properly --- agent/config.go | 2 +- agent/config_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/agent/config.go b/agent/config.go index cc0958c85d..c6679064ec 100644 --- a/agent/config.go +++ b/agent/config.go @@ -2143,7 +2143,7 @@ func (c *Config) ResolveTmplAddrs() (err error) { return } if socketAllowed && socketPath(ip) == "" && ipAddr == nil { - err = fmt.Errorf("Failed to parse %s, is not a valid IP address or socket: %v", name, ip) + err = fmt.Errorf("Failed to parse %s, %q is not a valid IP address or socket", name, ip) return } diff --git a/agent/config_test.go b/agent/config_test.go index 13b3738c2a..101f87b8f2 100644 --- a/agent/config_test.go +++ b/agent/config_test.go @@ -73,7 +73,7 @@ func TestDecodeConfig(t *testing.T) { }, { in: `{"addresses":{"http":"notunix://blah"}}`, - parseTemplateErr: errors.New("Failed to parse HTTP address, is not a valid IP address or socket: notunix://blah"), + parseTemplateErr: errors.New("Failed to parse HTTP address, \"notunix://blah\" is not a valid IP address or socket"), c: &Config{Addresses: AddressConfig{HTTP: "notunix://blah"}}, },