Skip to content

feat(rest): enable SigV4 authentication from catalog properties - #2660

Draft
plusplusjiajia wants to merge 2 commits into
apache:mainfrom
plusplusjiajia:feat/rest-sigv4
Draft

plusplusjiajia wants to merge 2 commits into
apache:mainfrom
plusplusjiajia:feat/rest-sigv4

Conversation

@plusplusjiajia

@plusplusjiajia plusplusjiajia commented Jun 16, 2026 •

Copy link
Copy Markdown
Member

Which issue does this PR close?

Depends on #3092 and should merge after it. The manager and session implementation comes from that PR.

What changes are included in this PR?

Allows REST catalogs to select SigV4 with rest.auth.type=sigv4 when the sigv4 feature is enabled. Supports the legacy rest.sigv4-enabled switch and rest.auth.sigv4.delegate-auth-type (oauth2 by default, or none). Explicitly injected auth managers take precedence.

Are these changes tested?

Mock-server tests cover signed config handshakes, server-provided tokens, contextual credentials and redirects. Additional tests cover delegate selection, the legacy switch, manager overrides and the feature requirement.

@dannycjones

Copy link
Copy Markdown
Contributor

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.

@plusplusjiajia

plusplusjiajia commented Jun 17, 2026 •

Copy link
Copy Markdown
Member Author

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'm keen to help get SigV4 support landed either way. Since #2311 is further along, I'm happy to fold the base64 support into it rather than duplicate — or carry this one forward if you'd prefer. Whatever helps move it forward.

@CTTY

CTTY commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator

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 AuthManager so we have a clean interface before diving into the specific implementation.

Would be happy to hear more thoughts on this

Comment thread crates/catalog/rest/src/catalog.rs Outdated
}

/// Injects a custom request signer, overriding the `rest.sigv4-*` configuration.
pub fn with_signer(mut self, signer: Arc<dyn HttpRequestSigner>) -> Self {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need a more general design rather than just a signer. some authentication mechanism is token-based.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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,
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't know this detail until this PR. Thanks for capturing this!

@plusplusjiajia

Copy link
Copy Markdown
Member Author

Update: this PR will be reworked in place as a SigV4AuthManager on top of #2838 (implementation ready locally, tracking that PR's review). Will push once #2838 merges.

@plusplusjiajia plusplusjiajia changed the title feat(rest): support AWS SigV4 request signing for the REST catalog feat(rest): add SigV4 auth manager for the REST catalog Aug 5, 2026
@plusplusjiajia

Copy link
Copy Markdown
Member Author

Pushed the rework as planned — stacked on #2838 for now (see the note in the description; only the last commit is this PR). Will rebase to a single commit once #2838 merges.

@plusplusjiajia

Copy link
Copy Markdown
Member Author

@CTTY #2838 (#2838) has landed; this is now reworked into a SigV4AuthManager that wraps a delegate session, using the API from that PR. Could you take another look when you get a chance?

@plusplusjiajia
plusplusjiajia requested a review from CTTY August 14, 2026 15:23
@plusplusjiajia
plusplusjiajia force-pushed the feat/rest-sigv4 branch 3 times, most recently from b5e9930 to 83967aa Compare August 20, 2026 13:31

@CTTY CTTY left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks for the contribution, I want to move this faster but this PR is quite large and complicated, is there any way that we can break it down to smaller pieces?

As of now, the most concerning part to me is the custom signer. I think maintaining a custom signer in iceberg repo will be very hard. Have we explore any other alternatives? One idea I had was wrapping the aws_sigv4 signer with custom logic to adapt to the java behavior.

// configured `header.authorization` in place.
if !req.headers().contains_key(http::header::AUTHORIZATION) {
req.headers_mut().insert(http::header::AUTHORIZATION, value);
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be put under a different PR?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CTTY Happy to split if you'd prefer, though neither this nor the reordering it goes with is observable on its own — I reverted both and only the SigV4 test fails; for an OAuth2 or Noop session a configured header.authorization wins either way. They only matter once a session needs the final header set.

/// AWS SigV4 signer following Iceberg Java's `RESTSigV4AuthSession`: it adds the
/// required amz headers and signs all request headers except a small blacklist.
#[derive(Clone)]
pub struct SigV4Signer {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that there are behavior differences between java and rust aws sdks, but I'm still hesitant of maintaining a handrolled sigv4 signer in the iceberg repo. Have we explored other alternatives?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@CTTY Good call — it uses the aws-sigv4 crate now, which is already in the workspace lock via aws-config. PayloadChecksumKind::NoHeader keeps the base64 x-amz-content-sha256 header while the crate hashes the body in hex for the canonical request, and the Aws4Signer defaults are just settings. I also split the signing out into #3082 (#3082) so it can be reviewed on its own — this PR keeps the manager and the wiring.

@plusplusjiajia
plusplusjiajia force-pushed the feat/rest-sigv4 branch 5 times, most recently from 49de7bd to 2da1846 Compare October 10, 2026 02:32
@plusplusjiajia
plusplusjiajia marked this pull request as draft October 10, 2026 02:34
@plusplusjiajia plusplusjiajia changed the title feat(rest): add SigV4 auth manager for the REST catalog feat(rest): enable SigV4 authentication from catalog properties Oct 10, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants