SENTRY-1744: Simplify creation of DelegateSentryStore (Alex Kolbasov, reviewed by Vamsee Yarlagadda and Na Li)
Change-Id: I11959e4a3f1f0c2395d4c87eca9d2df5aea68a19 Reviewed-on: http://gerrit.sjc.cloudera.com:8080/23406 Tested-by: Jenkins User Reviewed-by: Alexander Kolbasov <[email protected]> Project: http://git-wip-us.apache.org/repos/asf/sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/ad806e3d Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/ad806e3d Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/ad806e3d Branch: refs/for/cdh5-1.5.1_ha Commit: ad806e3dbfc637a3c72d97e0290bd58124147ff7 Parents: acc1330 Author: Alexander Kolbasov <[email protected]> Authored: Fri Jun 2 17:25:05 2017 -0700 Committer: Alexander Kolbasov <[email protected]> Committed: Fri Jun 2 19:11:56 2017 -0700 ---------------------------------------------------------------------- .../core/common/utils/PolicyStoreConstants.java | 3 -- .../service/persistent/DelegateSentryStore.java | 2 +- .../thrift/SentryGenericPolicyProcessor.java | 45 +++++++------------- .../TestSentryGenericPolicyProcessor.java | 7 --- 4 files changed, 16 insertions(+), 41 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sentry/blob/ad806e3d/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PolicyStoreConstants.java ---------------------------------------------------------------------- diff --git a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PolicyStoreConstants.java b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PolicyStoreConstants.java index 8f73d01..797aead 100644 --- a/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PolicyStoreConstants.java +++ b/sentry-core/sentry-core-common/src/main/java/org/apache/sentry/core/common/utils/PolicyStoreConstants.java @@ -19,9 +19,6 @@ package org.apache.sentry.core.common.utils; public class PolicyStoreConstants { public static final String SENTRY_GENERIC_POLICY_NOTIFICATION = "sentry.generic.policy.notification"; - public static final String SENTRY_GENERIC_POLICY_STORE = "sentry.generic.policy.store"; - public static final String SENTRY_GENERIC_POLICY_STORE_DEFAULT = - "org.apache.sentry.provider.db.generic.service.persistent.DelegateSentryStore"; public static class PolicyStoreServerConfig { public static final String NOTIFICATION_HANDLERS = "sentry.policy.store.notification.handlers"; } http://git-wip-us.apache.org/repos/asf/sentry/blob/ad806e3d/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 30ad4db..c7e7ba1 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 @@ -61,7 +61,7 @@ public class DelegateSentryStore implements SentryStoreLayer { private Set<String> adminGroups; private PrivilegeOperatePersistence privilegeOperator; - DelegateSentryStore(Configuration conf) throws Exception { + public DelegateSentryStore(Configuration conf) throws Exception { this.privilegeOperator = new PrivilegeOperatePersistence(conf); this.conf = conf; //delegated old sentryStore http://git-wip-us.apache.org/repos/asf/sentry/blob/ad806e3d/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java index 97c0e1d..b5e16d2 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericPolicyProcessor.java @@ -22,6 +22,7 @@ import static org.apache.sentry.provider.common.ProviderConstants.KV_JOINER; import static org.apache.sentry.provider.common.ProviderConstants.AUTHORIZABLE_SPLITTER; import java.lang.reflect.Constructor; +import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -39,6 +40,7 @@ import org.apache.sentry.provider.db.SentryAlreadyExistsException; import org.apache.sentry.provider.db.SentryInvalidInputException; import org.apache.sentry.provider.db.SentryNoSuchObjectException; import org.apache.sentry.provider.db.SentryThriftAPIMismatchException; +import org.apache.sentry.provider.db.generic.service.persistent.DelegateSentryStore; import org.apache.sentry.provider.db.generic.service.persistent.PrivilegeObject; import org.apache.sentry.provider.db.generic.service.persistent.PrivilegeObject.Builder; import org.apache.sentry.provider.db.generic.service.persistent.SentryStoreLayer; @@ -78,8 +80,8 @@ public class SentryGenericPolicyProcessor implements SentryGenericPolicyService. public static final String SENTRY_GENERIC_SERVICE_NAME = "SentryGenericPolicyService"; private static final String ACCESS_DENIAL_MESSAGE = "Access denied to "; - public SentryGenericPolicyProcessor(Configuration conf) throws Exception { - this.store = createStore(conf); + SentryGenericPolicyProcessor(Configuration conf) throws Exception { + this.store = new DelegateSentryStore(conf); this.handerInvoker = new NotificationHandlerInvoker(createHandlers(conf)); this.conf = conf; adminGroups = ImmutableSet.copyOf((Sets.newHashSet(conf.getStrings( @@ -87,7 +89,7 @@ public class SentryGenericPolicyProcessor implements SentryGenericPolicyService. } @VisibleForTesting - public SentryGenericPolicyProcessor(Configuration conf, SentryStoreLayer store) throws Exception { + SentryGenericPolicyProcessor(Configuration conf, SentryStoreLayer store) throws Exception { this.store = store; this.handerInvoker = new NotificationHandlerInvoker(createHandlers(conf)); this.conf = conf; @@ -106,10 +108,10 @@ public class SentryGenericPolicyProcessor implements SentryGenericPolicyService. } private Set<String> toTrimmedLower(Set<String> s) { - if (null == s) { - return new HashSet<String>(); + if (s == null) { + return Collections.emptySet(); } - Set<String> result = Sets.newHashSet(); + Set<String> result = new HashSet<>(s.size()); for (String v : s) { result.add(v.trim().toLowerCase()); } @@ -117,10 +119,10 @@ public class SentryGenericPolicyProcessor implements SentryGenericPolicyService. } private Set<String> toTrimmed(Set<String> s) { - if (null == s) { - return new HashSet<String>(); + if (s == null) { + return Collections.emptySet(); } - Set<String> result = Sets.newHashSet(); + Set<String> result = new HashSet<>(s.size()); for (String v : s) { result.add(v.trim()); } @@ -134,32 +136,15 @@ public class SentryGenericPolicyProcessor implements SentryGenericPolicyService. return s.trim().toLowerCase(); } - public static Set<String> getRequestorGroups(Configuration conf, String userName) throws SentryUserException { + private static Set<String> getRequestorGroups(Configuration conf, String userName) throws SentryUserException { return SentryPolicyStoreProcessor.getGroupsFromUserName(conf, userName); } private boolean inAdminGroups(Set<String> requestorGroups) { - if (Sets.intersection(adminGroups, requestorGroups).isEmpty()) { - return false; - } else return true; + return !Sets.intersection(adminGroups, requestorGroups).isEmpty(); } - public static SentryStoreLayer createStore(Configuration conf) throws SentryConfigurationException { - SentryStoreLayer storeLayer = null; - String Store = conf.get(PolicyStoreConstants.SENTRY_GENERIC_POLICY_STORE, PolicyStoreConstants.SENTRY_GENERIC_POLICY_STORE_DEFAULT); - - if (Strings.isNullOrEmpty(Store)) { - throw new SentryConfigurationException("the parameter configuration for sentry.generic.policy.store can't be empty"); - } - try { - storeLayer = createInstance(Store, conf, SentryStoreLayer.class); - } catch (Exception e) { - throw new SentryConfigurationException("Create sentryStore error: " + e.getMessage(), e); - } - return storeLayer; - } - - public static List<NotificationHandler> createHandlers(Configuration conf) throws SentryConfigurationException { + static List<NotificationHandler> createHandlers(Configuration conf) throws SentryConfigurationException { List<NotificationHandler> handlers = Lists.newArrayList(); Iterable<String> notificationHandlers = Splitter.onPattern("[\\s,]").trimResults() @@ -175,7 +160,7 @@ public class SentryGenericPolicyProcessor implements SentryGenericPolicyService. } @SuppressWarnings("unchecked") - public static <T> T createInstance(String className, Configuration conf, Class<T> iface) throws Exception { + private static <T> T createInstance(String className, Configuration conf, Class<T> iface) throws Exception { T result; try { Class clazz = Class.forName(className); http://git-wip-us.apache.org/repos/asf/sentry/blob/ad806e3d/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java index 4411bdd..924be4f 100644 --- a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java +++ b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/generic/service/thrift/TestSentryGenericPolicyProcessor.java @@ -301,13 +301,6 @@ public class TestSentryGenericPolicyProcessor { SentryGenericPolicyProcessor.createHandlers(conf); } - @Test(expected=SentryConfigurationException.class) - public void testConfigCannotCreateSentryStore() throws Exception { - Configuration conf = new Configuration(); - conf.set(PolicyStoreConstants.SENTRY_GENERIC_POLICY_STORE,"junk"); - SentryGenericPolicyProcessor.createStore(conf); - } - public static class MockGroupMapping implements GroupMappingService { public MockGroupMapping(Configuration conf, String resource) { }
