diqiu50 commented on code in PR #11264:
URL: https://github.com/apache/gravitino/pull/11264#discussion_r3379289602
##########
core/src/main/java/org/apache/gravitino/connector/BaseCatalog.java:
##########
@@ -451,30 +458,62 @@ public Map<String, String> properties() {
}
}
}
- return properties;
+ if (!shouldBackfillCredential()) {
+ return properties;
+ }
+ Map<String, String> result = Maps.newHashMap(properties);
+ result.putAll(propertiesWithCredentialProviders());
+ return result;
}
/**
- * Retrieves the properties of the catalog including credential providers.
Subclasses should
- * override this method to inject auto-detected credential provider names
into the properties map
- * before the {@link CatalogCredentialManager} is initialized. The default
implementation returns
- * {@link #properties()} unchanged.
+ * Retrieves the properties of the catalog including credential providers.
Detects storage and
+ * catalog-specific credential providers from the raw entity properties
(including hidden ones)
+ * and injects them before {@link CatalogCredentialManager} is initialized.
Subclasses may
+ * override {@link #addCatalogSpecificCredentialProviders} to add additional
providers.
*
- * @return A map of properties including credential providers.
+ * @return A map of raw properties with credential providers set.
*/
public Map<String, String> propertiesWithCredentialProviders() {
- return properties();
+ Map<String, String> props = Maps.newHashMap(entity().getProperties());
+ if
(StringUtils.isNotBlank(props.get(CredentialConstants.CREDENTIAL_PROVIDERS))) {
+ return props;
+ }
+ List<String> credentialProviders = new ArrayList<>();
+ addCatalogSpecificCredentialProviders(props, credentialProviders);
+ if (!credentialProviders.isEmpty()) {
+ props.put(CredentialConstants.CREDENTIAL_PROVIDERS, String.join(",",
credentialProviders));
+ }
+ return props;
}
/**
- * Detects storage credential providers (S3, OSS, Azure) from catalog
properties and appends them
- * to the provided list. Subclasses can call this method in their {@link
- * #propertiesWithCredentialProviders()} implementation to avoid duplicating
storage credential
- * detection logic.
+ * Detects credential providers for this catalog type and appends them to
{@code
+ * credentialProviders}. The default implementation calls {@link
+ * #addStorageCredentialProviders(Map, List)} to detect S3/OSS/Azure/GCS
credentials. Subclasses
+ * override this to add catalog-specific providers (e.g., JDBC).
*
- * @param properties The catalog properties map to scan for storage
credentials.
- * @param credentialProviders The list to append detected storage credential
providers to.
+ * @param properties the raw catalog properties
+ * @param credentialProviders the list to append detected provider names to
*/
+ protected void addCatalogSpecificCredentialProviders(
+ Map<String, String> properties, List<String> credentialProviders) {
+ addStorageCredentialProviders(properties, credentialProviders);
+ }
+
+ /**
+ * Returns whether hidden credentials should be backfilled into catalog
properties for backward
+ * compatibility with connectors that do not support credential vending.
Controlled by
+ * server-level config {@code
gravitino.catalog.credential.backfillToProperties}.
+ *
+ * @return true if backfill is enabled
+ */
+ protected boolean shouldBackfillCredential() {
+ Config serverConfig = GravitinoEnv.getInstance().config();
Review Comment:
`serverConfig` should not null
--
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]
