This is an automated email from the ASF dual-hosted git repository.
jerryshao 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 b76d94243f [#13068] fix(trino-connector): Enable Iceberg REST per-user
sessions only under OAuth2 (#13078)
b76d94243f is described below
commit b76d94243f89b16dd9f4d06ef10a7f8fc52537e4
Author: Yuhui <[email protected]>
AuthorDate: Thu Sep 10 20:06:55 2026 +0800
[#13068] fix(trino-connector): Enable Iceberg REST per-user sessions only
under OAuth2 (#13078)
### What changes were proposed in this pull request?
Set `iceberg.rest-catalog.session=USER` only when the internal Iceberg
REST
catalog authenticates with OAuth2, instead of whenever `forwardUser` is
enabled.
### Why are the changes needed?
The per-user session mode makes the Iceberg client exchange a
Trino-signed JWT
for an access token. Without OAuth2 there is no token endpoint, so the
client
falls back to `{rest-uri}/v1/oauth/tokens` and every table load fails
with
HTTP 404 on Trino 473. It carries no user identity in that case either.
Fix: #13068
### Does this PR introduce _any_ user-facing change?
No new property keys. With `forwardUser=true` and non-OAuth2
authentication,
`iceberg.rest-catalog.session` is no longer set; it can be restored with
`gravitino.iceberg.rest-catalog.session=USER`.
### How was this patch tested?
`TestIcebergCatalogPropertyConverter`, covering both OAuth2 routes, the
explicit
override, and the simple-auth case.
---
docs/trino-connector/authentication.md | 3 +-
docs/trino-connector/catalog-iceberg.md | 16 +++++--
.../iceberg/IcebergCatalogPropertyConverter.java | 30 +++++++++++--
.../TestIcebergCatalogPropertyConverter.java | 52 ++++++++++++++++++++--
4 files changed, 90 insertions(+), 11 deletions(-)
diff --git a/docs/trino-connector/authentication.md
b/docs/trino-connector/authentication.md
index 42c7932f6b..e7166b3542 100644
--- a/docs/trino-connector/authentication.md
+++ b/docs/trino-connector/authentication.md
@@ -197,7 +197,8 @@ For an Iceberg catalog reached through the Gravitino
Iceberg REST server (IRC)
`lakehouse-iceberg` catalog for which the Gravitino server reports a running
IRC; see [Iceberg
catalog](./catalog-iceberg.md#how-trino-reaches-the-catalog) — the IRC's own
authentication is
configured once per Trino cluster with the `gravitino.iceberg.rest-catalog.`
prefix, and
-`iceberg.rest-catalog.session=USER` is set automatically when
`forwardUser=true`:
+`iceberg.rest-catalog.session=USER` is set automatically when
`forwardUser=true` and the IRC is
+configured with `gravitino.iceberg.rest-catalog.security=OAUTH2` (as below):
```properties
gravitino.iceberg.rest-catalog.security=OAUTH2
diff --git a/docs/trino-connector/catalog-iceberg.md
b/docs/trino-connector/catalog-iceberg.md
index 4fc7b35336..6244590e72 100644
--- a/docs/trino-connector/catalog-iceberg.md
+++ b/docs/trino-connector/catalog-iceberg.md
@@ -111,10 +111,18 @@ Four keys are reserved: `iceberg.rest-catalog.uri`,
`.warehouse`, `.prefix` and
`gravitino.iceberg.rest-catalog.` or a catalog's `trino.bypass.` has no effect
— the connector logs
when it ignores one.
-When `gravitino.client.session.forwardUser=true`, the connector also sets
-`iceberg.rest-catalog.session=USER` so that each query carries the end user's
identity to the IRC,
-keeping per-user credential vending and per-user authorization intact. Set
-`gravitino.iceberg.rest-catalog.session` explicitly to override it. See
+The connector sets `iceberg.rest-catalog.session=USER` automatically when both
hold:
+
+- `gravitino.client.session.forwardUser=true`
+- the IRC authenticates with OAuth2, through any one of:
+ - `gravitino.client.authType=oauth2`
+ - `gravitino.iceberg.rest-catalog.security=OAUTH2`
+ - `trino.bypass.iceberg.rest-catalog.security=OAUTH2`, on a catalog with its
own REST backend
+
+Each query then carries the end user's identity to the IRC, keeping per-user
credential vending and
+per-user authorization intact. Otherwise the session mode is deliberately left
off: the forwarded
+token cannot be exchanged, so it would carry no identity to the IRC. Set
+`gravitino.iceberg.rest-catalog.session` explicitly to override either way. See
[Authentication](./authentication.md) for the full setup.
### Limitations
diff --git
a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/iceberg/IcebergCatalogPropertyConverter.java
b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/iceberg/IcebergCatalogPropertyConverter.java
index 4c1d2f3e66..0bb8766e9b 100644
---
a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/iceberg/IcebergCatalogPropertyConverter.java
+++
b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/iceberg/IcebergCatalogPropertyConverter.java
@@ -65,6 +65,9 @@ public class IcebergCatalogPropertyConverter extends
CatalogPropertyConverter {
private static final String TRINO_ICEBERG_REST_VENDED_CREDENTIALS =
"iceberg.rest-catalog.vended-credentials-enabled";
private static final String TRINO_ICEBERG_REST_SESSION =
"iceberg.rest-catalog.session";
+ private static final String TRINO_ICEBERG_REST_SECURITY =
"iceberg.rest-catalog.security";
+ private static final String TRINO_ICEBERG_REST_SECURITY_OAUTH2 = "OAUTH2";
+ private static final String TRINO_ICEBERG_REST_SESSION_USER = "USER";
private static final String TRINO_FS_HADOOP_ENABLED = "fs.hadoop.enabled";
private static final String TRINO_FS_NATIVE_S3_ENABLED =
"fs.native-s3.enabled";
private static final String TRINO_FS_NATIVE_GCS_ENABLED =
"fs.native-gcs.enabled";
@@ -174,15 +177,15 @@ public class IcebergCatalogPropertyConverter extends
CatalogPropertyConverter {
// precedence, unrelated to HashMap's (unspecified) iteration order.
config.putAll(buildStorageProperties(catalog.getProperties()));
config.put(TRINO_ICEBERG_REST_VENDED_CREDENTIALS, "true");
- if (gravitinoConfig.isForwardUser()) {
- config.put(TRINO_ICEBERG_REST_SESSION, "USER");
- }
// The catalog's own trino.bypass properties override the defaults above,
so that a Trino
// release renaming one of them can be worked around without a connector
change.
config.putAll(super.gravitinoToEngineProperties(catalog.getProperties()));
// The IRC's own authentication is a cluster-level operational setting, so
it takes precedence
// over anything set on a single catalog.
config.putAll(gravitinoConfig.getIcebergRestCatalogConfig());
+ // Runs after the two putAll calls above, because it depends on both the
security mode and
+ // any explicit session value they settle.
+ applyForwardUserSession(gravitinoConfig, config);
warnOnReservedOverrides(catalog, config);
@@ -248,6 +251,27 @@ public class IcebergCatalogPropertyConverter extends
CatalogPropertyConverter {
return jdbcProperties;
}
+ /**
+ * Turns on Trino's per-user Iceberg REST sessions when user forwarding is
enabled and the REST
+ * catalog authenticates with OAuth2. In that session mode Trino signs a
subject JWT for the
+ * session user and attaches it to every request; the Iceberg client
consumes such a token through
+ * an OAuth2 token exchange. Under any other security mode there is no token
endpoint to exchange
+ * it at, so the token carries no user identity and the mode buys nothing.
+ *
+ * <p>An explicit {@code iceberg.rest-catalog.session} coming from the
catalog or the connector
+ * config is left untouched.
+ */
+ private void applyForwardUserSession(
+ GravitinoConfig gravitinoConfig, Map<String, String> config) {
+ if (!gravitinoConfig.isForwardUser() ||
config.containsKey(TRINO_ICEBERG_REST_SESSION)) {
+ return;
+ }
+ if (TRINO_ICEBERG_REST_SECURITY_OAUTH2.equalsIgnoreCase(
+ config.get(TRINO_ICEBERG_REST_SECURITY))) {
+ config.put(TRINO_ICEBERG_REST_SESSION, TRINO_ICEBERG_REST_SESSION_USER);
+ }
+ }
+
// Called before the reserved keys (type/uri/warehouse/prefix) are put into
`config` below, so
// any value already present at this point came from the catalog's own
properties or bypass
// config and is about to be silently overwritten; this logs that so it
isn't a silent no-op.
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 5b00e4d23e..06010c12c2 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
@@ -499,13 +499,27 @@ public class TestIcebergCatalogPropertyConverter {
.put("jdbc-driver", "org.postgresql.Driver")
.build();
- Map<String, String> config =
+ Map<String, String> oauth2Config =
+ buildConnectorConfig(
+ "catalog1",
+ properties,
+ icebergRestConfiguredConfig(
+ ImmutableMap.of(
+ "gravitino.client.session.forwardUser", "true",
+ "gravitino.client.authType", "oauth2")));
+ Assertions.assertEquals("USER",
oauth2Config.get("iceberg.rest-catalog.session"));
+
+ // The session mode can also be reached without the Gravitino client
itself using OAuth2, by
+ // pointing the REST catalog at its own OAuth2 provider.
+ Map<String, String> restOnlyOauth2Config =
buildConnectorConfig(
"catalog1",
properties,
icebergRestConfiguredConfig(
- ImmutableMap.of("gravitino.client.session.forwardUser",
"true")));
- Assertions.assertEquals("USER",
config.get("iceberg.rest-catalog.session"));
+ ImmutableMap.of(
+ "gravitino.client.session.forwardUser", "true",
+ "gravitino.iceberg.rest-catalog.security", "OAUTH2")));
+ Assertions.assertEquals("USER",
restOnlyOauth2Config.get("iceberg.rest-catalog.session"));
Map<String, String> explicitConfig =
buildConnectorConfig(
@@ -514,15 +528,47 @@ public class TestIcebergCatalogPropertyConverter {
icebergRestConfiguredConfig(
ImmutableMap.of(
"gravitino.client.session.forwardUser", "true",
+ "gravitino.client.authType", "oauth2",
"gravitino.iceberg.rest-catalog.session", "NONE")));
Assertions.assertEquals("NONE",
explicitConfig.get("iceberg.rest-catalog.session"));
+ // OAuth2 alone does not enable the mode; forwarding has to be asked for.
+ Map<String, String> noForwardingConfig =
+ buildConnectorConfig(
+ "catalog1",
+ properties,
+
icebergRestConfiguredConfig(ImmutableMap.of("gravitino.client.authType",
"oauth2")));
+
Assertions.assertNull(noForwardingConfig.get("iceberg.rest-catalog.session"));
+
Map<String, String> defaultConfig =
buildConnectorConfig(
"catalog1", properties,
icebergRestConfiguredConfig(ImmutableMap.of()));
Assertions.assertNull(defaultConfig.get("iceberg.rest-catalog.session"));
}
+ @Test
+ public void testBuildConnectorPropertiesSkipsSessionUserWithoutOauth2()
throws Exception {
+ Map<String, String> properties =
+ ImmutableMap.<String, String>builder()
+ .put("catalog-backend", "jdbc")
+ .put("uri", "jdbc:postgresql://localhost:5432/iceberg")
+ .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.
+ Map<String, String> config =
+ buildConnectorConfig(
+ "catalog1",
+ properties,
+ icebergRestConfiguredConfig(
+ ImmutableMap.of(
+ "gravitino.client.session.forwardUser", "true",
+ "gravitino.client.authType", "simple")));
+ Assertions.assertNull(config.get("iceberg.rest-catalog.session"));
+ }
+
@Test
public void testBuildConnectorPropertiesStorageDetection() throws Exception {
Map<String, String> s3Config =
