Copilot commented on code in PR #13078:
URL: https://github.com/apache/gravitino/pull/13078#discussion_r3977516213
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/iceberg/IcebergCatalogPropertyConverter.java:
##########
@@ -248,6 +249,28 @@ private Map<String, String>
buildJDBCBackendProperties(Map<String, String> prope
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, and the Iceberg client can
only turn such a
+ * token into a usable credential through an OAuth2 token exchange. Under
any other security mode
+ * there is no token endpoint to exchange it at, so the mode carries no user
identity and merely
+ * makes Iceberg clients older than 1.9 fail every request; newer ones
ignore the token instead.
+ *
+ * <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, "USER");
Review Comment:
The OAuth2 detection is sensitive to leading/trailing whitespace in the
configured security value (e.g., `\"OAUTH2 \"`), which would incorrectly skip
enabling the per-user session mode. Consider normalizing the value (e.g., trim
before comparison) to make the behavior robust to common configuration
formatting issues.
##########
docs/trino-connector/catalog-iceberg.md:
##########
Review Comment:
The docs say the IRC is configured with `security=OAUTH2`, but the actual
property key appears to be `gravitino.iceberg.rest-catalog.security` (or the
mapped engine key). To avoid confusion/misconfiguration, consider naming the
full key here (consistent with the surrounding `gravitino.*` properties) or
explicitly stating that `security=OAUTH2` refers to
`gravitino.iceberg.rest-catalog.security=OAUTH2`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
