Copilot commented on code in PR #11853:
URL: https://github.com/apache/gravitino/pull/11853#discussion_r3544449543
##########
iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/provider/DynamicIcebergConfigProvider.java:
##########
@@ -104,38 +110,71 @@ public Optional<IcebergConfig>
getIcebergCatalogConfig(String catalogName) {
"lakehouse-iceberg".equals(catalog.provider()),
String.format("%s.%s is not iceberg catalog", gravitinoMetalake,
catalogName));
- // Sensitive credentials (e.g. jdbc-password) are marked hidden in
PropertiesMetadata and
- // filtered out of catalog.properties(). We need two different strategies
to recover them:
+ // Sensitive credentials are marked hidden in PropertiesMetadata and
filtered out of
+ // catalog.properties(). We need two different strategies to recover them:
//
// Auxiliary mode: the catalog is a BaseCatalog running in the same JVM as
the Gravitino
// server. Call propertiesWithCredentialProviders() which returns the raw
entity properties
- // including all hidden fields.
+ // including all hidden fields, then generate credentials locally.
//
// Standalone mode: the catalog is a client-side object obtained via the
Gravitino REST API.
- // Call getCredentials() to retrieve vended credentials, then inject any
JdbcCredential
- // fields into the properties map so the JDBC backend can connect.
+ // Call supportsCredentials().getCredentials() to retrieve vended
credentials.
Map<String, String> catalogProperties;
if (catalog instanceof BaseCatalog) {
catalogProperties = ((BaseCatalog<?>)
catalog).propertiesWithCredentialProviders();
} else {
catalogProperties = new HashMap<>(catalog.properties());
- if (catalog instanceof SupportsCredentials) {
- Arrays.stream(((SupportsCredentials) catalog).getCredentials())
- .filter(c -> c instanceof JdbcCredential)
- .map(c -> (JdbcCredential) c)
- .findFirst()
- .ifPresent(
- jdbc -> {
- catalogProperties.putIfAbsent(
- IcebergConstants.GRAVITINO_JDBC_USER, jdbc.jdbcUser());
- catalogProperties.putIfAbsent(
- IcebergConstants.GRAVITINO_JDBC_PASSWORD,
jdbc.jdbcPassword());
- });
- }
}
+ Credential[] credentials = getCatalogCredentials(catalog,
catalogProperties);
+ Map<String, String> credentialProviderProperties =
+ IcebergServerCredentialUtils.hasRefreshableAwsCredential(credentials)
+ ? getCredentialProviderProperties(catalog, catalogProperties,
catalogName)
+ : null;
+ IcebergServerCredentialUtils.applyCredentials(
+ catalogName, credentials, credentialProviderProperties,
catalogProperties);
return
Optional.of(getIcebergConfigFromCatalogProperties(catalogProperties));
}
+ private Map<String, String> getCredentialProviderProperties(
+ Catalog catalog, Map<String, String> catalogProperties, String
catalogName) {
+ if (catalog instanceof BaseCatalog) {
+ return catalogProperties;
+ }
+
+ Map<String, String> providerProperties = new HashMap<>(properties);
+ providerProperties.put(
+ GravitinoIcebergAwsCredentialsProvider.SOURCE,
+ GravitinoIcebergAwsCredentialsProvider.SOURCE_REMOTE);
+ providerProperties.put(IcebergConstants.GRAVITINO_URI, getGravitinoUri());
+ providerProperties.put(IcebergConstants.GRAVITINO_METALAKE,
gravitinoMetalake);
+
providerProperties.put(GravitinoIcebergAwsCredentialsProvider.CATALOG_NAME,
catalogName);
+ return providerProperties;
Review Comment:
In remote mode, `getCredentialProviderProperties` copies the entire REST
server `properties` map into `client.credentials-provider.*` via
`IcebergServerCredentialUtils.applyCredentials`. This propagates unrelated
settings (and potentially sensitive values) into Iceberg catalog properties,
increasing the chance they get logged or exposed. Prefer building a minimal
provider-properties map that includes only the keys needed by
`GravitinoIcebergAwsCredentialsProvider` (auth-type + auth params) plus the
required Gravitino URI/metalake/catalog/source fields.
--
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]
