This is an automated email from the ASF dual-hosted git repository.
yufei 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 6a4d2a18c Policy Store: Check whether Policy is in use before dropping
and support `detach-all` flag (#1467)
6a4d2a18c is described below
commit 6a4d2a18c379df83cea2a8f0588fb145ad262fb7
Author: Honah (Jonas) J. <hon...@apache.org>
AuthorDate: Tue Apr 29 01:29:17 2025 -0500
Policy Store: Check whether Policy is in use before dropping and support
`detach-all` flag (#1467)
---
.../apache/polaris/service/it/env/PolicyApi.java | 15 +++++-
.../test/PolarisPolicyServiceIntegrationTest.java | 16 +++++-
.../AtomicOperationMetaStoreManager.java | 36 +++++++++++++-
.../core/persistence/dao/entity/BaseResult.java | 3 ++
.../TransactionalMetaStoreManagerImpl.java | 37 +++++++++++++-
.../TreeMapTransactionalPersistenceImpl.java | 4 +-
...PolicyValidator.java => PolicyMappingUtil.java} | 22 ++++----
.../policy/exceptions/PolicyInUseException.java | 34 +++++++++++++
.../BaseMaintenancePolicyValidator.java | 23 +--------
.../BasePolarisMetaStoreManagerTest.java | 5 ++
.../persistence/PolarisTestMetaStoreManager.java | 58 ++++++++++++++++++++++
.../service/quarkus/catalog/PolicyCatalogTest.java | 25 ++++++++++
.../service/catalog/policy/PolicyCatalog.java | 9 +++-
.../service/exception/PolarisExceptionMapper.java | 3 ++
14 files changed, 250 insertions(+), 40 deletions(-)
diff --git
a/integration-tests/src/main/java/org/apache/polaris/service/it/env/PolicyApi.java
b/integration-tests/src/main/java/org/apache/polaris/service/it/env/PolicyApi.java
index 7597aaef3..6061ef8c9 100644
---
a/integration-tests/src/main/java/org/apache/polaris/service/it/env/PolicyApi.java
+++
b/integration-tests/src/main/java/org/apache/polaris/service/it/env/PolicyApi.java
@@ -28,6 +28,7 @@ import java.util.Map;
import org.apache.iceberg.catalog.Namespace;
import org.apache.iceberg.rest.RESTUtil;
import org.apache.polaris.core.policy.PolicyType;
+import org.apache.polaris.core.policy.exceptions.PolicyInUseException;
import org.apache.polaris.service.types.ApplicablePolicy;
import org.apache.polaris.service.types.AttachPolicyRequest;
import org.apache.polaris.service.types.CreatePolicyRequest;
@@ -72,12 +73,24 @@ public class PolicyApi extends RestApi {
}
public void dropPolicy(String catalog, PolicyIdentifier policyIdentifier) {
+ dropPolicy(catalog, policyIdentifier, null);
+ }
+
+ public void dropPolicy(String catalog, PolicyIdentifier policyIdentifier,
Boolean detachAll) {
String ns = RESTUtil.encodeNamespace(policyIdentifier.getNamespace());
+ Map<String, String> queryParams = new HashMap<>();
+ if (detachAll != null) {
+ queryParams.put("detach-all", detachAll.toString());
+ }
try (Response res =
request(
"polaris/v1/{cat}/namespaces/{ns}/policies/{policy}",
- Map.of("cat", catalog, "ns", ns, "policy",
policyIdentifier.getName()))
+ Map.of("cat", catalog, "ns", ns, "policy",
policyIdentifier.getName()),
+ queryParams)
.delete()) {
+ if (res.getStatus() == Response.Status.BAD_REQUEST.getStatusCode()) {
+ throw new PolicyInUseException("Policy in use");
+ }
Assertions.assertThat(res.getStatus()).isEqualTo(Response.Status.NO_CONTENT.getStatusCode());
}
}
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 055fa411c..2b833ba74 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
@@ -50,6 +50,7 @@ import org.apache.polaris.core.admin.model.StorageConfigInfo;
import org.apache.polaris.core.catalog.PolarisCatalogHelpers;
import org.apache.polaris.core.entity.CatalogEntity;
import org.apache.polaris.core.policy.PredefinedPolicyTypes;
+import org.apache.polaris.core.policy.exceptions.PolicyInUseException;
import org.apache.polaris.service.it.env.ClientCredentials;
import org.apache.polaris.service.it.env.IcebergHelper;
import org.apache.polaris.service.it.env.IntegrationTestsHelper;
@@ -268,8 +269,21 @@ public class PolarisPolicyServiceIntegrationTest {
PredefinedPolicyTypes.DATA_COMPACTION,
EXAMPLE_TABLE_MAINTENANCE_POLICY_CONTENT,
"test policy");
- policyApi.dropPolicy(currentCatalogName, NS1_P1);
+
+ PolicyAttachmentTarget catalogTarget =
+
PolicyAttachmentTarget.builder().setType(PolicyAttachmentTarget.TypeEnum.CATALOG).build();
+ policyApi.attachPolicy(currentCatalogName, NS1_P1, catalogTarget,
Map.of());
+
+ // dropPolicy should fail because the policy is attached to the catalog
+ Assertions.assertThatThrownBy(() ->
policyApi.dropPolicy(currentCatalogName, NS1_P1))
+ .isInstanceOf(PolicyInUseException.class);
+
+ // with detach-all=true, the policy and the attachment should be dropped
+ policyApi.dropPolicy(currentCatalogName, NS1_P1, true);
Assertions.assertThat(policyApi.listPolicies(currentCatalogName,
NS1)).hasSize(0);
+ // The policy mapping record should be dropped
+ Assertions.assertThat(policyApi.getApplicablePolicies(currentCatalogName,
null, null, null))
+ .hasSize(0);
}
@Test
diff --git
a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java
b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java
index d6cfd8429..2170550dd 100644
---
a/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java
+++
b/polaris-core/src/main/java/org/apache/polaris/core/persistence/AtomicOperationMetaStoreManager.java
@@ -64,6 +64,7 @@ import
org.apache.polaris.core.persistence.dao.entity.ResolvedEntityResult;
import org.apache.polaris.core.persistence.dao.entity.ScopedCredentialsResult;
import org.apache.polaris.core.policy.PolarisPolicyMappingRecord;
import org.apache.polaris.core.policy.PolicyEntity;
+import org.apache.polaris.core.policy.PolicyMappingUtil;
import org.apache.polaris.core.policy.PolicyType;
import org.apache.polaris.core.storage.PolarisCredentialProperty;
import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo;
@@ -191,6 +192,27 @@ public class AtomicOperationMetaStoreManager extends
BaseMetaStoreManager {
ms.loadAllGrantRecordsOnSecurable(callCtx, entity.getCatalogId(),
entity.getId());
ms.deleteAllEntityGrantRecords(callCtx, entity, grantsOnGrantee,
grantsOnSecurable);
+ if (entity.getType() == PolarisEntityType.POLICY
+ || PolicyMappingUtil.isValidTargetEntityType(entity.getType(),
entity.getSubType())) {
+ // Best-effort cleanup - for policy and potential target entities, drop
all policy mapping
+ // records related
+ // TODO: Support some more formal garbage-collection mechanism similar
to grant records case
+ // above
+ try {
+ final List<PolarisPolicyMappingRecord> mappingOnPolicy =
+ (entity.getType() == PolarisEntityType.POLICY)
+ ? ms.loadAllTargetsOnPolicy(callCtx, entity.getCatalogId(),
entity.getId())
+ : List.of();
+ final List<PolarisPolicyMappingRecord> mappingOnTarget =
+ (entity.getType() == PolarisEntityType.POLICY)
+ ? List.of()
+ : ms.loadAllPoliciesOnTarget(callCtx, entity.getCatalogId(),
entity.getId());
+ ms.deleteAllEntityPolicyMappingRecords(callCtx, entity,
mappingOnTarget, mappingOnPolicy);
+ } catch (UnsupportedOperationException e) {
+ // Policy mapping persistence not implemented, but we should not block
dropping entities
+ }
+ }
+
// Now determine the set of entities on the other side of the grants we
just removed. Grants
// from/to these entities has been removed, hence we need to update the
grant version of
// each entity. Collect the id of each.
@@ -1177,6 +1199,18 @@ public class AtomicOperationMetaStoreManager extends
BaseMetaStoreManager {
callCtx, null, refreshEntityToDrop.getCatalogId(),
refreshEntityToDrop.getId())) {
return new
DropEntityResult(BaseResult.ReturnStatus.NAMESPACE_NOT_EMPTY, null);
}
+ } else if (refreshEntityToDrop.getType() == PolarisEntityType.POLICY &&
!cleanup) {
+ try {
+ // need to check if the policy is attached to any entity
+ List<PolarisPolicyMappingRecord> records =
+ ms.loadAllTargetsOnPolicy(
+ callCtx, refreshEntityToDrop.getCatalogId(),
refreshEntityToDrop.getId());
+ if (!records.isEmpty()) {
+ return new
DropEntityResult(BaseResult.ReturnStatus.POLICY_HAS_MAPPINGS, null);
+ }
+ } catch (UnsupportedOperationException e) {
+ // Policy mapping persistence not implemented, but we should not block
dropping entities
+ }
}
// simply delete that entity. Will be removed from entities_active, added
to the
@@ -1186,7 +1220,7 @@ public class AtomicOperationMetaStoreManager extends
BaseMetaStoreManager {
// if cleanup, schedule a cleanup task for the entity. do this here, so
that drop and scheduling
// the cleanup task is transactional. Otherwise, we'll be unable to
schedule the cleanup task
// later
- if (cleanup) {
+ if (cleanup && refreshEntityToDrop.getType() != PolarisEntityType.POLICY) {
PolarisBaseEntity taskEntity =
new PolarisEntity.Builder()
.setId(ms.generateNewId(callCtx))
diff --git
a/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/BaseResult.java
b/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/BaseResult.java
index a4eee22cc..a8f3d5aef 100644
---
a/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/BaseResult.java
+++
b/polaris-core/src/main/java/org/apache/polaris/core/persistence/dao/entity/BaseResult.java
@@ -117,6 +117,9 @@ public class BaseResult {
// policy mapping of same type already exists
POLICY_MAPPING_OF_SAME_TYPE_ALREADY_EXISTS(15),
+
+ // policy has mappings and cannot be dropped
+ POLICY_HAS_MAPPINGS(16),
;
// code for the enum
diff --git
a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java
b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java
index 22120c7a3..9f508eb4e 100644
---
a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java
+++
b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TransactionalMetaStoreManagerImpl.java
@@ -65,6 +65,7 @@ import
org.apache.polaris.core.persistence.dao.entity.ResolvedEntityResult;
import org.apache.polaris.core.persistence.dao.entity.ScopedCredentialsResult;
import org.apache.polaris.core.policy.PolarisPolicyMappingRecord;
import org.apache.polaris.core.policy.PolicyEntity;
+import org.apache.polaris.core.policy.PolicyMappingUtil;
import org.apache.polaris.core.policy.PolicyType;
import org.apache.polaris.core.storage.PolarisCredentialProperty;
import org.apache.polaris.core.storage.PolarisStorageConfigurationInfo;
@@ -193,6 +194,28 @@ public class TransactionalMetaStoreManagerImpl extends
BaseMetaStoreManager {
ms.writeEntityInCurrentTxn(callCtx, entityGrantChanged, false,
originalEntity);
}
+ if (entity.getType() == PolarisEntityType.POLICY
+ || PolicyMappingUtil.isValidTargetEntityType(entity.getType(),
entity.getSubType())) {
+ // Best-effort cleanup - for policy and potential target entities, drop
all policy mapping
+ // records related
+ try {
+ final List<PolarisPolicyMappingRecord> mappingOnPolicy =
+ (entity.getType() == PolarisEntityType.POLICY)
+ ? ms.loadAllTargetsOnPolicyInCurrentTxn(
+ callCtx, entity.getCatalogId(), entity.getId())
+ : List.of();
+ final List<PolarisPolicyMappingRecord> mappingOnTarget =
+ (entity.getType() == PolarisEntityType.POLICY)
+ ? List.of()
+ : ms.loadAllPoliciesOnTargetInCurrentTxn(
+ callCtx, entity.getCatalogId(), entity.getId());
+ ms.deleteAllEntityPolicyMappingRecordsInCurrentTxn(
+ callCtx, entity, mappingOnTarget, mappingOnPolicy);
+ } catch (UnsupportedOperationException e) {
+ // Policy mapping persistence not implemented, but we should not block
dropping entities
+ }
+ }
+
// remove the entity being dropped now
ms.deleteEntityInCurrentTxn(callCtx, entity);
@@ -1359,6 +1382,18 @@ public class TransactionalMetaStoreManagerImpl extends
BaseMetaStoreManager {
callCtx, null, refreshEntityToDrop.getCatalogId(),
refreshEntityToDrop.getId())) {
return new
DropEntityResult(BaseResult.ReturnStatus.NAMESPACE_NOT_EMPTY, null);
}
+ } else if (refreshEntityToDrop.getType() == PolarisEntityType.POLICY &&
!cleanup) {
+ // need to check if the policy is attached to any entity
+ try {
+ List<PolarisPolicyMappingRecord> records =
+ ms.loadAllTargetsOnPolicyInCurrentTxn(
+ callCtx, refreshEntityToDrop.getCatalogId(),
refreshEntityToDrop.getId());
+ if (!records.isEmpty()) {
+ return new
DropEntityResult(BaseResult.ReturnStatus.POLICY_HAS_MAPPINGS, null);
+ }
+ } catch (UnsupportedOperationException e) {
+ // Policy mapping persistence not implemented, but we should not block
dropping entities
+ }
}
// simply delete that entity. Will be removed from entities_active, added
to the
@@ -1368,7 +1403,7 @@ public class TransactionalMetaStoreManagerImpl extends
BaseMetaStoreManager {
// if cleanup, schedule a cleanup task for the entity. do this here, so
that drop and scheduling
// the cleanup task is transactional. Otherwise, we'll be unable to
schedule the cleanup task
// later
- if (cleanup) {
+ if (cleanup && refreshEntityToDrop.getType() != PolarisEntityType.POLICY) {
PolarisBaseEntity taskEntity =
new PolarisEntity.Builder()
.setId(ms.generateNewIdInCurrentTxn(callCtx))
diff --git
a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java
b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java
index e9cbd53f3..39ce364d3 100644
---
a/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java
+++
b/polaris-core/src/main/java/org/apache/polaris/core/persistence/transactional/TreeMapTransactionalPersistenceImpl.java
@@ -589,9 +589,9 @@ public class TreeMapTransactionalPersistenceImpl extends
AbstractTransactionalPe
// also delete the other side. We need to delete these mapping one at a
time versus doing a
// range delete
- mappingOnTarget.forEach(record ->
this.store.getSlicePolicyMappingRecords().delete(record));
- mappingOnPolicy.forEach(
+ mappingOnTarget.forEach(
record ->
this.store.getSlicePolicyMappingRecordsByPolicy().delete(record));
+ mappingOnPolicy.forEach(record ->
this.store.getSlicePolicyMappingRecords().delete(record));
}
/** {@inheritDoc} */
diff --git
a/polaris-core/src/main/java/org/apache/polaris/core/policy/validator/maintenance/BaseMaintenancePolicyValidator.java
b/polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyMappingUtil.java
similarity index 73%
copy from
polaris-core/src/main/java/org/apache/polaris/core/policy/validator/maintenance/BaseMaintenancePolicyValidator.java
copy to
polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyMappingUtil.java
index 86308f555..3bf1296c2 100644
---
a/polaris-core/src/main/java/org/apache/polaris/core/policy/validator/maintenance/BaseMaintenancePolicyValidator.java
+++
b/polaris-core/src/main/java/org/apache/polaris/core/policy/PolicyMappingUtil.java
@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.polaris.core.policy.validator.maintenance;
+package org.apache.polaris.core.policy;
import static org.apache.polaris.core.entity.PolarisEntityType.CATALOG;
import static org.apache.polaris.core.entity.PolarisEntityType.NAMESPACE;
@@ -25,21 +25,21 @@ import static
org.apache.polaris.core.entity.PolarisEntityType.TABLE_LIKE;
import java.util.Set;
import org.apache.polaris.core.entity.PolarisEntitySubType;
import org.apache.polaris.core.entity.PolarisEntityType;
-import org.apache.polaris.core.policy.validator.InvalidPolicyException;
-import org.apache.polaris.core.policy.validator.PolicyValidator;
-public class BaseMaintenancePolicyValidator implements PolicyValidator {
- public static final BaseMaintenancePolicyValidator INSTANCE =
- new BaseMaintenancePolicyValidator();
+public class PolicyMappingUtil {
private static final Set<PolarisEntityType> ATTACHABLE_ENTITY_TYPES =
Set.of(CATALOG, NAMESPACE, TABLE_LIKE);
- @Override
- public void validate(String content) throws InvalidPolicyException {}
-
- @Override
- public boolean canAttach(PolarisEntityType entityType, PolarisEntitySubType
entitySubType) {
+ /**
+ * Checks if the given entity type and subtype are valid targets for policy
attachment.
+ *
+ * <p>This method excludes non-attachable entities such as catalog-role and
task. Each policy type
+ * may impose additional specific rules * to determine whether it can target
a particular entity
+ * type or subtype.
+ */
+ public static boolean isValidTargetEntityType(
+ PolarisEntityType entityType, PolarisEntitySubType entitySubType) {
if (entityType == null) {
return false;
}
diff --git
a/polaris-core/src/main/java/org/apache/polaris/core/policy/exceptions/PolicyInUseException.java
b/polaris-core/src/main/java/org/apache/polaris/core/policy/exceptions/PolicyInUseException.java
new file mode 100644
index 000000000..14d165497
--- /dev/null
+++
b/polaris-core/src/main/java/org/apache/polaris/core/policy/exceptions/PolicyInUseException.java
@@ -0,0 +1,34 @@
+/*
+ * 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.policy.exceptions;
+
+import com.google.errorprone.annotations.FormatMethod;
+import org.apache.polaris.core.exceptions.PolarisException;
+
+public class PolicyInUseException extends PolarisException {
+ @FormatMethod
+ public PolicyInUseException(String message, Object... args) {
+ super(String.format(message, args));
+ }
+
+ @FormatMethod
+ public PolicyInUseException(Throwable cause, String message, Object... args)
{
+ super(String.format(message, args), cause);
+ }
+}
diff --git
a/polaris-core/src/main/java/org/apache/polaris/core/policy/validator/maintenance/BaseMaintenancePolicyValidator.java
b/polaris-core/src/main/java/org/apache/polaris/core/policy/validator/maintenance/BaseMaintenancePolicyValidator.java
index 86308f555..f5a93e8ca 100644
---
a/polaris-core/src/main/java/org/apache/polaris/core/policy/validator/maintenance/BaseMaintenancePolicyValidator.java
+++
b/polaris-core/src/main/java/org/apache/polaris/core/policy/validator/maintenance/BaseMaintenancePolicyValidator.java
@@ -18,13 +18,9 @@
*/
package org.apache.polaris.core.policy.validator.maintenance;
-import static org.apache.polaris.core.entity.PolarisEntityType.CATALOG;
-import static org.apache.polaris.core.entity.PolarisEntityType.NAMESPACE;
-import static org.apache.polaris.core.entity.PolarisEntityType.TABLE_LIKE;
-
-import java.util.Set;
import org.apache.polaris.core.entity.PolarisEntitySubType;
import org.apache.polaris.core.entity.PolarisEntityType;
+import org.apache.polaris.core.policy.PolicyMappingUtil;
import org.apache.polaris.core.policy.validator.InvalidPolicyException;
import org.apache.polaris.core.policy.validator.PolicyValidator;
@@ -32,26 +28,11 @@ public class BaseMaintenancePolicyValidator implements
PolicyValidator {
public static final BaseMaintenancePolicyValidator INSTANCE =
new BaseMaintenancePolicyValidator();
- private static final Set<PolarisEntityType> ATTACHABLE_ENTITY_TYPES =
- Set.of(CATALOG, NAMESPACE, TABLE_LIKE);
-
@Override
public void validate(String content) throws InvalidPolicyException {}
@Override
public boolean canAttach(PolarisEntityType entityType, PolarisEntitySubType
entitySubType) {
- if (entityType == null) {
- return false;
- }
-
- if (!ATTACHABLE_ENTITY_TYPES.contains(entityType)) {
- return false;
- }
-
- if (entityType == TABLE_LIKE && entitySubType !=
PolarisEntitySubType.ICEBERG_TABLE) {
- return false;
- }
-
- return true;
+ return PolicyMappingUtil.isValidTargetEntityType(entityType,
entitySubType);
}
}
diff --git
a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BasePolarisMetaStoreManagerTest.java
b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BasePolarisMetaStoreManagerTest.java
index d11bed82d..f58313712 100644
---
a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BasePolarisMetaStoreManagerTest.java
+++
b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/BasePolarisMetaStoreManagerTest.java
@@ -292,6 +292,11 @@ public abstract class BasePolarisMetaStoreManagerTest {
polarisTestMetaStoreManager.testPolicyMapping();
}
+ @Test
+ protected void testPolicyMappingCleanup() {
+ polarisTestMetaStoreManager.testPolicyMappingCleanup();
+ }
+
@Test
protected void testLoadTasks() {
for (int i = 0; i < 20; i++) {
diff --git
a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java
b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java
index b401d4efb..428ecf00c 100644
---
a/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java
+++
b/polaris-core/src/testFixtures/java/org/apache/polaris/core/persistence/PolarisTestMetaStoreManager.java
@@ -835,6 +835,12 @@ public class PolarisTestMetaStoreManager {
Assertions.assertThat(dropResult.isSuccess()).isFalse();
Assertions.assertThat(dropResult.failedBecauseNotEmpty()).isTrue();
Assertions.assertThat(dropResult.isEntityUnDroppable()).isFalse();
+ } else if (entityToDrop.getType() == PolarisEntityType.POLICY) {
+ // When dropping policy with cleanup = true, we do not need cleanup task
+ Assertions.assertThat(dropResult.isSuccess()).isEqualTo(exists);
+ Assertions.assertThat(dropResult.failedBecauseNotEmpty()).isFalse();
+ Assertions.assertThat(dropResult.isEntityUnDroppable()).isFalse();
+ Assertions.assertThat(dropResult.getCleanupTaskId()).isNull();
} else {
Assertions.assertThat(dropResult.isSuccess()).isEqualTo(exists);
Assertions.assertThat(dropResult.failedBecauseNotEmpty()).isFalse();
@@ -2788,4 +2794,56 @@ public class PolarisTestMetaStoreManager {
detachPolicyFromTarget(List.of(catalog, N1, N1_N2), N1_N2_T1,
List.of(catalog, N1), N1_P1);
detachPolicyFromTarget(List.of(catalog, N1, N1_N2), N1_N2_T1,
List.of(catalog, N5), N5_P3);
}
+
+ void testPolicyMappingCleanup() {
+ PolarisBaseEntity catalog = this.createTestCatalog("test");
+ Assertions.assertThat(catalog).isNotNull();
+
+ PolarisBaseEntity N1 =
+ this.ensureExistsByName(List.of(catalog), PolarisEntityType.NAMESPACE,
"N1");
+ PolarisBaseEntity N1_N2 =
+ this.ensureExistsByName(List.of(catalog, N1),
PolarisEntityType.NAMESPACE, "N2");
+ PolarisBaseEntity N1_N2_T1 =
+ this.ensureExistsByName(
+ List.of(catalog, N1, N1_N2),
+ PolarisEntityType.TABLE_LIKE,
+ PolarisEntitySubType.ANY_SUBTYPE,
+ "T1");
+
+ PolarisBaseEntity N1_N2_T3 =
+ this.createEntity(
+ List.of(catalog, N1, N1_N2),
+ PolarisEntityType.TABLE_LIKE,
+ PolarisEntitySubType.ICEBERG_TABLE,
+ "T3");
+ PolicyEntity N1_P1 =
+ this.createPolicy(List.of(catalog, N1), "P1",
PredefinedPolicyTypes.DATA_COMPACTION);
+
+ PolicyEntity N1_P2 =
+ this.createPolicy(List.of(catalog, N1), "P2",
PredefinedPolicyTypes.DATA_COMPACTION);
+
+ attachPolicyToTarget(List.of(catalog, N1, N1_N2), N1_N2_T3,
List.of(catalog, N1), N1_P1);
+ LoadPolicyMappingsResult loadPolicyMappingsResult =
+ polarisMetaStoreManager.loadPoliciesOnEntity(polarisCallContext,
N1_N2_T3);
+ Assertions.assertThat(loadPolicyMappingsResult.isSuccess()).isTrue();
+ Assertions.assertThat(loadPolicyMappingsResult.getEntities()).hasSize(1);
+
+ // Drop N1_N2_T1, the corresponding policy mapping should be cleaned-up
+ this.dropEntity(List.of(catalog, N1, N1_N2), N1_N2_T3);
+
+ BasePersistence ms = polarisCallContext.getMetaStore();
+ Assertions.assertThat(
+ ms.loadAllTargetsOnPolicy(polarisCallContext,
N1_P1.getCatalogId(), N1_P1.getId()))
+ .isEmpty();
+
+ attachPolicyToTarget(List.of(catalog, N1, N1_N2), N1_N2_T1,
List.of(catalog, N1), N1_P2);
+
+ // Drop N1_P2, the dropEntity helper will have cleanup enabled to detach
the policy from all
+ // targets
+ this.dropEntity(List.of(catalog, N1), N1_P2);
+ loadPolicyMappingsResult =
+ polarisMetaStoreManager.loadPoliciesOnEntity(polarisCallContext,
N1_N2_T1);
+ Assertions.assertThat(loadPolicyMappingsResult.isSuccess()).isTrue();
+ Assertions.assertThat(loadPolicyMappingsResult.getEntities()).isEmpty();
+ }
}
diff --git
a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolicyCatalogTest.java
b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolicyCatalogTest.java
index 57bd52132..a5bf98ba9 100644
---
a/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolicyCatalogTest.java
+++
b/quarkus/service/src/test/java/org/apache/polaris/service/quarkus/catalog/PolicyCatalogTest.java
@@ -77,6 +77,7 @@ import
org.apache.polaris.core.persistence.dao.entity.PrincipalSecretsResult;
import
org.apache.polaris.core.persistence.transactional.TransactionalPersistence;
import org.apache.polaris.core.policy.PredefinedPolicyTypes;
import org.apache.polaris.core.policy.exceptions.NoSuchPolicyException;
+import org.apache.polaris.core.policy.exceptions.PolicyInUseException;
import
org.apache.polaris.core.policy.exceptions.PolicyVersionMismatchException;
import org.apache.polaris.core.policy.validator.InvalidPolicyException;
import org.apache.polaris.core.secrets.UserSecretsManager;
@@ -506,6 +507,30 @@ public class PolicyCatalogTest {
.isInstanceOf(NoSuchPolicyException.class);
}
+ @Test
+ public void testDropPolicyInUse() {
+ icebergCatalog.createNamespace(NS);
+ policyCatalog.createPolicy(
+ POLICY1, PredefinedPolicyTypes.DATA_COMPACTION.getName(), "test",
"{\"enable\": false}");
+ var target = new
PolicyAttachmentTarget(PolicyAttachmentTarget.TypeEnum.CATALOG, List.of());
+ policyCatalog.attachPolicy(POLICY1, target, null);
+
+ assertThatThrownBy(() -> policyCatalog.dropPolicy(POLICY1, false))
+ .isInstanceOf(PolicyInUseException.class);
+
+ // The policy is still attached to the catalog
+ List<ApplicablePolicy> applicablePolicies =
+ policyCatalog.getApplicablePolicies(null, null, null);
+ assertThat(applicablePolicies.size()).isEqualTo(1);
+
+ // Drop the policy with detach-all flag
+ policyCatalog.dropPolicy(POLICY1, true);
+
+ // The policy should be detached from the catalog and dropped
+ applicablePolicies = policyCatalog.getApplicablePolicies(null, null, null);
+ assertThat(applicablePolicies.size()).isEqualTo(0);
+ }
+
@Test
public void testDropPolicyNotExist() {
icebergCatalog.createNamespace(NS);
diff --git
a/service/common/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java
b/service/common/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java
index 45e5f17e6..066d513a3 100644
---
a/service/common/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java
+++
b/service/common/src/main/java/org/apache/polaris/service/catalog/policy/PolicyCatalog.java
@@ -18,6 +18,7 @@
*/
package org.apache.polaris.service.catalog.policy;
+import static
org.apache.polaris.core.persistence.dao.entity.BaseResult.ReturnStatus.POLICY_HAS_MAPPINGS;
import static
org.apache.polaris.core.persistence.dao.entity.BaseResult.ReturnStatus.POLICY_MAPPING_OF_SAME_TYPE_ALREADY_EXISTS;
import static
org.apache.polaris.service.types.PolicyAttachmentTarget.TypeEnum.CATALOG;
@@ -54,6 +55,7 @@ import org.apache.polaris.core.policy.PolicyEntity;
import org.apache.polaris.core.policy.PolicyType;
import org.apache.polaris.core.policy.exceptions.NoSuchPolicyException;
import org.apache.polaris.core.policy.exceptions.PolicyAttachException;
+import org.apache.polaris.core.policy.exceptions.PolicyInUseException;
import
org.apache.polaris.core.policy.exceptions.PolicyVersionMismatchException;
import org.apache.polaris.core.policy.validator.PolicyValidators;
import org.apache.polaris.service.types.ApplicablePolicy;
@@ -254,7 +256,6 @@ public class PolicyCatalog {
}
public boolean dropPolicy(PolicyIdentifier policyIdentifier, boolean
detachAll) {
- // TODO: Implement detachAll when we support attach/detach policy
var resolvedPolicyPath = getResolvedPathWrapper(policyIdentifier);
var catalogPath = resolvedPolicyPath.getRawParentPath();
var policyEntity = resolvedPolicyPath.getRawLeafEntity();
@@ -265,9 +266,13 @@ public class PolicyCatalog {
PolarisEntity.toCoreList(catalogPath),
policyEntity,
Map.of(),
- false);
+ detachAll);
if (!result.isSuccess()) {
+ if (result.getReturnStatus() == POLICY_HAS_MAPPINGS) {
+ throw new PolicyInUseException("Policy %s is still attached to
entities", policyIdentifier);
+ }
+
throw new IllegalStateException(
String.format(
"Failed to drop policy %s error status: %s with extraInfo: %s",
diff --git
a/service/common/src/main/java/org/apache/polaris/service/exception/PolarisExceptionMapper.java
b/service/common/src/main/java/org/apache/polaris/service/exception/PolarisExceptionMapper.java
index 57df2cbc5..7eac23bd5 100644
---
a/service/common/src/main/java/org/apache/polaris/service/exception/PolarisExceptionMapper.java
+++
b/service/common/src/main/java/org/apache/polaris/service/exception/PolarisExceptionMapper.java
@@ -28,6 +28,7 @@ import org.apache.polaris.core.exceptions.PolarisException;
import org.apache.polaris.core.persistence.PolicyMappingAlreadyExistsException;
import org.apache.polaris.core.policy.exceptions.NoSuchPolicyException;
import org.apache.polaris.core.policy.exceptions.PolicyAttachException;
+import org.apache.polaris.core.policy.exceptions.PolicyInUseException;
import
org.apache.polaris.core.policy.exceptions.PolicyVersionMismatchException;
import org.apache.polaris.core.policy.validator.InvalidPolicyException;
import org.apache.polaris.service.context.UnresolvableRealmContextException;
@@ -59,6 +60,8 @@ public class PolarisExceptionMapper implements
ExceptionMapper<PolarisException>
return Response.Status.CONFLICT;
} else if (exception instanceof PolicyMappingAlreadyExistsException) {
return Response.Status.CONFLICT;
+ } else if (exception instanceof PolicyInUseException) {
+ return Response.Status.BAD_REQUEST;
} else {
return Response.Status.INTERNAL_SERVER_ERROR;
}
