diff --git a/fe/fe-core/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java b/fe/fe-core/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java index 536ca5633bf30a..a66a2ae4432947 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java +++ b/fe/fe-core/src/main/java/org/apache/doris/common/util/PropertyAnalyzer.java @@ -34,6 +34,7 @@ import org.apache.doris.common.AnalysisException; import org.apache.doris.common.Config; import org.apache.doris.common.DdlException; +import org.apache.doris.common.Pair; import org.apache.doris.datasource.CatalogIf; import org.apache.doris.datasource.ExternalCatalog; import org.apache.doris.policy.Policy; @@ -1183,14 +1184,50 @@ public static String analyzeStoragePolicy(Map properties) { return storagePolicy; } - public static String analyzeStorageVault(Map properties) { - String storageVault = null; + /** + * @param properties + * @return + * @throws AnalysisException + */ + public static Pair analyzeStorageVault(Map properties) throws AnalysisException { + String storageVaultName = null; if (properties != null && properties.containsKey(PROPERTIES_STORAGE_VAULT_NAME)) { - storageVault = properties.get(PROPERTIES_STORAGE_VAULT_NAME); + storageVaultName = properties.get(PROPERTIES_STORAGE_VAULT_NAME); properties.remove(PROPERTIES_STORAGE_VAULT_NAME); } - return storageVault; + if (Strings.isNullOrEmpty(storageVaultName)) { + // If user does not specify one storage vault then FE would use the default vault + Pair info = Env.getCurrentEnv().getStorageVaultMgr().getDefaultStorageVault(); + if (info == null) { + throw new AnalysisException("No default storage vault." + + " You can use `SHOW STORAGE VAULT` to get all available vaults," + + " and pick one set default vault with `SET AS DEFAULT STORAGE VAULT`"); + } + storageVaultName = info.first; + LOG.info("Using default storage vault, name:{} id:{}", info.first, info.second); + } + + if (Strings.isNullOrEmpty(storageVaultName)) { + throw new AnalysisException("Invalid Storage Vault. " + + " You can use `SHOW STORAGE VAULT` to get all available vaults," + + " and pick one to set the table property `\"storage_vault_name\" = \"\"`"); + } + + String storageVaultId = Env.getCurrentEnv().getStorageVaultMgr().getVaultIdByName(storageVaultName); + if (Strings.isNullOrEmpty(storageVaultId)) { + throw new AnalysisException("Storage vault '" + storageVaultName + "' does not exist. " + + "You can use `SHOW STORAGE VAULT` to get all available vaults, " + + "or create a new one with `CREATE STORAGE VAULT`."); + } + + if (properties != null && properties.containsKey(PROPERTIES_STORAGE_VAULT_ID)) { + Preconditions.checkArgument(storageVaultId.equals(properties.get(PROPERTIES_STORAGE_VAULT_ID)), + "storageVaultId check failed, %s-%s", storageVaultId, properties.get(PROPERTIES_STORAGE_VAULT_ID)); + properties.remove(PROPERTIES_STORAGE_VAULT_ID); + } + + return Pair.of(storageVaultName, storageVaultId); } // analyze property like : "type" = "xxx"; diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java index bfffd3e154b3b1..5b3c106bbc8e52 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/InternalCatalog.java @@ -2736,44 +2736,20 @@ private boolean createOlapTable(Database db, CreateTableStmt stmt) throws UserEx olapTable.setEnableSingleReplicaCompaction(enableSingleReplicaCompaction); if (Config.isCloudMode() && ((CloudEnv) env).getEnableStorageVault()) { - // set storage vault - String storageVaultName = PropertyAnalyzer.analyzeStorageVault(properties); - String storageVaultId = null; - // If user does not specify one storage vault then FE would use the default vault - if (Strings.isNullOrEmpty(storageVaultName)) { - Pair info = env.getStorageVaultMgr().getDefaultStorageVault(); - if (info != null) { - storageVaultName = info.first; - storageVaultId = info.second; - LOG.info("Using default storage vault: name={}, id={}", storageVaultName, storageVaultId); - } else { - throw new DdlException("No default storage vault." - + " You can use `SHOW STORAGE VAULT` to get all available vaults," - + " and pick one set default vault with `SET AS DEFAULT STORAGE VAULT`"); - } - } - - if (storageVaultName == null || storageVaultName.isEmpty()) { - throw new DdlException("Invalid Storage Vault. " - + " You can use `SHOW STORAGE VAULT` to get all available vaults," - + " and pick one to set the table property `\"storage_vault_name\" = \"\"`"); - } + // + Pair storageVaultInfoPair = PropertyAnalyzer.analyzeStorageVault(properties); // Check if user has storage vault usage privilege - if (ctx != null && !env.getAuth() - .checkStorageVaultPriv(ctx.getCurrentUserIdentity(), storageVaultName, PrivPredicate.USAGE)) { + if (ConnectContext.get() != null && !env.getAuth() + .checkStorageVaultPriv(ctx.getCurrentUserIdentity(), + storageVaultInfoPair.first, PrivPredicate.USAGE)) { throw new DdlException("USAGE denied to user '" + ConnectContext.get().getQualifiedUser() + "'@'" + ConnectContext.get().getRemoteIP() - + "' for storage vault '" + storageVaultName + "'"); - } - - storageVaultId = env.getStorageVaultMgr().getVaultIdByName(storageVaultName); - if (Strings.isNullOrEmpty(storageVaultId)) { - throw new DdlException("Storage vault '" + storageVaultName + "' does not exist. " - + "You can use `SHOW STORAGE VAULT` to get all available vaults, " - + "or create a new one with `CREATE STORAGE VAULT`."); + + "' for storage vault '" + storageVaultInfoPair.first + "'"); } - olapTable.setStorageVaultId(storageVaultId); + Preconditions.checkArgument(StringUtils.isNumeric(storageVaultInfoPair.second), + "Invaild storage vault id :%s", storageVaultInfoPair.second); + olapTable.setStorageVaultId(storageVaultInfoPair.second); } // check `update on current_timestamp` diff --git a/regression-test/suites/vault_p0/privilege/test_vault_privilege_with_role.groovy b/regression-test/suites/vault_p0/privilege/test_vault_privilege_with_role.groovy index 54cace642d4188..7192dc40aed816 100644 --- a/regression-test/suites/vault_p0/privilege/test_vault_privilege_with_role.groovy +++ b/regression-test/suites/vault_p0/privilege/test_vault_privilege_with_role.groovy @@ -46,6 +46,16 @@ suite("test_vault_privilege_with_role", "nonConcurrent") { sql """CREATE USER ${userName} identified by '${userPassword}' DEFAULT ROLE '${roleName}'""" sql """GRANT create_priv ON *.*.* TO '${userName}'; """ + sql """ + CREATE STORAGE VAULT ${hdfsVaultName} + PROPERTIES ( + "type"="HDFS", + "fs.defaultFS"="${getHmsHdfsFs()}", + "path_prefix" = "${hdfsVaultName}", + "hadoop.username" = "${getHmsUser()}" + ); + """ + connect(userName, userPassword, context.config.jdbcUrl) { expectExceptionLike({ sql """ @@ -65,16 +75,6 @@ suite("test_vault_privilege_with_role", "nonConcurrent") { sql """ GRANT usage_priv ON STORAGE VAULT '${hdfsVaultName}' TO ROLE '${roleName}';""" - sql """ - CREATE STORAGE VAULT ${hdfsVaultName} - PROPERTIES ( - "type"="HDFS", - "fs.defaultFS"="${getHmsHdfsFs()}", - "path_prefix" = "${hdfsVaultName}", - "hadoop.username" = "${getHmsUser()}" - ); - """ - connect(userName, userPassword, context.config.jdbcUrl) { sql """ CREATE TABLE IF NOT EXISTS ${dbName}.${tableName} (