You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sentry.apache.org by ja...@apache.org on 2014/06/07 17:29:17 UTC

git commit: SENTRY-261: Improve test coverage for grant/revoke statements in Hive e2e tests

Repository: incubator-sentry
Updated Branches:
  refs/heads/master 6e0def2cb -> c6a6dff68


SENTRY-261: Improve test coverage for grant/revoke statements in Hive e2e tests

(Sravya Tirukkovalur via Jarek Jarcec Cecho)


Project: http://git-wip-us.apache.org/repos/asf/incubator-sentry/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-sentry/commit/c6a6dff6
Tree: http://git-wip-us.apache.org/repos/asf/incubator-sentry/tree/c6a6dff6
Diff: http://git-wip-us.apache.org/repos/asf/incubator-sentry/diff/c6a6dff6

Branch: refs/heads/master
Commit: c6a6dff6856d184f1bd6eb7f25ac1cb555148d4b
Parents: 6e0def2
Author: Jarek Jarcec Cecho <ja...@apache.org>
Authored: Sat Jun 7 08:28:34 2014 -0700
Committer: Jarek Jarcec Cecho <ja...@apache.org>
Committed: Sat Jun 7 08:28:34 2014 -0700

----------------------------------------------------------------------
 .../hive/ql/exec/SentryGrantRevokeTask.java     |   4 +-
 .../thrift/TListSentryPrivilegesResponse.java   |  81 +++---
 .../thrift/TListSentryRolesResponse.java        |  81 +++---
 .../thrift/SentryPolicyServiceClient.java       |   2 +-
 .../thrift/SentryPolicyStoreProcessor.java      |   6 +
 .../main/resources/sentry_policy_service.thrift |   4 +-
 .../thrift/TestSentryServiceIntegration.java    |   6 +-
 .../e2e/dbprovider/PolicyProviderForTest.java   |   2 +-
 .../e2e/dbprovider/TestDatabaseProvider.java    | 265 ++++++++++++++++---
 .../tests/e2e/dbprovider/TestDbEndToEnd.java    |   4 +-
 .../TestDbSentryOnFailureHookLoading.java       | 140 ++++++----
 .../apache/sentry/tests/e2e/hive/Context.java   |   7 +-
 .../e2e/hive/DummySentryOnFailureHook.java      |  21 +-
 13 files changed, 438 insertions(+), 185 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/c6a6dff6/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 6ea1ca0..122d137 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
