From 6f511ac85933f8d7cf04be60a2389e7fc0197faa Mon Sep 17 00:00:00 2001 From: slothever <18522955+wsjz@users.noreply.github.com> Date: Tue, 23 May 2023 16:20:07 +0800 Subject: [PATCH] [fix](s3)fix s3 resource check (#19933) fix s3 resource check: ERROR 1105 (HY000): Unexpected exception: org.apache.doris.common.DdlException: errCode = 2, detailMessage = Missing [AWS_ACCESS_KEY] in properties. we should use new properties to check s3 available --- .../org/apache/doris/catalog/S3Resource.java | 9 ++++++++- .../property/constants/S3Properties.java | 15 ++++++++++---- .../apache/doris/catalog/S3ResourceTest.java | 20 +++++++++++++++++++ 3 files changed, 39 insertions(+), 5 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 6427123037..a12fd15861 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 @@ -19,6 +19,7 @@ package org.apache.doris.catalog; import org.apache.doris.backup.Status; import org.apache.doris.common.DdlException; +import org.apache.doris.common.FeConstants; import org.apache.doris.common.proc.BaseProcResult; import org.apache.doris.common.util.PrintableMap; import org.apache.doris.datasource.credentials.CloudCredentialWithEndpoint; @@ -131,6 +132,9 @@ public class S3Resource extends Resource { S3FileSystem fileSystem = new S3FileSystem(properties); String testFile = bucket + rootPath + "/test-object-valid.txt"; String content = "doris will be better"; + if (FeConstants.runningUnitTest) { + return true; + } try { Status status = fileSystem.directUpload(content, testFile); if (status != Status.OK) { @@ -167,11 +171,14 @@ public class S3Resource extends Resource { LOG.debug("s3 info need check validity : {}", needCheck); if (needCheck) { S3Properties.requiredS3PingProperties(this.properties); + Map changedProperties = new HashMap<>(this.properties); + changedProperties.putAll(properties); String bucketName = properties.getOrDefault(S3Properties.BUCKET, this.properties.get(S3Properties.BUCKET)); String rootPath = properties.getOrDefault(S3Properties.ROOT_PATH, this.properties.get(S3Properties.ROOT_PATH)); - boolean available = pingS3(getS3PingCredentials(properties), bucketName, rootPath, properties); + boolean available = pingS3(getS3PingCredentials(changedProperties), + bucketName, rootPath, changedProperties); if (!available) { throw new DdlException("S3 can't use, please check your properties"); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/constants/S3Properties.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/constants/S3Properties.java index a100982995..e42eba9b80 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/property/constants/S3Properties.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/property/constants/S3Properties.java @@ -161,7 +161,8 @@ public class S3Properties extends BaseProperties { // Try to convert env properties to uniform properties // compatible with old version S3Properties.convertToStdProperties(properties); - if (properties.containsKey(S3Properties.Env.ENDPOINT)) { + if (properties.containsKey(S3Properties.Env.ENDPOINT) + && !properties.containsKey(S3Properties.ENDPOINT)) { for (String field : S3Properties.Env.REQUIRED_FIELDS) { checkRequiredProperty(properties, field); } @@ -196,12 +197,18 @@ public class S3Properties extends BaseProperties { } public static void convertToStdProperties(Map properties) { - properties.putIfAbsent(S3Properties.ENDPOINT, properties.get(S3Properties.Env.ENDPOINT)); + if (properties.containsKey(S3Properties.Env.ENDPOINT)) { + properties.putIfAbsent(S3Properties.ENDPOINT, properties.get(S3Properties.Env.ENDPOINT)); + } if (properties.containsKey(S3Properties.Env.REGION)) { properties.putIfAbsent(S3Properties.REGION, properties.get(S3Properties.Env.REGION)); } - properties.putIfAbsent(S3Properties.ACCESS_KEY, properties.get(S3Properties.Env.ACCESS_KEY)); - properties.putIfAbsent(S3Properties.SECRET_KEY, properties.get(S3Properties.Env.SECRET_KEY)); + if (properties.containsKey(S3Properties.Env.ACCESS_KEY)) { + properties.putIfAbsent(S3Properties.ACCESS_KEY, properties.get(S3Properties.Env.ACCESS_KEY)); + } + if (properties.containsKey(S3Properties.Env.SECRET_KEY)) { + properties.putIfAbsent(S3Properties.SECRET_KEY, properties.get(S3Properties.Env.SECRET_KEY)); + } if (properties.containsKey(S3Properties.Env.TOKEN)) { properties.putIfAbsent(S3Properties.SESSION_TOKEN, properties.get(S3Properties.Env.TOKEN)); } diff --git a/fe/fe-core/src/test/java/org/apache/doris/catalog/S3ResourceTest.java b/fe/fe-core/src/test/java/org/apache/doris/catalog/S3ResourceTest.java index a4995ebde8..720e2690c0 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/catalog/S3ResourceTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/catalog/S3ResourceTest.java @@ -21,6 +21,7 @@ import org.apache.doris.analysis.AccessTestUtil; import org.apache.doris.analysis.Analyzer; import org.apache.doris.analysis.CreateResourceStmt; import org.apache.doris.common.DdlException; +import org.apache.doris.common.FeConstants; import org.apache.doris.common.FeMetaVersion; import org.apache.doris.common.UserException; import org.apache.doris.datasource.property.constants.S3Properties; @@ -202,4 +203,23 @@ public class S3ResourceTest { s3Dis.close(); Files.deleteIfExists(path); } + + @Test + public void testModifyProperties() throws Exception { + Map properties = new HashMap<>(); + properties.put("AWS_ENDPOINT", "aaa"); + properties.put("AWS_REGION", "bbb"); + properties.put("AWS_ROOT_PATH", "/path/to/root"); + properties.put("AWS_ACCESS_KEY", "xxx"); + properties.put("AWS_SECRET_KEY", "yyy"); + properties.put("AWS_BUCKET", "test-bucket"); + properties.put("s3_validity_check", "false"); + S3Resource s3Resource = new S3Resource("t_source"); + s3Resource.setProperties(properties); + FeConstants.runningUnitTest = true; + + Map modify = new HashMap<>(); + modify.put("s3.access_key", "aaa"); + s3Resource.modifyProperties(modify); + } }