[auth] Hot-reload registry auth via nydusd config API#718
Open
Fricounet wants to merge 1 commit intocontainerd:mainfrom
Open
[auth] Hot-reload registry auth via nydusd config API#718Fricounet wants to merge 1 commit intocontainerd:mainfrom
Fricounet wants to merge 1 commit intocontainerd:mainfrom
Conversation
Hook together the token renewal subsystem with the nydusd config API. Move renewal orchestration from pkg/auth to snapshot to avoid circular dependencies as the renewal logic now needs access to the dameons client while the daemon package already indirectly imports the auth package. pkg/auth/renewal.go becomes a pure credential cache with exported InitCredentialStore, RenewCredential, and EvictStaleCredentials. The reconciliation loop formerly in auth/renewal.go and now in snapshot/renewal.go walks all managers->daemons->rags and renews credentials, and hot-reloads daemons directly. It also updates the daemon config file so that they can use fresh creds when they restart.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #718 +/- ##
==========================================
+ Coverage 22.02% 23.25% +1.22%
==========================================
Files 130 132 +2
Lines 11931 12109 +178
==========================================
+ Hits 2628 2816 +188
+ Misses 8960 8946 -14
- Partials 343 347 +4
🚀 New features to boost your workflow:
|
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Overview
Please briefly describe the changes your pull request makes.
Hook together the token renewal subsystem with the nydusd config API so that nydusd credentials can be actively renewed.
Note that during my tests I realized that there's a little quirk in the hot-reload api of nydusd. Nydusd will keep a local cache of its last working token which means that even if we update the config's token, it won't get picked up until there's a 401 error on the cached token. It is not ideal because we could avoid this 401 call by proactively updating the cached token but it's not a huge deal so I think we can proceed with this change regardless.
I've opened #1893 to decide what to do regarding this on the nydusd side.
Related Issues
Please link to the relevant issue. For example:
Fix #123orRelated #456.Fixes #690
Change Details
Please describe your changes in detail:
Move renewal orchestration from pkg/auth to snapshot to avoid circular dependencies as the renewal logic now needs access to the dameons client while the daemon package already indirectly imports the auth package.
pkg/auth/renewal.go becomes a pure credential cache with exported InitCredentialStore, RenewCredential, and EvictStaleCredentials. The reconciliation loop formerly in auth/renewal.go and now in snapshot/renewal.go walks all managers->daemons->rags and renews credentials, and hot-reloads daemons directly. It also updates the daemon config file so that they can use fresh creds when they restart.
I'll work on #709 afterwards to avoid writing the auth config to disk anymore
Test Results
If you have any relevant screenshots or videos that can help illustrate your changes, please add them here.
Tested locally with a private ECR. When the renewal thread kicks in, it finds new creds and correctly calls nydusd to update them:
Change Type
Please select the type of change your pull request relates to:
Self-Checklist
Before submitting a pull request, please ensure you have completed the following: