kaveti opened a new pull request, #15752: URL: https://github.com/apache/iceberg/pull/15752
## What [RESTSessionCatalog.newFileIO()](cci:1://file:///Users/rkaveti/Documents/rkaveti/iceberg/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java:1175:2-1197:3) has two code paths for creating a `FileIO`: 1. **ioBuilder path** — used when a custom `ioBuilder` is provided (e.g. by Trino) 2. **Reflection path** — used when `ioBuilder` is null, delegates to `CatalogUtil.loadFileIO()` The reflection path correctly passes storage credentials (vended via [LoadTableResponse](cci:2://file:///Users/rkaveti/Documents/rkaveti/iceberg/core/src/main/java/org/apache/iceberg/rest/responses/LoadTableResponse.java:40:0-141:1)) to `FileIO` implementations that implement `SupportsStorageCredentials`. The `ioBuilder` path, however, completely ignores the `storageCredentials` parameter — silently discarding them. ## Why this matters This was introduced in #12591, which added storage credentials V3 support but only wired up credential passing for the reflection path. The `ioBuilder` path was missed because Trino is currently the only engine that uses it. In practice, this means that when a REST catalog server vends storage credentials (e.g. short-lived S3/GCS/ADLS tokens), any engine using a custom `ioBuilder` never receives them. For Trino specifically, this blocks the ability to use vended credentials with their custom `FileIO` — which is the subject of ongoing work in [trinodb/trino#28425](https://github.com/trinodb/trino/pull/28425). This was [confirmed as unintentional](https://github.com/trinodb/trino/pull/28425#discussion_r2980583819) by @nastra. ## The fix After `ioBuilder.apply(context, properties)` creates the `FileIO`, we now check if the instance implements `SupportsStorageCredentials` and call [setCredentials()](cci:1://file:///Users/rkaveti/Documents/rkaveti/iceberg/core/src/test/java/org/apache/iceberg/TestCatalogUtil.java:572:4-575:5) with the converted credentials — matching exactly what `CatalogUtil.loadFileIO()` already does at lines 418–419. The change is 8 lines of logic, non-breaking, and covers all 5 call sites that flow through [tableFileIO()](cci:1://file:///Users/rkaveti/Documents/rkaveti/iceberg/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java:1199:2-1208:3) → [newFileIO()](cci:1://file:///Users/rkaveti/Documents/rkaveti/iceberg/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java:1175:2-1197:3): - [loadTable()](cci:1://file:///Users/rkaveti/Documents/rkaveti/iceberg/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java:445:2-558:3) - [registerTable()](cci:1://file:///Users/rkaveti/Documents/rkaveti/iceberg/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java:670:2-720:3) - `Builder.load()` - [Builder.createTransaction()](cci:1://file:///Users/rkaveti/Documents/rkaveti/iceberg/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java:991:4-1019:5) - [Builder.replaceTransaction()](cci:1://file:///Users/rkaveti/Documents/rkaveti/iceberg/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java:1021:4-1084:5) -- 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]
