Repository: incubator-sentry Updated Branches: refs/heads/master 5659d39d6 -> 1665b93d0
SENTRY-1048: Fix "Critical" issues identified by analysis.apache.org (Colm O hEigeartaigh, Reviewed by: Sravya Tirukkovalur) Change-Id: I1a4a4351a5b86d2e7893cb30ce34d3656c456e40 Project: http://git-wip-us.apache.org/repos/asf/incubator-sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-sentry/commit/1665b93d Tree: http://git-wip-us.apache.org/repos/asf/incubator-sentry/tree/1665b93d Diff: http://git-wip-us.apache.org/repos/asf/incubator-sentry/diff/1665b93d Branch: refs/heads/master Commit: 1665b93d04be174bf43ad5367fa7b686b2413b70 Parents: 5659d39 Author: Sravya Tirukkovalur <[email protected]> Authored: Wed Feb 10 16:26:45 2016 -0800 Committer: Sravya Tirukkovalur <[email protected]> Committed: Wed Feb 10 16:26:45 2016 -0800 ---------------------------------------------------------------------- .../hive/ql/exec/SentryGrantRevokeTask.java | 9 ++-- .../binding/hive/HiveAuthzBindingHook.java | 6 ++- .../SentryHiveAuthorizationTaskFactoryImpl.java | 2 +- .../hive/SentryIniPolicyFileFormatter.java | 6 +-- .../binding/hive/authz/HiveAuthzBinding.java | 7 +-- .../SentryMetastorePostEventListener.java | 21 ++++---- .../java/org/apache/sentry/hdfs/HMSPaths.java | 3 +- .../hdfs/SentryAuthorizationProvider.java | 2 +- .../apache/sentry/hdfs/SentryPermissions.java | 11 +++-- .../org/apache/sentry/hdfs/MetastorePlugin.java | 14 ++++-- .../service/persistent/DelegateSentryStore.java | 12 +++-- .../db/generic/tools/SentryShellSolr.java | 9 ++-- .../db/service/persistent/HAContext.java | 2 +- .../db/service/persistent/SentryStore.java | 51 +++++++++++--------- .../persistent/SentryStoreSchemaInfo.java | 4 +- .../SentryPolicyServiceClientDefaultImpl.java | 12 ++--- .../service/thrift/SentryKerberosContext.java | 2 +- .../apache/sentry/provider/file/PolicyFile.java | 11 +++-- .../sentry/provider/file/PolicyFiles.java | 13 ++--- .../SentryIndexAuthorizationSingleton.java | 4 +- .../handler/admin/SecureCollectionsHandler.java | 2 +- 21 files changed, 115 insertions(+), 88 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/1665b93d/sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java ---------------------------------------------------------------------- diff --git a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java index 5e2d8a1..31eb5e8 100644 --- a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java +++ b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java @@ -387,10 +387,11 @@ public class SentryGrantRevokeTask extends Task<DDLWork> implements Serializable FSDataOutputStream out = fs.create(resFile); try { if (data != null && !data.isEmpty()) { - OutputStreamWriter writer = new OutputStreamWriter(out, "UTF-8"); - writer.write(data); - writer.write((char) terminator); - writer.flush(); + try (OutputStreamWriter writer = new OutputStreamWriter(out, "UTF-8")) { + writer.write(data); + writer.write((char) terminator); + writer.flush(); + } } } finally { closeQuiet(out); http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/1665b93d/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java ---------------------------------------------------------------------- diff --git a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java index 7d56435..08c0e98 100644 --- a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java +++ b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java @@ -394,10 +394,12 @@ public class HiveAuthzBindingHook extends AbstractSemanticAnalyzerHook { authorizeWithHiveBindings(context, stmtAuthObject, stmtOperation); } catch (AuthorizationException e) { executeOnFailureHooks(context, stmtOperation, e); - String permsRequired = ""; + StringBuilder permsBuilder = new StringBuilder(); for (String perm : hiveAuthzBinding.getLastQueryPrivilegeErrors()) { - permsRequired += perm + ";"; + permsBuilder.append(perm); + permsBuilder.append(";"); } + String permsRequired = permsBuilder.toString(); SessionState.get().getConf().set(HiveAuthzConf.HIVE_SENTRY_AUTH_ERRORS, permsRequired); String msgForLog = HiveAuthzConf.HIVE_SENTRY_PRIVILEGE_ERROR_MESSAGE + "\n Required privileges for this query: " http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/1665b93d/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java ---------------------------------------------------------------------- diff --git a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java index 617a8bc..caf32cf 100644 --- a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java +++ b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryHiveAuthorizationTaskFactoryImpl.java @@ -161,7 +161,7 @@ public class SentryHiveAuthorizationTaskFactoryImpl implements HiveAuthorization ASTNode astChild = (ASTNode) ast.getChild(2); privilegeObj = analyzePrivilegeObject(astChild); } - if (privilegeObj.getPartSpec() != null) { + if (privilegeObj != null && privilegeObj.getPartSpec() != null) { throw new SemanticException(SentryHiveConstants.PARTITION_PRIVS_NOT_SUPPORTED); } for (PrincipalDesc princ : principalDesc) { http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/1665b93d/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryIniPolicyFileFormatter.java ---------------------------------------------------------------------- diff --git a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryIniPolicyFileFormatter.java b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryIniPolicyFileFormatter.java index 1e83a6b..45747df 100644 --- a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryIniPolicyFileFormatter.java +++ b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/SentryIniPolicyFileFormatter.java @@ -151,9 +151,9 @@ public class SentryIniPolicyFileFormatter implements SentryPolicyFileFormatter { } List<String> lines = Lists.newArrayList(); lines.add("[" + name + "]"); - for (String key : mappingData.keySet()) { - lines.add(PolicyConstants.KV_JOINER.join(key, - PolicyConstants.ROLE_JOINER.join(mappingData.get(key)))); + for (Map.Entry<String, Set<String>> entry : mappingData.entrySet()) { + lines.add(PolicyConstants.KV_JOINER.join(entry.getKey(), + PolicyConstants.ROLE_JOINER.join(entry.getValue()))); } return Joiner.on(NL).join(lines); } http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/1665b93d/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java ---------------------------------------------------------------------- diff --git a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java index 6066100..0a1d0e8 100644 --- a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java +++ b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/authz/HiveAuthzBinding.java @@ -310,17 +310,18 @@ public class HiveAuthzBinding { } boolean found = false; - for(AuthorizableType key: requiredInputPrivileges.keySet()) { + for (Map.Entry<AuthorizableType, EnumSet<DBModelAction>> entry : requiredInputPrivileges.entrySet()) { + AuthorizableType key = entry.getKey(); for (List<DBModelAuthorizable> inputHierarchy : inputHierarchyList) { if (getAuthzType(inputHierarchy).equals(key)) { found = true; - if (!authProvider.hasAccess(subject, inputHierarchy, requiredInputPrivileges.get(key), activeRoleSet)) { + if (!authProvider.hasAccess(subject, inputHierarchy, entry.getValue(), activeRoleSet)) { throw new AuthorizationException("User " + subject.getName() + " does not have privileges for " + hiveOp.name()); } } } - if(!found && !(key.equals(AuthorizableType.URI)) && !(hiveOp.equals(HiveOperation.QUERY)) + if (!found && !key.equals(AuthorizableType.URI) && !(hiveOp.equals(HiveOperation.QUERY)) && !(hiveOp.equals(HiveOperation.CREATETABLE_AS_SELECT))) { //URI privileges are optional for some privileges: anyPrivilege, tableDDLAndOptionalUriPrivilege //Query can mean select/insert/analyze where all of them have different required privileges. http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/1665b93d/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java ---------------------------------------------------------------------- diff --git a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java index cb797af..452757e 100644 --- a/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java +++ b/sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/metastore/SentryMetastorePostEventListener.java @@ -61,6 +61,12 @@ public class SentryMetastorePostEventListener extends MetaStoreEventListener { public SentryMetastorePostEventListener(Configuration config) { super(config); + if (!(config instanceof HiveConf)) { + String error = "Could not initialize Plugin - Configuration is not an instanceof HiveConf"; + LOGGER.error(error); + throw new RuntimeException(error); + } + authzConf = HiveAuthzConf.getAuthzConf((HiveConf)config); server = new Server(authzConf.get(AuthzConfVars.AUTHZ_SERVER_NAME.getVar())); Iterable<String> pluginClasses = ConfUtilties.CLASS_SPLITTER @@ -204,7 +210,6 @@ public class SentryMetastorePostEventListener extends MetaStoreEventListener { */ @Override public void onAlterTable (AlterTableEvent tableEvent) throws MetaException { - String oldTableName = null, newTableName = null; // don't sync privileges if the operation has failed if (!tableEvent.getStatus()) { @@ -213,17 +218,11 @@ public class SentryMetastorePostEventListener extends MetaStoreEventListener { return; } - if (tableEvent.getOldTable() != null) { - oldTableName = tableEvent.getOldTable().getTableName(); - } - - if (tableEvent.getNewTable() != null) { - newTableName = tableEvent.getNewTable().getTableName(); - } - renameSentryTablePrivilege(tableEvent.getOldTable().getDbName(), - oldTableName, tableEvent.getOldTable().getSd().getLocation(), - tableEvent.getNewTable().getDbName(), newTableName, + tableEvent.getOldTable().getTableName(), + tableEvent.getOldTable().getSd().getLocation(), + tableEvent.getNewTable().getDbName(), + tableEvent.getNewTable().getTableName(), tableEvent.getNewTable().getSd().getLocation()); } http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/1665b93d/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java ---------------------------------------------------------------------- diff --git a/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java b/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java index 135ea20..ceb1da8 100644 --- a/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java +++ b/sentry-hdfs/sentry-hdfs-common/src/main/java/org/apache/sentry/hdfs/HMSPaths.java @@ -414,7 +414,8 @@ public class HMSPaths implements AuthzPaths { public HMSPaths(String[] pathPrefixes) { boolean rootPrefix = false; - this.prefixes = pathPrefixes; + // Copy the array to avoid external modification + this.prefixes = Arrays.copyOf(pathPrefixes, pathPrefixes.length); for (String pathPrefix : pathPrefixes) { rootPrefix = rootPrefix || pathPrefix.equals(Path.SEPARATOR); } http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/1665b93d/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationProvider.java ---------------------------------------------------------------------- diff --git a/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationProvider.java b/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationProvider.java index 4de130a..cf85fa5 100644 --- a/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationProvider.java +++ b/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryAuthorizationProvider.java @@ -343,7 +343,7 @@ public class SentryAuthorizationProvider } } if (LOG.isDebugEnabled()) { - LOG.debug("### getAclEntry \n[" + (p == null ? "null" : p) + "] : [" + LOG.debug("### getAclEntry \n[" + p + "] : [" + "isPreifxed=" + isPrefixed + ", isStale=" + isStale + ", hasAuthzObj=" + hasAuthzObj http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/1665b93d/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryPermissions.java ---------------------------------------------------------------------- diff --git a/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryPermissions.java b/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryPermissions.java index 107d3e1..c01ff68 100644 --- a/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryPermissions.java +++ b/sentry-hdfs/sentry-hdfs-namenode-plugin/src/main/java/org/apache/sentry/hdfs/SentryPermissions.java @@ -81,12 +81,13 @@ public class SentryPermissions implements AuthzPermissions { private final Map<String, RoleInfo> roles = new TreeMap<String, RoleInfo>(String.CASE_INSENSITIVE_ORDER); String getParentAuthzObject(String authzObject) { - int dot = authzObject.indexOf('.'); - if (dot > 0) { - return authzObject.substring(0, dot); - } else { - return authzObject; + if (authzObject != null) { + int dot = authzObject.indexOf('.'); + if (dot > 0) { + return authzObject.substring(0, dot); + } } + return authzObject; } void addParentChildMappings(String authzObject) { http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/1665b93d/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java ---------------------------------------------------------------------- diff --git a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java index 6e14c29..10ea37b 100644 --- a/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java +++ b/sentry-hdfs/sentry-hdfs-service/src/main/java/org/apache/sentry/hdfs/MetastorePlugin.java @@ -114,7 +114,14 @@ public class MetastorePlugin extends SentryMetastoreListenerPlugin { public MetastorePlugin(Configuration conf, Configuration sentryConf) { this.notificiationLock = new ReentrantLock(); + + if (!(conf instanceof HiveConf)) { + String error = "Configuration is not an instanceof HiveConf"; + LOGGER.error(error); + throw new RuntimeException(error); + } this.conf = new HiveConf((HiveConf)conf); + this.sentryConf = new Configuration(sentryConf); this.conf.unset(HiveConf.ConfVars.METASTORE_PRE_EVENT_LISTENERS.varname); this.conf.unset(HiveConf.ConfVars.METASTORE_EVENT_LISTENERS.varname); @@ -297,10 +304,11 @@ public class MetastorePlugin extends SentryMetastoreListenerPlugin { protected void notifySentry(PathsUpdate update) { notificiationLock.lock(); - if (!syncSent) { - new SyncTask().run(); - } try { + if (!syncSent) { + new SyncTask().run(); + } + notifySentryNoLock(update); } finally { lastSentSeqNum = update.getSeqNum(); http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/1665b93d/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 4c5ceca..fcd40e8 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 @@ -352,7 +352,9 @@ public class DelegateSentryStore implements SentryStoreLayer { } return groupNames; } finally { - commitTransaction(pm); + if (pm != null) { + commitTransaction(pm); + } } } @@ -377,7 +379,9 @@ public class DelegateSentryStore implements SentryStoreLayer { } privileges.addAll(privilegeOperator.getPrivilegesByRole(mRoles, pm)); } finally { - commitTransaction(pm); + if (pm != null) { + commitTransaction(pm); + } } return privileges; } @@ -417,7 +421,9 @@ public class DelegateSentryStore implements SentryStoreLayer { //get the privileges privileges.addAll(privilegeOperator.getPrivilegesByProvider(component, service, mRoles, authorizables, pm)); } finally { - commitTransaction(pm); + if (pm != null) { + commitTransaction(pm); + } } return privileges; } http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/1665b93d/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java index 3e21faf..de718e9 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/generic/tools/SentryShellSolr.java @@ -95,13 +95,16 @@ public class SentryShellSolr extends SentryShellCommon { sentryShell.executeShell(args); } catch (Exception e) { LOGGER.error(e.getMessage(), e); - Throwable current = e; + Throwable current = e; // find the first printable message; while (current != null && current.getMessage() == null) { current = current.getCause(); } - System.out.println("The operation failed." + - (current.getMessage() == null ? "" : " Message: " + current.getMessage())); + String error = ""; + if (current != null && current.getMessage() != null) { + error = "Message: " + current.getMessage(); + } + System.out.println("The operation failed. " + error); System.exit(1); } } http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/1665b93d/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HAContext.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HAContext.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HAContext.java index eac10a0..7bce741 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HAContext.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/HAContext.java @@ -52,7 +52,7 @@ import com.google.common.collect.Lists; public class HAContext { private static final Logger LOGGER = LoggerFactory.getLogger(HAContext.class); - private static HAContext serverHAContext = null; + private static volatile HAContext serverHAContext = null; private static boolean aclChecked = false; public final static String SENTRY_SERVICE_REGISTER_NAMESPACE = "sentry-service"; http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/1665b93d/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java index 6a4d50d..9cebe1e 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStore.java @@ -345,7 +345,9 @@ public class SentryStore { size = (Long)query.execute(); } finally { - commitTransaction(pm); + if (pm != null) { + commitTransaction(pm); + } } return size; } @@ -1013,7 +1015,7 @@ public class SentryStore { pm = openTransaction(); Query query = pm.newQuery(MSentryPrivilege.class); StringBuilder filters = new StringBuilder(); - if (roleNames.size() == 0 || roleNames == null) { + if (roleNames == null || roleNames.isEmpty()) { filters.append(" !roles.isEmpty() "); } else { query.declareVariables("org.apache.sentry.provider.db.service.model.MSentryRole role"); @@ -1895,12 +1897,15 @@ public class SentryStore { */ public void incPrivRemoval(int numDeletions) { if (privCleanerThread != null) { - lock.lock(); - currentNotifies += numDeletions; - if (currentNotifies > NOTIFY_THRESHOLD) { - cond.signal(); + try { + lock.lock(); + currentNotifies += numDeletions; + if (currentNotifies > NOTIFY_THRESHOLD) { + cond.signal(); + } + } finally { + lock.unlock(); } - lock.unlock(); } } @@ -2258,15 +2263,15 @@ public class SentryStore { Map<String, Set<String>> groupRolesMap) { Map<String, Set<TSentryGroup>> roleGroupsMap = Maps.newHashMap(); if (groupRolesMap != null) { - for (String groupName : groupRolesMap.keySet()) { - Set<String> roleNames = groupRolesMap.get(groupName); + for (Map.Entry<String, Set<String>> entry : groupRolesMap.entrySet()) { + Set<String> roleNames = entry.getValue(); if (roleNames != null) { for (String roleName : roleNames) { Set<TSentryGroup> tSentryGroups = roleGroupsMap.get(roleName); if (tSentryGroups == null) { tSentryGroups = Sets.newHashSet(); } - tSentryGroups.add(new TSentryGroup(groupName)); + tSentryGroups.add(new TSentryGroup(entry.getKey())); roleGroupsMap.put(roleName, tSentryGroups); } } @@ -2280,11 +2285,11 @@ public class SentryStore { if (importedRoleGroupsMap == null || importedRoleGroupsMap.keySet() == null) { return; } - for (String roleName : importedRoleGroupsMap.keySet()) { - if (!existRoleNames.contains(roleName)) { - createSentryRoleCore(pm, roleName); + for (Map.Entry<String, Set<TSentryGroup>> entry : importedRoleGroupsMap.entrySet()) { + if (!existRoleNames.contains(entry.getKey())) { + createSentryRoleCore(pm, entry.getKey()); } - alterSentryRoleAddGroupsCore(pm, roleName, importedRoleGroupsMap.get(roleName)); + alterSentryRoleAddGroupsCore(pm, entry.getKey(), entry.getValue()); } } @@ -2306,15 +2311,15 @@ public class SentryStore { Map<String, Set<String>> newSentryGroupRolesMap = Maps.newHashMap(); Map<String, Set<TSentryPrivilege>> newSentryRolePrivilegesMap = Maps.newHashMap(); // for mapping data [group,role] - for (String groupName : sentryGroupRolesMap.keySet()) { - Collection<String> lowcaseRoles = Collections2.transform(sentryGroupRolesMap.get(groupName), + for (Map.Entry<String, Set<String>> entry : sentryGroupRolesMap.entrySet()) { + Collection<String> lowcaseRoles = Collections2.transform(entry.getValue(), new Function<String, String>() { @Override public String apply(String input) { return input.toString().toLowerCase(); } }); - newSentryGroupRolesMap.put(groupName, Sets.newHashSet(lowcaseRoles)); + newSentryGroupRolesMap.put(entry.getKey(), Sets.newHashSet(lowcaseRoles)); } // for mapping data [role,privilege] @@ -2331,16 +2336,16 @@ public class SentryStore { private void importSentryRolePrivilegeMapping(PersistenceManager pm, Set<String> existRoleNames, Map<String, Set<TSentryPrivilege>> sentryRolePrivilegesMap) throws Exception { if (sentryRolePrivilegesMap != null) { - for (String roleName : sentryRolePrivilegesMap.keySet()) { + for (Map.Entry<String, Set<TSentryPrivilege>> entry : sentryRolePrivilegesMap.entrySet()) { // if the rolenName doesn't exist, create it. - if (!existRoleNames.contains(roleName)) { - createSentryRoleCore(pm, roleName); - existRoleNames.add(roleName); + if (!existRoleNames.contains(entry.getKey())) { + createSentryRoleCore(pm, entry.getKey()); + existRoleNames.add(entry.getKey()); } // get the privileges for the role - Set<TSentryPrivilege> tSentryPrivileges = sentryRolePrivilegesMap.get(roleName); + Set<TSentryPrivilege> tSentryPrivileges = entry.getValue(); for (TSentryPrivilege tSentryPrivilege : tSentryPrivileges) { - alterSentryRoleGrantPrivilegeCore(pm, roleName, tSentryPrivilege); + alterSentryRoleGrantPrivilegeCore(pm, entry.getKey(), tSentryPrivilege); } } } http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/1665b93d/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreSchemaInfo.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreSchemaInfo.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreSchemaInfo.java index dd5880a..fdadcb8 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreSchemaInfo.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/persistent/SentryStoreSchemaInfo.java @@ -47,9 +47,7 @@ public class SentryStoreSchemaInfo { List<String> upgradeOrderList = new ArrayList<String>(); String upgradeListFile = getSentryStoreScriptDir() + File.separator + VERSION_UPGRADE_LIST + "." + dbType; - try { - BufferedReader bfReader = new BufferedReader(new FileReader( - upgradeListFile)); + try (BufferedReader bfReader = new BufferedReader(new FileReader(upgradeListFile))) { String currSchemaVersion; while ((currSchemaVersion = bfReader.readLine()) != null) { upgradeOrderList.add(currSchemaVersion.trim()); http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/1665b93d/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java index c40edca..edc5661 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClientDefaultImpl.java @@ -890,13 +890,13 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService Map<String, Set<String>> rolePrivilegesMap) { Map<String, Set<TSentryPrivilege>> rolePrivilegesMapResult = Maps.newHashMap(); if (rolePrivilegesMap != null) { - for (String tempRoleName : rolePrivilegesMap.keySet()) { + for (Map.Entry<String, Set<String>> entry : rolePrivilegesMap.entrySet()) { Set<TSentryPrivilege> tempTSentryPrivileges = Sets.newHashSet(); - Set<String> tempPrivileges = rolePrivilegesMap.get(tempRoleName); + Set<String> tempPrivileges = entry.getValue(); for (String tempPrivilege : tempPrivileges) { tempTSentryPrivileges.add(SentryServiceUtil.convertToTSentryPrivilege(tempPrivilege)); } - rolePrivilegesMapResult.put(tempRoleName, tempTSentryPrivileges); + rolePrivilegesMapResult.put(entry.getKey(), tempTSentryPrivileges); } } return rolePrivilegesMapResult; @@ -927,8 +927,8 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService Map<String, Set<TSentryPrivilege>> rolePrivilegesMap) { Map<String, Set<String>> rolePrivilegesMapForFile = Maps.newHashMap(); if (rolePrivilegesMap != null) { - for (String tempRoleName : rolePrivilegesMap.keySet()) { - Set<TSentryPrivilege> tempSentryPrivileges = rolePrivilegesMap.get(tempRoleName); + for (Map.Entry<String, Set<TSentryPrivilege>> entry : rolePrivilegesMap.entrySet()) { + Set<TSentryPrivilege> tempSentryPrivileges = entry.getValue(); Set<String> tempStrPrivileges = Sets.newHashSet(); for (TSentryPrivilege tSentryPrivilege : tempSentryPrivileges) { // convert TSentryPrivilege to privilege in string @@ -937,7 +937,7 @@ public class SentryPolicyServiceClientDefaultImpl implements SentryPolicyService tempStrPrivileges.add(privilegeStr); } } - rolePrivilegesMapForFile.put(tempRoleName, tempStrPrivileges); + rolePrivilegesMapForFile.put(entry.getKey(), tempStrPrivileges); } } return rolePrivilegesMapForFile; http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/1665b93d/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryKerberosContext.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryKerberosContext.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryKerberosContext.java index fc7bc05..93481cb 100644 --- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryKerberosContext.java +++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/service/thrift/SentryKerberosContext.java @@ -108,11 +108,11 @@ public class SentryKerberosContext implements Runnable { LOGGER.info("Sentry Ticket renewer thread started"); while (!shutDownRenewer) { KerberosTicket tgt = getTGT(); - long nextRefresh = getRefreshTime(tgt); if (tgt == null) { LOGGER.warn("No ticket found in the cache"); return; } + long nextRefresh = getRefreshTime(tgt); while (System.currentTimeMillis() < nextRefresh) { Thread.sleep(1000); if (shutDownRenewer) { http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/1665b93d/sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFile.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFile.java b/sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFile.java index 835e732..991a95f 100644 --- a/sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFile.java +++ b/sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFile.java @@ -26,6 +26,7 @@ import java.io.File; import java.util.Collection; import java.util.List; import java.util.Map; +import java.util.Map.Entry; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -88,9 +89,9 @@ public class PolicyFile { LOGGER.warn("Static user:group mapping is not being used"); return add(usersToGroups.get(userName), allowDuplicates, groupNames); } - public PolicyFile setUserGroupMapping(Map<String, String> mapping){ - for(String key: mapping.keySet()){ - usersToGroups.put(key, mapping.get(key)); + public PolicyFile setUserGroupMapping(Map<String, String> mapping) { + for (Entry<String, String> entry : mapping.entrySet()) { + usersToGroups.put(entry.getKey(), entry.getValue()); } return this; } @@ -155,8 +156,8 @@ public class PolicyFile { Joiner kvJoiner = Joiner.on(" = "); List<String> lines = Lists.newArrayList(); lines.add("[" + name + "]"); - for(String key : mapping.keySet()) { - lines.add(kvJoiner.join(key, mapping.get(key))); + for (Entry<String, String> entry : mapping.entrySet()) { + lines.add(kvJoiner.join(entry.getKey(), entry.getValue())); } return Joiner.on(NL).join(lines); } http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/1665b93d/sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFiles.java ---------------------------------------------------------------------- diff --git a/sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFiles.java b/sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFiles.java index d537e3b..378f63c 100644 --- a/sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFiles.java +++ b/sentry-provider/sentry-provider-file/src/main/java/org/apache/sentry/provider/file/PolicyFiles.java @@ -61,12 +61,13 @@ public class PolicyFiles { public static void copyFilesToDir(FileSystem fs, Path dest, File inputFile) throws IOException { - InputStream input = new FileInputStream(inputFile.getPath()); - FSDataOutputStream out = fs.create(new Path(dest, inputFile.getName())); - ByteStreams.copy(input, out); - input.close(); - out.hflush(); - out.close(); + try (InputStream input = new FileInputStream(inputFile.getPath()); + FSDataOutputStream out = fs.create(new Path(dest, inputFile.getName()))) { + ByteStreams.copy(input, out); + input.close(); + out.hflush(); + out.close(); + } } http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/1665b93d/sentry-solr/solr-sentry-core/src/main/java/org/apache/solr/sentry/SentryIndexAuthorizationSingleton.java ---------------------------------------------------------------------- diff --git a/sentry-solr/solr-sentry-core/src/main/java/org/apache/solr/sentry/SentryIndexAuthorizationSingleton.java b/sentry-solr/solr-sentry-core/src/main/java/org/apache/solr/sentry/SentryIndexAuthorizationSingleton.java index c9d2414..c8f0560 100644 --- a/sentry-solr/solr-sentry-core/src/main/java/org/apache/solr/sentry/SentryIndexAuthorizationSingleton.java +++ b/sentry-solr/solr-sentry-core/src/main/java/org/apache/solr/sentry/SentryIndexAuthorizationSingleton.java @@ -148,12 +148,12 @@ public class SentryIndexAuthorizationSingleton { + "no SolrCore attached to request"; if (errorIfNoCollection) { auditLogger.log(userName.getName(), impersonator, ipAddress, - operation, paramString, eventTime, AuditLogger.UNAUTHORIZED, collectionName); + operation, paramString, eventTime, AuditLogger.UNAUTHORIZED, ""); throw new SolrException(SolrException.ErrorCode.UNAUTHORIZED, msg); } else { // just warn log.warn(msg); auditLogger.log(userName.getName(), impersonator, ipAddress, - operation, paramString, eventTime, AuditLogger.ALLOWED, collectionName); + operation, paramString, eventTime, AuditLogger.ALLOWED, ""); return; } } http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/1665b93d/sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/admin/SecureCollectionsHandler.java ---------------------------------------------------------------------- diff --git a/sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/admin/SecureCollectionsHandler.java b/sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/admin/SecureCollectionsHandler.java index 7490ad0..b5edf20 100644 --- a/sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/admin/SecureCollectionsHandler.java +++ b/sentry-solr/solr-sentry-handlers/src/main/java/org/apache/solr/handler/admin/SecureCollectionsHandler.java @@ -81,7 +81,7 @@ public class SecureCollectionsHandler extends CollectionsHandler { * ex: When the collection has been deleted, the privileges related to the collection * were also needed to drop. */ - if (action.equals(CollectionAction.DELETE)) { + if (CollectionAction.DELETE.equals(action)) { SecureRequestHandlerUtil.syncDeleteCollection(collection); }
