diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java index 8cf61a5c44..ccb9bc18d3 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogViewIntegrationBase.java @@ -20,7 +20,6 @@ import static org.apache.polaris.service.it.env.PolarisClient.polarisClient; -import com.google.common.collect.ImmutableMap; import java.lang.reflect.Method; import java.nio.file.Path; import java.nio.file.Paths; @@ -88,6 +87,7 @@ public abstract class PolarisRestCatalogViewIntegrationBase extends ViewCatalogT private static ManagementApi managementApi; private RESTCatalog restCatalog; + private StorageConfigInfo storageConfig; @BeforeAll static void setup(PolarisApiEndpoints apiEndpoints, ClientCredentials credentials) { @@ -114,7 +114,7 @@ public void before(TestInfo testInfo) { Method method = testInfo.getTestMethod().orElseThrow(); String catalogName = client.newEntityName(method.getName()); - StorageConfigInfo storageConfig = getStorageConfigInfo(); + storageConfig = getStorageConfigInfo(); String defaultBaseLocation = storageConfig.getAllowedLocations().getFirst() + "/" @@ -201,16 +201,10 @@ public void createViewWithCustomMetadataLocationUsingPolaris(@TempDir Path tempD TableIdentifier identifier = TableIdentifier.of("ns", "view"); String location = Paths.get(tempDir.toUri().toString()).toString(); - String customLocation = Paths.get(tempDir.toUri().toString(), "custom-location").toString(); - String customLocation2 = Paths.get(tempDir.toUri().toString(), "custom-location2").toString(); - String customLocationChild = - Paths.get(tempDir.toUri().toString(), "custom-location/child").toString(); + String customLocation = + Paths.get(storageConfig.getAllowedLocations().getFirst(), "/custom-location1").toString(); - catalog() - .createNamespace( - identifier.namespace(), - ImmutableMap.of( - IcebergTableLikeEntity.USER_SPECIFIED_WRITE_METADATA_LOCATION_KEY, location)); + catalog().createNamespace(identifier.namespace()); Assertions.assertThat(catalog().viewExists(identifier)).as("View should not exist").isFalse(); @@ -234,35 +228,5 @@ public void createViewWithCustomMetadataLocationUsingPolaris(@TempDir Path tempD Assertions.assertThat(((BaseView) view).operations().current().metadataFileLocation()) .isNotNull() .startsWith(customLocation); - - // CANNOT update the view with a new metadata location `baseLocation/customLocation2`, - // even though the new location is still under the parent namespace's - // `write.metadata.path=baseLocation`. - Assertions.assertThatThrownBy( - () -> - catalog() - .loadView(identifier) - .updateProperties() - .set( - IcebergTableLikeEntity.USER_SPECIFIED_WRITE_METADATA_LOCATION_KEY, - customLocation2) - .commit()) - .isInstanceOf(ForbiddenException.class) - .hasMessageContaining("Forbidden: Invalid locations"); - - // CANNOT update the view with a child metadata location `baseLocation/customLocation/child`, - // even though it is a subpath of the original view's - // `write.metadata.path=baseLocation/customLocation`. - Assertions.assertThatThrownBy( - () -> - catalog() - .loadView(identifier) - .updateProperties() - .set( - IcebergTableLikeEntity.USER_SPECIFIED_WRITE_METADATA_LOCATION_KEY, - customLocationChild) - .commit()) - .isInstanceOf(ForbiddenException.class) - .hasMessageContaining("Forbidden: Invalid locations"); } } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/InMemoryStorageIntegration.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/InMemoryStorageIntegration.java index 7e719a91da..ea4ca1c594 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/InMemoryStorageIntegration.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/InMemoryStorageIntegration.java @@ -53,16 +53,15 @@ protected InMemoryStorageIntegration(T config, String identifierOrId) { * implementation, all actions have the same validation result, as we only verify the * locations are equal to or subdirectories of the allowed locations. */ - public static Map> - validateSubpathsOfAllowedLocations( - @Nonnull RealmConfig realmConfig, - @Nonnull PolarisStorageConfigurationInfo storageConfig, - @Nonnull Set actions, - @Nonnull Set locations) { + public static Map> validateAllowedLocations( + @Nonnull RealmConfig realmConfig, + @Nonnull List allowedLocationsToValid, + @Nonnull Set actions, + @Nonnull Set locations) { // trim trailing / from allowed locations so that locations missing the trailing slash still // match Set allowedLocationStrings = - storageConfig.getAllowedLocations().stream() + allowedLocationsToValid.stream() .map( str -> { if (str.endsWith("/") && str.length() > 1) { @@ -123,6 +122,7 @@ public Map> validateAccessT @Nonnull T storageConfig, @Nonnull Set actions, @Nonnull Set locations) { - return validateSubpathsOfAllowedLocations(realmConfig, storageConfig, actions, locations); + return validateAllowedLocations( + realmConfig, storageConfig.getAllowedLocations(), actions, locations); } } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/LocationRestrictions.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/LocationRestrictions.java new file mode 100644 index 0000000000..90ccf50d6d --- /dev/null +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/LocationRestrictions.java @@ -0,0 +1,123 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.polaris.core.storage; + +import jakarta.annotation.Nonnull; +import java.util.List; +import java.util.Set; +import org.apache.iceberg.catalog.TableIdentifier; +import org.apache.iceberg.exceptions.ForbiddenException; +import org.apache.polaris.core.config.RealmConfig; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** Defines storage location access restrictions for Polaris entities within a specific context. */ +public class LocationRestrictions { + private static final Logger LOGGER = LoggerFactory.getLogger(LocationRestrictions.class); + + /** + * The complete set of storage locations that are permitted for access. + * + *

This list contains all storage URIs that entities can read from or write to, including both + * catalog-level allowed locations and any additional user-specified locations when unstructured + * table access is enabled. + * + *

All locations in this list have been validated to conform to the storage type's URI scheme + * requirements during construction. + */ + private final List allowedLocations; + + /** + * The parent location for structured table enforcement. + * + *

When non-null, this location represents the root under which all new tables must be created, + * enforcing a structured hierarchy in addition to residing under {@code allowedLocations}. When + * null, table creation is allowed anywhere within the {@code allowedLocations}. + */ + private final String parentLocation; + + public LocationRestrictions( + @Nonnull PolarisStorageConfigurationInfo storageConfigurationInfo, String parentLocation) { + this.allowedLocations = storageConfigurationInfo.getAllowedLocations(); + allowedLocations.forEach(storageConfigurationInfo::validatePrefixForStorageType); + this.parentLocation = parentLocation; + } + + public LocationRestrictions(@Nonnull PolarisStorageConfigurationInfo storageConfigurationInfo) { + this(storageConfigurationInfo, null); + } + + /** + * Validates that the requested storage locations are permitted for the given table identifier. + * + *

This method performs location validation by checking the requested locations against: + * + *

    + *
  • The parent location (if configured) for structured table enforcement + *
  • The allowed locations list for general access permissions + *
+ * + *

The validation ensures that all requested locations conform to the storage access + * restrictions defined for this context. If a parent location is configured, all requests must be + * under that location hierarchy in addition to being within the allowed locations. + * + * @param realmConfig the realm configuration containing storage validation rules + * @param identifier the table identifier for which locations are being validated + * @param requestLocations the set of storage locations that need validation + * @throws ForbiddenException if any of the requested locations violate the configured + * restrictions + */ + public void validate( + RealmConfig realmConfig, TableIdentifier identifier, Set requestLocations) { + if (parentLocation != null) { + validateLocations(realmConfig, List.of(parentLocation), requestLocations, identifier); + } + + validateLocations(realmConfig, allowedLocations, requestLocations, identifier); + } + + private void validateLocations( + RealmConfig realmConfig, + List allowedLocations, + Set requestLocations, + TableIdentifier identifier) { + var validationResults = + InMemoryStorageIntegration.validateAllowedLocations( + realmConfig, allowedLocations, Set.of(PolarisStorageActions.ALL), requestLocations); + validationResults + .values() + .forEach( + actionResult -> + actionResult + .values() + .forEach( + result -> { + if (!result.isSuccess()) { + throw new ForbiddenException( + "Invalid locations '%s' for identifier '%s': %s", + requestLocations, identifier, result.getMessage()); + } else { + LOGGER.debug( + "Validated locations '{}' for identifier '{}'", + requestLocations, + identifier); + } + })); + } +} diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisStorageConfigurationInfo.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisStorageConfigurationInfo.java index e3947a20be..d94879e289 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisStorageConfigurationInfo.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/PolarisStorageConfigurationInfo.java @@ -26,14 +26,12 @@ import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.DeserializationFeature; import com.fasterxml.jackson.databind.ObjectMapper; -import com.google.common.collect.ImmutableList; import jakarta.annotation.Nonnull; import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Locale; import java.util.Optional; -import java.util.Set; import org.apache.polaris.core.admin.model.Catalog; import org.apache.polaris.core.config.FeatureConfiguration; import org.apache.polaris.core.config.RealmConfig; @@ -116,7 +114,7 @@ public static PolarisStorageConfigurationInfo deserialize(final @Nonnull String } } - public static Optional forEntityPath( + public static Optional forEntityPath( RealmConfig realmConfig, List entityPath) { return findStorageInfoFromHierarchy(entityPath) .map( @@ -150,22 +148,13 @@ public static Optional forEntityPath( LOGGER.debug( "Not allowing unstructured table location for entity: {}", entityPathReversed.get(0).getName()); - return new StorageConfigurationOverride(configInfo, List.of(baseLocation)); + return new LocationRestrictions(configInfo, baseLocation); } else { LOGGER.debug( "Allowing unstructured table location for entity: {}", entityPathReversed.get(0).getName()); - // TODO: figure out the purpose of adding `userSpecifiedWriteLocations` - Set locations = - StorageUtil.getLocationsAllowedToBeAccessed( - null, entityPathReversed.get(0).getPropertiesAsMap()); - return new StorageConfigurationOverride( - configInfo, - ImmutableList.builder() - .addAll(configInfo.getAllowedLocations()) - .addAll(locations) - .build()); + return new LocationRestrictions(configInfo); } }); } diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageConfigurationOverride.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageConfigurationOverride.java deleted file mode 100644 index b38ebf2fc8..0000000000 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageConfigurationOverride.java +++ /dev/null @@ -1,68 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.polaris.core.storage; - -import jakarta.annotation.Nonnull; -import java.util.List; - -/** - * Allows overriding the allowed locations for specific entities. Only the allowedLocations - * specified in the constructor are allowed. allowedLocations are not inherited from the parent - * storage configuration. All other storage configuration is inherited from the parent configuration - * and cannot be overridden. - */ -public class StorageConfigurationOverride extends PolarisStorageConfigurationInfo { - - private final PolarisStorageConfigurationInfo parentStorageConfiguration; - private final List allowedLocations; - - public StorageConfigurationOverride( - @Nonnull PolarisStorageConfigurationInfo parentStorageConfiguration, - List allowedLocations) { - this.parentStorageConfiguration = parentStorageConfiguration; - this.allowedLocations = List.copyOf(allowedLocations); - allowedLocations.forEach(this::validatePrefixForStorageType); - } - - @Override - public List getAllowedLocations() { - return allowedLocations; - } - - @Override - public StorageType getStorageType() { - return parentStorageConfiguration.getStorageType(); - } - - @Override - public String getFileIoImplClassName() { - return parentStorageConfiguration.getFileIoImplClassName(); - } - - @Override - public boolean validatePrefix() { - return false; - } - - // delegate to the wrapped class in case they override the parent behavior - @Override - protected void validatePrefixForStorageType(String loc) { - parentStorageConfiguration.validatePrefixForStorageType(loc); - } -} diff --git a/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageUtil.java b/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageUtil.java index 3a67d1e733..16fdf80249 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageUtil.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/storage/StorageUtil.java @@ -70,12 +70,12 @@ public class StorageUtil { } /** Given a TableMetadata, extracts the locations where the table's [meta]data might be found. */ - public static @Nonnull Set getLocationsAllowedToBeAccessed(TableMetadata tableMetadata) { - return getLocationsAllowedToBeAccessed(tableMetadata.location(), tableMetadata.properties()); + public static @Nonnull Set getLocationsUsedByTable(TableMetadata tableMetadata) { + return getLocationsUsedByTable(tableMetadata.location(), tableMetadata.properties()); } /** Given a baseLocation and entity (table?) properties, extracts the relevant locations */ - public static @Nonnull Set getLocationsAllowedToBeAccessed( + public static @Nonnull Set getLocationsUsedByTable( String baseLocation, Map properties) { Set locations = new HashSet<>(); locations.add(baseLocation); @@ -87,7 +87,7 @@ public class StorageUtil { } /** Given a ViewMetadata, extracts the locations where the view's [meta]data might be found. */ - public static @Nonnull Set getLocationsAllowedToBeAccessed(ViewMetadata viewMetadata) { + public static @Nonnull Set getLocationsUsedByTable(ViewMetadata viewMetadata) { return Set.of(viewMetadata.location()); } diff --git a/polaris-core/src/test/java/org/apache/polaris/core/storage/StorageUtilTest.java b/polaris-core/src/test/java/org/apache/polaris/core/storage/StorageUtilTest.java index 593aecc6ad..b3b9cfb298 100644 --- a/polaris-core/src/test/java/org/apache/polaris/core/storage/StorageUtilTest.java +++ b/polaris-core/src/test/java/org/apache/polaris/core/storage/StorageUtilTest.java @@ -70,38 +70,37 @@ public void testAuthorityWithPort() { } @Test - public void getLocationsAllowedToBeAccessed() { - Assertions.assertThat(StorageUtil.getLocationsAllowedToBeAccessed(null, Map.of())).isEmpty(); - Assertions.assertThat(StorageUtil.getLocationsAllowedToBeAccessed("", Map.of())).isNotEmpty(); - Assertions.assertThat(StorageUtil.getLocationsAllowedToBeAccessed("/foo/", Map.of())) - .contains("/foo/"); + public void getLocationsUsedByTable() { + Assertions.assertThat(StorageUtil.getLocationsUsedByTable(null, Map.of())).isEmpty(); + Assertions.assertThat(StorageUtil.getLocationsUsedByTable("", Map.of())).isNotEmpty(); + Assertions.assertThat(StorageUtil.getLocationsUsedByTable("/foo/", Map.of())).contains("/foo/"); Assertions.assertThat( - StorageUtil.getLocationsAllowedToBeAccessed( + StorageUtil.getLocationsUsedByTable( "/foo/", Map.of(IcebergTableLikeEntity.USER_SPECIFIED_WRITE_DATA_LOCATION_KEY, "/foo/"))) .contains("/foo/"); Assertions.assertThat( - StorageUtil.getLocationsAllowedToBeAccessed( + StorageUtil.getLocationsUsedByTable( "/foo/", Map.of(IcebergTableLikeEntity.USER_SPECIFIED_WRITE_DATA_LOCATION_KEY, "/bar/"))) .contains("/foo/", "/bar/"); Assertions.assertThat( - StorageUtil.getLocationsAllowedToBeAccessed( + StorageUtil.getLocationsUsedByTable( "/foo/", Map.of(IcebergTableLikeEntity.USER_SPECIFIED_WRITE_DATA_LOCATION_KEY, "/foo/bar/"))) .contains("/foo/"); Assertions.assertThat( - StorageUtil.getLocationsAllowedToBeAccessed( + StorageUtil.getLocationsUsedByTable( "/foo/bar/", Map.of(IcebergTableLikeEntity.USER_SPECIFIED_WRITE_DATA_LOCATION_KEY, "/foo/"))) .contains("/foo/"); Assertions.assertThat( - StorageUtil.getLocationsAllowedToBeAccessed( + StorageUtil.getLocationsUsedByTable( "/foo/bar/", Map.of(IcebergTableLikeEntity.USER_SPECIFIED_WRITE_METADATA_LOCATION_KEY, "/foo/"))) .contains("/foo/"); Assertions.assertThat( - StorageUtil.getLocationsAllowedToBeAccessed( + StorageUtil.getLocationsUsedByTable( "/1/", Map.of( IcebergTableLikeEntity.USER_SPECIFIED_WRITE_DATA_LOCATION_KEY, "/2/", diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java index 5da1eeb123..03aa41c80a 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/iceberg/IcebergCatalog.java @@ -119,11 +119,9 @@ import org.apache.polaris.core.persistence.resolver.ResolverPath; import org.apache.polaris.core.persistence.resolver.ResolverStatus; import org.apache.polaris.core.storage.AccessConfig; -import org.apache.polaris.core.storage.InMemoryStorageIntegration; import org.apache.polaris.core.storage.PolarisCredentialVendor; import org.apache.polaris.core.storage.PolarisStorageActions; import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo; -import org.apache.polaris.core.storage.PolarisStorageIntegration; import org.apache.polaris.core.storage.StorageLocation; import org.apache.polaris.core.storage.StorageUtil; import org.apache.polaris.core.storage.cache.StorageCredentialCache; @@ -845,7 +843,7 @@ public AccessConfig getAccessConfig( storageCredentialCache, getCredentialVendor(), tableIdentifier, - StorageUtil.getLocationsAllowedToBeAccessed(tableMetadata), + StorageUtil.getLocationsUsedByTable(tableMetadata), storageActions, storageInfo.get(), refreshCredentialsEndpoint); @@ -1006,51 +1004,27 @@ private void validateLocationsForTableLike( TableIdentifier identifier, Set locations, PolarisResolvedPathWrapper resolvedStorageEntity) { - Optional optStorageConfiguration = - PolarisStorageConfigurationInfo.forEntityPath( - realmConfig, resolvedStorageEntity.getRawFullPath()); - - optStorageConfiguration.ifPresentOrElse( - storageConfigInfo -> { - Map> - validationResults = - InMemoryStorageIntegration.validateSubpathsOfAllowedLocations( - realmConfig, storageConfigInfo, Set.of(PolarisStorageActions.ALL), locations); - validationResults - .values() - .forEach( - actionResult -> - actionResult - .values() - .forEach( - result -> { - if (!result.isSuccess()) { - throw new ForbiddenException( - "Invalid locations '%s' for identifier '%s': %s", - locations, identifier, result.getMessage()); - } else { - LOGGER.debug( - "Validated locations '{}' for identifier '{}'", - locations, - identifier); - } - })); - }, - () -> { - List allowedStorageTypes = - realmConfig.getConfig(FeatureConfiguration.SUPPORTED_CATALOG_STORAGE_TYPES); - if (!allowedStorageTypes.contains(StorageConfigInfo.StorageTypeEnum.FILE.name())) { - List invalidLocations = - locations.stream() - .filter(location -> location.startsWith("file:") || location.startsWith("http")) - .collect(Collectors.toList()); - if (!invalidLocations.isEmpty()) { - throw new ForbiddenException( - "Invalid locations '%s' for identifier '%s': File locations are not allowed", - invalidLocations, identifier); - } - } - }); + + PolarisStorageConfigurationInfo.forEntityPath( + realmConfig, resolvedStorageEntity.getRawFullPath()) + .ifPresentOrElse( + restrictions -> restrictions.validate(realmConfig, identifier, locations), + () -> { + List allowedStorageTypes = + realmConfig.getConfig(FeatureConfiguration.SUPPORTED_CATALOG_STORAGE_TYPES); + if (!allowedStorageTypes.contains(StorageConfigInfo.StorageTypeEnum.FILE.name())) { + List invalidLocations = + locations.stream() + .filter( + location -> location.startsWith("file:") || location.startsWith("http")) + .collect(Collectors.toList()); + if (!invalidLocations.isEmpty()) { + throw new ForbiddenException( + "Invalid locations '%s' for identifier '%s': File locations are not allowed", + invalidLocations, identifier); + } + } + }); } /** @@ -1437,7 +1411,7 @@ public void doCommit(TableMetadata base, TableMetadata metadata) { tableFileIO = loadFileIOForTableLike( tableIdentifier, - StorageUtil.getLocationsAllowedToBeAccessed(metadata), + StorageUtil.getLocationsUsedByTable(metadata), resolvedStorageEntity, new HashMap<>(metadata.properties()), Set.of( @@ -1460,7 +1434,7 @@ public void doCommit(TableMetadata base, TableMetadata metadata) { // If location is changing then we must validate that the requested location is valid // for the storage configuration inherited under this entity's path. Set dataLocations = - StorageUtil.getLocationsAllowedToBeAccessed(metadata.location(), metadata.properties()); + StorageUtil.getLocationsUsedByTable(metadata.location(), metadata.properties()); validateLocationsForTableLike(tableIdentifier, dataLocations, resolvedStorageEntity); // also validate that the table location doesn't overlap an existing table dataLocations.forEach( @@ -1821,7 +1795,7 @@ public void doCommit(ViewMetadata base, ViewMetadata metadata) { viewFileIO = loadFileIOForTableLike( identifier, - StorageUtil.getLocationsAllowedToBeAccessed(metadata), + StorageUtil.getLocationsUsedByTable(metadata), resolvedStorageEntity, tableProperties, Set.of(PolarisStorageActions.READ, PolarisStorageActions.WRITE)); 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 new file mode 100644 index 0000000000..f24d3dd376 --- /dev/null +++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/IcebergAllowedLocationTest.java @@ -0,0 +1,189 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.polaris.service.catalog.iceberg; + +import static org.apache.polaris.core.config.FeatureConfiguration.OPTIMIZED_SIBLING_CHECK; +import static org.apache.polaris.service.admin.PolarisAuthzTestBase.SCHEMA; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import jakarta.ws.rs.core.Response; +import java.nio.file.Path; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.UUID; +import org.apache.iceberg.catalog.Namespace; +import org.apache.iceberg.exceptions.ForbiddenException; +import org.apache.iceberg.rest.requests.CreateNamespaceRequest; +import org.apache.iceberg.rest.requests.CreateTableRequest; +import org.apache.polaris.core.admin.model.Catalog; +import org.apache.polaris.core.admin.model.CatalogProperties; +import org.apache.polaris.core.admin.model.CreateCatalogRequest; +import org.apache.polaris.core.admin.model.FileStorageConfigInfo; +import org.apache.polaris.core.admin.model.StorageConfigInfo; +import org.apache.polaris.service.TestServices; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +public class IcebergAllowedLocationTest { + private static final String namespace = "ns"; + private static final String catalog = "test-catalog"; + + private String getTableName() { + return "table_" + UUID.randomUUID(); + } + + @Test + void testCreateTableOutSideOfCatalogAllowedLocations(@TempDir Path tmpDir) { + var services = getTestServices(); + + var catalogLocation = tmpDir.resolve(catalog).toAbsolutePath().toUri().toString(); + var namespaceLocation = tmpDir.resolve(namespace).toAbsolutePath().toUri().toString(); + assertNotEquals(catalogLocation, namespaceLocation); + + createCatalog(services, Map.of(), catalogLocation, null); + + // create a namespace outside of catalog allowed locations + createNamespace(services, namespaceLocation); + + // create a table under the namespace + var createTableRequest = + CreateTableRequest.builder().withName(getTableName()).withSchema(SCHEMA).build(); + + assertThrows( + ForbiddenException.class, + () -> + services + .restApi() + .createTable( + catalog, + namespace, + createTableRequest, + null, + services.realmContext(), + services.securityContext())); + } + + @Test + void testCreateTableInsideOfCatalogAllowedLocations(@TempDir Path tmpDir) { + var services = getTestServices(); + + var catalogLocation = tmpDir.resolve(catalog).toAbsolutePath().toUri().toString(); + var namespaceLocation = tmpDir.resolve(namespace).toAbsolutePath().toUri().toString(); + assertNotEquals(catalogLocation, namespaceLocation); + + // add the namespace location to the allowed locations of the catalog + createCatalog(services, Map.of(), catalogLocation, List.of(namespaceLocation)); + + createNamespace(services, namespaceLocation); + + // create a table under the namespace + var createTableRequest = + CreateTableRequest.builder().withName(getTableName()).withSchema(SCHEMA).build(); + + var response = + services + .restApi() + .createTable( + catalog, + namespace, + createTableRequest, + null, + services.realmContext(), + services.securityContext()); + + assertEquals(response.getStatus(), Response.Status.OK.getStatusCode()); + } + + private static TestServices getTestServices() { + Map strictServicesWithOptimizedOverlapCheck = + Map.of( + "ALLOW_TABLE_LOCATION_OVERLAP", + "true", + "ALLOW_INSECURE_STORAGE_TYPES", + "true", + "SUPPORTED_CATALOG_STORAGE_TYPES", + List.of("FILE"), + OPTIMIZED_SIBLING_CHECK.key(), + "true"); + TestServices services = + TestServices.builder().config(strictServicesWithOptimizedOverlapCheck).build(); + return services; + } + + private void createCatalog( + TestServices services, + Map catalogConfig, + String catalogLocation, + List allowedLocations) { + CatalogProperties.Builder propertiesBuilder = + CatalogProperties.builder() + .setDefaultBaseLocation(String.format("%s/%s", catalogLocation, catalog)) + .putAll(catalogConfig); + + StorageConfigInfo config = + FileStorageConfigInfo.builder() + .setStorageType(StorageConfigInfo.StorageTypeEnum.FILE) + .setAllowedLocations(allowedLocations) + .build(); + + Catalog catalogObject = + new Catalog( + Catalog.TypeEnum.INTERNAL, + catalog, + propertiesBuilder.build(), + 1725487592064L, + 1725487592064L, + 1, + config); + try (Response response = + services + .catalogsApi() + .createCatalog( + new CreateCatalogRequest(catalogObject), + services.realmContext(), + services.securityContext())) { + assertThat(response.getStatus()).isEqualTo(Response.Status.CREATED.getStatusCode()); + } + } + + private void createNamespace(TestServices services, String location) { + Map properties = new HashMap<>(); + properties.put("location", location); + CreateNamespaceRequest createNamespaceRequest = + CreateNamespaceRequest.builder() + .withNamespace(Namespace.of(namespace)) + .setProperties(properties) + .build(); + try (Response response = + services + .restApi() + .createNamespace( + catalog, + createNamespaceRequest, + services.realmContext(), + services.securityContext())) { + assertThat(response.getStatus()).isEqualTo(Response.Status.OK.getStatusCode()); + } + } +}