visit2rahul commented on code in PR #4422:
URL: https://github.com/apache/polaris/pull/4422#discussion_r3407014088


##########
runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java:
##########
@@ -915,11 +968,37 @@ private void validateUpdateCatalogDiffOrThrow(
         // so we must restore it explicitly in the updateBuilder.
         updateBuilder.setDefaultBaseLocation(defaultBaseLocation);
       } else {
-        // New base location is already in the updated properties; we'll also 
potentially
-        // plumb it into the logic for setting an updated 
StorageConfigurationInfo.
+        // New base location is already in the updated properties; capture the 
effective value
+        // and record whether it actually changed (used by the storage-config 
validation below).
         defaultBaseLocation = newDefaultBaseLocation;
+        baseChanging = 
!newDefaultBaseLocation.equals(currentCatalogEntity.getBaseLocation());
       }
     }
+
+    // Validate the effective defaultBaseLocation against the effective 
allowed-locations BEFORE
+    // the builder runs. The two flags say exactly what's being modified:
+    //   - storageConfigChanging: request has a new StorageConfigInfo. 
Validate against the
+    //     USER-SUBMITTED allowed list. 
CatalogEntity.Builder.setStorageConfigurationInfo silently
+    //     appends defaultBaseLocation to allowedLocations (see TODO in that 
method), so
+    //     validating post-build would be a no-op for this case.
+    //   - baseChanging: properties block changed the default-base-location to 
a non-empty value
+    //     different from the current. Validate against the current allowed 
list.
+    // Skip when neither flag fires, or when there is no storage config to 
validate against.
+    boolean storageConfigChanging = updateRequest.getStorageConfigInfo() != 
null;
+    if (defaultBaseLocation != null && (storageConfigChanging || 
baseChanging)) {

Review Comment:
   done, somehow missed this comment earlier! 



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