diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java index f7545bfc50..123ab99435 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/LocalIcebergCatalog.java @@ -877,7 +877,13 @@ public boolean setProperties(Namespace namespace, Map properties PolarisEntity updatedEntity = new PolarisEntity.Builder(entity).setProperties(newProperties).build(); - if (!realmConfig.getConfig(FeatureConfiguration.ALLOW_NAMESPACE_LOCATION_OVERLAP)) { + boolean locationChanged = + !Objects.equal( + NamespaceEntity.of(entity).getBaseLocation(), + NamespaceEntity.of(updatedEntity).getBaseLocation()); + + if (locationChanged + && !realmConfig.getConfig(FeatureConfiguration.ALLOW_NAMESPACE_LOCATION_OVERLAP)) { LOGGER.debug("Validating no overlap with sibling tables or namespaces"); validateNoLocationOverlap( NamespaceEntity.of(updatedEntity), resolvedEntities.getRawParentPath()); diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/IcebergAllowedLocationTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/IcebergAllowedLocationTest.java index 66b2ecbbc6..aef8b9eb75 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/IcebergAllowedLocationTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/IcebergAllowedLocationTest.java @@ -40,6 +40,8 @@ import java.util.Map; import java.util.Optional; import java.util.UUID; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicInteger; import org.apache.iceberg.DataFile; import org.apache.iceberg.GenericStatisticsFile; import org.apache.iceberg.ImmutableGenericPartitionStatisticsFile; @@ -61,6 +63,7 @@ import org.apache.iceberg.rest.requests.CreateViewRequest; import org.apache.iceberg.rest.requests.ImmutableCreateViewRequest; import org.apache.iceberg.rest.requests.ImmutableRegisterTableRequest; +import org.apache.iceberg.rest.requests.UpdateNamespacePropertiesRequest; import org.apache.iceberg.rest.requests.UpdateTableRequest; import org.apache.iceberg.view.ImmutableSQLViewRepresentation; import org.apache.iceberg.view.ImmutableViewVersion; @@ -70,11 +73,13 @@ import org.apache.polaris.core.admin.model.FileStorageConfigInfo; import org.apache.polaris.core.admin.model.StorageConfigInfo; import org.apache.polaris.core.admin.model.UpdateCatalogRequest; +import org.apache.polaris.core.persistence.PolarisMetaStoreManager; import org.apache.polaris.service.TestServices; import org.apache.polaris.service.catalog.AccessDelegationMode; import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.mockito.Mockito; public class IcebergAllowedLocationTest { private static final String namespace = "ns"; @@ -730,6 +735,76 @@ private void createNamespace(TestServices services, String location) { } } + private void updateNamespaceProperties(TestServices services, Map updates) { + UpdateNamespacePropertiesRequest.Builder request = UpdateNamespacePropertiesRequest.builder(); + updates.forEach(request::update); + try (Response response = + services + .restApi() + .updateProperties( + catalog, + namespace, + request.build(), + IDEMPOTENCY_KEY, + services.realmContext(), + services.securityContext())) { + assertThat(response.getStatus()).isEqualTo(Response.Status.OK.getStatusCode()); + } + } + + @Test + void testNamespacePropertyUpdateValidatesLocationOnlyWhenItChanges(@TempDir Path tmpDir) { + AtomicBoolean counting = new AtomicBoolean(false); + AtomicInteger listEntitiesCalls = new AtomicInteger(); + + TestServices services = + TestServices.builder() + .config( + Map.of( + "ALLOW_INSECURE_STORAGE_TYPES", + "true", + "SUPPORTED_CATALOG_STORAGE_TYPES", + List.of("FILE"), + "ALLOW_NAMESPACE_CUSTOM_LOCATION", + "true", + OPTIMIZED_SIBLING_CHECK.key(), + "false")) + .metaStoreManagerDecorator( + msm -> { + PolarisMetaStoreManager spy = Mockito.spy(msm); + Mockito.doAnswer( + invocation -> { + if (counting.get()) { + listEntitiesCalls.incrementAndGet(); + } + return invocation.callRealMethod(); + }) + .when(spy) + .listEntities( + Mockito.any(), + Mockito.any(), + Mockito.any(), + Mockito.any(), + Mockito.any()); + return spy; + }) + .build(); + + String catalogLocation = tmpDir.toAbsolutePath().toUri().toString(); + createCatalog(services, Map.of(), catalogLocation, List.of(catalogLocation)); + String namespaceLocation = String.format("%s/%s/%s", catalogLocation, catalog, namespace); + createNamespace(services, namespaceLocation); + + counting.set(true); + + updateNamespaceProperties(services, Map.of("owner", "bob")); + assertThat(listEntitiesCalls.get()).isZero(); + + listEntitiesCalls.set(0); + updateNamespaceProperties(services, Map.of("location", namespaceLocation + "_moved")); + assertThat(listEntitiesCalls.get()).isPositive(); + } + private void updateCatalogAllowedLocations( TestServices services, List newAllowedLocations) { // Fetch current catalog to get entity version for update.