mneethiraj commented on code in PR #1316:
URL: https://github.com/apache/ranger/pull/1316#discussion_r4418460072
##########
agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefHelper.java:
##########
@@ -316,6 +317,66 @@ public Set<List<RangerResourceDef>>
getResourceHierarchies(Integer policyType, C
return ret;
}
+ /**
+ * Resource hierarchies for one constraint. Each access hierarchy
contributes its longest prefix
+ * whose resources are all named on the constraint def. A def that lists
no resources uses the access hierarchy.
+ * A prefix is returned only when it contains every key.
+ */
+ public Set<List<RangerResourceDef>>
getResourceHierarchiesForConstraint(String constraint, Collection<String> keys)
{
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("==> getResourceHierarchiesForConstraint(constraint={},
keys={})", constraint, StringUtils.join(keys, ","));
+ }
+
+ Set<List<RangerResourceDef>> ret = new HashSet<>();
+ RangerConstraintDef constraintDef =
getConstraintDef(constraint);
+
+ if (constraintDef == null) {
+ ret = Collections.emptySet();
+ } else if (CollectionUtils.isEmpty(constraintDef.getResources())) {
+ ret.addAll(getResourceHierarchies(RangerPolicy.POLICY_TYPE_ACCESS,
keys));
+ } else {
+ Set<String> names = new HashSet<>(constraintDef.getResources());
+
+ for (List<RangerResourceDef> hierarchy :
getResourceHierarchies(RangerPolicy.POLICY_TYPE_ACCESS)) {
Review Comment:
Why use resource hierarchies from `POLICY_TYPE_ACCESS`? Can
`constraintDef.getResources()` be used directly?
Also, I suggest pre-computing hierachries for each constraint and caching it
in `RangerServiceDefHelper` - similar to `hierarchies` member for policy types.
##########
agents-common/src/main/java/org/apache/ranger/plugin/policyevaluator/RangerAbstractPolicyEvaluator.java:
##########
@@ -525,6 +525,20 @@ private Map<String, RangerPolicyResource>
getPolicyResourcesWithMacrosReplaced(M
return ret;
}
+ private Set<String> getConstraintNames() {
+ Set<String> ret = new HashSet<>();
Review Comment:
Cache this in `RangerAbstractPolicyEvaluator`?
##########
agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefHelper.java:
##########
@@ -316,6 +317,66 @@ public Set<List<RangerResourceDef>>
getResourceHierarchies(Integer policyType, C
return ret;
}
+ /**
+ * Resource hierarchies for one constraint. Each access hierarchy
contributes its longest prefix
+ * whose resources are all named on the constraint def. A def that lists
no resources uses the access hierarchy.
+ * A prefix is returned only when it contains every key.
+ */
+ public Set<List<RangerResourceDef>>
getResourceHierarchiesForConstraint(String constraint, Collection<String> keys)
{
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("==> getResourceHierarchiesForConstraint(constraint={},
keys={})", constraint, StringUtils.join(keys, ","));
+ }
+
+ Set<List<RangerResourceDef>> ret = new HashSet<>();
+ RangerConstraintDef constraintDef =
getConstraintDef(constraint);
+
+ if (constraintDef == null) {
+ ret = Collections.emptySet();
+ } else if (CollectionUtils.isEmpty(constraintDef.getResources())) {
+ ret.addAll(getResourceHierarchies(RangerPolicy.POLICY_TYPE_ACCESS,
keys));
+ } else {
+ Set<String> names = new HashSet<>(constraintDef.getResources());
+
+ for (List<RangerResourceDef> hierarchy :
getResourceHierarchies(RangerPolicy.POLICY_TYPE_ACCESS)) {
+ List<RangerResourceDef> prefix = new ArrayList<>();
+
+ for (RangerResourceDef resourceDef : hierarchy) {
+ if (!names.contains(resourceDef.getName())) {
+ break;
+ }
+
+ prefix.add(resourceDef);
+ }
+
+ if (CollectionUtils.isNotEmpty(prefix) &&
hierarchyHasAllResources(prefix, keys)) {
+ ret.add(prefix);
+ }
+ }
+ }
+
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("<== getResourceHierarchiesForConstraint(constraint={},
keys={}): {}", constraint, StringUtils.join(keys, ","), StringUtils.join(ret,
""));
+ }
+
+ return ret;
+ }
+
+ private RangerConstraintDef getConstraintDef(String constraint) {
+ RangerConstraintDef ret = null;
+
+ if (StringUtils.isNotEmpty(constraint) &&
CollectionUtils.isNotEmpty(delegate.servicedef.getConstraintDefs())) {
Review Comment:
Instead of iterating at each call, consider caching in
`RangerServiceDefHelper`.
##########
agents-common/src/main/java/org/apache/ranger/plugin/policyengine/RangerPolicyRepository.java:
##########
@@ -954,6 +1010,9 @@ private void init(RangerPolicyEngineOptions options) {
rowFilterPolicyEvaluators.sort(RangerPolicyEvaluator.NAME_COMPARATOR);
this.rowFilterPolicyEvaluators = rowFilterPolicyEvaluators;
+ // Constraint matches are all returned. Do not sort them by policy
priority.
Review Comment:
By keeping `constraintPolicyEvaluators` sorted, the result doesn't need to
be sorted on each authorization call to policy engine.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
