sunchao commented on code in PR #5872:
URL: https://github.com/apache/datafusion-comet/pull/5872#discussion_r4171571171
##########
native/core/src/parquet/objectstore/s3.rs:
##########
@@ -1000,11 +1174,80 @@ impl CredentialProviderMetadata {
}
}
+/// The STS region the Java SDK falls back to for a role profile when no
region is found.
+const STS_FALLBACK_REGION: &str = "us-east-1";
+
+/// Builds the profile credentials provider on `provider_config`, with
`default_region` standing
+/// in for the SDK's default region chain.
+async fn build_profile_provider(
+ provider_config: ProviderConfig,
+ default_region: &impl ProvideRegion,
+ name: Option<&str>,
+ file: Option<&str>,
+ credentials_only: bool,
+) -> ProfileFileCredentialsProvider {
+ // Hadoop's ProfileAWSCredentialsProvider loads the configured file, or
the shared
+ // credentials file, as a credentials-format file and reads nothing else,
so a same-name
+ // role profile in the SDK's config file never applies.
+ let credentials_file = match (file, credentials_only) {
+ (Some(file), _) => Some(file.to_string()),
+ (None, true) => Some(default_shared_credentials_file(
+ std::env::var("AWS_SHARED_CREDENTIALS_FILE").ok(),
+ std::env::var("HOME").ok(),
+ )),
+ (None, false) => None,
+ };
+ let profile_files = credentials_file.map(|file| {
+ EnvConfigFiles::builder()
+ .with_file(EnvConfigFileKind::Credentials, file)
+ .build()
+ });
+ let region = if credentials_only {
+ // The Java SDK sends a role profile's STS request to the profile's
own `region`, then
+ // to its default region chain's, then to us-east-1. The region
provider here also
+ // tries the profile's `source_profile` chain before the default
chain. A file that
+ // fails to load yields no region here and surfaces from the
credentials provider.
+ let mut region_provider =
ProfileFileRegionProvider::builder().configure(&provider_config);
+ if let Some(name) = name {
+ region_provider = region_provider.profile_name(name);
+ }
+ if let Some(files) = &profile_files {
+ region_provider = region_provider.profile_files(files.clone());
+ }
+ // The default chain can probe IMDS, so it runs only when the profile
has no region.
+ let region = match
ProvideRegion::region(®ion_provider.build()).await {
+ Some(region) => region,
+ None => default_region
+ .region()
+ .await
+ .unwrap_or_else(|| Region::from_static(STS_FALLBACK_REGION)),
Review Comment:
[P2] Could we retain Hadoop’s global STS endpoint when neither the selected
profile nor the default chain supplies a region? For an assume-role credentials
file without region properties, empty SDK defaults and IMDS disabled, Hadoop’s
Java provider explicitly uses `https://sts.amazonaws.com`, signed for
`us-east-1`. Substituting only `us-east-1` here makes the Rust provider request
`https://sts.us-east-1.amazonaws.com` instead. A deployment whose egress
permits the global endpoint can therefore authenticate through Hadoop but fail
native reads. Preserve the global endpoint override as well as the signing
region specifically for this fallback.
Evidence: The same current-source Rust probe recorded
`https://sts.us-east-1.amazonaws.com/` for the no-region fixture. Java SDK
2.29.52’s actual provider reported `region=us-east-1` and
`endpointOverride=Optional[https://sts.amazonaws.com]`, matching
`StsProfileCredentialsProviderFactory.configureEndpoint`. Restricting the
in-memory connector to the Hadoop-selected endpoint caused native credential
resolution to fail. The explicit profile-region control succeeded. Evidence is
in `/tmp/comet5872-db7a9-probe/native.log`, `java.log`, and `restricted.log`;
no external STS requests were made.
##########
native/core/src/parquet/objectstore/s3.rs:
##########
@@ -1000,11 +1174,80 @@ impl CredentialProviderMetadata {
}
}
+/// The STS region the Java SDK falls back to for a role profile when no
region is found.
+const STS_FALLBACK_REGION: &str = "us-east-1";
+
+/// Builds the profile credentials provider on `provider_config`, with
`default_region` standing
+/// in for the SDK's default region chain.
+async fn build_profile_provider(
+ provider_config: ProviderConfig,
+ default_region: &impl ProvideRegion,
+ name: Option<&str>,
+ file: Option<&str>,
+ credentials_only: bool,
+) -> ProfileFileCredentialsProvider {
+ // Hadoop's ProfileAWSCredentialsProvider loads the configured file, or
the shared
+ // credentials file, as a credentials-format file and reads nothing else,
so a same-name
+ // role profile in the SDK's config file never applies.
+ let credentials_file = match (file, credentials_only) {
+ (Some(file), _) => Some(file.to_string()),
+ (None, true) => Some(default_shared_credentials_file(
+ std::env::var("AWS_SHARED_CREDENTIALS_FILE").ok(),
+ std::env::var("HOME").ok(),
+ )),
+ (None, false) => None,
+ };
+ let profile_files = credentials_file.map(|file| {
+ EnvConfigFiles::builder()
+ .with_file(EnvConfigFileKind::Credentials, file)
+ .build()
+ });
+ let region = if credentials_only {
+ // The Java SDK sends a role profile's STS request to the profile's
own `region`, then
+ // to its default region chain's, then to us-east-1. The region
provider here also
+ // tries the profile's `source_profile` chain before the default
chain. A file that
+ // fails to load yields no region here and surfaces from the
credentials provider.
+ let mut region_provider =
ProfileFileRegionProvider::builder().configure(&provider_config);
+ if let Some(name) = name {
+ region_provider = region_provider.profile_name(name);
+ }
+ if let Some(files) = &profile_files {
+ region_provider = region_provider.profile_files(files.clone());
+ }
+ // The default chain can probe IMDS, so it runs only when the profile
has no region.
+ let region = match
ProvideRegion::region(®ion_provider.build()).await {
Review Comment:
[P2] Could we preserve Hadoop’s per-role STS region selection instead of
resolving one region through `source_profile` and applying it to the whole
provider? With a selected `[analytics]` role lacking `region`, a static
`[source]` containing `region=eu-west-1`, and the default region chain
returning `eu-central-1`, Hadoop requests `sts.eu-central-1.amazonaws.com`.
This code instead requests `sts.eu-west-1.amazonaws.com` and never consults the
default chain. A multi-role chain also loses individual regions: an
intermediate role configured for `eu-central-1` is requested in the selected
outer role’s `us-west-2`. These differences break credential resolution where
the required STS endpoint or region is restricted. Resolve each role’s own
region followed by Hadoop’s default chain, without inheriting the source
profile’s region.
Evidence: An executable probe extracted `build_profile_provider` verbatim
from this head and used locked `aws-config 1.12.0`/`aws-runtime 1.9.2` with an
in-memory STS connector. The conflicting-source case requested
`https://sts.eu-west-1.amazonaws.com/` with zero default-chain calls. The
two-role case requested `us-west-2` twice. Instantiating Java SDK 2.29.52’s
actual `StsProfileCredentialsProvider` selected `eu-central-1` for the
corresponding default-chain and intermediate-role cases. The synthetic
endpoint-restricted connector produced credential-resolution failures.
Reproduction and outputs: `/tmp/comet5872-db7a9-probe/main.rs`,
`JavaRegionOracle.java`, `native.log`, `java.log`, and `restricted.log`.
--
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]