dimas-b commented on code in PR #4422:
URL: https://github.com/apache/polaris/pull/4422#discussion_r3283155967
##########
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:
TBH, I'm not sure we need this `if` at all. Base location validity should be
an invariant. It should hold in all update cases.
The only iffy part is where the get the list of allowed locations.
--
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]
