diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/exception/CatalogExceptionMessage.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/exception/CatalogExceptionMessage.java index 4b855207440..8e37aed9993 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/exception/CatalogExceptionMessage.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/exception/CatalogExceptionMessage.java @@ -183,4 +183,8 @@ public final class CatalogExceptionMessage { public static String failedToEvaluate(String message) { return String.format("Failed to evaluate - %s", message); } + + public static String deletionNotAllowed(String entityType, String name) { + return String.format("Deletion of %s %s is not allowed", entityType, name); + } } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/PolicyRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/PolicyRepository.java index a6d91d3cac9..b129ee44015 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/PolicyRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/PolicyRepository.java @@ -13,6 +13,7 @@ package org.openmetadata.catalog.jdbi3; +import static java.lang.Boolean.FALSE; import static org.openmetadata.catalog.Entity.FIELD_DESCRIPTION; import static org.openmetadata.catalog.Entity.FIELD_OWNER; import static org.openmetadata.catalog.Entity.LOCATION; @@ -139,6 +140,13 @@ public class PolicyRepository extends EntityRepository { return new PolicyUpdater(original, updated, operation); } + @Override + protected void preDelete(Policy entity) { + if (FALSE.equals(entity.getAllowDelete())) { + throw new IllegalArgumentException(CatalogExceptionMessage.deletionNotAllowed(Entity.POLICY, entity.getName())); + } + } + public void validateRules(Policy policy) throws IOException { // Resolve JSON blobs into Rule object and perform schema based validation List rules = policy.getRules(); diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/RoleRepository.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/RoleRepository.java index 34d131127f8..cbd2054d8f0 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/RoleRepository.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/RoleRepository.java @@ -13,6 +13,7 @@ package org.openmetadata.catalog.jdbi3; +import static java.lang.Boolean.FALSE; import static org.openmetadata.catalog.Entity.POLICIES; import static org.openmetadata.catalog.util.EntityUtil.entityReferenceMatch; import static org.openmetadata.common.utils.CommonUtil.listOrEmpty; @@ -110,6 +111,13 @@ public class RoleRepository extends EntityRepository { return new RoleUpdater(original, updated, operation); } + @Override + protected void preDelete(Role entity) { + if (FALSE.equals(entity.getAllowDelete())) { + throw new IllegalArgumentException(CatalogExceptionMessage.deletionNotAllowed(Entity.ROLE, entity.getName())); + } + } + /** Handles entity updated from PUT and POST operation. */ public class RoleUpdater extends EntityUpdater { public RoleUpdater(Role original, Role updated, Operation operation) { diff --git a/catalog-rest-service/src/main/resources/json/data/policy/DataConsumerPolicy.json b/catalog-rest-service/src/main/resources/json/data/policy/DataConsumerPolicy.json index d982650f866..e808e279c13 100644 --- a/catalog-rest-service/src/main/resources/json/data/policy/DataConsumerPolicy.json +++ b/catalog-rest-service/src/main/resources/json/data/policy/DataConsumerPolicy.json @@ -4,6 +4,7 @@ "fullyQualifiedName": "DataConsumerPolicy", "description": "Policy for Data Consumer to perform operations on metadata entities", "enabled": true, + "allowDelete": false, "rules": [ { "name": "DataConsumerPolicy-EditRule", diff --git a/catalog-rest-service/src/main/resources/json/data/policy/DataStewardPolicy.json b/catalog-rest-service/src/main/resources/json/data/policy/DataStewardPolicy.json index 0000191418c..7a217080591 100644 --- a/catalog-rest-service/src/main/resources/json/data/policy/DataStewardPolicy.json +++ b/catalog-rest-service/src/main/resources/json/data/policy/DataStewardPolicy.json @@ -4,6 +4,7 @@ "fullyQualifiedName": "DataStewardPolicy", "description": "Policy for Data Steward Role to perform operations on metadata entities", "enabled": true, + "allowDelete": false, "rules": [ { "name": "DataStewardPolicy-EditRule", diff --git a/catalog-rest-service/src/main/resources/json/data/policy/OrganizationPolicy.json b/catalog-rest-service/src/main/resources/json/data/policy/OrganizationPolicy.json index d337fcd7659..c4cb55adaf3 100644 --- a/catalog-rest-service/src/main/resources/json/data/policy/OrganizationPolicy.json +++ b/catalog-rest-service/src/main/resources/json/data/policy/OrganizationPolicy.json @@ -4,6 +4,7 @@ "fullyQualifiedName": "OrganizationPolicy", "description": "Policy for all the users of an organization.", "enabled": true, + "allowDelete": false, "rules": [ { "name": "OrganizationPolicy-Owner-Rule", diff --git a/catalog-rest-service/src/main/resources/json/data/policy/TeamOnlyPolicy.json b/catalog-rest-service/src/main/resources/json/data/policy/TeamOnlyPolicy.json index 703f803022b..4937463edca 100644 --- a/catalog-rest-service/src/main/resources/json/data/policy/TeamOnlyPolicy.json +++ b/catalog-rest-service/src/main/resources/json/data/policy/TeamOnlyPolicy.json @@ -4,6 +4,7 @@ "fullyQualifiedName": "TeamOnlyPolicy", "description": "Policy when attached to a team allows only users with in the team hierarchy to access the resources.", "enabled": true, + "allowDelete": false, "rules": [ { "name": "TeamOnlyPolicy-Rule", diff --git a/catalog-rest-service/src/main/resources/json/data/role/DataConsumer.json b/catalog-rest-service/src/main/resources/json/data/role/DataConsumer.json index 7aa9bef2c9e..7c02af3e8ec 100644 --- a/catalog-rest-service/src/main/resources/json/data/role/DataConsumer.json +++ b/catalog-rest-service/src/main/resources/json/data/role/DataConsumer.json @@ -2,6 +2,7 @@ "name": "DataConsumer", "displayName": "Data Consumer", "description": "Users with Data Consumer role use different data assets for their day to day work.", + "allowDelete": false, "policies" : [ { "type" : "policy", diff --git a/catalog-rest-service/src/main/resources/json/data/role/DataSteward.json b/catalog-rest-service/src/main/resources/json/data/role/DataSteward.json index e3cac44e537..003482e62e8 100644 --- a/catalog-rest-service/src/main/resources/json/data/role/DataSteward.json +++ b/catalog-rest-service/src/main/resources/json/data/role/DataSteward.json @@ -2,7 +2,7 @@ "name": "DataSteward", "displayName": "Data Steward", "description": "Users with Data Steward role are responsible for ensuring correctness of metadata for data assets, thereby facilitating data governance principles within the organization.
Data Stewards can update metadata for any entity.", - "deleted": false, + "allowDelete": false, "policies" : [ { "type" : "policy", diff --git a/catalog-rest-service/src/main/resources/json/schema/entity/policies/policy.json b/catalog-rest-service/src/main/resources/json/schema/entity/policies/policy.json index cd76efb87a7..ea0df0d72d0 100644 --- a/catalog-rest-service/src/main/resources/json/schema/entity/policies/policy.json +++ b/catalog-rest-service/src/main/resources/json/schema/entity/policies/policy.json @@ -83,6 +83,14 @@ "$ref": "../../type/entityReference.json", "default": null }, + "allowDelete" : { + "description": "Some system policies can't be deleted", + "type" : "boolean" + }, + "allowEdit" : { + "description": "Some system roles can't be edited", + "type" : "boolean" + }, "deleted": { "description": "When `true` indicates the entity has been soft deleted.", "type": "boolean", diff --git a/catalog-rest-service/src/main/resources/json/schema/entity/teams/role.json b/catalog-rest-service/src/main/resources/json/schema/entity/teams/role.json index a6cd4ffbe87..44800217bd1 100644 --- a/catalog-rest-service/src/main/resources/json/schema/entity/teams/role.json +++ b/catalog-rest-service/src/main/resources/json/schema/entity/teams/role.json @@ -51,6 +51,14 @@ "description": "Change that lead to this version of the entity.", "$ref": "../../type/entityHistory.json#/definitions/changeDescription" }, + "allowDelete" : { + "description": "Some system roles can't be deleted", + "type" : "boolean" + }, + "allowEdit" : { + "description": "Some system roles can't be edited", + "type" : "boolean" + }, "deleted": { "description": "When `true` indicates the entity has been soft deleted.", "type": "boolean", diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/policies/PolicyResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/policies/PolicyResourceTest.java index 8c9df3282cd..19fd3953fd9 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/policies/PolicyResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/policies/PolicyResourceTest.java @@ -59,6 +59,7 @@ import org.openmetadata.catalog.entity.policies.accessControl.Rule; import org.openmetadata.catalog.entity.policies.accessControl.Rule.Effect; import org.openmetadata.catalog.entity.teams.Role; import org.openmetadata.catalog.entity.teams.Team; +import org.openmetadata.catalog.exception.CatalogExceptionMessage; import org.openmetadata.catalog.resources.CollectionRegistry; import org.openmetadata.catalog.resources.EntityResourceTest; import org.openmetadata.catalog.resources.locations.LocationResourceTest; @@ -232,6 +233,20 @@ public class PolicyResourceTest extends EntityResourceTest failsToEvaluate(policyName, "abc"); } + @Test + void delete_Disallowed() { + List policies = new ArrayList<>(DATA_CONSUMER_ROLE.getPolicies()); + policies.addAll(DATA_STEWARD_ROLE.getPolicies()); + policies.add(TEAM_ONLY_POLICY.getEntityReference()); + + for (EntityReference policy : policies) { + assertResponse( + () -> deleteEntity(policy.getId(), ADMIN_AUTH_HEADERS), + BAD_REQUEST, + CatalogExceptionMessage.deletionNotAllowed(Entity.POLICY, policy.getName())); + } + } + private void failsToParse(String policyName, String condition) { validateCondition(policyName, condition, "Failed to parse"); } diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/RoleResourceTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/RoleResourceTest.java index 552afe62359..74e2d799182 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/RoleResourceTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/teams/RoleResourceTest.java @@ -136,6 +136,16 @@ public class RoleResourceTest extends EntityResourceTest { CatalogExceptionMessage.EMPTY_POLICIES_IN_ROLE); } + @Test + void delete_Disallowed() { + for (Role role : List.of(DATA_CONSUMER_ROLE, DATA_STEWARD_ROLE)) { + assertResponse( + () -> deleteEntity(role.getId(), ADMIN_AUTH_HEADERS), + BAD_REQUEST, + CatalogExceptionMessage.deletionNotAllowed(Entity.ROLE, role.getName())); + } + } + private static void validateRole( Role role, String expectedDescription, String expectedDisplayName, String expectedUpdatedBy) { assertListNotNull(role.getId(), role.getHref());