diff --git a/client/client.go b/client/client.go index 2b4f4456d..0b73c63bb 100644 --- a/client/client.go +++ b/client/client.go @@ -369,7 +369,7 @@ func (c *Client) init() error { // client's TLS configuration changes from plaintext to TLS func (c *Client) ReloadTLSConnections(newConfig *nconfig.TLSConfig) error { var tlsWrap tlsutil.RegionWrapper - if newConfig.EnableRPC { + if newConfig != nil && newConfig.EnableRPC { tw, err := c.config.NewTLSConfiguration(newConfig).OutgoingTLSWrapper() if err != nil { @@ -378,15 +378,21 @@ func (c *Client) ReloadTLSConnections(newConfig *nconfig.TLSConfig) error { tlsWrap = tw } - c.connPool.ReloadTLS(tlsWrap) - c.configLock.Lock() c.config.TLSConfig = newConfig c.configLock.Unlock() + c.connPool.ReloadTLS(tlsWrap) + return nil } +// Reload allows a client to reload RPC connections if the +// client's TLS configuration changes +func (c *Client) Reload(newConfig *config.Config) error { + return c.reloadTLSConnections(newConfig.TLSConfig) +} + // Leave is used to prepare the client to leave the cluster func (c *Client) Leave() error { // TODO diff --git a/client/client_test.go b/client/client_test.go index 92b299595..ae2f2cdc4 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -1032,7 +1032,7 @@ func TestClient_ReloadTLS_UpgradePlaintextToTLS(t *testing.T) { KeyFile: fookey, } - err := c1.ReloadTLSConnections(newConfig) + err := c1.reloadTLSConnections(newConfig) assert.Nil(err) req := structs.NodeSpecificRequest{ @@ -1085,7 +1085,7 @@ func TestClient_ReloadTLS_DowngradeTLSToPlaintext(t *testing.T) { newConfig := &nconfig.TLSConfig{} - err := c1.ReloadTLSConnections(newConfig) + err := c1.reloadTLSConnections(newConfig) assert.Nil(err) req := structs.NodeSpecificRequest{ diff --git a/command/agent/agent.go b/command/agent/agent.go index 59bd4244a..1f6b400e7 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -782,24 +782,6 @@ func (a *Agent) Reload(newConfig *Config) error { a.logger.Println("[INFO] agent: Upgrading from plaintext configuration to TLS") } - // Reload the TLS configuration for the client or server, depending on how - // the agent is configured to run. - if s := a.Server(); s != nil { - err := s.ReloadTLSConnections(a.config.TLSConfig) - if err != nil { - a.logger.Printf("[WARN] agent: error reloading the server's TLS configuration: %v", err.Error()) - return err - } - } - if c := a.Client(); c != nil { - - err := c.ReloadTLSConnections(a.config.TLSConfig) - if err != nil { - a.logger.Printf("[WARN] agent: error reloading the server's TLS configuration: %v", err.Error()) - return err - } - } - return nil } diff --git a/command/agent/command.go b/command/agent/command.go index 06369773f..8844e78ed 100644 --- a/command/agent/command.go +++ b/command/agent/command.go @@ -640,18 +640,11 @@ func (c *Command) handleReload() { shouldReload := c.agent.ShouldReload(newConf) if shouldReload { - // Reloads configuration for an agent running in both client and server mode err := c.agent.Reload(newConf) if err != nil { c.agent.logger.Printf("[ERR] agent: failed to reload the config: %v", err) return } - - err = c.reloadHTTPServer(newConf) - if err != nil { - c.agent.logger.Printf("[ERR] http: failed to reload the config: %v", err) - return - } } if s := c.agent.Server(); s != nil { @@ -661,9 +654,34 @@ func (c *Command) handleReload() { } else { if err := s.Reload(sconf); err != nil { c.agent.logger.Printf("[ERR] agent: reloading server config failed: %v", err) + return } } } + + if s := c.agent.Client(); s != nil { + clientConfig, err := c.agent.clientConfig() + if err != nil { + c.agent.logger.Printf("[ERR] agent: reloading client config failed: %v", err) + return + } + if err := c.agent.Client().Reload(clientConfig); err != nil { + c.agent.logger.Printf("[ERR] agent: reloading client config failed: %v", err) + return + } + } + + // reload HTTP server after we have reloaded both client and server, in case + // we error in either of the above cases. For example, reloading the http + // server to a TLS connection could succeed, while reloading the server's rpc + // connections could fail. + if shouldReload { + err := c.reloadHTTPServer(newConf) + if err != nil { + c.agent.logger.Printf("[ERR] http: failed to reload the config: %v", err) + return + } + } } // setupTelemetry is used ot setup the telemetry sub-systems diff --git a/nomad/server.go b/nomad/server.go index 51f6e5bba..4ea115694 100644 --- a/nomad/server.go +++ b/nomad/server.go @@ -400,8 +400,8 @@ func getTLSConf(enableRPC bool, tlsConf *tlsutil.Config) (*tls.Config, tlsutil.R } // ReloadTLSConnections updates a server's TLS configuration and reloads RPC -// connections -func (s *Server) ReloadTLSConnections(newTLSConfig *config.TLSConfig) error { +// connections. This will handle both TLS upgrades and downgrades. +func (s *Server) reloadTLSConnections(newTLSConfig *config.TLSConfig) error { s.logger.Printf("[INFO] nomad: reloading server connections due to configuration changes") tlsConf := s.config.newTLSConfig(newTLSConfig) @@ -619,6 +619,12 @@ func (s *Server) Reload(newConfig *Config) error { } } + if !newConfig.TLSConfig.Equals(s.config.TLSConfig) { + if err := s.reloadTLSConnections(newConfig.TLSConfig); err != nil { + multierror.Append(&mErr, err) + } + } + return mErr.ErrorOrNil() } diff --git a/nomad/server_test.go b/nomad/server_test.go index bc7eee0a1..89e6810d7 100644 --- a/nomad/server_test.go +++ b/nomad/server_test.go @@ -309,7 +309,7 @@ func TestServer_Reload_TLSConnections_PlaintextToTLS(t *testing.T) { KeyFile: fookey, } - err := s1.ReloadTLSConnections(newTLSConfig) + err := s1.reloadTLSConnections(newTLSConfig) assert.Nil(err) assert.True(s1.config.TLSConfig.Equals(newTLSConfig)) @@ -330,7 +330,7 @@ func TestServer_Reload_TLSConnections_PlaintextToTLS(t *testing.T) { // Tests that the server will successfully reload its network connections, // downgrading from TLS to plaintext if the server's TLS configuration changes. -func TestServer_Reload_TLSConnections_TLSToPlaintext(t *testing.T) { +func TestServer_reload_TLSConnections_TLSToPlaintext(t *testing.T) { t.Parallel() assert := assert.New(t) @@ -357,7 +357,7 @@ func TestServer_Reload_TLSConnections_TLSToPlaintext(t *testing.T) { newTLSConfig := &config.TLSConfig{} - err := s1.ReloadTLSConnections(newTLSConfig) + err := s1.reloadTLSConnections(newTLSConfig) assert.Nil(err) assert.True(s1.config.TLSConfig.Equals(newTLSConfig)) diff --git a/nomad/structs/config/tls.go b/nomad/structs/config/tls.go index e309093e3..be1e8f42a 100644 --- a/nomad/structs/config/tls.go +++ b/nomad/structs/config/tls.go @@ -188,9 +188,14 @@ func (t *TLSConfig) Merge(b *TLSConfig) *TLSConfig { // Equals compares the fields of two TLS configuration objects, returning a // boolean indicating if they are the same. -// NewConfig should never be nil- calling code is responsible for walways -// passing a valid TLSConfig object +// It is possible for either the calling TLSConfig to be nil, or the TLSConfig +// that it is being compared against, so we need to handle both places. See +// server.go Reload for example. func (t *TLSConfig) Equals(newConfig *TLSConfig) bool { + if t == nil || newConfig == nil { + return t == newConfig + } + return t.EnableRPC == newConfig.EnableRPC && t.CAFile == newConfig.CAFile && t.CertFile == newConfig.CertFile &&