[improvement](s3) Default S3 access to HTTPS

conf/be.conf

@@ -37,6 +37,9 @@ heartbeat_service_port = 9050
 brpc_port = 8060
 arrow_flight_sql_port = 8050
 
+# Scheme used by S3 clients when the endpoint has no explicit scheme.
+s3_client_http_scheme = https
+
 # HTTPS configures
 enable_https = false
 # path of certificate in PEM format.

conf/fe.conf

@@ -49,6 +49,9 @@ query_port = 9030
 edit_log_port = 9010
 arrow_flight_sql_port = 8070
 
+# Scheme added to S3 endpoints without an explicit scheme.
+s3_client_http_scheme = https
+
 # Choose one if there are more than one ip except loopback address. 
 # Note that there should at most one ip match this list.
 # If no ip match this rule, will choose one randomly.

fe/fe-common/src/main/java/org/apache/doris/common/Config.java

@@ -3242,6 +3242,11 @@ public static int metaServiceRpcRetryTimes() {
                     + "For example: s3_load_endpoint_white_list=a,b,c"})
     public static String[] s3_load_endpoint_white_list = {};
 
+    @ConfField(description = {
+            "The scheme added to S3 endpoints without an explicit scheme. Valid values are http and https."},
+            options = {"http", "https"})
+    public static String s3_client_http_scheme = "http";
+
     @ConfField(mutable = true, description = {
             "For deterministic S3 paths (without wildcards like *, ?), use HEAD requests instead of "
                     + "ListObjects to avoid requiring ListBucket permission. Brace patterns {1,2,3} and "

fe/fe-core/src/main/java/org/apache/doris/catalog/S3Resource.java

@@ -18,9 +18,9 @@
 package org.apache.doris.catalog;
 
 import org.apache.doris.common.DdlException;
-import org.apache.doris.common.credentials.CloudCredentialWithEndpoint;
 import org.apache.doris.common.proc.BaseProcResult;
 import org.apache.doris.common.util.DatasourcePrintableMap;
+import org.apache.doris.common.util.S3Util;
 import org.apache.doris.datasource.property.storage.S3Properties;
 import org.apache.doris.filesystem.UploadPartResult;
 import org.apache.doris.filesystem.spi.ObjFileSystem;
@@ -103,12 +103,9 @@ protected void setProperties(ImmutableMap<String, String> newProperties) throws
         }
 
         // the endpoint for ping need add uri scheme.
-        String pingEndpoint = properties.get(S3Properties.ENDPOINT);
-        if (!pingEndpoint.startsWith("http://") && !pingEndpoint.startsWith("https://")) {
-            pingEndpoint = "http://" + properties.get(S3Properties.ENDPOINT);
-            properties.put(S3Properties.ENDPOINT, pingEndpoint);
-            properties.put(S3Properties.Env.ENDPOINT, pingEndpoint);
-        }
+        String pingEndpoint = S3Util.buildEndpointUrl(properties.get(S3Properties.ENDPOINT));
+        properties.put(S3Properties.ENDPOINT, pingEndpoint);
+        properties.put(S3Properties.Env.ENDPOINT, pingEndpoint);
         String region = S3Properties.getRegionOfEndpoint(pingEndpoint);
         properties.putIfAbsent(S3Properties.REGION, region);
 
@@ -232,6 +229,11 @@ public void modifyProperties(Map<String, String> properties) throws DdlException
         }
         // compatible with old version, Need convert if modified properties map uses old properties.
         S3Properties.convertToStdProperties(properties);
+        if (!Strings.isNullOrEmpty(properties.get(S3Properties.ENDPOINT))) {
+            String endpoint = S3Util.buildEndpointUrl(properties.get(S3Properties.ENDPOINT));
+            properties.put(S3Properties.ENDPOINT, endpoint);
+            properties.put(S3Properties.Env.ENDPOINT, endpoint);
+        }
         boolean needCheck = isNeedCheck(properties);
         if (LOG.isDebugEnabled()) {
             LOG.debug("s3 info need check validity : {}", needCheck);
@@ -278,18 +280,6 @@ public void modifyProperties(Map<String, String> properties) throws DdlException
         super.modifyProperties(properties);
     }
 
