This is an automated email from the ASF dual-hosted git repository. kxiao pushed a commit to branch branch-2.0 in repository https://gitbox.apache.org/repos/asf/doris.git
commit 37c75353ecdc756eddae0eb4001738ff49e102bb Author: Mingyu Chen <[email protected]> AuthorDate: Wed Aug 30 19:24:11 2023 +0800 [improvement](catalog) avoid calling checksum when replaying creating jdbc catalog and fix ranger issue (#22369) 1. jdbc Before, in the constructor of Jdbc catalog, we may call checksum action of the jdbc driver. But the download link of the jdbc driver may not be available when replaying, causing replay error. This PR change the logic to avoid calling checksum when replaying creating jdbc catalog. 2. ranger After this PR, when creating catalog, it will try to init access controller to make sure the config is ok. 3. catalog priv check When creating/dropping/altering/ catalog, doris will only use internal access controller to check catalog level priv. --- .../org/apache/doris/datasource/CatalogFactory.java | 10 +++++++++- .../org/apache/doris/datasource/ExternalCatalog.java | 8 +++++--- .../apache/doris/datasource/HMSExternalCatalog.java | 5 ++++- .../doris/datasource/jdbc/JdbcExternalCatalog.java | 12 ++++++++++++ .../doris/mysql/privilege/AccessControllerManager.java | 18 ++++++++++++------ .../hive/test_external_catalog_hive.groovy | 15 +++++++++++++++ 6 files changed, 57 insertions(+), 11 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogFactory.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogFactory.java index 5aa75daaad..b5579a2438 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogFactory.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogFactory.java @@ -142,7 +142,15 @@ public class CatalogFactory { if (!isReplay) { // set some default properties when creating catalog. // do not call this method when replaying edit log. Because we need to keey the original properties. - catalog.setDefaultProps(); + catalog.setDefaultPropsWhenCreating(isReplay); + // This will check if the customized access controller can be created successfully. + // If failed, it will throw exception and the catalog will not be created. + try { + catalog.initAccessController(true); + } catch (Exception e) { + LOG.warn("Failed to init access controller", e); + throw new DdlException("Failed to init access controller: " + e.getMessage()); + } } return catalog; } diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java index 38e4fe2642..8f322cfd73 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java @@ -106,7 +106,7 @@ public abstract class ExternalCatalog + "listDatabaseNames from remote client when init catalog with " + logType.name()); } - public void setDefaultProps() { + public void setDefaultPropsWhenCreating(boolean isReplay) throws DdlException { // set some default properties when creating catalog } @@ -199,8 +199,10 @@ public abstract class ExternalCatalog * "access_controller.properties.prop1" = "xxx", * "access_controller.properties.prop2" = "yyy", * ) + * <p> + * isDryRun: if true, it will try to create the custom access controller, but will not add it to the access manager. */ - public void initAccessController() { + public void initAccessController(boolean isDryRun) { Map<String, String> properties = getCatalogProperty().getProperties(); // 1. get access controller class name String className = properties.getOrDefault(CatalogMgr.ACCESS_CONTROLLER_CLASS_PROP, ""); @@ -220,7 +222,7 @@ public abstract class ExternalCatalog } // 3. create access controller - Env.getCurrentEnv().getAccessManager().createAccessController(name, className, acProperties); + Env.getCurrentEnv().getAccessManager().createAccessController(name, className, acProperties, isDryRun); } // init schema related objects diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/HMSExternalCatalog.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/HMSExternalCatalog.java index 462fd3527a..40430f8dea 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/HMSExternalCatalog.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/HMSExternalCatalog.java @@ -289,7 +289,10 @@ public class HMSExternalCatalog extends ExternalCatalog { } @Override - public void setDefaultProps() { + public void setDefaultPropsWhenCreating(boolean isReplay) { + if (isReplay) { + return; + } if (catalogProperty.getOrDefault(PROP_ALLOW_FALLBACK_TO_SIMPLE_AUTH, "").isEmpty()) { // always allow fallback to simple auth, so to support both kerberos and simple auth catalogProperty.addProperty(PROP_ALLOW_FALLBACK_TO_SIMPLE_AUTH, "true"); diff --git a/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/JdbcExternalCatalog.java b/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/JdbcExternalCatalog.java index 753a5d6906..7617ad7180 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/JdbcExternalCatalog.java +++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/jdbc/JdbcExternalCatalog.java @@ -166,4 +166,16 @@ public class JdbcExternalCatalog extends ExternalCatalog { makeSureInitialized(); return jdbcClient.isTableExist(dbName, tblName); } + + @Override + public void setDefaultPropsWhenCreating(boolean isReplay) throws DdlException { + if (isReplay) { + return; + } + Map<String, String> properties = Maps.newHashMap(); + if (properties.containsKey(JdbcResource.DRIVER_URL) && !properties.containsKey(JdbcResource.CHECK_SUM)) { + properties.put(JdbcResource.CHECK_SUM, + JdbcResource.computeObjectChecksum(properties.get(JdbcResource.DRIVER_URL))); + } + } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/AccessControllerManager.java b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/AccessControllerManager.java index 3e0ff23915..257a6e88bc 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/AccessControllerManager.java +++ b/fe/fe-core/src/main/java/org/apache/doris/mysql/privilege/AccessControllerManager.java @@ -59,7 +59,7 @@ public class AccessControllerManager { ctlToCtlAccessController.put(InternalCatalog.INTERNAL_CATALOG_NAME, internalAccessController); } - private CatalogAccessController getAccessControllerOrDefault(String ctl) { + public CatalogAccessController getAccessControllerOrDefault(String ctl) { CatalogAccessController catalogAccessController = ctlToCtlAccessController.get(ctl); if (catalogAccessController != null) { return catalogAccessController; @@ -77,7 +77,7 @@ public class AccessControllerManager { if (ctlToCtlAccessController.containsKey(catalog.getName())) { return; } - catalog.initAccessController(); + catalog.initAccessController(false); if (!ctlToCtlAccessController.containsKey(catalog.getName())) { ctlToCtlAccessController.put(catalog.getName(), internalAccessController); } @@ -88,14 +88,17 @@ public class AccessControllerManager { return ctlToCtlAccessController.containsKey(ctl); } - public void createAccessController(String ctl, String acFactoryClassName, Map<String, String> prop) { + public void createAccessController(String ctl, String acFactoryClassName, Map<String, String> prop, + boolean isDryRun) { Class<?> factoryClazz = null; try { factoryClazz = Class.forName(acFactoryClassName); AccessControllerFactory factory = (AccessControllerFactory) factoryClazz.newInstance(); CatalogAccessController accessController = factory.createAccessController(prop); - ctlToCtlAccessController.put(ctl, accessController); - LOG.info("create access controller {} for catalog {}", ctl, acFactoryClassName); + if (!isDryRun) { + ctlToCtlAccessController.put(ctl, accessController); + LOG.info("create access controller {} for catalog {}", ctl, acFactoryClassName); + } } catch (ClassNotFoundException e) { throw new RuntimeException(e); } catch (InstantiationException e) { @@ -130,7 +133,10 @@ public class AccessControllerManager { public boolean checkCtlPriv(UserIdentity currentUser, String ctl, PrivPredicate wanted) { boolean hasGlobal = sysAccessController.checkGlobalPriv(currentUser, wanted); - return getAccessControllerOrDefault(ctl).checkCtlPriv(hasGlobal, currentUser, ctl, wanted); + // for checking catalog priv, always use InternalCatalogAccessController. + // because catalog priv is only saved in InternalCatalogAccessController. + return getAccessControllerOrDefault(InternalCatalog.INTERNAL_CATALOG_NAME).checkCtlPriv(hasGlobal, currentUser, + ctl, wanted); } // ==== Database ==== diff --git a/regression-test/suites/external_table_p2/hive/test_external_catalog_hive.groovy b/regression-test/suites/external_table_p2/hive/test_external_catalog_hive.groovy index 1f32218b34..fed3ba60af 100644 --- a/regression-test/suites/external_table_p2/hive/test_external_catalog_hive.groovy +++ b/regression-test/suites/external_table_p2/hive/test_external_catalog_hive.groovy @@ -128,5 +128,20 @@ suite("test_external_catalog_hive", "p2") { logger.info("recoding select: " + res3.toString()) sql """alter catalog hms rename ${catalog_name};""" + + // test wrong access controller + test { + def tmp_name = "${catalog_name}" + "_wrong" + sql "drop catalog if exists ${tmp_name}" + sql """ + create catalog if not exists ${tmp_name} properties ( + 'type'='hms', + 'hive.metastore.uris' = 'thrift://${extHiveHmsHost}:${extHiveHmsPort}', + 'access_controller.properties.ranger.service.name' = 'hive_wrong', + 'access_controller.class' = 'org.apache.doris.catalog.authorizer.RangerHiveAccessControllerFactory' + ); + """ + exception "Failed to init access controller: bound must be positive" + } } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
