XJDKC commented on code in PR #2782:
URL: https://github.com/apache/polaris/pull/2782#discussion_r2426858961
##########
runtime/service/src/main/java/org/apache/polaris/service/config/ProductionReadinessChecks.java:
##########
@@ -335,4 +337,64 @@ public ProductionReadinessCheck
checkOverlappingSiblingCheckSettings(
? ProductionReadinessCheck.OK
: ProductionReadinessCheck.of(errors.toArray(new Error[0]));
}
+
+ @Produces
+ @SuppressWarnings("unchecked")
+ public ProductionReadinessCheck checkConnectionCredentialVendors(
+ Instance<ConnectionCredentialVendor> credentialVendors,
+ FeaturesConfiguration featureConfiguration) {
+ var mapper = new ObjectMapper();
+ var defaults = featureConfiguration.parseDefaults(mapper);
+ var realmOverrides = featureConfiguration.parseRealmOverrides(mapper);
+
+ var federationKey = FeatureConfiguration.ENABLE_CATALOG_FEDERATION.key();
+ var authTypesKey =
FeatureConfiguration.SUPPORTED_EXTERNAL_CATALOG_AUTHENTICATION_TYPES.key();
+ var federationEnabled =
+ Boolean.parseBoolean(defaults.getOrDefault(federationKey,
false).toString());
+ var defaultAuthTypes = (List<String>) defaults.getOrDefault(authTypesKey,
List.of());
+
+ var allAuthTypes = new java.util.HashSet<String>();
+ if (federationEnabled) allAuthTypes.addAll(defaultAuthTypes);
+
+ realmOverrides.forEach(
+ (id, overrides) -> {
+ if (Boolean.parseBoolean(
+ overrides.getOrDefault(federationKey,
federationEnabled).toString())) {
+ allAuthTypes.addAll(
+ (List<String>)
+ (overrides.containsKey(authTypesKey)
+ ? overrides.get(authTypesKey)
+ : defaultAuthTypes));
+ }
+ });
+
+ var errors = new ArrayList<Error>();
+ for (var name : allAuthTypes) {
+ try {
+ var type =
org.apache.polaris.core.connection.AuthenticationType.valueOf(name);
+ if
(credentialVendors.select(AuthType.Literal.of(type)).isUnsatisfied()) {
+ errors.add(
+ Error.of(
+ format(
+ "Catalog federation is enabled but no
ConnectionCredentialVendor found for "
+ + "authentication type '%s'. External catalog
connections using this "
+ + "authentication type will fail.",
+ type),
Review Comment:
I see, I will switch to `severe`.
> A particular deployment only needs to provide ConnectionCredentialVendor
implementations for auth types that are actually used by created catalogs. I
think we only need to check types for existing catalogs, not for all types
listed in the enum
The logic is: if we enable the catalog federation and all the enabled auth
types must have a corresponding `ConnectionCredentialVendor`. IIRC, we don't
want to defer the check during the runtime based on the feedback in the
previous PR.
If vendors only implement one / two auth types for catalog federation, they
need to only enable them, right?
--
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]
