diff --git a/client/client.go b/client/client.go index c58ff7fa5..11b716600 100644 --- a/client/client.go +++ b/client/client.go @@ -371,7 +371,6 @@ func (c *Client) reloadTLSConnections(newConfig *nconfig.TLSConfig) error { var tlsWrap tlsutil.RegionWrapper if newConfig != nil && newConfig.EnableRPC { tw, err := c.config.NewTLSConfiguration(newConfig).OutgoingTLSWrapper() - if err != nil { return err } diff --git a/client/client_test.go b/client/client_test.go index d4cba7db5..95ff480d3 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -1007,7 +1007,7 @@ func TestClient_ReloadTLS_UpgradePlaintextToTLS(t *testing.T) { assert := assert.New(t) s1, addr := testServer(t, func(c *nomad.Config) { - c.Region = "foo" + c.Region = "regionFoo" }) defer s1.Shutdown() testutil.WaitForLeader(t, s1.RPC) @@ -1023,6 +1023,27 @@ func TestClient_ReloadTLS_UpgradePlaintextToTLS(t *testing.T) { }) defer c1.Shutdown() + // Registering a node over plaintext should succeed + { + req := structs.NodeSpecificRequest{ + NodeID: c1.Node().ID, + QueryOptions: structs.QueryOptions{Region: "regionFoo"}, + } + + testutil.WaitForResult(func() (bool, error) { + var out structs.SingleNodeResponse + err := c1.RPC("Node.GetNode", &req, &out) + if err != nil { + return false, fmt.Errorf("client RPC failed when it should have succeeded:\n%+v", err) + } + return true, nil + }, + func(err error) { + t.Fatalf(err.Error()) + }, + ) + } + newConfig := &nconfig.TLSConfig{ EnableHTTP: true, EnableRPC: true, @@ -1035,23 +1056,26 @@ func TestClient_ReloadTLS_UpgradePlaintextToTLS(t *testing.T) { err := c1.reloadTLSConnections(newConfig) assert.Nil(err) - req := structs.NodeSpecificRequest{ - NodeID: c1.Node().ID, - QueryOptions: structs.QueryOptions{Region: "dc1"}, - } - var out structs.SingleNodeResponse - testutil.AssertUntil(100*time.Millisecond, - func() (bool, error) { + // Registering a node over plaintext should fail after the node has upgraded + // to TLS + { + req := structs.NodeSpecificRequest{ + NodeID: c1.Node().ID, + QueryOptions: structs.QueryOptions{Region: "regionFoo"}, + } + testutil.WaitForResult(func() (bool, error) { + var out structs.SingleNodeResponse err := c1.RPC("Node.GetNode", &req, &out) if err == nil { return false, fmt.Errorf("client RPC succeeded when it should have failed:\n%+v", err) } return true, nil }, - func(err error) { - t.Fatalf(err.Error()) - }, - ) + func(err error) { + t.Fatalf(err.Error()) + }, + ) + } } func TestClient_ReloadTLS_DowngradeTLSToPlaintext(t *testing.T) { @@ -1059,7 +1083,7 @@ func TestClient_ReloadTLS_DowngradeTLSToPlaintext(t *testing.T) { assert := assert.New(t) s1, addr := testServer(t, func(c *nomad.Config) { - c.Region = "foo" + c.Region = "regionFoo" }) defer s1.Shutdown() testutil.WaitForLeader(t, s1.RPC) @@ -1083,26 +1107,50 @@ func TestClient_ReloadTLS_DowngradeTLSToPlaintext(t *testing.T) { }) defer c1.Shutdown() + // assert that when one node is running in encrypted mode, a RPC request to a + // node running in plaintext mode should fail + { + req := structs.NodeSpecificRequest{ + NodeID: c1.Node().ID, + QueryOptions: structs.QueryOptions{Region: "regionFoo"}, + } + testutil.WaitForResult(func() (bool, error) { + var out structs.SingleNodeResponse + err := c1.RPC("Node.GetNode", &req, &out) + if err == nil { + return false, fmt.Errorf("client RPC succeeded when it should have failed :\n%+v", err) + } + return true, nil + }, + func(err error) { + t.Fatalf(err.Error()) + }, + ) + } + newConfig := &nconfig.TLSConfig{} err := c1.reloadTLSConnections(newConfig) assert.Nil(err) - req := structs.NodeSpecificRequest{ - NodeID: c1.Node().ID, - QueryOptions: structs.QueryOptions{Region: "foo"}, - } - var out structs.SingleNodeResponse - testutil.AssertUntil(100*time.Millisecond, - func() (bool, error) { + // assert that when both nodes are in plaintext mode, a RPC request should + // succeed + { + req := structs.NodeSpecificRequest{ + NodeID: c1.Node().ID, + QueryOptions: structs.QueryOptions{Region: "regionFoo"}, + } + testutil.WaitForResult(func() (bool, error) { + var out structs.SingleNodeResponse err := c1.RPC("Node.GetNode", &req, &out) if err != nil { return false, fmt.Errorf("client RPC failed when it should have succeeded:\n%+v", err) } return true, nil }, - func(err error) { - t.Fatalf(err.Error()) - }, - ) + func(err error) { + t.Fatalf(err.Error()) + }, + ) + } } diff --git a/command/agent/agent.go b/command/agent/agent.go index 1f6b400e7..3ec9e00f0 100644 --- a/command/agent/agent.go +++ b/command/agent/agent.go @@ -732,14 +732,14 @@ 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 { +func (a *Agent) ShouldReload(newConfig *Config) (bool, bool) { a.configLock.Lock() defer a.configLock.Unlock() if a.config.TLSConfig.Equals(newConfig.TLSConfig) { - return false + return false, false } - return true + return true, true // requires a reload of both agent and http server } // Reload handles configuration changes for the agent. Provides a method that diff --git a/command/agent/agent_test.go b/command/agent/agent_test.go index 6042d6902..196b00d77 100644 --- a/command/agent/agent_test.go +++ b/command/agent/agent_test.go @@ -888,8 +888,9 @@ func TestServer_ShouldReload_ReturnFalseForNoChanges(t *testing.T) { config: agentConfig, } - shouldReload := agent.ShouldReload(sameAgentConfig) - assert.False(shouldReload) + shouldReloadAgent, shouldReloadHTTPServer := agent.ShouldReload(sameAgentConfig) + assert.False(shouldReloadAgent) + assert.False(shouldReloadHTTPServer) } func TestServer_ShouldReload_ReturnTrueForConfigChanges(t *testing.T) { @@ -935,6 +936,7 @@ func TestServer_ShouldReload_ReturnTrueForConfigChanges(t *testing.T) { config: agentConfig, } - shouldReload := agent.ShouldReload(newConfig) - assert.True(shouldReload) + shouldReloadAgent, shouldReloadHTTPServer := agent.ShouldReload(newConfig) + assert.True(shouldReloadAgent) + assert.True(shouldReloadHTTPServer) } diff --git a/command/agent/command.go b/command/agent/command.go index 5a7c67831..cac8616c4 100644 --- a/command/agent/command.go +++ b/command/agent/command.go @@ -634,40 +634,40 @@ func (c *Command) handleReload() { newConf.LogLevel = c.agent.GetConfig().LogLevel } - shouldReload := c.agent.ShouldReload(newConf) - if shouldReload { + shouldReloadAgent, shouldReloadHTTPServer := 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 s := c.agent.Server(); s != nil { - sconf, err := convertServerConfig(newConf, c.logOutput) - c.agent.logger.Printf("[DEBUG] agent: starting reload of server config") - if err != nil { - c.agent.logger.Printf("[ERR] agent: failed to convert server config: %v", err) - return - } else { - if err := s.Reload(sconf); err != nil { - c.agent.logger.Printf("[ERR] agent: reloading server config failed: %v", err) + if s := c.agent.Server(); s != nil { + sconf, err := convertServerConfig(newConf, c.logOutput) + c.agent.logger.Printf("[DEBUG] agent: starting reload of server config") + if err != nil { + c.agent.logger.Printf("[ERR] agent: failed to convert server config: %v", err) return + } 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() - c.agent.logger.Printf("[DEBUG] agent: starting reload of client config") - 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 + if s := c.agent.Client(); s != nil { + clientConfig, err := c.agent.clientConfig() + c.agent.logger.Printf("[DEBUG] agent: starting reload of client config") + 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 + } } } @@ -675,7 +675,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 shouldReload { + if shouldReloadHTTPServer { err := c.reloadHTTPServer(newConf) if err != nil { c.agent.logger.Printf("[ERR] http: failed to reload the config: %v", err)