From 8430f5b728596c5264fb4c7e0fb2170113a4d5ae Mon Sep 17 00:00:00 2001 From: Calvin Kirs Date: Tue, 19 Aug 2025 20:13:39 +0800 Subject: [PATCH 1/7] [fix](params-refactor)Enhance Object Storage Parameter Validation and Exception Handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Key Changes Improved Object Storage Parameter Validation AccessKey/SecretKey check: If AccessKey is set, SecretKey must also be set and neither can be empty. ↳ ExternalId and S3 IAM Role check: If ExternalId is set, S3 IAM Role must also be configured. ↳ Removed redundant validation logic to simplify and streamline parameter checks. Enhanced Error Messages Throw clear and descriptive exceptions for missing or invalid parameters, avoiding vague runtime errors. Fix Iceberg HMS Kerberos Parameter Issue Previously, exceptions during Catalog initialization with Kerberos were swallowed. ↳ Now, exceptions are properly thrown with detailed messages when parameter construction or initialization fails. --- .../doris/datasource/CatalogProperty.java | 24 ++++++++--- .../doris/datasource/ExternalCatalog.java | 27 ++---------- .../datasource/iceberg/dlf/DLFCatalog.java | 27 +++++------- .../property/PropertyConverter.java | 3 -- .../IcebergFileSystemMetaStoreProperties.java | 9 +--- .../AbstractS3CompatibleProperties.java | 5 +++ .../property/storage/COSProperties.java | 17 -------- .../property/storage/MinioProperties.java | 17 -------- .../property/storage/OBSProperties.java | 17 -------- .../property/storage/OSSProperties.java | 12 ------ .../property/storage/S3Properties.java | 42 +++++++------------ .../property/storage/OSSPropertiesTest.java | 10 +++++ .../property/storage/S3PropertiesTest.java | 5 +++ 13 files changed, 70 insertions(+), 145 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 502d558b69834b..22f5b3c6c7c4f9 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 @@ -18,13 +18,13 @@ package org.apache.doris.datasource; import org.apache.doris.common.UserException; -import org.apache.doris.datasource.property.PropertyConverter; import org.apache.doris.datasource.property.metastore.MetastoreProperties; import org.apache.doris.datasource.property.storage.StorageProperties; 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; @@ -78,7 +78,7 @@ public Map getProperties() { public void modifyCatalogProps(Map props) { synchronized (this) { - properties.putAll(PropertyConverter.convertToMetaProperties(props)); + properties.putAll(props); resetAllCaches(); } } @@ -190,9 +190,23 @@ public Map getHadoopProperties() { if (hadoopProperties == null) { synchronized (this) { if (hadoopProperties == null) { - Map result = getProperties(); - result.putAll(PropertyConverter.convertToHadoopFSProperties(getProperties())); - this.hadoopProperties = result; + hadoopProperties = new HashMap<>(); + Map 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); + } + }); + } else { + LOG.warn("Hadoop storage config is null for storage type: {}", sp.getType()); + } + } } } } 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 f751e7aff706c6..73ca710c44fab8 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 @@ -51,7 +51,6 @@ 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; @@ -169,13 +168,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 convertedProperties = null; - protected Optional useMetaCache = Optional.empty(); protected MetaCache> metaCache; protected ExecutionAuthenticator executionAuthenticator; @@ -343,6 +335,8 @@ public final synchronized void makeSureInitialized() { } } catch (Exception e) { this.errorMsg = ExceptionUtils.getRootCauseMessage(e); + throw new RuntimeException("Failed to init catalog: " + name + ", error: " + + this.errorMsg, e); } finally { isInitializing = false; } @@ -579,10 +573,6 @@ private List> getFilteredDatabaseNames() { public synchronized void resetToUninitialized(boolean invalidCache) { this.objectCreated = false; this.initialized = false; - synchronized (this.propLock) { - this.convertedProperties = null; - } - synchronized (this.confLock) { this.cachedConf = null; } @@ -752,17 +742,7 @@ public List getDbIds() { @Override public Map 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 @@ -1039,7 +1019,6 @@ public void gsonPostProcess() throws IOException { } } } - this.propLock = new byte[0]; this.confLock = new byte[0]; this.initialized = false; setDefaultPropsIfMissing(true); 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 e51292feff2dc9..fb90260f09aeae 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.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 @@ protected TableOperations newTableOps(TableIdentifier tableIdentifier) { protected FileIO initializeFileIO(Map 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 b3464eb382f44e..1a9137ede5c7bf 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 fa94f79df77943..7dd97b028b202a 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.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 Catalog initCatalog(String catalogName, Map catalogProps, List storagePropertiesList) { Configuration configuration = buildConfiguration(storagePropertiesList); HadoopCatalog catalog = new HadoopCatalog(); + buildCatalogProps(catalogProps, storagePropertiesList); catalog.setConf(configuration); try { this.executionAuthenticator.execute(() -> { @@ -72,9 +72,7 @@ private Configuration buildConfiguration(List storageProperti return configuration; } - private Map buildCatalogProps(List storagePropertiesList) { - Map props = new HashMap<>(origProps); - + private void buildCatalogProps(Map props, List storagePropertiesList) { if (storagePropertiesList.size() == 1 && storagePropertiesList.get(0) instanceof HdfsProperties) { HdfsProperties hdfsProps = (HdfsProperties) storagePropertiesList.get(0); if (hdfsProps.isKerberos()) { @@ -83,9 +81,6 @@ private Map buildCatalogProps(List storagePro 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 bd5a4a0824c738..e4ba31d5dcdf89 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 void initNormalizeAndCheckProps() { 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 access key and 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 8e09576891e3f3..b284ba5666e2a1 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 @@ protected COSProperties(Map origProps) { 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 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 6f0496ae5ff562..dd8d2735324c8f 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 @@ protected MinioProperties(Map origProps) { 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 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 5e9c513d67e048..eea6c24ddc42aa 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 OBSProperties(Map origProps) { // 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 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 4cb43b3155a15c..d5f9625bc20bfd 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 @@ 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; @@ -247,17 +246,6 @@ public void initNormalizeAndCheckProps() { 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 bd05f39d4abb02..e4a15fb5a8ec86 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 @@ 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.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 S3Properties(Map origProps) { @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 @@ protected static boolean guessIsMe(Map origProps) { * 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 @@ protected static boolean guessIsMe(Map origProps) { .filter(Objects::nonNull) .findFirst() .orElse(null); - if (!Strings.isNullOrEmpty(region)) { + if (StringUtils.isNotBlank(region)) { return true; } return false; @@ -260,9 +244,10 @@ protected Set endpointPatterns() { public Map getBackendConfigProperties() { Map 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 AwsCredentialsProvider getAwsCredentialsProvider() { } if (StringUtils.isNotBlank(s3IAMRole)) { StsClient stsClient = StsClient.builder() + .region(Region.of(region)) .credentialsProvider(InstanceProfileCredentialsProvider.create()) .build(); @@ -289,7 +275,7 @@ public AwsCredentialsProvider getAwsCredentialsProvider() { .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 void initializeHadoopStorageConfig() { 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/storage/OSSPropertiesTest.java b/fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/OSSPropertiesTest.java index 20efe3ebe97a4a..56590016764a86 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 @@ -151,6 +151,16 @@ public void testMissingAccessKey() { () -> StorageProperties.createPrimary(origProps)); } + @Test + public void testDlfProperties() { + Map 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 public void testMissingSecretKey() { Map origProps = new HashMap<>(); 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 a766be98f15282..9009ac8fc37192 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 @@ -221,6 +221,11 @@ public void testS3IamRoleWithExternalId() throws UserException { 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 From 62aac30b4757ed375415faa4d5159e9cc8cb3253 Mon Sep 17 00:00:00 2001 From: Calvin Kirs Date: Wed, 20 Aug 2025 11:40:20 +0800 Subject: [PATCH 2/7] checkstyle --- .../apache/doris/datasource/ExternalCatalog.java | 11 +++++++---- .../datasource/property/storage/OSSProperties.java | 2 +- .../property/storage/OSSPropertiesTest.java | 14 ++++++++++++++ 3 files changed, 22 insertions(+), 5 deletions(-) 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 73ca710c44fab8..0f72377a93cd10 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 @@ -195,7 +195,8 @@ public ExternalCatalog(long catalogId, String name, InitCatalogLog.Type logType, */ protected synchronized void initPreExecutionAuthenticator() { if (executionAuthenticator == null) { - executionAuthenticator = new ExecutionAuthenticator(){}; + executionAuthenticator = new ExecutionAuthenticator() { + }; } } @@ -336,7 +337,7 @@ public final synchronized void makeSureInitialized() { } catch (Exception e) { this.errorMsg = ExceptionUtils.getRootCauseMessage(e); throw new RuntimeException("Failed to init catalog: " + name + ", error: " - + this.errorMsg, e); + + this.errorMsg + " You can use 'SHOW CATALOGS' to find the root cause", e); } finally { isInitializing = false; } @@ -908,7 +909,8 @@ public Optional> getDbForReplay(String * @return */ protected ExternalDatabase buildDbForInit(String remoteDbName, String localDbName, - long dbId, InitCatalogLog.Type logType, boolean checkExists) { + long dbId, InitCatalogLog.Type logType, + boolean checkExists) { // Step 1: Map local database name if not already provided if (localDbName == null && remoteDbName != null) { localDbName = fromRemoteDatabaseName(remoteDbName); @@ -1307,7 +1309,7 @@ public boolean enableAutoAnalyze() { @Override public void truncateTable(String dbName, String tableName, PartitionNames partitionNames, boolean forceDrop, - String rawTruncateSql) throws DdlException { + String rawTruncateSql) throws DdlException { makeSureInitialized(); if (metadataOps == null) { throw new DdlException("Truncate table is not supported for catalog: " + getName()); @@ -1398,6 +1400,7 @@ public ThreadPoolExecutor getThreadPoolWithPreAuth() { /** * Check if an external view exists. + * * @param dbName * @param viewName * @return 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 d5f9625bc20bfd..0d7d41f2cc12f2 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 @@ -202,7 +202,7 @@ private static boolean isKnownObjectStorage(String value) { 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; 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 56590016764a86..7a949dc0d36a7d 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 @@ -186,4 +186,18 @@ public void testNotEndpoint() throws UserException { 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 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()); + } + } From 41c5c45f564f43be902d5d4f4a78bc4b7d74f20d Mon Sep 17 00:00:00 2001 From: Calvin Kirs Date: Wed, 20 Aug 2025 11:42:42 +0800 Subject: [PATCH 3/7] checkstyle --- .../org/apache/doris/datasource/ExternalCatalog.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) 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 0f72377a93cd10..266f3f1744cc53 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 @@ -195,8 +195,7 @@ public ExternalCatalog(long catalogId, String name, InitCatalogLog.Type logType, */ protected synchronized void initPreExecutionAuthenticator() { if (executionAuthenticator == null) { - executionAuthenticator = new ExecutionAuthenticator() { - }; + executionAuthenticator = new ExecutionAuthenticator(){}; } } @@ -909,8 +908,7 @@ public Optional> getDbForReplay(String * @return */ protected ExternalDatabase buildDbForInit(String remoteDbName, String localDbName, - long dbId, InitCatalogLog.Type logType, - boolean checkExists) { + long dbId, InitCatalogLog.Type logType, boolean checkExists) { // Step 1: Map local database name if not already provided if (localDbName == null && remoteDbName != null) { localDbName = fromRemoteDatabaseName(remoteDbName); @@ -1309,7 +1307,7 @@ public boolean enableAutoAnalyze() { @Override public void truncateTable(String dbName, String tableName, PartitionNames partitionNames, boolean forceDrop, - String rawTruncateSql) throws DdlException { + String rawTruncateSql) throws DdlException { makeSureInitialized(); if (metadataOps == null) { throw new DdlException("Truncate table is not supported for catalog: " + getName()); @@ -1400,7 +1398,6 @@ public ThreadPoolExecutor getThreadPoolWithPreAuth() { /** * Check if an external view exists. - * * @param dbName * @param viewName * @return From 838f8a85a88b62a264aed805628306b6f1e2a57d Mon Sep 17 00:00:00 2001 From: Calvin Kirs Date: Wed, 20 Aug 2025 12:05:33 +0800 Subject: [PATCH 4/7] checkstyle --- .../main/java/org/apache/doris/datasource/CatalogProperty.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 22f5b3c6c7c4f9..eeee177f625bcf 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 @@ -204,7 +204,7 @@ public Map getHadoopProperties() { } }); } else { - LOG.warn("Hadoop storage config is null for storage type: {}", sp.getType()); + LOG.info("Hadoop storage config is null for storage type: {}", sp.getType()); } } } From a8f526ffc4aa71ee43e5503631ed22b1c0e6459c Mon Sep 17 00:00:00 2001 From: Calvin Kirs Date: Wed, 20 Aug 2025 12:08:38 +0800 Subject: [PATCH 5/7] checkstyle --- .../main/java/org/apache/doris/datasource/CatalogProperty.java | 2 -- 1 file changed, 2 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 eeee177f625bcf..d2c4fb7acbd88a 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 @@ -203,8 +203,6 @@ public Map getHadoopProperties() { hadoopProperties.put(key, value); } }); - } else { - LOG.info("Hadoop storage config is null for storage type: {}", sp.getType()); } } } From 884584009e6863ac379df3eace394c34bb66b108 Mon Sep 17 00:00:00 2001 From: Calvin Kirs Date: Wed, 20 Aug 2025 14:13:27 +0800 Subject: [PATCH 6/7] checkstyle --- .../storage/AbstractS3CompatibleProperties.java | 2 +- .../property/storage/COSPropertiesTest.java | 8 +++++--- .../property/storage/MinioPropertiesTest.java | 13 ++++++++----- .../property/storage/OBSPropertyTest.java | 17 ++++++++++------- .../property/storage/OSSPropertiesTest.java | 13 ++++++++----- .../property/storage/S3PropertiesTest.java | 5 ++--- 6 files changed, 34 insertions(+), 24 deletions(-) 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 e4ba31d5dcdf89..3b0bca283652ce 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 @@ -138,7 +138,7 @@ public void initNormalizeAndCheckProps() { //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 access key and secret key must be set."); + throw new IllegalArgumentException("Both the access key and the secret key must be set."); } } 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 5d718e0cbf2267..0cf30e49c04ade 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 void testGetRegionWithDefault() throws UserException { 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 void testMissingAccessKey() { 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 fe6d8c5e859a21..bd6fbf2239a17a 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 @@ 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 void testGuessIsMeWithMinio() { 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 e700ede2de3acf..2c6f5ffab21556 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 @@ 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 void testBasicCreateTest() throws UserException { // 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 void testGetRegionWithDefault() throws UserException { 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 7a949dc0d36a7d..31b56a2bd9b429 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 @@ 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,11 @@ public void testMissingAccessKey() { Map 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 @@ -166,9 +167,11 @@ public void testMissingSecretKey() { Map 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 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 9009ac8fc37192..4852eff903201f 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 @@ 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 void testS3Properties() { 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)); } From c5b6245dfe7abc1e3bcd0682f8510bc652f7bf36 Mon Sep 17 00:00:00 2001 From: Calvin Kirs Date: Wed, 20 Aug 2025 15:14:49 +0800 Subject: [PATCH 7/7] checkstyle --- .../apache/doris/datasource/property/PropertyConverterTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 498b42e56798c4..506ac6cdea2237 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 @@ -234,7 +234,7 @@ public void testOssHdfsProperties() throws Exception { 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"