Skip to content

Commit 726bd10

Browse files
committed
feat(storage): global S3 credential-chain switches
1 parent eab3cea commit 726bd10

File tree

7 files changed

+81
-20
lines changed

7 files changed

+81
-20
lines changed

src/common/storage/src/config.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,31 @@ pub struct StorageConfig {
4444
pub num_cpus: u64,
4545
pub allow_insecure: bool,
4646
pub params: StorageParams,
47+
/// Global switches that affect AWS S3 credential chain behavior.
48+
///
49+
/// Notes:
50+
/// - These are runtime-only controls and are not persisted in meta.
51+
/// - They apply to all S3 operators created in this process.
52+
pub s3_disable_config_load: bool,
53+
pub s3_disable_ec2_metadata: bool,
54+
}
55+
56+
/// Runtime-only switches for AWS S3 credential chain behavior.
57+
#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)]
58+
pub struct S3CredentialChainConfig {
59+
pub disable_config_load: bool,
60+
pub disable_ec2_metadata: bool,
61+
}
62+
63+
impl S3CredentialChainConfig {
64+
pub fn init(cfg: S3CredentialChainConfig) -> databend_common_exception::Result<()> {
65+
GlobalInstance::set(cfg);
66+
Ok(())
67+
}
68+
69+
pub fn try_get() -> Option<S3CredentialChainConfig> {
70+
GlobalInstance::try_get()
71+
}
4772
}
4873

4974
// TODO: This config should be moved out of common-storage crate.

src/common/storage/src/operator.rs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ use crate::http_client::get_storage_http_client;
6767
use crate::metrics_layer::METRICS_LAYER;
6868
use crate::operator_cache::get_operator_cache;
6969
use crate::runtime_layer::RuntimeLayer;
70+
use crate::config::S3CredentialChainConfig;
7071

