[fix](multi-catalog)fix old s3 properties check (#18430)

fix old s3 properties check
fix for #18005 (comment)
This commit is contained in:
slothever
2023-04-18 09:58:13 +08:00
committed by GitHub
parent 0753dc2cc1
commit 98b8efc2c2
10 changed files with 414 additions and 18 deletions

View File

@ -205,6 +205,9 @@ public class Repository implements Writable {
// create repository dir and repo info file
public Status initRepository() {
if (FeConstants.runningUnitTest) {
return Status.OK;
}
String repoInfoFilePath = assembleRepoInfoFilePath();
// check if the repo is already exist in remote
List<RemoteFile> remoteFiles = Lists.newArrayList();

View File

@ -97,11 +97,6 @@ public class S3Storage extends BlobStorage {
public void setProperties(Map<String, String> properties) {
super.setProperties(properties);
caseInsensitiveProperties.putAll(properties);
if (!caseInsensitiveProperties.containsKey(S3Properties.ENDPOINT)) {
// try to get new properties from old version
// compatible with old version
S3Properties.convertToStdProperties(caseInsensitiveProperties);
}
try {
S3Properties.requiredS3Properties(caseInsensitiveProperties);
} catch (DdlException e) {
@ -136,7 +131,6 @@ public class S3Storage extends BlobStorage {
@Override
public FileSystem getFileSystem(String remotePath) throws UserException {
if (dfsFileSystem == null) {
S3Properties.requiredS3Properties(caseInsensitiveProperties);
Configuration conf = new Configuration();
System.setProperty("com.amazonaws.services.s3.enableV4", "true");
PropertyConverter.convertToHadoopFSProperties(caseInsensitiveProperties).forEach(conf::set);
@ -151,7 +145,6 @@ public class S3Storage extends BlobStorage {
private S3Client getClient(String bucket) throws UserException {
if (client == null) {
S3Properties.requiredS3Properties(caseInsensitiveProperties);
URI tmpEndpoint = URI.create(caseInsensitiveProperties.get(S3Properties.ENDPOINT));
StaticCredentialsProvider scp;
if (!caseInsensitiveProperties.containsKey(S3Properties.SESSION_TOKEN)) {

View File

@ -86,9 +86,6 @@ public class S3Resource extends Resource {
@Override
protected void setProperties(Map<String, String> properties) throws DdlException {
Preconditions.checkState(properties != null);
// compatible with old version
S3Properties.convertToStdProperties(properties);
// check properties
S3Properties.requiredS3PingProperties(properties);
// default need check resource conf valid, so need fix ut and regression case
@ -164,7 +161,7 @@ public class S3Resource extends Resource {
throw new DdlException("current not support modify property : " + any.get());
}
}
// compatible with old version
// compatible with old version, Need convert if modified properties map uses old properties.
S3Properties.convertToStdProperties(properties);
boolean needCheck = isNeedCheck(properties);
LOG.debug("s3 info need check validity : {}", needCheck);

View File

@ -129,7 +129,9 @@ public class PropertyConverter {
properties.put(S3Properties.REGION, credential.getRegion());
properties.put(S3Properties.ACCESS_KEY, credential.getAccessKey());
properties.put(S3Properties.SECRET_KEY, credential.getSecretKey());
properties.put(S3Properties.SESSION_TOKEN, credential.getSessionToken());
if (properties.containsKey(S3Properties.Env.TOKEN)) {
properties.put(S3Properties.SESSION_TOKEN, credential.getSessionToken());
}
if (properties.containsKey(S3Properties.Env.MAX_CONNECTIONS)) {
properties.put(S3Properties.MAX_CONNECTIONS, properties.get(S3Properties.Env.MAX_CONNECTIONS));
}
@ -158,6 +160,7 @@ public class PropertyConverter {
s3Properties.put(Constants.SOCKET_TIMEOUT, properties.get(S3Properties.CONNECTION_TIMEOUT_MS));
}
setS3FsAccess(s3Properties, properties, credential);
s3Properties.putAll(properties);
return s3Properties;
}
@ -315,7 +318,6 @@ public class PropertyConverter {
String endpoint = props.get(GlueProperties.ENDPOINT);
props.put(AWSGlueConfig.AWS_GLUE_ENDPOINT, endpoint);
String region = S3Properties.getRegionOfEndpoint(endpoint);
props.put(GlueProperties.REGION, region);
props.put(AWSGlueConfig.AWS_REGION, region);
if (credential.isWhole()) {
props.put(AWSGlueConfig.AWS_GLUE_ACCESS_KEY, credential.getAccessKey());
@ -357,7 +359,7 @@ public class PropertyConverter {
// "s3.secret_key" = "yy"
// )
String endpoint = props.get(GlueProperties.ENDPOINT);
String region = props.getOrDefault(GlueProperties.REGION, S3Properties.getRegionOfEndpoint(endpoint));
String region = S3Properties.getRegionOfEndpoint(endpoint);
if (!Strings.isNullOrEmpty(region)) {
props.put(S3Properties.REGION, region);
String s3Endpoint = "s3." + region + ".amazonaws.com";

View File

@ -39,4 +39,14 @@ public class GlueProperties extends BaseProperties {
public static CloudCredential getCredential(Map<String, String> props) {
return getCloudCredential(props, ACCESS_KEY, SECRET_KEY, SESSION_TOKEN);
}
public static CloudCredential getCompatibleCredential(Map<String, String> props) {
// Compatible with older versions.
CloudCredential credential = getCloudCredential(props, AWSGlueConfig.AWS_GLUE_ACCESS_KEY,
AWSGlueConfig.AWS_GLUE_SECRET_KEY, AWSGlueConfig.AWS_GLUE_SESSION_TOKEN);
if (!credential.isWhole()) {
credential = BaseProperties.getCompatibleCredential(props);
}
return credential;
}
}

View File

@ -97,7 +97,7 @@ public class S3Properties extends BaseProperties {
public static final String DEFAULT_MAX_CONNECTIONS = "50";
public static final String DEFAULT_REQUEST_TIMEOUT_MS = "3000";
public static final String DEFAULT_CONNECTION_TIMEOUT_MS = "1000";
public static final List<String> REQUIRED_FIELDS = Arrays.asList(ENDPOINT, REGION, ACCESS_KEY, SECRET_KEY);
public static final List<String> REQUIRED_FIELDS = Arrays.asList(ENDPOINT, ACCESS_KEY, SECRET_KEY);
public static final List<String> FS_KEYS = Arrays.asList(ENDPOINT, REGION, ACCESS_KEY, SECRET_KEY, TOKEN,
ROOT_PATH, BUCKET, MAX_CONNECTIONS, REQUEST_TIMEOUT_MS, CONNECTION_TIMEOUT_MS);
}
@ -155,8 +155,17 @@ public class S3Properties extends BaseProperties {
}
public static void requiredS3Properties(Map<String, String> properties) throws DdlException {
for (String field : S3Properties.REQUIRED_FIELDS) {
checkRequiredProperty(properties, field);
// Try to convert env properties to uniform properties
// compatible with old version
S3Properties.convertToStdProperties(properties);
if (properties.containsKey(S3Properties.Env.ENDPOINT)) {
for (String field : S3Properties.Env.REQUIRED_FIELDS) {
checkRequiredProperty(properties, field);
}
} else {
for (String field : S3Properties.REQUIRED_FIELDS) {
checkRequiredProperty(properties, field);
}
}
}

View File

@ -315,6 +315,9 @@ public abstract class ExternalFileTableValuedFunction extends TableValuedFunctio
@Override
public List<Column> getTableColumns() throws AnalysisException {
if (FeConstants.runningUnitTest) {
return Lists.newArrayList();
}
if (!csvSchema.isEmpty()) {
return csvSchema;
}

View File

@ -18,8 +18,11 @@
package org.apache.doris.tablefunction;
import org.apache.doris.analysis.BrokerDesc;
import org.apache.doris.analysis.StorageBackend;
import org.apache.doris.analysis.StorageBackend.StorageType;
import org.apache.doris.backup.BlobStorage;
import org.apache.doris.common.AnalysisException;
import org.apache.doris.common.FeConstants;
import org.apache.doris.common.UserException;
import org.apache.doris.common.util.S3URI;
import org.apache.doris.datasource.credentials.CloudCredentialWithEndpoint;
@ -27,6 +30,7 @@ import org.apache.doris.datasource.property.PropertyConverter;
import org.apache.doris.datasource.property.constants.S3Properties;
import org.apache.doris.thrift.TFileType;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableSet;
import java.util.HashMap;
@ -83,7 +87,17 @@ public class S3TableValuedFunction extends ExternalFileTableValuedFunction {
locationProperties = S3Properties.credentialToMap(credential);
String usePathStyle = tvfParams.getOrDefault(PropertyConverter.USE_PATH_STYLE, "false");
locationProperties.put(PropertyConverter.USE_PATH_STYLE, usePathStyle);
parseFile();
if (FeConstants.runningUnitTest) {
// Just check
BlobStorage.create(null, StorageBackend.StorageType.S3, locationProperties);
} else {
parseFile();
}
}
@VisibleForTesting
public static Map<String, String> getParams(Map<String, String> params) throws AnalysisException {
return getValidParams(params);
}
private static Map<String, String> getValidParams(Map<String, String> params) throws AnalysisException {