visit2rahul commented on code in PR #4422:
URL: https://github.com/apache/polaris/pull/4422#discussion_r3406982653
##########
runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java:
##########
@@ -911,11 +911,33 @@ 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.
defaultBaseLocation = newDefaultBaseLocation;
}
}
+
+ // Base-location validity is an invariant on every update. Update-side
semantics are strict:
+ // unlike CatalogEntity.Builder.setStorageConfigurationInfo (which has a
create-time
+ // convenience that auto-populates allowed-locations from
default-base-location when the
+ // caller supplied none), updates that touch storage_config must declare
allowed-locations
+ // explicitly. This prevents silent loss of an existing allowed-locations
list during a
+ // partial storage_config update (e.g. role rotation).
+ if (defaultBaseLocation != null && updateRequest.getStorageConfigInfo() !=
null) {
+ List<String> submittedAllowedLocations =
+ updateRequest.getStorageConfigInfo().getAllowedLocations();
+ if (submittedAllowedLocations == null ||
submittedAllowedLocations.isEmpty()) {
+ throw new BadRequestException(
+ "Cannot update Catalog %s: when updating storage_config,
allowed-locations must"
+ + " be specified explicitly (omitting it would silently lose
the catalog's"
+ + " existing allowed-locations list)",
+ name);
+ }
+ // default-base-location/allowed-locations consistency is enforced inside
+ // Builder.setStorageConfigurationInfo() below.
+ }
+ if (defaultBaseLocation != null && updateRequest.getStorageConfigInfo() ==
null) {
+ updateBuilder.validateDefaultBaseLocation();
Review Comment:
yes, i think that'll make it more resilient and deterministic. Moving 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]
