This is an automated email from the ASF dual-hosted git repository.

zykkk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git


The following commit(s) were added to refs/heads/master by this push:
     new b7404896fa [improvement](catalog) avoid calling checksum when 
replaying creating jdbc catalog and fix ranger issue (#22369)
b7404896fa is described below

commit b7404896fa8e22716288c0f956633a8f8d12ba90
Author: Mingyu Chen <morning...@163.com>
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 2293250d5c..07a03ed861 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
@@ -146,7 +146,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 89083a1b30..f213f9a302 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
@@ -110,7 +110,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
     }
 
@@ -203,8 +203,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, "");
@@ -224,7 +226,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 8a076fabec..8bdea35e45 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
@@ -296,7 +296,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 7a9a80aff0..09394a6122 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,external,hive,external_remote,external_r
         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: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org

Reply via email to