Reload VaultConfig if CAFile, CertFile, KeyFile have changed

Fixes hashicorp#4395, hashicorp#6052

nomad/structs/config/vault.go

@@ -74,6 +74,9 @@ type VaultConfig struct {
 
 	// TLSServerName, if set, is used to set the SNI host when connecting via TLS.
 	TLSServerName string `mapstructure:"tls_server_name"`
+
+	// Checksum is a MD5 hash of the TLSCaFile, TLSCertFile, and TLSKeyFile.
+	Checksum string
 }
 
 // DefaultVaultConfig() returns the canonical defaults for the Nomad
@@ -184,52 +187,75 @@ func (c *VaultConfig) Copy() *VaultConfig {
 
 // IsEqual compares two Vault configurations and returns a boolean indicating
 // if they are equal.
-func (a *VaultConfig) IsEqual(b *VaultConfig) bool {
+func (a *VaultConfig) IsEqual(b *VaultConfig) (bool, error) {
 	if a == nil && b != nil {
-		return false
+		return false, nil
 	}
 	if a != nil && b == nil {
-		return false
+		return false, nil
 	}
 
 	if a.Token != b.Token {
-		return false
+		return false, nil
 	}
 	if a.Role != b.Role {
-		return false
+		return false, nil
 	}
 	if a.TaskTokenTTL != b.TaskTokenTTL {
-		return false
+		return false, nil
 	}
 	if a.Addr != b.Addr {
-		return false
+		return false, nil
 	}
 	if a.ConnectionRetryIntv.Nanoseconds() != b.ConnectionRetryIntv.Nanoseconds() {
-		return false
+		return false, nil
 	}
 	if a.TLSCaFile != b.TLSCaFile {
-		return false
+		return false, nil
 	}
 	if a.TLSCaPath != b.TLSCaPath {
-		return false
+		return false, nil
 	}
 	if a.TLSCertFile != b.TLSCertFile {
-		return false
+		return false, nil
 	}
 	if a.TLSKeyFile != b.TLSKeyFile {
-		return false
+		return false, nil
 	}
 	if a.TLSServerName != b.TLSServerName {
-		return false
+		return false, nil
 	}
 	if a.AllowUnauthenticated != b.AllowUnauthenticated {
-		return false
+		return false, nil
 	}
 	if a.TLSSkipVerify != b.TLSSkipVerify {
-		return false
+		return false, nil
 	}
 	if a.Enabled != b.Enabled {
-		return false
+		return false, nil
+	}
+
+	if a.Checksum == "" {
+		if err := a.SetChecksum(); err != nil {
+			return true, err
+		}
+	}
+
+	if b.Checksum == "" {
+		if err := b.SetChecksum(); err != nil {
+			return true, err
+		}
 	}
-	return true
+	return a.Checksum == b.Checksum, nil
+}
+
+// SetChecksum generates and sets the checksum for a Vault configuration.
+func (a *VaultConfig) SetChecksum() error {
+	newChecksum, err := createChecksumOfFiles(a.TLSCaFile, a.TLSCertFile, a.TLSKeyFile)
+	if err != nil {
+		return err
+	}
+
+	a.Checksum = newChecksum
+	return nil
 }

nomad/structs/config/vault_test.go

@@ -62,68 +62,107 @@ func TestVaultConfig_Merge(t *testing.T) {
 
 func TestVaultConfig_IsEqual(t *testing.T) {
 	require := require.New(t)
-
 	trueValue, falseValue := true, false
-	c1 := &VaultConfig{
-		Enabled:              &falseValue,
-		Token:                "1",
-		Role:                 "1",
-		AllowUnauthenticated: &trueValue,
-		TaskTokenTTL:         "1",
-		Addr:                 "1",
-		TLSCaFile:            "1",
-		TLSCaPath:            "1",
-		TLSCertFile:          "1",
-		TLSKeyFile:           "1",
-		TLSSkipVerify:        &trueValue,
-		TLSServerName:        "1",
-	}
 
-	c2 := &VaultConfig{
-		Enabled:              &falseValue,
-		Token:                "1",
-		Role:                 "1",
-		AllowUnauthenticated: &trueValue,
-		TaskTokenTTL:         "1",
-		Addr:                 "1",
-		TLSCaFile:            "1",
-		TLSCaPath:            "1",
-		TLSCertFile:          "1",
-		TLSKeyFile:           "1",
-		TLSSkipVerify:        &trueValue,
-		TLSServerName:        "1",
+	const (
+		cafile   = "../../../helper/tlsutil/testdata/ca.pem"
+		cafile2  = "../../../helper/tlsutil/testdata/global-ca.pem"
+		foocert  = "../../../helper/tlsutil/testdata/nomad-foo.pem"
+		fookey   = "../../../helper/tlsutil/testdata/nomad-foo-key.pem"
+		foocert2 = "../../../helper/tlsutil/testdata/nomad-bad.pem"
+		fookey2  = "../../../helper/tlsutil/testdata/nomad-bad-key.pem"
+	)
+
+	// Assert mistmatching certificates, key files are considered unequal
+	{
+		a := &VaultConfig{
+			Enabled:     &falseValue,
+			TLSCaFile:   caFile,
+			TLSCertFile: foocert,
+			TLSKeyFile:  fookey,
+		}
+
+		b := &VaultConfig{
+			Enabled:     &falseValue,
+			TLSCaFile:   caFile,
+			TLSCertFile: foocert2,
+			TLSKeyFile:  fookey2,
+		}
+		isEqual, err := a.IsEqual(b)
+		require.Nil(err)
+		require.False(isEqual)
 	}
 
-	require.True(c1.IsEqual(c2))
+	// Assert mismatching CA files are considered unequal
+	{
+		a := &VaultConfig{
+			Enabled:     &falseValue,
+			TLSCaFile:   caFile,
+			TLSCertFile: foocert,
+			TLSKeyFile:  fookey,
+		}
+		b := &VaultConfig{
+			Enabled:     &falseValue,
+			TLSCaFile:   cafile2,
+			TLSCertFile: foocert,
+			TLSKeyFile:  fookey,
+		}
+		isEqual, err := a.IsEqual(b)
+		require.Nil(err)
+		require.False(isEqual)
+	}
 
-	c3 := &VaultConfig{
-		Enabled:              &trueValue,
-		Token:                "1",
-		Role:                 "1",
-		AllowUnauthenticated: &trueValue,
-		TaskTokenTTL:         "1",
-		Addr:                 "1",
-		TLSCaFile:            "1",
-		TLSCaPath:            "1",
-		TLSCertFile:          "1",
-		TLSKeyFile:           "1",
-		TLSSkipVerify:        &trueValue,
-		TLSServerName:        "1",
+	// Assert that invalid files return an error
+	{
+		a := &VaultConfig{
+			Enabled:     &falseValue,
+			TLSCaFile:   "1",
+			TLSCertFile: "1",
+			TLSKeyFile:  "1",
+		}
+		b := &VaultConfig{
+			Enabled:     &falseValue,
+			TLSCaFile:   "2",
+			TLSCertFile: "2",
+			TLSKeyFile:  "2",
+		}
+		isEqual, err := a.IsEqual(b)
+		require.NotNil(err)
+		require.False(isEqual)
 	}
 
-	c4 := &VaultConfig{
-		Enabled:              &falseValue,
-		Token:                "1",
-		Role:                 "1",
-		AllowUnauthenticated: &trueValue,
-		TaskTokenTTL:         "1",
-		Addr:                 "1",
-		TLSCaFile:            "1",
-		TLSCaPath:            "1",
-		TLSCertFile:          "1",
-		TLSKeyFile:           "1",
-		TLSSkipVerify:        &trueValue,
-		TLSServerName:        "1",
+	// Assert configs are equal when everything are equal, including CAFile, CertFile, KeyFile
+	{
+		a := &VaultConfig{
+			Enabled:              &falseValue,
+			Token:                "1",
+			Role:                 "1",
+			AllowUnauthenticated: &trueValue,
+			TaskTokenTTL:         "1",
+			Addr:                 "1",
+			TLSCaFile:            caFile,
+			TLSCaPath:            "1",
+			TLSCertFile:          foocert,
+			TLSKeyFile:           fookey,
+			TLSSkipVerify:        &trueValue,
+			TLSServerName:        "1",
+		}
+		b := &VaultConfig{
+			Enabled:              &falseValue,
+			Token:                "1",
+			Role:                 "1",
+			AllowUnauthenticated: &trueValue,
+			TaskTokenTTL:         "1",
+			Addr:                 "1",
+			TLSCaFile:            caFile,
+			TLSCaPath:            "1",
+			TLSCertFile:          foocert,
+			TLSKeyFile:           fookey,
+			TLSSkipVerify:        &trueValue,
+			TLSServerName:        "1",
+		}
+		isEqual, err := a.IsEqual(b)
+		require.NotNil(err)
+		require.True(isEqual)
 	}
-	require.False(c3.IsEqual(c4))
 }

nomad/vault.go

@@ -317,7 +317,10 @@ func (v *vaultClient) SetConfig(config *config.VaultConfig) error {
 	defer v.l.Unlock()
 
 	// If reloading the same config, no-op
-	if v.config.IsEqual(config) {
+	isEqual, err := v.config.IsEqual(config)
+	if err != nil
+		return err
+	} else if isEqual {
 		return nil
 	}