Skip to content

Commit 6b0c853

Browse files
committed
Address PR feedback.
1 parent f08805e commit 6b0c853

2 files changed

Lines changed: 27 additions & 25 deletions

File tree

native/core/src/cloud/s3/credential_bridge.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,11 @@ pub enum AccessMode {
6161
/// identity for the `(provider_class, dispatch_key, catalog_properties)` triple returned by
6262
/// `ensureInitialized`. `bucket_jstr` / `path_jstr` are interned once at construction to avoid
6363
/// per-call `new_string` allocations on the hot path.
64+
///
65+
/// Granularity: although the JVM SPI accepts `(bucket, path)`, neither
66+
/// `object_store::CredentialProvider::get_credential` nor
67+
/// `reqsign_core::ProvideCredential::provide_credential` carries a per-request path, so the
68+
/// effective identity is per-bucket (Parquet) or per-table-location (Iceberg).
6469
pub struct CometS3CredentialBridge {
6570
provider_class: String,
6671
dispatch_key: String,

native/core/src/parquet/objectstore/s3.rs

Lines changed: 22 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -81,32 +81,29 @@ pub fn create_store(
8181

8282
// Parquet path: catalog_properties is empty; vendors here read from Hadoop conf.
8383
let empty_props: HashMap<String, String> = HashMap::new();
84-
let bridge = match lookup_provider_class(configs, bucket) {
85-
Some(provider_class) => match CometS3CredentialBridge::new(
86-
provider_class,
87-
bucket,
88-
bucket,
89-
url.path(),
90-
AccessMode::Read,
91-
&empty_props,
92-
) {
93-
Ok(b) => Some(b),
94-
Err(e) => {
95-
log::warn!(
96-
"Failed to initialize CometS3CredentialBridge for {bucket}: {e}; \
97-
falling back to default credential chain"
98-
);
99-
None
84+
builder = match lookup_provider_class(configs, bucket) {
85+
Some(provider_class) => {
86+
// Fail rather than fall back to the default chain, which could resolve to the wrong
87+
// identity for a user who explicitly named a provider.
88+
let bridge = CometS3CredentialBridge::new(
89+
provider_class,
90+
bucket,
91+
bucket,
92+
url.path(),
93+
AccessMode::Read,
94+
&empty_props,
95+
)
96+
.map_err(|e| object_store::Error::Generic {
97+
store: "S3",
98+
source: format!("CometS3CredentialBridge init failed for {bucket}: {e}").into(),
99+
})?;
100+
builder.with_credentials(Arc::new(bridge))
101+
}
102+
None => {
103+
match get_runtime().block_on(build_credential_provider(configs, bucket, min_ttl))? {
104+
Some(provider) => builder.with_credentials(Arc::new(provider)),
105+
None => builder.with_skip_signature(true),
100106
}
101-
},
102-
None => None,
103-
};
104-
builder = if let Some(bridge) = bridge {
105-
builder.with_credentials(Arc::new(bridge))
106-
} else {
107-
match get_runtime().block_on(build_credential_provider(configs, bucket, min_ttl))? {
108-
Some(provider) => builder.with_credentials(Arc::new(provider)),
109-
None => builder.with_skip_signature(true),
110107
}
111108
};
112109

0 commit comments

Comments
 (0)