-
Notifications
You must be signed in to change notification settings - Fork 27
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
Add federation token endpoint in Director, and implement routine for caches to fetch one #1985
Merged
jhiemstrawisc
merged 11 commits into
PelicanPlatform:main
from
jhiemstrawisc:issue-1912
Feb 21, 2025
Merged
Add federation token endpoint in Director, and implement routine for caches to fetch one #1985
jhiemstrawisc
merged 11 commits into
PelicanPlatform:main
from
jhiemstrawisc:issue-1912
Feb 21, 2025
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
Okay, I think I provided a sensible set of tests in the latest round of commits. @h2zh, let's discuss this in person to make sure you understand the overall goals and scope of this PR -- I think that'll be important for a good review. |
9344a4f
to
3eb2939
Compare
h2zh
requested changes
Feb 18, 2025
This new endpoint allows caches or origins to get a federation-signed token to provide other federation services that may require proof of federation membership. The immediate usecase is for caches to provide Origins that haven't enabled DirectReads, although I think this may also be useful for a future where we support third-party copies between origins. I tried to add a little bit of flexibility to the endpoint to support that use case, even though we don't have the immediate need. The authz scheme here is that a server requesting a fed token must provide its hostname, server type and a valid advertisement token to the Director. The Director then verifies the advertisement token, which is de facto proof of federation membership, and if valid issues a personally-signed token.
This is still missing a lot of necessary testing, but I'm worked into a hole the way I started implementing this that makes sorting out cyclic imports hard. Rather than fight through that now, I decided to commit what I have so others can comment on the overall approach.
Previously I grabbed a default configuration from `Director.FedTokenLifetime`, but that bothered me because we can tell the token's actual lifetime just by looking at it. So that's what I do now. However, I'm punting on dynamic adjustment of this refresh cycle, which might be needed if the Director ever starts issuing shorter-lived tokens.
These use the new e2e fed test package so that I can spin up an entire federation while still importing individual components of packages that are also used by the federation. In particular, these tests probe Director/Registry/Cache interactions to make sure the whole chain works, as well as making sure the cache's fed token maintenance thread handles updating tokens based on their detected lifetimes. I'm punting on any dynamic adjustment of the period caches request this token, e.g. in the case the Director reboots with a much shorter fed token lifetime.
cd26b70
to
02fe7cb
Compare
h2zh
approved these changes
Feb 21, 2025
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.
🚀
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
cache
Issue relating to the cache component
director
Issue relating to the director component
enhancement
New feature or request
security
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.
I believe this implementation accomplishes the goal (and I can observe it working when I set things up locally), but there's still a lot of unit testing to do. I worked myself into a hole with the approach that now makes testing very hard due to cyclic imports.
Rather than finish all the refactoring to fix that up, I decided I'd open a draft to have others comment on the approach first. If the token management/verification checks out, I'll end up spending some time untangling things so I can run this through a proper test that spins up an entire fed-in-a-box.
Note that I started down this road because I assume Origins will also need fed tokens eventually, especially for third-party copies from origin to origin.