ranger-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mad...@apache.org
Subject incubator-ranger git commit: RANGER-529 Policy Validation: resources of a policy must match one of the resource hierarchies of the service def
Date Thu, 04 Jun 2015 00:37:13 GMT
Repository: incubator-ranger
Updated Branches:
  refs/heads/master de019a805 -> cfc0f39ac


RANGER-529 Policy Validation: resources of a policy must match one of the resource hierarchies
of the service def

Signed-off-by: Madhan Neethiraj <madhan@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/cfc0f39a
Tree: http://git-wip-us.apache.org/repos/asf/incubator-ranger/tree/cfc0f39a
Diff: http://git-wip-us.apache.org/repos/asf/incubator-ranger/diff/cfc0f39a

Branch: refs/heads/master
Commit: cfc0f39acfeab1d03eb186492f43e6cc0e93c2b2
Parents: de019a8
Author: Alok Lal <alal@hortonworks.com>
Authored: Wed Jun 3 13:58:43 2015 -0700
Committer: Madhan Neethiraj <madhan@apache.org>
Committed: Wed Jun 3 17:31:41 2015 -0700

----------------------------------------------------------------------
 .../model/validation/RangerPolicyValidator.java | 129 +++++++++++++------
 .../validation/RangerServiceDefHelper.java      |  20 ++-
 .../validation/TestRangerPolicyValidator.java   |  30 ++++-
 .../validation/TestRangerServiceDefHelper.java  |   4 +-
 4 files changed, 140 insertions(+), 43 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/cfc0f39a/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 caaf68f..cea3e05 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
@@ -20,7 +20,7 @@
 package org.apache.ranger.plugin.model.validation;
 
 import java.util.ArrayList;
