SENTRY-1788: Sentry Store may use JDO object after the associated data is removed in DB (Kalyan Kalvagadda, reviewed by Vamsee Yarlagadda and Alex Kolbasov) CDH-54250
Change-Id: I6220fb4b918adf4a3d13091abbb44cbca0b81573 Reviewed-on: http://gerrit.sjc.cloudera.com:8080/23108 Tested-by: Jenkins User Reviewed-by: Kalyan Kumar Kalvagadda <[email protected]> (cherry picked from commit 7c789e05ffeb71769a8da7950720419f5290ac75) Reviewed-on: http://gerrit.sjc.cloudera.com:8080/23321 Project: http://git-wip-us.apache.org/repos/asf/sentry/repo Commit: http://git-wip-us.apache.org/repos/asf/sentry/commit/acc13302 Tree: http://git-wip-us.apache.org/repos/asf/sentry/tree/acc13302 Diff: http://git-wip-us.apache.org/repos/asf/sentry/diff/acc13302 Branch: refs/for/cdh5-1.5.1_ha Commit: acc13302c0bcb98417eb8ff8520ebd41fbb377b5 Parents: 926c6d0 Author: Alexander Kolbasov <[email protected]> Authored: Wed May 31 16:19:52 2017 -0700 Committer: Kalyan Kumar Kalvagadda <[email protected]> Committed: Thu Jun 1 10:34:06 2017 -0700 ---------------------------------------------------------------------- .../db/service/persistent/SentryStore.java | 53 +++++++++++--------- 1 file changed, 29 insertions(+), 24 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/sentry/blob/acc13302/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 2a96811..891c1b1 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 @@ -910,7 +910,7 @@ public class SentryStore { * The responsibility to commit/rollback the transaction should be handled by the caller. */ private void populateChildren(PersistenceManager pm, Set<String> roleNames, MSentryPrivilege priv, - Set<MSentryPrivilege> children) throws SentryInvalidInputException { + Collection<MSentryPrivilege> children) throws SentryInvalidInputException { Preconditions.checkNotNull(pm); if ((!isNULL(priv.getServerName())) || (!isNULL(priv.getDbName())) || (!isNULL(priv.getTableName()))) { @@ -2050,44 +2050,49 @@ public class SentryStore { TSentryPrivilege tPrivilege, TSentryPrivilege newTPrivilege) throws SentryNoSuchObjectException, SentryInvalidInputException { - HashSet<MSentryRole> roleSet = Sets.newHashSet(); - + Collection<MSentryRole> roleSet = new HashSet<>(); List<MSentryPrivilege> mPrivileges = getMSentryPrivileges(tPrivilege, pm); - if (mPrivileges != null && !mPrivileges.isEmpty()) { - for (MSentryPrivilege mPrivilege : mPrivileges) { - roleSet.addAll(ImmutableSet.copyOf((mPrivilege.getRoles()))); + for (MSentryPrivilege mPrivilege : mPrivileges) { + roleSet.addAll(ImmutableSet.copyOf(mPrivilege.getRoles())); + } + // Dropping the privilege + if (newTPrivilege == null) { + for (MSentryRole role : roleSet) { + alterSentryRoleRevokePrivilegeCore(pm, role.getRoleName(), tPrivilege); } + return; } - + // Renaming privilege MSentryPrivilege parent = getMSentryPrivilege(tPrivilege, pm); + if (parent != null) { + // When all the roles associated with that privilege are revoked, privilege + // will be removed from the database. + // parent is an JDO object which is associated with privilege data in the database. + // When the associated row is deleted in database, JDO should be not be + // dereferenced. If object has to be used even after that it should have been detached. + parent = pm.detachCopy(parent); + } for (MSentryRole role : roleSet) { // 1. get privilege and child privileges - Set<MSentryPrivilege> privilegeGraph = Sets.newHashSet(); + Collection<MSentryPrivilege> privilegeGraph = new HashSet<>(); if (parent != null) { - - // Parent privilege object is used after revoking. - // If the privilege was associated to only role which is revoked, - // privilege object should not be used. It is safe to insert - // a copy of the parent object in the privilegeGraph - privilegeGraph.add(convertToMSentryPrivilege(convertToTSentryPrivilege(parent))); + privilegeGraph.add(parent); populateChildren(pm, Sets.newHashSet(role.getRoleName()), parent, privilegeGraph); } else { populateChildren(pm, Sets.newHashSet(role.getRoleName()), convertToMSentryPrivilege(tPrivilege), - privilegeGraph); + privilegeGraph); } // 2. revoke privilege and child privileges alterSentryRoleRevokePrivilegeCore(pm, role.getRoleName(), tPrivilege); // 3. add new privilege and child privileges with new tableName - if (newTPrivilege != null) { - for (MSentryPrivilege m : privilegeGraph) { - TSentryPrivilege t = convertToTSentryPrivilege(m); - if (newTPrivilege.getPrivilegeScope().equals(PrivilegeScope.DATABASE.name())) { - t.setDbName(newTPrivilege.getDbName()); - } else if (newTPrivilege.getPrivilegeScope().equals(PrivilegeScope.TABLE.name())) { - t.setTableName(newTPrivilege.getTableName()); - } - alterSentryRoleGrantPrivilegeCore(pm, role.getRoleName(), t); + for (MSentryPrivilege mPriv : privilegeGraph) { + TSentryPrivilege tPriv = convertToTSentryPrivilege(mPriv); + if (newTPrivilege.getPrivilegeScope().equals(PrivilegeScope.DATABASE.name())) { + tPriv.setDbName(newTPrivilege.getDbName()); + } else if (newTPrivilege.getPrivilegeScope().equals(PrivilegeScope.TABLE.name())) { + tPriv.setTableName(newTPrivilege.getTableName()); } + alterSentryRoleGrantPrivilegeCore(pm, role.getRoleName(), tPriv); } } }
