Sensible keyring error #7272
Merged
Sensible keyring error #7272
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes #7231. Before an agent would always emit a warning when there is an encrypt key in the configuration and an existing keyring stored, which is happening on restart. Now it only emits that warning when the encrypt key from the configuration is not part of the keyring.
hanshasselberg
added
the
waiting-reply
|
test failures are related. will fix. |
ghost
removed
the
waiting-reply
mkeeler
approved these changes
Feb 13, 2020
agent/agent.go
Outdated
| config.SerfLANConfig.MemberlistConfig.Keyring, | ||
| a.config.EncryptKey, | ||
| ) { | ||
| a.logger.Warn("LAN" + msg) |
There was a problem hiding this comment.
nit:
I think this would be more consistent with existing logs if it were:
a.logger.Warn(msg, "keyring", "WAN")
During the hclog refactor it looked like most of the places that we had previously formatted a log message we turned into having a constant string message and passing the other data to the logger as kv pairs.
The same thing goes for the log for the "LAN" case too. If you really prefer this form, then thats fine with me too. My opinions on the subject are not very strong either way.
There was a problem hiding this comment.
Thank you Matt! This was actually a thing I kept wondering how to do with the new logger! I like your suggestion and will go with it!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #7231. Before an agent would always emit a warning when there is
an encrypt key in the configuration and an existing keyring stored,
which is happening on restart.
Now it only emits that warning when the encrypt key from the
configuration is not part of the keyring.
I feel a little bad, because
setupBaseKeyringsis not pretty and I would like to make it better. But that would mean more changes that don't contribute to the fix at hand. Will leave that for later.