Allow TLS configurations for HTTP and RPC connections to be reloaded …

command/agent/agent.go

@@ -777,14 +777,34 @@ func (a *Agent) Stats() map[string]map[string]string {
 
 // ShouldReload determines if we should reload the configuration and agent
 // connections. If the TLS Configuration has not changed, we shouldn't reload.
-func (a *Agent) ShouldReload(newConfig *Config) (bool, bool) {
+func (a *Agent) ShouldReload(newConfig *Config) (bool, bool, bool) {
+	var shouldReloadAgent, shouldReloadHTTP, shouldReloadRPC bool
+
 	a.configLock.Lock()
 	defer a.configLock.Unlock()
-	if a.config.TLSConfig.Equals(newConfig.TLSConfig) {
-		return false, false
+
+	var tlsInfoChanged bool
+	if !a.config.TLSConfig.CertificateInfoIsEqual(newConfig.TLSConfig) {
+		tlsInfoChanged = true
+	}
+
+	// Allow the ability to only reload HTTP connections
+	if a.config.TLSConfig.EnableHTTP != newConfig.TLSConfig.EnableHTTP || tlsInfoChanged {
+		shouldReloadHTTP = true
+	}
+
+	// Allow the ability to only reload HTTP connections
+	if a.config.TLSConfig.EnableRPC != newConfig.TLSConfig.EnableRPC || tlsInfoChanged {
+		shouldReloadRPC = true
+	}
+
+	// Always reload the agent if either HTTP or RPC connections need to reload,
+	// or if the TLS configuration itself has changed
+	if shouldReloadHTTP || shouldReloadRPC || tlsInfoChanged {
+		shouldReloadAgent = true
 	}
 
-	return true, true // requires a reload of both agent and http server
+	return shouldReloadAgent, shouldReloadHTTP, shouldReloadRPC
 }
 
 // Reload handles configuration changes for the agent. Provides a method that

command/agent/agent_test.go

@@ -771,9 +771,104 @@ func TestServer_ShouldReload_ReturnFalseForNoChanges(t *testing.T) {
 		config: agentConfig,
 	}
 
-	shouldReloadAgent, shouldReloadHTTPServer := agent.ShouldReload(sameAgentConfig)
+	shouldReloadAgent, shouldReloadHTTP, shouldReloadRPC := agent.ShouldReload(sameAgentConfig)
 	assert.False(shouldReloadAgent)
-	assert.False(shouldReloadHTTPServer)
+	assert.False(shouldReloadHTTP)
+	assert.False(shouldReloadRPC)
+}
+
+func TestServer_ShouldReload_ReturnTrueForOnlyHTTPChanges(t *testing.T) {
+	t.Parallel()
+	assert := assert.New(t)
+
+	const (
+		cafile  = "../../helper/tlsutil/testdata/ca.pem"
+		foocert = "../../helper/tlsutil/testdata/nomad-foo.pem"
+		fookey  = "../../helper/tlsutil/testdata/nomad-foo-key.pem"
+	)
+	dir := tmpDir(t)
+	defer os.RemoveAll(dir)
+
+	logger := log.New(ioutil.Discard, "", 0)
+
+	agentConfig := &Config{
+		TLSConfig: &sconfig.TLSConfig{
+			EnableHTTP:           false,
+			EnableRPC:            true,
+			VerifyServerHostname: true,
+			CAFile:               cafile,
+			CertFile:             foocert,
+			KeyFile:              fookey,
+		},
+	}
+
+	sameAgentConfig := &Config{
+		TLSConfig: &sconfig.TLSConfig{
+			EnableHTTP:           true,
+			EnableRPC:            true,
+			VerifyServerHostname: true,
+			CAFile:               cafile,
+			CertFile:             foocert,
+			KeyFile:              fookey,
+		},
+	}
+
+	agent := &Agent{
+		logger: logger,
+		config: agentConfig,
+	}
+
+	shouldReloadAgent, shouldReloadHTTP, shouldReloadRPC := agent.ShouldReload(sameAgentConfig)
+	assert.True(shouldReloadAgent)
+	assert.True(shouldReloadHTTP)
+	assert.False(shouldReloadRPC)
+}
+
+func TestServer_ShouldReload_ReturnTrueForOnlyRPCChanges(t *testing.T) {
+	t.Parallel()
+	assert := assert.New(t)
+
+	const (
+		cafile  = "../../helper/tlsutil/testdata/ca.pem"
+		foocert = "../../helper/tlsutil/testdata/nomad-foo.pem"
+		fookey  = "../../helper/tlsutil/testdata/nomad-foo-key.pem"
+	)
+	dir := tmpDir(t)
+	defer os.RemoveAll(dir)
+
+	logger := log.New(ioutil.Discard, "", 0)
+
+	agentConfig := &Config{
+		TLSConfig: &sconfig.TLSConfig{
+			EnableHTTP:           true,
+			EnableRPC:            false,
+			VerifyServerHostname: true,
+			CAFile:               cafile,
+			CertFile:             foocert,
+			KeyFile:              fookey,
+		},
+	}
+
+	sameAgentConfig := &Config{
+		TLSConfig: &sconfig.TLSConfig{
+			EnableHTTP:           true,
+			EnableRPC:            true,
+			VerifyServerHostname: true,
+			CAFile:               cafile,
+			CertFile:             foocert,
+			KeyFile:              fookey,
+		},
+	}
+
+	agent := &Agent{
+		logger: logger,
+		config: agentConfig,
+	}
+
+	shouldReloadAgent, shouldReloadHTTP, shouldReloadRPC := agent.ShouldReload(sameAgentConfig)
+	assert.True(shouldReloadAgent)
+	assert.False(shouldReloadHTTP)
+	assert.True(shouldReloadRPC)
 }
 
 func TestServer_ShouldReload_ReturnTrueForConfigChanges(t *testing.T) {
@@ -819,7 +914,8 @@ func TestServer_ShouldReload_ReturnTrueForConfigChanges(t *testing.T) {
 		config: agentConfig,
 	}
 
-	shouldReloadAgent, shouldReloadHTTPServer := agent.ShouldReload(newConfig)
+	shouldReloadAgent, shouldReloadHTTP, shouldReloadRPC := agent.ShouldReload(newConfig)
 	assert.True(shouldReloadAgent)
-	assert.True(shouldReloadHTTPServer)
+	assert.True(shouldReloadHTTP)
+	assert.True(shouldReloadRPC)
 }

command/agent/command.go

@@ -640,15 +640,17 @@ func (c *Command) handleReload() {
 		newConf.LogLevel = c.agent.GetConfig().LogLevel
 	}
 
-	shouldReloadAgent, shouldReloadHTTPServer := c.agent.ShouldReload(newConf)
+	shouldReloadAgent, shouldReloadHTTP, shouldReloadRPC := c.agent.ShouldReload(newConf)
 	if shouldReloadAgent {
 		c.agent.logger.Printf("[DEBUG] agent: starting reload of agent config")
 		err := c.agent.Reload(newConf)
 		if err != nil {
 			c.agent.logger.Printf("[ERR] agent: failed to reload the config: %v", err)
 			return
 		}
+	}
 
+	if shouldReloadRPC {
 		if s := c.agent.Server(); s != nil {
 			sconf, err := convertServerConfig(newConf, c.logOutput)
 			c.agent.logger.Printf("[DEBUG] agent: starting reload of server config")
@@ -681,7 +683,7 @@ func (c *Command) handleReload() {
 	// 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 shouldReloadHTTPServer {
+	if shouldReloadHTTP {
 		err := c.reloadHTTPServer()
 		if err != nil {
 			c.agent.logger.Printf("[ERR] http: failed to reload the config: %v", err)

nomad/server.go

@@ -658,7 +658,7 @@ func (s *Server) Reload(newConfig *Config) error {
 		}
 	}
 
-	if !newConfig.TLSConfig.Equals(s.config.TLSConfig) {
+	if !newConfig.TLSConfig.CertificateInfoIsEqual(s.config.TLSConfig) || newConfig.TLSConfig.EnableRPC != s.config.TLSConfig.EnableRPC {
 		if err := s.reloadTLSConnections(newConfig.TLSConfig); err != nil {
 			s.logger.Printf("[ERR] nomad: error reloading server TLS configuration: %s", err)
 			multierror.Append(&mErr, err)

nomad/server_test.go

@@ -271,7 +271,7 @@ func TestServer_Reload_TLSConnections_PlaintextToTLS(t *testing.T) {
 
 	err := s1.reloadTLSConnections(newTLSConfig)
 	assert.Nil(err)
-	assert.True(s1.config.TLSConfig.Equals(newTLSConfig))
+	assert.True(s1.config.TLSConfig.CertificateInfoIsEqual(newTLSConfig))
 
 	codec := rpcClient(t, s1)
 
@@ -319,7 +319,61 @@ func TestServer_Reload_TLSConnections_TLSToPlaintext_RPC(t *testing.T) {
 
 	err := s1.reloadTLSConnections(newTLSConfig)
 	assert.Nil(err)
-	assert.True(s1.config.TLSConfig.Equals(newTLSConfig))
+	assert.True(s1.config.TLSConfig.CertificateInfoIsEqual(newTLSConfig))
+
+	codec := rpcClient(t, s1)
+
+	node := mock.Node()
+	req := &structs.NodeRegisterRequest{
+		Node:         node,
+		WriteRequest: structs.WriteRequest{Region: "global"},
+	}
+
+	var resp structs.GenericResponse
+	err = msgpackrpc.CallWithCodec(codec, "Node.Register", req, &resp)
+	assert.Nil(err)
+}
+
+// Tests that the server will successfully reload its network connections,
+// downgrading only RPC connections
+func TestServer_Reload_TLSConnections_TLSToPlaintext_OnlyRPC(t *testing.T) {
+	t.Parallel()
+	assert := assert.New(t)
+
+	const (
+		cafile  = "../helper/tlsutil/testdata/ca.pem"
+		foocert = "../helper/tlsutil/testdata/nomad-foo.pem"
+		fookey  = "../helper/tlsutil/testdata/nomad-foo-key.pem"
+	)
+
+	dir := tmpDir(t)
+	defer os.RemoveAll(dir)
+
+	s1 := TestServer(t, func(c *Config) {
+		c.DataDir = path.Join(dir, "nodeB")
+		c.TLSConfig = &config.TLSConfig{
+			EnableHTTP:           true,
+			EnableRPC:            true,
+			VerifyServerHostname: true,
+			CAFile:               cafile,
+			CertFile:             foocert,
+			KeyFile:              fookey,
+		}
+	})
+	defer s1.Shutdown()
+
+	newTLSConfig := &config.TLSConfig{
+		EnableHTTP:           true,
+		EnableRPC:            false,
+		VerifyServerHostname: true,
+		CAFile:               cafile,
+		CertFile:             foocert,
+		KeyFile:              fookey,
+	}
+
+	err := s1.reloadTLSConnections(newTLSConfig)
+	assert.Nil(err)
+	assert.True(s1.config.TLSConfig.CertificateInfoIsEqual(newTLSConfig))
 
 	codec := rpcClient(t, s1)
 

nomad/structs/config/tls.go

@@ -186,20 +186,17 @@ func (t *TLSConfig) Merge(b *TLSConfig) *TLSConfig {
 	return result
 }
 
-// Equals compares the fields of two TLS configuration objects, returning a
-// boolean indicating if they are the same.
+// CertificateInfoIsEqual compares the fields of two TLS configuration objects
+// for the fields that are specific to configuring a TLS connection
 // 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 {
+func (t *TLSConfig) CertificateInfoIsEqual(newConfig *TLSConfig) bool {
 	if t == nil || newConfig == nil {
 		return t == newConfig
 	}
 
-	return t.EnableRPC == newConfig.EnableRPC &&
-		t.CAFile == newConfig.CAFile &&
+	return t.CAFile == newConfig.CAFile &&
 		t.CertFile == newConfig.CertFile &&
-		t.KeyFile == newConfig.KeyFile &&
-		t.RPCUpgradeMode == newConfig.RPCUpgradeMode &&
-		t.VerifyHTTPSClient == newConfig.VerifyHTTPSClient
+		t.KeyFile == newConfig.KeyFile
 }

nomad/structs/config/tls_test.go

@@ -26,32 +26,32 @@ func TestTLSConfig_Merge(t *testing.T) {
 	assert.Equal(b, new)
 }
 
-func TestTLS_Equals_TrueWhenEmpty(t *testing.T) {
+func TestTLS_CertificateInfoIsEqual_TrueWhenEmpty(t *testing.T) {
 	assert := assert.New(t)
 	a := &TLSConfig{}
 	b := &TLSConfig{}
-	assert.True(a.Equals(b))
+	assert.True(a.CertificateInfoIsEqual(b))
 }
 
-func TestTLS_Equals_FalseWhenUnequal(t *testing.T) {
+func TestTLS_CertificateInfoIsEqual_FalseWhenUnequal(t *testing.T) {
 	assert := assert.New(t)
 	a := &TLSConfig{CAFile: "abc", CertFile: "def", KeyFile: "ghi"}
 	b := &TLSConfig{CAFile: "jkl", CertFile: "def", KeyFile: "ghi"}
-	assert.False(a.Equals(b))
+	assert.False(a.CertificateInfoIsEqual(b))
 }
 
-func TestTLS_Equals_TrueWhenEqual(t *testing.T) {
+func TestTLS_CertificateInfoIsEqual_TrueWhenEqual(t *testing.T) {
 	assert := assert.New(t)
 	a := &TLSConfig{CAFile: "abc", CertFile: "def", KeyFile: "ghi"}
 	b := &TLSConfig{CAFile: "abc", CertFile: "def", KeyFile: "ghi"}
-	assert.True(a.Equals(b))
+	assert.True(a.CertificateInfoIsEqual(b))
 }
 
 func TestTLS_Copy(t *testing.T) {
 	assert := assert.New(t)
 	a := &TLSConfig{CAFile: "abc", CertFile: "def", KeyFile: "ghi"}
 	aCopy := a.Copy()
-	assert.True(a.Equals(aCopy))
+	assert.True(a.CertificateInfoIsEqual(aCopy))
 }
 
 // GetKeyLoader should always return an initialized KeyLoader for a TLSConfig