kazuyukitanimura opened a new pull request, #7026:
URL: https://github.com/apache/datafusion-comet/pull/7026
## Which issue does this PR close?
Closes #5662.
Closes #5658.
## Rationale for this change
NativeConfig.extractObjectStoreOptions forwards fs.s3a.* options to the
native S3 client. Comet's native client resolves several of them differently
from Hadoop's
S3AFileSystem: lookupPassword bucket precedence, JCEKS credential aliases,
the clear-text fallback, assumed-role session policies, provider class
semantics, and
scheme-less endpoints. A plain Parquet read through the regular native
scan could therefore run under a different identity or endpoint than Spark
would, silently.
The Delta contrib (#5365) already has a comparator that detects these
cases and declines to Spark. It lived in DeltaScanSupport, so the regular scan
could not use
it. This PR moves it next to the object store configuration code (#5658)
and applies it to every native scan that forwards S3 options (#5662).
## What changes are included in this PR?
- New org.apache.comet.objectstore.S3ConfigGate.
- It holds the encryption, proxy, assumed-role-policy, endpoint,
config-divergence and provider-class checks, moved out of DeltaScanSupport.
- The code is otherwise unchanged. The decline text now says "Native
scan …" instead of "Native Delta scan …".
- There is one entry point, S3ConfigGate.declineReason(hadoopConf,
uris), which returns a decline reason or None.
- It does nothing for URIs that are not s3/s3a.
- Applied to regular scans. NativeConfig.extractScanObjectStoreOptions
runs the gate on one URI per S3 bucket and returns Left(reason).
CometNativeScan.convert
already falls back to Spark on Left and tags the reason. The CSV native
scan calls the same method, so it gets the check too.
- Delta delegates to the gate.
- DeltaScanSupport.declineReason makes one S3ConfigGate.declineReason
call in place of the six separate gate calls, in the same order.
- CometDeltaNativeScan.convert passes verifyS3Config = false, since the
claim gate already ran the check over a superset of those files.
- Comet credential-provider SPI. When
fs.s3a.comet.credential.provider.class resolves non-blank for a bucket, native
never reads fs.s3a.aws.credentials.provider or
the assumed-role settings. For those buckets the provider-class and
assumed-role-policy checks are skipped. Proxy, endpoint, encryption and
divergence checks still
apply. Without this, the documented built-in adapters would have started
falling back.
- Native key-sync test. The s3.rs test that checks the compared key list
against NATIVE_S3A_CONFIG_PROPERTIES now parses S3ConfigGate.scala. It is
renamed
gate_compares_every_native_s3a_property.
- Tests moved. The S3-gate unit tests moved from DeltaScanContribSuite to
the new core S3ConfigGateSuite, so they run in the PR tier instead of only
under -Pdelta.
The suite is registered in pr_build_linux.yml and pr_build_macos.yml.
- Docs. datasources.md has a new S3 limitation listing when a scan falls
back to Spark. s3-credential-providers.md no longer says an unsupported
provider only fails
the scan.
Not covered, and left for follow-ups: native Parquet writes, the Iceberg
scan, and opted-in S3-compliant alias schemes (fs.comet.s3Compliant.schemes)
still forward
fs.s3a.* options without this check.
## How are these changes tested?
- S3ConfigGateSuite (moved). The existing Delta gate tests, about 100, now
run in core against real Hadoop code (S3AUtils, real JCEKS keystores). The
discovery-harness tests that check the key list against s3.rs run rather
than cancel.
- S3ConfigGateSuite (new). Four tests cover the credential-provider SPI. A
Hadoop-only provider chain is admitted when the SPI is set, per bucket. A blank
per-bucket
value opts out, and the proxy decline is not relaxed.
- NativeConfigSuite (new).
- A proxy conf declines, and the reason names the bucket but not the
host.
- The diverging bucket is the one named.
- verifyS3Config = false skips the check.
- Non-S3 schemes are untouched.
- A MinIO-style conf (static keys, http:// endpoint, path style, region)
still goes native.
- Run locally. S3ConfigGateSuite, NativeConfigSuite and
AbfsAuthResolverSuite: 174 passed, 0 failed (2 Azure tests cancelled). The
s3.rs tests pass: 47. mvn -o
spotless:apply is clean and dev/ci/check-suites.py reports no missing
suites.
- Not run locally.
- The contrib/delta-spark module can't be built here, because the Delta
artifacts aren't in the local Maven cache. The Delta edits were checked by
reading only.
Please apply the run-delta-tests label.
- No Spark-session test asserts the end-to-end fallback for S3, because
Spark-session suites can't run in my sandbox. run-all-spark-profiles covers the
other Spark
profiles.
--
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]
