vault: detect namespace change in config reload (#14298)

The `namespace` field was not included in the equality check between old and new
Vault configurations, which meant that a Vault config change that only changed
the namespace would not be detected as a change and the clients would not be
reloaded.

Also, the comparison for boolean fields such as `enabled` and
`allow_unauthenticated` was on the pointer and not the value of that pointer,
which results in spurious reloads in real config reload that is easily missed in
typical test scenarios.

Includes a minor refactor of the order of fields for `Copy` and `Merge` to match
the struct fields in hopes it makes it harder to make this mistake in the
future, as well as additional test coverage.
This commit is contained in:
Tim Gross
2022-08-24 17:03:29 -04:00
committed by GitHub
parent ee501f4f7d
commit e886d5d055
7 changed files with 135 additions and 50 deletions

View File

@@ -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)
agent := NewTestAgent(t, t.Name(), func(c *Config) {
c.Server.NumSchedulers = pointer.Of(0)
c.Vault = &config.VaultConfig{
Enabled: pointer.Of(true),
Token: "vault-token",
Namespace: "vault-namespace",
Addr: "https://vault.consul:8200",
}
})
defer agent.Shutdown()
newConfig := agent.GetConfig().Copy()
newConfig.Vault = &config.VaultConfig{
Enabled: pointer.Of(true),
Token: "vault-token",
Namespace: "another-namespace",
Addr: "https://vault.consul:8200",
}
sconf, err := convertServerConfig(newConfig)
must.NoError(t, err)
agent.finalizeServerConfig(sconf)
// TODO: the vault client isn't accessible here, and we don't actually
// overwrite the agent's server config on reload. We probably should? See
// tests in nomad/server_test.go for verification of this code path's
// behavior on the VaultClient
must.NoError(t, agent.server.Reload(sconf))
}
func TestServer_ShouldReload_ReturnFalseForNoChanges(t *testing.T) {
ci.Parallel(t)
assert := assert.New(t)