-import java.util.Iterator;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -39,9 +39,6 @@ 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 {
 
 	private static final Log LOG = LogFactory.getLog(RangerPolicyValidator.class);
@@ -304,51 +301,38 @@ public class RangerPolicyValidator extends RangerValidator {
 		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!");
+			LOG.warn("RangerPolicyValidator.isValidResourceNames: serviceDef does not have any resource
hierarchies, possibly due to a old/migrated service def!  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!
+			 * A policy is for a single hierarchy however, it doesn't specify which one.  So we have
to guess which hierarchy(s) it possibly be for.  First, see if the policy could be for 
+			 * any of the known hierarchies?  A candidate hierarchy is one whose resource levels are
a superset of those in the policy.
+			 * Why?  What we want to catch at this stage is policies that straddles multiple hierarchies,
e.g. db, udf and column for a hive policy.
+			 * This has the side effect of catch spurious levels specified on the policy, e.g. having
a level "blah" on a hive policy.  
 			 */
-			if (!foundHierarchyMatch) {
+			Set<List<RangerResourceDef>> candidateHierarchies = filterHierarchies_hierarchyHasAllPolicyResources(policyResources,
hierarchies, defHelper);
+			if (candidateHierarchies.isEmpty()) {
+				// let's build a helpful message for user
 				failures.add(new ValidationFailureDetailsBuilder()
 					.field("resources")
-					.subField(missingResourcesCopyForErrorMessage.iterator().next()) // We return any one
missing resource here!
-					.isMissing()
-					.becauseOf("required resources [" + missingResourcesCopyForErrorMessage + "] are missing")
+					.subField("incompatible")
+					.isSemanticallyIncorrect()
+					.becauseOf(String.format("policy resources [%s] were incompatible with all the hierarchies
for this service defs! Valid hierarchies are: %s", 
+							policyResources.toString(), toStringHierarchies_all(hierarchies, defHelper)))
 					.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.
+			 * Among the candidate hierarchies there should be at least one for which policy specifies
all of the mandatory resources.  Note that there could be multiple 
+			 * hierarchies that meet that criteria, e.g. a hive policy that specified only DB.  It
is not clear if it belongs to DB->UDF or DB->TBL->COL hierarchy.
+			 * However, if both UDF and TBL were required then we can detect that policy does not
specify mandatory levels for any of the candidate hierarchies.
 			 */
-			Set<String> allResource = defHelper.getResorceMap().keySet();
-			Set<String> unknownResources = Sets.difference(policyResources, allResource);
-			if (!unknownResources.isEmpty()) {
+			Set<List<RangerResourceDef>> validHierarchies = filterHierarchies_mandatoryResourcesSpecifiedInPolicy(policyResources,
candidateHierarchies, defHelper);
+			if (validHierarchies.isEmpty()) {
 				failures.add(new ValidationFailureDetailsBuilder()
 					.field("resources")
-					.subField(unknownResources.iterator().next()) // we return any one parameter!
+					.subField("missing mandatory")
 					.isSemanticallyIncorrect()
-					.becauseOf("resource[" + unknownResources + "] is not valid for service-def[" + serviceDef.getName()
+ "]")
+					.becauseOf("policy is missing required resources. Mandatory fields of potential hierarchies
are: " + toStringHierarchies_mandatory(candidateHierarchies, defHelper))
 					.build());
 				valid = false;
 			}
@@ -360,6 +344,79 @@ public class RangerPolicyValidator extends RangerValidator {
 		return valid;
 	}
 	
+	/**
+	 * String representation of mandatory resources of all the hierarchies suitable of showing
to user.  Mandatory resources within a hierarchy are not ordered per the hierarchy. 
+	 * @param hierarchies
+	 * @param defHelper
+	 * @return
+	 */
+	String toStringHierarchies_mandatory(Set<List<RangerResourceDef>> hierarchies,
RangerServiceDefHelper defHelper) {
+
+		// helper function skipping sanity checks of getting null arguments passed
+		StringBuilder builder = new StringBuilder();
+		for (List<RangerResourceDef> aHierarchy : hierarchies) {
+			builder.append(defHelper.getMandatoryResourceNames(aHierarchy));
+			builder.append(" ");
+		}
+		return builder.toString();
+	}
+	
+	/**
+	 * String representation of all resources of all hierarchies.  Resources within a hierarchy
are ordered per the hierarchy.
+	 * @param hierarchies
+	 * @param defHelper
+	 * @return
+	 */
+	String toStringHierarchies_all(Set<List<RangerResourceDef>> hierarchies, RangerServiceDefHelper
defHelper) {
+
+		// helper function skipping sanity checks of getting null arguments passed
+		StringBuilder builder = new StringBuilder();
+		for (List<RangerResourceDef> aHierarchy : hierarchies) {
+			builder.append(defHelper.getAllResourceNamesOrdered(aHierarchy));
+			builder.append(" ");
+		}
+		return builder.toString();
+	}
+	/**
+	 * Returns the subset of all hierarchies that are a superset of the policy's resources.

+	 * @param policyResources
+	 * @param hierarchies
+	 * @return
+	 */
+	Set<List<RangerResourceDef>> filterHierarchies_hierarchyHasAllPolicyResources(Set<String>
policyResources, Set<List<RangerResourceDef>> hierarchies, RangerServiceDefHelper
defHelper) {
+		
+		// helper function skipping sanity checks of getting null arguments passed
+		Set<List<RangerResourceDef>> result = new HashSet<List<RangerResourceDef>>(hierarchies.size());
+		for (List<RangerResourceDef> aHierarchy : hierarchies) {
+			Set<String> hierarchyResources = defHelper.getAllResourceNames(aHierarchy);
+			if (hierarchyResources.containsAll(policyResources)) {
+				result.add(aHierarchy);
+			}
+		}
+		return result;
+	}
+	
+	/**
+	 * Returns the subset of hierarchies all of whose mandatory resources were found in policy's
resource set.  candidate hierarchies are expected to have passed 
+	 * <code>filterHierarchies_hierarchyHasAllPolicyResources</code> check first.
+	 * @param policyResources
+	 * @param hierarchies
+	 * @param defHelper
+	 * @return
+	 */
+	Set<List<RangerResourceDef>> filterHierarchies_mandatoryResourcesSpecifiedInPolicy(Set<String>
policyResources, Set<List<RangerResourceDef>> hierarchies, RangerServiceDefHelper
defHelper) {
+		
+		// helper function skipping sanity checks of getting null arguments passed
+		Set<List<RangerResourceDef>> result = new HashSet<List<RangerResourceDef>>(hierarchies.size());
+		for (List<RangerResourceDef> aHierarchy : hierarchies) {
+			Set<String> mandatoryResources = defHelper.getMandatoryResourceNames(aHierarchy);
+			if (policyResources.containsAll(mandatoryResources)) {
+				result.add(aHierarchy);
+			}
+		}
+		return result;
+	}
+	
 	boolean isValidResourceFlags(final Map<String, RangerPolicyResource> inputPolicyResources,
final List<ValidationFailureDetails> failures,
 			final List<RangerResourceDef> resourceDefs, final String serviceDefName, final String
policyName, boolean isAdmin) {
 		if(LOG.isDebugEnabled()) {

http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/cfc0f39a/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
index d3bcc1a..53df193 100644
--- 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
@@ -119,7 +119,25 @@ public class RangerServiceDefHelper {
 		return result;
 	}
 	
-	public List<String> getAllResourceNames(List<RangerResourceDef> hierarchy) {
+	/**
+	 * Set view of a hierarchy's resource names for efficient searching
+	 * @param hierarchy
+	 * @return
+	 */
+	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;
+	}
+	
+	/**
+	 * Resources names matching the order of list of resource defs passed in.
+	 * @param hierarchy
+	 * @return
+	 */
+	public List<String> getAllResourceNamesOrdered(List<RangerResourceDef> hierarchy)
{
 		List<String> result = new ArrayList<String>(hierarchy.size());
 		for (RangerResourceDef resourceDef : hierarchy) {
 			result.add(resourceDef.getName());

http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/cfc0f39a/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 27c6318..6236d71 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
@@ -142,6 +142,19 @@ public class TestRangerPolicyValidator {
 			{"extra", new String[] { "extra1", "extra2" }, null, null } // spurious "extra" specified
 	};
 
+	private final Object[][] policyResourceMap_bad_multiple_hierarchies = new Object[][] {
+			// resource-name, values, excludes, recursive
+			{  "db", new String[] { "db1", "db2" }, null, true }, 
+			{ "tbl", new String[] { "tbl11", "tbl2" }, null, true }, 
+			{ "col", new String[] { "col1", "col2" }, true, true },
+			{ "udf", new String[] { "extra1", "extra2" }, null, null } // either udf or tbl/db/col
should be specified, not both
+	};
+
+	private final Object[][] policyResourceMap_bad_multiple_hierarchies_missing_mandatory =
new Object[][] {
+			// resource-name, values, excludes, recursive
+			{  "db", new String[] { "db1", "db2" }, null, true }
+	};
+
 	@Test
 	public final void testIsValid_long() throws Exception {
 		// this validation should be removed if we start supporting other than delete action
@@ -454,8 +467,6 @@ public class TestRangerPolicyValidator {
 				_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"
 			}
 		}
 		
@@ -755,8 +766,19 @@ public class TestRangerPolicyValidator {
 		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");
+		_utils.checkFailureForSemanticError(_failures, "resources");
+		
+		// another bad resource map that straddles multiple hierarchies
+		policyResources = _utils.createPolicyResourceMap(policyResourceMap_bad_multiple_hierarchies);
+		when(_policy.getResources()).thenReturn(policyResources);
+		_failures.clear(); assertFalse("Policy with resources for multiple hierarchies", _validator.isValidResourceNames(_policy,
_failures, _serviceDef));
+		_utils.checkFailureForSemanticError(_failures, "resources", "incompatible");
+		
+		// another bad policy resource map that could match multiple hierarchies but is short on
mandatory resources for all of those matches
+		policyResources = _utils.createPolicyResourceMap(policyResourceMap_bad_multiple_hierarchies_missing_mandatory);
+		when(_policy.getResources()).thenReturn(policyResources);
+		_failures.clear(); assertFalse("Policy with resources for multiple hierarchies missing
mandatory resources for all pontential matches", _validator.isValidResourceNames(_policy,
_failures, _serviceDef));
+		_utils.checkFailureForSemanticError(_failures, "resources", "missing mandatory");
 	}
 	
 	

http://git-wip-us.apache.org/repos/asf/incubator-ranger/blob/cfc0f39a/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
index 883b808..d9e50e4 100644
--- 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
@@ -145,7 +145,7 @@ public class TestRangerServiceDefHelper {
 		expectedHierarchies.add(Lists.newArrayList("namespace", "function"));
 		
 		for (List<RangerResourceDef> aHierarchy : hierarchies) {
-			List<String> resourceNames = _helper.getAllResourceNames(aHierarchy);
+			List<String> resourceNames = _helper.getAllResourceNamesOrdered(aHierarchy);
 			assertTrue(expectedHierarchies.contains(resourceNames));
 			expectedHierarchies.remove(resourceNames);
 		}
@@ -186,7 +186,7 @@ public class TestRangerServiceDefHelper {
 		expectedHierarchies.add(Lists.newArrayList("namespace", "function"));
 		
 		for (List<RangerResourceDef> aHierarchy : hierarchies) {
-			List<String> resourceNames = _helper.getAllResourceNames(aHierarchy);
+			List<String> resourceNames = _helper.getAllResourceNamesOrdered(aHierarchy);
 			assertTrue(expectedHierarchies.contains(resourceNames));
 			expectedHierarchies.remove(resourceNames);
 		}


Mime
View raw message