From f673d5a7378ac6e5e9dff9445d53ad35ab197a88 Mon Sep 17 00:00:00 2001 From: Kevin Schoonover Date: Sun, 28 Nov 2021 15:58:16 -0700 Subject: [PATCH 01/14] progress save creates multiple http server but doesn't handle cleanup / refresh. See TODOs for more details. --- command/agent/agent.go | 4 +- command/agent/command.go | 10 ++- command/agent/config.go | 72 +++++++++++++-- command/agent/config_test.go | 57 +++++++++++- command/agent/http.go | 168 ++++++++++++++++++----------------- command/agent/testagent.go | 6 +- 6 files changed, 218 insertions(+), 99 deletions(-) diff --git a/command/agent/agent.go b/command/agent/agent.go index b070de555968..d2966620d3ab 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -935,7 +935,9 @@ func (a *Agent) setupClient() error { // If no HTTP health check can be supported nil is returned. func (a *Agent) agentHTTPCheck(server bool) *structs.ServiceCheck { // Resolve the http check address - httpCheckAddr := a.config.normalizedAddrs.HTTP + // TODO: evaluate if this is the best way to handle this default, maybe + // BindAddr? + httpCheckAddr := a.config.normalizedAddrs.HTTP[0] if *a.config.Consul.ChecksUseAdvertise { httpCheckAddr = a.config.AdvertiseAddrs.HTTP } diff --git a/command/agent/command.go b/command/agent/command.go index 558e63ee120e..c8bc0d454d54 100644 --- a/command/agent/command.go +++ b/command/agent/command.go @@ -500,13 +500,14 @@ func (c *Command) setupAgent(config *Config, logger hclog.InterceptLogger, logOu c.agent = agent // Setup the HTTP server - http, err := NewHTTPServer(agent, config) + httpServers, err := NewHTTPServers(agent, config) if err != nil { agent.Shutdown() c.Ui.Error(fmt.Sprintf("Error starting http server: %s", err)) return err } - c.httpServer = http + // TODO: update for multiple or change the type of NewHTTPServer + c.httpServer = &httpServers[0] // If DisableUpdateCheck is not enabled, set up update checking // (DisableUpdateCheck is false by default) @@ -904,11 +905,12 @@ func (c *Command) reloadHTTPServer() error { c.httpServer.Shutdown() - http, err := NewHTTPServer(c.agent, c.agent.config) + httpServers, err := NewHTTPServers(c.agent, c.agent.config) if err != nil { return err } - c.httpServer = http + // TODO: change type or handle with list + c.httpServer = &httpServers[0] return nil } diff --git a/command/agent/config.go b/command/agent/config.go index 740da6cb6b18..dfe3242cca2b 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -85,7 +85,7 @@ type Config struct { Addresses *Addresses `hcl:"addresses"` // normalizedAddr is set to the Address+Port by normalizeAddrs() - normalizedAddrs *Addresses + normalizedAddrs *NormalizedAddrs // AdvertiseAddrs is used to control the addresses we advertise. AdvertiseAddrs *AdvertiseAddrs `hcl:"advertise"` @@ -774,6 +774,15 @@ type Addresses struct { ExtraKeysHCL []string `hcl:",unusedKeys" json:"-"` } +// AdvertiseAddrs is used to control the addresses we advertise out for +// different network services. All are optional and default to BindAddr and +// their default Port. +type NormalizedAddrs struct { + HTTP []string + RPC string + Serf string +} + // AdvertiseAddrs is used to control the addresses we advertise out for // different network services. All are optional and default to BindAddr and // their default Port. @@ -1228,13 +1237,13 @@ func (c *Config) normalizeAddrs() error { c.BindAddr = ipStr } - addr, err := normalizeBind(c.Addresses.HTTP, c.BindAddr) + httpAddrs, err := normalizeMultipleBind(c.Addresses.HTTP, c.BindAddr) if err != nil { return fmt.Errorf("Failed to parse HTTP address: %v", err) } - c.Addresses.HTTP = addr + c.Addresses.HTTP = strings.Join(httpAddrs, " ") - addr, err = normalizeBind(c.Addresses.RPC, c.BindAddr) + addr, err := normalizeBind(c.Addresses.RPC, c.BindAddr) if err != nil { return fmt.Errorf("Failed to parse RPC address: %v", err) } @@ -1246,13 +1255,20 @@ func (c *Config) normalizeAddrs() error { } c.Addresses.Serf = addr - c.normalizedAddrs = &Addresses{ - HTTP: net.JoinHostPort(c.Addresses.HTTP, strconv.Itoa(c.Ports.HTTP)), + c.normalizedAddrs = &NormalizedAddrs{ + HTTP: joinHostPorts(httpAddrs, strconv.Itoa(c.Ports.HTTP)), RPC: net.JoinHostPort(c.Addresses.RPC, strconv.Itoa(c.Ports.RPC)), Serf: net.JoinHostPort(c.Addresses.Serf, strconv.Itoa(c.Ports.Serf)), } - addr, err = normalizeAdvertise(c.AdvertiseAddrs.HTTP, c.Addresses.HTTP, c.Ports.HTTP, c.DevMode) + // defaultHTTPAdvertiseAddr := c.BindAddr + // // Preserving the old behavior to defaulting to address.http field if only + // // 1 ip is specified + // if len(httpAddrs) == 1 { + // defaultHTTPAdvertiseAddr = httpAddrs[0] + // } + + addr, err = normalizeAdvertise(c.AdvertiseAddrs.HTTP, c.BindAddr, c.Ports.HTTP, c.DevMode) if err != nil { return fmt.Errorf("Failed to parse HTTP advertise address (%v, %v, %v, %v): %v", c.AdvertiseAddrs.HTTP, c.Addresses.HTTP, c.Ports.HTTP, c.DevMode, err) } @@ -1335,6 +1351,23 @@ func parseSingleIPTemplate(ipTmpl string) (string, error) { } } +// parseMultipleIPTemplate is used as a helper function to parse out a multiple IP +// addresses from a config parameter. +func parseMultipleIPTemplate(ipTmpl string) ([]string, error) { + out, err := template.Parse(ipTmpl) + if err != nil { + return []string{}, fmt.Errorf("Unable to parse address template %q: %v", ipTmpl, err) + } + + // TODO: deduplicate the parsed ips + ips := strings.Split(out, " ") + if len(ips) == 0 { + return []string{}, errors.New("No addresses found, please configure one.") + } + + return ips, nil +} + // normalizeBind returns a normalized bind address. // // If addr is set it is used, if not the default bind address is used. @@ -1345,6 +1378,16 @@ func normalizeBind(addr, bind string) (string, error) { return parseSingleIPTemplate(addr) } +// normalizeMultipleBind returns normalized bind addresses. +// +// If addr is set it is used, if not the default bind address is used. +func normalizeMultipleBind(addr, bind string) ([]string, error) { + if addr == "" { + return []string{bind}, nil + } + return parseMultipleIPTemplate(addr) +} + // normalizeAdvertise returns a normalized advertise address. // // If addr is set, it is used and the default port is appended if no port is @@ -1363,7 +1406,9 @@ func normalizeAdvertise(addr string, bind string, defport int, dev bool) (string return "", fmt.Errorf("Error parsing advertise address template: %v", err) } + fmt.Println("addr", addr) if addr != "" { + fmt.Println("test1") // Default to using manually configured address _, _, err = net.SplitHostPort(addr) if err != nil { @@ -1378,6 +1423,8 @@ func normalizeAdvertise(addr string, bind string, defport int, dev bool) (string return addr, nil } + fmt.Println("bind", bind) + fmt.Println("test2") // Fallback to bind address first, and then try resolving the local hostname ips, err := net.LookupIP(bind) if err != nil { @@ -1996,6 +2043,17 @@ func LoadConfigDir(dir string) (*Config, error) { return result, nil } +// joinHostPorts joins every addr in addrs with the specified port +func joinHostPorts(addrs []string, port string) []string { + localAddrs := addrs + for i, k := range addrs { + localAddrs[i] = net.JoinHostPort(k, port) + + } + + return localAddrs +} + // isTemporaryFile returns true or false depending on whether the // provided file name is a temporary file for the following editors: // emacs or vim. diff --git a/command/agent/config_test.go b/command/agent/config_test.go index 8bb39a98fed6..1d88bbc33374 100644 --- a/command/agent/config_test.go +++ b/command/agent/config_test.go @@ -747,7 +747,7 @@ func TestConfig_normalizeAddrs_DevMode(t *testing.T) { t.Fatalf("expected BindAddr 127.0.0.1, got %s", c.BindAddr) } - if c.normalizedAddrs.HTTP != "127.0.0.1:4646" { + if c.normalizedAddrs.HTTP[0] != "127.0.0.1:4646" { t.Fatalf("expected HTTP address 127.0.0.1:4646, got %s", c.normalizedAddrs.HTTP) } @@ -880,6 +880,56 @@ func TestConfig_normalizeAddrs_IPv6Loopback(t *testing.T) { } } +// TestConfig_normalizeAddrs_MultipleInterface asserts that normalizeAddrs will +// handle normalizing multiple interfaces in a single protocol. +func TestConfig_normalizeAddrs_MultipleInterfaces(t *testing.T) { + // TODO: add test with go-sockaddr templates + testCases := []struct { + name string + addressConfig *Addresses + expectedNormalizedAddrs *NormalizedAddrs + expectErr bool + }{ + { + name: "multiple http addresses", + addressConfig: &Addresses{ + HTTP: "127.0.0.1 127.0.0.2", + }, + expectedNormalizedAddrs: &NormalizedAddrs{ + HTTP: []string{"127.0.0.1:4646", "127.0.0.2:4646"}, + RPC: "127.0.0.1:4647", + Serf: "127.0.0.1:4648", + }, + expectErr: false, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + c := &Config{ + BindAddr: "127.0.0.1", + Ports: &Ports{ + HTTP: 4646, + RPC: 4647, + Serf: 4648, + }, + Addresses: tc.addressConfig, + AdvertiseAddrs: &AdvertiseAddrs{ + HTTP: "127.0.0.1", + RPC: "127.0.0.1", + Serf: "127.0.0.1", + }, + } + err := c.normalizeAddrs() + if tc.expectErr { + require.Error(t, err) + return + } + require.NoError(t, err) + require.Equal(t, tc.expectedNormalizedAddrs, c.normalizedAddrs) + }) + } +} + func TestConfig_normalizeAddrs(t *testing.T) { c := &Config{ BindAddr: "169.254.1.5", @@ -907,8 +957,9 @@ func TestConfig_normalizeAddrs(t *testing.T) { t.Fatalf("expected BindAddr 169.254.1.5, got %s", c.BindAddr) } - if c.AdvertiseAddrs.HTTP != "169.254.1.10:4646" { - t.Fatalf("expected HTTP advertise address 169.254.1.10:4646, got %s", c.AdvertiseAddrs.HTTP) + // was this test case incorrect? should default to bind addr if not specified (https://www.nomadproject.io/docs/configuration#advertise) + if c.AdvertiseAddrs.HTTP != "169.254.1.5:4646" { + t.Fatalf("expected HTTP advertise address 169.254.1.5:4646, got %s", c.AdvertiseAddrs.HTTP) } if c.AdvertiseAddrs.RPC != "169.254.1.40:4647" { diff --git a/command/agent/http.go b/command/agent/http.go index 0b656f959e59..b336f124fa14 100644 --- a/command/agent/http.go +++ b/command/agent/http.go @@ -76,88 +76,94 @@ type HTTPServer struct { } // NewHTTPServer starts new HTTP server over the agent -func NewHTTPServer(agent *Agent, config *Config) (*HTTPServer, error) { - // Start the listener - lnAddr, err := net.ResolveTCPAddr("tcp", config.normalizedAddrs.HTTP) - if err != nil { - return nil, err - } - ln, err := config.Listener("tcp", lnAddr.IP.String(), lnAddr.Port) - if err != nil { - return nil, fmt.Errorf("failed to start HTTP listener: %v", err) - } - - // If TLS is enabled, wrap the listener with a TLS listener - if config.TLSConfig.EnableHTTP { - tlsConf, err := tlsutil.NewTLSConfiguration(config.TLSConfig, config.TLSConfig.VerifyHTTPSClient, true) - if err != nil { - return nil, err - } - - tlsConfig, err := tlsConf.IncomingTLSConfig() - if err != nil { - return nil, err - } - ln = tls.NewListener(tcpKeepAliveListener{ln.(*net.TCPListener)}, tlsConfig) - } - - // Create the mux - mux := http.NewServeMux() - - wsUpgrader := &websocket.Upgrader{ - ReadBufferSize: 2048, - WriteBufferSize: 2048, - } +func NewHTTPServers(agent *Agent, config *Config) ([]HTTPServer, error) { + var srvs []HTTPServer - // Create the server - srv := &HTTPServer{ - agent: agent, - mux: mux, - listener: ln, - listenerCh: make(chan struct{}), - logger: agent.httpLogger, - Addr: ln.Addr().String(), - wsUpgrader: wsUpgrader, - } - srv.registerHandlers(config.EnableDebug) - - // Handle requests with gzip compression - gzip, err := gziphandler.GzipHandlerWithOpts(gziphandler.MinSize(0)) - if err != nil { - return nil, err - } - - // Get connection handshake timeout limit - handshakeTimeout, err := time.ParseDuration(config.Limits.HTTPSHandshakeTimeout) - if err != nil { - return nil, fmt.Errorf("error parsing https_handshake_timeout: %v", err) - } else if handshakeTimeout < 0 { - return nil, fmt.Errorf("https_handshake_timeout must be >= 0") - } - - // Get max connection limit - maxConns := 0 - if mc := config.Limits.HTTPMaxConnsPerClient; mc != nil { - maxConns = *mc - } - if maxConns < 0 { - return nil, fmt.Errorf("http_max_conns_per_client must be >= 0") - } - - // Create HTTP server with timeouts - httpServer := http.Server{ - Addr: srv.Addr, - Handler: gzip(mux), - ConnState: makeConnState(config.TLSConfig.EnableHTTP, handshakeTimeout, maxConns), - ErrorLog: newHTTPServerLogger(srv.logger), - } - - go func() { - defer close(srv.listenerCh) - httpServer.Serve(ln) - }() - - return srv, nil + // Start the listener + for _, addr := range config.normalizedAddrs.HTTP { + lnAddr, err := net.ResolveTCPAddr("tcp", addr) + if err != nil { + return nil, err + } + ln, err := config.Listener("tcp", lnAddr.IP.String(), lnAddr.Port) + if err != nil { + return nil, fmt.Errorf("failed to start HTTP listener: %v", err) + } + + // If TLS is enabled, wrap the listener with a TLS listener + if config.TLSConfig.EnableHTTP { + tlsConf, err := tlsutil.NewTLSConfiguration(config.TLSConfig, config.TLSConfig.VerifyHTTPSClient, true) + if err != nil { + return nil, err + } + + tlsConfig, err := tlsConf.IncomingTLSConfig() + if err != nil { + return nil, err + } + ln = tls.NewListener(tcpKeepAliveListener{ln.(*net.TCPListener)}, tlsConfig) + } + + // Create the mux + mux := http.NewServeMux() + + wsUpgrader := &websocket.Upgrader{ + ReadBufferSize: 2048, + WriteBufferSize: 2048, + } + + // Create the server + srv := &HTTPServer{ + agent: agent, + mux: mux, + listener: ln, + listenerCh: make(chan struct{}), + logger: agent.httpLogger, + Addr: ln.Addr().String(), + wsUpgrader: wsUpgrader, + } + srv.registerHandlers(config.EnableDebug) + + // Handle requests with gzip compression + gzip, err := gziphandler.GzipHandlerWithOpts(gziphandler.MinSize(0)) + if err != nil { + return nil, err + } + + // Get connection handshake timeout limit + handshakeTimeout, err := time.ParseDuration(config.Limits.HTTPSHandshakeTimeout) + if err != nil { + return nil, fmt.Errorf("error parsing https_handshake_timeout: %v", err) + } else if handshakeTimeout < 0 { + return nil, fmt.Errorf("https_handshake_timeout must be >= 0") + } + + // Get max connection limit + maxConns := 0 + if mc := config.Limits.HTTPMaxConnsPerClient; mc != nil { + maxConns = *mc + } + if maxConns < 0 { + return nil, fmt.Errorf("http_max_conns_per_client must be >= 0") + } + + // Create HTTP server with timeouts + httpServer := http.Server{ + Addr: srv.Addr, + Handler: gzip(mux), + ConnState: makeConnState(config.TLSConfig.EnableHTTP, handshakeTimeout, maxConns), + ErrorLog: newHTTPServerLogger(srv.logger), + } + + go func() { + defer close(srv.listenerCh) + httpServer.Serve(ln) + }() + + srvs = append(srvs, *srv) + } + + return srvs, nil } // makeConnState returns a ConnState func for use in an http.Server. If diff --git a/command/agent/testagent.go b/command/agent/testagent.go index 698a8efb00b8..c281f29349f5 100644 --- a/command/agent/testagent.go +++ b/command/agent/testagent.go @@ -255,12 +255,12 @@ func (a *TestAgent) start() (*Agent, error) { } // Setup the HTTP server - http, err := NewHTTPServer(agent, a.Config) + httpServers, err := NewHTTPServers(agent, a.Config) if err != nil { return agent, err } - - a.Server = http + // TODO: change type or handle better + a.Server = &httpServers[0] return agent, nil } From ea1481f79fe1b028770dfa05c7a6ee3ad8d4b869 Mon Sep 17 00:00:00 2001 From: Kevin Schoonover Date: Sun, 28 Nov 2021 17:23:29 -0700 Subject: [PATCH 02/14] support multiple addresses in 'address.http' --- command/agent/agent.go | 4 +- command/agent/command.go | 18 ++-- command/agent/config.go | 11 -- command/agent/config_test.go | 4 +- command/agent/http.go | 189 +++++++++++++++++++---------------- command/agent/testagent.go | 2 +- 6 files changed, 118 insertions(+), 110 deletions(-) diff --git a/command/agent/agent.go b/command/agent/agent.go index d2966620d3ab..093e5b6658e3 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -935,8 +935,8 @@ func (a *Agent) setupClient() error { // If no HTTP health check can be supported nil is returned. func (a *Agent) agentHTTPCheck(server bool) *structs.ServiceCheck { // Resolve the http check address - // TODO: evaluate if this is the best way to handle this default, maybe - // BindAddr? + // TODO: evaluate if this is the best way to handle this default, maybe + // BindAddr? httpCheckAddr := a.config.normalizedAddrs.HTTP[0] if *a.config.Consul.ChecksUseAdvertise { httpCheckAddr = a.config.AdvertiseAddrs.HTTP diff --git a/command/agent/command.go b/command/agent/command.go index c8bc0d454d54..8cfb64bb6737 100644 --- a/command/agent/command.go +++ b/command/agent/command.go @@ -51,7 +51,7 @@ type Command struct { args []string agent *Agent - httpServer *HTTPServer + httpServers []HTTPServer logFilter *logutils.LevelFilter logOutput io.Writer retryJoinErrCh chan struct{} @@ -506,8 +506,7 @@ func (c *Command) setupAgent(config *Config, logger hclog.InterceptLogger, logOu c.Ui.Error(fmt.Sprintf("Error starting http server: %s", err)) return err } - // TODO: update for multiple or change the type of NewHTTPServer - c.httpServer = &httpServers[0] + c.httpServers = httpServers // If DisableUpdateCheck is not enabled, set up update checking // (DisableUpdateCheck is false by default) @@ -701,8 +700,10 @@ func (c *Command) Run(args []string) int { // Shutdown the http server at the end, to ease debugging if // the agent takes long to shutdown - if c.httpServer != nil { - c.httpServer.Shutdown() + if len(c.httpServers) > 0 { + for _, srv := range c.httpServers { + srv.Shutdown() + } } }() @@ -903,14 +904,15 @@ WAIT: func (c *Command) reloadHTTPServer() error { c.agent.logger.Info("reloading HTTP server with new TLS configuration") - c.httpServer.Shutdown() + for _, srv := range c.httpServers { + srv.Shutdown() + } httpServers, err := NewHTTPServers(c.agent, c.agent.config) if err != nil { return err } - // TODO: change type or handle with list - c.httpServer = &httpServers[0] + c.httpServers = httpServers return nil } diff --git a/command/agent/config.go b/command/agent/config.go index dfe3242cca2b..4dcf7b7f3389 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -1261,13 +1261,6 @@ func (c *Config) normalizeAddrs() error { Serf: net.JoinHostPort(c.Addresses.Serf, strconv.Itoa(c.Ports.Serf)), } - // defaultHTTPAdvertiseAddr := c.BindAddr - // // Preserving the old behavior to defaulting to address.http field if only - // // 1 ip is specified - // if len(httpAddrs) == 1 { - // defaultHTTPAdvertiseAddr = httpAddrs[0] - // } - addr, err = normalizeAdvertise(c.AdvertiseAddrs.HTTP, c.BindAddr, c.Ports.HTTP, c.DevMode) if err != nil { return fmt.Errorf("Failed to parse HTTP advertise address (%v, %v, %v, %v): %v", c.AdvertiseAddrs.HTTP, c.Addresses.HTTP, c.Ports.HTTP, c.DevMode, err) @@ -1406,9 +1399,7 @@ func normalizeAdvertise(addr string, bind string, defport int, dev bool) (string return "", fmt.Errorf("Error parsing advertise address template: %v", err) } - fmt.Println("addr", addr) if addr != "" { - fmt.Println("test1") // Default to using manually configured address _, _, err = net.SplitHostPort(addr) if err != nil { @@ -1423,8 +1414,6 @@ func normalizeAdvertise(addr string, bind string, defport int, dev bool) (string return addr, nil } - fmt.Println("bind", bind) - fmt.Println("test2") // Fallback to bind address first, and then try resolving the local hostname ips, err := net.LookupIP(bind) if err != nil { diff --git a/command/agent/config_test.go b/command/agent/config_test.go index 1d88bbc33374..26f0067b3614 100644 --- a/command/agent/config_test.go +++ b/command/agent/config_test.go @@ -883,7 +883,7 @@ func TestConfig_normalizeAddrs_IPv6Loopback(t *testing.T) { // TestConfig_normalizeAddrs_MultipleInterface asserts that normalizeAddrs will // handle normalizing multiple interfaces in a single protocol. func TestConfig_normalizeAddrs_MultipleInterfaces(t *testing.T) { - // TODO: add test with go-sockaddr templates + // TODO: add test with go-sockaddr templates testCases := []struct { name string addressConfig *Addresses @@ -957,7 +957,7 @@ func TestConfig_normalizeAddrs(t *testing.T) { t.Fatalf("expected BindAddr 169.254.1.5, got %s", c.BindAddr) } - // was this test case incorrect? should default to bind addr if not specified (https://www.nomadproject.io/docs/configuration#advertise) + // was this test case incorrect? should default to bind addr if not specified (https://www.nomadproject.io/docs/configuration#advertise) if c.AdvertiseAddrs.HTTP != "169.254.1.5:4646" { t.Fatalf("expected HTTP advertise address 169.254.1.5:4646, got %s", c.AdvertiseAddrs.HTTP) } diff --git a/command/agent/http.go b/command/agent/http.go index b336f124fa14..6b7f4401c03e 100644 --- a/command/agent/http.go +++ b/command/agent/http.go @@ -20,6 +20,7 @@ import ( "github.com/hashicorp/go-connlimit" log "github.com/hashicorp/go-hclog" "github.com/hashicorp/go-msgpack/codec" + multierror "github.com/hashicorp/go-multierror" "github.com/rs/cors" "github.com/hashicorp/nomad/helper/noxssrw" @@ -75,95 +76,111 @@ type HTTPServer struct { wsUpgrader *websocket.Upgrader } -// NewHTTPServer starts new HTTP server over the agent +// NewHTTPServers starts an HTTP server for every address.http configured in +// the agent. func NewHTTPServers(agent *Agent, config *Config) ([]HTTPServer, error) { - var srvs []HTTPServer + var srvs []HTTPServer + var error error // Start the listener - for _, addr := range config.normalizedAddrs.HTTP { - lnAddr, err := net.ResolveTCPAddr("tcp", addr) - if err != nil { - return nil, err - } - ln, err := config.Listener("tcp", lnAddr.IP.String(), lnAddr.Port) - if err != nil { - return nil, fmt.Errorf("failed to start HTTP listener: %v", err) - } - - // If TLS is enabled, wrap the listener with a TLS listener - if config.TLSConfig.EnableHTTP { - tlsConf, err := tlsutil.NewTLSConfiguration(config.TLSConfig, config.TLSConfig.VerifyHTTPSClient, true) - if err != nil { - return nil, err - } - - tlsConfig, err := tlsConf.IncomingTLSConfig() - if err != nil { - return nil, err - } - ln = tls.NewListener(tcpKeepAliveListener{ln.(*net.TCPListener)}, tlsConfig) - } - - // Create the mux - mux := http.NewServeMux() - - wsUpgrader := &websocket.Upgrader{ - ReadBufferSize: 2048, - WriteBufferSize: 2048, - } - - // Create the server - srv := &HTTPServer{ - agent: agent, - mux: mux, - listener: ln, - listenerCh: make(chan struct{}), - logger: agent.httpLogger, - Addr: ln.Addr().String(), - wsUpgrader: wsUpgrader, - } - srv.registerHandlers(config.EnableDebug) - - // Handle requests with gzip compression - gzip, err := gziphandler.GzipHandlerWithOpts(gziphandler.MinSize(0)) - if err != nil { - return nil, err - } - - // Get connection handshake timeout limit - handshakeTimeout, err := time.ParseDuration(config.Limits.HTTPSHandshakeTimeout) - if err != nil { - return nil, fmt.Errorf("error parsing https_handshake_timeout: %v", err) - } else if handshakeTimeout < 0 { - return nil, fmt.Errorf("https_handshake_timeout must be >= 0") - } - - // Get max connection limit - maxConns := 0 - if mc := config.Limits.HTTPMaxConnsPerClient; mc != nil { - maxConns = *mc - } - if maxConns < 0 { - return nil, fmt.Errorf("http_max_conns_per_client must be >= 0") - } - - // Create HTTP server with timeouts - httpServer := http.Server{ - Addr: srv.Addr, - Handler: gzip(mux), - ConnState: makeConnState(config.TLSConfig.EnableHTTP, handshakeTimeout, maxConns), - ErrorLog: newHTTPServerLogger(srv.logger), - } - - go func() { - defer close(srv.listenerCh) - httpServer.Serve(ln) - }() - - srvs = append(srvs, *srv) - } - - return srvs, nil + for _, addr := range config.normalizedAddrs.HTTP { + lnAddr, err := net.ResolveTCPAddr("tcp", addr) + if err != nil { + multierror.Append(error, err) + continue + } + ln, err := config.Listener("tcp", lnAddr.IP.String(), lnAddr.Port) + if err != nil { + error = multierror.Append(error, fmt.Errorf("failed to start HTTP listener: %v", err)) + continue + } + + // If TLS is enabled, wrap the listener with a TLS listener + if config.TLSConfig.EnableHTTP { + tlsConf, err := tlsutil.NewTLSConfiguration(config.TLSConfig, config.TLSConfig.VerifyHTTPSClient, true) + if err != nil { + error = multierror.Append(error, err) + continue + } + + tlsConfig, err := tlsConf.IncomingTLSConfig() + if err != nil { + error = multierror.Append(error, err) + continue + } + ln = tls.NewListener(tcpKeepAliveListener{ln.(*net.TCPListener)}, tlsConfig) + } + + // Create the mux + mux := http.NewServeMux() + + wsUpgrader := &websocket.Upgrader{ + ReadBufferSize: 2048, + WriteBufferSize: 2048, + } + + // Create the server + srv := &HTTPServer{ + agent: agent, + mux: mux, + listener: ln, + listenerCh: make(chan struct{}), + logger: agent.httpLogger, + Addr: ln.Addr().String(), + wsUpgrader: wsUpgrader, + } + srv.registerHandlers(config.EnableDebug) + + // Handle requests with gzip compression + gzip, err := gziphandler.GzipHandlerWithOpts(gziphandler.MinSize(0)) + if err != nil { + error = multierror.Append(error, err) + continue + } + + // Get connection handshake timeout limit + handshakeTimeout, err := time.ParseDuration(config.Limits.HTTPSHandshakeTimeout) + if err != nil { + error = multierror.Append(error, fmt.Errorf("error parsing https_handshake_timeout: %v", err)) + continue + } else if handshakeTimeout < 0 { + error = multierror.Append(error, fmt.Errorf("https_handshake_timeout must be >= 0")) + continue + } + + // Get max connection limit + maxConns := 0 + if mc := config.Limits.HTTPMaxConnsPerClient; mc != nil { + maxConns = *mc + } + if maxConns < 0 { + error = multierror.Append(error, fmt.Errorf("http_max_conns_per_client must be >= 0")) + continue + } + + // Create HTTP server with timeouts + httpServer := http.Server{ + Addr: srv.Addr, + Handler: gzip(mux), + ConnState: makeConnState(config.TLSConfig.EnableHTTP, handshakeTimeout, maxConns), + ErrorLog: newHTTPServerLogger(srv.logger), + } + + go func() { + defer close(srv.listenerCh) + httpServer.Serve(ln) + }() + + srvs = append(srvs, *srv) + } + + if error != nil { + for _, srv := range srvs { + srv.Shutdown() + } + } + + return srvs, error } // makeConnState returns a ConnState func for use in an http.Server. If diff --git a/command/agent/testagent.go b/command/agent/testagent.go index c281f29349f5..8174eba70b16 100644 --- a/command/agent/testagent.go +++ b/command/agent/testagent.go @@ -259,7 +259,7 @@ func (a *TestAgent) start() (*Agent, error) { if err != nil { return agent, err } - // TODO: change type or handle better + // TODO: change type or handle better a.Server = &httpServers[0] return agent, nil } From cc2f0d1678113a6a9b1826242afae41caec0b9b4 Mon Sep 17 00:00:00 2001 From: Kevin Schoonover Date: Sun, 28 Nov 2021 18:02:04 -0700 Subject: [PATCH 03/14] fix defaulting to address --- command/agent/agent_test.go | 6 +++--- command/agent/config.go | 4 ++-- command/agent/config_test.go | 3 +-- command/agent/http_test.go | 6 +++--- 4 files changed, 9 insertions(+), 10 deletions(-) diff --git a/command/agent/agent_test.go b/command/agent/agent_test.go index f1a834cd4edb..9fb04e62652a 100644 --- a/command/agent/agent_test.go +++ b/command/agent/agent_test.go @@ -105,7 +105,7 @@ func TestAgent_ServerConfig(t *testing.T) { require.Equal(t, "127.0.0.2", conf.Addresses.HTTP) require.Equal(t, "127.0.0.2", conf.Addresses.RPC) require.Equal(t, "127.0.0.2", conf.Addresses.Serf) - require.Equal(t, "127.0.0.2:4646", conf.normalizedAddrs.HTTP) + require.Equal(t, []string{"127.0.0.2:4646"}, conf.normalizedAddrs.HTTP) require.Equal(t, "127.0.0.2:4003", conf.normalizedAddrs.RPC) require.Equal(t, "127.0.0.2:4004", conf.normalizedAddrs.Serf) require.Equal(t, "10.0.0.10:4646", conf.AdvertiseAddrs.HTTP) @@ -563,7 +563,7 @@ func TestAgent_HTTPCheck(t *testing.T) { logger: logger, config: &Config{ AdvertiseAddrs: &AdvertiseAddrs{HTTP: "advertise:4646"}, - normalizedAddrs: &Addresses{HTTP: "normalized:4646"}, + normalizedAddrs: &NormalizedAddrs{HTTP: []string{"normalized:4646"}}, Consul: &config.ConsulConfig{ ChecksUseAdvertise: helper.BoolToPtr(false), }, @@ -587,7 +587,7 @@ func TestAgent_HTTPCheck(t *testing.T) { if check.Protocol != "http" { t.Errorf("expected http proto not: %q", check.Protocol) } - if expected := a.config.normalizedAddrs.HTTP; check.PortLabel != expected { + if expected := a.config.normalizedAddrs.HTTP[0]; check.PortLabel != expected { t.Errorf("expected normalized addr not %q", check.PortLabel) } }) diff --git a/command/agent/config.go b/command/agent/config.go index 4dcf7b7f3389..fe142d0c7234 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -1261,7 +1261,7 @@ func (c *Config) normalizeAddrs() error { Serf: net.JoinHostPort(c.Addresses.Serf, strconv.Itoa(c.Ports.Serf)), } - addr, err = normalizeAdvertise(c.AdvertiseAddrs.HTTP, c.BindAddr, c.Ports.HTTP, c.DevMode) + addr, err = normalizeAdvertise(c.AdvertiseAddrs.HTTP, httpAddrs[0], c.Ports.HTTP, c.DevMode) if err != nil { return fmt.Errorf("Failed to parse HTTP advertise address (%v, %v, %v, %v): %v", c.AdvertiseAddrs.HTTP, c.Addresses.HTTP, c.Ports.HTTP, c.DevMode, err) } @@ -2034,7 +2034,7 @@ func LoadConfigDir(dir string) (*Config, error) { // joinHostPorts joins every addr in addrs with the specified port func joinHostPorts(addrs []string, port string) []string { - localAddrs := addrs + localAddrs := make([]string, len(addrs)) for i, k := range addrs { localAddrs[i] = net.JoinHostPort(k, port) diff --git a/command/agent/config_test.go b/command/agent/config_test.go index 26f0067b3614..38ebb04dbdda 100644 --- a/command/agent/config_test.go +++ b/command/agent/config_test.go @@ -957,8 +957,7 @@ func TestConfig_normalizeAddrs(t *testing.T) { t.Fatalf("expected BindAddr 169.254.1.5, got %s", c.BindAddr) } - // was this test case incorrect? should default to bind addr if not specified (https://www.nomadproject.io/docs/configuration#advertise) - if c.AdvertiseAddrs.HTTP != "169.254.1.5:4646" { + if c.AdvertiseAddrs.HTTP != "169.254.1.10:4646" { t.Fatalf("expected HTTP advertise address 169.254.1.5:4646, got %s", c.AdvertiseAddrs.HTTP) } diff --git a/command/agent/http_test.go b/command/agent/http_test.go index 6083779d2a41..e924ed9a82d8 100644 --- a/command/agent/http_test.go +++ b/command/agent/http_test.go @@ -945,8 +945,8 @@ func TestHTTPServer_Limits_Error(t *testing.T) { t.Parallel() conf := &Config{ - normalizedAddrs: &Addresses{ - HTTP: "localhost:0", // port is never used + normalizedAddrs: &NormalizedAddrs{ + HTTP: []string{"localhost:0"}, // port is never used }, TLSConfig: &config.TLSConfig{ EnableHTTP: tc.tls, @@ -964,7 +964,7 @@ func TestHTTPServer_Limits_Error(t *testing.T) { config: conf, } - srv, err := NewHTTPServer(agent, conf) + srv, err := NewHTTPServers(agent, conf) require.Error(t, err) require.Nil(t, srv) require.Contains(t, err.Error(), tc.expectedErr) From f04f6f1898cf3f851d8f2bc0dbb0d30e2bc0848e Mon Sep 17 00:00:00 2001 From: Kevin Schoonover Date: Sun, 28 Nov 2021 18:13:03 -0700 Subject: [PATCH 04/14] fix tests --- command/agent/agent_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command/agent/agent_test.go b/command/agent/agent_test.go index 9fb04e62652a..f61e47c049c4 100644 --- a/command/agent/agent_test.go +++ b/command/agent/agent_test.go @@ -166,7 +166,7 @@ func TestAgent_ServerConfig(t *testing.T) { require.Equal(t, "127.0.0.3", conf.Addresses.HTTP) require.Equal(t, "127.0.0.3", conf.Addresses.RPC) require.Equal(t, "127.0.0.3", conf.Addresses.Serf) - require.Equal(t, "127.0.0.3:4646", conf.normalizedAddrs.HTTP) + require.Equal(t, []string{"127.0.0.3:4646"}, conf.normalizedAddrs.HTTP) require.Equal(t, "127.0.0.3:4647", conf.normalizedAddrs.RPC) require.Equal(t, "127.0.0.3:4648", conf.normalizedAddrs.Serf) From b35a04e68535000f9cd3fb6a27d5f3bc39ba2ae8 Mon Sep 17 00:00:00 2001 From: Kevin Schoonover Date: Fri, 3 Dec 2021 19:29:10 -0800 Subject: [PATCH 05/14] test nomad http server binds to multiple interfaces --- command/agent/http_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/command/agent/http_test.go b/command/agent/http_test.go index e924ed9a82d8..3551cea95d83 100644 --- a/command/agent/http_test.go +++ b/command/agent/http_test.go @@ -69,6 +69,24 @@ func BenchmarkHTTPRequests(b *testing.B) { }) } +func TestMultipleInterfaces(t *testing.T) { + httpIps := []string{ "127.0.0.1", "127.0.0.2"} + + s := makeHTTPServer(t, func(c *Config) { + c.Addresses.HTTP = strings.Join(httpIps, " ") + c.ACL.Enabled = true + }) + defer s.Shutdown() + + httpPort := s.ports[0] + for _, ip := range httpIps { + resp, err := http.Get(fmt.Sprintf("http://%s:%d/", ip, httpPort)) + + assert.Nil(t, err) + assert.Equal(t, resp.StatusCode, 200) + } +} + // TestRootFallthrough tests rootFallthrough handler to // verify redirect and 404 behavior func TestRootFallthrough(t *testing.T) { From 6a9002e7a5323f05d18170fe2a19e2723a722c2c Mon Sep 17 00:00:00 2001 From: Kevin Schoonover Date: Fri, 3 Dec 2021 20:01:48 -0800 Subject: [PATCH 06/14] prevent server leak if multiple http servers are created --- command/agent/http_test.go | 18 +++++++++--------- command/agent/testagent.go | 12 ++++++++++-- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/command/agent/http_test.go b/command/agent/http_test.go index 3551cea95d83..596c3a58c7b6 100644 --- a/command/agent/http_test.go +++ b/command/agent/http_test.go @@ -70,21 +70,21 @@ func BenchmarkHTTPRequests(b *testing.B) { } func TestMultipleInterfaces(t *testing.T) { - httpIps := []string{ "127.0.0.1", "127.0.0.2"} + httpIps := []string{"127.0.0.1", "127.0.0.2"} - s := makeHTTPServer(t, func(c *Config) { - c.Addresses.HTTP = strings.Join(httpIps, " ") + s := makeHTTPServer(t, func(c *Config) { + c.Addresses.HTTP = strings.Join(httpIps, " ") c.ACL.Enabled = true }) defer s.Shutdown() - httpPort := s.ports[0] - for _, ip := range httpIps { - resp, err := http.Get(fmt.Sprintf("http://%s:%d/", ip, httpPort)) + httpPort := s.ports[0] + for _, ip := range httpIps { + resp, err := http.Get(fmt.Sprintf("http://%s:%d/", ip, httpPort)) - assert.Nil(t, err) - assert.Equal(t, resp.StatusCode, 200) - } + assert.Nil(t, err) + assert.Equal(t, resp.StatusCode, 200) + } } // TestRootFallthrough tests rootFallthrough handler to diff --git a/command/agent/testagent.go b/command/agent/testagent.go index 8174eba70b16..a706e5edde9c 100644 --- a/command/agent/testagent.go +++ b/command/agent/testagent.go @@ -66,7 +66,11 @@ type TestAgent struct { // Key is the optional encryption key for the keyring. Key string - // Server is a reference to the started HTTP endpoint. + // All HTTP servers started. Used to prevent server leaks and preserve + // backwards compability. + Servers []HTTPServer + + // Server is a reference to the primary, started HTTP endpoint. // It is valid after Start(). Server *HTTPServer @@ -260,6 +264,7 @@ func (a *TestAgent) start() (*Agent, error) { return agent, err } // TODO: change type or handle better + a.Servers = httpServers a.Server = &httpServers[0] return agent, nil } @@ -284,7 +289,10 @@ func (a *TestAgent) Shutdown() error { ch := make(chan error, 1) go func() { defer close(ch) - a.Server.Shutdown() + for _, srv := range a.Servers { + srv.Shutdown() + } + ch <- a.Agent.Shutdown() }() From 24fe02972f19dac0a2bc6945a5a878b383411f0e Mon Sep 17 00:00:00 2001 From: Kevin Schoonover Date: Wed, 8 Dec 2021 18:46:03 -0800 Subject: [PATCH 07/14] address some comments --- command/agent/config_test.go | 2 +- command/agent/http.go | 91 +++++++++++++++++------------------- 2 files changed, 44 insertions(+), 49 deletions(-) diff --git a/command/agent/config_test.go b/command/agent/config_test.go index 38ebb04dbdda..425314981e60 100644 --- a/command/agent/config_test.go +++ b/command/agent/config_test.go @@ -958,7 +958,7 @@ func TestConfig_normalizeAddrs(t *testing.T) { } if c.AdvertiseAddrs.HTTP != "169.254.1.10:4646" { - t.Fatalf("expected HTTP advertise address 169.254.1.5:4646, got %s", c.AdvertiseAddrs.HTTP) + t.Fatalf("expected HTTP advertise address 169.254.1.10:4646, got %s", c.AdvertiseAddrs.HTTP) } if c.AdvertiseAddrs.RPC != "169.254.1.40:4647" { diff --git a/command/agent/http.go b/command/agent/http.go index 6b7f4401c03e..f2c2d33cafaa 100644 --- a/command/agent/http.go +++ b/command/agent/http.go @@ -80,45 +80,67 @@ type HTTPServer struct { // the agent. func NewHTTPServers(agent *Agent, config *Config) ([]HTTPServer, error) { var srvs []HTTPServer - var error error + var serverInitializationErrors error + // Handle requests with gzip compression + gzip, err := gziphandler.GzipHandlerWithOpts(gziphandler.MinSize(0)) + if err != nil { + return srvs, err + } + + // Get connection handshake timeout limit + handshakeTimeout, err := time.ParseDuration(config.Limits.HTTPSHandshakeTimeout) + if err != nil { + return srvs, fmt.Errorf("error parsing https_handshake_timeout: %v", err) + } else if handshakeTimeout < 0 { + return srvs, fmt.Errorf("https_handshake_timeout must be >= 0") + } + + // Get max connection limit + maxConns := 0 + if mc := config.Limits.HTTPMaxConnsPerClient; mc != nil { + maxConns = *mc + } + if maxConns < 0 { + return srvs, fmt.Errorf("http_max_conns_per_client must be >= 0") + } + + tlsConf, err := tlsutil.NewTLSConfiguration(config.TLSConfig, config.TLSConfig.VerifyHTTPSClient, true) + if err != nil && config.TLSConfig.EnableHTTP { + return srvs, fmt.Errorf("failed to initialize HTTP server TLS configuration: %s", err) + } + + wsUpgrader := &websocket.Upgrader{ + ReadBufferSize: 2048, + WriteBufferSize: 2048, + } + + // Create the mux + mux := http.NewServeMux() // Start the listener for _, addr := range config.normalizedAddrs.HTTP { + lnAddr, err := net.ResolveTCPAddr("tcp", addr) if err != nil { - multierror.Append(error, err) + serverInitializationErrors = multierror.Append(serverInitializationErrors, err) continue } ln, err := config.Listener("tcp", lnAddr.IP.String(), lnAddr.Port) if err != nil { - error = multierror.Append(error, fmt.Errorf("failed to start HTTP listener: %v", err)) + serverInitializationErrors = multierror.Append(serverInitializationErrors, fmt.Errorf("failed to start HTTP listener: %v", err)) continue } // If TLS is enabled, wrap the listener with a TLS listener if config.TLSConfig.EnableHTTP { - tlsConf, err := tlsutil.NewTLSConfiguration(config.TLSConfig, config.TLSConfig.VerifyHTTPSClient, true) - if err != nil { - error = multierror.Append(error, err) - continue - } - tlsConfig, err := tlsConf.IncomingTLSConfig() if err != nil { - error = multierror.Append(error, err) + serverInitializationErrors = multierror.Append(serverInitializationErrors, err) continue } ln = tls.NewListener(tcpKeepAliveListener{ln.(*net.TCPListener)}, tlsConfig) } - // Create the mux - mux := http.NewServeMux() - - wsUpgrader := &websocket.Upgrader{ - ReadBufferSize: 2048, - WriteBufferSize: 2048, - } - // Create the server srv := &HTTPServer{ agent: agent, @@ -131,33 +153,6 @@ func NewHTTPServers(agent *Agent, config *Config) ([]HTTPServer, error) { } srv.registerHandlers(config.EnableDebug) - // Handle requests with gzip compression - gzip, err := gziphandler.GzipHandlerWithOpts(gziphandler.MinSize(0)) - if err != nil { - error = multierror.Append(error, err) - continue - } - - // Get connection handshake timeout limit - handshakeTimeout, err := time.ParseDuration(config.Limits.HTTPSHandshakeTimeout) - if err != nil { - error = multierror.Append(error, fmt.Errorf("error parsing https_handshake_timeout: %v", err)) - continue - } else if handshakeTimeout < 0 { - error = multierror.Append(error, fmt.Errorf("https_handshake_timeout must be >= 0")) - continue - } - - // Get max connection limit - maxConns := 0 - if mc := config.Limits.HTTPMaxConnsPerClient; mc != nil { - maxConns = *mc - } - if maxConns < 0 { - error = multierror.Append(error, fmt.Errorf("http_max_conns_per_client must be >= 0")) - continue - } - // Create HTTP server with timeouts httpServer := http.Server{ Addr: srv.Addr, @@ -174,13 +169,13 @@ func NewHTTPServers(agent *Agent, config *Config) ([]HTTPServer, error) { srvs = append(srvs, *srv) } - if error != nil { + if serverInitializationErrors != nil { for _, srv := range srvs { srv.Shutdown() } } - return srvs, error + return srvs, serverInitializationErrors } // makeConnState returns a ConnState func for use in an http.Server. If @@ -272,7 +267,7 @@ func (s *HTTPServer) Shutdown() { } // registerHandlers is used to attach our handlers to the mux -func (s *HTTPServer) registerHandlers(enableDebug bool) { +func (s HTTPServer) registerHandlers(enableDebug bool) { s.mux.HandleFunc("/v1/jobs", s.wrap(s.JobsRequest)) s.mux.HandleFunc("/v1/jobs/parse", s.wrap(s.JobsParseRequest)) s.mux.HandleFunc("/v1/job/", s.wrap(s.JobSpecificRequest)) From 290b5f4199bb144dfbed9a08afb65f30fe6ac4e4 Mon Sep 17 00:00:00 2001 From: Kevin Schoonover Date: Thu, 9 Dec 2021 20:53:24 -0800 Subject: [PATCH 08/14] initialize mux in loop --- command/agent/http.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/command/agent/http.go b/command/agent/http.go index f2c2d33cafaa..7b39d3e7825b 100644 --- a/command/agent/http.go +++ b/command/agent/http.go @@ -115,10 +115,10 @@ func NewHTTPServers(agent *Agent, config *Config) ([]HTTPServer, error) { WriteBufferSize: 2048, } - // Create the mux - mux := http.NewServeMux() // Start the listener for _, addr := range config.normalizedAddrs.HTTP { + // Create the mux + mux := http.NewServeMux() lnAddr, err := net.ResolveTCPAddr("tcp", addr) if err != nil { From 3007e0ade64b6c02f54010562e662e0213a93ac0 Mon Sep 17 00:00:00 2001 From: Kevin Schoonover Date: Thu, 9 Dec 2021 21:21:28 -0800 Subject: [PATCH 09/14] update documentation --- website/content/docs/configuration/consul.mdx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/website/content/docs/configuration/consul.mdx b/website/content/docs/configuration/consul.mdx index 80a2dc3de41a..4c4c8161a0fd 100644 --- a/website/content/docs/configuration/consul.mdx +++ b/website/content/docs/configuration/consul.mdx @@ -68,7 +68,11 @@ configuring Nomad to talk to Consul via DNS such as consul.service.consul Consul communication. If this is set then you need to also set `key_file`. - `checks_use_advertise` `(bool: false)` - Specifies if Consul health checks - should bind to the advertise address. By default, this is the bind address. + should bind to the advertise address. By default, this is the first [HTTP + address](https://www.nomadproject.io/docs/configuration#http). If no + [HTTP address](https://www.nomadproject.io/docs/configuration#http) is + specified, it will fall back to the + [bind_addr](https://www.nomadproject.io/docs/configuration#bind_addr) - `client_auto_join` `(bool: true)` - Specifies if the Nomad clients should automatically discover servers in the same region by searching for the Consul From 4aa63073115b2612dd8f3e856308eedcf36b5301 Mon Sep 17 00:00:00 2001 From: Kevin Schoonover Date: Thu, 9 Dec 2021 21:33:18 -0800 Subject: [PATCH 10/14] deduplicate ips returned from parseMultipleIPTemplates --- command/agent/agent.go | 2 -- command/agent/config.go | 15 ++++++++++++++- command/agent/config_test.go | 23 +++++++++++++++++++++++ 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/command/agent/agent.go b/command/agent/agent.go index 093e5b6658e3..c608e1d8ba20 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -935,8 +935,6 @@ func (a *Agent) setupClient() error { // If no HTTP health check can be supported nil is returned. func (a *Agent) agentHTTPCheck(server bool) *structs.ServiceCheck { // Resolve the http check address - // TODO: evaluate if this is the best way to handle this default, maybe - // BindAddr? httpCheckAddr := a.config.normalizedAddrs.HTTP[0] if *a.config.Consul.ChecksUseAdvertise { httpCheckAddr = a.config.AdvertiseAddrs.HTTP diff --git a/command/agent/config.go b/command/agent/config.go index fe142d0c7234..30c33236ced5 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -1358,7 +1358,7 @@ func parseMultipleIPTemplate(ipTmpl string) ([]string, error) { return []string{}, errors.New("No addresses found, please configure one.") } - return ips, nil + return deduplicateAddrs(ips), nil } // normalizeBind returns a normalized bind address. @@ -2051,3 +2051,16 @@ func isTemporaryFile(name string) bool { strings.HasPrefix(name, ".#") || // emacs (strings.HasPrefix(name, "#") && strings.HasSuffix(name, "#")) // emacs } + +func deduplicateAddrs(addrs []string) []string { + keys := make(map[string]bool) + list := []string{} + + for _, entry := range addrs { + if _, value := keys[entry]; !value { + keys[entry] = true + list = append(list, entry) + } + } + return list +} diff --git a/command/agent/config_test.go b/command/agent/config_test.go index 425314981e60..c0d3c629a093 100644 --- a/command/agent/config_test.go +++ b/command/agent/config_test.go @@ -1365,3 +1365,26 @@ func TestEventBroker_Parse(t *testing.T) { require.Equal(20000, *result.EventBufferSize) } } + +func TestParseMultipleIPTemplates(t *testing.T) { + testCases := []struct { + name string + tmpl string + expectedOut []string + expectErr bool + }{ + { + name: "deduplicates same ip", + tmpl: "127.0.0.1 127.0.0.1", + expectedOut: []string{"127.0.0.1"}, + expectErr: false, + }, + } + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + out, err := parseMultipleIPTemplate(tc.tmpl) + require.NoError(t, err) + require.Equal(t, tc.expectedOut, out) + }) + } +} From 760b0be0c3e9d885eb9d47248bef845b0b3b3ad8 Mon Sep 17 00:00:00 2001 From: Kevin Schoonover Date: Thu, 9 Dec 2021 21:35:21 -0800 Subject: [PATCH 11/14] missing period --- website/content/docs/configuration/consul.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/content/docs/configuration/consul.mdx b/website/content/docs/configuration/consul.mdx index 4c4c8161a0fd..a53cf6d89bb5 100644 --- a/website/content/docs/configuration/consul.mdx +++ b/website/content/docs/configuration/consul.mdx @@ -72,7 +72,7 @@ configuring Nomad to talk to Consul via DNS such as consul.service.consul address](https://www.nomadproject.io/docs/configuration#http). If no [HTTP address](https://www.nomadproject.io/docs/configuration#http) is specified, it will fall back to the - [bind_addr](https://www.nomadproject.io/docs/configuration#bind_addr) + [bind_addr](https://www.nomadproject.io/docs/configuration#bind_addr). - `client_auto_join` `(bool: true)` - Specifies if the Nomad clients should automatically discover servers in the same region by searching for the Consul From 013092b75a4256ed87292bf56b822a1418092330 Mon Sep 17 00:00:00 2001 From: Kevin Schoonover Date: Mon, 13 Dec 2021 17:12:24 -0800 Subject: [PATCH 12/14] go fmt --- command/agent/config.go | 20 ++++++++++---------- command/agent/config_test.go | 20 ++++++++++---------- command/agent/http.go | 4 ++-- 3 files changed, 22 insertions(+), 22 deletions(-) diff --git a/command/agent/config.go b/command/agent/config.go index 30c33236ced5..a87c4deeb810 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -2053,14 +2053,14 @@ func isTemporaryFile(name string) bool { } func deduplicateAddrs(addrs []string) []string { - keys := make(map[string]bool) - list := []string{} - - for _, entry := range addrs { - if _, value := keys[entry]; !value { - keys[entry] = true - list = append(list, entry) - } - } - return list + keys := make(map[string]bool) + list := []string{} + + for _, entry := range addrs { + if _, value := keys[entry]; !value { + keys[entry] = true + list = append(list, entry) + } + } + return list } diff --git a/command/agent/config_test.go b/command/agent/config_test.go index c0d3c629a093..a8415008d3fb 100644 --- a/command/agent/config_test.go +++ b/command/agent/config_test.go @@ -1367,24 +1367,24 @@ func TestEventBroker_Parse(t *testing.T) { } func TestParseMultipleIPTemplates(t *testing.T) { - testCases := []struct { - name string - tmpl string + testCases := []struct { + name string + tmpl string expectedOut []string - expectErr bool + expectErr bool }{ { - name: "deduplicates same ip", - tmpl: "127.0.0.1 127.0.0.1", + name: "deduplicates same ip", + tmpl: "127.0.0.1 127.0.0.1", expectedOut: []string{"127.0.0.1"}, - expectErr: false, + expectErr: false, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - out, err := parseMultipleIPTemplate(tc.tmpl) - require.NoError(t, err) - require.Equal(t, tc.expectedOut, out) + out, err := parseMultipleIPTemplate(tc.tmpl) + require.NoError(t, err) + require.Equal(t, tc.expectedOut, out) }) } } diff --git a/command/agent/http.go b/command/agent/http.go index 7b39d3e7825b..020138972794 100644 --- a/command/agent/http.go +++ b/command/agent/http.go @@ -117,8 +117,8 @@ func NewHTTPServers(agent *Agent, config *Config) ([]HTTPServer, error) { // Start the listener for _, addr := range config.normalizedAddrs.HTTP { - // Create the mux - mux := http.NewServeMux() + // Create the mux + mux := http.NewServeMux() lnAddr, err := net.ResolveTCPAddr("tcp", addr) if err != nil { From 738ea8f6d4fb241ab39d48ae359abfdecf38c34b Mon Sep 17 00:00:00 2001 From: Kevin Schoonover Date: Wed, 22 Dec 2021 20:31:45 -0800 Subject: [PATCH 13/14] update comments --- command/agent/config.go | 1 - command/agent/config_test.go | 1 - command/agent/testagent.go | 4 +++- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/command/agent/config.go b/command/agent/config.go index a87c4deeb810..48573396c1f2 100644 --- a/command/agent/config.go +++ b/command/agent/config.go @@ -1352,7 +1352,6 @@ func parseMultipleIPTemplate(ipTmpl string) ([]string, error) { return []string{}, fmt.Errorf("Unable to parse address template %q: %v", ipTmpl, err) } - // TODO: deduplicate the parsed ips ips := strings.Split(out, " ") if len(ips) == 0 { return []string{}, errors.New("No addresses found, please configure one.") diff --git a/command/agent/config_test.go b/command/agent/config_test.go index a8415008d3fb..9dc7f9b8cab4 100644 --- a/command/agent/config_test.go +++ b/command/agent/config_test.go @@ -883,7 +883,6 @@ func TestConfig_normalizeAddrs_IPv6Loopback(t *testing.T) { // TestConfig_normalizeAddrs_MultipleInterface asserts that normalizeAddrs will // handle normalizing multiple interfaces in a single protocol. func TestConfig_normalizeAddrs_MultipleInterfaces(t *testing.T) { - // TODO: add test with go-sockaddr templates testCases := []struct { name string addressConfig *Addresses diff --git a/command/agent/testagent.go b/command/agent/testagent.go index a706e5edde9c..1fd5811ba198 100644 --- a/command/agent/testagent.go +++ b/command/agent/testagent.go @@ -263,7 +263,9 @@ func (a *TestAgent) start() (*Agent, error) { if err != nil { return agent, err } - // TODO: change type or handle better + + // TODO: investigate if there is a way to remove the requirement by updating test. + // Initial pass at implementing this is https://github.com/kevinschoonover/nomad/tree/tests. a.Servers = httpServers a.Server = &httpServers[0] return agent, nil From 3e5169f9e0f241ee7b73d04c2501293d1cd90b68 Mon Sep 17 00:00:00 2001 From: Kevin Schoonover Date: Thu, 23 Dec 2021 22:10:47 -0800 Subject: [PATCH 14/14] []HttpServer -> []*HTTPServer --- command/agent/command.go | 2 +- command/agent/http.go | 6 +++--- command/agent/testagent.go | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/command/agent/command.go b/command/agent/command.go index 8cfb64bb6737..0cdfc76aab82 100644 --- a/command/agent/command.go +++ b/command/agent/command.go @@ -51,7 +51,7 @@ type Command struct { args []string agent *Agent - httpServers []HTTPServer + httpServers []*HTTPServer logFilter *logutils.LevelFilter logOutput io.Writer retryJoinErrCh chan struct{} diff --git a/command/agent/http.go b/command/agent/http.go index 020138972794..142f3a6ef90c 100644 --- a/command/agent/http.go +++ b/command/agent/http.go @@ -78,8 +78,8 @@ type HTTPServer struct { // NewHTTPServers starts an HTTP server for every address.http configured in // the agent. -func NewHTTPServers(agent *Agent, config *Config) ([]HTTPServer, error) { - var srvs []HTTPServer +func NewHTTPServers(agent *Agent, config *Config) ([]*HTTPServer, error) { + var srvs []*HTTPServer var serverInitializationErrors error // Handle requests with gzip compression @@ -166,7 +166,7 @@ func NewHTTPServers(agent *Agent, config *Config) ([]HTTPServer, error) { httpServer.Serve(ln) }() - srvs = append(srvs, *srv) + srvs = append(srvs, srv) } if serverInitializationErrors != nil { diff --git a/command/agent/testagent.go b/command/agent/testagent.go index 1fd5811ba198..18c463213ae6 100644 --- a/command/agent/testagent.go +++ b/command/agent/testagent.go @@ -68,7 +68,7 @@ type TestAgent struct { // All HTTP servers started. Used to prevent server leaks and preserve // backwards compability. - Servers []HTTPServer + Servers []*HTTPServer // Server is a reference to the primary, started HTTP endpoint. // It is valid after Start(). @@ -267,7 +267,7 @@ func (a *TestAgent) start() (*Agent, error) { // TODO: investigate if there is a way to remove the requirement by updating test. // Initial pass at implementing this is https://github.com/kevinschoonover/nomad/tree/tests. a.Servers = httpServers - a.Server = &httpServers[0] + a.Server = httpServers[0] return agent, nil }