From 5f456ab85201ecf4766466b8fa03af5cfcf5b88e Mon Sep 17 00:00:00 2001 From: morningman Date: Sun, 18 Feb 2024 16:18:32 +0800 Subject: [PATCH 1/5] 1 --- .../org/apache/doris/catalog/S3Resource.java | 28 ++++++++----------- .../doris/common/util/PrintableMap.java | 9 ++++-- 2 files changed, 17 insertions(+), 20 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/S3Resource.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/S3Resource.java index b8ef318077f48f..6b67fa726c0222 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/S3Resource.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/S3Resource.java @@ -109,18 +109,15 @@ protected void setProperties(Map properties) throws DdlException if (needCheck) { String bucketName = properties.get(S3Properties.BUCKET); String rootPath = properties.get(S3Properties.ROOT_PATH); - boolean available = pingS3(credential, bucketName, rootPath, properties); - if (!available) { - throw new DdlException("S3 can't use, please check your properties"); - } + pingS3(credential, bucketName, rootPath, properties); } // optional S3Properties.optionalS3Property(properties); this.properties = properties; } - private static boolean pingS3(CloudCredentialWithEndpoint credential, String bucketName, String rootPath, - Map properties) { + private static void pingS3(CloudCredentialWithEndpoint credential, String bucketName, String rootPath, + Map properties) throws DdlException { String bucket = "s3://" + bucketName + "/"; Map propertiesPing = new HashMap<>(); propertiesPing.put(S3Properties.Env.ACCESS_KEY, credential.getAccessKey()); @@ -134,24 +131,25 @@ private static boolean pingS3(CloudCredentialWithEndpoint credential, String buc String testFile = bucket + rootPath + "/test-object-valid.txt"; String content = "doris will be better"; if (FeConstants.runningUnitTest) { - return true; + return; } try { Status status = fileSystem.directUpload(content, testFile); if (status != Status.OK) { - LOG.warn("ping update file status: {}, properties: {}", status, propertiesPing); - return false; + throw new DdlException( + "ping s3 failed(upload), status: " + status + ", properties: " + new PrintableMap<>( + propertiesPing, "=", true, false, true, false)); } } finally { Status delete = fileSystem.delete(testFile); if (delete != Status.OK) { - LOG.warn("ping delete file status: {}, properties: {}", delete, propertiesPing); - return false; + throw new DdlException( + "ping s3 failed(delete), status: " + delete + ", properties: " + new PrintableMap<>( + propertiesPing, "=", true, false, true, false)); } } LOG.info("success to ping s3"); - return true; } @Override @@ -178,11 +176,7 @@ public void modifyProperties(Map properties) throws DdlException String rootPath = properties.getOrDefault(S3Properties.ROOT_PATH, this.properties.get(S3Properties.ROOT_PATH)); - boolean available = pingS3(getS3PingCredentials(changedProperties), - bucketName, rootPath, changedProperties); - if (!available) { - throw new DdlException("S3 can't use, please check your properties"); - } + pingS3(getS3PingCredentials(changedProperties), bucketName, rootPath, changedProperties); } // modify properties diff --git a/fe/fe-core/src/main/java/org/apache/doris/common/util/PrintableMap.java b/fe/fe-core/src/main/java/org/apache/doris/common/util/PrintableMap.java index 27d6468827b521..7a86c5e9278f71 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/common/util/PrintableMap.java +++ b/fe/fe-core/src/main/java/org/apache/doris/common/util/PrintableMap.java @@ -26,6 +26,8 @@ import org.apache.doris.datasource.property.constants.OssProperties; import org.apache.doris.datasource.property.constants.S3Properties; +import com.aliyun.datalake.metastore.common.DataLakeConfig; +import com.amazonaws.glue.catalog.util.AWSGlueConfig; import com.google.common.collect.Sets; import java.util.ArrayList; @@ -59,9 +61,10 @@ public class PrintableMap { GCSProperties.SECRET_KEY, CosProperties.SECRET_KEY, GlueProperties.SECRET_KEY, MCProperties.SECRET_KEY, DLFProperties.SECRET_KEY)); HIDDEN_KEY = Sets.newHashSet(); - HIDDEN_KEY.addAll(S3Properties.Env.FS_KEYS); - HIDDEN_KEY.addAll(GlueProperties.META_KEYS); - HIDDEN_KEY.addAll(DLFProperties.META_KEYS); + HIDDEN_KEY.add(S3Properties.Env.SECRET_KEY); + HIDDEN_KEY.add(AWSGlueConfig.AWS_GLUE_SECRET_KEY); + HIDDEN_KEY.add(DataLakeConfig.CATALOG_SECURITY_TOKEN); + HIDDEN_KEY.add(DataLakeConfig.CATALOG_ACCESS_KEY_SECRET); } public PrintableMap(Map map, String keyValueSeparator, From 88145217d00a06c28c5dc6489c8dce6d81645e4c Mon Sep 17 00:00:00 2001 From: morningman Date: Mon, 19 Feb 2024 13:51:06 +0800 Subject: [PATCH 2/5] 1 --- .../java/org/apache/doris/catalog/S3Resource.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/S3Resource.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/S3Resource.java index 6b67fa726c0222..e3c32dca090b33 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/S3Resource.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/S3Resource.java @@ -133,19 +133,21 @@ private static void pingS3(CloudCredentialWithEndpoint credential, String bucket if (FeConstants.runningUnitTest) { return; } + Status status = Status.OK; try { - Status status = fileSystem.directUpload(content, testFile); + status = fileSystem.directUpload(content, testFile); if (status != Status.OK) { throw new DdlException( "ping s3 failed(upload), status: " + status + ", properties: " + new PrintableMap<>( propertiesPing, "=", true, false, true, false)); } } finally { - Status delete = fileSystem.delete(testFile); - if (delete != Status.OK) { - throw new DdlException( - "ping s3 failed(delete), status: " + delete + ", properties: " + new PrintableMap<>( - propertiesPing, "=", true, false, true, false)); + if (status.ok()) { + Status delete = fileSystem.delete(testFile); + if (delete != Status.OK) { + LOG.warn("delete test file failed, status: {}, properties: {}", delete, new PrintableMap<>( + propertiesPing, "=", true, false, true, false)); + } } } From edd09cf494d98f4c7538d539f3cb9fd1a0aabcb8 Mon Sep 17 00:00:00 2001 From: morningman Date: Mon, 19 Feb 2024 16:33:54 +0800 Subject: [PATCH 3/5] pathStyleAccessEnabled(true) --- .../src/main/java/org/apache/doris/catalog/ResourceMgr.java | 2 +- .../src/main/java/org/apache/doris/common/util/S3Util.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/ResourceMgr.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/ResourceMgr.java index 34dfb3fa919c52..93cb194c902202 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/ResourceMgr.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/ResourceMgr.java @@ -78,7 +78,7 @@ public void createResource(CreateResourceStmt stmt) throws DdlException { Resource resource = Resource.fromStmt(stmt); if (createResource(resource, stmt.isIfNotExists())) { Env.getCurrentEnv().getEditLog().logCreateResource(resource); - LOG.info("Create resource success. Resource: {}", resource); + LOG.info("Create resource success. Resource: {}", resource.getName()); } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/common/util/S3Util.java b/fe/fe-core/src/main/java/org/apache/doris/common/util/S3Util.java index 05bb2f6a10a271..0195783191cf75 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/common/util/S3Util.java +++ b/fe/fe-core/src/main/java/org/apache/doris/common/util/S3Util.java @@ -92,7 +92,7 @@ public static S3Client buildS3Client(URI endpoint, String region, CloudCredentia // use virtual hosted-style access .serviceConfiguration(S3Configuration.builder() .chunkedEncodingEnabled(false) - .pathStyleAccessEnabled(false) + .pathStyleAccessEnabled(true) .build()) .build(); } From 40ed5fcf2e4d6a2cf1b7feb4f8bee0d8ae17f0be Mon Sep 17 00:00:00 2001 From: morningman Date: Mon, 19 Feb 2024 19:14:21 +0800 Subject: [PATCH 4/5] fix test --- .../java/org/apache/doris/common/util/PrintableMap.java | 9 +++------ .../suites/cold_heat_separation/policy/create.groovy | 4 ++-- 2 files changed, 5 insertions(+), 8 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/common/util/PrintableMap.java b/fe/fe-core/src/main/java/org/apache/doris/common/util/PrintableMap.java index 7a86c5e9278f71..27d6468827b521 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/common/util/PrintableMap.java +++ b/fe/fe-core/src/main/java/org/apache/doris/common/util/PrintableMap.java @@ -26,8 +26,6 @@ import org.apache.doris.datasource.property.constants.OssProperties; import org.apache.doris.datasource.property.constants.S3Properties; -import com.aliyun.datalake.metastore.common.DataLakeConfig; -import com.amazonaws.glue.catalog.util.AWSGlueConfig; import com.google.common.collect.Sets; import java.util.ArrayList; @@ -61,10 +59,9 @@ public class PrintableMap { GCSProperties.SECRET_KEY, CosProperties.SECRET_KEY, GlueProperties.SECRET_KEY, MCProperties.SECRET_KEY, DLFProperties.SECRET_KEY)); HIDDEN_KEY = Sets.newHashSet(); - HIDDEN_KEY.add(S3Properties.Env.SECRET_KEY); - HIDDEN_KEY.add(AWSGlueConfig.AWS_GLUE_SECRET_KEY); - HIDDEN_KEY.add(DataLakeConfig.CATALOG_SECURITY_TOKEN); - HIDDEN_KEY.add(DataLakeConfig.CATALOG_ACCESS_KEY_SECRET); + HIDDEN_KEY.addAll(S3Properties.Env.FS_KEYS); + HIDDEN_KEY.addAll(GlueProperties.META_KEYS); + HIDDEN_KEY.addAll(DLFProperties.META_KEYS); } public PrintableMap(Map map, String keyValueSeparator, diff --git a/regression-test/suites/cold_heat_separation/policy/create.groovy b/regression-test/suites/cold_heat_separation/policy/create.groovy index 81f8fa5001e93c..4a2ad7b9b3b404 100644 --- a/regression-test/suites/cold_heat_separation/policy/create.groovy +++ b/regression-test/suites/cold_heat_separation/policy/create.groovy @@ -197,7 +197,7 @@ suite("create_policy") { "AWS_MAX_CONNECTIONS" = "50", "AWS_REQUEST_TIMEOUT_MS" = "3000", "AWS_CONNECTION_TIMEOUT_MS" = "1000", - "AWS_BUCKET" = "test-bucket", + "AWS_BUCKET" = "test-bucket" ); """ // errCode = 2, detailMessage = Missing [s3_validity_check] in properties. @@ -221,7 +221,7 @@ suite("create_policy") { ); """ // can read AWS_ACCESS_KEY from environment variable - assertEquals(failed_create_2, [[0]]) + assertEquals(failed_create_2, null) } if (has_created_2.size() == 0) { From c6ee55a5c79f73c0f3eb7bcaae069c74e943fd1d Mon Sep 17 00:00:00 2001 From: morningman Date: Mon, 19 Feb 2024 20:52:36 +0800 Subject: [PATCH 5/5] fix --- .../suites/cold_heat_separation/policy/create.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/regression-test/suites/cold_heat_separation/policy/create.groovy b/regression-test/suites/cold_heat_separation/policy/create.groovy index 4a2ad7b9b3b404..b382f05a4e00c3 100644 --- a/regression-test/suites/cold_heat_separation/policy/create.groovy +++ b/regression-test/suites/cold_heat_separation/policy/create.groovy @@ -221,7 +221,7 @@ suite("create_policy") { ); """ // can read AWS_ACCESS_KEY from environment variable - assertEquals(failed_create_2, null) + assertEquals(failed_create_2, [[0]]) } if (has_created_2.size() == 0) {