@@ -238,13 +238,13 @@ public class SentryGrantRevokeTask extends Task<DDLWork> implements Serializable
         writeToFile(writeRoleGrantsInfo(roles), desc.getResFile());
         return RETURN_CODE_SUCCESS;
       } else if(operation.equals(RoleDDLDesc.RoleOperation.SHOW_ROLES)) {
-        Set<TSentryRole> roles = sentryClient.listRoles(subject, subjectGroups);
+        Set<TSentryRole> roles = sentryClient.listRoles(subject);
         writeToFile(writeRolesInfo(roles), desc.getResFile());
         return RETURN_CODE_SUCCESS;
       } else if(operation.equals(RoleDDLDesc.RoleOperation.SHOW_CURRENT_ROLE)) {
         ActiveRoleSet roleSet = hiveAuthzBinding.getActiveRoleSet();
         if( roleSet.isAll()) {
-          Set<TSentryRole> roles = sentryClient.listRoles(subject, subjectGroups);
+          Set<TSentryRole> roles = sentryClient.listRoles(subject);
           writeToFile(writeRolesInfo(roles), desc.getResFile());
           return RETURN_CODE_SUCCESS;
         } else {

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/c6a6dff6/sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryPrivilegesResponse.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryPrivilegesResponse.java b/sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryPrivilegesResponse.java
index 7d1e18b..d34205a 100644
--- a/sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryPrivilegesResponse.java
+++ b/sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryPrivilegesResponse.java
@@ -44,7 +44,7 @@ public class TListSentryPrivilegesResponse implements org.apache.thrift.TBase<TL
   }
 
   private org.apache.sentry.service.thrift.TSentryResponseStatus status; // required
-  private Set<TSentryPrivilege> privileges; // required
+  private Set<TSentryPrivilege> privileges; // optional
 
   /** The set of fields this struct contains, along with convenience methods for finding and manipulating them. */
   public enum _Fields implements org.apache.thrift.TFieldIdEnum {
@@ -108,12 +108,13 @@ public class TListSentryPrivilegesResponse implements org.apache.thrift.TBase<TL
   }
 
   // isset id assignments
+  private _Fields optionals[] = {_Fields.PRIVILEGES};
   public static final Map<_Fields, org.apache.thrift.meta_data.FieldMetaData> metaDataMap;
   static {
     Map<_Fields, org.apache.thrift.meta_data.FieldMetaData> tmpMap = new EnumMap<_Fields, org.apache.thrift.meta_data.FieldMetaData>(_Fields.class);
     tmpMap.put(_Fields.STATUS, new org.apache.thrift.meta_data.FieldMetaData("status", org.apache.thrift.TFieldRequirementType.REQUIRED, 
         new org.apache.thrift.meta_data.StructMetaData(org.apache.thrift.protocol.TType.STRUCT, org.apache.sentry.service.thrift.TSentryResponseStatus.class)));
-    tmpMap.put(_Fields.PRIVILEGES, new org.apache.thrift.meta_data.FieldMetaData("privileges", org.apache.thrift.TFieldRequirementType.REQUIRED, 
+    tmpMap.put(_Fields.PRIVILEGES, new org.apache.thrift.meta_data.FieldMetaData("privileges", org.apache.thrift.TFieldRequirementType.OPTIONAL, 
         new org.apache.thrift.meta_data.SetMetaData(org.apache.thrift.protocol.TType.SET, 
             new org.apache.thrift.meta_data.StructMetaData(org.apache.thrift.protocol.TType.STRUCT, TSentryPrivilege.class))));
     metaDataMap = Collections.unmodifiableMap(tmpMap);
@@ -124,12 +125,10 @@ public class TListSentryPrivilegesResponse implements org.apache.thrift.TBase<TL
   }
 
   public TListSentryPrivilegesResponse(
-    org.apache.sentry.service.thrift.TSentryResponseStatus status,
-    Set<TSentryPrivilege> privileges)
+    org.apache.sentry.service.thrift.TSentryResponseStatus status)
   {
     this();
     this.status = status;
-    this.privileges = privileges;
   }
 
   /**
@@ -373,14 +372,16 @@ public class TListSentryPrivilegesResponse implements org.apache.thrift.TBase<TL
       sb.append(this.status);
     }
     first = false;
-    if (!first) sb.append(", ");
-    sb.append("privileges:");
-    if (this.privileges == null) {
-      sb.append("null");
-    } else {
-      sb.append(this.privileges);
+    if (isSetPrivileges()) {
+      if (!first) sb.append(", ");
+      sb.append("privileges:");
+      if (this.privileges == null) {
+        sb.append("null");
+      } else {
+        sb.append(this.privileges);
+      }
+      first = false;
     }
-    first = false;
     sb.append(")");
     return sb.toString();
   }
@@ -391,10 +392,6 @@ public class TListSentryPrivilegesResponse implements org.apache.thrift.TBase<TL
       throw new org.apache.thrift.protocol.TProtocolException("Required field 'status' is unset! Struct:" + toString());
     }
 
-    if (!isSetPrivileges()) {
-      throw new org.apache.thrift.protocol.TProtocolException("Required field 'privileges' is unset! Struct:" + toString());
-    }
-
     // check for sub-struct validity
     if (status != null) {
       status.validate();
@@ -482,16 +479,18 @@ public class TListSentryPrivilegesResponse implements org.apache.thrift.TBase<TL
         oprot.writeFieldEnd();
       }
       if (struct.privileges != null) {
-        oprot.writeFieldBegin(PRIVILEGES_FIELD_DESC);
-        {
-          oprot.writeSetBegin(new org.apache.thrift.protocol.TSet(org.apache.thrift.protocol.TType.STRUCT, struct.privileges.size()));
-          for (TSentryPrivilege _iter35 : struct.privileges)
+        if (struct.isSetPrivileges()) {
+          oprot.writeFieldBegin(PRIVILEGES_FIELD_DESC);
           {
-            _iter35.write(oprot);
+            oprot.writeSetBegin(new org.apache.thrift.protocol.TSet(org.apache.thrift.protocol.TType.STRUCT, struct.privileges.size()));
+            for (TSentryPrivilege _iter35 : struct.privileges)
+            {
+              _iter35.write(oprot);
+            }
+            oprot.writeSetEnd();
           }
-          oprot.writeSetEnd();
+          oprot.writeFieldEnd();
         }
-        oprot.writeFieldEnd();
       }
       oprot.writeFieldStop();
       oprot.writeStructEnd();
@@ -511,11 +510,18 @@ public class TListSentryPrivilegesResponse implements org.apache.thrift.TBase<TL
     public void write(org.apache.thrift.protocol.TProtocol prot, TListSentryPrivilegesResponse struct) throws org.apache.thrift.TException {
       TTupleProtocol oprot = (TTupleProtocol) prot;
       struct.status.write(oprot);
-      {
-        oprot.writeI32(struct.privileges.size());
-        for (TSentryPrivilege _iter36 : struct.privileges)
+      BitSet optionals = new BitSet();
+      if (struct.isSetPrivileges()) {
+        optionals.set(0);
+      }
+      oprot.writeBitSet(optionals, 1);
+      if (struct.isSetPrivileges()) {
         {
-          _iter36.write(oprot);
+          oprot.writeI32(struct.privileges.size());
+          for (TSentryPrivilege _iter36 : struct.privileges)
+          {
+            _iter36.write(oprot);
+          }
         }
       }
     }
@@ -526,18 +532,21 @@ public class TListSentryPrivilegesResponse implements org.apache.thrift.TBase<TL
       struct.status = new org.apache.sentry.service.thrift.TSentryResponseStatus();
       struct.status.read(iprot);
       struct.setStatusIsSet(true);
-      {
-        org.apache.thrift.protocol.TSet _set37 = new org.apache.thrift.protocol.TSet(org.apache.thrift.protocol.TType.STRUCT, iprot.readI32());
-        struct.privileges = new HashSet<TSentryPrivilege>(2*_set37.size);
-        for (int _i38 = 0; _i38 < _set37.size; ++_i38)
+      BitSet incoming = iprot.readBitSet(1);
+      if (incoming.get(0)) {
         {
-          TSentryPrivilege _elem39; // required
-          _elem39 = new TSentryPrivilege();
-          _elem39.read(iprot);
-          struct.privileges.add(_elem39);
+          org.apache.thrift.protocol.TSet _set37 = new org.apache.thrift.protocol.TSet(org.apache.thrift.protocol.TType.STRUCT, iprot.readI32());
+          struct.privileges = new HashSet<TSentryPrivilege>(2*_set37.size);
+          for (int _i38 = 0; _i38 < _set37.size; ++_i38)
+          {
+            TSentryPrivilege _elem39; // required
+            _elem39 = new TSentryPrivilege();
+            _elem39.read(iprot);
+            struct.privileges.add(_elem39);
+          }
         }
+        struct.setPrivilegesIsSet(true);
       }
-      struct.setPrivilegesIsSet(true);
     }
   }
 

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/c6a6dff6/sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryRolesResponse.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryRolesResponse.java b/sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryRolesResponse.java
index f3dfac2..13f22ff 100644
--- a/sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryRolesResponse.java
+++ b/sentry-provider/sentry-provider-db/src/gen/thrift/gen-javabean/org/apache/sentry/provider/db/service/thrift/TListSentryRolesResponse.java
@@ -44,7 +44,7 @@ public class TListSentryRolesResponse implements org.apache.thrift.TBase<TListSe
   }
 
   private org.apache.sentry.service.thrift.TSentryResponseStatus status; // required
-  private Set<TSentryRole> roles; // required
+  private Set<TSentryRole> roles; // optional
 
   /** The set of fields this struct contains, along with convenience methods for finding and manipulating them. */
   public enum _Fields implements org.apache.thrift.TFieldIdEnum {
@@ -108,12 +108,13 @@ public class TListSentryRolesResponse implements org.apache.thrift.TBase<TListSe
   }
 
   // isset id assignments
+  private _Fields optionals[] = {_Fields.ROLES};
   public static final Map<_Fields, org.apache.thrift.meta_data.FieldMetaData> metaDataMap;
   static {
     Map<_Fields, org.apache.thrift.meta_data.FieldMetaData> tmpMap = new EnumMap<_Fields, org.apache.thrift.meta_data.FieldMetaData>(_Fields.class);
     tmpMap.put(_Fields.STATUS, new org.apache.thrift.meta_data.FieldMetaData("status", org.apache.thrift.TFieldRequirementType.REQUIRED, 
         new org.apache.thrift.meta_data.StructMetaData(org.apache.thrift.protocol.TType.STRUCT, org.apache.sentry.service.thrift.TSentryResponseStatus.class)));
-    tmpMap.put(_Fields.ROLES, new org.apache.thrift.meta_data.FieldMetaData("roles", org.apache.thrift.TFieldRequirementType.REQUIRED, 
+    tmpMap.put(_Fields.ROLES, new org.apache.thrift.meta_data.FieldMetaData("roles", org.apache.thrift.TFieldRequirementType.OPTIONAL, 
         new org.apache.thrift.meta_data.SetMetaData(org.apache.thrift.protocol.TType.SET, 
             new org.apache.thrift.meta_data.StructMetaData(org.apache.thrift.protocol.TType.STRUCT, TSentryRole.class))));
     metaDataMap = Collections.unmodifiableMap(tmpMap);
@@ -124,12 +125,10 @@ public class TListSentryRolesResponse implements org.apache.thrift.TBase<TListSe
   }
 
   public TListSentryRolesResponse(
-    org.apache.sentry.service.thrift.TSentryResponseStatus status,
-    Set<TSentryRole> roles)
+    org.apache.sentry.service.thrift.TSentryResponseStatus status)
   {
     this();
     this.status = status;
-    this.roles = roles;
   }
 
   /**
@@ -373,14 +372,16 @@ public class TListSentryRolesResponse implements org.apache.thrift.TBase<TListSe
       sb.append(this.status);
     }
     first = false;
-    if (!first) sb.append(", ");
-    sb.append("roles:");
-    if (this.roles == null) {
-      sb.append("null");
-    } else {
-      sb.append(this.roles);
+    if (isSetRoles()) {
+      if (!first) sb.append(", ");
+      sb.append("roles:");
+      if (this.roles == null) {
+        sb.append("null");
+      } else {
+        sb.append(this.roles);
+      }
+      first = false;
     }
-    first = false;
     sb.append(")");
     return sb.toString();
   }
@@ -391,10 +392,6 @@ public class TListSentryRolesResponse implements org.apache.thrift.TBase<TListSe
       throw new org.apache.thrift.protocol.TProtocolException("Required field 'status' is unset! Struct:" + toString());
     }
 
-    if (!isSetRoles()) {
-      throw new org.apache.thrift.protocol.TProtocolException("Required field 'roles' is unset! Struct:" + toString());
-    }
-
     // check for sub-struct validity
     if (status != null) {
       status.validate();
@@ -482,16 +479,18 @@ public class TListSentryRolesResponse implements org.apache.thrift.TBase<TListSe
         oprot.writeFieldEnd();
       }
       if (struct.roles != null) {
-        oprot.writeFieldBegin(ROLES_FIELD_DESC);
-        {
-          oprot.writeSetBegin(new org.apache.thrift.protocol.TSet(org.apache.thrift.protocol.TType.STRUCT, struct.roles.size()));
-          for (TSentryRole _iter27 : struct.roles)
+        if (struct.isSetRoles()) {
+          oprot.writeFieldBegin(ROLES_FIELD_DESC);
           {
-            _iter27.write(oprot);
+            oprot.writeSetBegin(new org.apache.thrift.protocol.TSet(org.apache.thrift.protocol.TType.STRUCT, struct.roles.size()));
+            for (TSentryRole _iter27 : struct.roles)
+            {
+              _iter27.write(oprot);
+            }
+            oprot.writeSetEnd();
           }
-          oprot.writeSetEnd();
+          oprot.writeFieldEnd();
         }
-        oprot.writeFieldEnd();
       }
       oprot.writeFieldStop();
       oprot.writeStructEnd();
@@ -511,11 +510,18 @@ public class TListSentryRolesResponse implements org.apache.thrift.TBase<TListSe
     public void write(org.apache.thrift.protocol.TProtocol prot, TListSentryRolesResponse struct) throws org.apache.thrift.TException {
       TTupleProtocol oprot = (TTupleProtocol) prot;
       struct.status.write(oprot);
-      {
-        oprot.writeI32(struct.roles.size());
-        for (TSentryRole _iter28 : struct.roles)
+      BitSet optionals = new BitSet();
+      if (struct.isSetRoles()) {
+        optionals.set(0);
+      }
+      oprot.writeBitSet(optionals, 1);
+      if (struct.isSetRoles()) {
         {
-          _iter28.write(oprot);
+          oprot.writeI32(struct.roles.size());
+          for (TSentryRole _iter28 : struct.roles)
+          {
+            _iter28.write(oprot);
+          }
         }
       }
     }
@@ -526,18 +532,21 @@ public class TListSentryRolesResponse implements org.apache.thrift.TBase<TListSe
       struct.status = new org.apache.sentry.service.thrift.TSentryResponseStatus();
       struct.status.read(iprot);
       struct.setStatusIsSet(true);
-      {
-        org.apache.thrift.protocol.TSet _set29 = new org.apache.thrift.protocol.TSet(org.apache.thrift.protocol.TType.STRUCT, iprot.readI32());
-        struct.roles = new HashSet<TSentryRole>(2*_set29.size);
-        for (int _i30 = 0; _i30 < _set29.size; ++_i30)
+      BitSet incoming = iprot.readBitSet(1);
+      if (incoming.get(0)) {
         {
-          TSentryRole _elem31; // required
-          _elem31 = new TSentryRole();
-          _elem31.read(iprot);
-          struct.roles.add(_elem31);
+          org.apache.thrift.protocol.TSet _set29 = new org.apache.thrift.protocol.TSet(org.apache.thrift.protocol.TType.STRUCT, iprot.readI32());
+          struct.roles = new HashSet<TSentryRole>(2*_set29.size);
+          for (int _i30 = 0; _i30 < _set29.size; ++_i30)
+          {
+            TSentryRole _elem31; // required
+            _elem31 = new TSentryRole();
+            _elem31.read(iprot);
+            struct.roles.add(_elem31);
+          }
         }
+        struct.setRolesIsSet(true);
       }
-      struct.setRolesIsSet(true);
     }
   }
 

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/c6a6dff6/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java
index 62113c8..aec490c 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyServiceClient.java
@@ -222,7 +222,7 @@ public class SentryPolicyServiceClient {
     }
   }
 
-  public Set<TSentryRole> listRoles(String requestorUserName, Set<String> requestorUserGroupNames)
+  public Set<TSentryRole> listRoles(String requestorUserName)
        throws SentryUserException {
     return listRolesByGroupName(requestorUserName, null);
   }

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/c6a6dff6/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
index 322e90e..a1cf24a 100644
--- a/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
+++ b/sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryPolicyStoreProcessor.java
@@ -324,6 +324,9 @@ public class SentryPolicyStoreProcessor implements SentryPolicyService.Iface {
       String msg = "Role: " + request + " couldn't be retrieved.";
       LOGGER.error(msg, e);
       response.setStatus(Status.NoSuchObject(msg, e));
+    } catch (SentryAccessDeniedException e) {
+      LOGGER.error(e.getMessage(), e);
+      response.setStatus(Status.AccessDenied(e.getMessage(), e));
     } catch (Exception e) {
       String msg = "Unknown error for request: " + request + ", message: " + e.getMessage();
       LOGGER.error(msg, e);
@@ -357,6 +360,9 @@ public class SentryPolicyStoreProcessor implements SentryPolicyService.Iface {
       String msg = "Privilege: " + request + " couldn't be retrieved.";
       LOGGER.error(msg, e);
       response.setStatus(Status.NoSuchObject(msg, e));
+    } catch (SentryAccessDeniedException e) {
+      LOGGER.error(e.getMessage(), e);
+      response.setStatus(Status.AccessDenied(e.getMessage(), e));
     } catch (Exception e) {
       String msg = "Unknown error for request: " + request + ", message: " + e.getMessage();
       LOGGER.error(msg, e);

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/c6a6dff6/sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift b/sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift
index 5d584c0..86ff221 100644
--- a/sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift
+++ b/sentry-provider/sentry-provider-db/src/main/resources/sentry_policy_service.thrift
@@ -127,7 +127,7 @@ struct TSentryRole {
 }
 struct TListSentryRolesResponse {
 1: required sentry_common_service.TSentryResponseStatus status
-2: required set<TSentryRole> roles
+2: optional set<TSentryRole> roles
 }
 
 struct TSentryAuthorizable {
@@ -146,7 +146,7 @@ struct TListSentryPrivilegesRequest {
 }
 struct TListSentryPrivilegesResponse {
 1: required sentry_common_service.TSentryResponseStatus status
-2: required set<TSentryPrivilege> privileges
+2: optional set<TSentryPrivilege> privileges
 }
 
 # This API was created specifically for ProviderBackend.getPrivileges

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/c6a6dff6/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java
----------------------------------------------------------------------
diff --git a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java
index 1089390..efc5671 100644
--- a/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java
+++ b/sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java
@@ -50,7 +50,7 @@ public class TestSentryServiceIntegration extends SentryServiceIntegrationBase {
 
     client.createRole(requestorUserName, roleName);
 
-    Set<TSentryRole> roles = client.listRoles(requestorUserName, requestorUserGroupNames);
+    Set<TSentryRole> roles = client.listRoles(requestorUserName);
     assertEquals("Incorrect number of roles", 1, roles.size());
 
     for (TSentryRole role:roles) {
@@ -214,7 +214,7 @@ public class TestSentryServiceIntegration extends SentryServiceIntegrationBase {
     client.dropRoleIfExists(requestorUserName, roleName);
     client.createRole(requestorUserName, roleName);
 
-    Set<TSentryRole> roles = client.listRoles(requestorUserName, requestorUserGroupNames);
+    Set<TSentryRole> roles = client.listRoles(requestorUserName);
     assertEquals("Incorrect number of roles", 1, roles.size());
 
     client.grantRoleToGroup(requestorUserName, groupName, roleName);
@@ -244,7 +244,7 @@ public class TestSentryServiceIntegration extends SentryServiceIntegrationBase {
     client.dropRoleIfExists(requestorUserName, roleName);
     client.createRole(requestorUserName, roleName);
 
-    Set<TSentryRole> roles = client.listRoles(requestorUserName, requestorUserGroupNames);
+    Set<TSentryRole> roles = client.listRoles(requestorUserName);
     assertEquals("Incorrect number of roles", 1, roles.size());
 
     client.grantDatabasePrivilege(requestorUserName, roleName, server, db);

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/c6a6dff6/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/PolicyProviderForTest.java
----------------------------------------------------------------------
diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/PolicyProviderForTest.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/PolicyProviderForTest.java
index 79ca387..bc2920d 100644
--- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/PolicyProviderForTest.java
+++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/PolicyProviderForTest.java
@@ -74,7 +74,7 @@ public class PolicyProviderForTest extends PolicyFile {
     }
 
     // remove existing metadata
-    for (TSentryRole tRole : sentryClient.listRoles(ADMIN1, ADMIN_GROUP_SET)) {
+    for (TSentryRole tRole : sentryClient.listRoles(ADMIN1)) {
       sentryClient.dropRole(ADMIN1, tRole.getRoleName());
     }
 

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/c6a6dff6/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java
----------------------------------------------------------------------
diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java
index 84223a9..dd45a31 100644
--- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java
+++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDatabaseProvider.java
@@ -17,46 +17,26 @@
 
 package org.apache.sentry.tests.e2e.dbprovider;
 
-import org.apache.sentry.SentryUserException;
 import org.apache.sentry.provider.db.SentryAccessDeniedException;
-import org.apache.sentry.tests.e2e.hive.StaticUserGroup;
+import org.apache.sentry.provider.db.SentryAlreadyExistsException;
+import org.apache.sentry.provider.db.SentryNoSuchObjectException;
 import static org.hamcrest.Matchers.equalToIgnoringCase;
 import static org.hamcrest.Matchers.is;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 
-import java.io.File;
 import java.sql.Connection;
 import java.sql.ResultSet;
 import java.sql.ResultSetMetaData;
 import java.sql.SQLException;
 import java.sql.Statement;
 import java.util.HashSet;
-import java.util.Map;
 import java.util.Set;
-import java.util.concurrent.TimeoutException;
-
-import org.apache.commons.io.FileUtils;
-import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.hive.conf.HiveConf.ConfVars;
-import org.apache.hadoop.hive.ql.metadata.HiveException;
-import org.apache.hive.service.cli.HiveSQLException;
-import org.apache.sentry.binding.hive.SentryHiveAuthorizationTaskFactoryImpl;
-import org.apache.sentry.provider.db.SimpleDBProviderBackend;
-import org.apache.sentry.provider.file.PolicyFile;
-import org.apache.sentry.service.thrift.SentryService;
-import org.apache.sentry.service.thrift.SentryServiceFactory;
-import org.apache.sentry.service.thrift.ServiceConstants.ClientConfig;
-import org.apache.sentry.service.thrift.ServiceConstants.ServerConfig;
-import org.apache.sentry.tests.e2e.hive.hiveserver.HiveServerFactory;
-import org.junit.After;
+
 import org.junit.Before;
 import org.junit.Ignore;
 import org.junit.Test;
 
-import com.google.common.collect.Maps;
-import com.google.common.io.Files;
-
 public class TestDatabaseProvider extends AbstractTestWithDbProvider {
 
   @Before
@@ -89,7 +69,8 @@ public class TestDatabaseProvider extends AbstractTestWithDbProvider {
     connection.close();
     connection = context.createConnection(USER1_1);
     statement = context.createStatement(connection);
-    context.assertSentryServiceAccessDenied(statement, "CREATE ROLE r2");
+    context.assertSentryException(statement, "CREATE ROLE r2",
+        SentryAccessDeniedException.class.getSimpleName());
     // test default of ALL
     statement.execute("SELECT * FROM t1");
     // test a specific role
@@ -107,11 +88,11 @@ public class TestDatabaseProvider extends AbstractTestWithDbProvider {
 
 
   /**
-   * Revoke privilege
+   * Grant/Revoke privilege - Positive cases
    * @throws Exception
    */
   @Test
-  public void testRevokePrivileges() throws Exception {
+  public void testGrantRevokePrivileges() throws Exception {
     Connection connection;
     Statement statement;
     ResultSet resultSet;
@@ -120,50 +101,133 @@ public class TestDatabaseProvider extends AbstractTestWithDbProvider {
     statement = context.createStatement(connection);
     statement.execute("CREATE ROLE role1");
 
-    //Revoke All on server by admin
+    //Grant/Revoke All on server by admin
     statement.execute("GRANT ALL ON SERVER server1 to role role1");
     resultSet = statement.executeQuery("SHOW GRANT ROLE role1");
     assertResultSize(resultSet, 1);
+    while(resultSet.next()) {
+      assertThat(resultSet.getString(1), equalToIgnoringCase("*"));
+      assertThat(resultSet.getString(2), equalToIgnoringCase(""));
+      assertThat(resultSet.getString(3), equalToIgnoringCase(""));//partition
+      assertThat(resultSet.getString(4), equalToIgnoringCase(""));//column
+      assertThat(resultSet.getString(5), equalToIgnoringCase("role1"));//principalName
+      assertThat(resultSet.getString(6), equalToIgnoringCase("role"));//principalType
+      assertThat(resultSet.getString(7), equalToIgnoringCase("*"));
+      assertThat(resultSet.getBoolean(8), is(new Boolean("False")));//grantOption
+      //Create time is not tested
+      //assertThat(resultSet.getLong(9), is(new Long(0)));
+      assertThat(resultSet.getString(10), equalToIgnoringCase(ADMIN1));//grantor
+    }
+
     statement.execute("REVOKE ALL ON SERVER server1 from role role1");
     resultSet = statement.executeQuery("SHOW GRANT ROLE role1");
     assertResultSize(resultSet, 0);
 
-    //Revoke All on database by admin
+    //Grant/Revoke All on database by admin
     statement.execute("GRANT ALL ON DATABASE default to role role1");
     resultSet = statement.executeQuery("SHOW GRANT ROLE role1");
     assertResultSize(resultSet, 1);
+    while(resultSet.next()) {
+      assertThat(resultSet.getString(1), equalToIgnoringCase("default"));
+      assertThat(resultSet.getString(2), equalToIgnoringCase(""));
+      assertThat(resultSet.getString(3), equalToIgnoringCase(""));//partition
+      assertThat(resultSet.getString(4), equalToIgnoringCase(""));//column
+      assertThat(resultSet.getString(5), equalToIgnoringCase("role1"));//principalName
+      assertThat(resultSet.getString(6), equalToIgnoringCase("role"));//principalType
+      assertThat(resultSet.getString(7), equalToIgnoringCase("*"));
+      assertThat(resultSet.getBoolean(8), is(new Boolean("False")));//grantOption
+      //Create time is not tested
+      //assertThat(resultSet.getLong(9), is(new Long(0)));
+      assertThat(resultSet.getString(10), equalToIgnoringCase(ADMIN1));//grantor
+    }
+
     statement.execute("REVOKE ALL ON DATABASE default from role role1");
     resultSet = statement.executeQuery("SHOW GRANT ROLE role1");
     assertResultSize(resultSet, 0);
 
-    //Revoke All on URI by admin
+    //Grant/Revoke All on URI by admin
     statement.execute("GRANT ALL ON URI 'file:///path' to role role1");
     resultSet = statement.executeQuery("SHOW GRANT ROLE role1");
     assertResultSize(resultSet, 1);
+    while(resultSet.next()) {
+      assertThat(resultSet.getString(1), equalToIgnoringCase("file://path"));
+      assertThat(resultSet.getString(2), equalToIgnoringCase(""));
+      assertThat(resultSet.getString(3), equalToIgnoringCase(""));//partition
+      assertThat(resultSet.getString(4), equalToIgnoringCase(""));//column
+      assertThat(resultSet.getString(5), equalToIgnoringCase("role1"));//principalName
+      assertThat(resultSet.getString(6), equalToIgnoringCase("role"));//principalType
+      assertThat(resultSet.getString(7), equalToIgnoringCase("*"));
+      assertThat(resultSet.getBoolean(8), is(new Boolean("False")));//grantOption
+      //Create time is not tested
+      //assertThat(resultSet.getLong(9), is(new Long(0)));
+      assertThat(resultSet.getString(10), equalToIgnoringCase(ADMIN1));//grantor
+    }
     statement.execute("REVOKE ALL ON URI 'file:///path' from role role1");
     resultSet = statement.executeQuery("SHOW GRANT ROLE role1");
     assertResultSize(resultSet, 0);
 
-    //Revoke All on table by admin
+    //Grant/Revoke All on table by admin
     statement.execute("GRANT ALL ON TABLE tab1 to role role1");
     resultSet = statement.executeQuery("SHOW GRANT ROLE role1");
     assertResultSize(resultSet, 1);
+    while(resultSet.next()) {
+      assertThat(resultSet.getString(1), equalToIgnoringCase("default"));
+      assertThat(resultSet.getString(2), equalToIgnoringCase("tab1"));
+      assertThat(resultSet.getString(3), equalToIgnoringCase(""));//partition
+      assertThat(resultSet.getString(4), equalToIgnoringCase(""));//column
+      assertThat(resultSet.getString(5), equalToIgnoringCase("role1"));//principalName
+      assertThat(resultSet.getString(6), equalToIgnoringCase("role"));//principalType
+      assertThat(resultSet.getString(7), equalToIgnoringCase("*"));
+      assertThat(resultSet.getBoolean(8), is(new Boolean("False")));//grantOption
+      //Create time is not tested
+      //assertThat(resultSet.getLong(9), is(new Long(0)));
+      assertThat(resultSet.getString(10), equalToIgnoringCase(ADMIN1));//grantor
+    }
+
     statement.execute("REVOKE ALL ON TABLE tab1 from role role1");
     resultSet = statement.executeQuery("SHOW GRANT ROLE role1");
     assertResultSize(resultSet, 0);
 
-    //Revoke INSERT on table by admin
+    //Grant/Revoke INSERT on table by admin
     statement.execute("GRANT INSERT ON TABLE tab1 to role role1");
     resultSet = statement.executeQuery("SHOW GRANT ROLE role1");
     assertResultSize(resultSet, 1);
+    while(resultSet.next()) {
+      assertThat(resultSet.getString(1), equalToIgnoringCase("default"));
+      assertThat(resultSet.getString(2), equalToIgnoringCase("tab1"));
+      assertThat(resultSet.getString(3), equalToIgnoringCase(""));//partition
+      assertThat(resultSet.getString(4), equalToIgnoringCase(""));//column
+      assertThat(resultSet.getString(5), equalToIgnoringCase("role1"));//principalName
+      assertThat(resultSet.getString(6), equalToIgnoringCase("role"));//principalType
+      assertThat(resultSet.getString(7), equalToIgnoringCase("insert"));
+      assertThat(resultSet.getBoolean(8), is(new Boolean("False")));//grantOption
+      //Create time is not tested
+      //assertThat(resultSet.getLong(9), is(new Long(0)));
+      assertThat(resultSet.getString(10), equalToIgnoringCase(ADMIN1));//grantor
+    }
+
     statement.execute("REVOKE INSERT ON TABLE tab1 from role role1");
     resultSet = statement.executeQuery("SHOW GRANT ROLE role1");
     assertResultSize(resultSet, 0);
 
-    //Revoke SELECT on table by admin
+    //Grant/Revoke SELECT on table by admin
     statement.execute("GRANT SELECT ON TABLE tab1 to role role1");
     resultSet = statement.executeQuery("SHOW GRANT ROLE role1");
     assertResultSize(resultSet, 1);
+    while(resultSet.next()) {
+      assertThat(resultSet.getString(1), equalToIgnoringCase("default"));
+      assertThat(resultSet.getString(2), equalToIgnoringCase("tab1"));
+      assertThat(resultSet.getString(3), equalToIgnoringCase(""));//partition
+      assertThat(resultSet.getString(4), equalToIgnoringCase(""));//column
+      assertThat(resultSet.getString(5), equalToIgnoringCase("role1"));//principalName
+      assertThat(resultSet.getString(6), equalToIgnoringCase("role"));//principalType
+      assertThat(resultSet.getString(7), equalToIgnoringCase("select"));
+      assertThat(resultSet.getBoolean(8), is(new Boolean("False")));//grantOption
+      //Create time is not tested
+      //assertThat(resultSet.getLong(9), is(new Long(0)));
+      assertThat(resultSet.getString(10), equalToIgnoringCase(ADMIN1));//grantor
+    }
+
     statement.execute("REVOKE SELECT ON TABLE tab1 from role role1");
     resultSet = statement.executeQuery("SHOW GRANT ROLE role1");
     assertResultSize(resultSet, 0);
@@ -187,7 +251,6 @@ public class TestDatabaseProvider extends AbstractTestWithDbProvider {
       //Create time is not tested
       //assertThat(resultSet.getLong(9), is(new Long(0)));
       assertThat(resultSet.getString(10), equalToIgnoringCase(ADMIN1));//grantor
-
     }
 
     //Revoke Partial privilege on table by admin
@@ -225,6 +288,129 @@ public class TestDatabaseProvider extends AbstractTestWithDbProvider {
   }
 
   /**
+   * Create and Drop role by admin
+   * @throws Exception
+   */
+  @Test
+  public void testCreateDropRole() throws Exception {
+    Connection connection = context.createConnection(ADMIN1);
+    Statement statement = context.createStatement(connection);
+    statement.execute("CREATE ROLE role1");
+    ResultSet resultSet = statement.executeQuery("SHOW roles");
+    assertResultSize(resultSet, 1);
+    statement.execute("DROP ROLE role1");
+    resultSet = statement.executeQuery("SHOW roles");
+    assertResultSize(resultSet, 0);
+  }
+
+  /**
+   * Corner cases
+   * @throws Exception
+   */
+  @Test
+  public void testCornerCases() throws Exception {
+    Connection connection = context.createConnection(ADMIN1);
+    Statement statement = context.createStatement(connection);
+
+    //Drop a role which does not exist
+    context.assertSentryException(statement, "DROP ROLE role1",
+        SentryNoSuchObjectException.class.getSimpleName());
+
+    //Create a role which already exists
+    statement.execute("CREATE ROLE role1");
+    context.assertSentryException(statement, "CREATE ROLE role1",
+        SentryAlreadyExistsException.class.getSimpleName());
+
+    //Drop role when privileges mapping exists and create role with same name, old mappings should not exist
+    //state: role1
+    statement.execute("GRANT ROLE role1 TO GROUP " + USERGROUP1);
+    statement.execute("GRANT ALL ON SERVER server1 TO ROLE role1");
+    ResultSet resultSet = statement.executeQuery("SHOW GRANT ROLE role1");
+    assertResultSize(resultSet, 1);
+    statement.execute("DROP ROLE role1");
+    statement.execute("CREATE ROLE role1");
+    resultSet = statement.executeQuery("SHOW GRANT ROLE role1");
+    assertResultSize(resultSet, 0);
+
+
+    //Grant role, when role doesn't exist
+    //state: role1
+    context.assertSentryException(statement, "GRANT role role2 TO GROUP " + USERGROUP1,
+        SentryNoSuchObjectException.class.getSimpleName());
+
+
+    //Grant multiple roles to a group
+    //state: role1
+    statement.execute("CREATE ROLE role2");
+    statement.execute("GRANT ROLE role2 to GROUP " + USERGROUP1);
+    statement.execute("GRANT ROLE role1 to GROUP " + USERGROUP1);
+    resultSet = statement.executeQuery("SHOW ROLE GRANT GROUP " + USERGROUP1);
+    assertResultSize(resultSet, 2);
+
+    //Grant role when mapping exists
+    //state: role1, role2 -> usergroup1
+    statement.execute("GRANT ROLE role1 to GROUP " + USERGROUP1);
+
+    //Revoke role after role has been dropped
+    //state: role1, role2 -> usergroup1
+    statement.execute("DROP ROLE role2");
+    context.assertSentryException(statement, "REVOKE role role2 from group " + USERGROUP1,
+        SentryNoSuchObjectException.class.getSimpleName());
+
+    //Revoke role from a group when mapping doesnt exist
+    //state: role1 -> usergroup1
+    statement.execute("REVOKE ROLE role1 from GROUP " + USERGROUP1);
+    statement.execute("REVOKE ROLE role1 from GROUP " + USERGROUP1);
+
+    //Grant privilege to a role, privilege already exists, mapping already exists
+    //state: role1
+    //TODO: Remove this comment SENTRY-181
+    statement.execute("CREATE ROLE role2");
+    statement.execute("GRANT ALL ON SERVER server1 TO ROLE role1");
+    statement.execute("GRANT ALL ON SERVER server1 TO ROLE role1");
+    statement.execute("GRANT ALL ON SERVER server1 TO ROLE role2");
+    resultSet = statement.executeQuery("SHOW GRANT ROLE role1");
+    assertResultSize(resultSet, 1);
+    resultSet = statement.executeQuery("SHOW GRANT ROLE role2");
+    assertResultSize(resultSet, 1);
+
+    //Multiple privileges to a role
+    //state: role1,role2 -> grant all on server
+    statement.execute("GRANT ALL ON TABLE tab1 to ROLE role2");
+    statement.execute("GRANT ALL,INSERT ON TABLE tab1 to ROLE role2");
+    resultSet = statement.executeQuery("SHOW GRANT ROLE role2");
+    assertResultSize(resultSet, 3);
+    statement.execute("DROP role role2");
+
+    //Revoke privilege when privilege doesnt exist
+    //state: role1 -> grant all on server
+    statement.execute("REVOKE ALL ON TABLE tab1 from role role1");
+    resultSet = statement.executeQuery("SHOW GRANT ROLE role1");
+    int count = 0;
+    //Verify we still have all on server
+    while(resultSet.next()) {
+      count++;
+      assertThat(resultSet.getString(1), equalToIgnoringCase("*"));
+      assertThat(resultSet.getString(2), equalToIgnoringCase(""));
+      assertThat(resultSet.getString(7), equalToIgnoringCase("*"));
+
+    }
+    assertThat(count, is(1));
+
+    //Revoke privilege, when role doesnt exist
+    //state: role1 -> grant all on server
+    context.assertSentryException(statement, "REVOKE ALL ON SERVER server1 from role role2",
+        SentryNoSuchObjectException.class.getSimpleName());
+
+    //Revoke privilege when privilege exists but mapping doesnt exist
+    //state: role1 -> grant all on server
+    statement.execute("CREATE ROLE role2");
+    statement.execute("GRANT ALL on TABLE tab1 to ROLE role2");
+    statement.execute("REVOKE ALL on TABLE tab1 from Role role1");
+    resultSet = statement.executeQuery("SHOW GRANT ROLE role1");
+    assertResultSize(resultSet, 1);
+  }
+  /**
    * SHOW ROLES
    * @throws Exception
    */
@@ -232,9 +418,11 @@ public class TestDatabaseProvider extends AbstractTestWithDbProvider {
   public void testShowRoles() throws Exception {
     Connection connection = context.createConnection(ADMIN1);
     Statement statement = context.createStatement(connection);
+    ResultSet resultSet = statement.executeQuery("SHOW ROLES");
+    assertResultSize(resultSet, 0);
     statement.execute("CREATE ROLE role1");
     statement.execute("CREATE ROLE role2");
-    ResultSet resultSet = statement.executeQuery("SHOW ROLES");
+    resultSet = statement.executeQuery("SHOW ROLES");
     ResultSetMetaData resultSetMetaData = resultSet.getMetaData();
     assertThat(resultSetMetaData.getColumnCount(), is(1));
     assertThat(resultSetMetaData.getColumnName(1), equalToIgnoringCase("role"));
@@ -243,7 +431,7 @@ public class TestDatabaseProvider extends AbstractTestWithDbProvider {
     while ( resultSet.next()) {
       roles.add(resultSet.getString(1));
     }
-    assertThat(roles.size(), is(new Integer(2)));
+    assertThat(roles.size(), is(2));
     assertTrue(roles.contains("role1"));
     assertTrue(roles.contains("role2"));
     statement.close();
@@ -258,6 +446,8 @@ public class TestDatabaseProvider extends AbstractTestWithDbProvider {
   public void testShowRolesByGroup() throws Exception {
     Connection connection = context.createConnection(ADMIN1);
     Statement statement = context.createStatement(connection);
+    context.assertSentryException(statement,"SHOW ROLE GRANT GROUP " + ADMINGROUP,
+        SentryNoSuchObjectException.class.getSimpleName());
     statement.execute("CREATE ROLE role1");
     statement.execute("CREATE ROLE role2");
     statement.execute("CREATE ROLE role3");
@@ -290,10 +480,12 @@ public class TestDatabaseProvider extends AbstractTestWithDbProvider {
     Connection connection = context.createConnection(ADMIN1);
     Statement statement = context.createStatement(connection);
     statement.execute("CREATE ROLE role1");
+    ResultSet resultSet = statement.executeQuery("SHOW GRANT ROLE role1");
+    assertResultSize(resultSet, 0);
     statement.execute("CREATE ROLE role2");
     statement.execute("GRANT SELECT ON TABLE t1 TO ROLE role1");
 
-    ResultSet resultSet = statement.executeQuery("SHOW GRANT ROLE role1");
+    resultSet = statement.executeQuery("SHOW GRANT ROLE role1");
     ResultSetMetaData resultSetMetaData = resultSet.getMetaData();
     //| database  | table  | partition  | column  | principal_name  |
     // principal_type | privilege  | grant_option  | grant_time  | grantor  |
@@ -506,6 +698,7 @@ public class TestDatabaseProvider extends AbstractTestWithDbProvider {
    */
   @Test
   public void testShowCurrentRole() throws Exception {
+    //TODO: Add more test cases once we fix SENTRY-268
     Connection connection = context.createConnection(ADMIN1);
     Statement statement = context.createStatement(connection);
     statement.execute("CREATE ROLE role1");

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/c6a6dff6/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbEndToEnd.java
----------------------------------------------------------------------
diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbEndToEnd.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbEndToEnd.java
index 9c0c8b5..2198c05 100644
--- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbEndToEnd.java
+++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbEndToEnd.java
@@ -17,6 +17,7 @@
 
 package org.apache.sentry.tests.e2e.dbprovider;
 
+import org.apache.sentry.provider.db.SentryAccessDeniedException;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 
@@ -60,7 +61,8 @@ public class TestDbEndToEnd extends AbstractTestWithDbProvider {
     connection.close();
     connection = context.createConnection(USER1_1);
     statement = context.createStatement(connection);
-    context.assertSentryServiceAccessDenied(statement, "CREATE ROLE r2");
+    context.assertSentryException(statement, "CREATE ROLE r2",
+        SentryAccessDeniedException.class.getSimpleName());
     // test default of ALL
     statement.execute("SELECT * FROM t1");
     // test a specific role

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/c6a6dff6/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java
----------------------------------------------------------------------
diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java
index 8beedd7..a8ce2a2 100644
--- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java
+++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java
@@ -17,6 +17,9 @@
 
 package org.apache.sentry.tests.e2e.dbprovider;
 
+import org.apache.hadoop.hive.ql.metadata.AuthorizationException;
+import org.apache.hadoop.hive.ql.parse.SemanticException;
+import org.apache.sentry.provider.db.SentryAccessDeniedException;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
@@ -36,6 +39,7 @@ import org.apache.sentry.provider.file.PolicyFile;
 import org.apache.sentry.tests.e2e.hive.DummySentryOnFailureHook;
 import org.apache.sentry.tests.e2e.hive.StaticUserGroup;
 import org.apache.sentry.tests.e2e.hive.hiveserver.HiveServerFactory;
+import org.junit.Assume;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -56,6 +60,16 @@ public class TestDbSentryOnFailureHookLoading extends AbstractTestWithDbProvider
     policyFile = PolicyFile.setAdminOnServer1(ADMINGROUP);
     createContext(testProperties);
     DummySentryOnFailureHook.invoked = false;
+
+    // Do not run these tests if run with external HiveServer2
+    // This test checks for a static member, which will not
+    // be set if HiveServer2 and the test run in different JVMs
+    String hiveServer2Type = System
+        .getProperty(HiveServerFactory.HIVESERVER2_TYPE);
+    if(hiveServer2Type != null) {
+      Assume.assumeTrue(HiveServerFactory.HiveServer2Type.valueOf(hiveServer2Type.trim()) ==
+              HiveServerFactory.HiveServer2Type.InternalHiveServer2);
+    }
   }
 
   /* Admin creates database DB_2
@@ -64,30 +78,6 @@ public class TestDbSentryOnFailureHookLoading extends AbstractTestWithDbProvider
   @Test
   public void testOnFailureHookLoading() throws Exception {
 
-    // Do not run this test if run with external HiveServer2
-    // This test checks for a static member, which will not
-    // be set if HiveServer2 and the test run in different JVMs
-    String hiveServer2Type = System.getProperty(
-        HiveServerFactory.HIVESERVER2_TYPE);
-    if (hiveServer2Type != null &&
-        HiveServerFactory.HiveServer2Type.valueOf(hiveServer2Type.trim()) !=
-        HiveServerFactory.HiveServer2Type.InternalHiveServer2) {
-      return;
-    }
-
-    File dataDir = context.getDataDir();
-    //copy data file to test dir
-    File dataFile = new File(dataDir, SINGLE_TYPE_DATA_FILE_NAME);
-    FileOutputStream to = new FileOutputStream(dataFile);
-    Resources.copy(Resources.getResource(SINGLE_TYPE_DATA_FILE_NAME), to);
-    to.close();
-
-    policyFile
-.addRolesToGroup(USERGROUP1, "all_db1")
-        .addPermissionsToRole("all_db1", "server=server1->db=DB_1")
-        .setUserGroupMapping(StaticUserGroup.getStaticMapping())
-        .write(context.getPolicyFile());
-
     // setup db objects needed by the test
     Connection connection = context.createConnection(ADMIN1);
     Statement statement = context.createStatement(connection);
@@ -104,6 +94,7 @@ public class TestDbSentryOnFailureHookLoading extends AbstractTestWithDbProvider
     statement.execute("DROP DATABASE IF EXISTS DB_2 CASCADE");
     statement.execute("CREATE DATABASE DB_1");
     statement.execute("CREATE DATABASE DB_2");
+    statement.execute("CREATE TABLE db_2.tab1(a int )");
     statement.close();
     connection.close();
 
@@ -111,15 +102,19 @@ public class TestDbSentryOnFailureHookLoading extends AbstractTestWithDbProvider
     connection = context.createConnection(USER1_1);
     statement = context.createStatement(connection);
 
-    // negative test case: user can't create table in other user's database
-    assertFalse(DummySentryOnFailureHook.invoked);
-    DummySentryOnFailureHook.setHiveOp(HiveOperation.CREATETABLE);
-      try {
-      statement.execute("CREATE TABLE DB2.TAB2(id INT)");
-      Assert.fail("Expected SQL exception");
-    } catch (SQLException e) {
-      assertTrue(DummySentryOnFailureHook.invoked);
-    }
+    // Failure hook for create table when table doesnt exist
+    verifyFailureHook(statement, "CREATE TABLE DB_2.TAB2(id INT)", HiveOperation.CREATETABLE, "db_2", null, false);
+
+    // Failure hook for create table when table exist
+    verifyFailureHook(statement, "CREATE TABLE DB_2.TAB1(id INT)", HiveOperation.CREATETABLE, "db_2", null, false);
+
+    // Failure hook for select * from table when table exist
+    verifyFailureHook(statement, "select * from db_2.tab1", HiveOperation.QUERY,
+        null, null, false);
+
+    //Denied alter table is not invoking failurehook: SENTRY-269
+    //verifyFailureHook(statement, "ALTER TABLE db_2.tab1 CHANGE id id1 INT", HiveOperation.ALTERTABLE_RENAMECOL,
+    //    "db_2", "tab1", false);
 
     statement.close();
     connection.close();
@@ -141,17 +136,6 @@ public class TestDbSentryOnFailureHookLoading extends AbstractTestWithDbProvider
   @Test
   public void testOnFailureHookForAuthDDL() throws Exception {
 
-    // Do not run this test if run with external HiveServer2
-    // This test checks for a static member, which will not
-    // be set if HiveServer2 and the test run in different JVMs
-    String hiveServer2Type = System
-        .getProperty(HiveServerFactory.HIVESERVER2_TYPE);
-    if (hiveServer2Type != null
-        && HiveServerFactory.HiveServer2Type.valueOf(hiveServer2Type.trim()) != HiveServerFactory.HiveServer2Type.InternalHiveServer2) {
-      return;
-    }
-
-    File dataDir = context.getDataDir();
     policyFile.addRolesToGroup(USERGROUP1, "all_db1")
         .addPermissionsToRole("all_db1", "server=server1->db=DB_1")
         .setUserGroupMapping(StaticUserGroup.getStaticMapping())
@@ -179,19 +163,54 @@ public class TestDbSentryOnFailureHookLoading extends AbstractTestWithDbProvider
     statement.execute("CREATE TABLE foo (id int)");
 
     verifyFailureHook(statement, "CREATE ROLE fooTest",
-        HiveOperation.CREATEROLE);
-    verifyFailureHook(statement, "DROP ROLE fooTest", HiveOperation.DROPROLE);
+        HiveOperation.CREATEROLE, null, null, true);
+
+    verifyFailureHook(statement, "DROP ROLE fooTest",
+        HiveOperation.DROPROLE, null, null, true);
+
     verifyFailureHook(statement,
         "GRANT ALL ON SERVER server1 TO ROLE admin_role",
-        HiveOperation.GRANT_PRIVILEGE);
+        HiveOperation.GRANT_PRIVILEGE, null, null, true);
+
     verifyFailureHook(statement,
         "REVOKE ALL ON SERVER server1 FROM ROLE admin_role",
-        HiveOperation.REVOKE_PRIVILEGE);
+        HiveOperation.REVOKE_PRIVILEGE, null, null, true);
+
     verifyFailureHook(statement, "GRANT ROLE all_db1 TO GROUP " + USERGROUP1,
-        HiveOperation.GRANT_ROLE);
+        HiveOperation.GRANT_ROLE, null, null, true);
+
     verifyFailureHook(statement,
         "REVOKE ROLE all_db1 FROM GROUP " + USERGROUP1,
-        HiveOperation.GRANT_ROLE);
+        HiveOperation.REVOKE_ROLE, null, null, true);
+
+    verifyFailureHook(statement, "SHOW ROLES",
+        HiveOperation.SHOW_ROLES, null, null, true);
+
+    verifyFailureHook(statement, "SHOW ROLE GRANT group group1",
+        HiveOperation.SHOW_ROLE_GRANT, null, null, true);
+
+    verifyFailureHook(statement, "SHOW GRANT role role1",
+        HiveOperation.SHOW_GRANT, null, null, true);
+
+    //Grant privilege on table doesnt expose db and table objects
+    verifyFailureHook(statement,
+        "GRANT ALL ON TABLE tab1 TO ROLE admin_role",
+        HiveOperation.GRANT_PRIVILEGE, null, null, true);
+
+    //Revoke privilege on table doesnt expose db and table objects
+    verifyFailureHook(statement,
+        "REVOKE ALL ON TABLE server1 FROM ROLE admin_role",
+        HiveOperation.REVOKE_PRIVILEGE, null, null, true);
+
+    //Grant privilege on database doesnt expose db and table objects
+    verifyFailureHook(statement,
+        "GRANT ALL ON Database db_1 TO ROLE admin_role",
+        HiveOperation.GRANT_PRIVILEGE, null, null, true);
+
+    //Revoke privilege on database doesnt expose db and table objects
+    verifyFailureHook(statement,
+        "REVOKE ALL ON Database db_1 FROM ROLE admin_role",
+        HiveOperation.REVOKE_PRIVILEGE, null, null, true);
 
     statement.close();
     connection.close();
@@ -199,11 +218,10 @@ public class TestDbSentryOnFailureHookLoading extends AbstractTestWithDbProvider
   }
 
   // run the given statement and verify that failure hook is invoked as expected
-  private void verifyFailureHook(Statement statement, String sqlStr,
-      HiveOperation expectedOp) throws Exception {
+  private void verifyFailureHook(Statement statement, String sqlStr, HiveOperation expectedOp,
+       String dbName, String tableName, boolean checkSentryAccessDeniedException) throws Exception {
     // negative test case: non admin user can't create role
     assertFalse(DummySentryOnFailureHook.invoked);
-    DummySentryOnFailureHook.setHiveOp(HiveOperation.CREATEROLE);
     try {
       statement.execute(sqlStr);
       Assert.fail("Expected SQL exception for " + sqlStr);
@@ -212,5 +230,21 @@ public class TestDbSentryOnFailureHookLoading extends AbstractTestWithDbProvider
     } finally {
       DummySentryOnFailureHook.invoked = false;
     }
+    if (expectedOp != null) {
+      Assert.assertNotNull("Hive op is null for op: " + expectedOp, DummySentryOnFailureHook.hiveOp);
+      Assert.assertTrue(expectedOp.equals(DummySentryOnFailureHook.hiveOp));
+    }
+    if (checkSentryAccessDeniedException) {
+      Assert.assertTrue("Expected SentryDeniedException for op: " + expectedOp,
+          DummySentryOnFailureHook.exception.getCause() instanceof SentryAccessDeniedException);
+    }
+    if(tableName != null) {
+      Assert.assertNotNull("Table object is null for op: " + expectedOp, DummySentryOnFailureHook.table);
+      Assert.assertTrue(tableName.equalsIgnoreCase(DummySentryOnFailureHook.table.getName()));
+    }
+    if(dbName != null) {
+      Assert.assertNotNull("Database object is null for op: " + expectedOp, DummySentryOnFailureHook.db);
+      Assert.assertTrue(dbName.equalsIgnoreCase(DummySentryOnFailureHook.db.getName()));
+    }
   }
 }

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/c6a6dff6/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java
----------------------------------------------------------------------
diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java
index 39d411e..d8f5256 100644
--- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java
+++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/Context.java
@@ -173,16 +173,15 @@ public class Context {
     }
   }
 
-  public void assertSentryServiceAccessDenied(Statement statement, String query)
+  public void assertSentryException(Statement statement, String query, String exceptionType)
       throws SQLException {
     try {
       statement.execute(query);
       Assert.fail("Expected SQLException for '" + query + "'");
     } catch (SQLException e) {
       verifyAuthzExceptionForState(e, AUTHZ_LINK_FAILURE_SQL_STATE);
-      Assert.assertTrue("Expected SentryAccessDeniedException in " + e.getMessage(),
-          Strings.nullToEmpty(e.getMessage()).contains(SentryAccessDeniedException.class
-              .getSimpleName()));
+      Assert.assertTrue("Expected " + exceptionType + " : " + e.getMessage(),
+          Strings.nullToEmpty(e.getMessage()).contains(exceptionType));
     }
   }
 

http://git-wip-us.apache.org/repos/asf/incubator-sentry/blob/c6a6dff6/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/DummySentryOnFailureHook.java
----------------------------------------------------------------------
diff --git a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/DummySentryOnFailureHook.java b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/DummySentryOnFailureHook.java
index 079f273..4838f76 100644
--- a/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/DummySentryOnFailureHook.java
+++ b/sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/DummySentryOnFailureHook.java
@@ -19,28 +19,29 @@ package org.apache.sentry.tests.e2e.hive;
 
 import junit.framework.Assert;
 
+import org.apache.hadoop.hive.ql.metadata.AuthorizationException;
 import org.apache.hadoop.hive.ql.plan.HiveOperation;
 import org.apache.sentry.binding.hive.SentryOnFailureHook;
 import org.apache.sentry.binding.hive.SentryOnFailureHookContext;
+import org.apache.sentry.core.model.db.Database;
+import org.apache.sentry.core.model.db.Table;
+import org.apache.sentry.provider.db.SentryAccessDeniedException;
 
 public class DummySentryOnFailureHook implements SentryOnFailureHook {
 
   public static boolean invoked = false;
-  public static boolean checkHiveOp = false;
   public static HiveOperation hiveOp;
-
-  public static void setHiveOp(HiveOperation newHiveOp) {
-    checkHiveOp = true;
-    hiveOp = newHiveOp;
-  }
+  public static Database db;
+  public static Table table;
+  public static AuthorizationException exception;
 
   @Override
   public void run(SentryOnFailureHookContext failureHookContext)
       throws Exception {
     invoked = true;
-    if (checkHiveOp) {
-      checkHiveOp = false;
-      Assert.assertTrue(hiveOp.equals(failureHookContext.getHiveOp()));
-    }
+    hiveOp = failureHookContext.getHiveOp();
+    db = failureHookContext.getDatabase();
+    table = failureHookContext.getTable();
+    exception = failureHookContext.getException();
   }
 }