Skip to content

fix: honor the S3 profile name and file and Hadoop's addressing mode for custom endpoints - #5872

Open
dwsmith1983 wants to merge 1 commit into
apache:mainfrom
dwsmith1983:fix/s3-profile-and-virtual-hosted
Open

fix: honor the S3 profile name and file and Hadoop's addressing mode for custom endpoints#5872
dwsmith1983 wants to merge 1 commit into
apache:mainfrom
dwsmith1983:fix/s3-profile-and-virtual-hosted

Conversation

@dwsmith1983

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Closes #4245, closes #2802.

Rationale for this change

Two gaps in how the native S3 store is configured from fs.s3a.* settings.

The profile credentials provider ignored fs.s3a.auth.profile.name and fs.s3a.auth.profile.file, so a job that selects a named profile or a non-default credentials file on the Hadoop side got the SDK defaults on the native side.

fs.s3a.path.style.access was applied inverted: true set object_store's virtual_hosted_style_request to true and then appended /bucket to the endpoint, which object_store, treating a virtual-hosted endpoint as already containing the bucket, sent as a path-style URL anyway. The net effect was that every custom endpoint was addressed path-style whatever the flag said, and virtual-hosted addressing (bucket.host) was never produced.

What changes are included in this PR?

  • CredentialProviderMetadata::Profile carries name and file, read through the existing per-bucket config lookup with blanks treated as unset, and passed to the SDK builder. The file is loaded in credentials-file format, which is what Hadoop's ProfileAWSCredentialsProvider does; with no file the SDK default applies as before. aws-runtime becomes a direct dependency because the file-kind types re-exported by aws_config are deprecated aliases; it was already in the tree.
  • path.style.access is parsed the way Hadoop's Configuration.getBoolean parses it (default false, non-boolean text falls back to the default), virtual_hosted_style_request is its negation and is always passed, and normalize_endpoint returns the endpoint together with the effective mode so the two cannot disagree: virtual-hosted rebuilds scheme://bucket.host[:port][/path], path-style leaves the endpoint alone for object_store to append the bucket, and an IP-literal host forces path-style the way the AWS SDK's endpoint rules do, so http://127.0.0.1:9000 keeps working without the flag. localhost is not special-cased, matching Hadoop. The s3.amazonaws.com skip is unchanged.
  • The data sources page lists the two profile keys and explains the addressing rule.

Behavior change: a custom fs.s3a.endpoint with fs.s3a.path.style.access unset is now addressed virtual-hosted, as Hadoop S3A addresses it. Deployments on MinIO, Ceph RGW or similar services behind a hostname that relied on the previous always-path-style behavior need fs.s3a.path.style.access=true, which Hadoop already requires for those services; IP-address endpoints keep working either way. Vendor alias schemes are unaffected because the JVM side already synthesizes the flag for them.

How are these changes tested?

52 unit tests in the S3 module, 11 of them written first and failing on the previous code (the profile metadata carried no name or file; path.style.access unset produced no flag; a hostname endpoint was never rewritten; an IP endpoint was rewritten to bucket.127.0.0.1). Coverage: the flag unset, true, false, mixed case with whitespace, and an invalid value; per-bucket override of the flag and of the endpoint, each against a global value set the other way; the s3.amazonaws.com skip in both modes; scheme-less, http://, port, trailing slash, path suffix, an AWS regional host and a dotted bucket name; IPv4 and IPv6 hosts with and without a port; create_store called with a custom endpoint in each mode and with an IP endpoint; profile name only, file only, both, neither, blank, trimmed and per-bucket, plus the provider chain building with the profile provider among others. Two existing tests that had encoded the inverted flag were replaced; one that asserted an empty config now asserts the endpoint key is absent, since the flag is always present.

The create_store calls show object_store accepts each flag and endpoint pair but do not issue a request, since object_store parses the endpoint on first use. The four Scala tests that set an endpoint either set path-style access or are pure config-translation tests, so none needed changing. Full core crate suite passes, clippy and fmt clean.

…for custom endpoints

The profile credentials provider ignored fs.s3a.auth.profile.name and
fs.s3a.auth.profile.file, and fs.s3a.path.style.access was applied
inverted, so every custom endpoint was addressed path-style whatever the
flag said and virtual-hosted addressing was never produced.

Carry the profile name and file into the SDK builder, derive the
virtual-hosted flag from the path-style setting the way Hadoop does,
rebuild the endpoint as bucket.host for virtual-hosted addressing while
forcing path-style for IP-literal hosts as the AWS SDK does, and return
the effective mode with the endpoint so the two cannot disagree.

Closes apache#4245
Closes apache#2802
@github-actions github-actions Bot added bug Something isn't working area:scan Parquet scan / data reading labels Sep 12, 2026

@sunchao sunchao left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1917adca against base db790673. One verified P2 finding: the new default addressing mode breaks HTTPS buckets whose names contain dots.

