introduce a get-or-set method for keyloader

hashicorp · Nov 3, 2017 · 61bfd0c · 61bfd0c
1 parent a2f01cf
commit 61bfd0c
Show file tree

Hide file tree

Showing 10 changed files with 35 additions and 44 deletions.
diff --git a/client/client_test.go b/client/client_test.go
@@ -431,7 +431,6 @@ func TestClient_MixedTLS(t *testing.T) {
 			CAFile:               cafile,
 			CertFile:             foocert,
 			KeyFile:              fookey,
-			KeyLoader:            &nconfig.KeyLoader{},
 		}
 	})
 	defer s1.Shutdown()
@@ -482,7 +481,6 @@ func TestClient_BadTLS(t *testing.T) {
 			CAFile:               cafile,
 			CertFile:             foocert,
 			KeyFile:              fookey,
-			KeyLoader:            &nconfig.KeyLoader{},
 		}
 	})
 	defer s1.Shutdown()
@@ -497,7 +495,6 @@ func TestClient_BadTLS(t *testing.T) {
 			CAFile:               badca,
 			CertFile:             badcert,
 			KeyFile:              badkey,
-			KeyLoader:            &nconfig.KeyLoader{},
 		}
 	})
 	defer c1.Shutdown()

diff --git a/client/config/config.go b/client/config/config.go
@@ -212,15 +212,13 @@ func (c *Config) Copy() *Config {
 // DefaultConfig returns the default configuration
 func DefaultConfig() *Config {
 	return &Config{
-		Version:                 version.GetVersion(),
-		VaultConfig:             config.DefaultVaultConfig(),
-		ConsulConfig:            config.DefaultConsulConfig(),
-		LogOutput:               os.Stderr,
-		Region:                  "global",
-		StatsCollectionInterval: 1 * time.Second,
-		TLSConfig: &config.TLSConfig{
-			KeyLoader: &config.KeyLoader{},
-		},
+		Version:                    version.GetVersion(),
+		VaultConfig:                config.DefaultVaultConfig(),
+		ConsulConfig:               config.DefaultConsulConfig(),
+		LogOutput:                  os.Stderr,
+		Region:                     "global",
+		StatsCollectionInterval:    1 * time.Second,
+		TLSConfig:                  &config.TLSConfig{},
 		LogLevel:                   "DEBUG",
 		GCInterval:                 1 * time.Minute,
 		GCParallelDestroys:         2,
@@ -358,7 +356,7 @@ func (c *Config) TLSConfiguration() *tlsutil.Config {
 		CAFile:               c.TLSConfig.CAFile,
 		CertFile:             c.TLSConfig.CertFile,
 		KeyFile:              c.TLSConfig.KeyFile,
-		KeyLoader:            c.TLSConfig.KeyLoader,
+		KeyLoader:            c.TLSConfig.GetKeyLoader(),
 	}
 	return tlsConf
 }
diff --git a/command/agent/agent_test.go b/command/agent/agent_test.go
@@ -575,7 +575,6 @@ func TestServer_Reload_TLS_UpgradeToTLS(t *testing.T) {
 	agentConfig := &Config{
 		TLSConfig: &sconfig.TLSConfig{
 			EnableHTTP: false,
-			KeyLoader:  &sconfig.KeyLoader{},
 		},
 	}
 
@@ -594,12 +593,12 @@ func TestServer_Reload_TLS_UpgradeToTLS(t *testing.T) {
 		},
 	}
 
-	assert.Nil(agentConfig.TLSConfig.KeyLoader.Certificate)
+	assert.Nil(agentConfig.TLSConfig.GetKeyLoader().Certificate)
 
 	err := agent.Reload(newConfig)
 	assert.Nil(err)
 
-	assert.NotNil(agentConfig.TLSConfig.KeyLoader.Certificate)
+	assert.NotNil(agentConfig.TLSConfig.GetKeyLoader().Certificate)
 }
 
 func TestServer_Reload_TLS_DowngradeFromTLS(t *testing.T) {
@@ -622,7 +621,6 @@ func TestServer_Reload_TLS_DowngradeFromTLS(t *testing.T) {
 			CAFile:               cafile,
 			CertFile:             foocert,
 			KeyFile:              fookey,
-			KeyLoader:            &sconfig.KeyLoader{},
 		},
 	}
 
@@ -633,12 +631,11 @@ func TestServer_Reload_TLS_DowngradeFromTLS(t *testing.T) {
 	newConfig := &Config{
 		TLSConfig: &sconfig.TLSConfig{
 			EnableHTTP: false,
-			KeyLoader:  &sconfig.KeyLoader{},
 		},
 	}
 
 	err := agent.Reload(newConfig)
 	assert.Nil(err)
 
-	assert.Nil(agentConfig.TLSConfig.KeyLoader.Certificate)
+	assert.Nil(agentConfig.TLSConfig.GetKeyLoader().Certificate)
 }
diff --git a/command/agent/config.go b/command/agent/config.go
@@ -331,15 +331,22 @@ type ServerConfig struct {
 // SetTLSConfig will reload an agent's TLS configuration. If there is an error
 // while loading key and certificate files, the agent will remain at its
 // current configuration and return an error.
+// This only allows reloading the certificate and keyfile- other TLSConfig
+// fields are ignored.
 func (c *Config) SetTLSConfig(newConfig *config.TLSConfig) error {
-	newConfig.KeyLoader = &config.KeyLoader{}
-	_, err := newConfig.KeyLoader.LoadKeyPair(newConfig.CertFile, newConfig.KeyFile)
+	if c.TLSConfig == nil {
+		return fmt.Errorf("unable to update non-existing TLSConfig")
+	}
+
+	keyLoader := c.TLSConfig.GetKeyLoader()
+	_, err := keyLoader.LoadKeyPair(newConfig.CertFile, newConfig.KeyFile)
 
 	if err != nil {
 		return err
 	}
 
-	c.TLSConfig = newConfig
+	c.TLSConfig.CertFile = newConfig.CertFile
+	c.TLSConfig.KeyFile = newConfig.KeyFile
 	return nil
 }
 
@@ -610,11 +617,9 @@ func DefaultConfig() *Config {
 			CollectionInterval: "1s",
 			collectionInterval: 1 * time.Second,
 		},
-		TLSConfig: &config.TLSConfig{
-			KeyLoader: &config.KeyLoader{},
-		},
-		Sentinel: &config.SentinelConfig{},
-		Version:  version.GetVersion(),
+		TLSConfig: &config.TLSConfig{},
+		Sentinel:  &config.SentinelConfig{},
+		Version:   version.GetVersion(),
 	}
 }
 
