You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ranger.apache.org by ma...@apache.org on 2015/05/05 20:14:20 UTC
incubator-ranger git commit: RANGER-359: If policy update does not
change the resource then resource signature check should exclude itself from
policies with matched resources
Repository: incubator-ranger
Updated Branches:
refs/heads/master ee9d78bbb -> 8a9b57ed5
RANGER-359: If policy update does not change the resource then resource signature check should exclude itself from policies with matched resources
Signed-off-by: Madhan Neethiraj <ma...@apache.org>
Project: http://git-wip-us.apache.org/repos/asf/incubator-ranger/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-ranger/commit/8a9b57ed
Tree: http://git-wip-us.apache.org/repos/asf/incubator-ranger/tree/8a9b57ed
Diff: http://git-wip-us.apache.org/repos/asf/incubator-ranger/diff/8a9b57ed
Branch: refs/heads/master
Commit: 8a9b57ed59ddd1d20b5529b9a8336d8e179c5165
Parents: ee9d78b
Author: Alok Lal <al...@hortonworks.com>
Authored: Tue May 5 10:06:59 2015 -0700
Committer: Madhan Neethiraj <ma...@apache.org>
Committed: Tue May 5 11:04:31 2015 -0700
----------------------------------------------------------------------
.../model/validation/RangerPolicyValidator.java | 27 +++++++++--------
.../validation/TestRangerPolicyValidator.java | 31 +++++++++++++++-----
2 files changed, 39 insertions(+), 19 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/8a9b57ed/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyValidator.java
----------------------------------------------------------------------
diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyValidator.java b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyValidator.java
index 0092aaf..7f0318f 100644
--- a/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyValidator.java
+++ b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerPolicyValidator.java
@@ -242,7 +242,7 @@ public class RangerPolicyValidator extends RangerValidator {
boolean valid = true;
Map<String, RangerPolicyResource> resourceMap = policy.getResources();
if (resourceMap != null) { // following checks can't be done meaningfully otherwise
- valid = isPolicyResourceUnique(policy, failures) && valid;
+ valid = isPolicyResourceUnique(policy, failures, action) && valid;
if (serviceDef != null) { // following checks can't be done meaningfully otherwise
valid = isValidResourceNames(policy, failures, serviceDef) && valid;
valid = isValidResourceValues(resourceMap, failures, serviceDef) && valid;
@@ -256,10 +256,10 @@ public class RangerPolicyValidator extends RangerValidator {
return valid;
}
- boolean isPolicyResourceUnique(RangerPolicy policy, final List<ValidationFailureDetails> failures) {
+ boolean isPolicyResourceUnique(RangerPolicy policy, final List<ValidationFailureDetails> failures, Action action) {
if(LOG.isDebugEnabled()) {
- LOG.debug(String.format("==> RangerPolicyValidator.isPolicyResourceUnique(%s, %s)", policy, failures));
+ LOG.debug(String.format("==> RangerPolicyValidator.isPolicyResourceUnique(%s, %s, %s)", policy, failures, action));
}
boolean valid = true;
@@ -267,21 +267,24 @@ public class RangerPolicyValidator extends RangerValidator {
String signature = policySignature.getSignature();
List<RangerPolicy> policies = getPoliciesForResourceSignature(signature);
if (CollectionUtils.isNotEmpty(policies)) {
- RangerPolicy otherPolicy = policies.iterator().next();
- valid = false;
- failures.add(new ValidationFailureDetailsBuilder()
- .field("resources")
- .isSemanticallyIncorrect()
- .becauseOf("found another policy[" + otherPolicy.getName() + "] with matching resources[" + otherPolicy.getResources() + "]!")
- .build());
+ RangerPolicy matchedPolicy = policies.iterator().next();
+ // there shouldn't be a matching policy for create. During update only match should be to itself
+ if (action == Action.CREATE || (action == Action.UPDATE && (policies.size() > 1 || !matchedPolicy.getId().equals(policy.getId())))) {
+ failures.add(new ValidationFailureDetailsBuilder()
+ .field("resources")
+ .isSemanticallyIncorrect()
+ .becauseOf("found another policy[" + matchedPolicy.getName() + "] with matching resources[" + matchedPolicy.getResources() + "]!")
+ .build());
+ valid = false;
+ }
}
if(LOG.isDebugEnabled()) {
- LOG.debug(String.format("<== RangerPolicyValidator.isPolicyResourceUnique(%s, %s): %s", policy, failures, valid));
+ LOG.debug(String.format("<== RangerPolicyValidator.isPolicyResourceUnique(%s, %s, %s): %s", policy, failures, action, valid));
}
return valid;
}
-
+
boolean isValidResourceNames(final RangerPolicy policy, final List<ValidationFailureDetails> failures, final RangerServiceDef serviceDef) {
if(LOG.isDebugEnabled()) {
http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/8a9b57ed/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerPolicyValidator.java
----------------------------------------------------------------------
diff --git a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerPolicyValidator.java b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerPolicyValidator.java
index 1a4f366..d55cdc5 100644
--- a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerPolicyValidator.java
+++ b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerPolicyValidator.java
@@ -677,15 +677,32 @@ public class TestRangerPolicyValidator {
when(_factory.createPolicyResourceSignature(_policy)).thenReturn(signature);
List<RangerPolicy> policies = null;
when(_store.getPoliciesByResourceSignature(hash)).thenReturn(policies);
- assertTrue(_validator.isPolicyResourceUnique(_policy, _failures));
policies = new ArrayList<RangerPolicy>();
- assertTrue(_validator.isPolicyResourceUnique(_policy, _failures));
-
- // if store does have any policy then test should fail with appropriate error message.
- RangerPolicy policy1 = mock(RangerPolicy.class); policies.add(policy1);
- RangerPolicy policy2 = mock(RangerPolicy.class); policies.add(policy2);
+ for (Action action : cu) {
+ assertTrue(_validator.isPolicyResourceUnique(_policy, _failures, action));
+ assertTrue(_validator.isPolicyResourceUnique(_policy, _failures, action));
+ }
+ /*
+ * If store does have any policy then test should fail with appropriate error message.
+ * For create any match is a problem
+ */
+ RangerPolicy policy1 = mock(RangerPolicy.class); policies.add(policy1);
when(_store.getPoliciesByResourceSignature(hash)).thenReturn(policies);
- assertFalse(_validator.isPolicyResourceUnique(_policy, _failures));
+ assertFalse(_validator.isPolicyResourceUnique(_policy, _failures, Action.CREATE));
+ _utils.checkFailureForSemanticError(_failures, "resources");
+ // For Update match with itself is not a problem as long as it isn't itself, i.e. same id.
+ when(policy1.getId()).thenReturn(103L);
+ when(_policy.getId()).thenReturn(103L);
+ assertTrue(_validator.isPolicyResourceUnique(_policy, _failures, Action.UPDATE));
+ // matching policy can't be some other policy (i.e. different id) because that implies a conflict.
+ when(policy1.getId()).thenReturn(104L);
+ assertFalse(_validator.isPolicyResourceUnique(_policy, _failures, Action.UPDATE));
+ _utils.checkFailureForSemanticError(_failures, "resources");
+ // And validation should never pass if there are more than one policies with matching signature, regardless of their ID!!
+ RangerPolicy policy2 = mock(RangerPolicy.class);
+ when(policy2.getId()).thenReturn(103L); // has same id as the policy being tested (_policy)
+ policies.add(policy2);
+ assertFalse(_validator.isPolicyResourceUnique(_policy, _failures, Action.UPDATE));
_utils.checkFailureForSemanticError(_failures, "resources");
}