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


##########
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);
+    if (!supportedAuthenticationTypes.contains(authenticationType)) {
+      throw new IllegalStateException("Unsupported authentication type: " + 
authenticationType);

Review Comment:
   Done. Since there were no existiing tests for the PolarisServiceImpl, I 
added some setup code to mock the service/context needed by the 
PolarisServiceImpl; this is not necessary for the test but used by the service. 
Let me know if you have suggestions to avoid it. 



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