This is an automated email from the ASF dual-hosted git repository.
jerryshao pushed a commit to branch branch-1.3
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.3 by this push:
new 2d15222051 [Cherry-pick to branch-1.3] [#13357] fix(trino-connector):
Fail fast when Iceberg REST routing has no usable authentication (#13358)
(#13368)
2d15222051 is described below
commit 2d15222051d5e29603e994f094c1dc523d5515c4
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Mon Sep 21 15:13:00 2026 +0800
[Cherry-pick to branch-1.3] [#13357] fix(trino-connector): Fail fast when
Iceberg REST routing has no usable authentication (#13358) (#13368)
**Cherry-pick Information:**
- Original commit: 266139fad573cdc3e56205fb5efef964433098e9
- Target branch: `branch-1.3`
- Status: ✅ Clean cherry-pick (no conflicts)
Co-authored-by: Yuhui <[email protected]>
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"));
}
