github-actions[bot] commented on code in PR #63680:
URL: https://github.com/apache/doris/pull/63680#discussion_r3305826283
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/S3Properties.java:
##########
@@ -468,6 +469,7 @@ public static class Env {
public static final String ROLE_ARN = "AWS_ROLE_ARN";
public static final String EXTERNAL_ID = "AWS_EXTERNAL_ID";
Review Comment:
This adds the AWS_* spelling for the static cloud/thrift conversion helpers,
but the object-bound `S3Properties` path still does not bind that key because
the `@ConnectorProperty` for `awsCredentialsProviderType` only lists
`s3.credentials_provider_type` and `glue.credentials_provider_type`. A user
creating S3 properties with `AWS_ROLE_ARN` plus
`AWS_CREDENTIALS_PROVIDER_TYPE=env` goes through
`StorageProperties.createPrimary`, leaves `awsCredentialsProviderType` at
`DEFAULT`, and `getBackendConfigProperties()`/FE STS validation use the wrong
provider even though `getObjStoreInfoPB()` would accept it. Please add this
alias to the connector property binding (or normalize before binding) and cover
the `createPrimary`/backend-config path with an AWS_* test.
##########
be/src/util/s3_util.cpp:
##########
@@ -626,6 +626,16 @@ static CredProviderType
cred_provider_type_from_thrift(TCredProviderType::type c
return CredProviderType::Simple;
case TCredProviderType::INSTANCE_PROFILE:
return CredProviderType::InstanceProfile;
+ case TCredProviderType::ENV:
+ return CredProviderType::Env;
+ case TCredProviderType::SYSTEM_PROPERTIES:
+ return CredProviderType::SystemProperties;
+ case TCredProviderType::WEB_IDENTITY:
+ return CredProviderType::WebIdentity;
+ case TCredProviderType::CONTAINER:
Review Comment:
Exposing `TCredProviderType::CONTAINER` makes FE able to send this mode to
BE, but BE's `S3ClientFactory::_create_credentials_provider` container branch
only supports `AWS_CONTAINER_CREDENTIALS_RELATIVE_URI` and ignores
`AWS_CONTAINER_CREDENTIALS_FULL_URI`/`AWS_CONTAINER_AUTHORIZATION_TOKEN`. That
means the new thrift value fails for ECS/Fargate deployments that use the
full-URI credential endpoint. Please update the BE provider creation before
enabling this enum mapping, and add a full-URI test alongside the relative-URI
case.
##########
cloud/src/recycler/s3_accessor.cpp:
##########
@@ -314,6 +321,28 @@ std::shared_ptr<Aws::Auth::AWSCredentialsProvider>
S3Accessor::_get_aws_credenti
return std::make_shared<Aws::Auth::DefaultAWSCredentialsProviderChain>();
}
+std::shared_ptr<Aws::Auth::AWSCredentialsProvider>
S3Accessor::_create_credentials_provider(
+ CredProviderType type) {
+ switch (type) {
+ case CredProviderType::Env:
+ return
std::make_shared<Aws::Auth::EnvironmentAWSCredentialsProvider>();
+ case CredProviderType::SystemProperties:
+ return
std::make_shared<Aws::Auth::ProfileConfigFileAWSCredentialsProvider>();
+ case CredProviderType::WebIdentity:
+ return
std::make_shared<Aws::Auth::STSAssumeRoleWebIdentityCredentialsProvider>();
+ case CredProviderType::Container:
Review Comment:
The explicit `CONTAINER` provider only looks at
`AWS_CONTAINER_CREDENTIALS_RELATIVE_URI`. ECS/Fargate can provide credentials
via `AWS_CONTAINER_CREDENTIALS_FULL_URI` (with optional
`AWS_CONTAINER_AUTHORIZATION_TOKEN`), and the existing
`CustomAwsCredentialsProviderChain` in this repo already handles both forms.
With this implementation, a vault configured with
`s3.credentials_provider_type=CONTAINER` on a FULL_URI-only task constructs
`TaskRoleCredentialsProvider` with an empty relative path and the recycler
cannot obtain credentials. Please mirror the relative/full URI selection used
by `CustomAwsCredentialsProviderChain` here.
--
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]
