Ensure certificates retrieved through the cache get persisted with auto-config #8409
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
mkeeler
force-pushed
the
feature/auto-config/persist-certs
branch
2 times, most recently
from
4d21148
to
12ca9c2
Compare
crhino
reviewed
Jul 30, 2020
|
I agree that having the |
mkeeler
force-pushed
the
feature/auto-config/persist-certs
branch
from
12ca9c2
to
0a4fcaa
Compare
|
I have that refactoring about half way done but ran into now having a dependency on the agent/router.Router or agent/router.Manager so I need to move that into a shared component at the agent level passed down into the server/client. That is going to take more work so I quickly put this together to solve the problem in a less nice way. |
mkeeler
added a commit
that referenced
this pull request
Jul 30, 2020
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.
When implementing auto-config certificate generation without auto-encrypt I missed a crucial feature which is that certificates for auto-config must be persisted to allow for restarts to function properly after the agent has existed for 3 days. The initial certificates get persisted but I wasn't updating the persisted configuration once new certificates were retrieved.
This PR implements that persistence.
In the near future I am going to be doing more refactoring and merging. The auto-encrypt code will be consumed by the auto-config packages as well as the cert-monitor package. Basically, with this PR I don't love how the persistence has to happen through a series of callbacks. I think with that the conclusion I have come to is that the code in the cert-monitor package really needs to be a part of the auto-config package and subsequently so should the auto-encrypt code currently living on the Client struct. That is a much larger change and more work than would be able to be accomplished before the 1.8.1 release. Outward behavior that a user can observe is correct and will not change even with needing to refactor this to not be quite so bad next week.