You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@polaris.apache.org by sn...@apache.org on 2026/07/20 19:01:05 UTC
(polaris) branch main updated: Validate locations when registering tables and views (#5114)
This is an automated email from the ASF dual-hosted git repository.
snazy pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/polaris.git
The following commit(s) were added to refs/heads/main by this push:
new 7e822f23ab Validate locations when registering tables and views (#5114)
7e822f23ab is described below
commit 7e822f23aba00a440f9bddbbf16497bf69e3858c
Author: Robert Stupp <sn...@snazy.de>
AuthorDate: Mon Jul 20 21:01:00 2026 +0200
Validate locations when registering tables and views (#5114)
---------
Co-authored-by: Alexandre Dutra <ad...@apache.org>
---
.../catalog/iceberg/LocalIcebergCatalog.java | 85 ++++----
.../iceberg/AbstractLocalIcebergCatalogTest.java | 220 +++++++++++++++++++++
2 files changed, 270 insertions(+), 35 deletions(-)
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 5b77cdeb19..2a21e6ef51 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
@@ -79,7 +79,6 @@ import org.apache.iceberg.exceptions.ServiceFailureException;
import org.apache.iceberg.exceptions.UnprocessableEntityException;
import org.apache.iceberg.io.CloseableGroup;
import org.apache.iceberg.io.FileIO;
-import org.apache.iceberg.io.InputFile;
import org.apache.iceberg.io.LocationProvider;
import org.apache.iceberg.io.OutputFile;
import org.apache.iceberg.util.LocationUtil;
@@ -427,9 +426,10 @@ public class LocalIcebergCatalog extends BaseMetastoreViewCatalog
new HashMap<>(tableDefaultProperties),
Set.of(PolarisStorageActions.READ, PolarisStorageActions.LIST));
- InputFile metadataFile = fileIO.newInputFile(metadataFileLocation);
- TableMetadata metadata = TableMetadataParser.read(metadataFile);
- validateMetadataFileInTableDir(identifier, metadata);
+ TableMetadata metadata = TableMetadataParser.read(fileIO, metadataFileLocation);
+ validateTableMetadataLocations(
+ identifier, metadata, resolvedParent, resolvedParent.getRawFullPath());
+
ops.commit(null, metadata);
return new BaseTable(ops, fullTableName(name(), identifier), metricsReporter());
@@ -455,20 +455,8 @@ public class LocalIcebergCatalog extends BaseMetastoreViewCatalog
Set.of(PolarisStorageActions.READ, PolarisStorageActions.LIST));
TableMetadata metadata = TableMetadataParser.read(fileIO, metadataFileLocation);
- validateMetadataFileInTableDir(identifier, metadata);
-
- List<PolarisEntity> resolvedNamespace = resolvedPath.getRawParentPath();
- var tableLocations = StorageUtil.getLocationsUsedByTable(metadata);
- CatalogUtils.validateLocationsForTableLike(
- realmConfig, identifier, tableLocations, resolvedPath);
- tableLocations.forEach(
- location ->
- validateNoLocationOverlap(
- catalogEntity,
- identifier,
- resolvedNamespace,
- location,
- resolvedPath.getRawLeafEntity()));
+ validateTableMetadataLocations(
+ identifier, metadata, resolvedPath, resolvedPath.getRawParentPath());
PolarisEntity rawEntity = resolvedPath.getRawLeafEntity();
if (rawEntity.getSubType() != PolarisEntitySubType.ICEBERG_TABLE) {
@@ -1080,6 +1068,9 @@ public class LocalIcebergCatalog extends BaseMetastoreViewCatalog
throw new IllegalStateException(
String.format("Failed to fetch resolved parent for TableIdentifier '%s'", identifier));
}
+
+ validateLocationForTableLike(identifier, metadataFileLocation, resolvedParent);
+
FileIO fileIO =
loadFileIOForTableLike(
identifier,
@@ -1088,8 +1079,11 @@ public class LocalIcebergCatalog extends BaseMetastoreViewCatalog
new HashMap<>(tableDefaultProperties),
Set.of(PolarisStorageActions.READ, PolarisStorageActions.LIST));
- InputFile metadataFile = fileIO.newInputFile(metadataFileLocation);
- ViewMetadata metadata = ViewMetadataParser.read(metadataFile);
+ ViewMetadata metadata = ViewMetadataParser.read(fileIO, metadataFileLocation);
+
+ validateLocationForTableLike(identifier, metadata.location(), resolvedParent);
+ validateMetadataFileInTableDir(identifier, metadata.location(), metadataFileLocation);
+
ops.commit(null, metadata);
return new BaseView(ops, ViewUtil.fullViewName(name(), identifier));
@@ -1276,12 +1270,11 @@ public class LocalIcebergCatalog extends BaseMetastoreViewCatalog
StorageUtil.getLocationsUsedByTable(tableMetadata.location(), tableMetadata.properties());
CatalogUtils.validateLocationsForTableLike(
realmConfig, tableIdentifier, dataLocations, resolvedStorageEntity);
- List<PolarisEntity> resolvedNamespace = resolvedStorageEntity.getRawFullPath();
- PolarisEntity storageLeafEntity = resolvedStorageEntity.getRawLeafEntity();
- dataLocations.forEach(
- location ->
- validateNoLocationOverlap(
- catalogEntity, tableIdentifier, resolvedNamespace, location, storageLeafEntity));
+ validateNoLocationsOverlap(
+ tableIdentifier,
+ dataLocations,
+ resolvedStorageEntity,
+ resolvedStorageEntity.getRawFullPath());
}
/**
@@ -1317,6 +1310,35 @@ public class LocalIcebergCatalog extends BaseMetastoreViewCatalog
realmConfig, identifier, Set.of(location), resolvedStorageEntity);
}
+ private void validateTableMetadataLocations(
+ TableIdentifier identifier,
+ TableMetadata metadata,
+ PolarisResolvedPathWrapper resolvedStorageEntity,
+ List<PolarisEntity> resolvedNamespace) {
+ // checks throwing ForbiddenException (must come first)
+ Set<String> locations = StorageUtil.getLocationsUsedByTable(metadata);
+ CatalogUtils.validateLocationsForTableLike(
+ realmConfig, identifier, locations, resolvedStorageEntity);
+ validateNoLocationsOverlap(identifier, locations, resolvedStorageEntity, resolvedNamespace);
+ // checks throwing BadRequestException (must come after)
+ validateMetadataFileInTableDir(identifier, metadata);
+ }
+
+ private void validateNoLocationsOverlap(
+ TableIdentifier identifier,
+ Set<String> locations,
+ PolarisResolvedPathWrapper resolvedStorageEntity,
+ List<PolarisEntity> resolvedNamespace) {
+ locations.forEach(
+ location ->
+ validateNoLocationOverlap(
+ catalogEntity,
+ identifier,
+ resolvedNamespace,
+ location,
+ resolvedStorageEntity.getRawLeafEntity()));
+ }
+
/**
* Validates the table location has no overlap with other entities after checking the
* configuration of the service
@@ -1857,15 +1879,8 @@ public class LocalIcebergCatalog extends BaseMetastoreViewCatalog
// for the storage configuration inherited under this entity's path.
CatalogUtils.validateLocationsForTableLike(
realmConfig, tableIdentifier, requestedLocations, resolvedStorageEntity);
- // also validate that the table location doesn't overlap an existing table
- requestedLocations.forEach(
- location ->
- validateNoLocationOverlap(
- catalogEntity,
- tableIdentifier,
- resolvedNamespace,
- location,
- resolvedStorageEntity.getRawLeafEntity()));
+ validateNoLocationsOverlap(
+ tableIdentifier, requestedLocations, resolvedStorageEntity, resolvedNamespace);
// and that the metadata file points to a location within the table's directory structure
validateMetadataFileInTableDir(
tableIdentifier, metadata.location(), nextMetadataFileLocation(metadata));
diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractLocalIcebergCatalogTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractLocalIcebergCatalogTest.java
index a2f9367f6e..9524c452ba 100644
--- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractLocalIcebergCatalogTest.java
+++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractLocalIcebergCatalogTest.java
@@ -102,6 +102,10 @@ import org.apache.iceberg.rest.requests.UpdateTableRequest;
import org.apache.iceberg.types.Types;
import org.apache.iceberg.util.CharSequenceSet;
import org.apache.iceberg.util.LocationUtil;
+import org.apache.iceberg.view.ImmutableSQLViewRepresentation;
+import org.apache.iceberg.view.ImmutableViewVersion;
+import org.apache.iceberg.view.ViewMetadata;
+import org.apache.iceberg.view.ViewMetadataParser;
import org.apache.polaris.core.PolarisCallContext;
import org.apache.polaris.core.PolarisDiagnostics;
import org.apache.polaris.core.admin.model.AwsStorageConfigInfo;
@@ -2253,6 +2257,29 @@ public abstract class AbstractLocalIcebergCatalogTest extends CatalogTests<Local
return TableMetadata.newTableMetadata(schema, partitionSpec, tableLocation, ImmutableMap.of());
}
+ private ViewMetadata createSampleViewMetadata(String viewLocation) {
+ return ViewMetadata.builder()
+ .setLocation(viewLocation)
+ .addSchema(
+ new Schema(
+ Types.NestedField.required(1, "intType", Types.IntegerType.get()),
+ Types.NestedField.required(2, "stringType", Types.StringType.get())))
+ .addVersion(
+ ImmutableViewVersion.builder()
+ .versionId(1)
+ .schemaId(0)
+ .timestampMillis(0)
+ .defaultNamespace(Namespace.empty())
+ .addRepresentations(
+ ImmutableSQLViewRepresentation.builder()
+ .sql("SELECT 1")
+ .dialect("spark")
+ .build())
+ .build())
+ .setCurrentVersionId(1)
+ .build();
+ }
+
private void createNonExistingNamespaces(Namespace namespace) {
// Pre-create namespaces if they don't exist
for (int i = 1; i <= namespace.length(); i++) {
@@ -2547,6 +2574,199 @@ public abstract class AbstractLocalIcebergCatalogTest extends CatalogTests<Local
.hasMessageContaining("View with same name already exists");
}
+ @ParameterizedTest
+ @ValueSource(booleans = {false, true})
+ public void testRegisterTableRejectsMetadataFileOutsideAllowedLocations(boolean overwrite) {
+ LocalIcebergCatalog catalog = catalog();
+ Namespace namespace = Namespace.of("register_disallowed_metadata");
+ TableIdentifier table = TableIdentifier.of(namespace, "table");
+ if (requiresNamespaceCreate()) {
+ catalog.createNamespace(namespace);
+ }
+
+ String forbiddenMetadataLocation = "s3://forbidden-bucket/table/metadata/v1.metadata.json";
+
+ Assertions.assertThatThrownBy(
+ () -> catalog.registerTable(table, forbiddenMetadataLocation, overwrite))
+ .isInstanceOf(ForbiddenException.class)
+ .hasMessageContaining("Invalid location");
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {false, true})
+ public void testRegisterTableRejectsTableLocationOutsideAllowedLocations(boolean overwrite) {
+ LocalIcebergCatalog catalog = catalog();
+ Namespace namespace = Namespace.of("register_disallowed_table_location");
+ TableIdentifier table = TableIdentifier.of(namespace, "table");
+ if (requiresNamespaceCreate()) {
+ catalog.createNamespace(namespace);
+ }
+
+ // Metadata file is at an allowed location, but the table location embedded inside it is not.
+ String metadataLocation =
+ STORAGE_LOCATION + "/register_disallowed_table_location/table/metadata/v1.metadata.json";
+
+ fileIO.addFile(
+ metadataLocation,
+ TableMetadataParser.toJson(createSampleTableMetadata("s3://forbidden-bucket/table/"))
+ .getBytes(UTF_8));
+
+ Assertions.assertThatThrownBy(() -> catalog.registerTable(table, metadataLocation, overwrite))
+ .isInstanceOf(ForbiddenException.class)
+ .hasMessageContaining("Invalid location");
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {false, true})
+ public void testRegisterTableRejectsMetadataFileOutsideTableLocation(boolean overwrite) {
+ updateCatalogProperties(
+ Map.of(
+ FeatureConfiguration.ALLOW_EXTERNAL_METADATA_FILE_LOCATION.catalogConfig(), "false"));
+
+ LocalIcebergCatalog catalog = catalog();
+ Namespace namespace = Namespace.of("register_metadata_outside_table_location");
+ TableIdentifier table = TableIdentifier.of(namespace, "table");
+ if (requiresNamespaceCreate()) {
+ catalog.createNamespace(namespace);
+ }
+
+ if (overwrite) {
+ catalog.buildTable(table, SCHEMA).create();
+ }
+
+ // The table's base location and the metadata file location arg are both within STORAGE_LOCATION
+ // (allowed), but the metadata file arg is in a different directory than the table location
+ // embedded in the metadata content.
+ String tableLocation = STORAGE_LOCATION + "/register_metadata_outside_table_location/table/";
+ // Metadata file sits outside the table's directory tree
+ String metadataLocation =
+ STORAGE_LOCATION + "/register_metadata_outside_table_location/other_dir/v1.metadata.json";
+ TableMetadata tableMetadata = createSampleTableMetadata(tableLocation);
+ fileIO.addFile(metadataLocation, TableMetadataParser.toJson(tableMetadata).getBytes(UTF_8));
+
+ Assertions.assertThatThrownBy(() -> catalog.registerTable(table, metadataLocation, overwrite))
+ .isInstanceOf(BadRequestException.class)
+ .hasMessageContaining("is not allowed outside of table location");
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {false, true})
+ public void testRegisterTableAllowsMetadataFileOutsideTableLocationWhenFlagEnabled(
+ boolean overwrite) {
+ updateCatalogProperties(
+ Map.of(FeatureConfiguration.ALLOW_EXTERNAL_METADATA_FILE_LOCATION.catalogConfig(), "true"));
+
+ LocalIcebergCatalog catalog = catalog();
+ Namespace namespace = Namespace.of("register_metadata_outside_table_location_allowed");
+ TableIdentifier table = TableIdentifier.of(namespace, "table");
+ if (requiresNamespaceCreate()) {
+ catalog.createNamespace(namespace);
+ }
+
+ if (overwrite) {
+ catalog.buildTable(table, SCHEMA).create();
+ }
+
+ String tableLocation =
+ STORAGE_LOCATION + "/register_metadata_outside_table_location_allowed/table/";
+ String metadataLocation =
+ STORAGE_LOCATION
+ + "/register_metadata_outside_table_location_allowed/other_dir/v1.metadata.json";
+ fileIO.addFile(
+ metadataLocation,
+ TableMetadataParser.toJson(createSampleTableMetadata(tableLocation)).getBytes(UTF_8));
+
+ Assertions.assertThat(catalog.registerTable(table, metadataLocation, overwrite)).isNotNull();
+ }
+
+ @Test
+ public void testRegisterViewRejectsMetadataFileOutsideAllowedLocations() {
+ LocalIcebergCatalog catalog = catalog();
+ Namespace namespace = Namespace.of("register_view_disallowed_metadata");
+ TableIdentifier view = TableIdentifier.of(namespace, "view");
+ if (requiresNamespaceCreate()) {
+ catalog.createNamespace(namespace);
+ }
+
+ String forbiddenMetadataLocation = "s3://forbidden-bucket/view/metadata/v1.metadata.json";
+
+ Assertions.assertThatThrownBy(() -> catalog.registerView(view, forbiddenMetadataLocation))
+ .isInstanceOf(ForbiddenException.class)
+ .hasMessageContaining("Invalid location");
+ }
+
+ @Test
+ public void testRegisterViewRejectsViewLocationOutsideAllowedLocations() {
+ LocalIcebergCatalog catalog = catalog();
+ Namespace namespace = Namespace.of("register_view_disallowed_view_location");
+ TableIdentifier view = TableIdentifier.of(namespace, "view");
+ if (requiresNamespaceCreate()) {
+ catalog.createNamespace(namespace);
+ }
+ // Metadata file is at an allowed location, but the view location inside it is forbidden.
+ String metadataLocation =
+ STORAGE_LOCATION + "/register_view_disallowed_view_location/view/metadata/v1.metadata.json";
+ fileIO.addFile(
+ metadataLocation,
+ ViewMetadataParser.toJson(createSampleViewMetadata("s3://forbidden-bucket/view/"))
+ .getBytes(UTF_8));
+
+ Assertions.assertThatThrownBy(() -> catalog.registerView(view, metadataLocation))
+ .isInstanceOf(ForbiddenException.class)
+ .hasMessageContaining("Invalid location");
+ }
+
+ @Test
+ public void testRegisterViewRejectsMetadataFileOutsideViewLocation() {
+ updateCatalogProperties(
+ Map.of(
+ FeatureConfiguration.ALLOW_EXTERNAL_METADATA_FILE_LOCATION.catalogConfig(), "false"));
+
+ LocalIcebergCatalog catalog = catalog();
+ Namespace namespace = Namespace.of("register_view_metadata_outside_view_location");
+ TableIdentifier view = TableIdentifier.of(namespace, "view");
+ if (requiresNamespaceCreate()) {
+ catalog.createNamespace(namespace);
+ }
+
+ String viewLocation =
+ STORAGE_LOCATION + "/register_view_metadata_outside_view_location/other_view/";
+ String metadataLocation =
+ STORAGE_LOCATION
+ + "/register_view_metadata_outside_view_location/view/metadata/v1.metadata.json";
+ fileIO.addFile(
+ metadataLocation,
+ ViewMetadataParser.toJson(createSampleViewMetadata(viewLocation)).getBytes(UTF_8));
+
+ Assertions.assertThatThrownBy(() -> catalog.registerView(view, metadataLocation))
+ .isInstanceOf(BadRequestException.class)
+ .hasMessageContaining("is not allowed outside of table location");
+ }
+
+ @Test
+ public void testRegisterViewAllowsMetadataFileOutsideViewLocationWhenFlagEnabled() {
+ updateCatalogProperties(
+ Map.of(FeatureConfiguration.ALLOW_EXTERNAL_METADATA_FILE_LOCATION.catalogConfig(), "true"));
+
+ LocalIcebergCatalog catalog = catalog();
+ Namespace namespace = Namespace.of("register_view_metadata_outside_view_location_allowed");
+ TableIdentifier view = TableIdentifier.of(namespace, "view");
+ if (requiresNamespaceCreate()) {
+ catalog.createNamespace(namespace);
+ }
+
+ String viewLocation =
+ STORAGE_LOCATION + "/register_view_metadata_outside_view_location_allowed/other_view/";
+ String metadataLocation =
+ STORAGE_LOCATION
+ + "/register_view_metadata_outside_view_location_allowed/view/metadata/v1.metadata.json";
+ fileIO.addFile(
+ metadataLocation,
+ ViewMetadataParser.toJson(createSampleViewMetadata(viewLocation)).getBytes(UTF_8));
+
+ Assertions.assertThat(catalog.registerView(view, metadataLocation)).isNotNull();
+ }
+
@Test
public void testRegisterTableRejectsMetadataLocationOutsideAllowedLocations() {
FileIOFactory fileIOFactorySpy = spy(fileIOFactory);