From cbf641cad1d5a0bb3d53e0b270c4353fcb97f3bd Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Thu, 30 Nov 2023 18:52:05 +0800 Subject: [PATCH] feat: Load credential while role_arn has been specified (#13875) * feat: Load credential while role_arn has been specified Signed-off-by: Xuanwo * Fix tests Signed-off-by: Xuanwo --------- Signed-off-by: Xuanwo --- src/query/sql/src/planner/binder/location.rs | 8 +++++--- src/query/sql/tests/location.rs | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/query/sql/src/planner/binder/location.rs b/src/query/sql/src/planner/binder/location.rs index 8f2c39ae27a3..3eac662ca594 100644 --- a/src/query/sql/src/planner/binder/location.rs +++ b/src/query/sql/src/planner/binder/location.rs @@ -175,6 +175,10 @@ fn parse_s3_params(l: &mut UriLocation, root: String) -> Result { ) })?; + // If role_arn is empty and we don't allow allow insecure, we should disable credential loader. + let disable_credential_loader = + role_arn.is_empty() && !GlobalConfig::instance().storage.allow_insecure; + let sp = StorageParams::S3(StorageS3Config { endpoint_url: secure_omission(endpoint), region, @@ -184,9 +188,7 @@ fn parse_s3_params(l: &mut UriLocation, root: String) -> Result { security_token, master_key, root, - // Disable credential load by default. - // TODO(xuanwo): we should support AssumeRole. - disable_credential_loader: !GlobalConfig::instance().storage.allow_insecure, + disable_credential_loader, enable_virtual_host_style, role_arn, external_id, diff --git a/src/query/sql/tests/location.rs b/src/query/sql/tests/location.rs index 29a24b6de3aa..6efe76af387c 100644 --- a/src/query/sql/tests/location.rs +++ b/src/query/sql/tests/location.rs @@ -287,7 +287,7 @@ async fn test_parse_uri_location() -> Result<()> { security_token: "".to_string(), master_key: "".to_string(), root: "/tmp/".to_string(), - disable_credential_loader: true, + disable_credential_loader: false, enable_virtual_host_style: false, role_arn: "aws::iam::xxxx".to_string(), external_id: "".to_string(),