morningman commented on code in PR #50031: URL: https://github.com/apache/doris/pull/50031#discussion_r2045464068
########## fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/StorageProperties.java: ########## @@ -0,0 +1,161 @@ +// 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.storage; + +import org.apache.doris.common.UserException; +import org.apache.doris.datasource.property.ConnectionProperties; +import org.apache.doris.datasource.property.ConnectorProperty; + +import com.google.common.collect.Lists; +import lombok.Getter; + +import java.lang.reflect.Field; +import java.util.List; +import java.util.Map; + +public abstract class StorageProperties extends ConnectionProperties { + + public static final String FS_HDFS_SUPPORT = "fs.hdfs.support"; + public static final String FS_S3_SUPPORT = "fs.s3.support"; + public static final String FS_GCS_SUPPORT = "fs.gcs.support"; + public static final String FS_AZURE_SUPPORT = "fs.azure.support"; + public static final String FS_OSS_SUPPORT = "fs.oss.support"; + public static final String FS_OBS_SUPPORT = "fs.obs.support"; + public static final String FS_COS_SUPPORT = "fs.cos.support"; + + public enum Type { + HDFS, + S3, + OSS, + OBS, + COS, + UNKNOWN + } + + public abstract Map<String, String> getBackendConfigProperties(); + + @Getter + protected Type type; + + /** + * The purpose of this method is to create a list of StorageProperties based on the user specified properties. + * + * @param origProps + * @return + * @throws UserException + */ + public static List<StorageProperties> create(Map<String, String> origProps) { + List<StorageProperties> storageProperties = Lists.newArrayList(); + // 1. parse the storage properties by user specified fs.xxx.support properties + if (isFsSupport(origProps, FS_HDFS_SUPPORT)) { + storageProperties.add(new HDFSProperties(origProps)); + } else { + // always try to add hdfs properties, because in previous version, we don't have fs.xxx.support properties, + // the hdfs properties may be loaded from the configuration file. + // so there is no way to guess the storage type. + storageProperties.add(new HDFSProperties(origProps)); + } + + if (isFsSupport(origProps, FS_S3_SUPPORT) || S3Properties.guessIsMe(origProps)) { + storageProperties.add(new S3Properties(origProps)); + } + // FIXME: This logic directly checks for FS types (OSS, OBS, COS) here, which is intrusive. + // We should refactor this so that the plugins themselves provide a method to check if they are supported, + // thus decoupling the logic and making the system more extensible. + if (isFsSupport(origProps, FS_OSS_SUPPORT)) { + storageProperties.add(new OSSProperties(origProps)); + } + if (isFsSupport(origProps, FS_OBS_SUPPORT)) { + storageProperties.add(new OBSProperties(origProps)); + } + if (isFsSupport(origProps, FS_COS_SUPPORT)) { + storageProperties.add(new COSProperties(origProps)); + } + + if (isFsSupport(origProps, FS_GCS_SUPPORT)) { + throw new RuntimeException("Unsupported native GCS filesystem"); + } + if (isFsSupport(origProps, FS_AZURE_SUPPORT)) { + throw new RuntimeException("Unsupported native AZURE filesystem"); + } + + for (StorageProperties storageProperty : storageProperties) { + storageProperty.normalizedAndCheckProps(); + } + + return storageProperties; + } + + public static StorageProperties createStorageProperties(Map<String, String> origProps) { + StorageProperties storageProperties = null; + // 1. parse the storage properties by user specified fs.xxx.support properties + if (isFsSupport(origProps, FS_HDFS_SUPPORT) || HDFSProperties.guessIsMe(origProps)) { + storageProperties = new HDFSProperties(origProps); Review Comment: Looks like each `if` block will overwrite the other? ########## fe/fe-core/src/main/java/org/apache/doris/datasource/property/ConnectionProperties.java: ########## @@ -0,0 +1,115 @@ +// 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; + +import org.apache.doris.common.CatalogConfigFileUtils; + +import com.google.common.base.Strings; +import com.google.common.collect.Maps; +import lombok.Getter; +import lombok.Setter; +import org.apache.hadoop.conf.Configuration; + +import java.lang.reflect.Field; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +public abstract class ConnectionProperties { + @Getter + @Setter + protected Map<String, String> origProps; + + @Getter + protected Map<String, String> matchedProperties = new HashMap<>(); Review Comment: Add comment for this field ########## fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/HDFSProperties.java: ########## @@ -0,0 +1,195 @@ +// 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.storage; + +import org.apache.doris.common.UserException; +import org.apache.doris.datasource.property.ConnectorProperty; + +import com.google.common.base.Strings; +import org.apache.commons.collections.MapUtils; +import org.apache.commons.lang3.StringUtils; +import org.apache.hadoop.conf.Configuration; + +import java.util.HashMap; +import java.util.Map; + +public class HDFSProperties extends StorageProperties { + + @ConnectorProperty(names = {"hdfs.authentication.type", "hadoop.security.authentication"}, + required = false, + description = "The authentication type of HDFS. The default value is 'none'.") + private String hdfsAuthenticationType = "simple"; + + @ConnectorProperty(names = {"hdfs.authentication.kerberos.principal", "hadoop.kerberos.principal"}, + required = false, + description = "The principal of the kerberos authentication.") + private String hdfsKerberosPrincipal = ""; + + @ConnectorProperty(names = {"hdfs.authentication.kerberos.keytab", "hadoop.kerberos.keytab"}, + required = false, + description = "The keytab of the kerberos authentication.") + private String hdfsKerberosKeytab = ""; + + @ConnectorProperty(names = {"hadoop.username"}, + required = false, + description = "The username of Hadoop. Doris will user this user to access HDFS") + private String hadoopUsername = ""; + + @ConnectorProperty(names = {"hadoop.config.resources"}, + required = false, + description = "The xml files of Hadoop configuration.") + private String hadoopConfigResources = ""; + + @ConnectorProperty(names = {"hdfs.impersonation.enabled"}, + required = false, + supported = false, + description = "Whether to enable the impersonation of HDFS.") + private boolean hdfsImpersonationEnabled = false; + + @ConnectorProperty(names = {"fs.defaultFS"}, required = false, description = "") + private String fsDefaultFS = ""; + + /** + * The final HDFS configuration map that determines the effective settings. + * Priority rules: + * 1. If a key exists in `overrideConfig` (user-provided settings), its value takes precedence. + * 2. If a key is not present in `overrideConfig`, the value from `hdfs-site.xml` or `core-site.xml` is used. + * 3. This map should be used to read the resolved HDFS configuration, ensuring the correct precedence is applied. + */ + Map<String, String> finalHdfsConfig; + + public HDFSProperties(Map<String, String> origProps) { + super(Type.HDFS, origProps); + loadFinalHdfsConfig(origProps); + } + + public static boolean guessIsMe(Map<String, String> props) { + if (MapUtils.isEmpty(props)) { + return false; + } + if (props.containsKey("hadoop.config.resources") || props.containsKey("hadoop.security.authentication")) { + return true; + } + return false; + } + + private void loadFinalHdfsConfig(Map<String, String> origProps) { + if (MapUtils.isEmpty(origProps)) { + return; + } + finalHdfsConfig = new HashMap<>(); + origProps.forEach((key, value) -> { + if (key.startsWith("hadoop.") || key.startsWith("dfs.") || key.equals("fs.defaultFS")) { + finalHdfsConfig.put(key, value); Review Comment: Looks like only properties with `hadoop/dfs/fs` prefix are saved in `finalHdfsConfig`? ########## fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/AbstractObjectStorageProperties.java: ########## @@ -0,0 +1,178 @@ +// 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.storage; + +import org.apache.doris.common.UserException; +import org.apache.doris.datasource.property.ConnectorProperty; + +import lombok.Getter; +import lombok.Setter; +import org.apache.commons.lang3.StringUtils; + +import java.util.HashMap; +import java.util.Map; + +/** + * Abstract base class for object storage system properties. This class provides common configuration + * settings for object storage systems and supports conversion of these properties into configuration + * maps for different protocols, such as AWS S3. All object storage systems should extend this class + * to inherit the common configuration properties and methods. + * <p> + * The properties include connection settings (e.g., timeouts and maximum connections) and a flag to + * determine if path-style URLs should be used for the storage system. + */ +public abstract class AbstractObjectStorageProperties extends StorageProperties implements ObjectStorageProperties { + + /** + * The maximum number of concurrent connections that can be made to the object storage system. + * This value is optional and can be configured by the user. + */ + @Getter + @ConnectorProperty(names = {"connection.maximum"}, required = false, description = "Maximum number of connections.") Review Comment: Better name it with `s3.` prefix? Same for other object storage properties ########## fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/COSProperties.java: ########## @@ -0,0 +1,97 @@ +// 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.storage; + +import org.apache.doris.datasource.property.ConnectorProperty; + +import com.google.common.base.Strings; +import lombok.Setter; + +import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +public class COSProperties extends AbstractObjectStorageProperties { + + @Setter + @ConnectorProperty(names = {"cos.endpoint", "endpoint", "s3.endpoint"}, + required = false, + description = "The endpoint of COS.") + protected String cosEndpoint = ""; + + @ConnectorProperty(names = {"cos.region", "region", "s3.region"}, + required = false, + description = "The region of COS.") + protected String cosRegion = ""; + + @ConnectorProperty(names = {"cos.access_key"}, + description = "The access key of S3.") + protected String cosAccessKey = ""; + + @ConnectorProperty(names = {"cos.secret_key"}, + description = "The secret key of S3.") + protected String cosSecretKey = ""; + + + protected COSProperties(Map<String, String> origProps) { + super(Type.COS, origProps); + } + + protected static boolean guessIsMe(Map<String, String> origProps) { + return origProps.containsKey("cos.access_key"); Review Comment: I think better use endpoint to guess? Because endpoint is unique among cloud vendors ########## fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/COSProperties.java: ########## @@ -0,0 +1,97 @@ +// 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.storage; + +import org.apache.doris.datasource.property.ConnectorProperty; + +import com.google.common.base.Strings; +import lombok.Setter; + +import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +public class COSProperties extends AbstractObjectStorageProperties { + + @Setter + @ConnectorProperty(names = {"cos.endpoint", "endpoint", "s3.endpoint"}, + required = false, + description = "The endpoint of COS.") + protected String cosEndpoint = ""; + + @ConnectorProperty(names = {"cos.region", "region", "s3.region"}, + required = false, + description = "The region of COS.") + protected String cosRegion = ""; + + @ConnectorProperty(names = {"cos.access_key"}, + description = "The access key of S3.") Review Comment: ```suggestion description = "The access key of COS.") ``` Same for secret key ########## fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/S3PropertyUtils.java: ########## @@ -0,0 +1,90 @@ +// 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.storage; + +import org.apache.doris.common.UserException; +import org.apache.doris.common.util.S3URI; + +import org.apache.commons.lang3.StringUtils; + +import java.util.Map; + +public class S3PropertyUtils { + + + private static final String URI_KEY = "uri"; + + public static String constructEndpointFromUrl(Map<String, String> props, + String stringUsePathStyle, String stringForceParsingByStandardUri) { + String uri = props.get(URI_KEY); + if (uri == null || uri.isEmpty()) { + return null; + } + boolean usePathStyle = Boolean.parseBoolean(stringUsePathStyle); + boolean forceParsingByStandardUri = Boolean.parseBoolean(stringForceParsingByStandardUri); + try { + S3URI s3uri = S3URI.create(uri, usePathStyle, forceParsingByStandardUri); + return s3uri.getEndpoint().orElse(null); + } catch (UserException e) { + return null; + } + } + + public static String constructRegionFromUrl(Map<String, String> props, String stringUsePathStyle, Review Comment: Why not use `boolean` type for `UsePathStyle` and `ForceParsingByStandardUri`? And you did not test this method with `UsePathStyle` and `ForceParsingByStandardUri` set to true ########## fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/HDFSProperties.java: ########## @@ -0,0 +1,195 @@ +// 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.storage; + +import org.apache.doris.common.UserException; +import org.apache.doris.datasource.property.ConnectorProperty; + +import com.google.common.base.Strings; +import org.apache.commons.collections.MapUtils; +import org.apache.commons.lang3.StringUtils; +import org.apache.hadoop.conf.Configuration; + +import java.util.HashMap; +import java.util.Map; + +public class HDFSProperties extends StorageProperties { + + @ConnectorProperty(names = {"hdfs.authentication.type", "hadoop.security.authentication"}, + required = false, + description = "The authentication type of HDFS. The default value is 'none'.") + private String hdfsAuthenticationType = "simple"; + + @ConnectorProperty(names = {"hdfs.authentication.kerberos.principal", "hadoop.kerberos.principal"}, + required = false, + description = "The principal of the kerberos authentication.") + private String hdfsKerberosPrincipal = ""; + + @ConnectorProperty(names = {"hdfs.authentication.kerberos.keytab", "hadoop.kerberos.keytab"}, + required = false, + description = "The keytab of the kerberos authentication.") + private String hdfsKerberosKeytab = ""; + + @ConnectorProperty(names = {"hadoop.username"}, + required = false, + description = "The username of Hadoop. Doris will user this user to access HDFS") + private String hadoopUsername = ""; + + @ConnectorProperty(names = {"hadoop.config.resources"}, + required = false, + description = "The xml files of Hadoop configuration.") + private String hadoopConfigResources = ""; + + @ConnectorProperty(names = {"hdfs.impersonation.enabled"}, + required = false, + supported = false, + description = "Whether to enable the impersonation of HDFS.") + private boolean hdfsImpersonationEnabled = false; + + @ConnectorProperty(names = {"fs.defaultFS"}, required = false, description = "") + private String fsDefaultFS = ""; + + /** + * The final HDFS configuration map that determines the effective settings. + * Priority rules: + * 1. If a key exists in `overrideConfig` (user-provided settings), its value takes precedence. + * 2. If a key is not present in `overrideConfig`, the value from `hdfs-site.xml` or `core-site.xml` is used. + * 3. This map should be used to read the resolved HDFS configuration, ensuring the correct precedence is applied. + */ + Map<String, String> finalHdfsConfig; + + public HDFSProperties(Map<String, String> origProps) { + super(Type.HDFS, origProps); + loadFinalHdfsConfig(origProps); + } + + public static boolean guessIsMe(Map<String, String> props) { + if (MapUtils.isEmpty(props)) { + return false; + } + if (props.containsKey("hadoop.config.resources") || props.containsKey("hadoop.security.authentication")) { Review Comment: Better add `fs.defaultFS` and `dfs.nameservices` too ########## fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/OBSProperties.java: ########## @@ -0,0 +1,88 @@ +// 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.storage; + +import org.apache.doris.datasource.property.ConnectorProperty; + +import com.google.common.base.Strings; +import lombok.Setter; + +import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +public class OBSProperties extends AbstractObjectStorageProperties { + + @Setter + @ConnectorProperty(names = {"obs.endpoint", "endpoint", "s3.endpoint"}, required = false, + description = "The endpoint of OBS.") + protected String obsEndpoint = ""; + + @ConnectorProperty(names = {"obs.access_key"}, description = "The access key of OBS.") + protected String obsAccessKey = ""; + + @ConnectorProperty(names = {"obs.secret_key"}, description = "The secret key of OBS.") + protected String obsSecretKey = ""; + + + @ConnectorProperty(names = {"obs.region", "region", "s3.region"}, required = false, + description = "The region of OBS.") + protected String region; + + public OBSProperties(Map<String, String> origProps) { + super(Type.OBS, origProps); + // Initialize fields from origProps + } + + protected static boolean guessIsMe(Map<String, String> origProps) { + return origProps.containsKey("obs.access_key"); + } + + @Override + public void toNativeS3Configuration(Map<String, String> config) { + config.putAll(generateAWSS3Properties(obsEndpoint, getRegion(), obsAccessKey, obsSecretKey)); + } + + public String getRegion() { Review Comment: And I saw the different logic between this and `getRegion` in `OSSProperties`. In `OSSProperties`, this `region` is set, it will be returned directly without parsing again. ########## fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/StorageProperties.java: ########## @@ -0,0 +1,161 @@ +// 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.storage; + +import org.apache.doris.common.UserException; +import org.apache.doris.datasource.property.ConnectionProperties; +import org.apache.doris.datasource.property.ConnectorProperty; + +import com.google.common.collect.Lists; +import lombok.Getter; + +import java.lang.reflect.Field; +import java.util.List; +import java.util.Map; + +public abstract class StorageProperties extends ConnectionProperties { + + public static final String FS_HDFS_SUPPORT = "fs.hdfs.support"; + public static final String FS_S3_SUPPORT = "fs.s3.support"; + public static final String FS_GCS_SUPPORT = "fs.gcs.support"; + public static final String FS_AZURE_SUPPORT = "fs.azure.support"; + public static final String FS_OSS_SUPPORT = "fs.oss.support"; + public static final String FS_OBS_SUPPORT = "fs.obs.support"; + public static final String FS_COS_SUPPORT = "fs.cos.support"; + + public enum Type { + HDFS, + S3, + OSS, + OBS, + COS, + UNKNOWN + } + + public abstract Map<String, String> getBackendConfigProperties(); + + @Getter + protected Type type; + + /** + * The purpose of this method is to create a list of StorageProperties based on the user specified properties. + * + * @param origProps + * @return + * @throws UserException + */ + public static List<StorageProperties> create(Map<String, String> origProps) { + List<StorageProperties> storageProperties = Lists.newArrayList(); + // 1. parse the storage properties by user specified fs.xxx.support properties + if (isFsSupport(origProps, FS_HDFS_SUPPORT)) { + storageProperties.add(new HDFSProperties(origProps)); + } else { + // always try to add hdfs properties, because in previous version, we don't have fs.xxx.support properties, + // the hdfs properties may be loaded from the configuration file. + // so there is no way to guess the storage type. + storageProperties.add(new HDFSProperties(origProps)); + } + + if (isFsSupport(origProps, FS_S3_SUPPORT) || S3Properties.guessIsMe(origProps)) { + storageProperties.add(new S3Properties(origProps)); + } + // FIXME: This logic directly checks for FS types (OSS, OBS, COS) here, which is intrusive. + // We should refactor this so that the plugins themselves provide a method to check if they are supported, + // thus decoupling the logic and making the system more extensible. + if (isFsSupport(origProps, FS_OSS_SUPPORT)) { + storageProperties.add(new OSSProperties(origProps)); + } + if (isFsSupport(origProps, FS_OBS_SUPPORT)) { + storageProperties.add(new OBSProperties(origProps)); + } + if (isFsSupport(origProps, FS_COS_SUPPORT)) { + storageProperties.add(new COSProperties(origProps)); + } + + if (isFsSupport(origProps, FS_GCS_SUPPORT)) { + throw new RuntimeException("Unsupported native GCS filesystem"); + } + if (isFsSupport(origProps, FS_AZURE_SUPPORT)) { + throw new RuntimeException("Unsupported native AZURE filesystem"); + } + + for (StorageProperties storageProperty : storageProperties) { + storageProperty.normalizedAndCheckProps(); + } + + return storageProperties; + } + + public static StorageProperties createStorageProperties(Map<String, String> origProps) { Review Comment: What is different between `createStorageProperties()` and `createI()`? ########## fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/COSProperties.java: ########## @@ -0,0 +1,97 @@ +// 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.storage; + +import org.apache.doris.datasource.property.ConnectorProperty; + +import com.google.common.base.Strings; +import lombok.Setter; + +import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +public class COSProperties extends AbstractObjectStorageProperties { + + @Setter + @ConnectorProperty(names = {"cos.endpoint", "endpoint", "s3.endpoint"}, + required = false, + description = "The endpoint of COS.") + protected String cosEndpoint = ""; + + @ConnectorProperty(names = {"cos.region", "region", "s3.region"}, + required = false, + description = "The region of COS.") + protected String cosRegion = ""; + + @ConnectorProperty(names = {"cos.access_key"}, + description = "The access key of S3.") + protected String cosAccessKey = ""; + + @ConnectorProperty(names = {"cos.secret_key"}, + description = "The secret key of S3.") + protected String cosSecretKey = ""; + + + protected COSProperties(Map<String, String> origProps) { + super(Type.COS, origProps); + } + + protected static boolean guessIsMe(Map<String, String> origProps) { + return origProps.containsKey("cos.access_key"); + } + + + @Override + public void toNativeS3Configuration(Map<String, String> config) { + config.putAll(generateAWSS3Properties(cosEndpoint, getRegion(), cosAccessKey, cosSecretKey)); + } + + public String getRegion() { + if (Strings.isNullOrEmpty(this.cosRegion)) { + if (cosEndpoint.contains("myqcloud.com")) { Review Comment: Give an example in comment to explain this transform ########## fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/COSProperties.java: ########## @@ -0,0 +1,97 @@ +// 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.storage; + +import org.apache.doris.datasource.property.ConnectorProperty; + +import com.google.common.base.Strings; +import lombok.Setter; + +import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +public class COSProperties extends AbstractObjectStorageProperties { + + @Setter + @ConnectorProperty(names = {"cos.endpoint", "endpoint", "s3.endpoint"}, + required = false, + description = "The endpoint of COS.") + protected String cosEndpoint = ""; + + @ConnectorProperty(names = {"cos.region", "region", "s3.region"}, + required = false, + description = "The region of COS.") + protected String cosRegion = ""; + + @ConnectorProperty(names = {"cos.access_key"}, + description = "The access key of S3.") + protected String cosAccessKey = ""; + + @ConnectorProperty(names = {"cos.secret_key"}, + description = "The secret key of S3.") + protected String cosSecretKey = ""; + + + protected COSProperties(Map<String, String> origProps) { + super(Type.COS, origProps); + } + + protected static boolean guessIsMe(Map<String, String> origProps) { + return origProps.containsKey("cos.access_key"); + } + + + @Override + public void toNativeS3Configuration(Map<String, String> config) { + config.putAll(generateAWSS3Properties(cosEndpoint, getRegion(), cosAccessKey, cosSecretKey)); Review Comment: Why for cos, there are only 4 properties? what about those timeout properties? ########## fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/S3PropertyUtils.java: ########## @@ -0,0 +1,90 @@ +// 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.storage; + +import org.apache.doris.common.UserException; +import org.apache.doris.common.util.S3URI; + +import org.apache.commons.lang3.StringUtils; + +import java.util.Map; + +public class S3PropertyUtils { + + + private static final String URI_KEY = "uri"; + + public static String constructEndpointFromUrl(Map<String, String> props, + String stringUsePathStyle, String stringForceParsingByStandardUri) { + String uri = props.get(URI_KEY); + if (uri == null || uri.isEmpty()) { + return null; + } + boolean usePathStyle = Boolean.parseBoolean(stringUsePathStyle); + boolean forceParsingByStandardUri = Boolean.parseBoolean(stringForceParsingByStandardUri); + try { + S3URI s3uri = S3URI.create(uri, usePathStyle, forceParsingByStandardUri); + return s3uri.getEndpoint().orElse(null); + } catch (UserException e) { Review Comment: I think we should print this exception for debugging. Maybe we can just throw this exception ########## fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/OBSProperties.java: ########## @@ -0,0 +1,88 @@ +// 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.storage; + +import org.apache.doris.datasource.property.ConnectorProperty; + +import com.google.common.base.Strings; +import lombok.Setter; + +import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +public class OBSProperties extends AbstractObjectStorageProperties { + + @Setter + @ConnectorProperty(names = {"obs.endpoint", "endpoint", "s3.endpoint"}, required = false, + description = "The endpoint of OBS.") + protected String obsEndpoint = ""; + + @ConnectorProperty(names = {"obs.access_key"}, description = "The access key of OBS.") + protected String obsAccessKey = ""; + + @ConnectorProperty(names = {"obs.secret_key"}, description = "The secret key of OBS.") + protected String obsSecretKey = ""; + + + @ConnectorProperty(names = {"obs.region", "region", "s3.region"}, required = false, + description = "The region of OBS.") + protected String region; + + public OBSProperties(Map<String, String> origProps) { + super(Type.OBS, origProps); + // Initialize fields from origProps + } + + protected static boolean guessIsMe(Map<String, String> origProps) { + return origProps.containsKey("obs.access_key"); + } + + @Override + public void toNativeS3Configuration(Map<String, String> config) { + config.putAll(generateAWSS3Properties(obsEndpoint, getRegion(), obsAccessKey, obsSecretKey)); + } + + public String getRegion() { Review Comment: Take care of these `tiny` methods. These method may be called very frequently in high concurrency scenarios. But here are 2 issues: 1. You Pattern.compile every time, which is time-consuming. 2. Why not set the `region` when creating this `OBSProperties`, for only once? ########## fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/COSProperties.java: ########## @@ -0,0 +1,97 @@ +// 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.storage; + +import org.apache.doris.datasource.property.ConnectorProperty; + +import com.google.common.base.Strings; +import lombok.Setter; + +import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +public class COSProperties extends AbstractObjectStorageProperties { + + @Setter + @ConnectorProperty(names = {"cos.endpoint", "endpoint", "s3.endpoint"}, + required = false, + description = "The endpoint of COS.") + protected String cosEndpoint = ""; + + @ConnectorProperty(names = {"cos.region", "region", "s3.region"}, + required = false, + description = "The region of COS.") + protected String cosRegion = ""; + + @ConnectorProperty(names = {"cos.access_key"}, Review Comment: also need other alias like `s3.access_key`, same suggestion for `secret_key` ########## fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/OBSProperties.java: ########## @@ -0,0 +1,88 @@ +// 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.storage; + +import org.apache.doris.datasource.property.ConnectorProperty; + +import com.google.common.base.Strings; +import lombok.Setter; + +import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +public class OBSProperties extends AbstractObjectStorageProperties { + + @Setter + @ConnectorProperty(names = {"obs.endpoint", "endpoint", "s3.endpoint"}, required = false, + description = "The endpoint of OBS.") + protected String obsEndpoint = ""; + + @ConnectorProperty(names = {"obs.access_key"}, description = "The access key of OBS.") + protected String obsAccessKey = ""; + + @ConnectorProperty(names = {"obs.secret_key"}, description = "The secret key of OBS.") + protected String obsSecretKey = ""; + Review Comment: Take care of these empty lines ########## fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/HDFSProperties.java: ########## @@ -0,0 +1,195 @@ +// 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.storage; + +import org.apache.doris.common.UserException; +import org.apache.doris.datasource.property.ConnectorProperty; + +import com.google.common.base.Strings; +import org.apache.commons.collections.MapUtils; +import org.apache.commons.lang3.StringUtils; +import org.apache.hadoop.conf.Configuration; + +import java.util.HashMap; +import java.util.Map; + +public class HDFSProperties extends StorageProperties { + + @ConnectorProperty(names = {"hdfs.authentication.type", "hadoop.security.authentication"}, + required = false, + description = "The authentication type of HDFS. The default value is 'none'.") + private String hdfsAuthenticationType = "simple"; + + @ConnectorProperty(names = {"hdfs.authentication.kerberos.principal", "hadoop.kerberos.principal"}, + required = false, + description = "The principal of the kerberos authentication.") + private String hdfsKerberosPrincipal = ""; + + @ConnectorProperty(names = {"hdfs.authentication.kerberos.keytab", "hadoop.kerberos.keytab"}, + required = false, + description = "The keytab of the kerberos authentication.") + private String hdfsKerberosKeytab = ""; + + @ConnectorProperty(names = {"hadoop.username"}, + required = false, + description = "The username of Hadoop. Doris will user this user to access HDFS") + private String hadoopUsername = ""; + + @ConnectorProperty(names = {"hadoop.config.resources"}, + required = false, + description = "The xml files of Hadoop configuration.") + private String hadoopConfigResources = ""; + + @ConnectorProperty(names = {"hdfs.impersonation.enabled"}, + required = false, + supported = false, + description = "Whether to enable the impersonation of HDFS.") + private boolean hdfsImpersonationEnabled = false; + + @ConnectorProperty(names = {"fs.defaultFS"}, required = false, description = "") + private String fsDefaultFS = ""; + + /** + * The final HDFS configuration map that determines the effective settings. + * Priority rules: + * 1. If a key exists in `overrideConfig` (user-provided settings), its value takes precedence. + * 2. If a key is not present in `overrideConfig`, the value from `hdfs-site.xml` or `core-site.xml` is used. + * 3. This map should be used to read the resolved HDFS configuration, ensuring the correct precedence is applied. + */ + Map<String, String> finalHdfsConfig; + + public HDFSProperties(Map<String, String> origProps) { + super(Type.HDFS, origProps); + loadFinalHdfsConfig(origProps); + } + + public static boolean guessIsMe(Map<String, String> props) { + if (MapUtils.isEmpty(props)) { + return false; + } + if (props.containsKey("hadoop.config.resources") || props.containsKey("hadoop.security.authentication")) { + return true; + } + return false; + } + + private void loadFinalHdfsConfig(Map<String, String> origProps) { + if (MapUtils.isEmpty(origProps)) { + return; + } + finalHdfsConfig = new HashMap<>(); + origProps.forEach((key, value) -> { + if (key.startsWith("hadoop.") || key.startsWith("dfs.") || key.equals("fs.defaultFS")) { + finalHdfsConfig.put(key, value); + } + }); + + } + + @Override + protected String getResourceConfigPropName() { + return "hadoop.config.resources"; + } + + protected void checkRequiredProperties() { + super.checkRequiredProperties(); + checkConfigFileIsValid(hadoopConfigResources); Review Comment: before calling `checkRequiredProperties()`, we already call `loadConfigFromFile()`. Which means for creation, we load config from file twice. ########## fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/S3PropertyUtils.java: ########## @@ -0,0 +1,90 @@ +// 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.storage; + +import org.apache.doris.common.UserException; +import org.apache.doris.common.util.S3URI; + +import org.apache.commons.lang3.StringUtils; + +import java.util.Map; + +public class S3PropertyUtils { + + + private static final String URI_KEY = "uri"; + + public static String constructEndpointFromUrl(Map<String, String> props, Review Comment: Add comment and add example in comment. Same for other methods ########## fe/fe-core/src/test/java/org/apache/doris/datasource/property/storage/S3PropertyUtilsTest.java: ########## @@ -0,0 +1,108 @@ +// 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.storage; + +import org.apache.doris.common.UserException; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.function.Executable; + +import java.util.HashMap; +import java.util.Map; + +class S3PropertyUtilsTest { + + @Test + void testCheckLoadPropsAndReturnUri_success() throws UserException { + Map<String, String> props = new HashMap<>(); + props.put("uri", "https://bucket.s3.us-west-2.amazonaws.com/key";); + String result = S3PropertyUtils.checkLoadPropsAndReturnUri(props); + Assertions.assertEquals("https://bucket.s3.us-west-2.amazonaws.com/key";, result); + } + + @Test + void testCheckLoadPropsAndReturnUri_missingKey() { + Map<String, String> props = new HashMap<>(); + Executable executable = () -> S3PropertyUtils.checkLoadPropsAndReturnUri(props); + UserException exception = Assertions.assertThrows(UserException.class, executable); + Assertions.assertEquals("errCode = 2, detailMessage = props is empty", exception.getMessage()); + + props.put("someKey", "value"); + executable = () -> S3PropertyUtils.checkLoadPropsAndReturnUri(props); + exception = Assertions.assertThrows(UserException.class, executable); + Assertions.assertEquals("errCode = 2, detailMessage = props must contain uri", exception.getMessage()); + } + + @Test + void testConstructEndpointFromUrl_success() { + Map<String, String> props = new HashMap<>(); + props.put("uri", "https://my-bucket.s3.us-east-1.amazonaws.com/some/file.txt";); + String endpoint = S3PropertyUtils.constructEndpointFromUrl(props, "false", "false"); + Assertions.assertEquals("s3.us-east-1.amazonaws.com", endpoint); + } + + @Test + void testConstructEndpointFromUrl_nullOrBlank() { + Map<String, String> props = new HashMap<>(); + Assertions.assertNull(S3PropertyUtils.constructEndpointFromUrl(props, "true", "false")); + + props.put("uri", ""); + Assertions.assertNull(S3PropertyUtils.constructEndpointFromUrl(props, "false", "true")); + + props.put("uri", "invalid uri without scheme"); + Assertions.assertNull(S3PropertyUtils.constructEndpointFromUrl(props, "true", "true")); + } + + @Test + void testConstructRegionFromUrl_success() { + Map<String, String> props = new HashMap<>(); + props.put("uri", "https://my-bucket.s3.us-west-1.amazonaws.com/test.txt";); + String region = S3PropertyUtils.constructRegionFromUrl(props, "false", "false"); + Assertions.assertEquals("us-west-1", region); + } + + @Test + void testConstructRegionFromUrl_nullOrInvalid() { + Map<String, String> props = new HashMap<>(); + Assertions.assertNull(S3PropertyUtils.constructRegionFromUrl(props, "false", "false")); + + props.put("uri", ""); + Assertions.assertNull(S3PropertyUtils.constructRegionFromUrl(props, "false", "true")); + + props.put("uri", "not a uri"); + Assertions.assertNull(S3PropertyUtils.constructRegionFromUrl(props, "false", "true")); + } + + @Test + void testConvertToS3Address_success() throws UserException { Review Comment: U are not testing case like from `oss://xx` to `s3://` ########## fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/S3PropertyUtils.java: ########## @@ -0,0 +1,90 @@ +// 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.storage; + +import org.apache.doris.common.UserException; +import org.apache.doris.common.util.S3URI; + +import org.apache.commons.lang3.StringUtils; + +import java.util.Map; + +public class S3PropertyUtils { + + + private static final String URI_KEY = "uri"; + + public static String constructEndpointFromUrl(Map<String, String> props, + String stringUsePathStyle, String stringForceParsingByStandardUri) { + String uri = props.get(URI_KEY); + if (uri == null || uri.isEmpty()) { + return null; + } + boolean usePathStyle = Boolean.parseBoolean(stringUsePathStyle); + boolean forceParsingByStandardUri = Boolean.parseBoolean(stringForceParsingByStandardUri); + try { + S3URI s3uri = S3URI.create(uri, usePathStyle, forceParsingByStandardUri); + return s3uri.getEndpoint().orElse(null); + } catch (UserException e) { + return null; + } + } + + public static String constructRegionFromUrl(Map<String, String> props, String stringUsePathStyle, + String stringForceParsingByStandardUri) { + String uri = props.get(URI_KEY); + if (uri == null || uri.isEmpty()) { + return null; + } + boolean usePathStyle = Boolean.parseBoolean(stringUsePathStyle); + boolean forceParsingByStandardUri = Boolean.parseBoolean(stringForceParsingByStandardUri); + + S3URI s3uri = null; + try { + s3uri = S3URI.create(uri, usePathStyle, forceParsingByStandardUri); + return s3uri.getRegion().orElse(null); + } catch (UserException e) { + return null; + } + } + + public static String convertToS3Address(String path, String stringUsePathStyle, + String stringForceParsingByStandardUri) throws UserException { + if (StringUtils.isBlank(path)) { + throw new UserException("path is null"); + } + if (path.startsWith("s3://")) { + return path; + } + + boolean usePathStyle = Boolean.parseBoolean(stringUsePathStyle); + boolean forceParsingByStandardUri = Boolean.parseBoolean(stringForceParsingByStandardUri); + S3URI s3uri = S3URI.create(path, usePathStyle, forceParsingByStandardUri); + return "s3" + S3URI.SCHEME_DELIM + s3uri.getBucket() + S3URI.PATH_DELIM + s3uri.getKey(); + } + + public static String checkLoadPropsAndReturnUri(Map<String, String> props) throws UserException { Review Comment: Looks like this method is only for check and return uri property? Why not name it as `checkAndReturnUri`? And actually, it does not CHECK the uri? ########## fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/S3PropertyUtils.java: ########## @@ -0,0 +1,90 @@ +// 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.storage; + +import org.apache.doris.common.UserException; +import org.apache.doris.common.util.S3URI; + +import org.apache.commons.lang3.StringUtils; + +import java.util.Map; + +public class S3PropertyUtils { + + + private static final String URI_KEY = "uri"; + + public static String constructEndpointFromUrl(Map<String, String> props, + String stringUsePathStyle, String stringForceParsingByStandardUri) { + String uri = props.get(URI_KEY); + if (uri == null || uri.isEmpty()) { + return null; + } + boolean usePathStyle = Boolean.parseBoolean(stringUsePathStyle); + boolean forceParsingByStandardUri = Boolean.parseBoolean(stringForceParsingByStandardUri); + try { + S3URI s3uri = S3URI.create(uri, usePathStyle, forceParsingByStandardUri); + return s3uri.getEndpoint().orElse(null); + } catch (UserException e) { + return null; + } + } + + public static String constructRegionFromUrl(Map<String, String> props, String stringUsePathStyle, + String stringForceParsingByStandardUri) { + String uri = props.get(URI_KEY); + if (uri == null || uri.isEmpty()) { + return null; + } + boolean usePathStyle = Boolean.parseBoolean(stringUsePathStyle); + boolean forceParsingByStandardUri = Boolean.parseBoolean(stringForceParsingByStandardUri); + + S3URI s3uri = null; + try { + s3uri = S3URI.create(uri, usePathStyle, forceParsingByStandardUri); + return s3uri.getRegion().orElse(null); + } catch (UserException e) { + return null; + } + } + + public static String convertToS3Address(String path, String stringUsePathStyle, Review Comment: ```suggestion public static String convertToS3Scheme(String path, String stringUsePathStyle, ``` ########## fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/OSSProperties.java: ########## @@ -0,0 +1,103 @@ +// 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.storage; + +import org.apache.doris.datasource.property.ConnectorProperty; + +import com.google.common.base.Strings; +import lombok.Setter; + +import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +public class OSSProperties extends AbstractObjectStorageProperties { + + @Setter + @ConnectorProperty(names = {"oss.endpoint", "endpoint", "s3.endpoint"}, required = false, + description = "The endpoint of OSS.") + protected String endpoint = ""; + + @ConnectorProperty(names = {"oss.access_key"}, description = "The access key of OSS.") + protected String accessKey = ""; + + @ConnectorProperty(names = {"oss.secret_key"}, description = "The secret key of OSS.") + protected String secretKey = ""; + + @ConnectorProperty(names = {"oss.region", "region", "s3.region"}, required = false, + description = "The region of OSS.") + protected String region; + + + protected OSSProperties(Map<String, String> origProps) { + super(Type.OSS, origProps); + } + + protected static boolean guessIsMe(Map<String, String> origProps) { + return origProps.containsKey("oss.access_key"); + } + + @Override + public void toNativeS3Configuration(Map<String, String> config) { + config.putAll(generateAWSS3Properties(endpoint, getRegion(), accessKey, secretKey)); + } + + public String getRegion() { + // Return the region if it is already set + if (!Strings.isNullOrEmpty(this.region)) { Review Comment: How to make thread-safe? ########## fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/HdfsPropertiesUtils.java: ########## @@ -0,0 +1,100 @@ +// 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.storage; + +import org.apache.doris.common.AnalysisException; +import org.apache.doris.common.UserException; +import org.apache.doris.common.util.URI; + +import org.apache.commons.lang3.StringUtils; + +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +public class HdfsPropertiesUtils { + private static final String URI_KEY = "uri"; + + private static Set<String> supportSchema = new HashSet<>(); + + static { + supportSchema.add("hdfs"); + supportSchema.add("viewfs"); + } + + public static String checkLoadPropsAndReturnUri(Map<String, String> props) throws UserException { Review Comment: Add example in comment for all these methods ########## fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/HDFSProperties.java: ########## @@ -0,0 +1,195 @@ +// 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.storage; + +import org.apache.doris.common.UserException; +import org.apache.doris.datasource.property.ConnectorProperty; + +import com.google.common.base.Strings; +import org.apache.commons.collections.MapUtils; +import org.apache.commons.lang3.StringUtils; +import org.apache.hadoop.conf.Configuration; + +import java.util.HashMap; +import java.util.Map; + +public class HDFSProperties extends StorageProperties { + + @ConnectorProperty(names = {"hdfs.authentication.type", "hadoop.security.authentication"}, + required = false, + description = "The authentication type of HDFS. The default value is 'none'.") + private String hdfsAuthenticationType = "simple"; + + @ConnectorProperty(names = {"hdfs.authentication.kerberos.principal", "hadoop.kerberos.principal"}, + required = false, + description = "The principal of the kerberos authentication.") + private String hdfsKerberosPrincipal = ""; + + @ConnectorProperty(names = {"hdfs.authentication.kerberos.keytab", "hadoop.kerberos.keytab"}, + required = false, + description = "The keytab of the kerberos authentication.") + private String hdfsKerberosKeytab = ""; + + @ConnectorProperty(names = {"hadoop.username"}, + required = false, + description = "The username of Hadoop. Doris will user this user to access HDFS") + private String hadoopUsername = ""; + + @ConnectorProperty(names = {"hadoop.config.resources"}, + required = false, + description = "The xml files of Hadoop configuration.") + private String hadoopConfigResources = ""; + + @ConnectorProperty(names = {"hdfs.impersonation.enabled"}, + required = false, + supported = false, + description = "Whether to enable the impersonation of HDFS.") + private boolean hdfsImpersonationEnabled = false; + + @ConnectorProperty(names = {"fs.defaultFS"}, required = false, description = "") + private String fsDefaultFS = ""; + + /** + * The final HDFS configuration map that determines the effective settings. + * Priority rules: + * 1. If a key exists in `overrideConfig` (user-provided settings), its value takes precedence. + * 2. If a key is not present in `overrideConfig`, the value from `hdfs-site.xml` or `core-site.xml` is used. + * 3. This map should be used to read the resolved HDFS configuration, ensuring the correct precedence is applied. + */ + Map<String, String> finalHdfsConfig; Review Comment: ```suggestion private Map<String, String> finalHdfsConfig; ``` ########## fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/HDFSProperties.java: ########## @@ -0,0 +1,195 @@ +// 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.storage; + +import org.apache.doris.common.UserException; +import org.apache.doris.datasource.property.ConnectorProperty; + +import com.google.common.base.Strings; +import org.apache.commons.collections.MapUtils; +import org.apache.commons.lang3.StringUtils; +import org.apache.hadoop.conf.Configuration; + +import java.util.HashMap; +import java.util.Map; + +public class HDFSProperties extends StorageProperties { + + @ConnectorProperty(names = {"hdfs.authentication.type", "hadoop.security.authentication"}, + required = false, + description = "The authentication type of HDFS. The default value is 'none'.") + private String hdfsAuthenticationType = "simple"; + + @ConnectorProperty(names = {"hdfs.authentication.kerberos.principal", "hadoop.kerberos.principal"}, + required = false, + description = "The principal of the kerberos authentication.") + private String hdfsKerberosPrincipal = ""; + + @ConnectorProperty(names = {"hdfs.authentication.kerberos.keytab", "hadoop.kerberos.keytab"}, + required = false, + description = "The keytab of the kerberos authentication.") + private String hdfsKerberosKeytab = ""; + + @ConnectorProperty(names = {"hadoop.username"}, + required = false, + description = "The username of Hadoop. Doris will user this user to access HDFS") + private String hadoopUsername = ""; + + @ConnectorProperty(names = {"hadoop.config.resources"}, + required = false, + description = "The xml files of Hadoop configuration.") + private String hadoopConfigResources = ""; + + @ConnectorProperty(names = {"hdfs.impersonation.enabled"}, + required = false, + supported = false, + description = "Whether to enable the impersonation of HDFS.") + private boolean hdfsImpersonationEnabled = false; + + @ConnectorProperty(names = {"fs.defaultFS"}, required = false, description = "") + private String fsDefaultFS = ""; + + /** + * The final HDFS configuration map that determines the effective settings. + * Priority rules: + * 1. If a key exists in `overrideConfig` (user-provided settings), its value takes precedence. + * 2. If a key is not present in `overrideConfig`, the value from `hdfs-site.xml` or `core-site.xml` is used. + * 3. This map should be used to read the resolved HDFS configuration, ensuring the correct precedence is applied. + */ + Map<String, String> finalHdfsConfig; + + public HDFSProperties(Map<String, String> origProps) { + super(Type.HDFS, origProps); + loadFinalHdfsConfig(origProps); + } + + public static boolean guessIsMe(Map<String, String> props) { + if (MapUtils.isEmpty(props)) { + return false; + } + if (props.containsKey("hadoop.config.resources") || props.containsKey("hadoop.security.authentication")) { + return true; + } + return false; + } + + private void loadFinalHdfsConfig(Map<String, String> origProps) { + if (MapUtils.isEmpty(origProps)) { + return; + } + finalHdfsConfig = new HashMap<>(); + origProps.forEach((key, value) -> { + if (key.startsWith("hadoop.") || key.startsWith("dfs.") || key.equals("fs.defaultFS")) { + finalHdfsConfig.put(key, value); + } + }); + + } + + @Override + protected String getResourceConfigPropName() { + return "hadoop.config.resources"; + } + + protected void checkRequiredProperties() { + super.checkRequiredProperties(); + checkConfigFileIsValid(hadoopConfigResources); + if ("kerberos".equalsIgnoreCase(hdfsAuthenticationType)) { + if (Strings.isNullOrEmpty(hdfsKerberosPrincipal) || Strings.isNullOrEmpty(hdfsKerberosKeytab)) { + throw new IllegalArgumentException("HDFS authentication type is kerberos, " + + "but principal or keytab is not set."); + } + } + if (StringUtils.isBlank(fsDefaultFS)) { + this.fsDefaultFS = HdfsPropertiesUtils.constructDefaultFsFromUri(origProps); + } + } + + private void checkConfigFileIsValid(String configFile) { + if (Strings.isNullOrEmpty(configFile)) { + return; + } + loadConfigFromFile(getResourceConfigPropName()); + } + + public void toHadoopConfiguration(Configuration conf) { Review Comment: 1. what is diff between `toHadoopConfiguration` and `getHadoopConfiguration`? 2. think twice about these methods. because we'd better load config only once instead of loading them every time ########## fe/fe-core/src/main/java/org/apache/doris/datasource/property/storage/COSProperties.java: ########## @@ -0,0 +1,97 @@ +// 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.storage; + +import org.apache.doris.datasource.property.ConnectorProperty; + +import com.google.common.base.Strings; +import lombok.Setter; + +import java.util.Map; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +public class COSProperties extends AbstractObjectStorageProperties { + + @Setter + @ConnectorProperty(names = {"cos.endpoint", "endpoint", "s3.endpoint"}, + required = false, + description = "The endpoint of COS.") + protected String cosEndpoint = ""; + + @ConnectorProperty(names = {"cos.region", "region", "s3.region"}, + required = false, + description = "The region of COS.") + protected String cosRegion = ""; + + @ConnectorProperty(names = {"cos.access_key"}, + description = "The access key of S3.") + protected String cosAccessKey = ""; + + @ConnectorProperty(names = {"cos.secret_key"}, + description = "The secret key of S3.") + protected String cosSecretKey = ""; + + + protected COSProperties(Map<String, String> origProps) { + super(Type.COS, origProps); + } + + protected static boolean guessIsMe(Map<String, String> origProps) { + return origProps.containsKey("cos.access_key"); + } + + + @Override + public void toNativeS3Configuration(Map<String, String> config) { + config.putAll(generateAWSS3Properties(cosEndpoint, getRegion(), cosAccessKey, cosSecretKey)); Review Comment: And the name is confusing. There are 3 names which point to same thing: 1. backend property 2. native s3 property 3. aws s3 property -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org
