Repository: sentry Updated Branches: refs/heads/master 1cbf44ade -> eceaaf8e0
SENTRY-1229: Add caching to SentryGenericProviderBackend (Ashish K Singh, Reviewed By: Gregory Chanan) Project: http://git-wip-us.apache.org/repos/asf/sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/eceaaf8e Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/eceaaf8e Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/eceaaf8e Branch: refs/heads/master Commit: eceaaf8e0f83930a28e81d9380e5f75e3ebc4019 Parents: 1cbf44a Author: Gregory Chanan <[email protected]> Authored: Sun May 15 19:14:57 2016 -0700 Committer: Gregory Chanan <[email protected]> Committed: Sun May 15 19:14:57 2016 -0700 ---------------------------------------------------------------------- .../sentry/provider/common/CacheProvider.java | 68 ++++++++ .../sentry/provider/common/ProviderBackend.java | 2 +- .../sentry/provider/common/TableCache.java | 25 +++ .../generic/SentryGenericProviderBackend.java | 118 ++++++++----- .../provider/db/generic/UpdatableCache.java | 171 +++++++++++++++++++ .../thrift/SentryGenericServiceClient.java | 3 +- .../SentryGenericServiceClientDefaultImpl.java | 1 - .../sentry/service/thrift/ServiceConstants.java | 9 + .../file/SimpleFileProviderBackend.java | 105 +++--------- .../e2e/kafka/AbstractKafkaSentryTestBase.java | 13 +- .../sentry/tests/e2e/kafka/TestAuthorize.java | 1 + 11 files changed, 393 insertions(+), 123 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sentry/blob/eceaaf8e/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/CacheProvider.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/CacheProvider.java b/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/CacheProvider.java new file mode 100644 index 0000000..d50a0bc --- /dev/null +++ b/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/CacheProvider.java @@ -0,0 +1,68 @@ +/** + * 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.sentry.provider.common; + +import com.google.common.collect.ImmutableSet; +import org.apache.sentry.core.common.ActiveRoleSet; +import org.apache.sentry.core.common.Authorizable; + +import java.util.Map; +import java.util.Set; + +public class CacheProvider { + private TableCache cache; + private volatile boolean initialized = false; + + public void initialize(TableCache cache) { + if (initialized) { + throw new IllegalStateException("CacheProvider has already been initialized, cannot be initialized twice."); + } + this.cache = cache; + this.initialized = true; + } + + public ImmutableSet<String> getPrivileges(Set<String> groups, ActiveRoleSet roleSet, + Authorizable... authorizableHierarchy) { + if (!initialized) { + throw new IllegalStateException("CacheProvider has not been properly initialized"); + } + ImmutableSet.Builder<String> resultBuilder = ImmutableSet.builder(); + for (String groupName : groups) { + for (Map.Entry<String, Set<String>> row : cache.getCache().row(groupName).entrySet()) { + if (roleSet.containsRole(row.getKey())) { + // TODO: SENTRY-1245: Filter by Authorizables, if provided + resultBuilder.addAll(row.getValue()); + } + } + } + return resultBuilder.build(); + } + + public ImmutableSet<String> getRoles(Set<String> groups, ActiveRoleSet roleSet) { + if (!initialized) { + throw new IllegalStateException("CacheProvider has not been properly initialized"); + } + ImmutableSet.Builder<String> resultBuilder = ImmutableSet.builder(); + if (groups != null) { + for (String groupName : groups) { + for (Map.Entry<String, Set<String>> row : cache.getCache().row(groupName) + .entrySet()) { + if (roleSet.containsRole(row.getKey())) { + resultBuilder.add(row.getKey()); + } + } + } + } + return resultBuilder.build(); + } +} http://git-wip-us.apache.org/repos/asf/sentry/blob/eceaaf8e/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ProviderBackend.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ProviderBackend.java b/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ProviderBackend.java index ffd3af4..c78718c 100644 --- a/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ProviderBackend.java +++ b/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/ProviderBackend.java @@ -40,7 +40,7 @@ public interface ProviderBackend { * policy engine knows the validators. Ideally we could change but since * both the policy engine and backend are exposed via configuration properties * that would be backwards incompatible. - * @param validators + * @param context */ void initialize(ProviderBackendContext context); http://git-wip-us.apache.org/repos/asf/sentry/blob/eceaaf8e/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/TableCache.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/TableCache.java b/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/TableCache.java new file mode 100644 index 0000000..3285c1b --- /dev/null +++ b/sentry-provider/sentry-provider-common/src/main/java/org/apache/sentry/provider/common/TableCache.java @@ -0,0 +1,25 @@ +/** + * 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 + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> + * 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.sentry.provider.common; + +import com.google.common.collect.Table; + +import java.util.Set; + +public interface TableCache { + /** + * Returns backing cache. Caller must not modify the returned cache. + * @return backing cache. + */ + Table<String, String, Set<String>> getCache(); +} http://git-wip-us.apache.org/repos/asf/sentry/blob/eceaaf8e/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java index 222b6fd..6de0a54 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/SentryGenericProviderBackend.java @@ -17,6 +17,8 @@ */ package org.apache.sentry.provider.db.generic; +import java.lang.reflect.Constructor; +import java.lang.reflect.InvocationTargetException; import java.util.Arrays; import java.util.Set; @@ -26,11 +28,14 @@ import org.apache.sentry.SentryUserException; import org.apache.sentry.core.common.ActiveRoleSet; import org.apache.sentry.core.common.Authorizable; import org.apache.sentry.core.common.SentryConfigurationException; +import org.apache.sentry.provider.common.CacheProvider; import org.apache.sentry.provider.common.ProviderBackend; import org.apache.sentry.provider.common.ProviderBackendContext; import org.apache.sentry.provider.db.generic.service.thrift.SentryGenericServiceClient; import org.apache.sentry.provider.db.generic.service.thrift.SentryGenericServiceClientFactory; import org.apache.sentry.provider.db.generic.service.thrift.TSentryRole; +import org.apache.sentry.provider.db.generic.tools.command.TSentryPrivilegeConvertor; +import org.apache.sentry.service.thrift.ServiceConstants; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -40,18 +45,22 @@ import com.google.common.collect.Sets; /** * This class used when any component such as Hive, Solr or Sqoop want to integration with the Sentry service */ -public class SentryGenericProviderBackend implements ProviderBackend { +public class SentryGenericProviderBackend extends CacheProvider implements ProviderBackend { private static final Logger LOGGER = LoggerFactory.getLogger(SentryGenericProviderBackend.class); private final Configuration conf; private volatile boolean initialized = false; private String componentType; private String serviceName; + private boolean enableCaching; + private String privilegeConverter; // ProviderBackend should have the same construct to support the reflect in authBinding, // eg:SqoopAuthBinding public SentryGenericProviderBackend(Configuration conf, String resource) //NOPMD throws Exception { this.conf = conf; + this.enableCaching = conf.getBoolean(ServiceConstants.ClientConfig.ENABLE_CACHING, ServiceConstants.ClientConfig.ENABLE_CACHING_DEFAULT); + this.privilegeConverter = conf.get(ServiceConstants.ClientConfig.PRIVILEGE_CONVERTER); } @Override @@ -59,6 +68,28 @@ public class SentryGenericProviderBackend implements ProviderBackend { if (initialized) { throw new IllegalStateException("SentryGenericProviderBackend has already been initialized, cannot be initialized twice"); } + if (enableCaching) { + if (privilegeConverter == null) { + throw new SentryConfigurationException(ServiceConstants.ClientConfig.PRIVILEGE_CONVERTER + " not configured."); + } + + Constructor<?> privilegeConverterConstructor; + TSentryPrivilegeConvertor sentryPrivilegeConvertor; + try { + privilegeConverterConstructor = Class.forName(privilegeConverter).getDeclaredConstructor(String.class, String.class); + privilegeConverterConstructor.setAccessible(true); + sentryPrivilegeConvertor = (TSentryPrivilegeConvertor) privilegeConverterConstructor.newInstance(getComponentType(), getServiceName()); + } catch (NoSuchMethodException | ClassNotFoundException | InstantiationException | InvocationTargetException | IllegalAccessException e) { + throw new RuntimeException("Failed to create privilege converter of type " + privilegeConverter, e); + } + UpdatableCache cache = new UpdatableCache(conf, getComponentType(), getServiceName(), sentryPrivilegeConvertor); + try { + cache.startUpdateThread(true); + } catch (Exception e) { + throw new RuntimeException("Failed to get privileges from Sentry to build cache.", e); + } + super.initialize(cache); + } this.initialized = true; } @@ -76,20 +107,24 @@ public class SentryGenericProviderBackend implements ProviderBackend { if (!initialized) { throw new IllegalStateException("SentryGenericProviderBackend has not been properly initialized"); } - SentryGenericServiceClient client = null; - try { - client = getClient(); - return ImmutableSet.copyOf(client.listPrivilegesForProvider(componentType, serviceName, - roleSet, groups, Arrays.asList(authorizableHierarchy))); - } catch (SentryUserException e) { - String msg = "Unable to obtain privileges from server: " + e.getMessage(); - LOGGER.error(msg, e); - } catch (Exception e) { - String msg = "Unable to obtain client:" + e.getMessage(); - LOGGER.error(msg, e); - } finally { - if (client != null) { - client.close(); + if (enableCaching) { + return super.getPrivileges(groups, roleSet, authorizableHierarchy); + } else { + SentryGenericServiceClient client = null; + try { + client = getClient(); + return ImmutableSet.copyOf(client.listPrivilegesForProvider(componentType, serviceName, + roleSet, groups, Arrays.asList(authorizableHierarchy))); + } catch (SentryUserException e) { + String msg = "Unable to obtain privileges from server: " + e.getMessage(); + LOGGER.error(msg, e); + } catch (Exception e) { + String msg = "Unable to obtain client:" + e.getMessage(); + LOGGER.error(msg, e); + } finally { + if (client != null) { + client.close(); + } } } return ImmutableSet.of(); @@ -100,32 +135,36 @@ public class SentryGenericProviderBackend implements ProviderBackend { if (!initialized) { throw new IllegalStateException("SentryGenericProviderBackend has not been properly initialized"); } - SentryGenericServiceClient client = null; - try { - Set<TSentryRole> tRoles = Sets.newHashSet(); - client = getClient(); - //get the roles according to group - String requestor = UserGroupInformation.getCurrentUser().getShortUserName(); - for (String group : groups) { - tRoles.addAll(client.listRolesByGroupName(requestor, group, getComponentType())); - } - Set<String> roles = Sets.newHashSet(); - for (TSentryRole tRole : tRoles) { - roles.add(tRole.getRoleName()); - } - return ImmutableSet.copyOf(roleSet.isAll() ? roles : Sets.intersection(roles, roleSet.getRoles())); - } catch (SentryUserException e) { - String msg = "Unable to obtain roles from server: " + e.getMessage(); - LOGGER.error(msg, e); - } catch (Exception e) { - String msg = "Unable to obtain client:" + e.getMessage(); - LOGGER.error(msg, e); - } finally { - if (client != null) { - client.close(); + if (enableCaching) { + return super.getRoles(groups, roleSet); + } else { + SentryGenericServiceClient client = null; + try { + Set<TSentryRole> tRoles = Sets.newHashSet(); + client = getClient(); + //get the roles according to group + String requestor = UserGroupInformation.getCurrentUser().getShortUserName(); + for (String group : groups) { + tRoles.addAll(client.listRolesByGroupName(requestor, group, getComponentType())); + } + Set<String> roles = Sets.newHashSet(); + for (TSentryRole tRole : tRoles) { + roles.add(tRole.getRoleName()); + } + return ImmutableSet.copyOf(roleSet.isAll() ? roles : Sets.intersection(roles, roleSet.getRoles())); + } catch (SentryUserException e) { + String msg = "Unable to obtain roles from server: " + e.getMessage(); + LOGGER.error(msg, e); + } catch (Exception e) { + String msg = "Unable to obtain client:" + e.getMessage(); + LOGGER.error(msg, e); + } finally { + if (client != null) { + client.close(); + } } + return ImmutableSet.of(); } - return ImmutableSet.of(); } /** @@ -165,5 +204,4 @@ public class SentryGenericProviderBackend implements ProviderBackend { public void setServiceName(String serviceName) { this.serviceName = serviceName; } - } http://git-wip-us.apache.org/repos/asf/sentry/blob/eceaaf8e/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java new file mode 100644 index 0000000..ccb349b --- /dev/null +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/UpdatableCache.java @@ -0,0 +1,171 @@ +/** + * 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.sentry.provider.db.generic; + +import com.google.common.collect.Table; +import com.google.common.collect.HashBasedTable; +import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.security.UserGroupInformation; +import org.apache.sentry.provider.common.TableCache; +import org.apache.sentry.provider.db.generic.service.thrift.*; +import org.apache.sentry.provider.db.generic.tools.command.TSentryPrivilegeConvertor; +import org.apache.sentry.service.thrift.ServiceConstants; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.HashSet; +import java.util.Set; +import java.util.Timer; +import java.util.TimerTask; +import java.util.concurrent.TimeUnit; + +class UpdatableCache implements TableCache { + private static final Logger LOGGER = LoggerFactory.getLogger(UpdatableCache.class); + + private final String componentType; + private final String serviceName; + private final long cacheTtlNs; + private final int allowedUpdateFailuresCount; + private final Configuration conf; + private final TSentryPrivilegeConvertor tSentryPrivilegeConvertor; + + private volatile long lastRefreshedNs = 0; + private int consecutiveUpdateFailuresCount = 0; + /** + * Sparse table where group is the row key and role is the cell. + * The value is the set of privileges located in the cell. For example, + * the following table would be generated for a policy where Group 1 + * has Role 1 and Role 2 while Group 2 has only Role 2. + * <table border="1"> + * <tbody> + * <tr> + * <td><!-- empty --></td> + * <td>Role 1</td> + * <td>Role 2</td> + * </tr> + * <tr> + * <td>Group 1</td> + * <td>Priv 1</td> + * <td>Priv 2, Priv 3</td> + * </tr> + * <tr> + * <td>Group 2</td> + * <td><!-- empty --></td> + * <td>Priv 2, Priv 3</td> + * </tr> + * </tbody> + * </table> + */ + private volatile Table<String, String, Set<String>> table; + + UpdatableCache(Configuration conf, String componentType, String serviceName, TSentryPrivilegeConvertor tSentryPrivilegeConvertor) { + this.conf = conf; + this.componentType = componentType; + this.serviceName = serviceName; + this.tSentryPrivilegeConvertor = tSentryPrivilegeConvertor; + + // check caching configuration + this.cacheTtlNs = TimeUnit.MILLISECONDS.toNanos(conf.getLong(ServiceConstants.ClientConfig.CACHE_TTL_MS, ServiceConstants.ClientConfig.CACHING_TTL_MS_DEFAULT)); + this.allowedUpdateFailuresCount = conf.getInt(ServiceConstants.ClientConfig.CACHE_UPDATE_FAILURES_BEFORE_PRIV_REVOKE, ServiceConstants.ClientConfig.CACHE_UPDATE_FAILURES_BEFORE_PRIV_REVOKE_DEFAULT); + } + + @Override + public Table<String, String, Set<String>> getCache() { + return table; + } + + /** + * Build cache replica with latest values + * + * @return cache replica with latest values + */ + private Table<String, String, Set<String>> loadFromRemote() throws Exception { + Table<String, String, Set<String>> tempCache = HashBasedTable.create(); + String requestor; + requestor = UserGroupInformation.getLoginUser().getShortUserName(); + + final SentryGenericServiceClient client = getClient(); + final Set<TSentryRole> tSentryRoles = client.listAllRoles(requestor, componentType); + + for (TSentryRole tSentryRole : tSentryRoles) { + final String roleName = tSentryRole.getRoleName(); + final Set<TSentryPrivilege> tSentryPrivileges = client.listPrivilegesByRoleName(requestor, roleName, componentType, serviceName); + for (String group : tSentryRole.getGroups()) { + Set<String> currentPrivileges = tempCache.get(group, roleName); + if (currentPrivileges == null) { + currentPrivileges = new HashSet<>(); + tempCache.put(group, roleName, currentPrivileges); + } + for (TSentryPrivilege tSentryPrivilege : tSentryPrivileges) { + currentPrivileges.add(tSentryPrivilegeConvertor.toString(tSentryPrivilege)); + } + } + } + return tempCache; + } + + /** + * The Sentry-296(generate client for connection pooling) has already finished development and reviewed by now. When it + * was committed to master, the getClient method was needed to refactor using the connection pool + * + * TODO: Avoid creating new client each time. + */ + private SentryGenericServiceClient getClient() throws Exception { + return SentryGenericServiceClientFactory.create(conf); + } + + void startUpdateThread(boolean blockUntilFirstReload) throws Exception { + if (blockUntilFirstReload) { + reloadData(); + } + + Timer timer = new Timer(); + final long refreshIntervalMs = TimeUnit.NANOSECONDS.toMillis(cacheTtlNs); + timer.scheduleAtFixedRate( + new TimerTask() { + public void run() { + if (shouldRefresh()) { + try { + LOGGER.debug("Loading all data."); + reloadData(); + } catch (Exception e) { + LOGGER.warn("Exception while updating data from DB", e); + revokeAllPrivilegesIfRequired(); + } + } + } + }, + blockUntilFirstReload ? refreshIntervalMs : 0, + refreshIntervalMs); + } + + private void revokeAllPrivilegesIfRequired() { + if (++consecutiveUpdateFailuresCount > allowedUpdateFailuresCount) { + // Clear cache to revoke all privileges. + // Update table cache to point to an empty table to avoid thread-unsafe characteristics of HashBasedTable. + this.table = HashBasedTable.create(); + LOGGER.error("Failed to update roles and privileges cache for " + consecutiveUpdateFailuresCount + " times." + + " Revoking all privileges from cache, which will cause all authorization requests to fail."); + } + } + + private void reloadData() throws Exception { + this.table = loadFromRemote(); + lastRefreshedNs = System.nanoTime(); + } + + private boolean shouldRefresh() { + final long currentTimeNs = System.nanoTime(); + return lastRefreshedNs + cacheTtlNs < currentTimeNs; + } +} http://git-wip-us.apache.org/repos/asf/sentry/blob/eceaaf8e/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java index 76ff15b..bf87d8b 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClient.java @@ -100,7 +100,6 @@ public interface SentryGenericServiceClient { /** * drop privilege * @param requestorUserName: user on whose behalf the request is issued - * @param roleName: Name of the role * @param component: The request is issued to which component * @param privilege * @throws SentryUserException @@ -142,7 +141,7 @@ public interface SentryGenericServiceClient { throws SentryUserException; /** - * Gets sentry privileges for a given roleName and Authorizable Hirerchys using the Sentry service + * Gets sentry privileges for a given roleName and Authorizable Hierarchy using the Sentry service * @param requestorUserName: user on whose behalf the request is issued * @param roleName: * @param component: The request is issued to which component http://git-wip-us.apache.org/repos/asf/sentry/blob/eceaaf8e/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java index 74b6963..744dcd7 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/service/thrift/SentryGenericServiceClientDefaultImpl.java @@ -350,7 +350,6 @@ public class SentryGenericServiceClientDefaultImpl implements SentryGenericServi /** * drop privilege * @param requestorUserName: user on whose behalf the request is issued - * @param roleName: Name of the role * @param component: The request is issued to which component * @param privilege * @throws SentryUserException http://git-wip-us.apache.org/repos/asf/sentry/blob/eceaaf8e/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 00e3fbd..d7ccc45 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 @@ -226,6 +226,15 @@ public class ServiceConstants { public static final int RETRY_COUNT_DEFAULT = 3; public static final String RETRY_INTERVAL_SEC_CONF = "sentry.provider.backend.db.retry.interval.seconds"; public static final int RETRY_INTERVAL_SEC_DEFAULT = 30; + + // provider backend cache settings + public static final String ENABLE_CACHING = "sentry.provider.backend.generic.cache.enabled"; + public static final boolean ENABLE_CACHING_DEFAULT = false; + public static final String CACHE_TTL_MS = "sentry.provider.backend.generic.cache.ttl.ms"; + public static final long CACHING_TTL_MS_DEFAULT = 30000; + public static final String CACHE_UPDATE_FAILURES_BEFORE_PRIV_REVOKE = "sentry.provider.backend.generic.cache.update.failures.count"; + public static final int CACHE_UPDATE_FAILURES_BEFORE_PRIV_REVOKE_DEFAULT = 3; + public static final String PRIVILEGE_CONVERTER = "sentry.provider.backend.generic.privilege.converter"; } /** http://git-wip-us.apache.org/repos/asf/sentry/blob/eceaaf8e/sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java b/sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java index 079d52a..2a64621 100644 --- a/sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java +++ b/sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/SimpleFileProviderBackend.java @@ -37,8 +37,10 @@ import org.apache.sentry.policy.common.PrivilegeUtils; import org.apache.sentry.core.common.validator.PrivilegeValidator; import org.apache.sentry.core.common.validator.PrivilegeValidatorContext; import org.apache.sentry.core.common.utils.PolicyFileConstants; +import org.apache.sentry.provider.common.CacheProvider; import org.apache.sentry.provider.common.ProviderBackend; import org.apache.sentry.provider.common.ProviderBackendContext; +import org.apache.sentry.provider.common.TableCache; import org.apache.shiro.config.Ini; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -57,7 +59,7 @@ import com.google.common.collect.Sets; import com.google.common.collect.Table; import com.google.common.collect.Table.Cell; -public class SimpleFileProviderBackend implements ProviderBackend { +public class SimpleFileProviderBackend extends CacheProvider implements ProviderBackend { private static final Logger LOGGER = LoggerFactory .getLogger(SimpleFileProviderBackend.class); @@ -67,33 +69,7 @@ public class SimpleFileProviderBackend implements ProviderBackend { private final Configuration conf; private final List<String> configErrors; private final List<String> configWarnings; - - /** - * Sparse table where group is the row key and role is the cell. - * The value is the set of privileges located in the cell. For example, - * the following table would be generated for a policy where Group 1 - * has Role 1 and Role 2 while Group 2 has only Role 2. - * <table border="1"> - * <tbody> - * <tr> - * <td><!-- empty --></td> - * <td>Role 1</td> - * <td>Role 2</td> - * </tr> - * <tr> - * <td>Group 1</td> - * <td>Priv 1</td> - * <td>Priv 2, Priv 3</td> - * </tr> - * <tr> - * <td>Group 2</td> - * <td><!-- empty --></td> - * <td>Priv 2, Priv 3</td> - * </tr> - * </tbody> - * </table> - */ - private final Table<String, String, Set<String>> groupRolePrivilegeTable; + private TableCache cache; /** * Each group, role, and privilege in groupRolePrivilegeTable is * interned using a weak interner so that we only store each string @@ -112,7 +88,6 @@ public class SimpleFileProviderBackend implements ProviderBackend { public SimpleFileProviderBackend(Configuration conf, Path resourcePath) throws IOException { this.resourcePath = resourcePath; this.fileSystem = resourcePath.getFileSystem(conf); - this.groupRolePrivilegeTable = HashBasedTable.create(); this.conf = conf; this.configErrors = Lists.newArrayList(); this.configWarnings = Lists.newArrayList(); @@ -132,28 +107,15 @@ public class SimpleFileProviderBackend implements ProviderBackend { } this.validators = context.getValidators(); this.allowPerDatabaseSection = context.isAllowPerDatabase(); - parse(); - this.initialized = true; - } - - /** - * {@inheritDoc} - */ - @Override - public ImmutableSet<String> getPrivileges(Set<String> groups, ActiveRoleSet roleSet, Authorizable... authorizableHierarchy) { - if (!initialized) { - throw new IllegalStateException("Backend has not been properly initialized"); - } - ImmutableSet.Builder<String> resultBuilder = ImmutableSet.builder(); - for (String groupName : groups) { - for (Map.Entry<String, Set<String>> row : groupRolePrivilegeTable.row(groupName) - .entrySet()) { - if (roleSet.containsRole(row.getKey())) { - resultBuilder.addAll(row.getValue()); - } + final Table<String, String, Set<String>> table = parse(); + this.cache = new TableCache() { + @Override + public Table<String, String, Set<String>> getCache() { + return table; } - } - return resultBuilder.build(); + }; + super.initialize(cache); + this.initialized = true; } @Override @@ -163,28 +125,6 @@ public class SimpleFileProviderBackend implements ProviderBackend { return getPrivileges(groups, roleSet, authorizableHierarchy); } - /** - * {@inheritDoc} - */ - @Override - public ImmutableSet<String> getRoles(Set<String> groups, ActiveRoleSet roleSet) { - if (!initialized) { - throw new IllegalStateException("Backend has not been properly initialized"); - } - ImmutableSet.Builder<String> resultBuilder = ImmutableSet.builder(); - if (groups != null) { - for (String groupName : groups) { - for (Map.Entry<String, Set<String>> row : groupRolePrivilegeTable.row(groupName) - .entrySet()) { - if (roleSet.containsRole(row.getKey())) { - resultBuilder.add(row.getKey()); - } - } - } - } - return resultBuilder.build(); - } - @Override public void close() { // SENTRY-847 will use HiveAuthBinding again, so groupRolePrivilegeTable shouldn't clear itself @@ -206,9 +146,10 @@ public class SimpleFileProviderBackend implements ProviderBackend { } } - private void parse() { + private Table<String, String, Set<String>> parse() { configErrors.clear(); configWarnings.clear(); + Table<String, String, Set<String>> groupRolePrivilegeTable = HashBasedTable.create(); Table<String, String, Set<String>> groupRolePrivilegeTableTemp = HashBasedTable.create(); Ini ini; LOGGER.info("Parsing " + resourcePath); @@ -237,7 +178,7 @@ public class SimpleFileProviderBackend implements ProviderBackend { } } parseIni(null, ini, validators, resourcePath, groupRolePrivilegeTableTemp); - mergeResult(groupRolePrivilegeTableTemp); + mergeResult(groupRolePrivilegeTable, groupRolePrivilegeTableTemp); groupRolePrivilegeTableTemp.clear(); Ini.Section filesSection = ini.getSection(PolicyFileConstants.DATABASES); if(filesSection == null) { @@ -265,6 +206,8 @@ public class SimpleFileProviderBackend implements ProviderBackend { throw new SentryConfigurationException("Per-db policy files cannot contain " + PolicyFileConstants.DATABASES + " section"); } parseIni(database, perDbIni, validators, perDbPolicy, groupRolePrivilegeTableTemp); + mergeResult(groupRolePrivilegeTable, groupRolePrivilegeTableTemp); + groupRolePrivilegeTableTemp.clear(); } catch (Exception e) { configErrors.add("Failed to read per-DB policy file " + perDbPolicy + " Error: " + e.getMessage()); @@ -272,12 +215,12 @@ public class SimpleFileProviderBackend implements ProviderBackend { } } } - mergeResult(groupRolePrivilegeTableTemp); - groupRolePrivilegeTableTemp.clear(); } catch (Exception e) { configErrors.add("Error processing file " + resourcePath + ". Message: " + e.getMessage()); LOGGER.error("Error processing file, ignoring " + resourcePath, e); } + + return groupRolePrivilegeTable; } /** @@ -289,7 +232,8 @@ public class SimpleFileProviderBackend implements ProviderBackend { return uri.getAuthority() == null && uri.getScheme() == null && !path.isUriPathAbsolute(); } - private void mergeResult(Table<String, String, Set<String>> groupRolePrivilegeTableTemp) { + private void mergeResult(Table<String, String, Set<String>> groupRolePrivilegeTable, + Table<String, String, Set<String>> groupRolePrivilegeTableTemp) { for (Cell<String, String, Set<String>> cell : groupRolePrivilegeTableTemp.cellSet()) { String groupName = cell.getRowKey(); String roleName = cell.getColumnKey(); @@ -387,7 +331,12 @@ public class SimpleFileProviderBackend implements ProviderBackend { } } + /** + * Returns backing table of group-role-privileges cache. + * Caller must not modify the returned table. + * @return backing table of cache. + */ public Table<String, String, Set<String>> getGroupRolePrivilegeTable() { - return groupRolePrivilegeTable; + return this.cache.getCache(); } } http://git-wip-us.apache.org/repos/asf/sentry/blob/eceaaf8e/sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java ---------------------------------------------------------------------- diff --git a/sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java b/sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java index a2cfa28..c4e3863 100644 --- a/sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java +++ b/sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/AbstractKafkaSentryTestBase.java @@ -61,6 +61,8 @@ public class AbstractKafkaSentryTestBase { protected static final String ADMIN_USER = "kafka"; protected static final String ADMIN_GROUP = "group_kafka"; protected static final String ADMIN_ROLE = "role_kafka"; + private static final long CACHE_TTL_MS = 1; + private static final int SAFETY_FACTOR = 2; // Sleep for specified times of expected time for an operation to complete. protected static SentryService sentryServer; protected static File sentrySitePath; @@ -205,13 +207,16 @@ public class AbstractKafkaSentryTestBase { /** set the Sentry client configuration for Kafka Service integration */ conf.set(ServerConfig.SECURITY_MODE, ServerConfig.SECURITY_MODE_NONE); conf.set(ClientConfig.SERVER_RPC_ADDRESS, sentryServer.getAddress().getHostName()); - conf.set(ClientConfig.SERVER_RPC_PORT, String.valueOf(sentryServer.getAddress().getPort())); + conf.setInt(ClientConfig.SERVER_RPC_PORT, sentryServer.getAddress().getPort()); conf.set(KafkaAuthConf.AuthzConfVars.AUTHZ_PROVIDER.getVar(), LocalGroupResourceAuthorizationProvider.class.getName()); conf.set(KafkaAuthConf.AuthzConfVars.AUTHZ_PROVIDER_BACKEND.getVar(), SentryGenericProviderBackend.class.getName()); conf.set(KafkaAuthConf.AuthzConfVars.AUTHZ_PROVIDER_RESOURCE.getVar(), policyFilePath.getPath()); + conf.setBoolean(ClientConfig.ENABLE_CACHING, true); + conf.setLong(ClientConfig.CACHE_TTL_MS, CACHE_TTL_MS); + conf.set(ClientConfig.PRIVILEGE_CONVERTER, "org.apache.sentry.provider.db.generic.tools.KafkaTSentryPrivilegeConvertor"); return conf; } @@ -224,4 +229,10 @@ public class AbstractKafkaSentryTestBase { kafkaServer.start(); bootstrapServers = kafkaServer.getBootstrapServers(); } + + static void sleepIfCachingEnabled() throws InterruptedException { + if (getClientConfig().getBoolean(ClientConfig.ENABLE_CACHING, false)) { + Thread.sleep(CACHE_TTL_MS * SAFETY_FACTOR); + } + } } http://git-wip-us.apache.org/repos/asf/sentry/blob/eceaaf8e/sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java ---------------------------------------------------------------------- diff --git a/sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java b/sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java index 250522e..6017451 100644 --- a/sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java +++ b/sentry-tests/sentry-tests-kafka/src/test/java/org/apache/sentry/tests/e2e/kafka/TestAuthorize.java @@ -173,6 +173,7 @@ public class TestAuthorize extends AbstractKafkaSentryTestBase { sentryClient = null; } } + sleepIfCachingEnabled(); } private void testProduce(String producerUser) throws Exception {
