This is an automated email from the ASF dual-hosted git repository.
yuqi1129 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 afde98bf64 [Cherry-pick to branch-1.3] [#13096] fix(trino-connector):
Reuse service metadata when the user token is missing (#13097) (#13102)
afde98bf64 is described below
commit afde98bf641f3e9a66eda27a06d10e8ceba2040e
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Sat Sep 12 07:27:35 2026 +0800
[Cherry-pick to branch-1.3] [#13096] fix(trino-connector): Reuse service
metadata when the user token is missing (#13097) (#13102)
**Cherry-pick Information:**
- Original commit: 77eae699f6b436c3d733052d9fe1c683b4274c0c
- Target branch: `branch-1.3`
- Status: ✅ Clean cherry-pick (no conflicts)
Co-authored-by: Yuhui <[email protected]>
---
docs/trino-connector/authentication.md | 34 +++----
.../trino/connector/GravitinoConnector.java | 6 ++
.../TestGravitinoConnectorForwardUser.java | 102 ++++++++++++++++++++-
3 files changed, 125 insertions(+), 17 deletions(-)
diff --git a/docs/trino-connector/authentication.md
b/docs/trino-connector/authentication.md
index e7166b3542..d95c926c1b 100644
--- a/docs/trino-connector/authentication.md
+++ b/docs/trino-connector/authentication.md
@@ -60,8 +60,8 @@ gravitino.client.basic.password=YourSecureGravitinoPassword
| Property | Description
| Default value | Required |
|-----------------------------------|-----------------------------------------------------------------|---------------|----------------------------|
| `gravitino.client.authType` | Authentication type: `simple`, `basic`,
`oauth2`, or `kerberos` | (none) | Yes (to enable Basic) |
-| `gravitino.client.basic.username` | Local user store username
| (none) | Yes if authType is `basic` |
-| `gravitino.client.basic.password` | Local user store password
| (none) | Yes if authType is `basic` |
+| `gravitino.client.basic.username` | Local user store username
| (none) | Yes if authType is `basic` |
+| `gravitino.client.basic.password` | Local user store password
| (none) | Yes if authType is `basic` |
### OAuth2 Authentication
@@ -154,7 +154,7 @@
gravitino.client.kerberos.keytabFilePath=/path/to/user.keytab
## Session Credential Forwarding
-Setting `gravitino.client.session.forwardUser=true` creates a dedicated
Gravitino client per Trino session user, so each user is visible in the
Gravitino audit log instead of the shared `gravitino.user` or service identity.
It is supported with `authType=simple` and `authType=oauth2`.
+Setting `gravitino.client.session.forwardUser=true` creates a dedicated
Gravitino client per Trino session user, so each user is visible in the
Gravitino audit log instead of the shared `gravitino.user` or service identity.
It is supported with `authType=simple` and `authType=oauth2`. For OAuth2
sessions without a forwarded token, the connector reuses the shared service
metadata instead.
**Configuration (`authType=simple`):**
@@ -184,7 +184,7 @@ gravitino.client.oauth2.scope=gravitino
gravitino.client.session.forwardUser=true
```
-With `authType=oauth2`, the end user's IdP access token is presented to
Gravitino directly instead of the shared client-credentials identity. This
requires the Trino coordinator to populate the session's extra-credentials with
the caller's access token under the key `token`; the connector reads it from
there, and `buildForSession` fails with a clear error if it's missing.
+With `authType=oauth2`, the end user's IdP access token is presented to
Gravitino directly when the Trino coordinator populates the session's
extra-credentials with the caller's access token under the key `token` (or the
configured `gravitino.client.session.userTokenCredentialKey`). If that
credential is absent, empty, or whitespace-only, the connector reuses the
shared service metadata. This allows password-authenticated sessions, including
internal catalog-management JDBC sessions, to [...]
Whether the coordinator can populate this extra-credential depends on the
Trino distribution:
@@ -215,11 +215,11 @@ which the connector does not re-route), the connector
does not set `iceberg.rest
**Configuration properties:**
-| Property | Description
| Default value | Required | Since version |
-|--------------------------------------------------------------|--------------------------------------------------------------------------------------------------|-----------------|------------|-----------------|
-| `gravitino.client.session.forwardUser` | When `true`
with `authType=simple` or `authType=oauth2`, forwards the Trino session
user/token to Gravitino per-query | `false` | No | 1.3.0
|
-| `gravitino.client.session.cache.maxSize` | Maximum
number of per-user sessions to keep in the cache
| `500` | No | 1.3.0 |
-| `gravitino.client.session.cache.expireAfterAccessSeconds` | Seconds
before an idle per-user session is evicted from the cache
| `3600` | No | 1.3.0 |
+| Property | Description
| Default value | Required | Since version |
+|-----------------------------------------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|---------------|----------|---------------|
+| `gravitino.client.session.forwardUser` | When `true` with
`authType=simple` or `authType=oauth2`, forwards the Trino session user/token
to Gravitino per-query; OAuth2 sessions without a token use the shared service
metadata | `false` | No | 1.3.0 |
+| `gravitino.client.session.cache.maxSize` | Maximum number
of per-user sessions to keep in the cache
| `500` | No | 1.3.0 |
+| `gravitino.client.session.cache.expireAfterAccessSeconds` | Seconds before
an idle per-user session is evicted from the cache
| `3600` | No | 1.3.0 |
### Example: OAuth2 Per-User Token Forwarding
@@ -244,8 +244,8 @@ populate the session's extra-credentials with the caller's
access token under th
- **Open-source Trino**: there is currently no equivalent coordinator setting.
Track
[trinodb/trino discussion
#24403](https://github.com/trinodb/trino/discussions/24403) and
[issue #27917](https://github.com/trinodb/trino/issues/27917) for this
feature request. Until
- it lands upstream, this connector's `authType=oauth2` forwardUser path
requires a Trino
- distribution that provides this extra-credential itself.
+ it lands upstream, forwarding OAuth2 user tokens requires a Trino
distribution that
+ provides this extra-credential itself. Sessions without it use the shared
service metadata.
**2. Gravitino server: enable OAuth2** (in `conf/gravitino.conf`):
@@ -273,8 +273,9 @@ gravitino.client.session.forwardUser=true
```
The `gravitino.client.oauth2.*` properties configure the shared service
identity used for catalog
-discovery; the per-user forwarded token (from step 1) is what each query
actually authenticates
-with once `forwardUser=true`.
+discovery and for metadata access by sessions without a forwarded token. With
+`forwardUser=true`, sessions carrying a token from step 1 authenticate
metadata requests with
+that token instead.
**4. Create the metalake and catalog.** Create the metalake `my_metalake`
first (via the
Gravitino REST API, SDK, or CLI — see
@@ -304,9 +305,10 @@ call gravitino.system.create_catalog(
);
```
-This call itself runs with the connector's own shared service identity, not
any forwarded user
-token — `forwardUser` only affects `SELECT`/`SHOW`-style queries against the
catalog afterward,
-not catalog registration itself. `create_catalog` both creates the catalog in
Gravitino and loads
+The procedure uses the connector's shared service client to create the catalog
in Gravitino.
+Catalog registration can also invoke the connector's metadata entry point; an
internal
+password-authenticated JDBC session without a forwarded token uses the shared
service metadata
+there. `create_catalog` both creates the catalog in Gravitino and loads
it into Trino as its own top-level catalog — not as a schema nested under a
single `gravitino`
catalog. If the two `trino.bypass.iceberg.rest-catalog.*` properties above are
omitted, the REST
catalog keeps its own default security setting, independent of
diff --git
a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConnector.java
b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConnector.java
index 66f7b78036..57dc9913c6 100644
---
a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConnector.java
+++
b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConnector.java
@@ -242,6 +242,12 @@ public class GravitinoConnector implements Connector {
GravitinoAuthProvider.USER_TOKEN_CREDENTIAL_KEY,
GravitinoAuthProvider.DEFAULT_USER_TOKEN_CREDENTIAL_KEY);
String token =
session.getIdentity().getExtraCredentials().get(credentialKey);
+ // Password-authenticated sessions have no OAuth token. Reuse the
configured service
+ // identity in that case; failures with a supplied token must still
propagate.
+ if (GravitinoAuthProvider.parseAuthType(authType) ==
GravitinoAuthProvider.AuthType.OAUTH2
+ && StringUtils.isBlank(token)) {
+ return connectorMetadata;
+ }
String credKey = sessionCacheKey(authType, session.getUser(), token);
try {
return perUserSessionCache.get(
diff --git
a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoConnectorForwardUser.java
b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoConnectorForwardUser.java
index 29b91f8ca3..82e73441e2 100644
---
a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoConnectorForwardUser.java
+++
b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoConnectorForwardUser.java
@@ -28,12 +28,18 @@ import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import com.google.common.collect.ImmutableMap;
import io.trino.spi.TrinoException;
+import io.trino.spi.connector.Connector;
+import io.trino.spi.connector.ConnectorMetadata;
import io.trino.spi.connector.ConnectorSession;
+import io.trino.spi.connector.ConnectorTransactionHandle;
import io.trino.spi.security.ConnectorIdentity;
+import java.util.List;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.function.Function;
import org.apache.gravitino.Catalog;
@@ -44,6 +50,7 @@ import org.apache.gravitino.client.GravitinoMetalake;
import org.apache.gravitino.rel.TableCatalog;
import org.apache.gravitino.trino.connector.catalog.CatalogConnectorContext;
import org.apache.gravitino.trino.connector.catalog.CatalogConnectorMetadata;
+import
org.apache.gravitino.trino.connector.catalog.CatalogConnectorMetadataAdapter;
import org.apache.gravitino.trino.connector.metadata.GravitinoCatalog;
import org.apache.gravitino.trino.connector.security.GravitinoAuthProvider;
import org.junit.jupiter.api.Test;
@@ -51,6 +58,98 @@ import org.junit.jupiter.api.Test;
/** Tests for forwardUser startup validation in {@link GravitinoConnector}. */
class TestGravitinoConnectorForwardUser {
+ @Test
+ void testPasswordSessionUsesServiceMetadataForCatalogLifecycleAndQueries() {
+ CatalogConnectorContext ctx =
+ mockContextWithConfig(
+ ImmutableMap.of(
+ GravitinoAuthProvider.FORWARD_SESSION_USER_KEY, "true",
+ GravitinoAuthProvider.AUTH_TYPE_KEY, "oauth2"));
+ Connector internal = mock(Connector.class);
+ ConnectorMetadata internalMetadata = mock(ConnectorMetadata.class);
+ when(ctx.getInternalConnector()).thenReturn(internal);
+ when(internal.getMetadata(any(), any())).thenReturn(internalMetadata);
+ GravitinoConnector connector =
+ new GravitinoConnector(ctx) {
+ @Override
+ protected GravitinoMetadata createGravitinoMetadata(
+ CatalogConnectorMetadata metadata,
+ CatalogConnectorMetadataAdapter adapter,
+ ConnectorMetadata delegate) {
+ return new GravitinoMetadata(metadata, adapter, delegate) {};
+ }
+ };
+ ConnectorSession session = mockSession("gravitino_catalog_manager", "");
+ GravitinoTransactionHandle transaction =
+ new GravitinoTransactionHandle(mock(ConnectorTransactionHandle.class));
+ ConnectorMetadata metadata =
+ assertDoesNotThrow(() -> connector.getMetadata(session, transaction));
+ assertDoesNotThrow(() -> metadata.beginQuery(session));
+ assertDoesNotThrow(() -> metadata.cleanupQuery(session));
+ verify(internalMetadata).beginQuery(session);
+ verify(internalMetadata).cleanupQuery(session);
+ SupportsSchemas schemas =
ctx.getMetalake().loadCatalog("catalog").asSchemas();
+ when(schemas.listSchemas()).thenReturn(new String[] {"test_schema"});
+ assertEquals(List.of("test_schema"), metadata.listSchemaNames(session));
+ verify(schemas).listSchemas();
+ verify(internalMetadata, never()).listSchemaNames(any());
+ }
+
+ @Test
+ void testMissingOAuthTokenReusesServiceMetadataWithoutBuildingUserClient() {
+ CatalogConnectorContext ctx =
+ mockContextWithConfig(
+ ImmutableMap.of(
+ GravitinoAuthProvider.FORWARD_SESSION_USER_KEY, "true",
+ GravitinoAuthProvider.AUTH_TYPE_KEY, "oauth2"));
+ GravitinoConnector connector =
+ newConnectorWithAuthClient(
+ ctx,
+ session -> {
+ throw new AssertionError("A missing token must not create a user
client");
+ });
+ CatalogConnectorMetadata service =
+ connector.resolveSessionMetadata(mockSession("manager", null));
+ assertSame(service, connector.resolveSessionMetadata(mockSession("alice",
"")));
+ assertSame(service, connector.resolveSessionMetadata(mockSession("bob", "
")));
+ }
+
+ @Test
+ void testCustomTokenKeyControlsForwardingAndFallback() {
+ CatalogConnectorContext ctx =
+ mockContextWithConfig(
+ ImmutableMap.of(
+ GravitinoAuthProvider.FORWARD_SESSION_USER_KEY, "true",
+ GravitinoAuthProvider.AUTH_TYPE_KEY, "oauth2",
+ GravitinoAuthProvider.USER_TOKEN_CREDENTIAL_KEY,
"custom-token"));
+ AtomicInteger count = new AtomicInteger();
+ GravitinoConnector connector =
+ newConnectorWithAuthClient(ctx, session ->
mockAdminClient(ctx.getMetalake(), count));
+ ConnectorSession session = mockSession("alice", "ignored-default-token");
+ CatalogConnectorMetadata service =
connector.resolveSessionMetadata(session);
+ assertEquals(0, count.get());
+ when(session.getIdentity().getExtraCredentials())
+ .thenReturn(ImmutableMap.of("custom-token", "user-token"));
+ assertNotSame(service, connector.resolveSessionMetadata(session));
+ assertEquals(1, count.get());
+ }
+
+ @Test
+ void testSimpleAuthStillForwardsUsernameWithoutToken() {
+ CatalogConnectorContext ctx =
+ mockContextWithConfig(
+ ImmutableMap.of(
+ GravitinoAuthProvider.FORWARD_SESSION_USER_KEY, "true",
+ GravitinoAuthProvider.AUTH_TYPE_KEY, "simple"));
+ AtomicInteger count = new AtomicInteger();
+ GravitinoConnector connector =
+ newConnectorWithAuthClient(ctx, session ->
mockAdminClient(ctx.getMetalake(), count));
+ assertNotSame(
+ connector.resolveSessionMetadata(mockSession("alice", null)),
+ connector.resolveSessionMetadata(mockSession("bob", null)));
+ assertEquals(2, count.get());
+ }
+
@Test
void testForwardUserWithoutAuthTypeThrowsAtConstruction() {
CatalogConnectorContext ctx =
@@ -249,7 +348,8 @@ class TestGravitinoConnectorForwardUser {
private static ConnectorSession mockSession(String user, String token) {
ConnectorIdentity identity = mock(ConnectorIdentity.class);
- when(identity.getExtraCredentials()).thenReturn(ImmutableMap.of("token",
token));
+ when(identity.getExtraCredentials())
+ .thenReturn(token == null ? ImmutableMap.of() :
ImmutableMap.of("token", token));
ConnectorSession session = mock(ConnectorSession.class);
when(session.getUser()).thenReturn(user);
when(session.getIdentity()).thenReturn(identity);
