diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/ConnectionProperties.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/ConnectionProperties.java index 8cafb5b6d5b6a3..28b23276b7030f 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/ConnectionProperties.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/ConnectionProperties.java @@ -32,7 +32,6 @@ public class ConnectionProperties { protected ConnectionProperties(Map origProps) { this.origProps = origProps; - normalizedAndCheckProps(); } protected void normalizedAndCheckProps() { @@ -65,7 +64,7 @@ protected void normalizedAndCheckProps() { // Subclass can override this method to load properties from file. // The return value is the properties loaded from file, not include original properties protected Map loadConfigFromFile(String resourceConfig) { - if (Strings.isNullOrEmpty(resourceConfig)) { + if (Strings.isNullOrEmpty(origProps.get(resourceConfig))) { return Maps.newHashMap(); } Configuration conf = ConfigurationUtils.loadConfigurationFromHadoopConfDir(resourceConfig); diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/ConnectorProperty.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/ConnectorProperty.java index 7e45836d84a973..abe4880e77da71 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/ConnectorProperty.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/ConnectorProperty.java @@ -18,6 +18,10 @@ package org.apache.doris.datasource.property; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +@Retention(RetentionPolicy.RUNTIME) public @interface ConnectorProperty { String[] names() default {}; String description() default ""; diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/PropertyUtils.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/PropertyUtils.java index ea4d1fa80aaefb..86c25655db7216 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/PropertyUtils.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/PropertyUtils.java @@ -28,6 +28,7 @@ public class PropertyUtils { public static List getConnectorProperties(Class clazz) { List fields = Lists.newArrayList(); for (Field field : clazz.getDeclaredFields()) { + field.setAccessible(true); if (field.isAnnotationPresent(ConnectorProperty.class)) { // Get annotation of the field ConnectorProperty connectorProperty = field.getAnnotation(ConnectorProperty.class); diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/metastore/AliyunDLFProperties.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/metastore/AliyunDLFProperties.java index bf27e6ca903903..966d738028071b 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/metastore/AliyunDLFProperties.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/metastore/AliyunDLFProperties.java @@ -21,7 +21,6 @@ import com.google.common.base.Strings; import com.google.common.collect.Maps; -import lombok.Getter; import org.apache.paimon.options.Options; import java.util.Map; @@ -30,7 +29,7 @@ public class AliyunDLFProperties extends MetastoreProperties { @ConnectorProperty(names = {"dlf.access_key", "dlf.catalog.accessKeyId"}, description = "The access key of the Aliyun DLF.") - private String dlfAccessKey = ""; + public String dlfAccessKey = ""; @ConnectorProperty(names = {"dlf.secret_key", "dlf.catalog.accessKeySecret"}, description = "The secret key of the Aliyun DLF.") @@ -50,6 +49,7 @@ public class AliyunDLFProperties extends MetastoreProperties { private String dlfUid = ""; @ConnectorProperty(names = {"dlf.access.public", "dlf.catalog.accessPublic"}, + required = false, description = "Enable public access to Aliyun DLF.") private String dlfAccessPublic = "false"; @@ -99,37 +99,6 @@ private String getEndpointOrFromRegion(String endpoint, String region, String dl @Override protected String getResourceConfigPropName() { - return "dlf.resouce_config"; - } - - @Getter - public static class OSSConfiguration { - private Map conf = Maps.newHashMap(); - - public OSSConfiguration(AliyunDLFProperties props) { - conf.put("oss.region", getOssRegionFromDlfRegion(props.dlfRegion)); - conf.put("oss.endpoint", getOssEndpointFromDlfRegion(props.dlfRegion, props.dlfAccessPublic)); - conf.put("oss.access_key", props.dlfAccessKey); - conf.put("oss.secret_key", props.dlfSecretKey); - } - - private String getOssRegionFromDlfRegion(String dlfRegion) { - return "oss-" + dlfRegion; - } - - private String getOssEndpointFromDlfRegion(String dlfRegion, String dlfAccessPublic) { - if ("true".equalsIgnoreCase(dlfAccessPublic)) { - return "oss-" + dlfRegion + ".aliyuncs.com"; - } else { - return "oss-" + dlfRegion + "-internal.aliyuncs.com"; - } - } - - private String getEndpointOrFromRegion(String endpoint, String region) { - if (!Strings.isNullOrEmpty(endpoint)) { - return endpoint; - } - return "dlf-vpc." + region + ".aliyuncs.com"; - } + return "dlf.resource_config"; } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/metastore/MetastoreProperties.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/metastore/MetastoreProperties.java index 5144c57c8e1018..970784716ecd4d 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/metastore/MetastoreProperties.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/metastore/MetastoreProperties.java @@ -52,71 +52,101 @@ public static MetastoreProperties create(Map origProps) { // 'type'='hms', // "paimon.catalog.type" = "hms",dlf, filesystem + Type msType = Type.UNKNOWN; if (origProps.containsKey(METASTORE_TYPE)) { String type = origProps.get(METASTORE_TYPE); switch (type) { case "hms": - return new HMSProperties(origProps); + msType = Type.HMS; case "glue": - return new AWSGlueProperties(origProps); + msType = Type.GLUE; case "dlf": - return new AliyunDLFProperties(origProps); + msType = Type.DLF; case "rest": - return new IcebergRestProperties(origProps); + msType = Type.ICEBERG_REST; case "dataproc": - return new DataProcProperties(origProps); + msType = Type.DATAPROC; case "filesystem": - return new FileMetastoreProperties(origProps); + msType = Type.FILE_SYSTEM; default: - throw new IllegalArgumentException("Unknown metastore type: " + type); + throw new IllegalArgumentException("Unknown 'metastore.type': " + type); } } else if (origProps.containsKey("hive.metastore.type")) { String type = origProps.get("hive.metastore.type"); switch (type) { case "hms": - return new HMSProperties(origProps); + msType = Type.HMS; case "glue": - return new AWSGlueProperties(origProps); + msType = Type.GLUE; case "dlf": - return new AliyunDLFProperties(origProps); + msType = Type.DLF; default: - throw new IllegalArgumentException("Unknown metastore type: " + type); + throw new IllegalArgumentException("Unknown 'hive.metastore.type': " + type); } } else if (origProps.containsKey("iceberg.catalog.type")) { String type = origProps.get("iceberg.catalog.type"); switch (type) { case "hms": - return new HMSProperties(origProps); + msType = Type.HMS; case "glue": - return new AWSGlueProperties(origProps); + msType = Type.GLUE; case "rest": - return new IcebergRestProperties(origProps); + msType = Type.ICEBERG_REST; case "hadoop": - return new FileMetastoreProperties(origProps); + msType = Type.FILE_SYSTEM; default: - throw new IllegalArgumentException("Unknown iceberg catalog type: " + type); + throw new IllegalArgumentException("Unknown 'iceberg.catalog.type': " + type); } } else if (origProps.containsKey("paimon.catalog.type")) { String type = origProps.get("paimon.catalog.type"); switch (type) { case "hms": - return new HMSProperties(origProps); + msType = Type.HMS; case "dlf": - return new AliyunDLFProperties(origProps); + msType = Type.DLF; default: // default is "filesystem" - return new FileMetastoreProperties(origProps); + msType = Type.FILE_SYSTEM; } } else if (origProps.containsKey("type")) { String type = origProps.get("type"); switch (type) { case "hms": - return new HMSProperties(origProps); + msType = Type.HMS; default: - throw new IllegalArgumentException("Unknown metastore type: " + type); + throw new IllegalArgumentException("Unknown metastore 'type': " + type); } } - throw new IllegalArgumentException("Can not find metastore type in properties"); + + return MetastoreProperties.create(msType, origProps); + } + + public static MetastoreProperties create(Type type, Map origProps) { + MetastoreProperties metastoreProperties; + switch (type) { + case HMS: + metastoreProperties = new HMSProperties(origProps); + break; + case GLUE: + metastoreProperties = new AWSGlueProperties(origProps); + break; + case DLF: + metastoreProperties = new AliyunDLFProperties(origProps); + break; + case ICEBERG_REST: + metastoreProperties = new IcebergRestProperties(origProps); + break; + case DATAPROC: + metastoreProperties = new DataProcProperties(origProps); + break; + case FILE_SYSTEM: + metastoreProperties = new FileMetastoreProperties(origProps); + break; + default: + throw new IllegalArgumentException("Unknown metastore type: " + type); + } + metastoreProperties.normalizedAndCheckProps(); + return metastoreProperties; } protected MetastoreProperties(Type type, Map origProps) { diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/StorageProperties.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/StorageProperties.java index 545069066cd0fb..6bd1a58492c7b1 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/StorageProperties.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/StorageProperties.java @@ -74,7 +74,14 @@ public static List create(Map origProps) { throw new RuntimeException("Unsupported native AZURE filesystem"); } - throw new RuntimeException("Unknown storage type"); + if (storageProperties.isEmpty()) { + throw new RuntimeException("Unknown storage type"); + } else { + for (StorageProperties storageProperty : storageProperties) { + storageProperty.normalizedAndCheckProps(); + } + } + return storageProperties; } protected StorageProperties(Type type, Map origProps) { @@ -88,6 +95,7 @@ private static boolean isFsSupport(Map origProps, String fsEnabl protected static boolean checkIdentifierKey(Map origProps, List fields) { for (Field field : fields) { + field.setAccessible(true); ConnectorProperty annotation = field.getAnnotation(ConnectorProperty.class); for (String key : annotation.names()) { if (origProps.containsKey(key)) { diff --git a/fe/fe-core/src/test/java/org/apache/doris/datasource/property/metastore/AliyunDLFPropertiesTest.java b/fe/fe-core/src/test/java/org/apache/doris/datasource/property/metastore/AliyunDLFPropertiesTest.java new file mode 100644 index 00000000000000..c9d369e627fab1 --- /dev/null +++ b/fe/fe-core/src/test/java/org/apache/doris/datasource/property/metastore/AliyunDLFPropertiesTest.java @@ -0,0 +1,311 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.datasource.property.metastore; + +import org.apache.doris.datasource.property.metastore.MetastoreProperties.Type; + +import com.google.common.collect.Maps; +import org.apache.paimon.options.Options; +import org.junit.Assert; +import org.junit.Test; + +import java.util.Map; + +public class AliyunDLFPropertiesTest { + + @Test + public void testBasicProperties() { + Map props = Maps.newHashMap(); + props.put("dlf.access_key", "test_access_key"); + props.put("dlf.secret_key", "test_secret_key"); + props.put("dlf.region", "cn-hangzhou"); + props.put("dlf.uid", "test_uid"); + + AliyunDLFProperties dlfProperties = (AliyunDLFProperties) MetastoreProperties.create(Type.DLF, props); + Options options = new Options(); + dlfProperties.toPaimonOptions(options); + + Assert.assertEquals("test_access_key", options.get("dlf.catalog.accessKeyId")); + Assert.assertEquals("test_secret_key", options.get("dlf.catalog.accessKeySecret")); + Assert.assertEquals("cn-hangzhou", options.get("dlf.catalog.region")); + Assert.assertEquals("test_uid", options.get("dlf.catalog.uid")); + Assert.assertEquals("false", options.get("dlf.catalog.accessPublic")); + Assert.assertEquals("dlf-vpc.cn-hangzhou.aliyuncs.com", options.get("dlf.catalog.endpoint")); + Assert.assertEquals("DLF_ONLY", options.get("dlf.catalog.proxyMode")); + Assert.assertEquals("false", options.get("dlf.catalog.createDefaultDBIfNotExist")); + } + + @Test + public void testCustomEndpoint() { + Map props = Maps.newHashMap(); + props.put("dlf.access_key", "test_access_key"); + props.put("dlf.secret_key", "test_secret_key"); + props.put("dlf.region", "cn-hangzhou"); + props.put("dlf.endpoint", "custom.endpoint.com"); + props.put("dlf.uid", "test_uid"); + props.put("dlf.access.public", "true"); + + AliyunDLFProperties dlfProperties = (AliyunDLFProperties) MetastoreProperties.create(Type.DLF, props); + Options options = new Options(); + dlfProperties.toPaimonOptions(options); + + Assert.assertEquals("custom.endpoint.com", options.get("dlf.catalog.endpoint")); + } + + @Test + public void testPublicAccess() { + Map props = Maps.newHashMap(); + props.put("dlf.access_key", "test_access_key"); + props.put("dlf.secret_key", "test_secret_key"); + props.put("dlf.region", "cn-hangzhou"); + props.put("dlf.uid", "test_uid"); + props.put("dlf.access.public", "true"); + + AliyunDLFProperties dlfProperties = (AliyunDLFProperties) MetastoreProperties.create(Type.DLF, props); + Options options = new Options(); + dlfProperties.toPaimonOptions(options); + + Assert.assertEquals("dlf.cn-hangzhou.aliyuncs.com", options.get("dlf.catalog.endpoint")); + } + + @Test + public void testOtherDlfProperties() { + Map props = Maps.newHashMap(); + props.put("dlf.access_key", "test_access_key"); + props.put("dlf.secret_key", "test_secret_key"); + props.put("dlf.region", "cn-hangzhou"); + props.put("dlf.uid", "test_uid"); + props.put("dlf.custom.property", "custom_value"); + + AliyunDLFProperties dlfProperties = (AliyunDLFProperties) MetastoreProperties.create(Type.DLF, props); + Options options = new Options(); + dlfProperties.toPaimonOptions(options); + + Assert.assertEquals("custom_value", options.get("dlf.custom.property")); + } + + @Test + public void testResourceConfigPropName() { + Map props = Maps.newHashMap(); + AliyunDLFProperties dlfProperties = (AliyunDLFProperties) MetastoreProperties.create(Type.DLF, props); + Assert.assertEquals("dlf.resource_config", dlfProperties.getResourceConfigPropName()); + } + + @Test + public void testEmptyProperties() { + Map props = Maps.newHashMap(); + try { + MetastoreProperties.create(Type.DLF, props); + Assert.fail("Should throw IllegalArgumentException"); + } catch (IllegalArgumentException e) { + // Any of these required properties should be mentioned in the error message + Assert.assertTrue("Error message should mention the missing property", + e.getMessage().contains("is required")); + } + } + + @Test + public void testMissingRequiredProperties() { + Map props = Maps.newHashMap(); + try { + MetastoreProperties.create(Type.DLF, props); + Assert.fail("Should throw IllegalArgumentException"); + } catch (IllegalArgumentException e) { + // Any of these required properties should be mentioned in the error message + Assert.assertTrue("Error message should mention the missing property", + e.getMessage().contains("dlf.access_key") || + e.getMessage().contains("dlf.catalog.accessKeyId")); + } + } + + @Test + public void testMissingAccessKey() { + Map props = Maps.newHashMap(); + props.put("dlf.secret_key", "test_secret_key"); + props.put("dlf.region", "cn-hangzhou"); + props.put("dlf.uid", "test_uid"); + + try { + MetastoreProperties.create(Type.DLF, props); + Assert.fail("Should throw IllegalArgumentException"); + } catch (IllegalArgumentException e) { + Assert.assertTrue("Error message should mention access_key", + e.getMessage().contains("dlf.access_key") || + e.getMessage().contains("dlf.catalog.accessKeyId")); + Assert.assertTrue("Error message should mention it's required", + e.getMessage().contains("is required")); + } + } + + @Test + public void testEmptyRequiredProperty() { + Map props = Maps.newHashMap(); + props.put("dlf.access_key", ""); // Empty access key + props.put("dlf.secret_key", "test_secret_key"); + props.put("dlf.region", "cn-hangzhou"); + props.put("dlf.uid", "test_uid"); + + try { + MetastoreProperties.create(Type.DLF, props); + Assert.fail("Should throw IllegalArgumentException"); + } catch (IllegalArgumentException e) { + Assert.assertTrue("Error message should mention access_key", + e.getMessage().contains("dlf.access_key") || + e.getMessage().contains("dlf.catalog.accessKeyId")); + Assert.assertTrue("Error message should mention it's required", + e.getMessage().contains("is required")); + } + } + + @Test + public void testMissingSecretKey() { + Map props = Maps.newHashMap(); + props.put("dlf.access_key", "test_access_key"); + props.put("dlf.region", "cn-hangzhou"); + props.put("dlf.uid", "test_uid"); + + try { + MetastoreProperties.create(Type.DLF, props); + Assert.fail("Should throw IllegalArgumentException"); + } catch (IllegalArgumentException e) { + Assert.assertTrue("Error message should mention secret_key", + e.getMessage().contains("dlf.secret_key") || + e.getMessage().contains("dlf.catalog.accessKeySecret")); + Assert.assertTrue("Error message should mention it's required", + e.getMessage().contains("is required")); + } + } + + @Test + public void testMissingRegion() { + Map props = Maps.newHashMap(); + props.put("dlf.access_key", "test_access_key"); + props.put("dlf.secret_key", "test_secret_key"); + props.put("dlf.uid", "test_uid"); + + try { + MetastoreProperties.create(Type.DLF, props); + Assert.fail("Should throw IllegalArgumentException"); + } catch (IllegalArgumentException e) { + Assert.assertTrue("Error message should mention region", + e.getMessage().contains("dlf.region")); + Assert.assertTrue("Error message should mention it's required", + e.getMessage().contains("is required")); + } + } + + @Test + public void testMissingUid() { + Map props = Maps.newHashMap(); + props.put("dlf.access_key", "test_access_key"); + props.put("dlf.secret_key", "test_secret_key"); + props.put("dlf.region", "cn-hangzhou"); + + try { + MetastoreProperties.create(Type.DLF, props); + Assert.fail("Should throw IllegalArgumentException"); + } catch (IllegalArgumentException e) { + Assert.assertTrue("Error message should mention uid", + e.getMessage().contains("dlf.uid") || + e.getMessage().contains("dlf.catalog.uid")); + Assert.assertTrue("Error message should mention it's required", + e.getMessage().contains("is required")); + } + } + + @Test + public void testLoadConfigFromFile() { + Map props = Maps.newHashMap(); + props.put("dlf.resource_config", "test-dlf-config.xml"); + props.put("dlf.access_key", "test_access_key"); + props.put("dlf.secret_key", "test_secret_key"); + props.put("dlf.region", "cn-hangzhou"); + props.put("dlf.uid", "test_uid"); + + AliyunDLFProperties dlfProperties = (AliyunDLFProperties) MetastoreProperties.create(Type.DLF, props); + // Verify that the properties are loaded correctly + // Note: This assumes the config file exists and contains certain properties + Options options = new Options(); + dlfProperties.toPaimonOptions(options); + Assert.assertEquals("test_access_key", options.get("dlf.catalog.accessKeyId")); + } + + @Test + public void testPropertyOverride() { + // Test that properties from original props override those from config file + Map props = Maps.newHashMap(); + props.put("dlf.resource_config", "test-dlf-config.xml"); + props.put("dlf.access_key", "override_access_key"); // This should override any value from config file + props.put("dlf.secret_key", "test_secret_key"); + props.put("dlf.region", "cn-hangzhou"); + props.put("dlf.uid", "test_uid"); + + AliyunDLFProperties dlfProperties = (AliyunDLFProperties) MetastoreProperties.create(Type.DLF, props); + Options options = new Options(); + dlfProperties.toPaimonOptions(options); + Assert.assertEquals("override_access_key", options.get("dlf.catalog.accessKeyId")); + } + + @Test + public void testAlternativePropertyNames() { + Map props = Maps.newHashMap(); + // Test alternative property name "dlf.catalog.accessKeyId" instead of "dlf.access_key" + props.put("dlf.catalog.accessKeyId", "test_access_key"); + props.put("dlf.catalog.accessKeySecret", "test_secret_key"); + props.put("dlf.region", "cn-hangzhou"); + props.put("dlf.uid", "test_uid"); + + AliyunDLFProperties dlfProperties = (AliyunDLFProperties) MetastoreProperties.create(Type.DLF, props); + Options options = new Options(); + dlfProperties.toPaimonOptions(options); + Assert.assertEquals("test_access_key", options.get("dlf.catalog.accessKeyId")); + } + + @Test + public void testNonRequiredProperties() { + Map props = Maps.newHashMap(); + // Set all required properties + props.put("dlf.access_key", "test_access_key"); + props.put("dlf.secret_key", "test_secret_key"); + props.put("dlf.region", "cn-hangzhou"); + props.put("dlf.uid", "test_uid"); + // Don't set non-required property dlf.endpoint + + AliyunDLFProperties dlfProperties = (AliyunDLFProperties) MetastoreProperties.create(Type.DLF, props); + Options options = new Options(); + dlfProperties.toPaimonOptions(options); + // Should still work and use default endpoint + Assert.assertEquals("dlf-vpc.cn-hangzhou.aliyuncs.com", options.get("dlf.catalog.endpoint")); + } + + @Test + public void testEmptyResourceConfig() { + Map props = Maps.newHashMap(); + props.put("dlf.access_key", "test_access_key"); + props.put("dlf.secret_key", "test_secret_key"); + props.put("dlf.region", "cn-hangzhou"); + props.put("dlf.uid", "test_uid"); + props.put("dlf.resource_config", ""); // Empty resource config + + AliyunDLFProperties dlfProperties = (AliyunDLFProperties) MetastoreProperties.create(Type.DLF, props); + Options options = new Options(); + dlfProperties.toPaimonOptions(options); + // Should still work with direct properties + Assert.assertEquals("test_access_key", options.get("dlf.catalog.accessKeyId")); + } +} \ No newline at end of file