[#6353] fix(authz): Fix the failure of chained plugin to load JDBC au…

…thz plugin (#6372) ### What changes were proposed in this pull request? 1. Load the JDBC driver of when the plugin is initializing 2. Fix the failure of chained plugin to load jdbc plugin ### Why are the changes needed? Fix: #6353 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Test by hand.
apache · Jan 26, 2025 · 519bcb0 · 519bcb0
1 parent 578adde
commit 519bcb0
Show file tree

Hide file tree

Showing 5 changed files with 16 additions and 31 deletions.
diff --git a/...in/src/main/java/org/apache/gravitino/authorization/chain/ChainedAuthorizationPlugin.java b/...in/src/main/java/org/apache/gravitino/authorization/chain/ChainedAuthorizationPlugin.java
@@ -33,7 +33,6 @@
 import org.apache.gravitino.authorization.Role;
 import org.apache.gravitino.authorization.RoleChange;
 import org.apache.gravitino.authorization.User;
-import org.apache.gravitino.authorization.common.AuthorizationProperties;
 import org.apache.gravitino.authorization.common.ChainedAuthorizationProperties;
 import org.apache.gravitino.connector.authorization.AuthorizationPlugin;
 import org.apache.gravitino.connector.authorization.BaseAuthorization;
@@ -55,16 +54,6 @@ private void initPlugins(String catalogProvider, Map<String, String> properties)
     ChainedAuthorizationProperties chainedAuthzProperties =
         new ChainedAuthorizationProperties(properties);
     chainedAuthzProperties.validate();
-    // Validate the properties for each plugin
-    chainedAuthzProperties
-        .plugins()
-        .forEach(
-            pluginName -> {
-              Map<String, String> pluginProperties =
-                  chainedAuthzProperties.fetchAuthPluginProperties(pluginName);
-              String authzProvider = chainedAuthzProperties.getPluginProvider(pluginName);
-              AuthorizationProperties.validate(authzProvider, pluginProperties);
-            });
     // Create the plugins
     chainedAuthzProperties
         .plugins()
@@ -83,8 +72,13 @@ private void initPlugins(String catalogProvider, Map<String, String> properties)
               try {
                 BaseAuthorization<?> authorization =
                     BaseAuthorization.createAuthorization(classLoader, authzProvider);
+
+                // Load the authorization plugin with the class loader of the catalog.
+                // Because the JDBC authorization plugin may load JDBC driver using the class
+                // loader.
                 AuthorizationPlugin authorizationPlugin =
-                    authorization.newPlugin(metalake, catalogProvider, pluginConfig);
+                    classLoader.withClassLoader(
+                        cl -> authorization.newPlugin(metalake, catalogProvider, pluginConfig));
                 plugins.add(authorizationPlugin);
               } catch (Exception e) {
                 throw new RuntimeException(e);

diff --git a/authorizations/authorization-common/build.gradle.kts b/authorizations/authorization-common/build.gradle.kts
@@ -49,6 +49,7 @@ dependencies {
   testImplementation(libs.junit.jupiter.api)
   testImplementation(libs.mockito.core)
   testImplementation(libs.testcontainers)
+  testImplementation(libs.h2db)
   testRuntimeOnly(libs.junit.jupiter.engine)
 }
 

diff --git a/...mmon/src/main/java/org/apache/gravitino/authorization/common/AuthorizationProperties.java b/...mmon/src/main/java/org/apache/gravitino/authorization/common/AuthorizationProperties.java
@@ -34,21 +34,4 @@ public AuthorizationProperties(Map<String, String> properties) {
   public abstract String getPropertiesPrefix();
 
   public abstract void validate();
-
-  public static void validate(String type, Map<String, String> properties) {
-    switch (type) {
-      case "ranger":
-        RangerAuthorizationProperties rangerAuthorizationProperties =
-            new RangerAuthorizationProperties(properties);
-        rangerAuthorizationProperties.validate();
-        break;
-      case "chain":
-        ChainedAuthorizationProperties chainedAuthzProperties =
-            new ChainedAuthorizationProperties(properties);
-        chainedAuthzProperties.validate();
-        break;
-      default:
-        throw new IllegalArgumentException("Unsupported authorization properties type: " + type);
-    }
-  }
 }
diff --git a/...common/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPlugin.java b/...common/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPlugin.java
@@ -85,6 +85,13 @@ public JdbcAuthorizationPlugin(Map<String, String> config) {
     dataSource.setTestOnReturn(BaseObjectPoolConfig.DEFAULT_TEST_ON_RETURN);
     dataSource.setLifo(BaseObjectPoolConfig.DEFAULT_LIFO);
     mappingProvider = new JdbcSecurableObjectMappingProvider();
+
+    try (Connection ignored = getConnection()) {
+      LOG.info("Load the JDBC driver first");
+    } catch (SQLException se) {
+      LOG.error("Jdbc authorization plugin exception: ", se);
+      throw toAuthorizationPluginException(se);
+    }
   }
 
   @Override

diff --git a/...on/src/test/java/org/apache/gravitino/authorization/jdbc/TestJdbcAuthorizationPlugin.java b/...on/src/test/java/org/apache/gravitino/authorization/jdbc/TestJdbcAuthorizationPlugin.java
@@ -52,13 +52,13 @@ public class TestJdbcAuthorizationPlugin {
   private static final Map<String, String> properties =
       ImmutableMap.of(
           JdbcAuthorizationProperties.JDBC_URL,
-          "xx",
+          "jdbc:h2:mem:test",
           JdbcAuthorizationProperties.JDBC_USERNAME,
           "xx",
           JdbcAuthorizationProperties.JDBC_PASSWORD,
           "xx",
           JdbcAuthorizationProperties.JDBC_DRIVER,
-          "xx");
+          "org.h2.Driver");
 
   private static final JdbcAuthorizationPlugin plugin =
       new JdbcAuthorizationPlugin(properties) {