@@ -703,11 +708,6 @@ func (c *Config) Merge(b *Config) *Config {
 		result.TLSConfig = result.TLSConfig.Merge(b.TLSConfig)
 	}
 
-	// Initialize the TLS Keyloader if necessasry
-	if result.TLSConfig != nil && result.TLSConfig.KeyLoader == nil {
-		result.TLSConfig.KeyLoader = &config.KeyLoader{}
-	}
-
 	// Apply the client config
 	if result.Client == nil && b.Client != nil {
 		client := *b.Client

diff --git a/command/agent/config_parse.go b/command/agent/config_parse.go
@@ -752,7 +752,6 @@ func parseTLSConfig(result **config.TLSConfig, list *ast.ObjectList) error {
 
 	// TLSConfig requires a Keyloader object for dynamic reloading of TLS
 	// configuration
-	tlsConfig.KeyLoader = &config.KeyLoader{}
 	*result = &tlsConfig
 	return nil
 }

diff --git a/command/agent/config_parse_test.go b/command/agent/config_parse_test.go
@@ -167,7 +167,6 @@ func TestConfig_Parse(t *testing.T) {
 					CertFile:             "bar",
 					KeyFile:              "pipe",
 					RPCUpgradeMode:       true,
-					KeyLoader:            &config.KeyLoader{},
 					VerifyHTTPSClient:    true,
 				},
 				HTTPAPIResponseHeaders: map[string]string{

diff --git a/command/agent/http.go b/command/agent/http.go
@@ -17,7 +17,6 @@ import (
 	assetfs "github.com/elazarl/go-bindata-assetfs"
 	"github.com/hashicorp/nomad/helper/tlsutil"
 	"github.com/hashicorp/nomad/nomad/structs"
-	nconfig "github.com/hashicorp/nomad/nomad/structs/config"
 	"github.com/rs/cors"
 	"github.com/ugorji/go/codec"
 )
@@ -76,7 +75,7 @@ func NewHTTPServer(agent *Agent, config *Config) (*HTTPServer, error) {
 			CAFile:               config.TLSConfig.CAFile,
 			CertFile:             config.TLSConfig.CertFile,
 			KeyFile:              config.TLSConfig.KeyFile,
-			KeyLoader:            &nconfig.KeyLoader{},
+			KeyLoader:            config.TLSConfig.GetKeyLoader(),
 		}
 		tlsConfig, err := tlsConf.IncomingTLSConfig()
 		if err != nil {

diff --git a/nomad/config.go b/nomad/config.go
@@ -312,15 +312,10 @@ func DefaultConfig() *Config {
 		ConsulConfig:                     config.DefaultConsulConfig(),
 		VaultConfig:                      config.DefaultVaultConfig(),
 		RPCHoldTimeout:                   5 * time.Second,
+		StatsCollectionInterval:          1 * time.Minute,
 		TLSConfig:                        &config.TLSConfig{},
 		ReplicationBackoff:               30 * time.Second,
 		SentinelGCInterval:               30 * time.Second,
-		StatsCollectionInterval:          1 * time.Minute,
-		TLSConfig: &config.TLSConfig{
-			KeyLoader: &config.KeyLoader{},
-		},
-		ReplicationBackoff: 30 * time.Second,
-		SentinelGCInterval: 30 * time.Second,
 	}
 
 	// Enable all known schedulers by default
@@ -360,6 +355,6 @@ func (c *Config) tlsConfig() *tlsutil.Config {
 		CAFile:               c.TLSConfig.CAFile,
 		CertFile:             c.TLSConfig.CertFile,
 		KeyFile:              c.TLSConfig.KeyFile,
-		KeyLoader:            c.TLSConfig.KeyLoader,
+		KeyLoader:            c.TLSConfig.GetKeyLoader(),
 	}
 }
diff --git a/nomad/server_test.go b/nomad/server_test.go
@@ -165,7 +165,6 @@ func TestServer_RPC_MixedTLS(t *testing.T) {
 			CAFile:               cafile,
 			CertFile:             foocert,
 			KeyFile:              fookey,
-			KeyLoader:            &config.KeyLoader{},
 		}
 	})
 	defer s1.Shutdown()

diff --git a/nomad/structs/config/tls.go b/nomad/structs/config/tls.go
@@ -77,6 +77,14 @@ func (k *KeyLoader) GetOutgoingCertificate(*tls.ClientHelloInfo) (*tls.Certifica
 	return k.Certificate, nil
 }
 
+func (t *TLSConfig) GetKeyLoader() *KeyLoader {
+	// If the keyloader has not yet been initialized, do it here
+	if t.KeyLoader == nil {
+		t.KeyLoader = &KeyLoader{}
+	}
+	return t.KeyLoader
+}
+
 // Merge is used to merge two TLS configs together
 func (t *TLSConfig) Merge(b *TLSConfig) *TLSConfig {
 	result := *t