diff --git a/command/agent/agent_test.go b/command/agent/agent_test.go index 49cd81f5a6b6..f2f39dcb4e72 100644 --- a/command/agent/agent_test.go +++ b/command/agent/agent_test.go @@ -1085,6 +1085,39 @@ func TestServer_Reload_TLS_DowngradeFromTLS(t *testing.T) { assert.True(agent.config.TLSConfig.IsEmpty()) } +func TestServer_Reload_VaultConfig(t *testing.T) { + ci.Parallel(t) + + logger := testlog.HCLogger(t) + + agentConfig := &Config{ + TLSConfig: &config.TLSConfig{}, + Vault: &config.VaultConfig{ + Enabled: pointer.Of(true), + Token: "vault-token", + Namespace: "vault-namespace", + }, + } + + agent := &Agent{ + auditor: &noOpAuditor{}, + logger: logger, + config: agentConfig, + } + + newConfig := &Config{ + TLSConfig: &config.TLSConfig{}, + Vault: &config.VaultConfig{ + Enabled: pointer.Of(true), + Token: "vault-token", + Namespace: "vault-namespace", + }, + } + + must.NoError(t, agent.Reload(newConfig)) + must.Equals(t, agent.config.Vault, newConfig.Vault) +} + func TestServer_ShouldReload_ReturnFalseForNoChanges(t *testing.T) { ci.Parallel(t) assert := assert.New(t) diff --git a/nomad/server_test.go b/nomad/server_test.go index 6e73e5c11c08..c4ea2cfc7424 100644 --- a/nomad/server_test.go +++ b/nomad/server_test.go @@ -214,6 +214,7 @@ func TestServer_Reload_Vault(t *testing.T) { config := DefaultConfig() config.VaultConfig.Enabled = &tr config.VaultConfig.Token = uuid.Generate() + config.VaultConfig.Namespace = "nondefault" if err := s1.Reload(config); err != nil { t.Fatalf("Reload failed: %v", err) diff --git a/nomad/structs/config/vault.go b/nomad/structs/config/vault.go index f3e9b290f3fe..13ffc449a72c 100644 --- a/nomad/structs/config/vault.go +++ b/nomad/structs/config/vault.go @@ -106,14 +106,20 @@ func (c *VaultConfig) AllowsUnauthenticated() bool { func (c *VaultConfig) Merge(b *VaultConfig) *VaultConfig { result := *c + if b.Enabled != nil { + result.Enabled = b.Enabled + } if b.Token != "" { result.Token = b.Token } + if b.Role != "" { + result.Role = b.Role + } if b.Namespace != "" { result.Namespace = b.Namespace } - if b.Role != "" { - result.Role = b.Role + if b.AllowUnauthenticated != nil { + result.AllowUnauthenticated = b.AllowUnauthenticated } if b.TaskTokenTTL != "" { result.TaskTokenTTL = b.TaskTokenTTL @@ -136,17 +142,11 @@ func (c *VaultConfig) Merge(b *VaultConfig) *VaultConfig { if b.TLSKeyFile != "" { result.TLSKeyFile = b.TLSKeyFile } - if b.TLSServerName != "" { - result.TLSServerName = b.TLSServerName - } - if b.AllowUnauthenticated != nil { - result.AllowUnauthenticated = b.AllowUnauthenticated - } if b.TLSSkipVerify != nil { result.TLSSkipVerify = b.TLSSkipVerify } - if b.Enabled != nil { - result.Enabled = b.Enabled + if b.TLSServerName != "" { + result.TLSServerName = b.TLSServerName } return &result @@ -188,9 +188,9 @@ func (c *VaultConfig) Copy() *VaultConfig { return nc } -// IsEqual compares two Vault configurations and returns a boolean indicating +// Equals compares two Vault configurations and returns a boolean indicating // if they are equal. -func (c *VaultConfig) IsEqual(b *VaultConfig) bool { +func (c *VaultConfig) Equals(b *VaultConfig) bool { if c == nil && b != nil { return false } @@ -198,12 +198,32 @@ func (c *VaultConfig) IsEqual(b *VaultConfig) bool { return false } + if c.Enabled == nil || b.Enabled == nil { + if c.Enabled != b.Enabled { + return false + } + } else if *c.Enabled != *b.Enabled { + return false + } + if c.Token != b.Token { return false } if c.Role != b.Role { return false } + if c.Namespace != b.Namespace { + return false + } + + if c.AllowUnauthenticated == nil || b.AllowUnauthenticated == nil { + if c.AllowUnauthenticated != b.AllowUnauthenticated { + return false + } + } else if *c.AllowUnauthenticated != *b.AllowUnauthenticated { + return false + } + if c.TaskTokenTTL != b.TaskTokenTTL { return false } @@ -225,17 +245,12 @@ func (c *VaultConfig) IsEqual(b *VaultConfig) bool { if c.TLSKeyFile != b.TLSKeyFile { return false } - if c.TLSServerName != b.TLSServerName { - return false - } - if c.AllowUnauthenticated != b.AllowUnauthenticated { - return false - } if c.TLSSkipVerify != b.TLSSkipVerify { return false } - if c.Enabled != b.Enabled { + if c.TLSServerName != b.TLSServerName { return false } + return true } diff --git a/nomad/structs/config/vault_test.go b/nomad/structs/config/vault_test.go index f6fa2c6bd4f5..bc8824812001 100644 --- a/nomad/structs/config/vault_test.go +++ b/nomad/structs/config/vault_test.go @@ -3,35 +3,37 @@ package config import ( "reflect" "testing" + "time" + + "github.com/shoenig/test/must" "github.com/hashicorp/nomad/ci" - "github.com/stretchr/testify/require" + "github.com/hashicorp/nomad/helper/pointer" ) func TestVaultConfig_Merge(t *testing.T) { ci.Parallel(t) - trueValue, falseValue := true, false c1 := &VaultConfig{ - Enabled: &falseValue, + Enabled: pointer.Of(false), Token: "1", Role: "1", - AllowUnauthenticated: &trueValue, + AllowUnauthenticated: pointer.Of(true), TaskTokenTTL: "1", Addr: "1", TLSCaFile: "1", TLSCaPath: "1", TLSCertFile: "1", TLSKeyFile: "1", - TLSSkipVerify: &trueValue, + TLSSkipVerify: pointer.Of(true), TLSServerName: "1", } c2 := &VaultConfig{ - Enabled: &trueValue, + Enabled: pointer.Of(true), Token: "2", Role: "2", - AllowUnauthenticated: &falseValue, + AllowUnauthenticated: pointer.Of(false), TaskTokenTTL: "2", Addr: "2", TLSCaFile: "2", @@ -43,17 +45,17 @@ func TestVaultConfig_Merge(t *testing.T) { } e := &VaultConfig{ - Enabled: &trueValue, + Enabled: pointer.Of(true), Token: "2", Role: "2", - AllowUnauthenticated: &falseValue, + AllowUnauthenticated: pointer.Of(false), TaskTokenTTL: "2", Addr: "2", TLSCaFile: "2", TLSCaPath: "2", TLSCertFile: "2", TLSKeyFile: "2", - TLSSkipVerify: &trueValue, + TLSSkipVerify: pointer.Of(true), TLSServerName: "2", } @@ -66,69 +68,75 @@ func TestVaultConfig_Merge(t *testing.T) { func TestVaultConfig_IsEqual(t *testing.T) { ci.Parallel(t) - require := require.New(t) - - trueValue, falseValue := true, false c1 := &VaultConfig{ - Enabled: &falseValue, + Enabled: pointer.Of(false), Token: "1", Role: "1", - AllowUnauthenticated: &trueValue, + Namespace: "1", + AllowUnauthenticated: pointer.Of(true), TaskTokenTTL: "1", Addr: "1", + ConnectionRetryIntv: time.Second, TLSCaFile: "1", TLSCaPath: "1", TLSCertFile: "1", TLSKeyFile: "1", - TLSSkipVerify: &trueValue, + TLSSkipVerify: pointer.Of(true), TLSServerName: "1", } c2 := &VaultConfig{ - Enabled: &falseValue, + Enabled: pointer.Of(false), Token: "1", Role: "1", - AllowUnauthenticated: &trueValue, + Namespace: "1", + AllowUnauthenticated: pointer.Of(true), TaskTokenTTL: "1", Addr: "1", + ConnectionRetryIntv: time.Second, TLSCaFile: "1", TLSCaPath: "1", TLSCertFile: "1", TLSKeyFile: "1", - TLSSkipVerify: &trueValue, + TLSSkipVerify: pointer.Of(true), TLSServerName: "1", } - require.True(c1.IsEqual(c2)) + must.Equals(t, c1, c2) c3 := &VaultConfig{ - Enabled: &trueValue, + Enabled: pointer.Of(true), Token: "1", Role: "1", - AllowUnauthenticated: &trueValue, + Namespace: "1", + AllowUnauthenticated: pointer.Of(true), TaskTokenTTL: "1", Addr: "1", + ConnectionRetryIntv: time.Second, TLSCaFile: "1", TLSCaPath: "1", TLSCertFile: "1", TLSKeyFile: "1", - TLSSkipVerify: &trueValue, + TLSSkipVerify: pointer.Of(true), TLSServerName: "1", } c4 := &VaultConfig{ - Enabled: &falseValue, + Enabled: pointer.Of(false), Token: "1", Role: "1", - AllowUnauthenticated: &trueValue, + Namespace: "1", + AllowUnauthenticated: pointer.Of(true), TaskTokenTTL: "1", Addr: "1", + ConnectionRetryIntv: time.Second, TLSCaFile: "1", TLSCaPath: "1", TLSCertFile: "1", TLSKeyFile: "1", - TLSSkipVerify: &trueValue, + TLSSkipVerify: pointer.Of(true), TLSServerName: "1", } - require.False(c3.IsEqual(c4)) + + must.NotEquals(t, c3, c4) } diff --git a/nomad/vault.go b/nomad/vault.go index 4e3ac0d577b4..7e2550b15801 100644 --- a/nomad/vault.go +++ b/nomad/vault.go @@ -363,7 +363,7 @@ func (v *vaultClient) SetConfig(config *config.VaultConfig) error { defer v.l.Unlock() // If reloading the same config, no-op - if v.config.IsEqual(config) { + if v.config.Equals(config) { return nil }