-    private CloudCredentialWithEndpoint getS3PingCredentials(Map<String, String> properties) {
-        String ak = properties.getOrDefault(S3Properties.ACCESS_KEY, this.properties.get(S3Properties.ACCESS_KEY));
-        String sk = properties.getOrDefault(S3Properties.SECRET_KEY, this.properties.get(S3Properties.SECRET_KEY));
-        String token = properties.getOrDefault(S3Properties.SESSION_TOKEN,
-                this.properties.get(S3Properties.SESSION_TOKEN));
-        String endpoint = properties.getOrDefault(S3Properties.ENDPOINT, this.properties.get(S3Properties.ENDPOINT));
-        String pingEndpoint = "http://" + endpoint;
-        String region = S3Properties.getRegionOfEndpoint(pingEndpoint);
-        properties.putIfAbsent(S3Properties.REGION, region);
-        return new CloudCredentialWithEndpoint(pingEndpoint, region, ak, sk, token);
-    }
-
     private boolean isNeedCheck(Map<String, String> newProperties) {
         boolean needCheck = !this.properties.containsKey(S3Properties.VALIDITY_CHECK)
                 || Boolean.parseBoolean(this.properties.get(S3Properties.VALIDITY_CHECK));
@@ -328,4 +318,3 @@ protected void getProcNodeData(BaseProcResult result) {
         readUnlock();
     }
 }
-

fe/fe-core/src/main/java/org/apache/doris/common/util/S3Util.java

