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(&region_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(&region_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]

Reply via email to