[#12949] fix(iceberg-rest): Forward access-delegation header on federated loadTable - #12950
Open
lasdf1234 wants to merge 7 commits into
Open
[#12949] fix(iceberg-rest): Forward access-delegation header on federated loadTable#12950lasdf1234 wants to merge 7 commits into
lasdf1234 wants to merge 7 commits into
Conversation
… federated loadTable REST-backend Iceberg catalogs ignored credential vending on load, so remote catalogs never returned storage-credentials. Co-authored-by: Cursor <cursoragent@cursor.com>
Code Coverage Report
Files
|
…ames Co-authored-by: Cursor <cursoragent@cursor.com>
…d keep refs credentials Always load federated tables via REST GET, forward access-delegation on create/register when vending, and preserve storage-credentials when filtering snapshots=refs. Co-authored-by: Cursor <cursoragent@cursor.com>
…y methods Co-authored-by: Cursor <cursoragent@cursor.com>
…r via REST Route createTable and registerTable through the same REST path as loadTable so requestCredential only toggles the access-delegation header. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
roryqi
self-requested a review
September 7, 2026 11:31
…m loadTable config Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
|
Did we test them by hand? We need some integration test cases. |
Collaborator
Author
I'll test it use my real environment.If we use IT to test,it'll cost so mutch time. |
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
3 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.
What changes were proposed in this pull request?
Forward
X-Iceberg-Access-Delegation: vended-credentialswhen a federated Iceberg REST catalog (catalog-backend: rest) loads a table, and rewrite remote credential refresh endpoints to this IRC catalog.FederatedCatalogWrapper.loadTablepreviously ignoredrequestCredentialand used Iceberg'sRESTCatalog.loadTable, which does not send the access-delegation header. Scan-plan federation already forwarded the header; load table now uses the same authenticated REST GET path.Why are the changes needed?
Fix: #12949
A REST-backend Iceberg catalog dropped the client's credential-vending request. The near-end IRC logged
credential vending: true, but the forwarded load arrived at the remote catalog withaccess delegation: null. Direct loads against the remote returnedstorage-credentials; federated loads returned only metadata, so engines failed on the first data read.Does this PR introduce any user-facing change?
Yes. Clients that send
X-Iceberg-Access-Delegation: vended-credentialsthrough a federated Iceberg REST catalog now receive remotestorage-credentialsonloadTable, matching a direct load against the remote catalog.No new APIs or property keys.
How was this patch tested?
./gradlew :iceberg:iceberg-rest-server:test --tests org.apache.gravitino.iceberg.service.TestCatalogWrapperForREST --tests org.apache.gravitino.iceberg.service.TestIcebergRESTUtils -PskipITsMade with Cursor