Correctness

Previously, the native S3 configuration ignored the two profile override keys and inverted fs.s3a.path.style.access. This change carries the selected profile name/file through provider metadata, preserves bucket-specific precedence, and returns an endpoint together with its effective addressing mode. The maintained Spark 3.5/4.0 sources pass spark.hadoop.* values into Hadoop configuration. Comet's existing prefix-based extraction carries these S3 options through to native code. The new boolean parsing matches Hadoop's trimmed, case-insensitive true/false handling and default-false behavior.

The addressing change needs one correction before merge. Setting virtual_hosted_style_request = !path_style_access forces virtual hosting for dotted bucket names over HTTPS, including ordinary AWS endpoints with no override. For review.dotted.bucket, the resulting host is review.dotted.bucket.s3.us-east-1.amazonaws.com. The AWS SDK chooses the path-style URL instead, because the dotted hostname does not match S3's wildcard certificate. The inline P2 requests the same eligibility check for default and custom HTTPS endpoints.

I reproduced the endpoint difference without credentials or storage requests. A Rust harness using the HEAD configuration expressions and the local object_store 0.13.2 endpoint expression produced the dotted hostname. The actual offline AWS Java SDK 2.29.52 endpoint resolver, the version declared by Hadoop 3.4.2, selected path style. Normal bucket names, explicit path style and synthetic profile/bucket precedence checks behaved as expected. These are isolated configuration checks, not native/JNI or live S3 tests.

At 2026-09-12 20:03:22 UTC, only the label check succeeded. CI, CodeQL, the Delta gate and title validation were awaiting workflow approval. The cached merge has the assigned base/head parents and HEAD's tree, but no product CI execution can be credited. The author's reported 52 S3 tests/full-core pass was not rerun locally. Exact locked aws-config 1.12.0 and aws-runtime 1.9.2 source was unavailable locally, so SDK profile-file loading and refresh remain unverified. Maintained Spark 3.4/4.1 source gaps also remain. The final publication check confirmed that the head, base and discussion were unchanged after the temporary API rate limit cleared.

Performance

The added profile string handling and URL parsing occur during store/provider construction. The existing store cache includes the full configuration hash, so changing a profile name or file selects a different cached store. This PR does not add work to the object-read loop or alter credential expiry caching.

The endpoint result keeps normalization to one pass, and the profile description allocates only during construction-time logging. I found no separate verified performance issue. No benchmark or speedup claim is established by this review, and an expression microbenchmark is not applicable to this configuration change.

Design

Returning the normalized endpoint and addressing mode together is a useful safeguard against the original disagreement between those two values. The missing piece is deciding whether a bucket is eligible for virtual hosting before producing either result. That decision must also run when the endpoint is omitted, where object_store constructs the normal AWS URL.

Profile name and file are independently optional, with bucket values taking precedence over global values. The PR preserves the existing provider chain and expiry wrapper. Its metadata tests demonstrate option selection but do not establish actual SDK file precedence or credential renewal. The review therefore keeps those validation boundaries explicit.

Abstraction & complexity

The small NormalizedEndpoint type and blank-filtering helper are proportionate to the change. Configuration lookup remains centralized, and the new direct aws-runtime dependency supplies file-kind types already present in the dependency graph.

No broader provider or endpoint framework is needed. The actionable change is to extend the addressing decision with HTTPS bucket eligibility and test the final URL, rather than relying on a configuration-map assertion or a builder that has not issued a request.

// and treats non-boolean text as that default. object_store expects the inverse flag.
let path_style_access = get_config_trimmed(configs, bucket, "path.style.access")
.is_some_and(|value| value.eq_ignore_ascii_case("true"));
let mut virtual_hosted_style_request = !path_style_access;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correctness

[P2] Preserve path-style addressing for dotted HTTPS buckets

Could we apply the AWS SDK's virtual-host eligibility rules before enabling this flag? With fs.s3a.endpoint.region=us-east-1 and path.style.access unset or false, a bucket such as review.dotted.bucket now becomes https://review.dotted.bucket.s3.us-east-1.amazonaws.com. BASE used path-style addressing, and the AWS SDK endpoint resolver still selects https://s3.us-east-1.amazonaws.com/review.dotted.bucket for this case. The dotted host does not match S3's wildcard TLS certificate, so this breaks native reads of otherwise valid buckets. The same eligibility issue exists for custom HTTPS endpoints. Please retain path-style addressing for dotted HTTPS buckets, including when no custom endpoint is configured, and add assertions on the resulting request URL. The current dotted-bucket test asserts the virtual-hosted string, which misses this regression.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:scan Parquet scan / data reading bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support fs.s3a.auth.profile.name and fs.s3a.auth.profile.file for ProfileCredentialsProvider virtual_hosted_style_request bad calculation

2 participants