This is an automated email from the ASF dual-hosted git repository. diqiu50 pushed a commit to branch trino-irc-1.3 in repository https://gitbox.apache.org/repos/asf/gravitino.git
commit aeec51586d0ae211cd9c8c6ddd1b5772d87ab35a Author: yuhui <[email protected]> AuthorDate: Mon Aug 24 15:29:27 2026 +0000 [Cherry-pick to branch-1.3] fix(trino): secure dynamic catalog credentials (cherry picked from commit 6032d8d368901e9ca3d02cf741e9fee535f59a9b) --- .../web/rest/IcebergRESTServiceOperations.java | 1 + .../web/rest/TestIcebergRESTServiceOperations.java | 11 +++++++ .../gravitino/trino/connector/GravitinoConfig.java | 35 ++++++++++++++++++++-- .../trino/connector/GravitinoConnectorFactory.java | 3 ++ .../trino/connector/catalog/CatalogRegister.java | 4 +-- .../trino/connector/TestGravitinoConfig.java | 25 ++++++++++++++++ .../connector/TestGravitinoConnectorFactory.java | 3 ++ 7 files changed, 77 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/apache/gravitino/server/web/rest/IcebergRESTServiceOperations.java b/server/src/main/java/org/apache/gravitino/server/web/rest/IcebergRESTServiceOperations.java index ce672875e8..fb58da651d 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/rest/IcebergRESTServiceOperations.java +++ b/server/src/main/java/org/apache/gravitino/server/web/rest/IcebergRESTServiceOperations.java @@ -86,6 +86,7 @@ public class IcebergRESTServiceOperations { * running, or does not serve the requested metalake */ @GET + @Produces("application/vnd.gravitino.v1+json") @Timed(name = "iceberg-rest-service." + MetricNames.HTTP_PROCESS_DURATION, absolute = true) @ResponseMetered(name = "iceberg-rest-service", absolute = true) public Response getIcebergRestServiceUri(@QueryParam("metalake") String metalake) { diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestIcebergRESTServiceOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestIcebergRESTServiceOperations.java index 5a559bf059..71b895f08b 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestIcebergRESTServiceOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestIcebergRESTServiceOperations.java @@ -26,6 +26,7 @@ import static org.mockito.Mockito.when; import com.google.common.collect.ImmutableMap; import java.util.Map; import javax.servlet.http.HttpServletRequest; +import javax.ws.rs.Produces; import javax.ws.rs.core.Response; import org.apache.gravitino.auxiliary.AuxiliaryServiceManager; import org.apache.gravitino.dto.responses.IcebergRESTServiceResponse; @@ -35,6 +36,16 @@ public class TestIcebergRESTServiceOperations { private static final String DYNAMIC_PROVIDER = "dynamic-config-provider"; + @Test + public void testDiscoveryEndpointProducesVersionedJson() throws Exception { + Produces produces = + IcebergRESTServiceOperations.class + .getMethod("getIcebergRestServiceUri", String.class) + .getAnnotation(Produces.class); + + assertEquals("application/vnd.gravitino.v1+json", produces.value()[0]); + } + private static Map<String, String> withDynamicProvider(Map<String, String> extra) { return ImmutableMap.<String, String>builder() .put("catalog-config-provider", DYNAMIC_PROVIDER) 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 5cdeba701e..cb9d71a3ff 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 @@ -80,6 +80,13 @@ public class GravitinoConfig { /** The Trino Iceberg REST catalog property prefix. */ private static final String TRINO_ICEBERG_REST_CATALOG_PREFIX = "iceberg.rest-catalog."; + /** Prefix for environment-variable references propagated to dynamic catalogs. */ + static final String GRAVITINO_DYNAMIC_CATALOG_ENV_PREFIX = + "gravitino.dynamic-catalog.environment-variable."; + + private static final Pattern ENVIRONMENT_VARIABLE_NAME = + Pattern.compile("[A-Za-z_][A-Za-z0-9_]*"); + private static final Map<String, ConfigEntry> CONFIG_DEFINITIONS = new HashMap<>(); private final Map<String, String> config; private final List<Pattern> skipCatalogPatternList; @@ -622,7 +629,8 @@ public class GravitinoConfig { continue; } String value = config.get(entry.getKey()); - if (value != null) { + if (value != null + && !GravitinoConnectorFactory.isSecuritySensitivePropertyName(entry.getKey())) { stringList.add(String.format("\"%s\"='%s'", entry.getKey(), value)); } } @@ -631,11 +639,32 @@ public class GravitinoConfig { config.entrySet().stream() .filter( entry -> - entry.getKey().startsWith(GRAVITINO_CLIENT_CONFIG_PREFIX.key) - || entry.getKey().startsWith(GRAVITINO_ICEBERG_REST_CATALOG_CONFIG_PREFIX.key)) + (entry.getKey().startsWith(GRAVITINO_CLIENT_CONFIG_PREFIX.key) + || entry + .getKey() + .startsWith(GRAVITINO_ICEBERG_REST_CATALOG_CONFIG_PREFIX.key)) + && !GravitinoConnectorFactory.isSecuritySensitivePropertyName(entry.getKey())) .forEach( entry -> stringList.add(String.format("\"%s\"='%s'", entry.getKey(), entry.getValue()))); + config.entrySet().stream() + .filter(entry -> entry.getKey().startsWith(GRAVITINO_DYNAMIC_CATALOG_ENV_PREFIX)) + .forEach( + entry -> { + String propertyName = + entry.getKey().substring(GRAVITINO_DYNAMIC_CATALOG_ENV_PREFIX.length()); + String environmentVariable = entry.getValue(); + if (propertyName.isEmpty() + || !ENVIRONMENT_VARIABLE_NAME.matcher(environmentVariable).matches()) { + throw new TrinoException( + GravitinoErrorCode.GRAVITINO_ILLEGAL_ARGUMENT, + String.format( + "Invalid dynamic catalog environment-variable mapping '%s'='%s'", + entry.getKey(), environmentVariable)); + } + stringList.add( + String.format("\"%s\"='${ENV:%s}'", propertyName, environmentVariable)); + }); return StringUtils.join(stringList, ','); } diff --git a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConnectorFactory.java b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConnectorFactory.java index b248ed7f82..a748c0ab58 100644 --- a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConnectorFactory.java +++ b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConnectorFactory.java @@ -245,6 +245,9 @@ public class GravitinoConnectorFactory implements ConnectorFactory { @VisibleForTesting static boolean isSecuritySensitivePropertyName(String propertyName) { + if (propertyName.startsWith(GravitinoConfig.GRAVITINO_DYNAMIC_CATALOG_ENV_PREFIX)) { + return false; + } String normalizedPropertyName = propertyName.toLowerCase(Locale.ROOT).replaceAll("[._-]", ""); return SECURITY_SENSITIVE_PROPERTY_SUFFIXES.stream().anyMatch(normalizedPropertyName::endsWith); } diff --git a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/CatalogRegister.java b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/CatalogRegister.java index fd971c3bce..11b183fd5d 100644 --- a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/CatalogRegister.java +++ b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/CatalogRegister.java @@ -472,14 +472,14 @@ public class CatalogRegister { throw e; } catch (Exception e) { failedException = e; - LOG.warn("Failed to execute command: {}", sql, e); + LOG.warn("Failed to execute command: {}", redactSecrets(sql), e); Thread.sleep(EXECUTE_QUERY_BACKOFF_TIME_SECOND * 1000); } } throw failedException; } catch (Exception e) { throw new TrinoException( - GravitinoErrorCode.GRAVITINO_RUNTIME_ERROR, "Failed to execute query: " + sql, e); + GravitinoErrorCode.GRAVITINO_RUNTIME_ERROR, "Failed to execute query", e); } } 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 d2792d75df..1f67109deb 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 @@ -390,6 +390,31 @@ public class TestGravitinoConfig { assertTrue(catalogConfig.contains("\"gravitino.iceberg.rest-catalog.security\"='OAUTH2'")); } + @Test + public void testToCatalogConfigPropagatesSecretReferencesInsteadOfSecretValues() { + GravitinoConfig config = + new GravitinoConfig( + ImmutableMap.of( + "gravitino.metalake", "user_001", + "gravitino.client.oauth2.credential", "client:management-secret", + "gravitino.iceberg.rest-catalog.oauth2.credential", "client:irc-secret", + "gravitino.dynamic-catalog.environment-variable.gravitino.client.oauth2.credential", + "GRAVITINO_CLIENT_CREDENTIAL", + "gravitino.dynamic-catalog.environment-variable.gravitino.iceberg.rest-catalog.oauth2.credential", + "IRC_CLIENT_CREDENTIAL")); + + String catalogConfig = config.toCatalogConfig(); + + assertFalse(catalogConfig.contains("management-secret")); + assertFalse(catalogConfig.contains("irc-secret")); + assertTrue( + catalogConfig.contains( + "\"gravitino.client.oauth2.credential\"='${ENV:GRAVITINO_CLIENT_CREDENTIAL}'")); + assertTrue( + catalogConfig.contains( + "\"gravitino.iceberg.rest-catalog.oauth2.credential\"='${ENV:IRC_CLIENT_CREDENTIAL}'")); + } + private static boolean skipCatalog(String catalogName, GravitinoConfig config) { for (Pattern pattern : config.getSkipCatalogPatterns()) { if (pattern.matcher(catalogName).matches()) { diff --git a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoConnectorFactory.java b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoConnectorFactory.java index 0b0c71c6b6..7c30ced137 100644 --- a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoConnectorFactory.java +++ b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoConnectorFactory.java @@ -39,6 +39,9 @@ class TestGravitinoConnectorFactory { Map.entry("hive.s3.aws-secret-key", "secret-key"), Map.entry("authentication.token", "token"), Map.entry("oauth2.credential", "credential"), + Map.entry( + "gravitino.dynamic-catalog.environment-variable.oauth2.credential", + "OAUTH_CREDENTIAL"), Map.entry("tls.private_key", "private-key"), Map.entry("gravitino.uri", "uri"));
