feat(catalog-rest): introduce AuthManager/AuthSession, rework SigV4 as a wrapping auth manager#2815
Draft
plusplusjiajia wants to merge 1 commit into
Draft
feat(catalog-rest): introduce AuthManager/AuthSession, rework SigV4 as a wrapping auth manager#2815plusplusjiajia wants to merge 1 commit into
plusplusjiajia wants to merge 1 commit into
Conversation
plusplusjiajia
force-pushed
the
feat/rest-auth-manager
branch
from
e58189f to
9b56016
Compare
plusplusjiajia
force-pushed
the
feat/rest-auth-manager
branch
from
9b56016 to
e1ba91d
Compare
plusplusjiajia
force-pushed
the
feat/rest-auth-manager
branch
from
e1ba91d to
a97a247
Compare
CTTY
reviewed
Jul 14, 2026
CTTY
left a comment
CTTY
left a comment
Collaborator
There was a problem hiding this comment.
Hey @plusplusjiajia , thanks for this contribution! I really like the general direction. To move forward, could you break this down to several smaller PRs? For example you can break it down to 2-3 PRs:
- Introduce AuthManager/Session traits and migrate existing oauth2 to use the new traits
- Add Sigv4 support
|
|
||
| /// How the payload hash is encoded in the `x-amz-content-sha256` header. | ||
| #[derive(Clone, Copy, Debug, PartialEq, Eq)] | ||
| pub enum PayloadHashMode { |
Collaborator
There was a problem hiding this comment.
these should live in sigv4.rs?
| warehouse: None, | ||
| props: HashMap::new(), | ||
| client: None, | ||
| custom_auth_manager: None, |
Collaborator
There was a problem hiding this comment.
let's just name it auth_manager
| pub(crate) async fn token(&self) -> Option<String> { | ||
| let mut req = self | ||
| .request(Method::GET, &self.token_endpoint) | ||
| .request(Method::GET, "http://localhost/unused") |
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
2 participants
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.
Prototype for the
AuthManagerdirection discussed in the review of #2660 (see the inline thread there). This draft includes #2660's SigV4 commits squashed in.What it does
Mirrors Java's AuthManager API:
AuthManager/AuthSessiontraits in a newauth/module:init_session()serves theGET /v1/confighandshake,catalog_session(merged_props)serves everything after, so a manager can rebuild itssession from server-merged properties.
Noop/OAuth2/SigV4managers, selected via a newrest.auth.typeproperty (the legacyrest.sigv4-enabledstill works), injectable throughRestCatalogBuilder::with_auth_manager.HttpClientintoOAuth2Manager, with the cached token surviving the config handshake.SigV4Managerwraps a delegate session instead of being mutually exclusive with token auth: the delegate'sAuthorizationheader is relocated toX-Iceberg-Authorizationbefore signing, so it is covered bythe signature — matching the Java
RESTSigV4AuthSessionbehavior.rest.signing-*from the config response is honored when building the post-handshake session;rest.auth.sigv4.delegate-auth-type(none/oauth2) explicitly selects the wrapped auth,defaulting to inference from token/credential presence;
OAuth2Manageris publicly constructible (new()+with_*).Java references:
org.apache.iceberg.rest.auth(AuthManager/AuthSession) andRESTSigV4AuthSession.Known prototype simplifications