This is an automated email from the ASF dual-hosted git repository. github-actions[bot] pushed a commit to branch cherry-pick-266139fa-to-branch-1.3 in repository https://gitbox.apache.org/repos/asf/gravitino.git
commit ecf45d06e55171a1e22d9f6a41fe12fa1211b088 Author: Yuhui <[email protected]> AuthorDate: Mon Sep 21 12:48:11 2026 +0800 [#13357] fix(trino-connector): Fail fast when Iceberg REST routing has no usable authentication (#13358) ### What changes were proposed in this pull request? `GravitinoConfig.getIcebergRestCatalogConfig()` now fails fast when a `lakehouse-iceberg` catalog is routed through the Iceberg REST server but the connector's `authType` (`basic`, `kerberos`) has no Trino Iceberg REST security equivalent and `gravitino.iceberg.rest-catalog.security` was not set explicitly. ### Why are the changes needed? Such a catalog previously registered successfully and every query failed at `fetchConfig` with a `NotAuthorizedException` far from its actual cause. Failing at registration surfaces the real problem with an actionable message instead. Fix: #13357 ### Does this PR introduce _any_ user-facing change? Yes. A `lakehouse-iceberg` catalog routed through the IRC with `authType=simple/basic/kerberos` and no explicit `gravitino.iceberg.rest-catalog.security` now fails to register instead of registering and failing every query. ### How was this patch tested? Added unit tests in `TestGravitinoConfig` covering the new failure and its `security=NONE` / explicit-security escape hatches; updated an existing `TestIcebergCatalogPropertyConverter` test that relied on the previously-silent behavior. `./gradlew :trino-connector:trino-connector:test -PskipITs` — 306 tests, 0 failures. --------- Co-authored-by: Claude Sonnet 5 <[email protected]> --- .../gravitino/trino/connector/GravitinoConfig.java | 49 +++++++++++++++++++++- .../trino/connector/TestGravitinoConfig.java | 49 ++++++++++++++++++++++ .../TestIcebergCatalogPropertyConverter.java | 6 +-- 3 files changed, 100 insertions(+), 4 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 4113101418..c87a193efc 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 @@ -31,6 +31,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Properties; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -80,6 +81,15 @@ public class GravitinoConfig { /** The Trino Iceberg REST catalog property prefix. */ private static final String TRINO_ICEBERG_REST_CATALOG_PREFIX = "iceberg.rest-catalog."; + /** + * {@code gravitino.client.authType} values that authenticate the connector's own Gravitino client + * but have no representation in Trino's Iceberg REST security modes ({@code NONE}/{@code + * OAUTH2}). A catalog routed through the Iceberg REST server under one of these types sends no + * credentials to it unless {@code gravitino.iceberg.rest-catalog.security} is set explicitly. + */ + private static final Set<String> AUTH_TYPES_WITHOUT_REST_CATALOG_EQUIVALENT = + Set.of("basic", "kerberos"); + private static final String OAUTH2 = "OAUTH2"; /** Prefix for environment-variable references propagated to dynamic catalogs. */ @@ -878,12 +888,18 @@ public class GravitinoConfig { * {@code gravitino.iceberg.rest-catalog.} prefix rewritten to {@code iceberg.rest-catalog.}. * * @return the Trino Iceberg REST catalog properties + * @throws TrinoException if the connector authenticates to Gravitino with a type that has no + * Trino Iceberg REST security equivalent and {@code gravitino.iceberg.rest-catalog.security} + * was not set explicitly to resolve the mismatch */ public Map<String, String> getIcebergRestCatalogConfig() { String prefix = GRAVITINO_ICEBERG_REST_CATALOG_CONFIG_PREFIX.key; Map<String, String> restCatalogConfig = new HashMap<>(); - if (OAUTH2.equalsIgnoreCase(config.get(GravitinoAuthProvider.AUTH_TYPE_KEY)) + String authType = config.get(GravitinoAuthProvider.AUTH_TYPE_KEY); + if ("simple".equalsIgnoreCase(authType)) { + restCatalogConfig.put(TRINO_ICEBERG_REST_CATALOG_PREFIX + "security", "NONE"); + } else if (OAUTH2.equalsIgnoreCase(authType) && OAUTH2.equalsIgnoreCase(config.getOrDefault(prefix + "security", OAUTH2))) { restCatalogConfig.put(TRINO_ICEBERG_REST_CATALOG_PREFIX + "security", OAUTH2); putIfNotBlank( @@ -911,9 +927,40 @@ public class GravitinoConfig { restCatalogConfig.put( TRINO_ICEBERG_REST_CATALOG_PREFIX + entry.getKey().substring(prefix.length()), entry.getValue())); + + validateRestCatalogAuthentication(restCatalogConfig); return restCatalogConfig; } + /** + * Fails fast when the resolved Iceberg REST catalog config would send no credentials to the + * Iceberg REST server, yet the connector authenticates to Gravitino itself with {@code basic} or + * {@code kerberos}, which Trino's Iceberg REST client cannot carry over. Left unchecked, such a + * catalog registers successfully and every query against it fails at {@code fetchConfig} once the + * REST server requires authentication, an error far removed from its cause. + */ + private void validateRestCatalogAuthentication(Map<String, String> restCatalogConfig) { + if (restCatalogConfig.containsKey(TRINO_ICEBERG_REST_CATALOG_PREFIX + "security")) { + return; + } + String authType = config.get(GravitinoAuthProvider.AUTH_TYPE_KEY); + if (authType == null + || !AUTH_TYPES_WITHOUT_REST_CATALOG_EQUIVALENT.contains( + authType.toLowerCase(Locale.ROOT))) { + return; + } + throw new TrinoException( + GravitinoErrorCode.GRAVITINO_MISSING_CONFIG, + String.format( + "Cannot route an Iceberg catalog through the Iceberg REST server: " + + "gravitino.client.authType=%s has no equivalent Trino Iceberg REST security " + + "mode, so no credentials would be sent to it. If the REST server requires " + + "authentication, set 'gravitino.iceberg.rest-catalog.security' (and any " + + "matching oauth2.* properties) explicitly. If it does not, set " + + "'gravitino.iceberg.rest-catalog.security=NONE' to confirm that.", + authType)); + } + private static void putIfNotBlank(Map<String, String> target, String key, String value) { if (StringUtils.isNotBlank(value)) { target.put(key, value); 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 78a8431da6..164780de5f 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 @@ -463,6 +463,55 @@ public class TestGravitinoConfig { "client_id:client_secret", restCatalogConfig.get("iceberg.rest-catalog.oauth2.credential")); } + @Test + public void testIcebergRestConfigRejectsBasicAuthWithoutExplicitSecurity() { + GravitinoConfig config = + new GravitinoConfig( + ImmutableMap.of( + "gravitino.metalake", "user_001", + "gravitino.client.authType", "basic", + "gravitino.client.basic.username", "admin", + "gravitino.client.basic.password", "admin-pass")); + + TrinoException e = assertThrows(TrinoException.class, config::getIcebergRestCatalogConfig); + assertTrue(e.getMessage().contains("gravitino.client.authType=basic")); + assertTrue(e.getMessage().contains("gravitino.iceberg.rest-catalog.security")); + } + + @Test + public void testIcebergRestConfigMapsSimpleAuthToNone() { + GravitinoConfig simpleConfig = + new GravitinoConfig( + ImmutableMap.of( + "gravitino.metalake", "user_001", + "gravitino.client.authType", "simple")); + assertEquals( + "NONE", simpleConfig.getIcebergRestCatalogConfig().get("iceberg.rest-catalog.security")); + } + + @Test + public void testIcebergRestConfigRejectsKerberosAuthWithoutExplicitSecurity() { + GravitinoConfig kerberosConfig = + new GravitinoConfig( + ImmutableMap.of( + "gravitino.metalake", "user_001", + "gravitino.client.authType", "KERBEROS")); + assertThrows(TrinoException.class, kerberosConfig::getIcebergRestCatalogConfig); + } + + @Test + public void testIcebergRestConfigAllowsBasicAuthWithExplicitSecurityOverride() { + GravitinoConfig config = + new GravitinoConfig( + ImmutableMap.of( + "gravitino.metalake", "user_001", + "gravitino.client.authType", "basic", + "gravitino.iceberg.rest-catalog.security", "NONE")); + + Map<String, String> restCatalogConfig = config.getIcebergRestCatalogConfig(); + assertEquals("NONE", restCatalogConfig.get("iceberg.rest-catalog.security")); + } + @Test public void testIcebergRestOAuthDefaultsToGravitinoClientOAuth() { GravitinoConfig config = diff --git a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/iceberg/TestIcebergCatalogPropertyConverter.java b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/iceberg/TestIcebergCatalogPropertyConverter.java index 3775d8444b..ec3ca36559 100644 --- a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/iceberg/TestIcebergCatalogPropertyConverter.java +++ b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/iceberg/TestIcebergCatalogPropertyConverter.java @@ -553,9 +553,8 @@ public class TestIcebergCatalogPropertyConverter { .put("jdbc-driver", "org.postgresql.Driver") .build(); - // With authType=simple no iceberg.rest-catalog.security is emitted, so the REST catalog has no - // token endpoint to exchange Trino's subject JWT at and the per-user session mode must stay - // off. + // authType=simple maps to iceberg.rest-catalog.security=NONE, so the REST catalog has no token + // endpoint to exchange Trino's subject JWT at and the per-user session mode must stay off. Map<String, String> config = buildConnectorConfig( "catalog1", @@ -564,6 +563,7 @@ public class TestIcebergCatalogPropertyConverter { ImmutableMap.of( "gravitino.client.session.forwardUser", "true", "gravitino.client.authType", "simple"))); + Assertions.assertEquals("NONE", config.get("iceberg.rest-catalog.security")); Assertions.assertNull(config.get("iceberg.rest-catalog.session")); }
