sam-1112 opened a new pull request, #6441:
URL: https://github.com/apache/datafusion-comet/pull/6441
## Which issue does this PR close?
Closes #6139.
## Rationale for this change
The native Iceberg write gate admits `s3` and `s3a` data locations without
verifying that the effective S3 configuration can be honoured by the native
writer.
Hadoop configuration is reduced to six translated `fs.s3a.*` keys. Other
Hadoop S3A settings, as well as unsupported `s3.*` and `client.*` FileIO
properties, may otherwise be forwarded and silently ignored by iceberg-rust. A
native write could therefore use the wrong credentials or drop settings such as
encryption, ACLs, tags, storage class, access points, or remote signing.
The gate should fail closed. If the native storage path cannot honour an
effective setting, the write must fall back to Iceberg's JVM writer with a safe
diagnostic containing property names only.
## What changes are included in this PR?
* For an `s3` or `s3a` data location, decline the native write when an
effective Hadoop `fs.s3a.*` setting is outside the keys translated by
`NativeConfig`:
* `access.key`
* `secret.key`
* `session.token`
* `endpoint`
* `endpoint.region`
* `path.style.access`
The allow-list is derived directly from
`NativeConfig.s3aSuffixToIcebergGlobalKey` to avoid drift.
* Apply the same rules to `fs.s3a.bucket.<data-bucket>.*` settings.
Per-bucket settings for other buckets do not affect the current write.
* Ignore Hadoop values whose only source is a built-in `*-default.xml`
resource. Values from site XML, Spark configuration, or programmatic
configuration remain effective and are checked.
* Ignore Spark's session-wide S3A vectored-read and
`downgrade.syncable.exceptions` settings because they do not affect an Iceberg
data-file write request.
* Separately inspect `table.io().properties()` and decline the native write
for unsupported `s3.*` or `client.*` properties.
* Allow only properties consumed by the pinned iceberg-rust S3 backend or
Comet's credential bridge. This includes endpoint, region, static and session
credentials, path-style access, supported server-side encryption, assume-role
settings, anonymous/config-chain flags, the Comet credential-provider class,
REST-vended token expiry, and the built-in web-identity settings.
* Validate `s3.sse.type` values. The native backend accepts `none`, `s3`,
`kms`, and `custom`; Iceberg's `dsse-kms` mode falls back because the pinned
iceberg-rust backend cannot honour it.
* When `s3.comet.credential.provider.class` is configured, continue
forwarding vendor-owned `s3.*` and `client.*` properties because the provider
receives the unfiltered FileIO property bag.
Iceberg-defined properties that the native storage path cannot honour
still cause fallback. The Iceberg property names are discovered from the
runtime `S3FileIOProperties` and `AwsClientProperties` classes so newly
introduced Iceberg settings fail closed.
* Report sorted property names only in fallback reasons. Property values,
credentials, and tokens are never included.
* Document the Hadoop and FileIO allow-lists in the user and contributor
guides.
## How are these changes tested?
`CometIcebergWriteDetectionSuite` covers:
* unknown Hadoop and FileIO property names are rejected deterministically
without exposing their values
* the Hadoop allow-list remains aligned with `NativeConfig`
* `fs.s3a.bucket.<data-bucket>.*` settings are checked while settings for
other buckets are ignored
* Hadoop `*-default.xml` values are ignored, while site XML and programmatic
settings remain effective
* unsupported Hadoop S3A settings on an S3 data location fall back
* S3-only gates do not affect local data locations
* unsupported Iceberg FileIO properties fall back
* unsupported `s3.sse.type` values, including `dsse-kms`, fall back
* supported Comet web-identity properties remain eligible
* vendor-owned `s3.*` and `client.*` properties remain eligible when a
custom Comet credential provider is configured
* unsupported Iceberg-defined properties still fall back when a custom
credential provider is configured
* fallback reasons contain sorted property names only and never secret values
The suite was run against all pinned Iceberg profiles:
* Spark 3.4 / Iceberg 1.5.2: 59 passed, 1 canceled because format v3 is
unavailable
* Spark 3.5 / Iceberg 1.8.1: 60 passed
* Spark 4.0 / Iceberg 1.10.0: 60 passed
* Spark 4.1 / Iceberg 1.11.0: 60 passed
--
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]
