Repository: incubator-sentry Updated Branches: refs/heads/master 0619d8a1a -> e3b77f06e
SENTRY-1119: Allow data engines to specify the ActionFactory from configuration(Bhooshan Mogal via Colin Ma, reviewed by Colin Ma, Sravya Tirukkovalur) Project: http://git-wip-us.apache.org/repos/asf/incubator-sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-sentry/commit/e3b77f06 Tree: http://git-wip-us.apache.org/repos/asf/incubator-sentry/tree/e3b77f06 Diff: http://git-wip-us.apache.org/repos/asf/incubator-sentry/diff/e3b77f06 Branch: refs/heads/master Commit: e3b77f06eeb9f64a413a51df1294d7e881d315f9 Parents: 0619d8a Author: Colin Ma <co...@apache.org> Authored: Fri Mar 11 15:27:55 2016 +0800 Committer: Colin Ma <co...@apache.org> Committed: Fri Mar 11 15:27:55 2016 +0800 ---------------------------------------------------------------------- .../service/persistent/DelegateSentryStore.java | 2 +- .../persistent/PrivilegeOperatePersistence.java | 56 ++++- .../sentry/service/thrift/ServiceConstants.java | 4 + .../persistent/SentryStoreIntegrationBase.java | 2 +- .../TestPrivilegeOperatePersistence.java | 214 +++++++++++++++---- 5 files changed, 227 insertions(+), 51 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/e3b77f06/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java index 74c52fa..d51b3ba 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/DelegateSentryStore.java @@ -68,7 +68,7 @@ public class DelegateSentryStore implements SentryStoreLayer { public DelegateSentryStore(Configuration conf) throws SentryNoSuchObjectException, SentryAccessDeniedException, SentryConfigurationException, IOException { - this.privilegeOperator = new PrivilegeOperatePersistence(); + this.privilegeOperator = new PrivilegeOperatePersistence(conf); // The generic model doesn't turn on the thread that cleans hive privileges conf.set(ServerConfig.SENTRY_STORE_ORPHANED_PRIVILEGE_REMOVAL,"false"); this.conf = conf; http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/e3b77f06/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java index 21e51cd..9a3a505 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/persistent/PrivilegeOperatePersistence.java @@ -17,6 +17,7 @@ */ package org.apache.sentry.provider.db.generic.service.persistent; +import java.lang.reflect.Constructor; import java.util.ArrayList; import java.util.LinkedList; import java.util.List; @@ -26,6 +27,7 @@ import java.util.Set; import javax.jdo.PersistenceManager; import javax.jdo.Query; +import org.apache.hadoop.conf.Configuration; import org.apache.sentry.SentryUserException; import org.apache.sentry.core.common.Action; import org.apache.sentry.core.common.Authorizable; @@ -41,18 +43,28 @@ import com.google.common.base.Joiner; import com.google.common.base.Strings; import com.google.common.collect.Maps; import com.google.common.collect.Sets; +import org.apache.sentry.service.thrift.ServiceConstants; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * This class used do some operations related privilege and make the results * persistence */ public class PrivilegeOperatePersistence { + private static final Logger LOGGER = LoggerFactory.getLogger(PrivilegeOperatePersistence.class); private static final Map<String, BitFieldActionFactory> actionFactories = Maps.newHashMap(); static{ actionFactories.put("solr", new SearchActionFactory()); actionFactories.put("sqoop", new SqoopActionFactory()); } + private final Configuration conf; + + public PrivilegeOperatePersistence(Configuration conf) { + this.conf = conf; + } + public boolean checkPrivilegeOption(Set<MSentryRole> roles, PrivilegeObject privilege, PersistenceManager pm) { MSentryGMPrivilege requestPrivilege = convertToPrivilege(privilege); boolean hasGrant = false; @@ -418,7 +430,7 @@ public class PrivilegeOperatePersistence { } } - public static BitFieldAction getAction(String component, String name) { + private BitFieldAction getAction(String component, String name) { BitFieldActionFactory actionFactory = getActionFactory(component); BitFieldAction action = actionFactory.getActionByName(name); if (action == null) { @@ -427,10 +439,44 @@ public class PrivilegeOperatePersistence { return action; } - public static BitFieldActionFactory getActionFactory(String component) { - BitFieldActionFactory actionFactory = actionFactories.get(component.toLowerCase()); - if (actionFactory == null) { - throw new RuntimeException("can't get actionFactory for component:" + component); + private BitFieldActionFactory getActionFactory(String component) { + String caseInsensitiveComponent = component.toLowerCase(); + if (actionFactories.containsKey(caseInsensitiveComponent)) { + return actionFactories.get(caseInsensitiveComponent); + } + BitFieldActionFactory actionFactory = createActionFactory(caseInsensitiveComponent); + actionFactories.put(caseInsensitiveComponent, actionFactory); + LOGGER.info("Action factory for component {} not found in cache. Loaded it from configuration as {}.", + component, actionFactory.getClass().getName()); + return actionFactory; + } + + private BitFieldActionFactory createActionFactory(String component) { + String actionFactoryClassName = + conf.get(String.format(ServiceConstants.ServerConfig.SENTRY_COMPONENT_ACTION_FACTORY_FORMAT, component)); + if (actionFactoryClassName == null) { + throw new RuntimeException("ActionFactory not defined for component " + component + + ". Please define the parameter " + + "sentry." + component + ".action.factory in configuration"); + } + Class<?> actionFactoryClass; + try { + actionFactoryClass = Class.forName(actionFactoryClassName); + } catch (ClassNotFoundException e) { + throw new RuntimeException("ActionFactory class " + actionFactoryClassName + " not found."); + } + if (!BitFieldActionFactory.class.isAssignableFrom(actionFactoryClass)) { + throw new RuntimeException("ActionFactory class " + actionFactoryClassName + " must extend " + + BitFieldActionFactory.class.getName()); + } + BitFieldActionFactory actionFactory; + try { + Constructor<?> actionFactoryConstructor = actionFactoryClass.getDeclaredConstructor(); + actionFactoryConstructor.setAccessible(true); + actionFactory = (BitFieldActionFactory) actionFactoryClass.newInstance(); + } catch (NoSuchMethodException | InstantiationException | IllegalAccessException e) { + throw new RuntimeException("Could not instantiate actionFactory " + actionFactoryClassName + + " for component: " + component, e); } return actionFactory; } http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/e3b77f06/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java index 32d813c..94bd2a9 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/ServiceConstants.java @@ -171,7 +171,11 @@ public class ServiceConstants { // max message size for thrift messages public static String SENTRY_POLICY_SERVER_THRIFT_MAX_MESSAGE_SIZE = "sentry.policy.server.thrift.max.message.size"; public static long SENTRY_POLICY_SERVER_THRIFT_MAX_MESSAGE_SIZE_DEFAULT = 100 * 1024 * 1024; + + // action factories for external components + public static final String SENTRY_COMPONENT_ACTION_FACTORY_FORMAT = "sentry.%s.action.factory"; } + public static class ClientConfig { public static final ImmutableMap<String, String> SASL_PROPERTIES = ServiceConstants.SASL_PROPERTIES; public static final String SERVER_RPC_PORT = "sentry.service.client.server.rpc-port"; http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/e3b77f06/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreIntegrationBase.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreIntegrationBase.java b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreIntegrationBase.java index 915a929..f14b586 100644 --- a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreIntegrationBase.java +++ b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/SentryStoreIntegrationBase.java @@ -33,7 +33,7 @@ public abstract class SentryStoreIntegrationBase { protected final static String[] adminGroups = { "adminGroup" }; private static File dataDir; private static File policyFilePath; - private static Configuration conf; + protected static Configuration conf; protected static DelegateSentryStore sentryStore; protected static PolicyFile policyFile; http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/e3b77f06/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java index 6b3a5e2..7541cb7 100644 --- a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java +++ b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/persistent/TestPrivilegeOperatePersistence.java @@ -21,15 +21,23 @@ import static junit.framework.Assert.assertEquals; import static junit.framework.Assert.fail; import java.util.Arrays; +import java.util.Collections; import java.util.List; +import com.google.common.collect.Lists; +import org.apache.hadoop.conf.Configuration; +import org.apache.sentry.SentryUserException; import org.apache.sentry.core.common.Authorizable; +import org.apache.sentry.core.common.BitFieldAction; +import org.apache.sentry.core.common.BitFieldActionFactory; import org.apache.sentry.core.model.search.Collection; import org.apache.sentry.core.model.search.Field; import org.apache.sentry.core.model.search.SearchConstants; +import org.apache.sentry.core.model.sqoop.SqoopActionConstant; import org.apache.sentry.provider.db.SentryGrantDeniedException; import org.apache.sentry.provider.db.generic.service.persistent.PrivilegeObject.Builder; import org.apache.sentry.provider.file.PolicyFile; +import org.apache.sentry.service.thrift.ServiceConstants; import org.junit.Before; import org.junit.Test; @@ -68,50 +76,7 @@ public class TestPrivilegeOperatePersistence extends SentryStoreIntegrationBase */ @Test public void testGrantPrivilege() throws Exception { - String roleName = "r1"; - /** - * grantor is admin, there is no need to check grant option - */ - String grantor = ADMIN_USER; - PrivilegeObject queryPrivilege = new Builder() - .setComponent(SEARCH) - .setAction(SearchConstants.QUERY) - .setService(SERVICE) - .setAuthorizables(Arrays.asList(new Collection(COLLECTION_NAME))) - .withGrantOption(null) - .build(); - - sentryStore.createRole(SEARCH, roleName, grantor); - sentryStore.alterRoleGrantPrivilege(SEARCH, roleName, queryPrivilege, grantor); - - assertEquals(Sets.newHashSet(queryPrivilege), - sentryStore.getPrivilegesByRole(SEARCH, Sets.newHashSet(roleName))); - - PrivilegeObject queryPrivilegeWithOption = new Builder() - .setComponent(SEARCH) - .setAction(SearchConstants.QUERY) - .setService(SERVICE) - .setAuthorizables(Arrays.asList(new Collection(COLLECTION_NAME))) - .withGrantOption(true) - .build(); - - sentryStore.alterRoleGrantPrivilege(SEARCH, roleName, queryPrivilegeWithOption, grantor); - - assertEquals(Sets.newHashSet(queryPrivilege, queryPrivilegeWithOption), - sentryStore.getPrivilegesByRole(SEARCH, Sets.newHashSet(roleName))); - - PrivilegeObject queryPrivilegeWithNoOption = new Builder() - .setComponent(SEARCH) - .setAction(SearchConstants.QUERY) - .setService(SERVICE) - .setAuthorizables(Arrays.asList(new Collection(COLLECTION_NAME))) - .withGrantOption(false) - .build(); - - sentryStore.alterRoleGrantPrivilege(SEARCH, roleName, queryPrivilegeWithNoOption, grantor); - - assertEquals(Sets.newHashSet(queryPrivilege, queryPrivilegeWithOption, queryPrivilegeWithNoOption), - sentryStore.getPrivilegesByRole(SEARCH, Sets.newHashSet(roleName))); + testGrantPrivilege(sentryStore, SEARCH); } @Test @@ -1008,4 +973,165 @@ public class TestPrivilegeOperatePersistence extends SentryStoreIntegrationBase assertEquals(2, sentryStore.getPrivilegesByAuthorizable(SEARCH, service1, Sets.newHashSet(roleName1,roleName2, roleName3), null).size()); } + + @Test(expected = RuntimeException.class) + public void testGrantPrivilegeExternalComponentMissingConf() throws SentryUserException { + testGrantPrivilege(sentryStore, "externalComponent"); + } + + @Test(expected = RuntimeException.class) + public void testGrantPrivilegeExternalComponentInvalidConf() throws Exception { + String externalComponent = "mycomponent"; + Configuration confCopy = new Configuration(conf); + confCopy.set(String.format(ServiceConstants.ServerConfig.SENTRY_COMPONENT_ACTION_FACTORY_FORMAT, externalComponent), + InvalidActionFactory.class.getName()); + SentryStoreLayer store = new DelegateSentryStore(confCopy); + testGrantPrivilege(store, externalComponent); + } + + @Test + public void testGrantPrivilegeExternalComponent() throws Exception { + String externalComponent = "mycomponent"; + Configuration confCopy = new Configuration(conf); + confCopy.set(String.format(ServiceConstants.ServerConfig.SENTRY_COMPONENT_ACTION_FACTORY_FORMAT, externalComponent), + MyComponentActionFactory.class.getName()); + SentryStoreLayer store = new DelegateSentryStore(confCopy); + testGrantPrivilege(store, externalComponent); + } + + @Test + public void testGrantPrivilegeExternalComponentCaseInsensitivity() throws Exception { + String externalComponent = "MyCoMpOnEnT"; + Configuration confCopy = new Configuration(conf); + confCopy.set(String.format(ServiceConstants.ServerConfig.SENTRY_COMPONENT_ACTION_FACTORY_FORMAT, "mycomponent"), + MyComponentActionFactory.class.getName()); + SentryStoreLayer store = new DelegateSentryStore(confCopy); + testGrantPrivilege(store, externalComponent); + } + + private void testGrantPrivilege(SentryStoreLayer sentryStore, String component) throws SentryUserException { + String roleName = "r1"; + /** + * grantor is admin, there is no need to check grant option + */ + String grantor = ADMIN_USER; + PrivilegeObject queryPrivilege = new Builder() + .setComponent(component) + .setAction(SearchConstants.QUERY) + .setService(SERVICE) + .setAuthorizables(Collections.singletonList(new Collection(COLLECTION_NAME))) + .withGrantOption(null) + .build(); + + sentryStore.createRole(component, roleName, grantor); + sentryStore.alterRoleGrantPrivilege(component, roleName, queryPrivilege, grantor); + + assertEquals(Sets.newHashSet(queryPrivilege), + sentryStore.getPrivilegesByRole(component, Sets.newHashSet(roleName))); + + PrivilegeObject queryPrivilegeWithOption = new Builder() + .setComponent(component) + .setAction(SearchConstants.QUERY) + .setService(SERVICE) + .setAuthorizables(Collections.singletonList(new Collection(COLLECTION_NAME))) + .withGrantOption(true) + .build(); + + sentryStore.alterRoleGrantPrivilege(component, roleName, queryPrivilegeWithOption, grantor); + + assertEquals(Sets.newHashSet(queryPrivilege, queryPrivilegeWithOption), + sentryStore.getPrivilegesByRole(component, Sets.newHashSet(roleName))); + + PrivilegeObject queryPrivilegeWithNoOption = new Builder() + .setComponent(component) + .setAction(SearchConstants.QUERY) + .setService(SERVICE) + .setAuthorizables(Collections.singletonList(new Collection(COLLECTION_NAME))) + .withGrantOption(false) + .build(); + + sentryStore.alterRoleGrantPrivilege(component, roleName, queryPrivilegeWithNoOption, grantor); + + assertEquals(Sets.newHashSet(queryPrivilege, queryPrivilegeWithOption, queryPrivilegeWithNoOption), + sentryStore.getPrivilegesByRole(component, Sets.newHashSet(roleName))); + } + + public static final class InvalidActionFactory { + + } + + public static final class MyComponentActionFactory extends BitFieldActionFactory { + + public enum MyComponentActionType { + FOO("foo", 1), + BAR("bar", 2), + QUERY(SearchConstants.QUERY, 4), + ALL("*", FOO.getCode() | BAR.getCode() | QUERY.getCode()); + + private String name; + private int code; + MyComponentActionType(String name, int code) { + this.name = name; + this.code = code; + } + + public int getCode() { + return code; + } + + public String getName() { + return name; + } + + static MyComponentActionType getActionByName(String name) { + for (MyComponentActionType action : MyComponentActionType.values()) { + if (action.name.equalsIgnoreCase(name)) { + return action; + } + } + throw new RuntimeException("can't get MyComponentActionType by name:" + name); + } + + static List<MyComponentActionType> getActionByCode(int code) { + List<MyComponentActionType> actions = Lists.newArrayList(); + for (MyComponentActionType action : MyComponentActionType.values()) { + if ((action.code & code) == action.code && action != MyComponentActionType.ALL) { + //MyComponentActionType.ALL action should not return in the list + actions.add(action); + } + } + if (actions.isEmpty()) { + throw new RuntimeException("can't get sqoopActionType by code:" + code); + } + return actions; + } + } + + public static class MyComponentAction extends BitFieldAction { + public MyComponentAction(String name) { + this(MyComponentActionType.getActionByName(name)); + } + public MyComponentAction(MyComponentActionType myComponentActionType) { + super(myComponentActionType.name, myComponentActionType.code); + } + } + + @Override + public List<? extends BitFieldAction> getActionsByCode(int actionCode) { + List<MyComponentAction> actions = Lists.newArrayList(); + for (MyComponentActionType action : MyComponentActionType.getActionByCode(actionCode)) { + actions.add(new MyComponentAction(action)); + } + return actions; + } + + @Override + public BitFieldAction getActionByName(String name) { + // Check the name is All + if (SqoopActionConstant.ALL_NAME.equalsIgnoreCase(name)) { + return new MyComponentAction(MyComponentActionType.ALL); + } + return new MyComponentAction(name); + } + } }
