You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@polaris.apache.org by yz...@apache.org on 2026/04/17 21:16:04 UTC
(polaris) branch yzou-generic-table-credential-vending updated: revert location check
This is an automated email from the ASF dual-hosted git repository.
gh-yzou pushed a commit to branch yzou-generic-table-credential-vending
in repository https://gitbox.apache.org/repos/asf/polaris.git
The following commit(s) were added to refs/heads/yzou-generic-table-credential-vending by this push:
new 2e215f667f revert location check
2e215f667f is described below
commit 2e215f667f1003f9361de084be287fbc3ac826ca
Author: Yun Zou <yu...@gmail.com>
AuthorDate: Fri Apr 17 14:15:57 2026 -0700
revert location check
---
.../service/catalog/common/CatalogUtils.java | 25 ---
.../generic/PolarisGenericTableCatalog.java | 6 -
.../service/catalog/iceberg/IcebergCatalog.java | 51 +++--
.../service/catalog/common/CatalogUtilsTest.java | 206 ---------------------
.../AbstractPolarisGenericTableCatalogTest.java | 29 +--
5 files changed, 40 insertions(+), 277 deletions(-)
diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/common/CatalogUtils.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/common/CatalogUtils.java
index c80ca4ec9b..3a859d2773 100644
--- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/common/CatalogUtils.java
+++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/common/CatalogUtils.java
@@ -98,31 +98,6 @@ public class CatalogUtils {
ResolvedPathKey.ofNamespace(tableIdentifier.namespace()));
}
- /**
- * Validates that the specified {@code location} is valid for whatever storage config is found for
- * this TableLike's parent hierarchy. Resolves the storage entity from the given entity view.
- */
- public static void validateLocationForTableLike(
- PolarisResolutionManifestCatalogView resolvedEntityView,
- RealmConfig realmConfig,
- TableIdentifier identifier,
- String location) {
- PolarisResolvedPathWrapper resolvedStorageEntity =
- resolvedEntityView.getResolvedPath(
- ResolvedPathKey.ofTableLike(identifier), PolarisEntitySubType.ANY_SUBTYPE);
- if (resolvedStorageEntity == null) {
- resolvedStorageEntity =
- resolvedEntityView.getResolvedPath(ResolvedPathKey.ofNamespace(identifier.namespace()));
- }
- if (resolvedStorageEntity == null) {
- resolvedStorageEntity =
- resolvedEntityView.getPassthroughResolvedPath(
- ResolvedPathKey.ofNamespace(identifier.namespace()));
- }
-
- validateLocationsForTableLike(realmConfig, identifier, Set.of(location), resolvedStorageEntity);
- }
-
/**
* Validates that the specified {@code locations} are valid for whatever storage config is found
* for the given entity's parent hierarchy.
diff --git a/runtime/service/src/main/java/org/apache/polaris/service/catalog/generic/PolarisGenericTableCatalog.java b/runtime/service/src/main/java/org/apache/polaris/service/catalog/generic/PolarisGenericTableCatalog.java
index f4c128c499..b232f0e3a4 100644
--- a/runtime/service/src/main/java/org/apache/polaris/service/catalog/generic/PolarisGenericTableCatalog.java
+++ b/runtime/service/src/main/java/org/apache/polaris/service/catalog/generic/PolarisGenericTableCatalog.java
@@ -42,7 +42,6 @@ import org.apache.polaris.core.persistence.dao.entity.EntityResult;
import org.apache.polaris.core.persistence.pagination.PageToken;
import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifestCatalogView;
import org.apache.polaris.core.persistence.resolver.ResolvedPathKey;
-import org.apache.polaris.service.catalog.common.CatalogUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -90,11 +89,6 @@ public class PolarisGenericTableCatalog implements GenericTableCatalog {
"Failed to fetch resolved parent for TableIdentifier '%s'", tableIdentifier));
}
- if (baseLocation != null && !baseLocation.isEmpty()) {
- CatalogUtils.validateLocationForTableLike(
- resolvedEntityView, callContext.getRealmConfig(), tableIdentifier, baseLocation);
- }
-
List<PolarisEntity> catalogPath = resolvedParent.getRawFullPath();
PolarisResolvedPathWrapper resolvedEntities =
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 1801039d1c..28b6ab12b1 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
@@ -1021,6 +1021,39 @@ public class IcebergCatalog extends BaseMetastoreViewCatalog
tableIdentifier, applyReplaceNewLocationWithCatalogDefault(location));
}
+ /**
+ * Validates that the specified {@code location} is valid for whatever storage config is found for
+ * this TableLike's parent hierarchy.
+ */
+ private void validateLocationForTableLike(TableIdentifier identifier, String location) {
+ PolarisResolvedPathWrapper resolvedStorageEntity =
+ resolvedEntityView.getResolvedPath(
+ ResolvedPathKey.ofTableLike(identifier), PolarisEntitySubType.ANY_SUBTYPE);
+ if (resolvedStorageEntity == null) {
+ resolvedStorageEntity =
+ resolvedEntityView.getResolvedPath(ResolvedPathKey.ofNamespace(identifier.namespace()));
+ }
+ if (resolvedStorageEntity == null) {
+ resolvedStorageEntity =
+ resolvedEntityView.getPassthroughResolvedPath(
+ ResolvedPathKey.ofNamespace(identifier.namespace()));
+ }
+
+ validateLocationForTableLike(identifier, location, resolvedStorageEntity);
+ }
+
+ /**
+ * Validates that the specified {@code location} is valid for whatever storage config is found for
+ * this TableLike's parent hierarchy.
+ */
+ private void validateLocationForTableLike(
+ TableIdentifier identifier,
+ String location,
+ PolarisResolvedPathWrapper resolvedStorageEntity) {
+ CatalogUtils.validateLocationsForTableLike(
+ realmConfig, identifier, Set.of(location), resolvedStorageEntity);
+ }
+
/**
* Validates the table location has no overlap with other entities after checking the
* configuration of the service
@@ -1939,8 +1972,7 @@ public class IcebergCatalog extends BaseMetastoreViewCatalog
if (base == null || !metadata.location().equals(base.location())) {
// If location is changing then we must validate that the requested location is valid
// for the storage configuration inherited under this entity's path.
- CatalogUtils.validateLocationsForTableLike(
- realmConfig, identifier, Set.of(metadata.location()), resolvedStorageEntity);
+ validateLocationForTableLike(identifier, metadata.location(), resolvedStorageEntity);
validateNoLocationOverlap(
catalogEntity,
identifier,
@@ -2329,8 +2361,7 @@ public class IcebergCatalog extends BaseMetastoreViewCatalog
// Make sure the metadata file is valid for our allowed locations.
String metadataLocation = icebergTableLikeEntity.getMetadataLocation();
- CatalogUtils.validateLocationsForTableLike(
- realmConfig, identifier, Set.of(metadataLocation), resolvedParent);
+ validateLocationForTableLike(identifier, metadataLocation, resolvedParent);
List<PolarisEntity> catalogPath = resolvedParent.getRawFullPath();
@@ -2396,8 +2427,7 @@ public class IcebergCatalog extends BaseMetastoreViewCatalog
// Make sure the metadata file is valid for our allowed locations.
String metadataLocation = icebergTableLikeEntity.getMetadataLocation();
- CatalogUtils.validateLocationsForTableLike(
- realmConfig, identifier, Set.of(metadataLocation), resolvedEntities);
+ validateLocationForTableLike(identifier, metadataLocation, resolvedEntities);
List<PolarisEntity> catalogPath = resolvedEntities.getRawParentPath();
EntityResult res =
@@ -2513,8 +2543,7 @@ public class IcebergCatalog extends BaseMetastoreViewCatalog
// Validate location against the resolvedStorageEntity
String metadataLocation =
transformTableLikeLocation(tableIdentifier, request.getPayload().getMetadataLocation());
- CatalogUtils.validateLocationsForTableLike(
- realmConfig, tableIdentifier, Set.of(metadataLocation), resolvedStorageEntity);
+ validateLocationForTableLike(tableIdentifier, metadataLocation, resolvedStorageEntity);
// Validate that we can construct a FileIO
String locationDir = metadataLocation.substring(0, metadataLocation.lastIndexOf("/"));
@@ -2572,8 +2601,7 @@ public class IcebergCatalog extends BaseMetastoreViewCatalog
.build();
}
// first validate we can read the metadata file
- CatalogUtils.validateLocationForTableLike(
- resolvedEntityView, realmConfig, tableIdentifier, newLocation);
+ validateLocationForTableLike(tableIdentifier, newLocation);
String locationDir = newLocation.substring(0, newLocation.lastIndexOf("/"));
@@ -2590,8 +2618,7 @@ public class IcebergCatalog extends BaseMetastoreViewCatalog
TableMetadata tableMetadata = TableMetadataParser.read(fileIO, newLocation);
// then validate that it points to a valid location for this table
- CatalogUtils.validateLocationForTableLike(
- resolvedEntityView, realmConfig, tableIdentifier, tableMetadata.location());
+ validateLocationForTableLike(tableIdentifier, tableMetadata.location());
// finally, validate that the metadata file is within the table directory
validateMetadataFileInTableDir(tableIdentifier, tableMetadata);
diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/common/CatalogUtilsTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/common/CatalogUtilsTest.java
deleted file mode 100644
index 26130c318c..0000000000
--- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/common/CatalogUtilsTest.java
+++ /dev/null
@@ -1,206 +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.service.catalog.common;
-
-import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.eq;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
-
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import org.apache.iceberg.catalog.Namespace;
-import org.apache.iceberg.catalog.TableIdentifier;
-import org.apache.iceberg.exceptions.ForbiddenException;
-import org.apache.polaris.core.admin.model.Catalog;
-import org.apache.polaris.core.config.FeatureConfiguration;
-import org.apache.polaris.core.config.PolarisConfiguration;
-import org.apache.polaris.core.config.RealmConfig;
-import org.apache.polaris.core.entity.CatalogEntity;
-import org.apache.polaris.core.entity.PolarisEntity;
-import org.apache.polaris.core.entity.PolarisEntityConstants;
-import org.apache.polaris.core.entity.PolarisEntitySubType;
-import org.apache.polaris.core.entity.PolarisEntityType;
-import org.apache.polaris.core.persistence.PolarisResolvedPathWrapper;
-import org.apache.polaris.core.persistence.ResolvedPolarisEntity;
-import org.apache.polaris.core.persistence.resolver.PolarisResolutionManifestCatalogView;
-import org.apache.polaris.core.persistence.resolver.ResolvedPathKey;
-import org.apache.polaris.core.storage.aws.AwsStorageConfigurationInfo;
-import org.assertj.core.api.Assertions;
-import org.junit.jupiter.api.BeforeEach;
-import org.junit.jupiter.api.Test;
-
-public class CatalogUtilsTest {
-
- private static final String ALLOWED_LOCATION = "s3://my-bucket/path/to/data";
- private static final Namespace NS = Namespace.of("ns");
- private static final TableIdentifier TABLE = TableIdentifier.of(NS, "t1");
-
- private RealmConfig realmConfig;
- private PolarisResolvedPathWrapper resolvedPathWrapper;
-
- @BeforeEach
- void setUp() {
- realmConfig = mock(RealmConfig.class);
- when(realmConfig.getConfig(any(PolarisConfiguration.class))).thenReturn(false);
- when(realmConfig.getConfig(any(PolarisConfiguration.class), any(CatalogEntity.class)))
- .thenReturn(false);
- when(realmConfig.getConfig(
- eq(FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION), any(CatalogEntity.class)))
- .thenReturn(true);
-
- AwsStorageConfigurationInfo storageConfig =
- AwsStorageConfigurationInfo.builder()
- .roleARN("arn:aws:iam::012345678901:role/testrole")
- .addAllowedLocation(ALLOWED_LOCATION)
- .build();
-
- PolarisEntity catalogEntity =
- new CatalogEntity.Builder()
- .setName("test-catalog")
- .setId(1L)
- .setCatalogId(1L)
- .setParentId(0L)
- .setCreateTimestamp(System.currentTimeMillis())
- .setCatalogType(Catalog.TypeEnum.INTERNAL.name())
- .setInternalProperties(
- Map.of(
- PolarisEntityConstants.getStorageConfigInfoPropertyName(),
- storageConfig.serialize(),
- CatalogEntity.CATALOG_TYPE_PROPERTY,
- Catalog.TypeEnum.INTERNAL.name()))
- .setProperties(Map.of(PolarisEntityConstants.ENTITY_BASE_LOCATION, ALLOWED_LOCATION))
- .build();
-
- PolarisEntity nsEntity =
- new PolarisEntity.Builder()
- .setName("ns")
- .setType(PolarisEntityType.NAMESPACE)
- .setId(2L)
- .setCatalogId(1L)
- .setParentId(1L)
- .setCreateTimestamp(System.currentTimeMillis())
- .build();
-
- List<ResolvedPolarisEntity> resolvedPath =
- List.of(
- new ResolvedPolarisEntity(catalogEntity, null, null),
- new ResolvedPolarisEntity(nsEntity, null, null));
- resolvedPathWrapper = new PolarisResolvedPathWrapper(resolvedPath);
- }
-
- @Test
- void testValidateLocationsForTableLike_validLocation() {
- Assertions.assertThatCode(
- () ->
- CatalogUtils.validateLocationsForTableLike(
- realmConfig, TABLE, Set.of(ALLOWED_LOCATION + "/ns/t1"), resolvedPathWrapper))
- .doesNotThrowAnyException();
- }
-
- @Test
- void testValidateLocationsForTableLike_locationOutsideAllowed() {
- Assertions.assertThatThrownBy(
- () ->
- CatalogUtils.validateLocationsForTableLike(
- realmConfig, TABLE, Set.of("s3://other-bucket/path"), resolvedPathWrapper))
- .isInstanceOf(ForbiddenException.class);
- }
-
- @Test
- void testValidateLocationsForTableLike_fileLocationRejected() {
- Assertions.assertThatThrownBy(
- () ->
- CatalogUtils.validateLocationsForTableLike(
- realmConfig, TABLE, Set.of("file:///tmp/data"), resolvedPathWrapper))
- .isInstanceOf(ForbiddenException.class);
- }
-
- @Test
- void testValidateLocationsForTableLike_multipleLocationsOneInvalid() {
- Assertions.assertThatThrownBy(
- () ->
- CatalogUtils.validateLocationsForTableLike(
- realmConfig,
- TABLE,
- Set.of(ALLOWED_LOCATION + "/ns/t1", "s3://unauthorized/path"),
- resolvedPathWrapper))
- .isInstanceOf(ForbiddenException.class);
- }
-
- @Test
- void testValidateLocationForTableLike_resolvesViaTableLikePath() {
- PolarisResolutionManifestCatalogView entityView = mock();
- when(entityView.getResolvedPath(
- eq(ResolvedPathKey.ofTableLike(TABLE)), eq(PolarisEntitySubType.ANY_SUBTYPE)))
- .thenReturn(resolvedPathWrapper);
-
- Assertions.assertThatCode(
- () ->
- CatalogUtils.validateLocationForTableLike(
- entityView, realmConfig, TABLE, ALLOWED_LOCATION + "/ns/t1"))
- .doesNotThrowAnyException();
- }
-
- @Test
- void testValidateLocationForTableLike_fallsBackToNamespacePath() {
- PolarisResolutionManifestCatalogView entityView = mock();
- when(entityView.getResolvedPath(
- eq(ResolvedPathKey.ofTableLike(TABLE)), eq(PolarisEntitySubType.ANY_SUBTYPE)))
- .thenReturn(null);
- when(entityView.getResolvedPath(ResolvedPathKey.ofNamespace(NS)))
- .thenReturn(resolvedPathWrapper);
-
- Assertions.assertThatCode(
- () ->
- CatalogUtils.validateLocationForTableLike(
- entityView, realmConfig, TABLE, ALLOWED_LOCATION + "/ns/t1"))
- .doesNotThrowAnyException();
- }
-
- @Test
- void testValidateLocationForTableLike_fallsBackToPassthroughPath() {
- PolarisResolutionManifestCatalogView entityView = mock();
- when(entityView.getResolvedPath(any(), any())).thenReturn(null);
- when(entityView.getResolvedPath(any(ResolvedPathKey.class))).thenReturn(null);
- when(entityView.getPassthroughResolvedPath(ResolvedPathKey.ofNamespace(NS)))
- .thenReturn(resolvedPathWrapper);
-
- Assertions.assertThatCode(
- () ->
- CatalogUtils.validateLocationForTableLike(
- entityView, realmConfig, TABLE, ALLOWED_LOCATION + "/ns/t1"))
- .doesNotThrowAnyException();
- }
-
- @Test
- void testValidateLocationForTableLike_rejectsInvalidLocation() {
- PolarisResolutionManifestCatalogView entityView = mock();
- when(entityView.getResolvedPath(
- eq(ResolvedPathKey.ofTableLike(TABLE)), eq(PolarisEntitySubType.ANY_SUBTYPE)))
- .thenReturn(resolvedPathWrapper);
-
- Assertions.assertThatThrownBy(
- () ->
- CatalogUtils.validateLocationForTableLike(
- entityView, realmConfig, TABLE, "s3://unauthorized-bucket/data"))
- .isInstanceOf(ForbiddenException.class);
- }
-}
diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/generic/AbstractPolarisGenericTableCatalogTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/generic/AbstractPolarisGenericTableCatalogTest.java
index f29f49dbd7..4372cc2f00 100644
--- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/generic/AbstractPolarisGenericTableCatalogTest.java
+++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/generic/AbstractPolarisGenericTableCatalogTest.java
@@ -34,7 +34,6 @@ import org.apache.iceberg.CatalogProperties;
import org.apache.iceberg.Schema;
import org.apache.iceberg.catalog.Namespace;
import org.apache.iceberg.catalog.TableIdentifier;
-import org.apache.iceberg.exceptions.ForbiddenException;
import org.apache.iceberg.types.Types;
import org.apache.polaris.core.PolarisCallContext;
import org.apache.polaris.core.PolarisDiagnostics;
@@ -296,7 +295,7 @@ public abstract class AbstractPolarisGenericTableCatalogTest {
@ParameterizedTest
@NullSource
- @ValueSource(strings = {"", "s3://my-bucket/path/to/data/ns/my-table"})
+ @ValueSource(strings = {"", "file://path/to/my/table"})
public void testGenericTableRoundTrip(String baseLocation) {
Namespace namespace = Namespace.of("ns");
icebergCatalog.createNamespace(namespace);
@@ -318,32 +317,6 @@ public abstract class AbstractPolarisGenericTableCatalogTest {
Assertions.assertThat(resultEntity.getBaseLocation()).isEqualTo(baseLocation);
}
- @ParameterizedTest
- @ValueSource(strings = {"file://path/to/my/table", "s3://unauthorized-bucket/path"})
- public void testCreateGenericTableWithInvalidLocation(String baseLocation) {
- Namespace namespace = Namespace.of("ns");
- icebergCatalog.createNamespace(namespace);
-
- Assertions.assertThatThrownBy(
- () ->
- genericTableCatalog.createGenericTable(
- TableIdentifier.of("ns", "t1"), "format", baseLocation, "doc", Map.of()))
- .isInstanceOf(ForbiddenException.class);
- }
-
- @Test
- public void testCreateGenericTableWithValidLocation() {
- Namespace namespace = Namespace.of("ns");
- icebergCatalog.createNamespace(namespace);
-
- String baseLocation = "s3://my-bucket/path/to/data/ns/valid-table";
- GenericTableEntity entity =
- genericTableCatalog.createGenericTable(
- TableIdentifier.of("ns", "t1"), "format", baseLocation, "doc", Map.of());
-
- Assertions.assertThat(entity.getBaseLocation()).isEqualTo(baseLocation);
- }
-
@Test
public void testLoadNonExistentTable() {
Namespace namespace = Namespace.of("ns");