-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reload VaultConfig if CAFile, CertFile, KeyFile have changed #6677
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -78,6 +78,9 @@ type VaultConfig struct { | |
|
||
// TLSServerName, if set, is used to set the SNI host when connecting via TLS. | ||
TLSServerName string `hcl:"tls_server_name"` | ||
|
||
// Checksum is a MD5 hash of the TLSCaFile, TLSCertFile, and TLSKeyFile. | ||
Checksum string | ||
} | ||
|
||
// DefaultVaultConfig() returns the canonical defaults for the Nomad | ||
|
@@ -191,52 +194,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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Populating We probably should call |
||
} | ||
|
||
// SetChecksum generates and sets the checksum for a Vault configuration. | ||
func (a *VaultConfig) SetChecksum() error { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This probably should be called by |
||
newChecksum, err := createChecksumOfFiles(a.TLSCaFile, a.TLSCertFile, a.TLSKeyFile) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
a.Checksum = newChecksum | ||
return nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feels like we should use false as the fail value. Besides being the zero value, if we were to accidentally ignore the returned error, we'd fail by refreshing vault config, which is a noop if files didn't actually change.
Noticed that the TLS config has that behavior in
nomad/nomad/structs/config/tls.go
Lines 253 to 260 in 8e4a5f1