From 30f785c36e244340e3fe1ecaa29ddf5a170c62f8 Mon Sep 17 00:00:00 2001 From: Lei Zhang Date: Fri, 10 Jan 2025 15:20:58 +0800 Subject: [PATCH] [fix](vault) Fix creating storage vault failed with azure backend * fix create azure storage vault should use `AzureResource` instead of `S3Resource` * show storage vaults display `use_path_style` info --- .../doris/analysis/CreateResourceStmt.java | 7 ++ .../apache/doris/catalog/AzureResource.java | 87 ++++++++----------- .../org/apache/doris/catalog/S3Resource.java | 3 +- .../apache/doris/catalog/StorageVault.java | 8 +- .../org/apache/doris/common/util/S3URI.java | 4 +- .../apache/doris/fs/obj/AzureObjStorage.java | 4 +- .../doris/fs/remote/AzureFileSystem.java | 4 - 7 files changed, 51 insertions(+), 66 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateResourceStmt.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateResourceStmt.java index 210c034300e327..4a358510fa015b 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateResourceStmt.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/CreateResourceStmt.java @@ -26,6 +26,7 @@ import org.apache.doris.common.FeNameFormat; import org.apache.doris.common.UserException; import org.apache.doris.common.util.PrintableMap; +import org.apache.doris.datasource.property.constants.AzureProperties; import org.apache.doris.mysql.privilege.PrivPredicate; import org.apache.doris.qe.ConnectContext; @@ -72,6 +73,12 @@ public void analyzeResourceType() throws UserException { if (type == null) { throw new AnalysisException("Resource type can't be null"); } + + if (AzureProperties.checkAzureProviderPropertyExist(properties)) { + resourceType = ResourceType.AZURE; + return; + } + resourceType = ResourceType.fromString(type); if (resourceType == ResourceType.UNKNOWN) { throw new AnalysisException("Unsupported resource type: " + type); diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/AzureResource.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/AzureResource.java index 7291d8f44ce5d3..53f52f3e8cc65c 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/AzureResource.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/AzureResource.java @@ -20,7 +20,6 @@ import org.apache.doris.backup.Status; import org.apache.doris.common.DdlException; import org.apache.doris.common.FeConstants; -import org.apache.doris.common.credentials.CloudCredentialWithEndpoint; import org.apache.doris.common.proc.BaseProcResult; import org.apache.doris.common.util.PrintableMap; import org.apache.doris.datasource.property.constants.S3Properties; @@ -39,9 +38,8 @@ import java.util.Optional; public class AzureResource extends Resource { - private static final Logger LOG = LogManager.getLogger(AzureResource.class); - private Map properties; + private Map properties = Maps.newHashMap(); public AzureResource() { super(); @@ -52,89 +50,84 @@ public AzureResource(String name) { } @Override - protected void setProperties(Map properties) throws DdlException { - Preconditions.checkState(properties != null); + protected void setProperties(Map newProperties) throws DdlException { + Preconditions.checkState(newProperties != null); // check properties - S3Properties.requiredS3PingProperties(properties); + S3Properties.requiredS3PingProperties(newProperties); // default need check resource conf valid, so need fix ut and regression case - boolean needCheck = isNeedCheck(properties); + boolean needCheck = isNeedCheck(newProperties); if (LOG.isDebugEnabled()) { LOG.debug("azure info need check validity : {}", needCheck); } // the endpoint for ping need add uri scheme. - String pingEndpoint = properties.get(S3Properties.ENDPOINT); + String pingEndpoint = newProperties.get(S3Properties.ENDPOINT); if (!pingEndpoint.startsWith("http://")) { - pingEndpoint = "http://" + properties.get(S3Properties.ENDPOINT); - properties.put(S3Properties.ENDPOINT, pingEndpoint); - properties.put(S3Properties.Env.ENDPOINT, pingEndpoint); + pingEndpoint = "http://" + newProperties.get(S3Properties.ENDPOINT); + newProperties.put(S3Properties.ENDPOINT, pingEndpoint); + newProperties.put(S3Properties.Env.ENDPOINT, pingEndpoint); } - String region = S3Properties.getRegionOfEndpoint(pingEndpoint); - properties.putIfAbsent(S3Properties.REGION, region); - String ak = properties.get(S3Properties.ACCESS_KEY); - String sk = properties.get(S3Properties.SECRET_KEY); - String token = properties.get(S3Properties.SESSION_TOKEN); - CloudCredentialWithEndpoint credential = new CloudCredentialWithEndpoint(pingEndpoint, region, ak, sk, token); if (needCheck) { - String bucketName = properties.get(S3Properties.BUCKET); - String rootPath = properties.get(S3Properties.ROOT_PATH); - pingAzure(credential, bucketName, rootPath, properties); + String bucketName = newProperties.get(S3Properties.BUCKET); + String rootPath = newProperties.get(S3Properties.ROOT_PATH); + pingAzure(bucketName, rootPath, newProperties); } // optional - S3Properties.optionalS3Property(properties); - this.properties = properties; + S3Properties.optionalS3Property(newProperties); + this.properties = newProperties; } - private static void pingAzure(CloudCredentialWithEndpoint credential, String bucketName, String rootPath, - Map properties) throws DdlException { - AzureFileSystem fileSystem = new AzureFileSystem(properties); - String testFile = rootPath + "/test-object-valid.txt"; + private static void pingAzure(String bucketName, String rootPath, + Map newProperties) throws DdlException { if (FeConstants.runningUnitTest) { return; } + + String testFile = "azure://" + bucketName + "/" + rootPath + "/test-object-valid.txt"; + AzureFileSystem fileSystem = new AzureFileSystem(newProperties); Status status = fileSystem.exists(testFile); - if (status != Status.OK || status.getErrCode() != Status.ErrCode.NOT_FOUND) { + if (status != Status.OK && status.getErrCode() != Status.ErrCode.NOT_FOUND) { throw new DdlException( "ping azure failed(head), status: " + status + ", properties: " + new PrintableMap<>( - properties, "=", true, false, true, false)); + newProperties, "=", true, false, true, false)); } - LOG.info("success to ping azure"); } @Override - public void modifyProperties(Map properties) throws DdlException { + public void modifyProperties(Map newProperties) throws DdlException { if (references.containsValue(ReferenceType.POLICY)) { // can't change, because remote fs use it info to find data. List cantChangeProperties = Arrays.asList(S3Properties.ENDPOINT, S3Properties.REGION, S3Properties.ROOT_PATH, S3Properties.BUCKET, S3Properties.Env.ENDPOINT, S3Properties.Env.REGION, S3Properties.Env.ROOT_PATH, S3Properties.Env.BUCKET); - Optional any = cantChangeProperties.stream().filter(properties::containsKey).findAny(); + Optional any = cantChangeProperties.stream().filter(newProperties::containsKey).findAny(); if (any.isPresent()) { throw new DdlException("current not support modify property : " + any.get()); } } // compatible with old version, Need convert if modified properties map uses old properties. - S3Properties.convertToStdProperties(properties); - boolean needCheck = isNeedCheck(properties); + S3Properties.convertToStdProperties(newProperties); + boolean needCheck = isNeedCheck(newProperties); if (LOG.isDebugEnabled()) { 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, + changedProperties.putAll(newProperties); + String bucketName = newProperties.getOrDefault(S3Properties.BUCKET, + this.properties.get(S3Properties.BUCKET)); + String rootPath = newProperties.getOrDefault(S3Properties.ROOT_PATH, this.properties.get(S3Properties.ROOT_PATH)); - pingAzure(getS3PingCredentials(changedProperties), bucketName, rootPath, changedProperties); + pingAzure(bucketName, rootPath, changedProperties); } // modify properties writeLock(); - for (Map.Entry kv : properties.entrySet()) { + for (Map.Entry kv : newProperties.entrySet()) { replaceIfEffectiveValue(this.properties, kv.getKey(), kv.getValue()); if (kv.getKey().equals(S3Properties.Env.TOKEN) || kv.getKey().equals(S3Properties.SESSION_TOKEN)) { @@ -143,19 +136,7 @@ public void modifyProperties(Map properties) throws DdlException } ++version; writeUnlock(); - super.modifyProperties(properties); - } - - private CloudCredentialWithEndpoint getS3PingCredentials(Map 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); + super.modifyProperties(newProperties); } private boolean isNeedCheck(Map newProperties) { @@ -169,7 +150,7 @@ private boolean isNeedCheck(Map newProperties) { @Override public Map getCopiedProperties() { - return Maps.newHashMap(properties); + return Maps.newHashMap(this.properties); } @Override @@ -178,7 +159,7 @@ protected void getProcNodeData(BaseProcResult result) { result.addRow(Lists.newArrayList(name, lowerCaseType, "id", String.valueOf(id))); readLock(); result.addRow(Lists.newArrayList(name, lowerCaseType, "version", String.valueOf(version))); - for (Map.Entry entry : properties.entrySet()) { + for (Map.Entry entry : this.properties.entrySet()) { if (PrintableMap.HIDDEN_KEY.contains(entry.getKey())) { continue; } 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 a40e91f47d46d5..26747e826fdadb 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 @@ -119,9 +119,8 @@ protected void setProperties(Map properties) throws DdlException private static void pingS3(CloudCredentialWithEndpoint credential, String bucketName, String rootPath, Map properties) throws DdlException { - String bucket = "s3://" + bucketName + "/"; S3FileSystem fileSystem = new S3FileSystem(properties); - String testFile = bucket + rootPath + "/test-object-valid.txt"; + String testFile = "s3://" + bucketName + "/" + rootPath + "/test-object-valid.txt"; String content = "doris will be better"; if (FeConstants.runningUnitTest) { return; diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/StorageVault.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/StorageVault.java index c1a22430d50f07..d7a049b515e2df 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/StorageVault.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/StorageVault.java @@ -28,8 +28,6 @@ import com.google.common.base.Strings; import com.google.protobuf.TextFormat; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; import java.util.ArrayList; import java.util.List; @@ -38,7 +36,6 @@ import java.util.stream.IntStream; public abstract class StorageVault { - private static final Logger LOG = LogManager.getLogger(StorageVault.class); public static final String REFERENCE_SPLIT = "@"; public static final String INCLUDE_DATABASE_LIST = "include_database_list"; public static final String EXCLUDE_DATABASE_LIST = "exclude_database_list"; @@ -149,6 +146,7 @@ public void setId(String id) { if (!stmt.getProperties().containsKey(PropertyConverter.USE_PATH_STYLE)) { stmt.getProperties().put(PropertyConverter.USE_PATH_STYLE, "true"); } + CreateResourceStmt resourceStmt = new CreateResourceStmt(false, ifNotExists, name, stmt.getProperties()); resourceStmt.analyzeResourceType(); @@ -215,6 +213,10 @@ public static List convertToShowStorageVaultProperties(Cloud.StorageVaul builder.mergeFrom(vault.getObjInfo()); builder.clearId(); builder.setSk("xxxxxxx"); + if (!vault.getObjInfo().hasUsePathStyle()) { + // There is no `use_path_style` field in old version, think `use_path_style` false + builder.setUsePathStyle(false); + } row.add(printer.shortDebugString(builder)); } row.add("false"); diff --git a/fe/fe-core/src/main/java/org/apache/doris/common/util/S3URI.java b/fe/fe-core/src/main/java/org/apache/doris/common/util/S3URI.java index 70263b4bf1649c..a560a8c6f9dc5c 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/common/util/S3URI.java +++ b/fe/fe-core/src/main/java/org/apache/doris/common/util/S3URI.java @@ -71,10 +71,10 @@ public class S3URI { public static final String SCHEME_DELIM = "://"; public static final String PATH_DELIM = "/"; private static final Set VALID_SCHEMES = ImmutableSet.of("http", "https", "s3", "s3a", "s3n", - "bos", "oss", "cos", "cosn", "obs"); + "bos", "oss", "cos", "cosn", "obs", "azure"); private static final Set OS_SCHEMES = ImmutableSet.of("s3", "s3a", "s3n", - "bos", "oss", "cos", "cosn", "obs"); + "bos", "oss", "cos", "cosn", "obs", "azure"); private URI uri; diff --git a/fe/fe-core/src/main/java/org/apache/doris/fs/obj/AzureObjStorage.java b/fe/fe-core/src/main/java/org/apache/doris/fs/obj/AzureObjStorage.java index 0253993cc425ef..cda78ba8773060 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/fs/obj/AzureObjStorage.java +++ b/fe/fe-core/src/main/java/org/apache/doris/fs/obj/AzureObjStorage.java @@ -137,8 +137,8 @@ public Status headObject(String remotePath) { try { S3URI uri = S3URI.create(remotePath, isUsePathStyle, forceParsingByStandardUri); BlobClient blobClient = getClient().getBlobContainerClient(uri.getBucket()).getBlobClient(uri.getKey()); - BlobProperties properties = blobClient.getProperties(); - LOG.info("head file {} success: {}", remotePath, properties.toString()); + LOG.info("headObject remotePath:{} bucket:{} key:{} properties:{}", + remotePath, uri.getBucket(), uri.getKey(), blobClient.getProperties()); return Status.OK; } catch (BlobStorageException e) { if (e.getStatusCode() == HttpStatus.SC_NOT_FOUND) { diff --git a/fe/fe-core/src/main/java/org/apache/doris/fs/remote/AzureFileSystem.java b/fe/fe-core/src/main/java/org/apache/doris/fs/remote/AzureFileSystem.java index c116182d3a4241..097c64a744b844 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/fs/remote/AzureFileSystem.java +++ b/fe/fe-core/src/main/java/org/apache/doris/fs/remote/AzureFileSystem.java @@ -24,15 +24,11 @@ import com.google.common.annotations.VisibleForTesting; import org.apache.hadoop.fs.FileSystem; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; import java.util.List; import java.util.Map; public class AzureFileSystem extends ObjFileSystem { - private static final Logger LOG = LogManager.getLogger(AzureFileSystem.class); - public AzureFileSystem(Map properties) { super(StorageType.AZURE.name(), StorageType.S3, new AzureObjStorage(properties)); initFsProperties();