yujunjun/OpenMetadata
1
0
Fork 0
mirror of https://github.com/open-metadata/OpenMetadata.git synced 2025-12-24 14:08:45 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fixes #7013 Backend : Policy rule description is not getting updated (#7027)

Browse Source 
* Fixing test failures

* Fixes #6978  Helper function for building ChangeDescription

* Fixes #7013 Backend : Policy rule description is not getting updated
This commit is contained in:
Suresh Srinivas 2022-08-29 22:03:38 -07:00 committed by GitHub
parent 3c77d49517
commit 1de569af61
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 92 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
catalog-rest-service/src/main/java/org/openmetadata/catalog/jdbi3/PolicyRepository.java
View File

@ -13,14 +13,17 @@
package org.openmetadata.catalog.jdbi3;
import static org.openmetadata.catalog.Entity.FIELD_DESCRIPTION;
import static org.openmetadata.catalog.Entity.FIELD_OWNER;
import static org.openmetadata.catalog.Entity.LOCATION;
import static org.openmetadata.catalog.Entity.POLICY;
import static org.openmetadata.catalog.util.EntityUtil.entityReferenceMatch;
import static org.openmetadata.catalog.util.EntityUtil.getRuleField;
import static org.openmetadata.catalog.util.EntityUtil.resolveRules;
import static org.openmetadata.catalog.util.EntityUtil.ruleMatch;
import static org.openmetadata.common.utils.CommonUtil.listOrEmpty;
import com.fasterxml.jackson.core.JsonProcessingException;
import java.io.IOException;
import java.net.URI;
import java.util.ArrayList;
@ -194,7 +197,7 @@ public class PolicyRepository extends EntityRepository<Policy> {
}
recordChange(ENABLED, original.getEnabled(), updated.getEnabled());
updateLocation(original, updated);
updateRules(original.getRules(), updated.getRules());
updateRules(resolveRules(original.getRules()), resolveRules(updated.getRules()));
}
private void updateLocation(Policy origPolicy, Policy updatedPolicy) throws IOException {
@ -215,12 +218,51 @@ public class PolicyRepository extends EntityRepository<Policy> {
recordChange("location", origPolicy.getLocation(), updatedPolicy.getLocation(), true, entityReferenceMatch);
}
private void updateRules(List<Object> origRules, List<Object> updatedRules) throws IOException {
private void updateRules(List<Rule> origRules, List<Rule> updatedRules) throws IOException {
// Record change description
List<Rule> deletedRules = new ArrayList<>();
List<Rule> addedRules = new ArrayList<>();
recordListChange(
"rules", resolveRules(origRules), resolveRules(updatedRules), addedRules, deletedRules, ruleMatch);
recordListChange("rules", origRules, updatedRules, addedRules, deletedRules, ruleMatch);
// Record changes based on updatedRule
for (Rule updated : updatedRules) {
Rule stored = origRules.stream().filter(c -> ruleMatch.test(c, updated)).findAny().orElse(null);
if (stored == null) { // New Rule added
continue;
}
updateRuleDescription(stored, updated);
updateRuleEffect(stored, updated);
updateRuleOperations(stored, updated);
updateRuleResources(stored, updated);
updateRuleCondition(stored, updated);
}
}
private void updateRuleDescription(Rule stored, Rule updated) throws JsonProcessingException {
String ruleField = getRuleField(stored, FIELD_DESCRIPTION);
recordChange(ruleField, stored.getDescription(), updated.getDescription());
}
private void updateRuleEffect(Rule stored, Rule updated) throws JsonProcessingException {
String ruleField = getRuleField(stored, "effect");
recordChange(ruleField, stored.getEffect(), updated.getEffect());
}
private void updateRuleOperations(Rule stored, Rule updated) throws JsonProcessingException {
String ruleField = getRuleField(stored, "operations");
recordChange(ruleField, stored.getOperations(), updated.getOperations());
}
private void updateRuleResources(Rule stored, Rule updated) throws JsonProcessingException {
String ruleField = getRuleField(stored, "resources");
recordChange(ruleField, stored.getResources(), updated.getResources());
}
private void updateRuleCondition(Rule stored, Rule updated) throws JsonProcessingException {
String ruleField = getRuleField(stored, "condition");
recordChange(ruleField, stored.getCondition(), updated.getCondition());
}
}
}

17
catalog-rest-service/src/main/java/org/openmetadata/catalog/util/EntityUtil.java
View File

