diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d8b1c9dfc1..844576b217b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,7 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti - Authorization failure messages (HTTP 403 / `ForbiddenException` from `PolarisAuthorizerImpl`) now log the specific missing privilege(s) and the entity each was checked against server-side (at `INFO` level), e.g. `missing TABLE_CREATE on NAMESPACE 'ns1'`. The client-facing 403 response remains a generic message to avoid leaking authorization metadata to untrusted clients. Operators can correlate client errors to server logs using the existing `X-Request-ID` header (present in default log MDC as `requestId`). ### Deprecations +- Deprecated `ALLOW_EXTERNAL_TABLE_LOCATION`. Use `ALLOW_EXTERNAL_METADATA_FILE_LOCATION` for external metadata file locations, including catalog config `polaris.config.allow.external.metadata.file.location`. ### Fixes - Async task execution (table cleanup, manifest and batch file cleanup) now retries when a handler returns false on transient errors (e.g. IO or delete failures). Previously `false` was swallowed with only a warning log and the task was never retried via the existing retry mechanism. diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisPolicyServiceIntegrationTest.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisPolicyServiceIntegrationTest.java index 3126e8c598f..1d56068b8f7 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisPolicyServiceIntegrationTest.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisPolicyServiceIntegrationTest.java @@ -133,7 +133,7 @@ public class PolarisPolicyServiceIntegrationTest { private static final Map DEFAULT_CATALOG_PROPERTIES = Map.of( "polaris.config.allow.unstructured.table.location", "true", - "polaris.config.allow.external.table.location", "true"); + "polaris.config.allow.external.metadata.file.location", "true"); @BeforeAll public static void setup( diff --git a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationBase.java b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationBase.java index ee99b07684e..502e0e3eb55 100644 --- a/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationBase.java +++ b/integration-tests/src/main/java/org/apache/polaris/service/it/test/PolarisRestCatalogIntegrationBase.java @@ -186,7 +186,7 @@ public abstract class PolarisRestCatalogIntegrationBase extends CatalogTests DEFAULT_CATALOG_PROPERTIES = Map.of( "polaris.config.allow.unstructured.table.location", "true", - "polaris.config.allow.external.table.location", "true", + "polaris.config.allow.external.metadata.file.location", "true", "polaris.config.list-pagination-enabled", "true", "polaris.config.namespace-custom-location.enabled", "true"); 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 e9ad80c5ad0..c6dbd7da38a 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 @@ -120,7 +120,8 @@ public void before(TestInfo testInfo) { CatalogProperties props = CatalogProperties.builder(defaultBaseLocation) - .addProperty(FeatureConfiguration.ALLOW_EXTERNAL_TABLE_LOCATION.catalogConfig(), "true") + .addProperty( + FeatureConfiguration.ALLOW_EXTERNAL_METADATA_FILE_LOCATION.catalogConfig(), "true") .addProperty( FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "true") .addProperty(FeatureConfiguration.DROP_WITH_PURGE_ENABLED.catalogConfig(), "true") diff --git a/polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java b/polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java index 0529c25cfa6..571e5e0f435 100644 --- a/polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java +++ b/polaris-core/src/main/java/org/apache/polaris/core/config/FeatureConfiguration.java @@ -335,17 +335,23 @@ public static void enforceFeatureEnabledOrThrow( .defaultValue(false) .buildFeatureConfiguration(); + /** + * @deprecated since 1.7.0, will be removed in 1.8.0. Use {@link + * #ALLOW_EXTERNAL_METADATA_FILE_LOCATION} instead. This legacy flag is retained as a + * compatibility alias for external metadata file locations. + */ + @SuppressWarnings("DeprecatedIsStillUsed") + @Deprecated(since = "1.7.0", forRemoval = true) public static final FeatureConfiguration ALLOW_EXTERNAL_TABLE_LOCATION = PolarisConfiguration.builder() .key("ALLOW_EXTERNAL_TABLE_LOCATION") .catalogConfig("polaris.config.allow.external.table.location") .legacyCatalogConfig("allow.external.table.location") .description( - "If set to true, Polaris treats table locations as externally managed instead of " - + "assuming the default managed structure. Allowed-location validation still " - + "applies, but metadata location checks are relaxed, so operators should keep " - + "allowed locations narrow and specific. This setting is typically used " - + "together with ALLOW_UNSTRUCTURED_TABLE_LOCATION.") + "Deprecated. Use ALLOW_EXTERNAL_METADATA_FILE_LOCATION instead. When enabled, this " + + "legacy compatibility flag relaxes metadata location checks; it does not " + + "control whether table locations may escape the structured namespace layout. " + + "Use ALLOW_UNSTRUCTURED_TABLE_LOCATION for that behavior.") .defaultValue(false) .buildFeatureConfiguration(); 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 52da0099b8f..c839b629055 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 @@ -1227,8 +1227,8 @@ private String applyDefaultLocationObjectStoragePrefix( FeatureConfiguration.DEFAULT_LOCATION_OBJECT_STORAGE_PREFIX_ENABLED.key(), FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.key())); } else if (!allowTableLocationOverlap) { - // TODO consider doing this check any time ALLOW_EXTERNAL_TABLE_LOCATION is enabled, not just - // here + // TODO consider doing this check any time ALLOW_UNSTRUCTURED_TABLE_LOCATION is enabled, not + // just here if (!optimizedSiblingCheck) { throw new IllegalStateException( String.format( @@ -2509,11 +2509,7 @@ private void validateMetadataFileInTableDir(TableIdentifier identifier, TableMet private void validateMetadataFileInTableDir( TableIdentifier identifier, String tableLocation, String metadataLocation) { - boolean allowEscape = - realmConfig.getConfig(FeatureConfiguration.ALLOW_EXTERNAL_TABLE_LOCATION, catalogEntity); - if (!allowEscape - && !realmConfig.getConfig( - FeatureConfiguration.ALLOW_EXTERNAL_METADATA_FILE_LOCATION, catalogEntity)) { + if (!allowExternalMetadataFileLocation()) { LOGGER.debug( "Validating base location {} for table {} in metadata file {}", tableLocation, @@ -2529,6 +2525,18 @@ private void validateMetadataFileInTableDir( } } + @SuppressWarnings({"deprecation", "removal"}) + private boolean allowExternalMetadataFileLocation() { + // ALLOW_EXTERNAL_TABLE_LOCATION is deprecated, but remains honored as a compatibility alias for + // existing catalogs during the migration to ALLOW_EXTERNAL_METADATA_FILE_LOCATION. + CatalogEntity resolvedCatalogEntity = + Optional.ofNullable(resolvedEntityView.getResolvedCatalogEntity()).orElse(catalogEntity); + return realmConfig.getConfig( + FeatureConfiguration.ALLOW_EXTERNAL_METADATA_FILE_LOCATION, resolvedCatalogEntity) + || realmConfig.getConfig( + FeatureConfiguration.ALLOW_EXTERNAL_TABLE_LOCATION, resolvedCatalogEntity); + } + private FileIO loadFileIOForTableLike( TableIdentifier identifier, Set readLocations, 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 013973825e7..fbd0042b9f2 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 @@ -184,7 +184,8 @@ public void before(TestInfo testInfo) { .setName(CATALOG_NAME) .setDefaultBaseLocation(storageLocation) .addProperty( - FeatureConfiguration.ALLOW_EXTERNAL_TABLE_LOCATION.catalogConfig(), "true") + FeatureConfiguration.ALLOW_EXTERNAL_METADATA_FILE_LOCATION.catalogConfig(), + "true") .addProperty( FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "true") diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractLocalIcebergCatalogOverlapTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractLocalIcebergCatalogOverlapTest.java index 9b7503b325b..9a3f40ab151 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractLocalIcebergCatalogOverlapTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractLocalIcebergCatalogOverlapTest.java @@ -164,7 +164,8 @@ public void before(TestInfo testInfo) { .setName(CATALOG_NAME) .setDefaultBaseLocation(STORAGE_LOCATION) .addProperty( - FeatureConfiguration.ALLOW_EXTERNAL_TABLE_LOCATION.catalogConfig(), "true") + FeatureConfiguration.ALLOW_EXTERNAL_METADATA_FILE_LOCATION.catalogConfig(), + "true") .addProperty( FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "true") 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 c69c2bf900a..5b99e50f394 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 @@ -318,7 +318,8 @@ public void before(TestInfo testInfo) { .setName(CATALOG_NAME) .setDefaultBaseLocation(STORAGE_LOCATION) .addProperty( - FeatureConfiguration.ALLOW_EXTERNAL_TABLE_LOCATION.catalogConfig(), + FeatureConfiguration.ALLOW_EXTERNAL_METADATA_FILE_LOCATION + .catalogConfig(), "true") .addProperty( FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), @@ -1229,7 +1230,7 @@ public void testCreateNotificationCreateTableInExternalLocation() { updateCatalogProperties( Map.of( - FeatureConfiguration.ALLOW_EXTERNAL_TABLE_LOCATION.catalogConfig(), "false", + FeatureConfiguration.ALLOW_EXTERNAL_METADATA_FILE_LOCATION.catalogConfig(), "false", FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "true")); LocalIcebergCatalog catalog = catalog(); TableMetadata tableMetadata = @@ -1284,7 +1285,7 @@ public void testCreateNotificationCreateTableOutsideOfMetadataLocation() { updateCatalogProperties( Map.of( - FeatureConfiguration.ALLOW_EXTERNAL_TABLE_LOCATION.catalogConfig(), "false", + FeatureConfiguration.ALLOW_EXTERNAL_METADATA_FILE_LOCATION.catalogConfig(), "false", FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "true")); LocalIcebergCatalog catalog = catalog(); TableMetadata tableMetadata = @@ -1336,7 +1337,7 @@ public void testUpdateNotificationCreateTableInExternalLocation() { updateCatalogProperties( Map.of( - FeatureConfiguration.ALLOW_EXTERNAL_TABLE_LOCATION.catalogConfig(), "false", + FeatureConfiguration.ALLOW_EXTERNAL_METADATA_FILE_LOCATION.catalogConfig(), "false", FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "true")); LocalIcebergCatalog catalog = catalog(); @@ -2008,7 +2009,8 @@ public void testDropTableWithPurgeDisabled() { .setName(noPurgeCatalogName) .setDefaultBaseLocation(storageLocation) .addProperty( - FeatureConfiguration.ALLOW_EXTERNAL_TABLE_LOCATION.catalogConfig(), "true") + FeatureConfiguration.ALLOW_EXTERNAL_METADATA_FILE_LOCATION.catalogConfig(), + "true") .addProperty( FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "true") @@ -2958,7 +2960,7 @@ public void testUpdatePropertiesRejectsOutOfTableWriteMetadataLocation() { updateCatalogProperties( Map.of( - FeatureConfiguration.ALLOW_EXTERNAL_TABLE_LOCATION.catalogConfig(), "false", + FeatureConfiguration.ALLOW_EXTERNAL_METADATA_FILE_LOCATION.catalogConfig(), "false", FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "true")); catalog.createNamespace(NS); @@ -2991,9 +2993,8 @@ public void testUpdatePropertiesAcceptsInTableWriteMetadataLocation() { updateCatalogProperties( Map.of( - FeatureConfiguration.ALLOW_EXTERNAL_TABLE_LOCATION.catalogConfig(), "false", - FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "true", - FeatureConfiguration.ALLOW_EXTERNAL_METADATA_FILE_LOCATION.catalogConfig(), "true")); + FeatureConfiguration.ALLOW_EXTERNAL_METADATA_FILE_LOCATION.catalogConfig(), "true", + FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "true")); catalog.createNamespace(NS); Table table = catalog.buildTable(TABLE, SCHEMA).create(); diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractLocalIcebergCatalogViewTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractLocalIcebergCatalogViewTest.java index b708062c6c0..bc58879d32e 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractLocalIcebergCatalogViewTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/iceberg/AbstractLocalIcebergCatalogViewTest.java @@ -171,7 +171,8 @@ public void before(TestInfo testInfo) { new CatalogEntity.Builder() .setName(CATALOG_NAME) .addProperty( - FeatureConfiguration.ALLOW_EXTERNAL_TABLE_LOCATION.catalogConfig(), "true") + FeatureConfiguration.ALLOW_EXTERNAL_METADATA_FILE_LOCATION.catalogConfig(), + "true") .addProperty( FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "true") diff --git a/runtime/service/src/test/java/org/apache/polaris/service/catalog/policy/AbstractPolicyCatalogTest.java b/runtime/service/src/test/java/org/apache/polaris/service/catalog/policy/AbstractPolicyCatalogTest.java index 6cd0c399728..28a376ba0f0 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/catalog/policy/AbstractPolicyCatalogTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/catalog/policy/AbstractPolicyCatalogTest.java @@ -205,7 +205,8 @@ public void before(TestInfo testInfo) { .setName(CATALOG_NAME) .setDefaultBaseLocation(storageLocation) .addProperty( - FeatureConfiguration.ALLOW_EXTERNAL_TABLE_LOCATION.catalogConfig(), "true") + FeatureConfiguration.ALLOW_EXTERNAL_METADATA_FILE_LOCATION.catalogConfig(), + "true") .addProperty( FeatureConfiguration.ALLOW_UNSTRUCTURED_TABLE_LOCATION.catalogConfig(), "true") diff --git a/site/content/in-dev/unreleased/configuration/config-sections/flags-polaris_features.md b/site/content/in-dev/unreleased/configuration/config-sections/flags-polaris_features.md index 89de9f2b046..34ad7212140 100644 --- a/site/content/in-dev/unreleased/configuration/config-sections/flags-polaris_features.md +++ b/site/content/in-dev/unreleased/configuration/config-sections/flags-polaris_features.md @@ -77,7 +77,7 @@ If set to true, Polaris allows metadata files to be located outside the table's ##### `polaris.features."ALLOW_EXTERNAL_TABLE_LOCATION"` -If set to true, Polaris treats table locations as externally managed instead of assuming the default managed structure. Allowed-location validation still applies, but metadata location checks are relaxed, so operators should keep allowed locations narrow and specific. This setting is typically used together with ALLOW_UNSTRUCTURED_TABLE_LOCATION. +Deprecated. Use ALLOW_EXTERNAL_METADATA_FILE_LOCATION instead. When enabled, this legacy compatibility flag relaxes metadata location checks; it does not control whether table locations may escape the structured namespace layout. Use ALLOW_UNSTRUCTURED_TABLE_LOCATION for that behavior. - **Type:** `Boolean` - **Default:** `false` diff --git a/site/content/in-dev/unreleased/configuration/configuring-polaris-for-production/_index.md b/site/content/in-dev/unreleased/configuration/configuring-polaris-for-production/_index.md index 085248ff459..da71c4e3509 100644 --- a/site/content/in-dev/unreleased/configuration/configuring-polaris-for-production/_index.md +++ b/site/content/in-dev/unreleased/configuration/configuring-polaris-for-production/_index.md @@ -224,10 +224,11 @@ Leave out `FILE` to prevent its use. Only include the storage types your setup n ### Review Location Compatibility Flags Treat non-default location compatibility flags as part of your production deployment review. -`ALLOW_UNSTRUCTURED_TABLE_LOCATION`, `ALLOW_EXTERNAL_TABLE_LOCATION`, -`ALLOW_EXTERNAL_METADATA_FILE_LOCATION`, `ALLOW_TABLE_LOCATION_OVERLAP`, and wildcard allowed -locations all relax the default storage boundary model and should only be enabled for specific -interoperability or migration requirements. `OPTIMIZED_SIBLING_CHECK` is not a bypass mode, but it +`ALLOW_UNSTRUCTURED_TABLE_LOCATION`, `ALLOW_EXTERNAL_METADATA_FILE_LOCATION`, +`ALLOW_TABLE_LOCATION_OVERLAP`, and wildcard allowed locations all relax the default storage +boundary model and should only be enabled for specific interoperability or migration requirements. +`ALLOW_EXTERNAL_TABLE_LOCATION` is a deprecated compatibility alias for +`ALLOW_EXTERNAL_METADATA_FILE_LOCATION`. `OPTIMIZED_SIBLING_CHECK` is not a bypass mode, but it changes how overlap detection is performed and should only be enabled when the required index and backfill state is known to be correct.