parthchandra opened a new pull request, #6023: URL: https://github.com/apache/datafusion-comet/pull/6023
### Which issue does this PR close? Closes #6022. ### Rationale for this change Comet's native Parquet reader only understands a fixed list of S3 credential provider class names and rejects anything else with `Unsupported credential provider`, so config that works in plain Spark can fail under Comet. Comet already has the `CometS3CredentialProvider` SPI to run JVM credential code from native, but ships nothing that uses it. This PR adds two ready-made implementations that adapt the standard, already-deployed provider types onto that SPI, so the fix for the failure is a one-line config change with the user's existing credential config left untouched. ### What changes are included in this PR? Two new user-facing classes under `org.apache.comet.cloud.s3`: - **`HadoopS3ACredentialProviderAdapter`** (recommended) — hands credential building to Hadoop S3A's own code, so it accepts everything the `fs.s3a.aws.credentials.provider` chain accepts (the default chain, web-identity, assumed-role, custom signers, per-bucket config). Needs no new config; it reads the existing `fs.s3a.aws.credentials.provider`. - **`AwsSdkCredentialProviderAdapter`** — wraps a single raw AWS SDK provider named in a new `fs.s3a.comet.credential.adapter.class` key. Each adapter has two bodies behind the same class name: an AWS SDK v1 body for the Spark 3.4/3.5 builds and a v2 body for Spark 4.0+, picked automatically by Comet's existing per-profile source sets. Users configure one class name and get the right one for their build. Supporting changes: - **Native**: on the Parquet path, forward the `fs.s3a.*` config to the SPI's `initialize()` (it used to get an empty map), minus the static-credential secrets (`*.access.key`, `*.secret.key`, `*.session.token`), so the adapters can read the provider list and rebuild a Hadoop `Configuration`. - **Build**: add `hadoop-aws` and the matching AWS SDK at `provided` scope, versioned per Spark profile. Nothing new is bundled. - **Docs**: user guide section for the adapters (pointing the `Unsupported credential provider` failure at the Hadoop adapter), plus design-doc and versioning-policy notes. Scope: Parquet native scan only. The existing Rust provider list stays as-is for the cases it already handles; the adapters are opt-in (naming one is what turns it on). The Iceberg path, making the adapter a default, and exact Spark error-message parity are out of scope. Example — leave your credential config alone and just name the adapter: ``` spark.hadoop.fs.s3a.comet.credential.provider.class=org.apache.comet.cloud.s3.HadoopS3ACredentialProviderAdapter spark.hadoop.fs.s3a.aws.credentials.provider=com.amazonaws.auth.DefaultAWSCredentialsProviderChain ``` ### How are these changes tested? - **Rust unit test** for the config forwarding (keeps `fs.s3a.*`, drops secrets and out-of-scope keys). - **JVM unit tests** (per SDK version): credential extraction (basic, session, with/without expiry); adapter delegate instantiation across each constructor/factory path; and the error paths (delegate class missing, wrong type, not named). - **MinIO end-to-end test** on the native Parquet path: the Hadoop adapter delegates to the AWS default chain — a class Comet's native list deliberately rejects — and the read succeeds, which it can only do if the adapter resolved credentials. This is the exact regression from the issue. - The public-API guard suite was updated for the two new `@Public` classes. -- 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]
