Repository: incubator-ranger Updated Branches: refs/heads/master dd547b8bb -> 9adfcbe06
RANGER-437 Policy validation should correctly detect missing policies for service defs with multi-hierarchy resource definitions. Signed-off-by: Madhan Neethiraj <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/incubator-ranger/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-ranger/commit/9adfcbe0 Tree: http://git-wip-us.apache.org/repos/asf/incubator-ranger/tree/9adfcbe0 Diff: http://git-wip-us.apache.org/repos/asf/incubator-ranger/diff/9adfcbe0 Branch: refs/heads/master Commit: 9adfcbe06fc38dcfad25cfbf4ea8515103b1c074 Parents: dd547b8 Author: Alok Lal <[email protected]> Authored: Thu Apr 30 16:23:58 2015 -0700 Committer: Madhan Neethiraj <[email protected]> Committed: Fri May 1 17:34:38 2015 -0700 ---------------------------------------------------------------------- .../model/validation/RangerPolicyValidator.java | 87 ++-- .../validation/RangerServiceDefHelper.java | 398 +++++++++++++++++++ .../model/validation/TestDirectedGraph.java | 77 ++++ .../validation/TestRangerPolicyValidator.java | 108 ++++- .../validation/TestRangerServiceDefHelper.java | 137 +++++++ .../model/validation/TestRangerValidator.java | 14 +- .../model/validation/ValidationTestUtils.java | 91 +++-- 7 files changed, 813 insertions(+), 99 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/9adfcbe0/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 1d7f450..a76e970 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 @@ -38,6 +38,7 @@ import org.apache.ranger.plugin.model.RangerServiceDef; import org.apache.ranger.plugin.model.RangerServiceDef.RangerResourceDef; import org.apache.ranger.plugin.store.ServiceStore; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; public class RangerPolicyValidator extends RangerValidator { @@ -238,14 +239,11 @@ public class RangerPolicyValidator extends RangerValidator { } boolean valid = true; - if (serviceDef != null) { // following checks can't be done meaningfully otherwise -// TODO - disabled till a more robust fix for Hive resources definition can be found -// valid = isValidResourceNames(policy, failures, serviceDef); - Map<String, RangerPolicyResource> resourceMap = policy.getResources(); - if (resourceMap != null) { // following checks can't be done meaningfully otherwise - valid = isValidResourceValues(resourceMap, failures, serviceDef) && valid; - valid = isValidResourceFlags(resourceMap, failures, serviceDef.getResources(), serviceDef.getName(), policy.getName(), isAdmin) && valid; - } + Map<String, RangerPolicyResource> resourceMap = policy.getResources(); + if (serviceDef != null && resourceMap != null) { // following checks can't be done meaningfully otherwise + valid = isValidResourceNames(policy, failures, serviceDef) && valid; + valid = isValidResourceValues(resourceMap, failures, serviceDef) && valid; + valid = isValidResourceFlags(resourceMap, failures, serviceDef.getResources(), serviceDef.getName(), policy.getName(), isAdmin) && valid; } if (StringUtils.isNotBlank(serviceName)) { // resource uniqueness check cannot be done meaningfully otherwise valid = isPolicyResourceUnique(policy, failures, action, serviceName) && valid; @@ -300,28 +298,59 @@ public class RangerPolicyValidator extends RangerValidator { } boolean valid = true; - Set<String> mandatoryResources = getMandatoryResourceNames(serviceDef); Set<String> policyResources = getPolicyResources(policy); - Set<String> missingResources = Sets.difference(mandatoryResources, policyResources); - if (!missingResources.isEmpty()) { - failures.add(new ValidationFailureDetailsBuilder() - .field("resources") - .subField(missingResources.iterator().next()) // we return any one parameter! - .isMissing() - .becauseOf("required resources[" + missingResources + "] are missing") - .build()); - valid = false; - } - Set<String> allResource = getAllResourceNames(serviceDef); - Set<String> unknownResources = Sets.difference(policyResources, allResource); - if (!unknownResources.isEmpty()) { - failures.add(new ValidationFailureDetailsBuilder() - .field("resources") - .subField(unknownResources.iterator().next()) // we return any one parameter! - .isSemanticallyIncorrect() - .becauseOf("resource[" + unknownResources + "] is not valid for service-def[" + serviceDef.getName() + "]") - .build()); - valid = false; + + RangerServiceDefHelper defHelper = new RangerServiceDefHelper(serviceDef); + Set<List<RangerResourceDef>> hierarchies = defHelper.getResourceHierarchies(); // this can be empty but not null! + if (hierarchies.isEmpty()) { + LOG.debug("RangerPolicyValidator.isValidResourceNames: serviceDef does not have any resource hierarchies! Skipping this check!"); + } else { + Iterator<List<RangerResourceDef>> iterator = hierarchies.iterator(); + boolean foundHierarchyMatch = false; + Set<String> missingResourcesCopyForErrorMessage = null; // used to give more helpful error message to the user + while (iterator.hasNext() && !foundHierarchyMatch) { + List<RangerResourceDef> aHierarchy = iterator.next(); + Set<String> mandatoryResources = defHelper.getMandatoryResourceNames(aHierarchy); + Set<String> missingResources = Sets.difference(mandatoryResources, policyResources); + if (missingResources.isEmpty()) { + foundHierarchyMatch = true; + } else { + /* + * Since user does not specify which hierarchy the policy is for, it is tricky to guess her intention and give error message about + * what resource is really missing. For example if only db is speicifed then it is tricky to say if just UDF was missed or TBL and COL were missed. + * So we punt and capture the first hierarchy that does not match. And if policy does not match any hierarchies then we use this cached value + * to report error back to the user. Ideal solution is to require user to specify hierarhcy in policy in case of multi-hierarchy policies. + */ + missingResourcesCopyForErrorMessage = ImmutableSet.copyOf(missingResources); + } + } + /* + * We have evaluated all valid resource hierarchies for the service. But policy did not have mandatory resources required by any of those hierarchies! + */ + if (!foundHierarchyMatch) { + failures.add(new ValidationFailureDetailsBuilder() + .field("resources") + .subField(missingResourcesCopyForErrorMessage.iterator().next()) // We return any one missing resource here! + .isMissing() + .becauseOf("required resources [" + missingResourcesCopyForErrorMessage + "] are missing") + .build()); + valid = false; + } + /* + * Since policy does not specify which hierarchy it belongs to, we can't reliably check if a policy is specifying levels not relevant for it. However, we can + * do a secular check if we got a level that is not applicable to any hierarchy for the service. + */ + Set<String> allResource = defHelper.getResorceMap().keySet(); + Set<String> unknownResources = Sets.difference(policyResources, allResource); + if (!unknownResources.isEmpty()) { + failures.add(new ValidationFailureDetailsBuilder() + .field("resources") + .subField(unknownResources.iterator().next()) // we return any one parameter! + .isSemanticallyIncorrect() + .becauseOf("resource[" + unknownResources + "] is not valid for service-def[" + serviceDef.getName() + "]") + .build()); + valid = false; + } } if(LOG.isDebugEnabled()) { http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/9adfcbe0/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefHelper.java ---------------------------------------------------------------------- diff --git a/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefHelper.java b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefHelper.java new file mode 100644 index 0000000..6381dfe --- /dev/null +++ b/agents-common/src/main/java/org/apache/ranger/plugin/model/validation/RangerServiceDefHelper.java @@ -0,0 +1,398 @@ +/* + * 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.ranger.plugin.model.validation; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.Date; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; + +import org.apache.commons.lang.StringUtils; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.apache.ranger.plugin.model.RangerServiceDef; +import org.apache.ranger.plugin.model.RangerServiceDef.RangerResourceDef; + +public class RangerServiceDefHelper { + + private static final Log LOG = LogFactory.getLog(RangerServiceDefHelper.class); + + static final Map<String, Delegate> _Cache = new ConcurrentHashMap<String, Delegate>(); + final Delegate _delegate; + + public RangerServiceDefHelper(RangerServiceDef serviceDef) { + // NOTE: we assume serviceDef, its name and update time are can never by null. + + if(LOG.isDebugEnabled()) { + LOG.debug(String.format("==> RangerPolicyValidator.isValidResourceNames(%s)", serviceDef)); + } + + String serviceName = serviceDef.getName(); + Date serviceDefFreshnessDate = serviceDef.getUpdateTime(); + + Delegate delegate = null; + if (_Cache.containsKey(serviceName)) { + LOG.debug("RangerServiceDefHelper(): found delegate in cache with matching serviceName. Need to check date"); + Delegate that = _Cache.get(serviceName); + if (Objects.equals(that.getServiceFreshnessDate(), serviceDefFreshnessDate)) { + delegate = that; + LOG.debug("RangerServiceDefHelper(): cached delegate matched in date, too! Will use it now."); + } else { + LOG.debug("RangerServiceDefHelper(): cached delegate date mismatch!"); + } + } + if (delegate == null) { // either not found in cache or date didn't match + delegate = new Delegate(serviceDef); + _Cache.put(serviceName, delegate); + LOG.debug("RangerServiceDefHelper(): Created new delegate and put in delegate cache!"); + } + _delegate = delegate; + } + + /** + * for a resource definition as follows: + * + * /-> E -> F + * A -> B -> C -> D + * \-> G -> H + * + * It would return a set with following ordered entries in it + * { [A B C D], [A E F], [A B G H] } + * + * @return + */ + public Set<List<RangerResourceDef>> getResourceHierarchies() { + return _delegate.getResourceHierarchies(); + } + + /** + * Converts service-def resources from list to a map for constant time lookup + * @return + */ + public Map<String, RangerResourceDef> getResorceMap() { + return _delegate.getResourceMap(); + } + + public Set<String> getMandatoryResourceNames(List<RangerResourceDef> hierarchy) { + Set<String> result = new HashSet<String>(hierarchy.size()); + for (RangerResourceDef resourceDef : hierarchy) { + if (Boolean.TRUE.equals(resourceDef.getMandatory())) { + result.add(resourceDef.getName()); + } + } + return result; + } + + public Set<String> getAllResourceNames(List<RangerResourceDef> hierarchy) { + Set<String> result = new HashSet<String>(hierarchy.size()); + for (RangerResourceDef resourceDef : hierarchy) { + result.add(resourceDef.getName()); + } + return result; + } + + /** + * Not designed for public access. Package level only for testability. + */ + static class Delegate { + + final Set<List<RangerResourceDef>> _hierarchies; + final Date _serviceDefFreshnessDate; + final String _serviceName; + final Map<String, RangerResourceDef> _resourceMap; + + public Delegate(RangerServiceDef serviceDef) { + + // NOTE: we assume serviceDef, its name and update time are can never by null. + _serviceName = serviceDef.getName(); + _serviceDefFreshnessDate = serviceDef.getUpdateTime(); + + // NOTE: we assume resource collection on a service def would never be null + List<RangerResourceDef> resourceDefs = serviceDef.getResources(); + _resourceMap = Collections.unmodifiableMap(getResourcesAsMap(resourceDefs)); + + DirectedGraph graph = createGraph(resourceDefs); + if (isValid(graph)) { + Set<List<String>> hierarchies = getHierarchies(graph); + _hierarchies = Collections.unmodifiableSet(convertHierarchies(hierarchies, _resourceMap)); + } else { + _hierarchies = Collections.unmodifiableSet(new HashSet<List<RangerResourceDef>>()); + } + if (LOG.isDebugEnabled()) { + String message = String.format("Found [%d] resource hierarchies for service [%s] update-date[%s]: %s", _hierarchies.size(), _serviceName, + _serviceDefFreshnessDate == null ? null : _serviceDefFreshnessDate.toString(), _hierarchies); + LOG.debug(message); + } + } + + public Set<List<RangerResourceDef>> getResourceHierarchies() { + return _hierarchies; + } + + public Map<String, RangerResourceDef> getResourceMap() { + return _resourceMap; + } + + public String getServiceName() { + return _serviceName; + } + + public Date getServiceFreshnessDate() { + return _serviceDefFreshnessDate; + } + /** + * Builds a directed graph where each resource is node and arc goes from parent level to child level + * + * @param resourceDefs + * @return + */ + DirectedGraph createGraph(List<RangerResourceDef> resourceDefs) { + DirectedGraph graph = new DirectedGraph(); + for (RangerResourceDef resourceDef : resourceDefs) { + String name = resourceDef.getName(); + graph.add(name); + String parent = resourceDef.getParent(); + if (StringUtils.isNotEmpty(parent)) { + graph.addArc(parent, name); + } + } + if (LOG.isDebugEnabled()) { + LOG.debug("Created graph for resources: " + graph); + } + return graph; + } + + /** + * A minimally valid resource graph has - at least one sink AND - and least one source. + * + * A more rigorous definition would require all of the following: - exactly one source (assuming this is required) - At least one sink - no cycles - all non-source nodes have an in-degree of + * exactly 1 - all non-sink nodes have an out-degree of 1 or more (if more than one source is allowed then this will changed) + * + * Anyhow, we don't need such a rigorous definition at this time, hence a more complete validity function is deferred till we need one! + * + * @param graph + * + * @return + */ + boolean isValid(DirectedGraph graph) { + if (graph.getSources().size() > 0 && graph.getSinks().size() > 0) { + return true; + } else { + return false; + } + } + + /** + * Returns all valid resource hierarchies for the configured resource-defs. Behavior is undefined if it is called on and invalid graph. Use <code>isValid</code> to check validation first. + * + * @param resourceDefs + * @param graph + * @return + */ + Set<List<String>> getHierarchies(DirectedGraph graph) { + Set<List<String>> hierarchies = new HashSet<List<String>>(); + Set<String> sources = graph.getSources(); + Set<String> sinks = graph.getSinks(); + for (String source : sources) { + for (String sink : sinks) { + List<String> path = graph.getAPath(source, sink, new HashSet<String>()); + if (!path.isEmpty()) { + hierarchies.add(path); + } + } + } + return hierarchies; + } + + Set<List<RangerResourceDef>> convertHierarchies(Set<List<String>> hierarchies, Map<String, RangerResourceDef> resourceMap) { + Set<List<RangerResourceDef>> result = new HashSet<List<RangerResourceDef>>(hierarchies.size()); + for (List<String> hierarchy : hierarchies) { + List<RangerResourceDef> resourceList = new ArrayList<RangerResourceDef>(hierarchy.size()); + for (String name : hierarchy) { + RangerResourceDef def = resourceMap.get(name); + resourceList.add(def); + } + result.add(resourceList); + } + return result; + } + + /** + * Converts resource list to resource map for efficient querying + * + * @param resourceList + * @return + */ + Map<String, RangerResourceDef> getResourcesAsMap(List<RangerResourceDef> resourceList) { + Map<String, RangerResourceDef> map = new HashMap<String, RangerResourceDef>(resourceList.size()); + for (RangerResourceDef resourceDef : resourceList) { + map.put(resourceDef.getName(), resourceDef); + } + return map; + } + } + + /** + * Limited DAG implementation to analyze resource graph for a service. Not designed for public access. Package level only for testability. + */ + static class DirectedGraph { + Map<String, Set<String>> _nodes = new HashMap<String, Set<String>>(); + + /** + * Add a node to the graph + * + * @param node + */ + void add(String node) { + if (node == null) { + throw new IllegalArgumentException("Node can't be null!"); + } else if (!_nodes.containsKey(node)) { // don't mess with a node's neighbors if it already exists in the graph + _nodes.put(node, new HashSet<String>()); + } + } + + /** + * Connects node "from" to node "to". Being a directed graph, after this call "to" will be in the list of neighbor's of "from". While the converse need not be true. + * + * @param from + * @param to + */ + void addArc(String from, String to) { + // connecting two nodes, implicitly adds nodes to the graph if they aren't already in it + if (!_nodes.containsKey(from)) { + add(from); + } + if (!_nodes.containsKey(to)) { + add(to); + } + _nodes.get(from).add(to); + } + + /** + * Returns true if "to" is in the list of neighbors of "from" + * + * @param from + * @param to + * @return + */ + boolean hasArc(String from, String to) { + if (_nodes.containsKey(from) && _nodes.containsKey(to) && _nodes.get(from).contains(to)) { + return true; + } else { + return false; + } + } + + /** + * Return the set of nodes with in degree of 0, i.e. those that are not in any other nodes' list of neighbors + * + * @return + */ + Set<String> getSources() { + Set<String> sources = new HashSet<String>(_nodes.keySet()); + for (Map.Entry<String, Set<String>> entry : _nodes.entrySet()) { + Set<String> nbrs = entry.getValue(); // can never be null + sources.removeAll(nbrs); // A source in a DAG can't be a neighbor of any other node + } + if (LOG.isDebugEnabled()) { + LOG.debug("Returning sinks: " + sources); + } + return sources; + } + + /** + * Returns the set of nodes with out-degree of 0, i.e. those nodes whose list of neighbors is empty + * + * @return + */ + Set<String> getSinks() { + Set<String> sinks = new HashSet<String>(); + for (Map.Entry<String, Set<String>> entry : _nodes.entrySet()) { + Set<String> nbrs = entry.getValue(); + if (nbrs.isEmpty()) { // A sink in a DAG doesn't have any neighbor + String node = entry.getKey(); + sinks.add(node); + } + } + if (LOG.isDebugEnabled()) { + LOG.debug("Returning sinks: " + sinks); + } + return sinks; + } + + /** + * Attempts to do a depth first traversal of a graph and returns the resulting path. Note that there could be several paths that connect node "from" to node "to". + * + * @param from + * @param to + * @return + */ + List<String> getAPath(String from, String to, Set<String> alreadyVisited) { + List<String> path = new ArrayList<String>(_nodes.size()); + if (_nodes.containsKey(from) && _nodes.containsKey(to)) { // one can never reach non-existent nodes + if (hasArc(from, to)) { + path.add(from); + path.add(to); + } else { + alreadyVisited.add(from); + Set<String> nbrs = _nodes.get(from); + for (String nbr : nbrs) { + if (!alreadyVisited.contains(nbr)) { + List<String> subPath = getAPath(nbr, to, alreadyVisited); + if (!subPath.isEmpty()) { + path.add(from); + path.addAll(subPath); + } + } + } + } + } + return path; + } + + @Override + public boolean equals(Object object) { + if (object == this) { + return true; + } + if (object == null || object.getClass() != this.getClass()) { + return false; + } + DirectedGraph that = (DirectedGraph)object; + return Objects.equals(this._nodes, that._nodes); + } + + @Override + public int hashCode() { + return Objects.hashCode(_nodes); + } + + @Override + public String toString() { + return "_nodes=" + Objects.toString(_nodes); + } + } +} http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/9adfcbe0/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestDirectedGraph.java ---------------------------------------------------------------------- diff --git a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestDirectedGraph.java b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestDirectedGraph.java new file mode 100644 index 0000000..f58ae2b --- /dev/null +++ b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestDirectedGraph.java @@ -0,0 +1,77 @@ +/* + * 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.ranger.plugin.model.validation; + +import static org.junit.Assert.assertEquals; + +import java.util.HashSet; + +import org.apache.ranger.plugin.model.validation.RangerServiceDefHelper.DirectedGraph; +import org.junit.Test; + +import com.google.common.collect.Lists; +import com.google.common.collect.Sets; + +public class TestDirectedGraph { + + @Test + public void test() { + /* + * create a graph as follows: + * GG + * \ + * v + * AA -> BB -> CC + * ^ | + * | v + * EE <- DD -> FF + * | + * v + * HH + * + * This directed graph has + * - 1 cycle [BB CC DD EE], + * - 2 sources [AA GG], + * - 2 sinks [HH FF] + * - 4 hierarchies { [AA BB CC DD FF], [AA BB CC DD EE HH], [GG CC DD FF], [GG CC DD EE HH] } + */ + DirectedGraph graph = new DirectedGraph(); + // first add all of the arcs - from top row to bottom row and from left to right + //1st row + graph.addArc("GG", "CC"); + // 2nd row + graph.addArc("AA", "BB"); graph.addArc("BB", "CC"); + // 3rd row + graph.addArc("EE", "BB"); graph.addArc("DD", "EE"); graph.addArc("CC", "DD"); graph.addArc("DD", "FF"); + // 4th row + graph.addArc("EE", "HH"); + + // now assert the structure + assertEquals(Sets.newHashSet("FF", "HH"), graph.getSinks()); + assertEquals(Sets.newHashSet("AA", "GG"), graph.getSources()); + // check paths + assertEquals(Lists.newArrayList("AA", "BB", "CC", "DD", "FF"), graph.getAPath("AA", "FF", new HashSet<String>())); + assertEquals(Lists.newArrayList(), graph.getAPath("GG", "AA", new HashSet<String>())); // BB is not reachable from GG + assertEquals(Lists.newArrayList("GG", "CC", "DD", "EE", "BB"), graph.getAPath("GG", "BB", new HashSet<String>())); + assertEquals(Lists.newArrayList("GG", "CC", "DD", "FF"), graph.getAPath("GG", "FF", new HashSet<String>())); + assertEquals(Lists.newArrayList("EE", "BB", "CC", "DD", "FF"), graph.getAPath("EE", "FF", new HashSet<String>())); + } + +} http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/9adfcbe0/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 2fd1d6a..f02b96e 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 @@ -28,6 +28,7 @@ import static org.mockito.Mockito.when; import java.util.ArrayList; import java.util.Arrays; +import java.util.Date; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -53,11 +54,31 @@ import com.google.common.collect.Sets; public class TestRangerPolicyValidator { + /** + * Wrapper class only so we clear out the RangerServiceDefHelper before every test. + * @author alal + * + */ + static class RangerPolicyValidatorWrapper extends RangerPolicyValidator { + public RangerPolicyValidatorWrapper(ServiceStore store) { + super(store); + } + + boolean isValid(Long id, Action action, List<ValidationFailureDetails> failures) { + RangerServiceDefHelper._Cache.clear(); + return super.isValid(id, action, failures); + } + + boolean isValid(RangerPolicy policy, Action action, boolean isAdmin, List<ValidationFailureDetails> failures) { + RangerServiceDefHelper._Cache.clear(); + return super.isValid(policy, action, isAdmin, failures); + } + } @Before public void setUp() throws Exception { _store = mock(ServiceStore.class); _policy = mock(RangerPolicy.class); - _validator = new RangerPolicyValidator(_store); + _validator = new RangerPolicyValidatorWrapper(_store); _serviceDef = mock(RangerServiceDef.class); _factory = mock(RangerObjectFactory.class); _validator._factory = _factory; @@ -87,18 +108,32 @@ public class TestRangerPolicyValidator { String[] accessTypes_bad = new String[] { "r", "w", "xx", }; // two missing (x, a), one new that isn't on bad (xx) private final Object[][] resourceDefData = new Object[][] { - // { name, mandatory, reg-exp, excludesSupported, recursiveSupported } - { "db", true, "db\\d+", null, null }, // valid values: db1, db22, db983, etc.; invalid: db, db12x, ttx11, etc.; null => false for excludes and recursive - { "tbl", true, null, true, true }, // regex == null => anything goes; excludes == true, recursive == true - { "col", false, "col\\d{1,2}", false, true } // valid: col1, col47, etc.; invalid: col, col238, col1, etc., excludes == false, recursive == true + // { name, excludesSupported, recursiveSupported, mandatory, reg-exp, parent-level } + { "db", null, null, true, "db\\d+", null }, // valid values: db1, db22, db983, etc.; invalid: db, db12x, ttx11, etc.; null => false for excludes and recursive + { "tbl", true, true, true, null, "db" }, // regex == null => anything goes; excludes == true, recursive == true + { "col", false, true, false, "col\\d{1,2}", "tbl" }, // valid: col1, col47, etc.; invalid: col, col238, col1, etc., excludes == false, recursive == true }; + private final Object[][] resourceDefData_multipleHierarchies = new Object[][] { + // { name, excludesSupported, recursiveSupported, mandatory, reg-exp, parent-level } + { "db", null, null, true, "db\\d+", null }, // valid values: db1, db22, db983, etc.; invalid: db, db12x, ttx11, etc.; null => false for excludes and recursive + { "tbl", true, true, true, null, "db" }, // regex == null => anything goes; excludes == true, recursive == true + { "col", false, true, false, "col\\d{1,2}", "tbl" }, // valid: col1, col47, etc.; invalid: col, col238, col1, etc., excludes == false, recursive == true + { "udf", true, true, true, null, "db" } // same parent as tbl (simulating hive's multiple resource hierarchies) + }; + private final Object[][] policyResourceMap_good = new Object[][] { // resource-name, values, excludes, recursive { "db", new String[] { "db1", "db2" }, null, null }, { "TBL", new String[] { "tbl1", "tbl2" }, true, false } // case should not matter }; + private final Object[][] policyResourceMap_goodMultipleHierarchies = new Object[][] { + // resource-name, values, excludes, recursive + { "db", new String[] { "db1", "db2" }, null, null }, + { "UDF", new String[] { "udf1", "udf2" }, true, false } // case should not matter + }; + private final Object[][] policyResourceMap_bad = new Object[][] { // resource-name, values, excludes, recursive { "db", new String[] { "db1", "db2" }, null, true }, // mandatory "tbl" missing; recursive==true specified when resource-def does not support it (null) @@ -140,6 +175,7 @@ public class TestRangerPolicyValidator { when(_store.getServiceByName("service-name")).thenReturn(service); // service points to a valid service-def _serviceDef = _utils.createServiceDefWithAccessTypes(accessTypes); + when(_serviceDef.getName()).thenReturn("service-type"); when(_store.getServiceDefByName("service-type")).thenReturn(_serviceDef); // a matching policy should exist for create when checked by id and not exist when checked by name. when(_store.getPolicy(7L)).thenReturn(null); @@ -382,7 +418,7 @@ public class TestRangerPolicyValidator { } // service-def should contain the right access types on it. - _serviceDef = _utils.createServiceDefWithAccessTypes(accessTypes_bad); + _serviceDef = _utils.createServiceDefWithAccessTypes(accessTypes_bad, "service-type"); when(_store.getServiceDefByName("service-type")).thenReturn(_serviceDef); for (Action action : cu) { for (boolean isAdmin : new boolean[] { true, false }) { @@ -392,7 +428,7 @@ public class TestRangerPolicyValidator { } // create the right service def with right resource defs - this is the same as in the happypath test above. - _serviceDef = _utils.createServiceDefWithAccessTypes(accessTypes); + _serviceDef = _utils.createServiceDefWithAccessTypes(accessTypes, "service-type"); when(_store.getPolicies(filter)).thenReturn(null); List<RangerResourceDef> resourceDefs = _utils.createResourceDefs(resourceDefData); when(_serviceDef.getResources()).thenReturn(resourceDefs); @@ -401,17 +437,16 @@ public class TestRangerPolicyValidator { // one mandatory is missing (tbl) and one unknown resource is specified (extra), and values of option resource don't conform to validation pattern (col) Map<String, RangerPolicyResource> policyResources = _utils.createPolicyResourceMap(policyResourceMap_bad); when(_policy.getResources()).thenReturn(policyResources); -// TODO disabled till a more robust fix for Hive resources definition can be found -// for (Action action : cu) { -// for (boolean isAdmin : new boolean[] { true, false }) { -// _failures.clear(); assertFalse(_validator.isValid(_policy, action, isAdmin, _failures)); -// _utils.checkFailureForMissingValue(_failures, "resources", "tbl"); // for missing resource: tbl -// _utils.checkFailureForSemanticError(_failures, "resources", "extra"); // for spurious resource: "extra" -// _utils.checkFailureForSemanticError(_failures, "resource-values", "col"); // for spurious resource: "extra" -// _utils.checkFailureForSemanticError(_failures, "isRecursive", "db"); // for specifying it as true when def did not allow it -// _utils.checkFailureForSemanticError(_failures, "isExcludes", "col"); // for specifying it as true when def did not allow it -// } -// } + for (Action action : cu) { + for (boolean isAdmin : new boolean[] { true, false }) { + _failures.clear(); assertFalse(_validator.isValid(_policy, action, isAdmin, _failures)); + _utils.checkFailureForSemanticError(_failures, "resource-values", "col"); // for spurious resource: "extra" + _utils.checkFailureForSemanticError(_failures, "isRecursive", "db"); // for specifying it as true when def did not allow it + _utils.checkFailureForSemanticError(_failures, "isExcludes", "col"); // for specifying it as true when def did not allow it + _utils.checkFailureForMissingValue(_failures, "resources", "tbl"); // for missing resource: tbl + _utils.checkFailureForSemanticError(_failures, "resources", "extra"); // for spurious resource: "extra" + } + } // create the right resource def but let it clash with another policy with matching resource-def policyResources = _utils.createPolicyResourceMap(policyResourceMap_good); @@ -597,7 +632,7 @@ public class TestRangerPolicyValidator { public final void test_isValidResourceFlags_happyPath() { Map<String, RangerPolicyResource> resourceMap = _utils.createPolicyResourceMap(policyResourceMap_happyPath); - List<RangerResourceDef> resourceDefs = _utils.createResourceDefs2(resourceDef_happyPath); + List<RangerResourceDef> resourceDefs = _utils.createResourceDefs(resourceDef_happyPath); when(_serviceDef.getResources()).thenReturn(resourceDefs); assertTrue(_validator.isValidResourceFlags(resourceMap, _failures, resourceDefs, "a-service-def", "a-policy", true)); @@ -617,7 +652,7 @@ public class TestRangerPolicyValidator { @Test public final void test_isValidResourceFlags_failures() { // passing true when def says false/null - List<RangerResourceDef> resourceDefs = _utils.createResourceDefs2(resourceDef_happyPath); + List<RangerResourceDef> resourceDefs = _utils.createResourceDefs(resourceDef_happyPath); Map<String, RangerPolicyResource> resourceMap = _utils.createPolicyResourceMap(policyResourceMap_failures); when(_serviceDef.getResources()).thenReturn(resourceDefs); // should not error out on @@ -664,6 +699,39 @@ public class TestRangerPolicyValidator { assertTrue("No duplication if updating policy", _validator.isPolicyResourceUnique(policies[0], _failures, Action.UPDATE, serviceName)); } + @Test + public final void test_isValidResourceNames_happyPath() { + String serviceName = "a-service-def"; + // setup service-def + Date now = new Date(); + when(_serviceDef.getName()).thenReturn(serviceName ); + when(_serviceDef.getUpdateTime()).thenReturn(now); + List<RangerResourceDef> resourceDefs = _utils.createResourceDefs(resourceDefData_multipleHierarchies); + when(_serviceDef.getResources()).thenReturn(resourceDefs); + // setup policy + Map<String, RangerPolicyResource> policyResources = _utils.createPolicyResourceMap(policyResourceMap_goodMultipleHierarchies); + when(_policy.getResources()).thenReturn(policyResources); + assertTrue(_validator.isValidResourceNames(_policy, _failures, _serviceDef)); + } + + @Test + public final void test_isValidResourceNames_failures() { + String serviceName = "a-service-def"; + // setup service-def + Date now = new Date(); + when(_serviceDef.getName()).thenReturn(serviceName ); + when(_serviceDef.getUpdateTime()).thenReturn(now); + List<RangerResourceDef> resourceDefs = _utils.createResourceDefs(resourceDefData_multipleHierarchies); + when(_serviceDef.getResources()).thenReturn(resourceDefs); + // setup policy + Map<String, RangerPolicyResource> policyResources = _utils.createPolicyResourceMap(policyResourceMap_bad); + when(_policy.getResources()).thenReturn(policyResources); + assertFalse("Missing required resource and unknown resource", _validator.isValidResourceNames(_policy, _failures, _serviceDef)); + _utils.checkFailureForMissingValue(_failures, "resources", new String[] {"tbl", "udf"} ); + _utils.checkFailureForSemanticError(_failures, "resources", "extra"); + } + + private ValidationTestUtils _utils = new ValidationTestUtils(); private List<ValidationFailureDetails> _failures = new ArrayList<ValidationFailureDetails>(); private ServiceStore _store; http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/9adfcbe0/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceDefHelper.java ---------------------------------------------------------------------- diff --git a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceDefHelper.java b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceDefHelper.java new file mode 100644 index 0000000..a004f84 --- /dev/null +++ b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerServiceDefHelper.java @@ -0,0 +1,137 @@ +/* + * 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.ranger.plugin.model.validation; + +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.Calendar; +import java.util.Date; +import java.util.GregorianCalendar; +import java.util.List; +import java.util.Set; + +import org.apache.ranger.plugin.model.RangerServiceDef; +import org.apache.ranger.plugin.model.RangerServiceDef.RangerResourceDef; +import org.apache.ranger.plugin.model.validation.RangerServiceDefHelper; +import org.apache.ranger.plugin.model.validation.RangerServiceDefHelper.Delegate; +import org.junit.Test; + +import com.google.common.collect.Lists; + +public class TestRangerServiceDefHelper { + + @Test + public void test_getResourceHierarchies() { + /* + * Create a service-def with following resource graph + * + * Database -> UDF + * | + * v + * Table -> Column + * | + * v + * Table-Attribute + * + * It contains following hierarchies + * - [ Database UDF] + * - [ Database Table Column ] + * - [ Database Table Table-Attribute ] + */ + RangerResourceDef Database = createResourceDef("Database", ""); + RangerResourceDef UDF = createResourceDef("UDF", "Database"); + RangerResourceDef Table = createResourceDef("Table", "Database"); + RangerResourceDef Column = createResourceDef("Column", "Table"); + RangerResourceDef Table_Atrribute = createResourceDef("Table-Attribute", "Table"); + // order of resources in list sould not matter + List<RangerResourceDef> resourceDefs = Lists.newArrayList(Column, Database, Table, Table_Atrribute, UDF); + // stuff this into a service-def + RangerServiceDef serviceDef = mock(RangerServiceDef.class); + when(serviceDef.getResources()).thenReturn(resourceDefs); + when(serviceDef.getName()).thenReturn("a-service-def"); + // now assert the behavior + RangerServiceDefHelper serviceDefHelper = new RangerServiceDefHelper(serviceDef); + Set<List<RangerResourceDef>> hierarchies = serviceDefHelper.getResourceHierarchies(); + // there should be + List<RangerResourceDef> hierarchy = Lists.newArrayList(Database, UDF); + assertTrue(hierarchies.contains(hierarchy)); + hierarchy = Lists.newArrayList(Database, Table, Column); + assertTrue(hierarchies.contains(hierarchy)); + hierarchy = Lists.newArrayList(Database, Table, Table_Atrribute); + assertTrue(hierarchies.contains(hierarchy)); + } + + @Test + public final void test_cacheBehavior() { + // wipe the cache clean + RangerServiceDefHelper._Cache.clear(); + // let's add one entry to the cache + Delegate delegate = mock(Delegate.class); + Date aDate = getNow(); + String serviceName = "a-service-def"; + when(delegate.getServiceFreshnessDate()).thenReturn(aDate); + when(delegate.getServiceName()).thenReturn(serviceName); + RangerServiceDefHelper._Cache.put(serviceName, delegate); + + // create a service def with matching date value + RangerServiceDef serviceDef = mock(RangerServiceDef.class); + when(serviceDef.getName()).thenReturn(serviceName); + when(serviceDef.getUpdateTime()).thenReturn(aDate); + + // since cache has it, we should get back the one that we have added + RangerServiceDefHelper serviceDefHelper = new RangerServiceDefHelper(serviceDef); + assertTrue("Didn't get back the same object that was put in cache", delegate == serviceDefHelper._delegate); + + // if we change the date then that should force helper to create a new delegate instance + /* + * NOTE:: current logic would replace the cache instance even if the one in the cache is newer. This is not likely to happen but it is important to call this out + * as in rare cases one may end up creating re creating delegate if threads have stale copies of service def. + */ + when(serviceDef.getUpdateTime()).thenReturn(getLastMonth()); + serviceDefHelper = new RangerServiceDefHelper(serviceDef); + assertTrue("Didn't get a delegate different than what was put in the cache", delegate != serviceDefHelper._delegate); + // now that a new instance was added to the cache let's ensure that it got added to the cache + Delegate newDelegate = serviceDefHelper._delegate; + serviceDefHelper = new RangerServiceDefHelper(serviceDef); + assertTrue("Didn't get a delegate different than what was put in the cache", newDelegate == serviceDefHelper._delegate); + } + + RangerResourceDef createResourceDef(String name, String parent) { + RangerResourceDef resourceDef = mock(RangerResourceDef.class); + when(resourceDef.getName()).thenReturn(name); + when(resourceDef.getParent()).thenReturn(parent); + return resourceDef; + } + + Date getLastMonth() { + Calendar cal = GregorianCalendar.getInstance(); + cal.add( Calendar.MONTH, 1); + Date lastMonth = cal.getTime(); + return lastMonth; + } + + Date getNow() { + Calendar cal = GregorianCalendar.getInstance(); + Date now = cal.getTime(); + return now; + } +} http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/9adfcbe0/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerValidator.java ---------------------------------------------------------------------- diff --git a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerValidator.java b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerValidator.java index f014552..81ecfdd 100644 --- a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerValidator.java +++ b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/TestRangerValidator.java @@ -280,12 +280,14 @@ public class TestRangerValidator { // access type defs with null empty blank names are skipped, spaces within names are preserved Object[][] data = { - { "a", true }, // all good - null, // this should put a null element in the resource def! - { "b", null }, // mandatory field is null, i.e. false - { "c", false }, // non-mandatory field false - upper case - { "D", true }, // resource specified in upper case - { "E", false }, // all good + // { name, excludes recursive mandatory, reg-exp, parent-level } + // Supported?, Supported?, + { "a", null, null, true }, // all good + null, // this should put a null element in the resource def! + { "b", null, null, null }, // mandatory field is null, i.e. false + { "c", null, null, false }, // non-mandatory field false - upper case + { "D", null, null, true }, // resource specified in upper case + { "E", null, null, false }, // all good }; resourceDefs.addAll(_utils.createResourceDefs(data)); accessTypes = _validator.getMandatoryResourceNames(serviceDef); http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/9adfcbe0/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/ValidationTestUtils.java ---------------------------------------------------------------------- diff --git a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/ValidationTestUtils.java b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/ValidationTestUtils.java index 1e81ec3..432c9d4 100644 --- a/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/ValidationTestUtils.java +++ b/agents-common/src/test/java/org/apache/ranger/plugin/model/validation/ValidationTestUtils.java @@ -83,6 +83,24 @@ public class ValidationTestUtils { checkFailure(failures, null, true, null, field, subField); } + // check if any one of the sub-fields is present + void checkFailureForMissingValue(List<ValidationFailureDetails> failures, String field, String[] subFields) { + if (CollectionUtils.isEmpty(failures)) { + fail("List of failures is null/empty!"); + } else { + boolean found = false; + int i = 0; + while (!found && i < subFields.length) { + String subField = subFields[i]; + if (hasFailure(failures, null, true, null, field, subField)) { + found = true; + } + i++; + } + assertTrue(failures.toString(), found); + } + } + void checkFailureForInternalError(List<ValidationFailureDetails> failures, String fieldName) { checkFailure(failures, true, null, null, fieldName, null); } @@ -95,18 +113,23 @@ public class ValidationTestUtils { if (CollectionUtils.isEmpty(failures)) { fail("List of failures is null/empty!"); } else { - boolean found = false; - for (ValidationFailureDetails f : failures) { - if ((internalError == null || internalError == f._internalError) && - (missing == null || missing == f._missing) && - (semanticError == null || semanticError == f._semanticError) && - (field == null || field.equals(f._fieldName)) && - (subField == null || subField.equals(f._subFieldName))) { - found = true; - } + boolean found = hasFailure(failures, internalError, missing, semanticError, field, subField); + assertTrue(failures.toString(), found); + } + } + + boolean hasFailure(List<ValidationFailureDetails> failures, Boolean internalError, Boolean missing, Boolean semanticError, String field, String subField) { + boolean found = false; + for (ValidationFailureDetails f : failures) { + if ((internalError == null || internalError == f._internalError) && + (missing == null || missing == f._missing) && + (semanticError == null || semanticError == f._semanticError) && + (field == null || field.equals(f._fieldName)) && + (subField == null || subField.equals(f._subFieldName))) { + found = true; } - assertTrue(found); } + return found; } List<RangerAccessTypeDef> createAccessTypeDefs(String[] names) { @@ -144,6 +167,12 @@ public class ValidationTestUtils { return result; } + RangerServiceDef createServiceDefWithAccessTypes(String[] accesses, String serviceName) { + RangerServiceDef serviceDef = createServiceDefWithAccessTypes(accesses); + when(serviceDef.getName()).thenReturn(serviceName); + return serviceDef; + } + RangerServiceDef createServiceDefWithAccessTypes(String[] accesses) { RangerServiceDef serviceDef = mock(RangerServiceDef.class); List<RangerAccessTypeDef> accessTypeDefs = new ArrayList<RangerServiceDef.RangerAccessTypeDef>(); @@ -221,15 +250,18 @@ public class ValidationTestUtils { String regExPattern = null; Boolean isExcludesSupported = null; Boolean isRecursiveSupported = null; + String parent = null; switch(row.length) { + case 6: + parent = (String) row[5]; case 5: - isRecursiveSupported = (Boolean)row[4]; + regExPattern = (String)row[4]; case 4: - isExcludesSupported = (Boolean)row[3]; + mandatory = (Boolean)row[3]; case 3: - regExPattern = (String)row[2]; + isRecursiveSupported = (Boolean)row[2]; case 2: - mandatory = (Boolean)row[1]; + isExcludesSupported = (Boolean)row[1]; case 1: name = (String)row[0]; } @@ -239,36 +271,7 @@ public class ValidationTestUtils { when(aDef.getValidationRegEx()).thenReturn(regExPattern); when(aDef.getExcludesSupported()).thenReturn(isExcludesSupported); when(aDef.getRecursiveSupported()).thenReturn(isRecursiveSupported); - } - defs.add(aDef); - } - return defs; - } - - List<RangerResourceDef> createResourceDefs2(Object[][] data) { - // if data itself is null then return null back - if (data == null) { - return null; - } - List<RangerResourceDef> defs = new ArrayList<RangerResourceDef>(); - for (Object[] row : data) { - RangerResourceDef aDef = null; - if (row != null) { - String name = null; - Boolean isExcludesSupported = null; - Boolean isRecursiveSupported = null; - switch(row.length) { - case 3: - isRecursiveSupported = (Boolean)row[2]; // note: falls through to next case - case 2: - isExcludesSupported = (Boolean)row[1]; // note: falls through to next case - case 1: - name = (String)row[0]; - } - aDef = mock(RangerResourceDef.class); - when(aDef.getName()).thenReturn(name); - when(aDef.getExcludesSupported()).thenReturn(isExcludesSupported); - when(aDef.getRecursiveSupported()).thenReturn(isRecursiveSupported); + when(aDef.getParent()).thenReturn(parent); } defs.add(aDef); }
