harishch1998 commented on code in PR #1931:
URL: https://github.com/apache/polaris/pull/1931#discussion_r2167389098


##########
service/common/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java:
##########
@@ -169,30 +171,51 @@ private void validateStorageConfig(StorageConfigInfo 
storageConfigInfo) {
     }
   }
 
-  private void validateConnectionConfigInfo(Catalog catalog) {
+  private void validateExternalCatalog(Catalog catalog) {
     if (catalog.getType() == Catalog.TypeEnum.EXTERNAL) {
       if (catalog instanceof ExternalCatalog externalCatalog) {
-        if (externalCatalog.getConnectionConfigInfo() != null) {
-          String connectionType =
-              
externalCatalog.getConnectionConfigInfo().getConnectionType().name();
-          List<String> supportedConnectionTypes =
-              callContext
-                  .getPolarisCallContext()
-                  .getConfigurationStore()
-                  .getConfiguration(
-                      callContext.getRealmContext(),
-                      FeatureConfiguration.SUPPORTED_CATALOG_CONNECTION_TYPES)
-                  .stream()
-                  .map(s -> s.toUpperCase(Locale.ROOT))
-                  .toList();
-          if (!supportedConnectionTypes.contains(connectionType)) {
-            throw new IllegalStateException("Unsupported connection type: " + 
connectionType);
-          }
+        ConnectionConfigInfo connectionConfigInfo = 
externalCatalog.getConnectionConfigInfo();
+        if (connectionConfigInfo != null) {
+          validateConnectionConfigInfo(connectionConfigInfo);
+          
validateAuthenticationParameters(connectionConfigInfo.getAuthenticationParameters());
         }
       }
     }
   }
 
+  private void validateConnectionConfigInfo(ConnectionConfigInfo 
connectionConfigInfo) {
+
+    String connectionType = connectionConfigInfo.getConnectionType().name();
+    List<String> supportedConnectionTypes =
+        callContext
+            .getPolarisCallContext()
+            .getConfigurationStore()
+            .getConfiguration(
+                callContext.getRealmContext(),
+                FeatureConfiguration.SUPPORTED_CATALOG_CONNECTION_TYPES)
+            .stream()
+            .map(s -> s.toUpperCase(Locale.ROOT))
+            .toList();
+    if (!supportedConnectionTypes.contains(connectionType)) {
+      throw new IllegalStateException("Unsupported connection type: " + 
connectionType);
+    }
+  }
+
+  private void validateAuthenticationParameters(AuthenticationParameters 
authenticationParameters) {
+
+    String authenticationType = 
authenticationParameters.getAuthenticationType().name();
+    List<String> supportedAuthenticationTypes =
+        callContext
+            .getPolarisCallContext()
+            .getConfigurationStore()
+            .getConfiguration(
+                callContext.getRealmContext(),
+                
FeatureConfiguration.SUPPORTED_FEDERATION_AUTHENTICATION_TYPES);

Review Comment:
   Do we need to handle uppercasing for this, similar to 
`supportedConnectionTypes` above? It might be redundant since those enums are 
uppercase anyway, so it's optional and upto you to make the change :p



-- 
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]

Reply via email to