@@ -394,11 +394,7 @@ public static String extendGlobNumberRange(String pathPattern) {
     public static void validateAndTestEndpoint(String endpoint) throws UserException {
         HttpURLConnection connection = null;
         try {
-            String urlStr = endpoint;
-            // Add default protocol if not specified
-            if (!endpoint.startsWith("http://") && !endpoint.startsWith("https://")) {
-                urlStr = "http://" + endpoint;
-            }
+            String urlStr = buildEndpointUrl(endpoint);
             endpoint = endpoint.replaceFirst("^http://", "");
             endpoint = endpoint.replaceFirst("^https://", "");
             List<String> whiteList = new ArrayList<>(Arrays.asList(Config.s3_load_endpoint_white_list));
@@ -434,6 +430,13 @@ public static void validateAndTestEndpoint(String endpoint) throws UserException
         }
     }
 
+    public static String buildEndpointUrl(String endpoint) {
+        if (endpoint.contains("://")) {
+            return endpoint;
+        }
+        return Config.s3_client_http_scheme + "://" + endpoint;
+    }
+
     /**
      * Check if a path pattern is deterministic, meaning all file paths can be determined
      * without listing. A pattern is deterministic if it contains no true wildcard characters

fe/fe-core/src/main/java/org/apache/doris/datasource/connectivity/AbstractS3CompatibleConnectivityTester.java

@@ -62,7 +62,7 @@ public void testFeConnection() throws Exception {
         String endpoint = properties.getEndpoint();
 
         try (S3Client client = S3Util.buildS3Client(
-                URI.create(endpoint),
+                URI.create(S3Util.buildEndpointUrl(endpoint)),
                 properties.getRegion(),
                 Boolean.parseBoolean(properties.getUsePathStyle()),
                 properties.getAwsCredentialsProvider())) {

fe/fe-core/src/main/java/org/apache/doris/datasource/iceberg/dlf/DLFCatalog.java

@@ -61,10 +61,7 @@ protected FileIO initializeFileIO(Map<String, String> properties, Configuration
         boolean isUsePathStyle = Boolean.parseBoolean(ossProperties.getUsePathStyle());
         // s3 file io just supports s3-like endpoint
         String s3Endpoint = endpoint.replace("oss-" + region, "s3.oss-" + region);
-        if (!s3Endpoint.contains("://")) {
-            s3Endpoint = "http://" + s3Endpoint;
-        }
-        URI endpointUri = URI.create(s3Endpoint);
+        URI endpointUri = URI.create(S3Util.buildEndpointUrl(s3Endpoint));
         FileIO io = new S3FileIO(() -> S3Util.buildS3Client(endpointUri, region, credential, isUsePathStyle));
         io.initialize(properties);
         return io;

fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/CreateStageCommand.java

@@ -38,6 +38,7 @@
 import org.apache.doris.common.FeNameFormat;
 import org.apache.doris.common.InternalErrorCode;
 import org.apache.doris.common.UserException;
+import org.apache.doris.common.util.S3Util;
 import org.apache.doris.datasource.property.storage.StorageProperties;
 import org.apache.doris.filesystem.spi.ObjFileSystem;
 import org.apache.doris.fs.FileSystemFactory;
@@ -145,7 +146,7 @@ private void checkObjectStorageInfo() throws UserException {
     private void tryConnect(String endpoint) throws Exception {
         HttpURLConnection connection = null;
         try {
-            String urlStr = "http://" + endpoint;
+            String urlStr = S3Util.buildEndpointUrl(endpoint);
             // TODO: Server-Side Request Forgery Check is still need?
             URL url = new URL(urlStr);
             SecurityChecker.getInstance().startSSRFChecking(urlStr);

fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/info/StageProperties.java

@@ -21,6 +21,7 @@
 import org.apache.doris.cloud.proto.Cloud.ObjectStoreInfoPB.Provider;
 import org.apache.doris.cloud.proto.Cloud.StagePB.StageAccessType;
 import org.apache.doris.common.util.FileFormatConstants;
+import org.apache.doris.common.util.S3Util;
 import org.apache.doris.nereids.exceptions.AnalysisException;
 
 import com.google.common.collect.ImmutableSet;
@@ -155,7 +156,8 @@ public boolean isDryRun() {
      * getObjectStoreInfoPB
      */
     public ObjectStoreInfoPB getObjectStoreInfoPB() {
-        ObjectStoreInfoPB.Builder builder = ObjectStoreInfoPB.newBuilder().setEndpoint(properties.get(ENDPOINT))
+        ObjectStoreInfoPB.Builder builder = ObjectStoreInfoPB.newBuilder()
+                .setEndpoint(S3Util.buildEndpointUrl(properties.get(ENDPOINT)))
                 .setRegion(properties.get(REGION)).setBucket(properties.get(BUCKET)).setPrefix(properties.get(PREFIX))
                 .setProvider(Provider.valueOf(properties.get(PROVIDER).toUpperCase()));
         if (getAccessType() == StageAccessType.AKSK) {

fe/fe-core/src/test/java/org/apache/doris/catalog/S3ResourceTest.java

@@ -194,7 +194,9 @@ public void testSerialization() throws Exception {
         Assert.assertEquals("s3_1", rS3Resource1.getName());
         Assert.assertEquals("s3_2", rS3Resource2.getName());
 
-        Assert.assertEquals(rS3Resource2.getProperty(S3Properties.ENDPOINT), "http://aaa");
+        Assert.assertEquals("http://aaa", rS3Resource2.getProperty(S3Properties.ENDPOINT));
+        Assert.assertEquals("http://aaa",
+                S3Properties.getObjStoreInfoPB(rS3Resource2.getCopiedProperties()).getEndpoint());
         Assert.assertEquals(rS3Resource2.getProperty(S3Properties.REGION), "bbb");
         Assert.assertEquals(rS3Resource2.getProperty(S3Properties.ROOT_PATH), "/path/to/root");
         Assert.assertEquals(rS3Resource2.getProperty(S3Properties.ACCESS_KEY), "xxx");
@@ -226,11 +228,36 @@ public void testModifyProperties() throws Exception {
         Map<String, String> modify = new HashMap<>();
         modify.put("s3.access_key", "aaa");
         s3Resource.modifyProperties(modify);
+
+        modify.clear();
+        modify.put(S3Properties.ENDPOINT, "new-endpoint");
+        s3Resource.modifyProperties(modify);
+        Assert.assertEquals("http://new-endpoint", s3Resource.getProperty(S3Properties.ENDPOINT));
+        Assert.assertEquals("http://new-endpoint", s3Resource.getProperty(S3Properties.Env.ENDPOINT));
+        Assert.assertEquals("http://new-endpoint",
+                S3Properties.getObjStoreInfoPB(s3Resource.getCopiedProperties()).getEndpoint());
+
+        modify.clear();
+        modify.put(S3Properties.Env.ENDPOINT, "http://other-endpoint");
+        s3Resource.modifyProperties(modify);
+        Assert.assertEquals("http://other-endpoint", s3Resource.getProperty(S3Properties.ENDPOINT));
+        Assert.assertEquals("http://other-endpoint", s3Resource.getProperty(S3Properties.Env.ENDPOINT));
+
+        modify.clear();
+        modify.put(S3Properties.ENDPOINT, "");
+        s3Resource.modifyProperties(modify);
+        Assert.assertEquals("http://other-endpoint", s3Resource.getProperty(S3Properties.ENDPOINT));
+        Assert.assertEquals("http://other-endpoint", s3Resource.getProperty(S3Properties.Env.ENDPOINT));
+
+        modify.clear();
+        modify.put(S3Properties.Env.ENDPOINT, "");
+        s3Resource.modifyProperties(modify);
+        Assert.assertEquals("http://other-endpoint", s3Resource.getProperty(S3Properties.ENDPOINT));
+        Assert.assertEquals("http://other-endpoint", s3Resource.getProperty(S3Properties.Env.ENDPOINT));
     }
 
     @Test
     public void testHttpScheme() throws DdlException {
-        // if https:// is set, it should be replaced with http://
         ImmutableMap<String, String> properties = ImmutableMap.of(
                 "AWS_ENDPOINT", "https://aaa",
                 "AWS_REGION", "bbb",
@@ -242,7 +269,23 @@ public void testHttpScheme() throws DdlException {
         );
         S3Resource s3Resource = new S3Resource("s3_2");
         s3Resource.setProperties(properties);
-        Assert.assertEquals(s3Resource.getProperty(S3Properties.ENDPOINT), "https://aaa");
+        Assert.assertEquals("https://aaa", s3Resource.getProperty(S3Properties.ENDPOINT));
+    }
+
+    @Test
+    public void testExplicitHttpScheme() throws DdlException {
+        ImmutableMap<String, String> properties = ImmutableMap.of(
+                "AWS_ENDPOINT", "http://aaa",
+                "AWS_REGION", "bbb",
+                "AWS_ROOT_PATH", "/path/to/root",
+                "AWS_ACCESS_KEY", "xxx",
+                "AWS_SECRET_KEY", "yyy",
+                "AWS_BUCKET", "test-bucket",
+                "s3_validity_check", "false"
+        );
+        S3Resource s3Resource = new S3Resource("s3_2");
+        s3Resource.setProperties(properties);
+        Assert.assertEquals("http://aaa", s3Resource.getProperty(S3Properties.ENDPOINT));
     }
 
     @Test

fe/fe-core/src/test/java/org/apache/doris/common/util/S3UtilTest.java

@@ -17,14 +17,68 @@
 
 package org.apache.doris.common.util;
 
+import org.apache.doris.common.Config;
+
+import org.junit.After;
 import org.junit.Assert;
+import org.junit.Before;
 import org.junit.Test;
 
 import java.util.Arrays;
 import java.util.List;
 
 public class S3UtilTest {
 
+    private String originalS3ClientHttpScheme;
+
+    @Before
+    public void setUp() {
+        originalS3ClientHttpScheme = Config.s3_client_http_scheme;
+    }
+
+    @After
+    public void tearDown() {
+        Config.s3_client_http_scheme = originalS3ClientHttpScheme;
+    }
+
+    @Test
+    public void testBuildEndpointUrlDefaultsToHttp() {
+        Assert.assertEquals("http", Config.s3_client_http_scheme);
+        Assert.assertEquals("http://s3.us-east-1.amazonaws.com",
+                S3Util.buildEndpointUrl("s3.us-east-1.amazonaws.com"));
+    }
+
+    @Test
+    public void testBuildEndpointUrlUsesConfiguredHttpsScheme() {
+        Config.s3_client_http_scheme = "https";
+        Assert.assertEquals("https://s3.us-east-1.amazonaws.com",
+                S3Util.buildEndpointUrl("s3.us-east-1.amazonaws.com"));
+    }
+
+    @Test
+    public void testBuildEndpointUrlKeepsExplicitHttp() {
+        Assert.assertEquals("http://127.0.0.1:9000",
+                S3Util.buildEndpointUrl("http://127.0.0.1:9000"));
+    }
+
+    @Test
+    public void testBuildEndpointUrlKeepsExplicitHttps() {
+        Assert.assertEquals("https://s3.us-east-1.amazonaws.com",
+                S3Util.buildEndpointUrl("https://s3.us-east-1.amazonaws.com"));
+    }
+
+    @Test
+    public void testBuildEndpointUrlKeepsUppercaseScheme() {
+        Assert.assertEquals("HTTP://127.0.0.1:9000",
+                S3Util.buildEndpointUrl("HTTP://127.0.0.1:9000"));
+    }
+
+    @Test
+    public void testBuildEndpointUrlKeepsExistingScheme() {
+        Assert.assertEquals("ftp://127.0.0.1:21",
+                S3Util.buildEndpointUrl("ftp://127.0.0.1:21"));
+    }
+
     @Test
     public void testExtendGlobNumberRange_simpleRange() {
         // Test simple range expansion {1..3}
@@ -457,4 +511,3 @@ public void testExpandBracketPatterns_malformedBracket() {
         Assert.assertEquals("file[abc.csv", S3Util.expandBracketPatterns("file[abc.csv"));
     }
 }
-

fe/fe-core/src/test/java/org/apache/doris/nereids/trees/plans/commands/CreateStageCommandTest.java

@@ -101,13 +101,20 @@ public void testAnalyzeStorageProperties() {
         CreateStageCommand command3 = getCreateStageCommand(sql);
         ObjectStoreInfoPB objectStoreInfoPB = command3.getStageProperties().getObjectStoreInfoPB();
 
-        Assertions.assertEquals("cos.ap-beijing.myqcloud.com", objectStoreInfoPB.getEndpoint());
+        Assertions.assertEquals("http://cos.ap-beijing.myqcloud.com", objectStoreInfoPB.getEndpoint());
         Assertions.assertEquals("ap-beijing", objectStoreInfoPB.getRegion());
         Assertions.assertEquals("tmp-bucket", objectStoreInfoPB.getBucket());
         Assertions.assertEquals("tmp_prefix", objectStoreInfoPB.getPrefix());
         Assertions.assertEquals("tmp_ak", objectStoreInfoPB.getAk());
         Assertions.assertEquals("tmp_sk", objectStoreInfoPB.getSk());
         Assertions.assertEquals(Provider.COS, objectStoreInfoPB.getProvider());
+
+        sql = "create stage if not exists ex_stage_1 "
+                + OBJ_INFO.replace("cos.ap-beijing.myqcloud.com", "http://cos.ap-beijing.myqcloud.com") + ")";
+        ObjectStoreInfoPB explicitHttpObjectStoreInfoPB =
+                getCreateStageCommand(sql).getStageProperties().getObjectStoreInfoPB();
+        Assertions.assertEquals("http://cos.ap-beijing.myqcloud.com",
+                explicitHttpObjectStoreInfoPB.getEndpoint());
     }
 
     @Test
@@ -292,7 +299,7 @@ public void testStagePB() throws Exception {
         Assertions.assertEquals("ex_stage_1", stagePB.getName());
         Assertions.assertTrue(StringUtils.isNotBlank(stagePB.getStageId()));
         ObjectStoreInfoPB objInfo = stagePB.getObjInfo();
-        Assertions.assertEquals("cos.ap-beijing.myqcloud.com", objInfo.getEndpoint());
+        Assertions.assertEquals("http://cos.ap-beijing.myqcloud.com", objInfo.getEndpoint());
         Map<String, String> propertiesMap = stagePB.getPropertiesMap();
         Assertions.assertEquals(4, propertiesMap.size());
         Assertions.assertEquals("csv", propertiesMap.get("default.file.type"));

regression-test/suites/cold_heat_separation/policy/alter.groovy

@@ -132,7 +132,7 @@ suite("alter_policy") {
         // AWS_CONNECTION_TIMEOUT_MS
         assertEquals(show_alter_result[5][3], "2222")
         // AWS_ENDPOINT
-        assertEquals(show_alter_result[6][3], "11111111")
+        assertEquals(show_alter_result[6][3], "https://11111111")
         // AWS_REGION
         assertEquals(show_alter_result[7][3], "8888")
         // s3_rootpath
@@ -174,7 +174,7 @@ suite("alter_policy") {
         // AWS_CONNECTION_TIMEOUT_MS
         assertEquals(show_alter_result[5][3], "2222")
         // AWS_ENDPOINT
-        assertEquals(show_alter_result[6][3], "http://bj.s3.comaaaa")
+        assertEquals(show_alter_result[6][3], "https://bj.s3.comaaaa")
         // AWS_REGION
         assertEquals(show_alter_result[7][3], "bj")
         // s3_rootpath