feat(rest): add SigV4 auth manager for the REST catalog - #2660
feat(rest): add SigV4 auth manager for the REST catalog#2660plusplusjiajia wants to merge 1 commit into
Conversation
|
There's a PR open for SigV4 signing, is this picking up from that one? #2311 I ask as its had a few rounds of feedback already. |
@dannycjones Thanks for the pointer — I'd missed #2311, just took a look and compared the two. Mine isn't based on it: it follows Iceberg Java's RESTSigV4AuthSessionRESTSigV4AuthSession(apache/iceberg#11995) and the merged iceberg-cpp version(apache/iceberg-cpp#616). The main difference I see is the base64-encoded x-amz-content-sha256 convention (the Java behavior), which #2311's hex-only signing doesn't cover and which some REST servers require. |
|
I haven't thought quite clearly about this part yet, my general intuition is that it would be better to start from something like a Would be happy to hear more thoughts on this |
| } | ||
|
|
||
| /// Injects a custom request signer, overriding the `rest.sigv4-*` configuration. | ||
| pub fn with_signer(mut self, signer: Arc<dyn HttpRequestSigner>) -> Self { |
There was a problem hiding this comment.
I think we need a more general design rather than just a signer. some authentication mechanism is token-based.
There was a problem hiding this 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?
#[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.
There was a problem hiding this comment.
Prototype is up as a draft PR: #2815 — full AuthManager/AuthSession shape with Noop/OAuth2/SigV4 managers; details and known simplifications in the PR description.
| IcebergRest, | ||
| /// Standard AWS SigV4 style: hex everywhere (e.g. AWS Glue). | ||
| StandardAws, | ||
| } |
There was a problem hiding this comment.
I didn't know this detail until this PR. Thanks for capturing this!
f79d5dd to
a4c4d8e
Compare
a4c4d8e to
d141c70
Compare
1fa95f5 to
0f10a28
Compare
0f10a28 to
fcc77e1
Compare
Which issue does this PR close?
Follow-up to #2838, split from the #2815 prototype per review.
What changes are included in this PR?
SigV4 as a
SigV4AuthManagerwrapping a delegate auth manager, per #2838's design:Authorizationis relocated toOriginal-Authorization(Java convention) and included in the signature, then the request is SigV4-signed.SigV4Signerfollows AWS canonical-request rules with Java parity (path normalization + double encoding, all-headers-minus-blacklist,IcebergRest/StandardAwspayload-hash modes).AwsProperties(rest.signing-region,rest.signing-namedefaultexecute-api,rest.access-key-id/rest.secret-access-key/rest.session-token, single-source credential resolution). Delegate defaults tooauth2; an injected signer is never rebuilt from server properties.Three behavior changes worth calling out, all for Java parity:
header.authorizationstill wins, because the OAuth2 session now only sets the header when absent — as Java'sOAuth2Util.AuthSessiondoes withputIfAbsent.x-amz-date,x-amz-content-sha256,x-amz-security-token) is relocated toOriginal-*rather than dropped, matching Java'supdateRequestHeaders. Relocated values are marked sensitive.Original-Authorizationorx-amz-security-token.