snmvaughan opened a new issue, #6462: URL: https://github.com/apache/datafusion-comet/issues/6462
**Problem.** [#6031](https://github.com/apache/datafusion-comet/pull/6031) (for [#6207](https://github.com/apache/datafusion-comet/issues/6207)) gave native Parquet reads one credential per policy location through `CometS3LocationScopedCredentialProvider` and left the Iceberg path unchanged. On that path a provider still gets one credential per table. `load_file_io` builds each `FileIO` with a credential bridge bound to one reference path. For scans that is the table's metadata location (`iceberg_scan.rs:181`); for writes it is the data location (`iceberg_write.rs:484`). The bridge sends that bucket and path on every `getCredentialsForPath` call (`iceberg_common.rs:351`). iceberg-rust then attaches that same loader to the operator it builds for every file (`s3_config_build` in `iceberg-storage-opendal`). As a result, every data and delete file a scan reads, and every file a write produces, is signed with the reference path's credential. That is correct when one policy covers everything a table touches. It is wrong when a table's files span locations with different policies: - Data files outside the table location. This includes `write.data.path`, the object-storage layout, and files added by `add_files`, `migrate` or `snapshot` that stay where they were. - A narrower policy nested under the table location, for example a separate policy for `warehouse/db/t/data/region=eu`. - Data files in another bucket, because the bridge's bucket is also fixed by the reference path. Reads of those files fail with 403 when the reference credential does not cover them. When a broader credential does cover a path that a narrower policy restricts, the read succeeds even though the provider's own per-path answer for that file would refuse it. **Proposal.** Honor `getPolicyLocations` on the Iceberg path too. No new API is needed. - When the configured provider is location-scoped, `storage_factory_for` returns a Comet `StorageFactory` that wraps `OpenDalStorageFactory::S3`, the same way `BlobHostPromotingS3StorageFactory` does today. - Every iceberg-rust `Storage` method receives the path it operates on. The wrapper routes each call by that path to the longest covering location in the path's bucket, using the routing `LocationScopedObjectStore` already has. It then delegates to an inner OpenDAL S3 storage whose loader is a bridge bound to that location. Inner storages are built on first use. - `new_input` and `new_output` return files bound to the wrapper, so every read goes through the routing. - A read that fails with 403 refreshes the bucket's locations, at most once per snapshot. It retries if the path now routes to a different location, as on the Parquet path. On this path a provider exception reaches S3 as an unsigned request, so a 403 is the signal for both cases. Writes and deletes route by path without retry. - Providers that implement only the base interface keep today's behavior. **Compatibility.** This adds no public types or methods, but it changes documented behavior. The `CometS3LocationScopedCredentialProvider` Javadoc and the user guide both say Iceberg reads do not use locations. After this change, location-scoped providers receive: - `getPolicyLocations` calls for the buckets Iceberg reads and writes touch; - location paths in `getCredentialsForPath` calls from the Iceberg path. Both fall within the existing contract: a location's credential must authorize every path the location is the longest match for, and `getPolicyLocations` may be called from any thread on the driver or executors. Maintainers may still prefer to put the change behind a config for one release. **Related.** [#6207](https://github.com/apache/datafusion-comet/issues/6207) and [#6031](https://github.com/apache/datafusion-comet/pull/6031) cover the Parquet path. [#6293](https://github.com/apache/datafusion-comet/issues/6293) covers blocking JVM calls on Tokio workers; some of the `getPolicyLocations` and bridge calls this adds would run inside async `Storage` methods. A design sketch follows in a comment. -- 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]