@ -143,13 +143,7 @@ public final class EntityUtil {
public static final BiPredicate<CustomProperty, CustomProperty> customFieldMatch =
(ref1, ref2) -> ref1.getName().equals(ref2.getName());
public static final BiPredicate<Rule, Rule> ruleMatch =
(ref1, ref2) ->
ref1.getName().equals(ref2.getName())
&& ref1.getOperations().equals(ref2.getOperations())
&& ref1.getResources().equals(ref2.getResources())
&& ref1.getEffect().equals(ref2.getEffect())
&& Objects.equals(ref1.getCondition(), ref2.getCondition());
public static final BiPredicate<Rule, Rule> ruleMatch = (ref1, ref2) -> ref1.getName().equals(ref2.getName());
private EntityUtil() {}
@ -375,7 +369,7 @@ public final class EntityUtil {
return String.join(Entity.SEPARATOR, strings);
}
/** Return column field name of format columnName:fieldName */
/** Return column field name of format "columns".columnName.columnFieldName */
public static String getColumnField(Table table, Column column, String columnField) {
// Remove table FQN from column FQN to get the local name
String localColumnName =
@ -385,6 +379,13 @@ public final class EntityUtil {
: FullyQualifiedName.build("columns", localColumnName, columnField);
}
/** Return rule field name of format "rules".ruleName.ruleFieldName */
public static String getRuleField(Rule rule, String ruleField) {
return ruleField == null
? FullyQualifiedName.build("rules", rule.getName())
: FullyQualifiedName.build("rules", rule.getName(), ruleField);
}
public static Double nextVersion(Double version) {
return Math.round((version + 0.1) * 10.0) / 10.0;
}

45
catalog-rest-service/src/test/java/org/openmetadata/catalog/resources/policies/PolicyResourceTest.java
View File

@ -16,11 +16,15 @@ package org.openmetadata.catalog.resources.policies;
import static javax.ws.rs.core.Response.Status.BAD_REQUEST;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.openmetadata.catalog.Entity.FIELD_DESCRIPTION;
import static org.openmetadata.catalog.entity.policies.accessControl.Rule.Effect.ALLOW;
import static org.openmetadata.catalog.entity.policies.accessControl.Rule.Effect.DENY;
import static org.openmetadata.catalog.type.MetadataOperation.EDIT_ALL;
import static org.openmetadata.catalog.type.MetadataOperation.VIEW_ALL;
import static org.openmetadata.catalog.util.EntityUtil.fieldAdded;
import static org.openmetadata.catalog.util.EntityUtil.fieldDeleted;
import static org.openmetadata.catalog.util.EntityUtil.fieldUpdated;
import static org.openmetadata.catalog.util.EntityUtil.getRuleField;
import static org.openmetadata.catalog.util.EntityUtil.resolveRules;
import static org.openmetadata.catalog.util.TestUtils.ADMIN_AUTH_HEADERS;
import static org.openmetadata.catalog.util.TestUtils.UpdateType.MINOR_UPDATE;
@ -134,6 +138,12 @@ public class PolicyResourceTest extends EntityResourceTest<Policy, CreatePolicy>
List<Rule> expectedRule = (List<Rule>) expected;
List<Rule> actualRule = resolveRules(JsonUtils.readObjects(actual.toString(), Object.class));
assertEquals(expectedRule, actualRule);
} else if (fieldName.startsWith("rules") && (fieldName.endsWith("effect"))) {
Effect expectedEffect = (Effect) expected;
Effect actualEffect = Effect.fromValue(actual.toString());
assertEquals(expectedEffect, actualEffect);
} else if (fieldName.startsWith("rules") && (fieldName.endsWith("operations"))) {
assertEquals(expected.toString(), actual.toString());
} else {
assertCommonFieldChange(fieldName, expected, actual);
}
@ -265,16 +275,35 @@ public class PolicyResourceTest extends EntityResourceTest<Policy, CreatePolicy>
@Test
void patch_PolicyRules(TestInfo test) throws IOException {
Rule rule1 = accessControlRule("rule1", List.of("all"), List.of(MetadataOperation.VIEW_ALL), ALLOW);
Rule rule1 = accessControlRule("rule1", List.of("all"), List.of(VIEW_ALL), ALLOW);
Policy policy = createAndCheckEntity(createRequest(test).withRules(List.of(rule1)), ADMIN_AUTH_HEADERS);
// Change an existing rule1
// Change existing rule1 fields
String origJson = JsonUtils.pojoToJson(policy);
Rule updatedRule1 = accessControlRule("rule1", List.of("all"), List.of(MetadataOperation.ALL), ALLOW);
policy.setRules(List.of(updatedRule1));
ChangeDescription change = getChangeDescription(policy.getVersion());
fieldDeleted(change, "rules", List.of(rule1));
fieldAdded(change, "rules", List.of(updatedRule1));
rule1
.withDescription("description")
.withEffect(DENY)
.withResources(List.of("table"))
.withOperations(List.of(EDIT_ALL))
.withCondition("isOwner()");
fieldAdded(change, getRuleField(rule1, FIELD_DESCRIPTION), "description");
fieldUpdated(change, getRuleField(rule1, "effect"), ALLOW, DENY);
fieldUpdated(change, getRuleField(rule1, "resources"), List.of("all"), List.of("table"));
fieldUpdated(change, getRuleField(rule1, "operations"), List.of(VIEW_ALL), List.of(EDIT_ALL));
fieldAdded(change, getRuleField(rule1, "condition"), "isOwner()");
policy.setRules(List.of(rule1));
policy = patchEntityAndCheck(policy, origJson, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change);
// Change existing rule1 fields. Update description and condition
origJson = JsonUtils.pojoToJson(policy);
change = getChangeDescription(policy.getVersion());
rule1.withDescription("newDescription").withCondition("noOwner()");
fieldUpdated(change, getRuleField(rule1, FIELD_DESCRIPTION), "description", "newDescription");
fieldUpdated(change, getRuleField(rule1, "condition"), "isOwner()", "noOwner()");
policy.setRules(List.of(rule1));
policy = patchEntityAndCheck(policy, origJson, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change);
// Add a new rule
@ -285,11 +314,11 @@ public class PolicyResourceTest extends EntityResourceTest<Policy, CreatePolicy>
fieldAdded(change, "rules", List.of(newRule));
policy = patchEntityAndCheck(policy, origJson, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change);
// Delete newRule1 rule
// Delete rule1 rule
origJson = JsonUtils.pojoToJson(policy);
policy.setRules(List.of(newRule));
change = getChangeDescription(policy.getVersion());
fieldDeleted(change, "rules", List.of(updatedRule1));
fieldDeleted(change, "rules", List.of(rule1));
patchEntityAndCheck(policy, origJson, ADMIN_AUTH_HEADERS, MINOR_UPDATE, change);
}