add ability to upgrade/downgrade nomad agents tls configurations via …

…sighup
hashicorp · Nov 20, 2017 · ad970c3 · ad970c3
1 parent 0d1878f
commit ad970c3
Show file tree

Hide file tree

Showing 13 changed files with 744 additions and 41 deletions.
diff --git a/client/client.go b/client/client.go
@@ -30,6 +30,7 @@ import (
 	"github.com/hashicorp/nomad/helper/uuid"
 	"github.com/hashicorp/nomad/nomad"
 	"github.com/hashicorp/nomad/nomad/structs"
+	nconfig "github.com/hashicorp/nomad/nomad/structs/config"
 	vaultapi "github.com/hashicorp/vault/api"
 	"github.com/mitchellh/hashstructure"
 	"github.com/shirou/gopsutil/host"
@@ -363,6 +364,25 @@ func (c *Client) init() error {
 	return nil
 }
 
+// ReloadTLSConnectoins allows a client to reload RPC connections if the
+// client's TLS configuration changes from plaintext to TLS
+func (c *Client) ReloadTLSConnections(newConfig *nconfig.TLSConfig) error {
+	c.configLock.Lock()
+	defer c.configLock.Unlock()
+
+	c.config.TLSConfig = newConfig
+
+	if c.config.TLSConfig.EnableRPC {
+		tw, err := c.config.TLSConfiguration().OutgoingTLSWrapper()
+		if err != nil {
+			return err
+		}
+		c.connPool.ReloadTLS(tw)
+	}
+
+	return nil
+}
+
 // 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
@@ -1001,3 +1001,108 @@ func TestClient_ValidateMigrateToken_ACLDisabled(t *testing.T) {
 
 	assert.Equal(c.ValidateMigrateToken("", ""), true)
 }
+
+func TestClient_ReloadTLS_UpgradePlaintextToTLS(t *testing.T) {
+	t.Parallel()
+	assert := assert.New(t)
+
+	s1, addr := testServer(t, func(c *nomad.Config) {
+		c.Region = "dc1"
+	})
+	defer s1.Shutdown()
+	testutil.WaitForLeader(t, s1.RPC)
+
+	const (
+		cafile  = "../helper/tlsutil/testdata/ca.pem"
+		foocert = "../helper/tlsutil/testdata/nomad-foo.pem"
+		fookey  = "../helper/tlsutil/testdata/nomad-foo-key.pem"
+	)
+
+	c1 := testClient(t, func(c *config.Config) {
+		c.Servers = []string{addr}
+	})
+	defer c1.Shutdown()
+
+	newConfig := &nconfig.TLSConfig{
+		EnableHTTP:           true,
+		EnableRPC:            true,
+		VerifyServerHostname: true,
+		CAFile:               cafile,
+		CertFile:             foocert,
+		KeyFile:              fookey,
+	}
+
+	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) {
+			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 TestClient_ReloadTLS_DowngradeTLSToPlaintext(t *testing.T) {
+	t.Parallel()
+	assert := assert.New(t)
+
+	s1, addr := testServer(t, func(c *nomad.Config) {
+		c.Region = "dc1"
+	})
+	defer s1.Shutdown()
+	testutil.WaitForLeader(t, s1.RPC)
+
+	const (
+		cafile  = "../helper/tlsutil/testdata/ca.pem"
+		foocert = "../helper/tlsutil/testdata/nomad-foo.pem"
+		fookey  = "../helper/tlsutil/testdata/nomad-foo-key.pem"
+	)
+
+	c1 := testClient(t, func(c *config.Config) {
+		c.Servers = []string{addr}
+		c.TLSConfig = &nconfig.TLSConfig{
+			EnableHTTP:           true,
+			EnableRPC:            true,
+			VerifyServerHostname: true,
+			CAFile:               cafile,
+			CertFile:             foocert,
+			KeyFile:              fookey,
+		}
+	})
+	defer c1.Shutdown()
+
+	newConfig := &nconfig.TLSConfig{}
+
+	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) {
+			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())
+		},
+	)
+}
diff --git a/command/agent/agent.go b/command/agent/agent.go
@@ -726,29 +726,71 @@ func (a *Agent) Stats() map[string]map[string]string {
 	return stats
 }
 
+// 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(*Config) error) {
+	if a.config.TLSConfig.Equals(newConfig.TLSConfig) {
+		return false, nil
+	}
+
+	return true, a.Reload
+}
+
 // Reload handles configuration changes for the agent. Provides a method that
 // is easier to unit test, as this action is invoked via SIGHUP.
 func (a *Agent) Reload(newConfig *Config) error {
 	a.configLock.Lock()
 	defer a.configLock.Unlock()
 
-	if newConfig.TLSConfig != nil {
-
-		// TODO(chelseakomlo) In a later PR, we will introduce the ability to reload
-		// TLS configuration if the agent is not running with TLS enabled.
-		if a.config.TLSConfig != nil {
-			// Reload the certificates on the keyloader and on success store the
-			// updated TLS config. It is important to reuse the same keyloader
-			// as this allows us to dynamically reload configurations not only
-			// on the Agent but on the Server and Client too (they are
-			// referencing the same keyloader).
-			keyloader := a.config.TLSConfig.GetKeyLoader()
-			_, err := keyloader.LoadKeyPair(newConfig.TLSConfig.CertFile, newConfig.TLSConfig.KeyFile)
-			if err != nil {
-				return err
-			}
-			a.config.TLSConfig = newConfig.TLSConfig
-			a.config.TLSConfig.KeyLoader = keyloader
+	if newConfig == nil || newConfig.TLSConfig == nil {
+		return fmt.Errorf("cannot reload agent with nil configuration")
+	}
+
+	// This is just a TLS configuration reload, we don't need to refresh
+	// existing network connections
+	if !a.config.TLSConfig.IsEmpty() && !newConfig.TLSConfig.IsEmpty() {
+
+		// Reload the certificates on the keyloader and on success store the
+		// updated TLS config. It is important to reuse the same keyloader
+		// as this allows us to dynamically reload configurations not only
+		// on the Agent but on the Server and Client too (they are
+		// referencing the same keyloader).
+		keyloader := a.config.TLSConfig.GetKeyLoader()
+		_, err := keyloader.LoadKeyPair(newConfig.TLSConfig.CertFile, newConfig.TLSConfig.KeyFile)
+		if err != nil {
+			return err
+		}
+		a.config.TLSConfig = newConfig.TLSConfig
+		a.config.TLSConfig.KeyLoader = keyloader
+		return nil
+	}
+
+	// Completely reload the agent's TLS configuration (moving from non-TLS to
+	// TLS, or vice versa)
+	// This does not handle errors in loading the new TLS configuration
+	a.config.TLSConfig = newConfig.TLSConfig.Copy()
+
+	if newConfig.TLSConfig.IsEmpty() {
+		a.logger.Println("[WARN] Downgrading agent's existing TLS configuration to plaintext")
+	} else {
+		a.logger.Println("[INFO] 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: Issue reloading the server's TLS Configuration, consider a full system restart: %v", err.Error())
+			return err
+		}
+	}
+	if c := a.Client(); c != nil {
+
+		err := c.ReloadTLSConnections(a.config.TLSConfig)
+		if err != nil {
+			a.logger.Printf("[ERR] agent: Issue reloading the client's TLS Configuration, consider a full system restart: %v", err.Error())
+			return err
 		}
 	}