morningman commented on code in PR #56311:
URL: https://github.com/apache/doris/pull/56311#discussion_r2369780296
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/property/ParamRules.java:
##########
@@ -168,6 +168,57 @@ public ParamRules forbidIf(String a, String expectedValue,
String[] forbiddenVal
return this;
}
+ /**
+ * Require that all values must either all exist or all be null/empty.
+ *
+ * @param values array of values to check
+ * @param errorMessage error message if the requirement is violated
+ * @return this ParamRules instance for chaining
+ */
+ public ParamRules requireTogether(String[] values, String errorMessage) {
Review Comment:
Add ut for these methods
##########
fe/fe-core/src/main/java/com/amazonaws/glue/catalog/credentials/ConfigurationAWSCredentialsProvider.java:
##########
@@ -39,13 +41,28 @@ public AWSCredentials getCredentials() {
String accessKey =
StringUtils.trim(conf.get(AWSGlueConfig.AWS_GLUE_ACCESS_KEY));
String secretKey =
StringUtils.trim(conf.get(AWSGlueConfig.AWS_GLUE_SECRET_KEY));
String sessionToken =
StringUtils.trim(conf.get(AWSGlueConfig.AWS_GLUE_SESSION_TOKEN));
+ String roleArn =
StringUtils.trim(conf.get(AWSGlueConfig.AWS_GLUE_ROLE_ARN));
+ String externalId =
StringUtils.trim(conf.get(AWSGlueConfig.AWS_GLUE_EXTERNAL_ID));
if (!StringUtils.isNullOrEmpty(accessKey) &&
!StringUtils.isNullOrEmpty(secretKey)) {
return (StringUtils.isNullOrEmpty(sessionToken) ? new
BasicAWSCredentials(accessKey,
secretKey) : new BasicSessionCredentials(accessKey,
secretKey, sessionToken));
- } else {
- throw new SdkClientException(
- "Unable to load AWS credentials from hive conf
(aws.glue.access-key and aws.glue.secret-key)");
}
+
+ AWSCredentialsProvider longLivedProvider = new
DefaultAWSCredentialsProviderChain();
+
+ if (roleArn != null && !roleArn.isEmpty()) {
Review Comment:
!Strings.isNullOrEmpty(roleArn)
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/property/metastore/HMSGlueMetaStoreProperties.java:
##########
@@ -109,6 +108,17 @@ private void initHiveConf() {
hiveConf.set(AWS_CATALOG_CREDENTIALS_PROVIDER_FACTORY_CLASS_KEY,
"com.amazonaws.glue.catalog.credentials.ConfigurationAWSCredentialsProviderFactory");
hiveConf.set("hive.metastore.type", "glue");
+ setHiveConfPropertiesIfNotNull(hiveConf,
AWSGlueConfig.AWS_GLUE_ACCESS_KEY, baseProperties.glueAccessKey);
Review Comment:
No need to check if ak sk is set or not?
##########
fe/fe-core/src/main/java/com/amazonaws/glue/catalog/credentials/ConfigurationAWSCredentialsProvider.java:
##########
@@ -39,13 +41,28 @@ public AWSCredentials getCredentials() {
String accessKey =
StringUtils.trim(conf.get(AWSGlueConfig.AWS_GLUE_ACCESS_KEY));
String secretKey =
StringUtils.trim(conf.get(AWSGlueConfig.AWS_GLUE_SECRET_KEY));
String sessionToken =
StringUtils.trim(conf.get(AWSGlueConfig.AWS_GLUE_SESSION_TOKEN));
+ String roleArn =
StringUtils.trim(conf.get(AWSGlueConfig.AWS_GLUE_ROLE_ARN));
+ String externalId =
StringUtils.trim(conf.get(AWSGlueConfig.AWS_GLUE_EXTERNAL_ID));
if (!StringUtils.isNullOrEmpty(accessKey) &&
!StringUtils.isNullOrEmpty(secretKey)) {
return (StringUtils.isNullOrEmpty(sessionToken) ? new
BasicAWSCredentials(accessKey,
secretKey) : new BasicSessionCredentials(accessKey,
secretKey, sessionToken));
- } else {
- throw new SdkClientException(
- "Unable to load AWS credentials from hive conf
(aws.glue.access-key and aws.glue.secret-key)");
}
+
+ AWSCredentialsProvider longLivedProvider = new
DefaultAWSCredentialsProviderChain();
+
+ if (roleArn != null && !roleArn.isEmpty()) {
+ STSAssumeRoleSessionCredentialsProvider.Builder builder =
+ new
STSAssumeRoleSessionCredentialsProvider.Builder(roleArn, "local-session")
+
.withLongLivedCredentialsProvider(longLivedProvider);
+
+ if (externalId != null && !externalId.trim().isEmpty()) {
Review Comment:
!Strings.isNullOrEmpty(externalId)
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/property/metastore/IcebergGlueMetaStoreProperties.java:
##########
@@ -83,11 +83,31 @@ private void appendS3Props(Map<String, String> props) {
private void appendGlueProps(Map<String, String> props) {
props.put(AwsProperties.GLUE_CATALOG_ENDPOINT,
glueProperties.glueEndpoint);
- props.put("client.credentials-provider",
-
"com.amazonaws.glue.catalog.credentials.ConfigurationAWSCredentialsProvider2x");
- props.put("client.credentials-provider.glue.access_key",
glueProperties.glueAccessKey);
- props.put("client.credentials-provider.glue.secret_key",
glueProperties.glueSecretKey);
- props.put("aws.catalog.credentials.provider.factory.class",
-
"com.amazonaws.glue.catalog.credentials.ConfigurationAWSCredentialsProviderFactory");
+
+ if (StringUtils.isNotBlank(glueProperties.glueAccessKey) && StringUtils
+ .isNotBlank(glueProperties.glueSecretKey)) {
+ props.put("client.credentials-provider",
+
"com.amazonaws.glue.catalog.credentials.ConfigurationAWSCredentialsProvider2x");
+ props.put("client.credentials-provider.glue.access_key",
glueProperties.glueAccessKey);
+ props.put("client.credentials-provider.glue.secret_key",
glueProperties.glueSecretKey);
+ props.put("aws.catalog.credentials.provider.factory.class",
+
"com.amazonaws.glue.catalog.credentials.ConfigurationAWSCredentialsProviderFactory");
+ if (StringUtils.isNotBlank(glueProperties.glueSessionToken)) {
+ props.put("client.credentials-provider.glue.session_token",
glueProperties.glueSessionToken);
+ }
+ return;
+ }
Review Comment:
I suggest:
```
if (ak, sk) {
} else if (iam role) {
} else {
throw exception
}
```
--
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]
