This is an automated email from the ASF dual-hosted git repository.
diqiu50 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 266139fad5 [#13357] fix(trino-connector): Fail fast when Iceberg REST
routing has no usable authentication (#13358)
266139fad5 is described below
commit 266139fad573cdc3e56205fb5efef964433098e9
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 618f17737c..0bf5d71163 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 06010c12c2..063b62bbdb 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
@@ -555,9 +555,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",
@@ -566,6 +565,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"));
}