7172
static METRIC_OPENDAL_RETRIES_COUNT: LazyLock<FamilyCounter<Vec<(&'static str, String)>>> =
7273
LazyLock::new(|| register_counter_family("opendal_retries_count"));
@@ -416,9 +417,18 @@ fn init_s3_operator(cfg: &StorageS3Config) -> Result<impl Builder> {
416417
builder = builder.default_storage_class(cfg.storage_class.to_string().as_ref())
417418
}
418419

419-
// Disable credential loader
420+
// `disable_credential_loader` disables the ambient credential chain entirely.
421+
// When it is enabled, only explicit credentials are used and requests can fall back to unsigned.
420422
if cfg.disable_credential_loader {
421423
builder = builder.disable_config_load().disable_ec2_metadata();
424+
} else if let Some(global) = S3CredentialChainConfig::try_get() {
425+
// Apply global limits to the credential chain.
426+
if global.disable_config_load {
427+
builder = builder.disable_config_load();
428+
}
429+
if global.disable_ec2_metadata {
430+
builder = builder.disable_ec2_metadata();
431+
}
422432
}
423433

424434
// If credential loading is disabled and no credentials are provided, use unsigned requests.
@@ -569,6 +579,10 @@ impl DataOperator {
569579
conf: &StorageConfig,
570580
spill_params: Option<StorageParams>,
571581
) -> databend_common_exception::Result<()> {
582+
S3CredentialChainConfig::init(S3CredentialChainConfig {
583+
disable_config_load: conf.s3_disable_config_load,
584+
disable_ec2_metadata: conf.s3_disable_ec2_metadata,
585+
})?;
572586
GlobalInstance::set(Self::try_create(conf, spill_params).await?);
573587

574588
Ok(())

src/common/storage/src/stage.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,12 +90,12 @@ impl StageFileInfo {
9090

9191
pub fn init_stage_operator(stage_info: &StageInfo) -> Result<Operator> {
9292
if stage_info.stage_type == StageType::External {
93-
// External S3 stages don't load credentials by default; `role_arn` opts into assume-role.
93+
// External S3 stages disallow the ambient credential chain by default.
94+
// `role_arn` opts into using the credential chain as the source credential.
9495
let storage = match stage_info.stage_params.storage.clone() {
9596
StorageParams::S3(mut cfg) => {
96-
if cfg.role_arn.is_empty() {
97-
cfg.disable_credential_loader = true;
98-
}
97+
let allow_credential_chain = !cfg.role_arn.is_empty();
98+
cfg.disable_credential_loader = !allow_credential_chain;
9999
StorageParams::S3(cfg)
100100
}
101101
v => v,

src/query/config/src/config.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -542,6 +542,9 @@ impl From<InnerStorageConfig> for StorageConfig {
542542
v => unreachable!("{v:?} should not be used as storage backend"),
543543
}
544544

545+
cfg.s3.disable_config_load = inner.s3_disable_config_load;
546+
cfg.s3.disable_ec2_metadata = inner.s3_disable_ec2_metadata;
547+
545548
cfg
546549
}
547550
}
@@ -587,6 +590,8 @@ impl TryInto<InnerStorageConfig> for StorageConfig {
587590
Ok(InnerStorageConfig {
588591
num_cpus: self.storage_num_cpus,
589592
allow_insecure: self.allow_insecure,
593+
s3_disable_config_load: self.s3.disable_config_load,
594+
s3_disable_ec2_metadata: self.s3.disable_ec2_metadata,
590595
params: {
591596
match self.typ.as_str() {
592597
"azblob" => {
@@ -931,6 +936,14 @@ pub struct S3StorageConfig {
931936
#[clap(long = "storage-s3-enable-virtual-host-style", value_name = "VALUE")]
932937
pub enable_virtual_host_style: bool,
933938

939+
/// Disable loading credentials from env/shared config/web identity.
940+
#[clap(long = "storage-s3-disable-config-load")]
941+
pub disable_config_load: bool,
942+
943+
/// Disable EC2 instance metadata credential provider (IMDS).
944+
#[clap(long = "storage-s3-disable-ec2-metadata")]
945+
pub disable_ec2_metadata: bool,
946+
934947
#[clap(long = "storage-s3-role-arn", value_name = "VALUE", default_value_t)]
935948
#[serde(rename = "role_arn")]
936949
pub s3_role_arn: String,
@@ -963,6 +976,8 @@ impl Debug for S3StorageConfig {
963976
.field("bucket", &self.bucket)
964977
.field("root", &self.root)
965978
.field("enable_virtual_host_style", &self.enable_virtual_host_style)
979+
.field("disable_config_load", &self.disable_config_load)
980+
.field("disable_ec2_metadata", &self.disable_ec2_metadata)
966981
.field("role_arn", &self.s3_role_arn)
967982
.field("external_id", &self.s3_external_id)
968983
.field("access_key_id", &mask_string(&self.access_key_id, 3))
@@ -987,6 +1002,8 @@ impl From<InnerStorageS3Config> for S3StorageConfig {
9871002
root: inner.root,
9881003
master_key: inner.master_key,
9891004
enable_virtual_host_style: inner.enable_virtual_host_style,
1005+
disable_config_load: false,
1006+
disable_ec2_metadata: false,
9901007
s3_role_arn: inner.role_arn,
9911008
s3_external_id: inner.external_id,
9921009
storage_class: inner.storage_class,

src/query/service/tests/it/storages/testdata/configs_table_basic.txt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,8 @@ DB.Table: 'system'.'configs', Table: configs-table_id:1, ver:0, Engine: SystemCo
267267
| 'storage' | 'oss.server_side_encryption_key_id' | '' | '' |
268268
| 'storage' | 's3.access_key_id' | '' | '' |
269269
| 'storage' | 's3.bucket' | '' | '' |
270+
| 'storage' | 's3.disable_config_load' | 'false' | '' |
271+
| 'storage' | 's3.disable_ec2_metadata' | 'false' | '' |
270272
| 'storage' | 's3.enable_virtual_host_style' | 'false' | '' |
271273
| 'storage' | 's3.endpoint_url' | 'https://s3.amazonaws.com' | '' |
272274
| 'storage' | 's3.external_id' | '' | '' |
@@ -293,4 +295,3 @@ DB.Table: 'system'.'configs', Table: configs-table_id:1, ver:0, Engine: SystemCo
293295
| 'storage' | 'webhdfs.user_name' | '' | '' |
294296
+-----------+------------------------------------------------------+-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+----------+
295297

296-

src/query/sql/src/planner/binder/ddl/stage.rs

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -90,24 +90,32 @@ impl Binder {
9090
)
9191
.await?;
9292

93-
// External S3 stages don't load credentials by default; `role_arn` opts into assume-role.
94-
let stage_storage = match stage_storage {
93+
// Validate the input config using the same credential-chain policy as runtime stage access.
94+
let validate_storage = match stage_storage.clone() {
9595
StorageParams::S3(mut cfg) => {
96-
if cfg.role_arn.is_empty() {
97-
cfg.disable_credential_loader = true;
98-
}
96+
let allow_credential_chain = !cfg.role_arn.is_empty();
97+
cfg.disable_credential_loader = !allow_credential_chain;
9998
StorageParams::S3(cfg)
10099
}
101100
v => v,
102101
};
103102

104103
// Check the storage params via init operator.
105-
let _ = init_operator(&stage_storage).map_err(|err| {
104+
let _ = init_operator(&validate_storage).map_err(|err| {
106105
ErrorCode::InvalidConfig(format!(
107106
"Input storage config for stage is invalid: {err:?}"
108107
))
109108
})?;
110109

110+
// Persist stage configs without embedding runtime-only credential-chain policy.
111+
let stage_storage = match stage_storage {
112+
StorageParams::S3(mut cfg) => {
113+
cfg.disable_credential_loader = false;
114+
StorageParams::S3(cfg)
115+
}
116+
v => v,
117+
};
118+
111119
StageInfo::new_external_stage(stage_storage, true).with_stage_name(stage_name)
112120
}
113121
};

src/query/sql/src/planner/binder/location.rs

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ use databend_common_ast::ast::Connection;
2525
use databend_common_ast::ast::UriLocation;
2626
use databend_common_base::runtime::ThreadTracker;
2727
use databend_common_catalog::table_context::TableContext;
28-
use databend_common_config::GlobalConfig;
2928
use databend_common_exception::ErrorCode;
3029
use databend_common_meta_app::storage::S3StorageClass;
3130
use databend_common_meta_app::storage::STORAGE_GCS_DEFAULT_ENDPOINT;
@@ -173,16 +172,13 @@ fn parse_s3_params(l: &mut UriLocation, root: String) -> Result<StorageParams> {
173172
}
174173
.to_string();
175174

176-
// If role_arn is empty and we don't allow allow insecure, we should disable credential loader.
177-
let mut disable_credential_loader =
178-
role_arn.is_empty() && !GlobalConfig::instance().storage.allow_insecure;
179-
180-
// If we are in history table scope, we allow credential loader to be enabled
175+
// Allow credential chain only when it is explicitly needed (assume-role) or for history tables.
181176
let in_history_table_scope = ThreadTracker::capture_log_settings()
182177
.is_some_and(|settings| settings.level == LevelFilter::Off);
178+
let allow_credential_chain = !role_arn.is_empty() || in_history_table_scope;
179+
let disable_credential_loader = !allow_credential_chain;
183180
if in_history_table_scope {
184-
info!("Enable credential loader for history tables");
185-
disable_credential_loader = false;
181+
info!("Allow credential chain for history tables");
186182
}
187183

188184
let sp = StorageParams::S3(StorageS3Config {

0 commit comments

Comments
 (0)