plusplusjiajia commented on code in PR #2660:
URL: https://github.com/apache/iceberg-rust/pull/2660#discussion_r3564795062
##########
crates/catalog/rest/src/catalog.rs:
##########
@@ -145,6 +159,12 @@ impl RestCatalogBuilder {
self.config.client = Some(client);
self
}
+
+ /// Injects a custom request signer, overriding the `rest.sigv4-*`
configuration.
+ pub fn with_signer(mut self, signer: Arc<dyn HttpRequestSigner>) -> Self {
Review Comment:
@CTTY I dug into how Java structures this, and I think your point is well
taken: OAuth2 token handling is hardcoded inside `HttpClient` today, and this
PR adds a second, parallel mechanism that is mutually exclusive with token
auth. In Java the two compose — `SigV4AuthManager` wraps a delegate session,
relocates its `Authorization` header to `X-Iceberg-Authorization`, then signs —
so SigV4-over-OAuth2 is a real combination the current design can't express.
Is something along these lines what you had in mind, mirroring Java's
`AuthManager`/`AuthSession`?
```rust
#[async_trait]
pub trait AuthManager: Debug + Send + Sync {
/// Session used for catalog-level requests.
async fn catalog_session(
&self,
props: &HashMap<String, String>,
) -> Result<Arc<dyn AuthSession>>;
// room to grow, matching Java: init_session() for the config
// handshake, table_session() for table-scoped auth, close().
}
#[async_trait]
pub trait AuthSession: Debug + Send + Sync {
/// Applies authentication to an outgoing request (headers, signing,
...).
async fn authenticate(&self, request: &mut reqwest::Request) ->
Result<()>;
}
```
with `NoopAuthManager` / `OAuth2Manager` (existing logic extracted, behavior
unchanged) / `SigV4AuthManager` (wrapping a delegate, Java-style) as the
initial implementations, selected via `rest.auth.type` or injected through the
builder.
If that matches your intuition, my instinct would be to land the interface
plus the OAuth2 extraction as a small standalone refactor first, then rework
this PR on top as the SigV4 implementation — which would also give #2311 a
common landing spot.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
