agent: fix loading keyring on agent start

command/agent/agent.go

@@ -161,11 +161,6 @@ func (a *Agent) consulConfig() *consul.Config {
 	if a.config.DataDir != "" {
 		base.DataDir = a.config.DataDir
 	}
-	if a.config.EncryptKey != "" {
-		key, _ := a.config.EncryptBytes()
-		base.SerfLANConfig.MemberlistConfig.SecretKey = key
-		base.SerfWANConfig.MemberlistConfig.SecretKey = key
-	}
 	if a.config.NodeName != "" {
 		base.NodeName = a.config.NodeName
 	}
@@ -263,21 +258,8 @@ func (a *Agent) consulConfig() *consul.Config {
 func (a *Agent) setupServer() error {
 	config := a.consulConfig()
 
-	// Load a keyring file, if present
+	if err := a.setupKeyrings(config); err != nil {
-	keyfileLAN := filepath.Join(config.DataDir, serfLANKeyring)
+		return fmt.Errorf("Failed to configure keyring: %v", err)
-	if _, err := os.Stat(keyfileLAN); err == nil {
-		config.SerfLANConfig.KeyringFile = keyfileLAN
-	}
-	if err := loadKeyringFile(config.SerfLANConfig); err != nil {
-		return err
-	}
-
-	keyfileWAN := filepath.Join(config.DataDir, serfWANKeyring)
-	if _, err := os.Stat(keyfileWAN); err == nil {
-		config.SerfWANConfig.KeyringFile = keyfileWAN
-	}
-	if err := loadKeyringFile(config.SerfWANConfig); err != nil {
-		return err
 	}
 
 	server, err := consul.NewServer(config)
@@ -292,13 +274,8 @@ func (a *Agent) setupServer() error {
 func (a *Agent) setupClient() error {
 	config := a.consulConfig()
 
-	// Load a keyring file, if present
+	if err := a.setupKeyrings(config); err != nil {
-	keyfileLAN := filepath.Join(config.DataDir, serfLANKeyring)
+		return fmt.Errorf("Failed to configure keyring: %v", err)
-	if _, err := os.Stat(keyfileLAN); err == nil {
-		config.SerfLANConfig.KeyringFile = keyfileLAN
-	}
-	if err := loadKeyringFile(config.SerfLANConfig); err != nil {
-		return err
 	}
 
 	client, err := consul.NewClient(config)
@@ -309,6 +286,47 @@ func (a *Agent) setupClient() error {
 	return nil
 }
 
+// setupKeyrings is used to initialize and load keyrings during agent startup
+func (a *Agent) setupKeyrings(config *consul.Config) error {
+	fileLAN := filepath.Join(a.config.DataDir, serfLANKeyring)
+	fileWAN := filepath.Join(a.config.DataDir, serfWANKeyring)
+
+	if a.config.EncryptKey == "" {
+		goto LOAD
+	}
+	if _, err := os.Stat(fileLAN); err != nil {
+		if err := initKeyring(fileLAN, a.config.EncryptKey); err != nil {
+			return err
+		}
+	}
+	if a.config.Server {
+		if _, err := os.Stat(fileWAN); err != nil {
+			if err := initKeyring(fileWAN, a.config.EncryptKey); err != nil {
+				return err
+			}
+		}
+	}
+
+LOAD:
+	if _, err := os.Stat(fileLAN); err == nil {
+		config.SerfLANConfig.KeyringFile = fileLAN
+	}
+	if err := loadKeyringFile(config.SerfLANConfig); err != nil {
+		return err
+	}
+	if a.config.Server {
+		if _, err := os.Stat(fileWAN); err == nil {
+			config.SerfWANConfig.KeyringFile = fileWAN
+		}
+		if err := loadKeyringFile(config.SerfWANConfig); err != nil {
+			return err
+		}
+	}
+
+	// Success!
+	return nil
+}
+
 // RPC is used to make an RPC call to the Consul servers
 // This allows the agent to implement the Consul.Interface
 func (a *Agent) RPC(method string, args interface{}, reply interface{}) error {

command/agent/agent_test.go

@@ -81,11 +81,11 @@ func makeAgentKeyring(t *testing.T, conf *Config, key string) (string, *Agent) {
 	conf.DataDir = dir
 
 	fileLAN := filepath.Join(dir, serfLANKeyring)
-	if _, err := initKeyring(fileLAN, key); err != nil {
+	if err := initKeyring(fileLAN, key); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 	fileWAN := filepath.Join(dir, serfWANKeyring)
-	if _, err := initKeyring(fileWAN, key); err != nil {
+	if err := initKeyring(fileWAN, key); err != nil {
 		t.Fatalf("err: %s", err)
 	}
 

command/agent/command.go

@@ -154,30 +154,14 @@ func (c *Command) readConfig() *Config {
 			c.Ui.Error(fmt.Sprintf("Invalid encryption key: %s", err))
 			return nil
 		}
-
+		keyfileLAN := filepath.Join(config.DataDir, serfLANKeyring)
-		fileLAN := filepath.Join(config.DataDir, serfLANKeyring)
+		if _, err := os.Stat(keyfileLAN); err == nil {
-		done, err := initKeyring(fileLAN, config.EncryptKey)
+			c.Ui.Error("WARNING: LAN keyring exists but -encrypt given, ignoring")
-		if err != nil {
-			c.Ui.Error(fmt.Sprintf("Error initializing keyring: %s", err))
-			return nil
 		}
-		if !done {
-			c.Ui.Error(fmt.Sprintf(
-				"WARNING: keyring file %s already exists, not overwriting",
-				fileLAN))
-		}
-
 		if config.Server {
-			fileWAN := filepath.Join(config.DataDir, serfWANKeyring)
+			keyfileWAN := filepath.Join(config.DataDir, serfWANKeyring)
-			done, err := initKeyring(fileWAN, config.EncryptKey)
+			if _, err := os.Stat(keyfileWAN); err == nil {
-			if err != nil {
+				c.Ui.Error("WARNING: WAN keyring exists but -encrypt given, ignoring")
-				c.Ui.Error(fmt.Sprintf("Error initializing keyring: %s", err))
-				return nil
-			}
-			if !done {
-				c.Ui.Error(fmt.Sprintf(
-					"WARNING: keyring file %s already exists, not overwriting",
-					fileWAN))
 			}
 		}
 	}

command/agent/keyring.go

@@ -18,42 +18,41 @@ const (
 	serfWANKeyring = "serf/remote.keyring"
 )
 
-// initKeyring will create a keyring file at a given path. Returns whether any
+// initKeyring will create a keyring file at a given path.
-// action was taken and any applicable error.
+func initKeyring(path, key string) error {
-func initKeyring(path, key string) (bool, error) {
 	var keys []string
 
 	if _, err := base64.StdEncoding.DecodeString(key); err != nil {
-		return false, fmt.Errorf("Invalid key: %s", err)
+		return fmt.Errorf("Invalid key: %s", err)
 	}
 
 	// Just exit if the file already exists.
 	if _, err := os.Stat(path); err == nil {
-		return false, nil
+		return nil
 	}
 
 	keys = append(keys, key)
 	keyringBytes, err := json.Marshal(keys)
 	if err != nil {
-		return false, err
+		return err
 	}
 
 	if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil {
-		return false, err
+		return err
 	}
 
 	fh, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0600)
 	if err != nil {
-		return false, err
+		return err
 	}
 	defer fh.Close()
 
 	if _, err := fh.Write(keyringBytes); err != nil {
 		os.Remove(path)
-		return false, err
+		return err
 	}
 
-	return true, nil
+	return nil
 }
 
 // loadKeyringFile will load a gossip encryption keyring out of a file. The file

command/agent/keyring_test.go

@@ -66,7 +66,7 @@ func TestAgent_LoadKeyrings(t *testing.T) {
 		t.Fatalf("keyring should be loaded")
 	}
 	if c.SerfWANConfig.KeyringFile != "" {
-		t.Fatalf("bad: %#v", c.SerfLANConfig.KeyringFile)
+		t.Fatalf("bad: %#v", c.SerfWANConfig.KeyringFile)
 	}
 	if c.SerfWANConfig.MemberlistConfig.Keyring != nil {
 		t.Fatalf("keyring should not be loaded")
@@ -87,13 +87,9 @@ func TestAgent_InitKeyring(t *testing.T) {
 	file := filepath.Join(dir, "keyring")
 
 	// First initialize the keyring
-	done, err := initKeyring(file, key1)
+	if err := initKeyring(file, key1); err != nil {
-	if err != nil {
 		t.Fatalf("err: %s", err)
 	}
-	if !done {
-		t.Fatalf("should have modified keyring")
-	}
 
 	content, err := ioutil.ReadFile(file)
 	if err != nil {
@@ -104,14 +100,11 @@ func TestAgent_InitKeyring(t *testing.T) {
 	}
 
 	// Try initializing again with a different key
-	done, err = initKeyring(file, key2)
+	if err := initKeyring(file, key2); err != nil {
-	if err != nil {
 		t.Fatalf("err: %s", err)
 	}
-	if done {
-		t.Fatalf("should not have modified keyring")
-	}
 
+	// Content should still be the same
 	content, err = ioutil.ReadFile(file)
 	if err != nil {
 		t.Fatalf("err: %s", err)