This is an automated email from the ASF dual-hosted git repository.
roryqi pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 519bcb0567 [#6353] fix(authz): Fix the failure of chained plugin to
load JDBC authz plugin (#6372)
519bcb0567 is described below
commit 519bcb05674e0a7268f37855864bd58f8894180b
Author: roryqi <[email protected]>
AuthorDate: Sun Jan 26 17:15:44 2025 +0800
[#6353] fix(authz): Fix the failure of chained plugin to load JDBC authz
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.
---
.../chain/ChainedAuthorizationPlugin.java | 18 ++++++------------
authorizations/authorization-common/build.gradle.kts | 1 +
.../authorization/common/AuthorizationProperties.java | 17 -----------------
.../authorization/jdbc/JdbcAuthorizationPlugin.java | 7 +++++++
.../jdbc/TestJdbcAuthorizationPlugin.java | 4 ++--
5 files changed, 16 insertions(+), 31 deletions(-)
diff --git
a/authorizations/authorization-chain/src/main/java/org/apache/gravitino/authorization/chain/ChainedAuthorizationPlugin.java
b/authorizations/authorization-chain/src/main/java/org/apache/gravitino/authorization/chain/ChainedAuthorizationPlugin.java
index 120c355db0..982ee38cd2 100644
---
a/authorizations/authorization-chain/src/main/java/org/apache/gravitino/authorization/chain/ChainedAuthorizationPlugin.java
+++
b/authorizations/authorization-chain/src/main/java/org/apache/gravitino/authorization/chain/ChainedAuthorizationPlugin.java
@@ -33,7 +33,6 @@ import org.apache.gravitino.authorization.Owner;
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 @@ public class ChainedAuthorizationPlugin implements
AuthorizationPlugin {
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 @@ public class ChainedAuthorizationPlugin implements
AuthorizationPlugin {
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
index 9bab92dac3..0dfd8d2cc2 100644
--- 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/authorizations/authorization-common/src/main/java/org/apache/gravitino/authorization/common/AuthorizationProperties.java
b/authorizations/authorization-common/src/main/java/org/apache/gravitino/authorization/common/AuthorizationProperties.java
index 3ece6353d6..85fe5c72ed 100644
---
a/authorizations/authorization-common/src/main/java/org/apache/gravitino/authorization/common/AuthorizationProperties.java
+++
b/authorizations/authorization-common/src/main/java/org/apache/gravitino/authorization/common/AuthorizationProperties.java
@@ -34,21 +34,4 @@ public abstract class AuthorizationProperties {
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/authorizations/authorization-common/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPlugin.java
b/authorizations/authorization-common/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPlugin.java
index 2d74bd050e..4c8c3330a2 100644
---
a/authorizations/authorization-common/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPlugin.java
+++
b/authorizations/authorization-common/src/main/java/org/apache/gravitino/authorization/jdbc/JdbcAuthorizationPlugin.java
@@ -85,6 +85,13 @@ public abstract class JdbcAuthorizationPlugin implements
AuthorizationPlugin, Jd
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/authorizations/authorization-common/src/test/java/org/apache/gravitino/authorization/jdbc/TestJdbcAuthorizationPlugin.java
b/authorizations/authorization-common/src/test/java/org/apache/gravitino/authorization/jdbc/TestJdbcAuthorizationPlugin.java
index 8d3788617c..c229b7a92f 100644
---
a/authorizations/authorization-common/src/test/java/org/apache/gravitino/authorization/jdbc/TestJdbcAuthorizationPlugin.java
+++
b/authorizations/authorization-common/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) {
