This is an automated email from the ASF dual-hosted git repository.
morrysnow pushed a commit to branch branch-3.1
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-3.1 by this push:
new f5519f6ffbf branch-3.1: [fix](params-refactor)Enhance Object Storage
Parameter Validation and Exception Handling #55014 (#55086)
f5519f6ffbf is described below
commit f5519f6ffbf3e33477e7f351659ca7c72d4b1e0d
Author: Calvin Kirs <[email protected]>
AuthorDate: Thu Aug 21 16:45:38 2025 +0800
branch-3.1: [fix](params-refactor)Enhance Object Storage Parameter
Validation and Exception Handling #55014 (#55086)
picked from #55014
---
.../apache/doris/datasource/CatalogProperty.java | 22 +++++++--
.../apache/doris/datasource/ExternalCatalog.java | 28 +----------
.../doris/datasource/iceberg/dlf/DLFCatalog.java | 27 +++++------
.../datasource/property/PropertyConverter.java | 3 --
.../IcebergFileSystemMetaStoreProperties.java | 9 +---
.../storage/AbstractS3CompatibleProperties.java | 5 ++
.../datasource/property/storage/COSProperties.java | 17 -------
.../property/storage/MinioProperties.java | 17 -------
.../datasource/property/storage/OBSProperties.java | 17 -------
.../datasource/property/storage/OSSProperties.java | 14 +-----
.../datasource/property/storage/S3Properties.java | 42 ++++++----------
.../datasource/property/PropertyConverterTest.java | 56 +---------------------
.../property/storage/COSPropertiesTest.java | 8 ++--
.../property/storage/MinioPropertiesTest.java | 13 +++--
.../property/storage/OBSPropertyTest.java | 17 ++++---
.../property/storage/OSSPropertiesTest.java | 37 ++++++++++++--
.../property/storage/S3PropertiesTest.java | 10 ++--
17 files changed, 116 insertions(+), 226 deletions(-)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogProperty.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogProperty.java
index 8ad534ff86b..0fff877ef36 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogProperty.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogProperty.java
@@ -20,7 +20,6 @@ package org.apache.doris.datasource;
import org.apache.doris.common.UserException;
import org.apache.doris.common.io.Text;
import org.apache.doris.common.io.Writable;
-import org.apache.doris.datasource.property.PropertyConverter;
import org.apache.doris.datasource.property.metastore.MetastoreProperties;
import org.apache.doris.datasource.property.storage.StorageProperties;
import org.apache.doris.persist.gson.GsonUtils;
@@ -28,6 +27,7 @@ import org.apache.doris.persist.gson.GsonUtils;
import com.google.common.collect.Maps;
import com.google.gson.annotations.SerializedName;
import org.apache.commons.collections.MapUtils;
+import org.apache.hadoop.conf.Configuration;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
@@ -84,7 +84,7 @@ public class CatalogProperty implements Writable {
public void modifyCatalogProps(Map<String, String> props) {
synchronized (this) {
-
properties.putAll(PropertyConverter.convertToMetaProperties(props));
+ properties.putAll(props);
resetAllCaches();
}
}
@@ -206,9 +206,21 @@ public class CatalogProperty implements Writable {
if (hadoopProperties == null) {
synchronized (this) {
if (hadoopProperties == null) {
- Map<String, String> result = getProperties();
-
result.putAll(PropertyConverter.convertToHadoopFSProperties(getProperties()));
- this.hadoopProperties = result;
+ hadoopProperties = new HashMap<>();
+ Map<StorageProperties.Type, StorageProperties> storageMap
= getStoragePropertiesMap();
+
+ for (StorageProperties sp : storageMap.values()) {
+ Configuration configuration =
sp.getHadoopStorageConfig();
+ if (configuration != null) {
+ configuration.forEach(entry -> {
+ String key = entry.getKey();
+ String value = entry.getValue();
+ if (value != null) {
+ hadoopProperties.put(key, value);
+ }
+ });
+ }
+ }
}
}
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java
index d5312fe5e3f..083d3d20565 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java
@@ -53,7 +53,6 @@ import
org.apache.doris.datasource.maxcompute.MaxComputeExternalDatabase;
import org.apache.doris.datasource.metacache.MetaCache;
import org.apache.doris.datasource.operations.ExternalMetadataOps;
import org.apache.doris.datasource.paimon.PaimonExternalDatabase;
-import org.apache.doris.datasource.property.PropertyConverter;
import org.apache.doris.datasource.test.TestExternalCatalog;
import org.apache.doris.datasource.test.TestExternalDatabase;
import
org.apache.doris.datasource.trinoconnector.TrinoConnectorExternalDatabase;
@@ -167,13 +166,6 @@ public abstract class ExternalCatalog
private boolean objectCreated = false;
protected ExternalMetadataOps metadataOps;
protected TransactionManager transactionManager;
-
- private ExternalSchemaCache schemaCache;
- // A cached and being converted properties for external catalog.
- // generated from catalog properties.
- private byte[] propLock = new byte[0];
- private Map<String, String> convertedProperties = null;
-
protected Optional<Boolean> useMetaCache = Optional.empty();
protected MetaCache<ExternalDatabase<? extends ExternalTable>> metaCache;
protected ExecutionAuthenticator executionAuthenticator;
@@ -315,7 +307,6 @@ public abstract class ExternalCatalog
}
return;
}
- isInitializing = true;
try {
initLocalObjects();
if (!initialized) {
@@ -575,10 +566,6 @@ public abstract class ExternalCatalog
public synchronized void resetToUninitialized(boolean invalidCache) {
this.objectCreated = false;
this.initialized = false;
- synchronized (this.propLock) {
- this.convertedProperties = null;
- }
-
synchronized (this.confLock) {
this.cachedConf = null;
}
@@ -743,17 +730,7 @@ public abstract class ExternalCatalog
@Override
public Map<String, String> getProperties() {
- // convert properties may be a heavy operation, so we cache the result.
- if (convertedProperties != null) {
- return convertedProperties;
- }
- synchronized (propLock) {
- if (convertedProperties != null) {
- return convertedProperties;
- }
- convertedProperties =
PropertyConverter.convertToMetaProperties(catalogProperty.getProperties());
- return convertedProperties;
- }
+ return catalogProperty.getProperties();
}
@Override
@@ -1041,7 +1018,6 @@ public abstract class ExternalCatalog
}
}
}
- this.propLock = new byte[0];
this.confLock = new byte[0];
this.initialized = false;
setDefaultPropsIfMissing(true);
@@ -1165,7 +1141,7 @@ public abstract class ExternalCatalog
@Override
public void dropTable(String dbName, String tableName, boolean isView,
boolean isMtmv, boolean ifExists,
- boolean force) throws DdlException {
+ boolean force) throws DdlException {
makeSureInitialized();
if (metadataOps == null) {
throw new DdlException("Drop table is not supported for catalog: "
+ getName());
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/dlf/DLFCatalog.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/dlf/DLFCatalog.java
index e51292feff2..fb90260f09a 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/dlf/DLFCatalog.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/dlf/DLFCatalog.java
@@ -21,11 +21,10 @@ import org.apache.doris.common.credentials.CloudCredential;
import org.apache.doris.common.util.S3Util;
import org.apache.doris.datasource.iceberg.HiveCompatibleCatalog;
import org.apache.doris.datasource.iceberg.dlf.client.DLFCachedClientPool;
-import org.apache.doris.datasource.property.constants.OssProperties;
-import org.apache.doris.datasource.property.constants.S3Properties;
+import org.apache.doris.datasource.property.storage.OSSProperties;
+import org.apache.commons.lang3.StringUtils;
import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.fs.aliyun.oss.Constants;
import org.apache.iceberg.TableOperations;
import org.apache.iceberg.aws.s3.S3FileIO;
import org.apache.iceberg.catalog.TableIdentifier;
@@ -50,22 +49,18 @@ public class DLFCatalog extends HiveCompatibleCatalog {
protected FileIO initializeFileIO(Map<String, String> properties,
Configuration hadoopConf) {
// read from converted properties or default by old s3 aws properties
- String endpoint = properties.getOrDefault(Constants.ENDPOINT_KEY,
properties.get(S3Properties.Env.ENDPOINT));
+ OSSProperties ossProperties = OSSProperties.of(properties);
+ String endpoint = ossProperties.getEndpoint();
CloudCredential credential = new CloudCredential();
-
credential.setAccessKey(properties.getOrDefault(OssProperties.ACCESS_KEY,
- properties.get(S3Properties.Env.ACCESS_KEY)));
-
credential.setSecretKey(properties.getOrDefault(OssProperties.SECRET_KEY,
- properties.get(S3Properties.Env.SECRET_KEY)));
- if (properties.containsKey(OssProperties.SESSION_TOKEN)
- || properties.containsKey(S3Properties.Env.TOKEN)) {
-
credential.setSessionToken(properties.getOrDefault(OssProperties.SESSION_TOKEN,
- properties.get(S3Properties.Env.TOKEN)));
+ credential.setAccessKey(ossProperties.getAccessKey());
+ credential.setSecretKey(ossProperties.getSecretKey());
+ if (StringUtils.isNotBlank(ossProperties.getSessionToken())) {
+ credential.setSessionToken(ossProperties.getSessionToken());
}
- String region = properties.getOrDefault(OssProperties.REGION,
properties.get(S3Properties.Env.REGION));
- boolean isUsePathStyle = properties.getOrDefault("use_path_style",
"false")
- .equalsIgnoreCase("true");
+ String region = ossProperties.getRegion();
+ boolean isUsePathStyle =
Boolean.parseBoolean(ossProperties.getUsePathStyle());
// s3 file io just supports s3-like endpoint
- String s3Endpoint = endpoint.replace(region, "s3." + region);
+ String s3Endpoint = endpoint.replace("oss-" + region, "s3.oss-" +
region);
if (!s3Endpoint.contains("://")) {
s3Endpoint = "http://"; + s3Endpoint;
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/PropertyConverter.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/PropertyConverter.java
index aa22a83e1b0..4e26a80de5a 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/PropertyConverter.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/PropertyConverter.java
@@ -61,9 +61,6 @@ public class PropertyConverter {
private static final Logger LOG =
LogManager.getLogger(PropertyConverter.class);
public static final String USE_PATH_STYLE = "use_path_style";
- public static final String USE_PATH_STYLE_DEFAULT_VALUE = "false";
- public static final String FORCE_PARSING_BY_STANDARD_URI =
"force_parsing_by_standard_uri";
- public static final String FORCE_PARSING_BY_STANDARD_URI_DEFAULT_VALUE =
"false";
/**
* Convert properties defined at doris to metadata properties on Cloud
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/metastore/IcebergFileSystemMetaStoreProperties.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/metastore/IcebergFileSystemMetaStoreProperties.java
index fa94f79df77..7dd97b028b2 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/metastore/IcebergFileSystemMetaStoreProperties.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/metastore/IcebergFileSystemMetaStoreProperties.java
@@ -28,7 +28,6 @@ import org.apache.iceberg.CatalogProperties;
import org.apache.iceberg.catalog.Catalog;
import org.apache.iceberg.hadoop.HadoopCatalog;
-import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -48,6 +47,7 @@ public class IcebergFileSystemMetaStoreProperties extends
AbstractIcebergPropert
List<StorageProperties> storagePropertiesList) {
Configuration configuration =
buildConfiguration(storagePropertiesList);
HadoopCatalog catalog = new HadoopCatalog();
+ buildCatalogProps(catalogProps, storagePropertiesList);
catalog.setConf(configuration);
try {
this.executionAuthenticator.execute(() -> {
@@ -72,9 +72,7 @@ public class IcebergFileSystemMetaStoreProperties extends
AbstractIcebergPropert
return configuration;
}
- private Map<String, String> buildCatalogProps(List<StorageProperties>
storagePropertiesList) {
- Map<String, String> props = new HashMap<>(origProps);
-
+ private void buildCatalogProps(Map<String, String> props,
List<StorageProperties> storagePropertiesList) {
if (storagePropertiesList.size() == 1 && storagePropertiesList.get(0)
instanceof HdfsProperties) {
HdfsProperties hdfsProps = (HdfsProperties)
storagePropertiesList.get(0);
if (hdfsProps.isKerberos()) {
@@ -83,9 +81,6 @@ public class IcebergFileSystemMetaStoreProperties extends
AbstractIcebergPropert
this.executionAuthenticator = new
HadoopExecutionAuthenticator(hdfsProps.getHadoopAuthenticator());
}
}
-
- props.put(CatalogProperties.WAREHOUSE_LOCATION, warehouse);
- return props;
}
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/AbstractS3CompatibleProperties.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/AbstractS3CompatibleProperties.java
index bd5a4a0824c..3b0bca28365 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/AbstractS3CompatibleProperties.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/AbstractS3CompatibleProperties.java
@@ -135,6 +135,11 @@ public abstract class AbstractS3CompatibleProperties
extends StorageProperties i
throw new IllegalArgumentException("Invalid endpoint: " +
getEndpoint());
}
setRegionIfPossible();
+ //Allow anonymous access if both access_key and secret_key are empty
+ //But not recommended for production use.
+ if (StringUtils.isBlank(getAccessKey()) !=
StringUtils.isBlank(getSecretKey())) {
+ throw new IllegalArgumentException("Both the access key and the
secret key must be set.");
+ }
}
/**
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/COSProperties.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/COSProperties.java
index 8e09576891e..b284ba5666e 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/COSProperties.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/COSProperties.java
@@ -18,7 +18,6 @@
package org.apache.doris.datasource.property.storage;
import org.apache.doris.datasource.property.ConnectorProperty;
-import
org.apache.doris.datasource.property.storage.exception.StoragePropertiesException;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableSet;
@@ -126,22 +125,6 @@ public class COSProperties extends
AbstractS3CompatibleProperties {
super(Type.COS, origProps);
}
- @Override
- public void initNormalizeAndCheckProps() {
- super.initNormalizeAndCheckProps();
- // Check if credentials are provided properly - either both or neither
- if (StringUtils.isNotBlank(accessKey) &&
StringUtils.isNotBlank(secretKey)) {
- return;
- }
- // Allow anonymous access if both access_key and secret_key are empty
- if (StringUtils.isBlank(accessKey) && StringUtils.isBlank(secretKey)) {
- return;
- }
- // If only one is provided, it's an error
- throw new StoragePropertiesException(
- "Please set access_key and secret_key or omit both for
anonymous access to public bucket.");
- }
-
protected static boolean guessIsMe(Map<String, String> origProps) {
String value = Stream.of("cos.endpoint", "s3.endpoint",
"AWS_ENDPOINT", "endpoint", "ENDPOINT")
.map(origProps::get)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/MinioProperties.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/MinioProperties.java
index 6f0496ae5ff..dd8d2735324 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/MinioProperties.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/MinioProperties.java
@@ -18,7 +18,6 @@
package org.apache.doris.datasource.property.storage;
import org.apache.doris.datasource.property.ConnectorProperty;
-import
org.apache.doris.datasource.property.storage.exception.StoragePropertiesException;
import com.google.common.collect.ImmutableSet;
import lombok.Getter;
@@ -113,22 +112,6 @@ public class MinioProperties extends
AbstractS3CompatibleProperties {
super(Type.MINIO, origProps);
}
- @Override
- public void initNormalizeAndCheckProps() {
- super.initNormalizeAndCheckProps();
- // Check if credentials are provided properly - either both or neither
- if (StringUtils.isNotBlank(accessKey) &&
StringUtils.isNotBlank(secretKey)) {
- return;
- }
- // Allow anonymous access if both access_key and secret_key are empty
- if (StringUtils.isBlank(accessKey) && StringUtils.isBlank(secretKey)) {
- return;
- }
- // If only one is provided, it's an error
- throw new StoragePropertiesException(
- "Please set access_key and secret_key or omit both for
anonymous access to public bucket.");
- }
-
public static boolean guessIsMe(Map<String, String> origProps) {
//ugly, but we need to check if the user has set any of the identifiers
if (AzureProperties.guessIsMe(origProps) ||
COSProperties.guessIsMe(origProps)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/OBSProperties.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/OBSProperties.java
index 5e9c513d67e..eea6c24ddc4 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/OBSProperties.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/OBSProperties.java
@@ -18,7 +18,6 @@
package org.apache.doris.datasource.property.storage;
import org.apache.doris.datasource.property.ConnectorProperty;
-import
org.apache.doris.datasource.property.storage.exception.StoragePropertiesException;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableSet;
@@ -130,22 +129,6 @@ public class OBSProperties extends
AbstractS3CompatibleProperties {
// Initialize fields from origProps
}
- @Override
- public void initNormalizeAndCheckProps() {
- super.initNormalizeAndCheckProps();
- // Check if credentials are provided properly - either both or neither
- if (StringUtils.isNotBlank(accessKey) &&
StringUtils.isNotBlank(secretKey)) {
- return;
- }
- // Allow anonymous access if both access_key and secret_key are empty
- if (StringUtils.isBlank(accessKey) && StringUtils.isBlank(secretKey)) {
- return;
- }
- // If only one is provided, it's an error
- throw new StoragePropertiesException(
- "Please set access_key and secret_key or omit both for
anonymous access to public bucket.");
- }
-
protected static boolean guessIsMe(Map<String, String> origProps) {
String value = Stream.of("obs.endpoint", "s3.endpoint",
"AWS_ENDPOINT", "endpoint", "ENDPOINT")
.map(origProps::get)
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/OSSProperties.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/OSSProperties.java
index 4cb43b3155a..0d7d41f2cc1 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/OSSProperties.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/OSSProperties.java
@@ -19,7 +19,6 @@ package org.apache.doris.datasource.property.storage;
import org.apache.doris.datasource.property.ConnectorPropertiesUtils;
import org.apache.doris.datasource.property.ConnectorProperty;
-import
org.apache.doris.datasource.property.storage.exception.StoragePropertiesException;
import com.google.common.collect.ImmutableSet;
import lombok.Getter;
@@ -203,7 +202,7 @@ public class OSSProperties extends
AbstractS3CompatibleProperties {
if (!value.contains("aliyuncs.com")) {
return false;
}
- boolean isAliyunOss = (value.contains("oss-") ||
value.contains("dlf."));
+ boolean isAliyunOss = (value.contains("oss-"));
boolean isAmazonS3 = value.contains("s3.");
boolean isDls = value.contains("dls");
return isAliyunOss || isAmazonS3 || isDls;
@@ -247,17 +246,6 @@ public class OSSProperties extends
AbstractS3CompatibleProperties {
if (endpoint.contains("dlf") || endpoint.contains("oss-dls")) {
this.endpoint = getOssEndpoint(region,
BooleanUtils.toBoolean(dlfAccessPublic));
}
- // Check if credentials are provided properly - either both or neither
- if (StringUtils.isNotBlank(accessKey) &&
StringUtils.isNotBlank(secretKey)) {
- return;
- }
- // Allow anonymous access if both access_key and secret_key are empty
- if (StringUtils.isBlank(accessKey) && StringUtils.isBlank(secretKey)) {
- return;
- }
- // If only one is provided, it's an error
- throw new StoragePropertiesException(
- "Please set access_key and secret_key or omit both for
anonymous access to public bucket.");
}
private static String getOssEndpoint(String region, boolean publicAccess) {
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/S3Properties.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/S3Properties.java
index bd05f39d4ab..e4a15fb5a8e 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/S3Properties.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/S3Properties.java
@@ -19,9 +19,7 @@ package org.apache.doris.datasource.property.storage;
import org.apache.doris.datasource.property.ConnectorPropertiesUtils;
import org.apache.doris.datasource.property.ConnectorProperty;
-import
org.apache.doris.datasource.property.storage.exception.StoragePropertiesException;
-import com.google.common.base.Strings;
import com.google.common.collect.ImmutableSet;
import lombok.Getter;
import lombok.Setter;
@@ -34,6 +32,7 @@ import
software.amazon.awssdk.auth.credentials.InstanceProfileCredentialsProvide
import software.amazon.awssdk.auth.credentials.ProfileCredentialsProvider;
import
software.amazon.awssdk.auth.credentials.SystemPropertyCredentialsProvider;
import
software.amazon.awssdk.auth.credentials.WebIdentityTokenFileCredentialsProvider;
+import software.amazon.awssdk.regions.Region;
import software.amazon.awssdk.services.sts.StsClient;
import
software.amazon.awssdk.services.sts.auth.StsAssumeRoleCredentialsProvider;
@@ -187,25 +186,10 @@ public class S3Properties extends
AbstractS3CompatibleProperties {
@Override
public void initNormalizeAndCheckProps() {
super.initNormalizeAndCheckProps();
- convertGlueToS3EndpointIfNeeded();
- if (StringUtils.isNotBlank(accessKey) &&
StringUtils.isNotBlank(secretKey)) {
- return;
- }
- if (StringUtils.isNotBlank(s3ExternalId) &&
StringUtils.isNotBlank(s3IAMRole)) {
- return;
- }
- // When using vended credentials with a REST catalog, AK/SK are not
provided directly.
- // The credentials will be fetched from the REST service later.
- // So we skip the credential check in this case.
- if
(Boolean.parseBoolean(origProps.getOrDefault("iceberg.rest.vended-credentials-enabled",
"false"))) {
- return;
- }
- // Allow anonymous access if both access_key and secret_key are empty
- if (StringUtils.isBlank(accessKey) && StringUtils.isBlank(secretKey)) {
- return;
+ if (StringUtils.isNotBlank(s3ExternalId) &&
StringUtils.isBlank(s3IAMRole)) {
+ throw new IllegalArgumentException("s3.external_id must be used
with s3.role_arn");
}
- throw new StoragePropertiesException("Please set s3.access_key and
s3.secret_key or s3.role_arn and "
- + "s3.external_id or omit all for anonymous access to public
bucket.");
+ convertGlueToS3EndpointIfNeeded();
}
/**
@@ -226,7 +210,7 @@ public class S3Properties extends
AbstractS3CompatibleProperties {
* cause the type detection to fail, leading to missed recognition of
valid S3 properties.
* A more robust approach would allow further validation downstream
rather than failing early here.
*/
- if (!Strings.isNullOrEmpty(endpoint)) {
+ if (StringUtils.isNotBlank(endpoint)) {
return endpoint.contains("amazonaws.com");
}
@@ -245,7 +229,7 @@ public class S3Properties extends
AbstractS3CompatibleProperties {
.filter(Objects::nonNull)
.findFirst()
.orElse(null);
- if (!Strings.isNullOrEmpty(region)) {
+ if (StringUtils.isNotBlank(region)) {
return true;
}
return false;
@@ -260,9 +244,10 @@ public class S3Properties extends
AbstractS3CompatibleProperties {
public Map<String, String> getBackendConfigProperties() {
Map<String, String> backendProperties =
generateBackendS3Configuration();
- if (StringUtils.isNotBlank(s3ExternalId)
- && StringUtils.isNotBlank(s3IAMRole)) {
+ if (StringUtils.isNotBlank(s3IAMRole)) {
backendProperties.put("AWS_ROLE_ARN", s3IAMRole);
+ }
+ if (StringUtils.isNotBlank(s3ExternalId)) {
backendProperties.put("AWS_EXTERNAL_ID", s3ExternalId);
}
return backendProperties;
@@ -282,6 +267,7 @@ public class S3Properties extends
AbstractS3CompatibleProperties {
}
if (StringUtils.isNotBlank(s3IAMRole)) {
StsClient stsClient = StsClient.builder()
+ .region(Region.of(region))
.credentialsProvider(InstanceProfileCredentialsProvider.create())
.build();
@@ -289,7 +275,7 @@ public class S3Properties extends
AbstractS3CompatibleProperties {
.stsClient(stsClient)
.refreshRequest(builder -> {
builder.roleArn(s3IAMRole).roleSessionName("aws-sdk-java-v2-fe");
- if (!Strings.isNullOrEmpty(s3ExternalId)) {
+ if (StringUtils.isNotBlank(s3ExternalId)) {
builder.externalId(s3ExternalId);
}
}).build();
@@ -310,12 +296,14 @@ public class S3Properties extends
AbstractS3CompatibleProperties {
super.initializeHadoopStorageConfig();
//Set assumed_roles
//@See
https://hadoop.apache.org/docs/r3.4.1/hadoop-aws/tools/hadoop-aws/assumed_roles.html
- if (StringUtils.isNotBlank(s3ExternalId) &&
StringUtils.isNotBlank(s3IAMRole)) {
+ if (StringUtils.isNotBlank(s3IAMRole)) {
//@See org.apache.hadoop.fs.s3a.auth.AssumedRoleCredentialProvider
- hadoopStorageConfig.set("fs.s3a.assumed.role.external.id",
s3ExternalId);
hadoopStorageConfig.set("fs.s3a.assumed.role.arn", s3IAMRole);
hadoopStorageConfig.set("fs.s3a.aws.credentials.provider",
"org.apache.hadoop.fs.s3a.auth.AssumedRoleCredentialProvider");
+ if (StringUtils.isNotBlank(s3ExternalId)) {
+ hadoopStorageConfig.set("fs.s3a.assumed.role.external.id",
s3ExternalId);
+ }
}
}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/datasource/property/PropertyConverterTest.java
b/fe/fe-core/src/test/java/org/apache/doris/datasource/property/PropertyConverterTest.java
index 36e78d80c6b..6e9686481d0 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/datasource/property/PropertyConverterTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/datasource/property/PropertyConverterTest.java
@@ -23,8 +23,6 @@ import org.apache.doris.analysis.CreateResourceStmt;
import org.apache.doris.analysis.DropCatalogStmt;
import org.apache.doris.analysis.OutFileClause;
import org.apache.doris.analysis.QueryStmt;
-import org.apache.doris.analysis.SelectStmt;
-import org.apache.doris.analysis.TableValuedFunctionRef;
import org.apache.doris.backup.Repository;
import org.apache.doris.catalog.Env;
import org.apache.doris.catalog.Resource;
@@ -50,7 +48,6 @@ import
org.apache.doris.datasource.property.constants.ObsProperties;
import org.apache.doris.datasource.property.constants.OssProperties;
import org.apache.doris.datasource.property.constants.S3Properties;
import org.apache.doris.meta.MetaContext;
-import org.apache.doris.tablefunction.S3TableValuedFunction;
import org.apache.doris.thrift.TFileFormatType;
import org.apache.doris.utframe.TestWithFeService;
@@ -228,56 +225,6 @@ public class PropertyConverterTest extends
TestWithFeService {
Assertions.assertEquals(repositoryNew.getRemoteFileSystem().getProperties().size(),
4);
}
- @Test
- public void testS3TVFPropertiesConverter() throws Exception {
- FeConstants.runningUnitTest = true;
- String queryOld = "select * from s3(\n"
- + " 'uri' =
'http://s3.us-east-1.amazonaws.com/my-bucket/test.parquet',\n"
- + " 'access_key' = 'akk',\n"
- + " 'secret_key' = 'skk',\n"
- + " 'region' = 'us-east-1',\n"
- + " 'format' = 'parquet',\n"
- + " 'use_path_style' = 'true'\n"
- + ") limit 10;";
- SelectStmt analyzedStmt = createStmt(queryOld);
- Assertions.assertEquals(analyzedStmt.getTableRefs().size(), 1);
- TableValuedFunctionRef oldFuncTable = (TableValuedFunctionRef)
analyzedStmt.getTableRefs().get(0);
- S3TableValuedFunction s3Tvf = (S3TableValuedFunction)
oldFuncTable.getTableFunction();
- Assertions.assertEquals(5,
s3Tvf.getBrokerDesc().getProperties().size());
-
- String queryNew = "select * from s3(\n"
- + " 'uri' =
'http://s3.us-east-1.amazonaws.com/my-bucket/test.parquet',\n"
- + " 's3.access_key' = 'akk',\n"
- + " 's3.secret_key' = 'skk',\n"
- + " 'format' = 'parquet',\n"
- + " 'use_path_style' = 'true'\n"
- + ") limit 10;";
- SelectStmt analyzedStmtNew = createStmt(queryNew);
- Assertions.assertEquals(analyzedStmtNew.getTableRefs().size(), 1);
- TableValuedFunctionRef newFuncTable = (TableValuedFunctionRef)
analyzedStmt.getTableRefs().get(0);
- S3TableValuedFunction newS3Tvf = (S3TableValuedFunction)
newFuncTable.getTableFunction();
- Assertions.assertEquals(5,
newS3Tvf.getBrokerDesc().getProperties().size());
- }
-
- @Test
- public void testAWSOldCatalogPropertiesConverter() throws Exception {
- String queryOld = "create catalog hms_s3_old properties (\n"
- + " 'type'='hms',\n"
- + " 'hive.metastore.uris' = 'thrift://172.21.0.44:7004',\n"
- + " 'AWS_ENDPOINT' = 's3.us-east-1.amazonaws.com',\n"
- + " 'AWS_REGION' = 'us-east-1',\n"
- + " 'AWS_ACCESS_KEY' = 'akk',\n"
- + " 'AWS_SECRET_KEY' = 'skk'\n"
- + ");";
- CreateCatalogStmt analyzedStmt = createStmt(queryOld);
- HMSExternalCatalog catalog = createAndGetCatalog(analyzedStmt,
"hms_s3_old");
- Map<String, String> properties =
catalog.getCatalogProperty().getProperties();
- Assertions.assertEquals(9, properties.size());
-
- Map<String, String> hdProps =
catalog.getCatalogProperty().getHadoopProperties();
- Assertions.assertEquals(17, hdProps.size());
- }
-
@Disabled
@Test
public void testS3CatalogPropertiesConverter() throws Exception {
@@ -304,7 +251,7 @@ public class PropertyConverterTest extends
TestWithFeService {
String query1 = "create catalog " + catalogName1 + " properties (\n"
+ " 'type'='hms',\n"
+ " 'hive.metastore.uris' = 'thrift://172.21.0.1:7004',\n"
- + " 'oss.endpoint' = 'oss-cn-beijing.aliyuncs.com',\n"
+ + " 'oss.endpoint' = 'cn-beijing.oss-dls.aliyuncs.com',\n"
+ " 'oss.hdfs.enabled' = 'true',\n"
+ " 'oss.access_key' = 'akk',\n"
+ " 'oss.secret_key' = 'skk'\n"
@@ -313,7 +260,6 @@ public class PropertyConverterTest extends
TestWithFeService {
CreateCatalogStmt analyzedStmt = createStmt(query1);
HMSExternalCatalog catalog = createAndGetCatalog(analyzedStmt,
catalogName);
Map<String, String> hdProps =
catalog.getCatalogProperty().getHadoopProperties();
- Assertions.assertEquals("com.aliyun.jindodata.oss.JindoOssFileSystem",
hdProps.get("fs.oss.impl"));
Assertions.assertEquals("cn-beijing.oss-dls.aliyuncs.com",
hdProps.get("fs.oss.endpoint"));
}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/COSPropertiesTest.java
b/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/COSPropertiesTest.java
index 5d718e0cbf2..0cf30e49c04 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/COSPropertiesTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/COSPropertiesTest.java
@@ -148,7 +148,7 @@ public class COSPropertiesTest {
public void testMissingAccessKey() {
origProps.put("cos.endpoint", "cos.ap-beijing.myqcloud.com");
origProps.put("cos.secret_key", "myCOSSecretKey");
- Assertions.assertThrows(StoragePropertiesException.class, () ->
StorageProperties.createPrimary(origProps),
+ Assertions.assertThrows(IllegalArgumentException.class, () ->
StorageProperties.createPrimary(origProps),
"Please set access_key and secret_key or omit both for
anonymous access to public bucket.");
}
@@ -156,7 +156,9 @@ public class COSPropertiesTest {
public void testMissingSecretKey() {
origProps.put("cos.endpoint", "cos.ap-beijing.myqcloud.com");
origProps.put("cos.access_key", "myCOSAccessKey");
- Assertions.assertThrows(StoragePropertiesException.class, () ->
StorageProperties.createPrimary(origProps),
- "Please set access_key and secret_key or omit both for
anonymous access to public bucket.");
+ Assertions.assertThrows(IllegalArgumentException.class, () ->
StorageProperties.createPrimary(origProps),
+ "Both the access key and the secret key must be set.");
+ origProps.remove("cos.access_key");
+ Assertions.assertDoesNotThrow(() ->
StorageProperties.createPrimary(origProps));
}
}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/MinioPropertiesTest.java
b/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/MinioPropertiesTest.java
index fe6d8c5e859..bd6fbf2239a 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/MinioPropertiesTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/MinioPropertiesTest.java
@@ -19,7 +19,6 @@ package org.apache.doris.datasource.property.storage;
import org.apache.doris.common.ExceptionChecker;
import org.apache.doris.common.UserException;
-import
org.apache.doris.datasource.property.storage.exception.StoragePropertiesException;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
@@ -67,18 +66,22 @@ public class MinioPropertiesTest {
public void testMissingAccessKey() {
origProps.put("s3.endpoint", "http://localhost:9000";);
origProps.put("s3.secret_key", "minioSecretKey");
- ExceptionChecker.expectThrowsWithMsg(StoragePropertiesException.class,
- "Please set access_key and secret_key or omit both for
anonymous access to public bucket.",
+ ExceptionChecker.expectThrowsWithMsg(IllegalArgumentException.class,
+ "Both the access key and the secret key must be set.",
() -> StorageProperties.createPrimary(origProps));
+ origProps.remove("s3.secret_key");
+ Assertions.assertDoesNotThrow(() ->
StorageProperties.createPrimary(origProps));
}
@Test
public void testMissingSecretKey() {
origProps.put("s3.endpoint", "http://localhost:9000";);
origProps.put("s3.access_key", "minioAccessKey");
- ExceptionChecker.expectThrowsWithMsg(StoragePropertiesException.class,
- "Please set access_key and secret_key or omit both for
anonymous access to public bucket.",
+ ExceptionChecker.expectThrowsWithMsg(IllegalArgumentException.class,
+ "Both the access key and the secret key must be set.",
() -> StorageProperties.createPrimary(origProps));
+ origProps.remove("s3.access_key");
+ Assertions.assertDoesNotThrow(() ->
StorageProperties.createPrimary(origProps));
}
@Test
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/OBSPropertyTest.java
b/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/OBSPropertyTest.java
index e700ede2de3..2c6f5ffab21 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/OBSPropertyTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/OBSPropertyTest.java
@@ -19,7 +19,6 @@ package org.apache.doris.datasource.property.storage;
import org.apache.doris.common.ExceptionChecker;
import org.apache.doris.common.UserException;
-import
org.apache.doris.datasource.property.storage.exception.StoragePropertiesException;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
@@ -47,8 +46,8 @@ public class OBSPropertyTest {
// allow both access_key and secret_key to be empty for anonymous
access
ExceptionChecker.expectThrowsNoException(() ->
StorageProperties.createAll(origProps));
origProps.put("obs.access_key", "myOBSAccessKey");
- ExceptionChecker.expectThrowsWithMsg(StoragePropertiesException.class,
- "Please set access_key and secret_key or omit both for
anonymous access to public bucket.",
+ ExceptionChecker.expectThrowsWithMsg(IllegalArgumentException.class,
+ "Both the access key and the secret key must be set.",
() -> StorageProperties.createAll(origProps));
origProps.put("obs.secret_key", "myOBSSecretKey");
origProps.put("obs.endpoint", "obs.cn-north-4.myhuaweicloud.com");
@@ -131,18 +130,22 @@ public class OBSPropertyTest {
public void testmissingAccessKey() {
origProps.put("obs.endpoint", "obs.cn-north-4.myhuaweicloud.com");
origProps.put("obs.secret_key", "myOBSSecretKey");
- ExceptionChecker.expectThrowsWithMsg(StoragePropertiesException.class,
- "Please set access_key and secret_key or omit both for
anonymous access to public bucket.",
+ ExceptionChecker.expectThrowsWithMsg(IllegalArgumentException.class,
+ "Both the access key and the secret key must be set.",
() -> StorageProperties.createPrimary(origProps));
+ origProps.remove("obs.secret_key");
+ Assertions.assertDoesNotThrow(() ->
StorageProperties.createPrimary(origProps));
}
@Test
public void testMissingSecretKey() {
origProps.put("obs.endpoint", "obs.cn-north-4.myhuaweicloud.com");
origProps.put("obs.access_key", "myOBSAccessKey");
- ExceptionChecker.expectThrowsWithMsg(StoragePropertiesException.class,
- "Please set access_key and secret_key or omit both for
anonymous access to public bucket.",
+ ExceptionChecker.expectThrowsWithMsg(IllegalArgumentException.class,
+ "Both the access key and the secret key must be set.",
() -> StorageProperties.createPrimary(origProps));
+ origProps.remove("obs.access_key");
+ Assertions.assertDoesNotThrow(() ->
StorageProperties.createPrimary(origProps));
}
private static String obsAccessKey = "";
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/OSSPropertiesTest.java
b/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/OSSPropertiesTest.java
index 20efe3ebe97..31b56a2bd9b 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/OSSPropertiesTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/OSSPropertiesTest.java
@@ -19,7 +19,6 @@ package org.apache.doris.datasource.property.storage;
import org.apache.doris.common.ExceptionChecker;
import org.apache.doris.common.UserException;
-import
org.apache.doris.datasource.property.storage.exception.StoragePropertiesException;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
@@ -146,9 +145,21 @@ public class OSSPropertiesTest {
Map<String, String> origProps = new HashMap<>();
origProps.put("oss.endpoint", "oss-cn-hangzhou.aliyuncs.com");
origProps.put("oss.secret_key", "myOSSSecretKey");
- ExceptionChecker.expectThrowsWithMsg(StoragePropertiesException.class,
- "Please set access_key and secret_key or omit both for
anonymous access to public bucket.",
+ ExceptionChecker.expectThrowsWithMsg(IllegalArgumentException.class,
+ "Both the access key and the secret key must be set.",
() -> StorageProperties.createPrimary(origProps));
+ origProps.remove("oss.secret_key");
+ Assertions.assertDoesNotThrow(() ->
StorageProperties.createPrimary(origProps));
+ }
+
+ @Test
+ public void testDlfProperties() {
+ Map<String, String> origProps = new HashMap<>();
+ origProps.put("iceberg.catalog.type", "dlf");
+ origProps.put("dlf.region", "cn-beijing");
+ origProps.put("dlf.access.public", "true");
+ OSSProperties ossProperties = OSSProperties.of(origProps);
+ Assertions.assertEquals("oss-cn-beijing.aliyuncs.com",
ossProperties.getEndpoint());
}
@Test
@@ -156,9 +167,11 @@ public class OSSPropertiesTest {
Map<String, String> origProps = new HashMap<>();
origProps.put("oss.endpoint", "oss-cn-hangzhou.aliyuncs.com");
origProps.put("oss.access_key", "myOSSAccessKey");
- ExceptionChecker.expectThrowsWithMsg(StoragePropertiesException.class,
- "Please set access_key and secret_key or omit both for
anonymous access to public bucket.",
+ ExceptionChecker.expectThrowsWithMsg(IllegalArgumentException.class,
+ "Both the access key and the secret key must be set.",
() -> StorageProperties.createPrimary(origProps));
+ origProps.remove("oss.access_key");
+ Assertions.assertDoesNotThrow(() ->
StorageProperties.createPrimary(origProps));
}
@Test
@@ -176,4 +189,18 @@ public class OSSPropertiesTest {
origProps.put("uri",
"https://doris-regression-hk.oss-cn-hangzhou-internal.aliyuncs.com/regression/datalake/pipeline_data/data_page_v2_gzip.parquet";);
Assertions.assertEquals("oss-cn-hangzhou-internal.aliyuncs.com",
((OSSProperties) StorageProperties.createPrimary(origProps)).getEndpoint());
}
+
+ @Test
+ public void testOSSProperties() throws UserException {
+ Map<String, String> origProps = new HashMap<>();
+ origProps.put("warehouse", "new_dlf_paimon_catalog");
+ origProps.put("uri", "http://cn-beijing-vpc.dlf.aliyuncs.com";);
+ origProps.put("type", "paimon");
+ origProps.put("paimon.rest.token.provider", "dlf");
+ origProps.put("paimon.rest.dlf.access-key-secret", "XXXXX");
+ origProps.put("paimon.rest.dlf.access-key-id", "XXXXXX");
+ origProps.put("paimon.catalog.type", "rest");
+ Assertions.assertEquals(1,
StorageProperties.createAll(origProps).size());
+ }
+
}
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/S3PropertiesTest.java
b/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/S3PropertiesTest.java
index a766be98f15..4852eff9032 100644
---
a/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/S3PropertiesTest.java
+++
b/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/S3PropertiesTest.java
@@ -19,7 +19,6 @@ package org.apache.doris.datasource.property.storage;
import org.apache.doris.common.ExceptionChecker;
import org.apache.doris.common.UserException;
-import
org.apache.doris.datasource.property.storage.exception.StoragePropertiesException;
import com.google.common.collect.Maps;
import mockit.Expectations;
@@ -71,8 +70,8 @@ public class S3PropertiesTest {
origProps = new HashMap<>();
origProps.put("s3.endpoint",
"s3-fips.dualstack.us-east-2.amazonaws.com");
origProps.put("s3.access_key", "myS3AccessKey");
- ExceptionChecker.expectThrowsWithMsg(StoragePropertiesException.class,
- "Please set s3.access_key and s3.secret_key", () ->
StorageProperties.createAll(origProps));
+ ExceptionChecker.expectThrowsWithMsg(IllegalArgumentException.class,
+ "Both the access key and the secret key must be set.", () ->
StorageProperties.createAll(origProps));
origProps.put("s3.secret_key", "myS3SecretKey");
ExceptionChecker.expectThrowsNoException(() ->
StorageProperties.createAll(origProps));
}
@@ -221,6 +220,11 @@ public class S3PropertiesTest {
Assertions.assertEquals("arn:aws:iam::123456789012:role/MyTestRole",
backendProperties.get("AWS_ROLE_ARN"));
Assertions.assertEquals("external-123",
backendProperties.get("AWS_EXTERNAL_ID"));
+ origProps.remove("s3.external_id");
+ s3Props = (S3Properties) StorageProperties.createPrimary(origProps);
+ backendProperties = s3Props.getBackendConfigProperties();
+ Assertions.assertNull(backendProperties.get("AWS_EXTERNAL_ID"));
+ Assertions.assertEquals("arn:aws:iam::123456789012:role/MyTestRole",
backendProperties.get("AWS_ROLE_ARN"));
}
@Test
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
