This is an automated email from the ASF dual-hosted git repository. diqiu50 pushed a commit to branch trino-irc-1.3 in repository https://gitbox.apache.org/repos/asf/gravitino.git
commit d6212754f06d3dbd22d77beb2a44d501c5178b86 Author: yuhui <[email protected]> AuthorDate: Thu Aug 27 09:22:55 2026 +0000 fix(trino): reuse OAuth config for Iceberg REST --- .../gravitino/trino/connector/GravitinoConfig.java | 39 +++++++++++++++--- .../trino/connector/GravitinoConnectorFactory.java | 8 +++- .../trino/connector/TestGravitinoConfig.java | 48 ++++++++++++++++++++++ .../TestGravitinoConnectorFactoryStart.java | 16 ++++++++ 4 files changed, 104 insertions(+), 7 deletions(-) diff --git a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConfig.java b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConfig.java index 6f3f213418..553e091e4d 100644 --- a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConfig.java +++ b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConfig.java @@ -825,13 +825,42 @@ public class GravitinoConfig { */ public Map<String, String> getIcebergRestCatalogConfig() { String prefix = GRAVITINO_ICEBERG_REST_CATALOG_CONFIG_PREFIX.key; - return config.entrySet().stream() + Map<String, String> restCatalogConfig = new HashMap<>(); + + if ("oauth2".equalsIgnoreCase(config.get(GravitinoAuthProvider.AUTH_TYPE_KEY))) { + restCatalogConfig.put(TRINO_ICEBERG_REST_CATALOG_PREFIX + "security", "OAUTH2"); + putIfNotBlank( + restCatalogConfig, + TRINO_ICEBERG_REST_CATALOG_PREFIX + "oauth2.credential", + config.get(GravitinoAuthProvider.OAUTH_CREDENTIAL_KEY)); + putIfNotBlank( + restCatalogConfig, + TRINO_ICEBERG_REST_CATALOG_PREFIX + "oauth2.scope", + config.get(GravitinoAuthProvider.OAUTH_SCOPE_KEY)); + + String serverUri = config.get(GravitinoAuthProvider.OAUTH_SERVER_URI_KEY); + String path = config.get(GravitinoAuthProvider.OAUTH_PATH_KEY); + if (StringUtils.isNotBlank(serverUri) && StringUtils.isNotBlank(path)) { + restCatalogConfig.put( + TRINO_ICEBERG_REST_CATALOG_PREFIX + "oauth2.server-uri", + StringUtils.removeEnd(serverUri, "/") + "/" + StringUtils.removeStart(path, "/")); + } + } + + config.entrySet().stream() .filter(entry -> entry.getKey().startsWith(prefix)) - .collect( - Collectors.toMap( - entry -> + .forEach( + entry -> + restCatalogConfig.put( TRINO_ICEBERG_REST_CATALOG_PREFIX + entry.getKey().substring(prefix.length()), - Map.Entry::getValue)); + entry.getValue())); + return restCatalogConfig; + } + + private static void putIfNotBlank(Map<String, String> target, String key, String value) { + if (StringUtils.isNotBlank(value)) { + target.put(key, value); + } } private long parseLongConfigEntry(ConfigEntry entry) { diff --git a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConnectorFactory.java b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConnectorFactory.java index a748c0ab58..3c9e804286 100644 --- a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConnectorFactory.java +++ b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConnectorFactory.java @@ -126,11 +126,15 @@ public class GravitinoConnectorFactory implements ConnectorFactory { CatalogRegister catalogRegister = new CatalogRegister(); CatalogConnectorFactory catalogConnectorFactory = createCatalogConnectorFactory(config); - catalogConnectorManager = + CatalogConnectorManager newCatalogConnectorManager = new CatalogConnectorManager( catalogRegister, catalogConnectorFactory, this::getTrinoCatalogName); - catalogConnectorManager.config(config, client); + newCatalogConnectorManager.config(config, client); + // Publish the manager only after it has been configured successfully. Otherwise a + // failed client initialization leaves a shared manager with a null Gravitino client, + // causing later connector creation attempts to fail with a misleading NPE. + catalogConnectorManager = newCatalogConnectorManager; gravitinoSystemTableFactory = new GravitinoSystemTableFactory(catalogConnectorManager); } diff --git a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoConfig.java b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoConfig.java index b7ec6b8ff9..601af08ab3 100644 --- a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoConfig.java +++ b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoConfig.java @@ -359,6 +359,54 @@ public class TestGravitinoConfig { "client_id:client_secret", restCatalogConfig.get("iceberg.rest-catalog.oauth2.credential")); } + @Test + public void testIcebergRestOAuthDefaultsToGravitinoClientOAuth() { + GravitinoConfig config = + new GravitinoConfig( + ImmutableMap.<String, String>builder() + .put("gravitino.metalake", "user_001") + .put("gravitino.client.authType", "oauth2") + .put("gravitino.client.oauth2.serverUri", "https://oauth.example.com/";) + .put("gravitino.client.oauth2.path", "/realms/gravitino/token") + .put("gravitino.client.oauth2.credential", "shared-client:shared-secret") + .put("gravitino.client.oauth2.scope", "openid") + .build()); + + Map<String, String> restCatalogConfig = config.getIcebergRestCatalogConfig(); + assertEquals("OAUTH2", restCatalogConfig.get("iceberg.rest-catalog.security")); + assertEquals( + "shared-client:shared-secret", + restCatalogConfig.get("iceberg.rest-catalog.oauth2.credential")); + assertEquals("openid", restCatalogConfig.get("iceberg.rest-catalog.oauth2.scope")); + assertEquals( + "https://oauth.example.com/realms/gravitino/token";, + restCatalogConfig.get("iceberg.rest-catalog.oauth2.server-uri")); + } + + @Test + public void testIcebergRestOAuthOverridesGravitinoClientOAuthByField() { + GravitinoConfig config = + new GravitinoConfig( + ImmutableMap.<String, String>builder() + .put("gravitino.metalake", "user_001") + .put("gravitino.client.authType", "oauth2") + .put("gravitino.client.oauth2.serverUri", "https://oauth.example.com";) + .put("gravitino.client.oauth2.path", "realms/gravitino/token") + .put("gravitino.client.oauth2.credential", "shared-client:shared-secret") + .put("gravitino.client.oauth2.scope", "openid") + .put("gravitino.iceberg.rest-catalog.oauth2.credential", "irc-client:irc-secret") + .put("gravitino.iceberg.rest-catalog.oauth2.scope", "irc-scope") + .build()); + + Map<String, String> restCatalogConfig = config.getIcebergRestCatalogConfig(); + assertEquals( + "irc-client:irc-secret", restCatalogConfig.get("iceberg.rest-catalog.oauth2.credential")); + assertEquals("irc-scope", restCatalogConfig.get("iceberg.rest-catalog.oauth2.scope")); + assertEquals( + "https://oauth.example.com/realms/gravitino/token";, + restCatalogConfig.get("iceberg.rest-catalog.oauth2.server-uri")); + } + @Test public void testIcebergRestConfigScopedToMetalakeInMultiMetalakeMode() { ImmutableMap<String, String> configMap = diff --git a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoConnectorFactoryStart.java b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoConnectorFactoryStart.java index b104b4d509..cc189815df 100644 --- a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoConnectorFactoryStart.java +++ b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoConnectorFactoryStart.java @@ -20,6 +20,7 @@ package org.apache.gravitino.trino.connector; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mock; @@ -127,6 +128,21 @@ public class TestGravitinoConnectorFactoryStart { assertTrue(factory.isCatalogConnectorManagerStartTriggered()); } + @Test + public void testFailedClientInitializationDoesNotPublishManager() { + CoordinatorFactory factory = new CoordinatorFactory(null); + Map<String, String> brokenDynamicConfig = dynamicConfig(); + brokenDynamicConfig.put("gravitino.client.authType", "oauth2"); + + assertThrows( + Exception.class, + () -> factory.create("broken_catalog", brokenDynamicConfig, mockContext())); + + assertNull(factory.getCatalogConnectorManager()); + assertNotNull(factory.create("gravitino", staticConfig(), mockContext())); + assertTrue(factory.isCatalogConnectorManagerStartTriggered()); + } + @Test public void testStartIsAttemptedOnlyOnce() { CoordinatorFactory factory = newFactory();
