impala-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mjac...@apache.org
Subject [1/3] incubator-impala git commit: IMPALA-5582: Store sentry privileges in lower case
Date Sat, 22 Jul 2017 01:19:33 GMT
Repository: incubator-impala
Updated Branches:
  refs/heads/master daff8eb0c -> ad0c6e749


IMPALA-5582: Store sentry privileges in lower case

Privileges granted to a role assigned to a db/table whose name
contains upper case characters can disappear after a few seconds.
A privilege is inserted into the catalogObjectCache using a key
that uses the db/table name. The key gets converted to a lower
case before inserting.
Privilege name returned by sentryProxy is always lower case,
which might not match the privilegeName built in the catalog.
This triggers an update of the catalog object followed by a
removal of the old object. Since they both use the same key
in lower case it ends up deleting the newly updated object.

This change also adds a new catalogd startup option
(sentry_catalog_polling_frequency)
to configure the frequency at which catalogd polls the sentry service
to update any policy changes. The default value is 60 seconds.

Test:
Added a test which adds select privileges to 3 tables and dbs specified
in lower case, upper case and mixed case. The test verifies that the
privileges on the 3 tables do not disappear on a sentry update.

Change-Id: Ide3dfa601fcf77f5acc6adce9bea443aea600901
Reviewed-on: http://gerrit.cloudera.org:8080/7332
Reviewed-by: Matthew Jacobs <mj@cloudera.com>
Tested-by: Impala Public Jenkins


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

Branch: refs/heads/master
Commit: 322ccb0e4918623dad4edae68ad422018e58c7b5
Parents: daff8eb
Author: aphadke <aphadke@cloudera.com>
Authored: Tue Jun 27 12:14:04 2017 -0700
Committer: Impala Public Jenkins <impala-public-jenkins@gerrit.cloudera.org>
Committed: Fri Jul 21 19:16:57 2017 +0000

----------------------------------------------------------------------
 be/src/catalog/catalog.cc                       |   3 +
 be/src/util/backend-gflag-util.cc               |   2 +
 common/thrift/BackendGflags.thrift              |   6 +-
 .../impala/catalog/AuthorizationPolicy.java     |  11 +-
 .../apache/impala/catalog/RolePrivilege.java    |  34 ++++--
 .../apache/impala/service/BackendConfig.java    |   3 +
 .../org/apache/impala/util/SentryProxy.java     |   7 +-
 .../queries/QueryTest/grant_revoke.test         | 100 ++++++++--------
 tests/authorization/test_grant_revoke.py        | 115 +++++++++++++++----
 9 files changed, 188 insertions(+), 93 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/322ccb0e/be/src/catalog/catalog.cc
----------------------------------------------------------------------
diff --git a/be/src/catalog/catalog.cc b/be/src/catalog/catalog.cc
index 3ed8d7f..be7b93f 100644
--- a/be/src/catalog/catalog.cc
+++ b/be/src/catalog/catalog.cc
@@ -38,6 +38,9 @@ DEFINE_int32(num_metadata_loading_threads, 16,
 DEFINE_int32(initial_hms_cnxn_timeout_s, 120,
     "Number of seconds catalogd will wait to establish an initial connection to the HMS "
     "before exiting.");
+DEFINE_int64(sentry_catalog_polling_frequency_s, 60,
+    "Frequency (in seconds) at which the the catalogd polls the sentry service to update
"
+    "any policy changes.");
 DEFINE_string(sentry_config, "", "Local path to a sentry-site.xml configuration "
     "file. If set, authorization will be enabled.");
 

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/322ccb0e/be/src/util/backend-gflag-util.cc
----------------------------------------------------------------------
diff --git a/be/src/util/backend-gflag-util.cc b/be/src/util/backend-gflag-util.cc
index ab66a73..5523735 100644
--- a/be/src/util/backend-gflag-util.cc
+++ b/be/src/util/backend-gflag-util.cc
@@ -33,6 +33,7 @@ DECLARE_int32(read_size);
 DECLARE_int32(num_metadata_loading_threads);
 DECLARE_int32(initial_hms_cnxn_timeout_s);
 DECLARE_int32(kudu_operation_timeout_ms);
+DECLARE_int64(sentry_catalog_polling_frequency_s);
 DECLARE_int64(inc_stats_size_limit_bytes);
 DECLARE_string(principal);
 DECLARE_string(lineage_event_log_dir);
@@ -73,6 +74,7 @@ Status GetThriftBackendGflags(JNIEnv* jni_env, jbyteArray* cfg_bytes) {
   cfg.__set_local_library_path(FLAGS_local_library_dir);
   cfg.__set_kudu_operation_timeout_ms(FLAGS_kudu_operation_timeout_ms);
   cfg.__set_enable_partitioned_hash_join(FLAGS_enable_partitioned_hash_join);
+  cfg.__set_sentry_catalog_polling_frequency_s(FLAGS_sentry_catalog_polling_frequency_s);
   RETURN_IF_ERROR(SerializeThriftMsg(jni_env, &cfg, cfg_bytes));
   return Status::OK();
 }

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/322ccb0e/common/thrift/BackendGflags.thrift
----------------------------------------------------------------------
diff --git a/common/thrift/BackendGflags.thrift b/common/thrift/BackendGflags.thrift
index 3a410c2..b5f6838 100644
--- a/common/thrift/BackendGflags.thrift
+++ b/common/thrift/BackendGflags.thrift
@@ -31,8 +31,6 @@ struct TBackendGflags {
 
   5: required i64 inc_stats_size_limit_bytes
 
- 19: required bool enable_stats_extrapolation
-
   6: required string lineage_event_log_dir
 
   7: required bool load_catalog_in_background
@@ -58,4 +56,8 @@ struct TBackendGflags {
   17: required i32 initial_hms_cnxn_timeout_s
 
   18: required bool enable_partitioned_hash_join
+
+  19: required bool enable_stats_extrapolation
+
+  20: required i64 sentry_catalog_polling_frequency_s
 }

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/322ccb0e/fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java b/fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java
index 56fc2f7..624caad 100644
--- a/fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java
+++ b/fe/src/main/java/org/apache/impala/catalog/AuthorizationPolicy.java
@@ -328,12 +328,13 @@ public class AuthorizationPolicy implements PrivilegeCache {
         if (!privName.equalsIgnoreCase(privilege.getPrivilege_name())) continue;
       }
       TResultRowBuilder rowBuilder = new TResultRowBuilder();
-      rowBuilder.add(privilege.getScope().toString());
-      rowBuilder.add(Strings.nullToEmpty(privilege.getDb_name()));
-      rowBuilder.add(Strings.nullToEmpty(privilege.getTable_name()));
-      rowBuilder.add(Strings.nullToEmpty(privilege.getColumn_name()));
+      rowBuilder.add(privilege.getScope().toString().toLowerCase());
+      rowBuilder.add(Strings.nullToEmpty(privilege.getDb_name()).toLowerCase());
+      rowBuilder.add(Strings.nullToEmpty(privilege.getTable_name()).toLowerCase());
+      rowBuilder.add(Strings.nullToEmpty(privilege.getColumn_name()).toLowerCase());
+      // URIs are case sensitive
       rowBuilder.add(Strings.nullToEmpty(privilege.getUri()));
-      rowBuilder.add(privilege.getPrivilege_level().toString());
+      rowBuilder.add(privilege.getPrivilege_level().toString().toLowerCase());
       rowBuilder.add(Boolean.toString(privilege.isHas_grant_opt()));
       if (privilege.getCreate_time_ms() == -1) {
         rowBuilder.add(null);

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/322ccb0e/fe/src/main/java/org/apache/impala/catalog/RolePrivilege.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/catalog/RolePrivilege.java b/fe/src/main/java/org/apache/impala/catalog/RolePrivilege.java
index 6ce035f..87277af 100644
--- a/fe/src/main/java/org/apache/impala/catalog/RolePrivilege.java
+++ b/fe/src/main/java/org/apache/impala/catalog/RolePrivilege.java
@@ -66,30 +66,42 @@ public class RolePrivilege implements CatalogObject {
       Preconditions.checkNotNull(scope);
       switch (scope) {
         case SERVER: {
-          authorizable.add(KV_JOINER.join("server", privilege.getServer_name()));
+          authorizable.add(KV_JOINER.join("server", privilege.getServer_name().
+              toLowerCase()));
           break;
         }
         case URI: {
-          authorizable.add(KV_JOINER.join("server", privilege.getServer_name()));
+          authorizable.add(KV_JOINER.join("server", privilege.getServer_name().
+              toLowerCase()));
+          // (IMPALA-2695) URIs are case sensitive
           authorizable.add(KV_JOINER.join("uri", privilege.getUri()));
           break;
         }
         case DATABASE: {
-          authorizable.add(KV_JOINER.join("server", privilege.getServer_name()));
-          authorizable.add(KV_JOINER.join("db", privilege.getDb_name()));
+          authorizable.add(KV_JOINER.join("server", privilege.getServer_name().
+              toLowerCase()));
+          authorizable.add(KV_JOINER.join("db", privilege.getDb_name().
+              toLowerCase()));
           break;
         }
         case TABLE: {
-          authorizable.add(KV_JOINER.join("server", privilege.getServer_name()));
-          authorizable.add(KV_JOINER.join("db", privilege.getDb_name()));
-          authorizable.add(KV_JOINER.join("table", privilege.getTable_name()));
+          authorizable.add(KV_JOINER.join("server", privilege.getServer_name().
+              toLowerCase()));
+          authorizable.add(KV_JOINER.join("db", privilege.getDb_name().
+              toLowerCase()));
+          authorizable.add(KV_JOINER.join("table", privilege.getTable_name().
+              toLowerCase()));
           break;
         }
         case COLUMN: {
-          authorizable.add(KV_JOINER.join("server", privilege.getServer_name()));
-          authorizable.add(KV_JOINER.join("db", privilege.getDb_name()));
-          authorizable.add(KV_JOINER.join("table", privilege.getTable_name()));
-          authorizable.add(KV_JOINER.join("column", privilege.getColumn_name()));
+          authorizable.add(KV_JOINER.join("server", privilege.getServer_name().
+              toLowerCase()));
+          authorizable.add(KV_JOINER.join("db", privilege.getDb_name().
+              toLowerCase()));
+          authorizable.add(KV_JOINER.join("table", privilege.getTable_name().
+              toLowerCase()));
+          authorizable.add(KV_JOINER.join("column", privilege.getColumn_name().
+              toLowerCase()));
           break;
         }
         default: {

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/322ccb0e/fe/src/main/java/org/apache/impala/service/BackendConfig.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/service/BackendConfig.java b/fe/src/main/java/org/apache/impala/service/BackendConfig.java
index a1566c7..5b641e2 100644
--- a/fe/src/main/java/org/apache/impala/service/BackendConfig.java
+++ b/fe/src/main/java/org/apache/impala/service/BackendConfig.java
@@ -60,6 +60,9 @@ public class BackendConfig {
 
   public int getImpalaLogLevel() { return backendCfg_.impala_log_lvl; }
   public int getNonImpalaJavaVlogLevel() { return backendCfg_.non_impala_java_vlog; }
+  public long getSentryCatalogPollingFrequency() {
+    return backendCfg_.sentry_catalog_polling_frequency_s;
+  }
 
   public boolean isPartitionedHashJoinEnabled() {
     return backendCfg_.enable_partitioned_hash_join;

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/322ccb0e/fe/src/main/java/org/apache/impala/util/SentryProxy.java
----------------------------------------------------------------------
diff --git a/fe/src/main/java/org/apache/impala/util/SentryProxy.java b/fe/src/main/java/org/apache/impala/util/SentryProxy.java
index 07669ec..642a778 100644
--- a/fe/src/main/java/org/apache/impala/util/SentryProxy.java
+++ b/fe/src/main/java/org/apache/impala/util/SentryProxy.java
@@ -37,6 +37,7 @@ import org.apache.impala.catalog.Role;
 import org.apache.impala.catalog.RolePrivilege;
 import org.apache.impala.common.ImpalaException;
 import org.apache.impala.common.ImpalaRuntimeException;
+import org.apache.impala.service.BackendConfig;
 import org.apache.impala.thrift.TPrivilege;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Strings;
@@ -85,9 +86,9 @@ public class SentryProxy {
       processUser_ = new User(kerberosPrincipal);
     }
     sentryPolicyService_ = new SentryPolicyService(sentryConfig);
-    // Sentry Service is enabled.
-    // TODO: Make this configurable
-    policyReader_.scheduleAtFixedRate(new PolicyReader(), 0, 60,
+
+    policyReader_.scheduleAtFixedRate(new PolicyReader(), 0,
+        BackendConfig.INSTANCE.getSentryCatalogPollingFrequency(),
         TimeUnit.SECONDS);
   }
 

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/322ccb0e/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
----------------------------------------------------------------------
diff --git a/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test b/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
index 8beb04f..3f219c5 100644
--- a/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
+++ b/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
@@ -59,7 +59,7 @@ STRING
 ---- QUERY
 show grant role grant_revoke_test_ALL_SERVER
 ---- RESULTS
-'SERVER','','','','','ALL',FALSE,regex:.+
+'server','','','','','all',false,regex:.+
 ---- LABELS
 scope, database, table, column, uri, privilege, grant_option, create_time
 ---- TYPES
@@ -68,7 +68,7 @@ STRING, STRING, STRING, STRING, STRING, STRING, BOOLEAN, STRING
 ---- QUERY
 show grant role grant_revoke_test_ALL_SERVER on server
 ---- RESULTS
-'SERVER','','','','','ALL',FALSE,regex:.+
+'server','','','','','all',false,regex:.+
 ---- LABELS
 scope, database, table, column, uri, privilege, grant_option, create_time
 ---- TYPES
@@ -262,9 +262,9 @@ STRING, BIGINT, BIGINT, STRING, STRING, STRING, STRING, STRING, STRING
 ---- QUERY
 show grant role grant_revoke_test_ALL_URI
 ---- RESULTS
-'URI','','','','$NAMENODE/test-warehouse/grant_rev_test_tbl2','ALL',FALSE,regex:.+
-'URI','','','','$NAMENODE/test-warehouse/GRANT_REV_TEST_TBL3','ALL',FALSE,regex:.+
-'URI','','','','$NAMENODE/test-warehouse/grant_rev_test_prt','ALL',FALSE,regex:.+
+'uri','','','','$NAMENODE/test-warehouse/grant_rev_test_tbl2','all',false,regex:.+
+'uri','','','','$NAMENODE/test-warehouse/GRANT_REV_TEST_TBL3','all',false,regex:.+
+'uri','','','','$NAMENODE/test-warehouse/grant_rev_test_prt','all',false,regex:.+
 ---- LABELS
 scope, database, table, column, uri, privilege, grant_option, create_time
 ---- TYPES
@@ -313,8 +313,8 @@ GRANT INSERT ON TABLE grant_rev_db.test_tbl1 TO grant_revoke_test_SELECT_INSERT_
 ---- QUERY
 show grant role grant_revoke_test_SELECT_INSERT_TEST_TBL on table grant_rev_db.test_tbl1
 ---- RESULTS
-'TABLE','grant_rev_db','test_tbl1','','','SELECT',FALSE,regex:.+
-'TABLE','grant_rev_db','test_tbl1','','','INSERT',FALSE,regex:.+
+'table','grant_rev_db','test_tbl1','','','select',false,regex:.+
+'table','grant_rev_db','test_tbl1','','','insert',false,regex:.+
 ---- LABELS
 scope, database, table, column, uri, privilege, grant_option, create_time
 ---- TYPES
@@ -415,8 +415,8 @@ root
 ---- QUERY
 show grant role grant_revoke_test_ROOT
 ---- RESULTS
-'DATABASE','functional','','','','ALL',TRUE,regex:.+
-'TABLE','functional','alltypes','','','ALL',FALSE,regex:.+
+'database','functional','','','','all',true,regex:.+
+'table','functional','alltypes','','','all',false,regex:.+
 ---- LABELS
 scope, database, table, column, uri, privilege, grant_option, create_time
 ---- TYPES
@@ -477,7 +477,7 @@ STRING,STRING
 # Privilege still exists, but grant option is set to false
 show grant role grant_revoke_test_ROOT
 ---- RESULTS
-'DATABASE','functional','','','','ALL',FALSE,regex:.+
+'database','functional','','','','all',false,regex:.+
 ---- LABELS
 scope, database, table, column, uri, privilege, grant_option, create_time
 ---- TYPES
@@ -513,10 +513,10 @@ GRANT SELECT (a, b, x) ON TABLE grant_rev_db.test_tbl3 TO grant_revoke_test_ALL_
 ---- QUERY
 show grant role grant_revoke_test_ALL_SERVER
 ---- RESULTS: VERIFY_IS_EQUAL_SORTED
-'SERVER','','','','','ALL',FALSE,regex:.+
-'COLUMN','grant_rev_db','test_tbl3','a','','SELECT',FALSE,regex:.+
-'COLUMN','grant_rev_db','test_tbl3','b','','SELECT',FALSE,regex:.+
-'COLUMN','grant_rev_db','test_tbl3','x','','SELECT',FALSE,regex:.+
+'server','','','','','all',false,regex:.+
+'column','grant_rev_db','test_tbl3','a','','select',false,regex:.+
+'column','grant_rev_db','test_tbl3','b','','select',false,regex:.+
+'column','grant_rev_db','test_tbl3','x','','select',false,regex:.+
 ---- LABELS
 scope, database, table, column, uri, privilege, grant_option, create_time
 ---- TYPES
@@ -529,13 +529,13 @@ GRANT SELECT (c, d, y) ON TABLE grant_rev_db.test_tbl3 TO grant_revoke_test_ALL_
 ---- QUERY
 show grant role grant_revoke_test_ALL_SERVER
 ---- RESULTS: VERIFY_IS_EQUAL_SORTED
-'SERVER','','','','','ALL',FALSE,regex:.+
-'COLUMN','grant_rev_db','test_tbl3','a','','SELECT',FALSE,regex:.+
-'COLUMN','grant_rev_db','test_tbl3','b','','SELECT',FALSE,regex:.+
-'COLUMN','grant_rev_db','test_tbl3','c','','SELECT',FALSE,regex:.+
-'COLUMN','grant_rev_db','test_tbl3','d','','SELECT',FALSE,regex:.+
-'COLUMN','grant_rev_db','test_tbl3','x','','SELECT',FALSE,regex:.+
-'COLUMN','grant_rev_db','test_tbl3','y','','SELECT',FALSE,regex:.+
+'server','','','','','all',false,regex:.+
+'column','grant_rev_db','test_tbl3','a','','select',false,regex:.+
+'column','grant_rev_db','test_tbl3','b','','select',false,regex:.+
+'column','grant_rev_db','test_tbl3','c','','select',false,regex:.+
+'column','grant_rev_db','test_tbl3','d','','select',false,regex:.+
+'column','grant_rev_db','test_tbl3','x','','select',false,regex:.+
+'column','grant_rev_db','test_tbl3','y','','select',false,regex:.+
 ---- LABELS
 scope, database, table, column, uri, privilege, grant_option, create_time
 ---- TYPES
@@ -548,14 +548,14 @@ GRANT SELECT (a, a, e, x) ON TABLE grant_rev_db.test_tbl3 TO grant_revoke_test_A
 ---- QUERY
 show grant role grant_revoke_test_ALL_SERVER
 ---- RESULTS: VERIFY_IS_EQUAL_SORTED
-'SERVER','','','','','ALL',FALSE,regex:.+
-'COLUMN','grant_rev_db','test_tbl3','a','','SELECT',FALSE,regex:.+
-'COLUMN','grant_rev_db','test_tbl3','b','','SELECT',FALSE,regex:.+
-'COLUMN','grant_rev_db','test_tbl3','c','','SELECT',FALSE,regex:.+
-'COLUMN','grant_rev_db','test_tbl3','d','','SELECT',FALSE,regex:.+
-'COLUMN','grant_rev_db','test_tbl3','e','','SELECT',FALSE,regex:.+
-'COLUMN','grant_rev_db','test_tbl3','x','','SELECT',FALSE,regex:.+
-'COLUMN','grant_rev_db','test_tbl3','y','','SELECT',FALSE,regex:.+
+'server','','','','','all',false,regex:.+
+'column','grant_rev_db','test_tbl3','a','','select',false,regex:.+
+'column','grant_rev_db','test_tbl3','b','','select',false,regex:.+
+'column','grant_rev_db','test_tbl3','c','','select',false,regex:.+
+'column','grant_rev_db','test_tbl3','d','','select',false,regex:.+
+'column','grant_rev_db','test_tbl3','e','','select',false,regex:.+
+'column','grant_rev_db','test_tbl3','x','','select',false,regex:.+
+'column','grant_rev_db','test_tbl3','y','','select',false,regex:.+
 ---- LABELS
 scope, database, table, column, uri, privilege, grant_option, create_time
 ---- TYPES
@@ -569,11 +569,11 @@ REVOKE SELECT (a, b, b, y) ON TABLE grant_rev_db.test_tbl3 FROM grant_revoke_tes
 ---- QUERY
 show grant role grant_revoke_test_ALL_SERVER
 ---- RESULTS: VERIFY_IS_EQUAL_SORTED
-'SERVER','','','','','ALL',FALSE,regex:.+
-'COLUMN','grant_rev_db','test_tbl3','c','','SELECT',FALSE,regex:.+
-'COLUMN','grant_rev_db','test_tbl3','d','','SELECT',FALSE,regex:.+
-'COLUMN','grant_rev_db','test_tbl3','e','','SELECT',FALSE,regex:.+
-'COLUMN','grant_rev_db','test_tbl3','x','','SELECT',FALSE,regex:.+
+'server','','','','','all',false,regex:.+
+'column','grant_rev_db','test_tbl3','c','','select',false,regex:.+
+'column','grant_rev_db','test_tbl3','d','','select',false,regex:.+
+'column','grant_rev_db','test_tbl3','e','','select',false,regex:.+
+'column','grant_rev_db','test_tbl3','x','','select',false,regex:.+
 ---- LABELS
 scope, database, table, column, uri, privilege, grant_option, create_time
 ---- TYPES
@@ -586,9 +586,9 @@ REVOKE SELECT (a, b, c, x) ON TABLE grant_rev_db.test_tbl3 FROM grant_revoke_tes
 ---- QUERY
 show grant role grant_revoke_test_ALL_SERVER
 ---- RESULTS: VERIFY_IS_EQUAL_SORTED
-'SERVER','','','','','ALL',FALSE,regex:.+
-'COLUMN','grant_rev_db','test_tbl3','d','','SELECT',FALSE,regex:.+
-'COLUMN','grant_rev_db','test_tbl3','e','','SELECT',FALSE,regex:.+
+'server','','','','','all',false,regex:.+
+'column','grant_rev_db','test_tbl3','d','','select',false,regex:.+
+'column','grant_rev_db','test_tbl3','e','','select',false,regex:.+
 ---- LABELS
 scope, database, table, column, uri, privilege, grant_option, create_time
 ---- TYPES
@@ -601,7 +601,7 @@ REVOKE SELECT (a, b, c, d, e) ON TABLE grant_rev_db.test_tbl3 FROM grant_revoke_
 ---- QUERY
 show grant role grant_revoke_test_ALL_SERVER
 ---- RESULTS: VERIFY_IS_EQUAL_SORTED
-'SERVER','','','','','ALL',FALSE,regex:.+
+'server','','','','','all',false,regex:.+
 ---- LABELS
 scope, database, table, column, uri, privilege, grant_option, create_time
 ---- TYPES
@@ -619,7 +619,7 @@ root
 ---- QUERY
 show grant role grant_revoke_test_ROOT
 ---- RESULTS: VERIFY_IS_EQUAL_SORTED
-'TABLE','grant_rev_db','test_tbl3','','','SELECT',FALSE,regex:.+
+'table','grant_rev_db','test_tbl3','','','select',false,regex:.+
 ---- LABELS
 scope, database, table, column, uri, privilege, grant_option, create_time
 ---- TYPES
@@ -652,8 +652,8 @@ root
 ---- QUERY
 show grant role grant_revoke_test_ROOT
 ---- RESULTS: VERIFY_IS_EQUAL_SORTED
-'TABLE','grant_rev_db','test_tbl3','','','SELECT',TRUE,regex:.+
-'COLUMN','grant_rev_db','test_tbl3','a','','SELECT',FALSE,regex:.+
+'table','grant_rev_db','test_tbl3','','','select',true,regex:.+
+'column','grant_rev_db','test_tbl3','a','','select',false,regex:.+
 ---- LABELS
 scope, database, table, column, uri, privilege, grant_option, create_time
 ---- TYPES
@@ -666,10 +666,10 @@ GRANT SELECT (a, c, e) ON TABLE grant_rev_db.test_tbl3 TO grant_revoke_test_ALL_
 ---- QUERY
 show grant role grant_revoke_test_ALL_SERVER
 ---- RESULTS: VERIFY_IS_EQUAL_SORTED
-'SERVER','','','','','ALL',FALSE,regex:.+
-'COLUMN','grant_rev_db','test_tbl3','a','','SELECT',TRUE,regex:.+
-'COLUMN','grant_rev_db','test_tbl3','c','','SELECT',TRUE,regex:.+
-'COLUMN','grant_rev_db','test_tbl3','e','','SELECT',TRUE,regex:.+
+'server','','','','','all',false,regex:.+
+'column','grant_rev_db','test_tbl3','a','','select',true,regex:.+
+'column','grant_rev_db','test_tbl3','c','','select',true,regex:.+
+'column','grant_rev_db','test_tbl3','e','','select',true,regex:.+
 ---- LABELS
 scope, database, table, column, uri, privilege, grant_option, create_time
 ---- TYPES
@@ -683,10 +683,10 @@ REVOKE GRANT OPTION FOR SELECT (a, c) ON TABLE grant_rev_db.test_tbl3
FROM grant
 # TODO: Add a test case that exercises the cascading effect of REVOKE ALL.
 show grant role grant_revoke_test_ALL_SERVER
 ---- RESULTS: VERIFY_IS_EQUAL_SORTED
-'SERVER','','','','','ALL',FALSE,regex:.+
-'COLUMN','grant_rev_db','test_tbl3','a','','SELECT',FALSE,regex:.+
-'COLUMN','grant_rev_db','test_tbl3','c','','SELECT',FALSE,regex:.+
-'COLUMN','grant_rev_db','test_tbl3','e','','SELECT',TRUE,regex:.+
+'server','','','','','all',false,regex:.+
+'column','grant_rev_db','test_tbl3','a','','select',false,regex:.+
+'column','grant_rev_db','test_tbl3','c','','select',false,regex:.+
+'column','grant_rev_db','test_tbl3','e','','select',true,regex:.+
 ---- LABELS
 scope, database, table, column, uri, privilege, grant_option, create_time
 ---- TYPES
@@ -723,7 +723,7 @@ does not have privileges to execute 'CREATE' on: grant_rev_db
 ---- QUERY
 show grant role grant_revoke_test_ALL_SERVER1
 ---- RESULTS: VERIFY_IS_EQUAL_SORTED
-'SERVER','','','','','ALL',FALSE,regex:.+
+'server','','','','','all',false,regex:.+
 ---- LABELS
 scope, database, table, column, uri, privilege, grant_option, create_time
 ---- TYPES

http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/322ccb0e/tests/authorization/test_grant_revoke.py
----------------------------------------------------------------------
diff --git a/tests/authorization/test_grant_revoke.py b/tests/authorization/test_grant_revoke.py
index 987aecb..9430350 100644
--- a/tests/authorization/test_grant_revoke.py
+++ b/tests/authorization/test_grant_revoke.py
@@ -26,6 +26,7 @@ from time import sleep
 from tests.common.custom_cluster_test_suite import CustomClusterTestSuite
 from tests.common.impala_test_suite import ImpalaTestSuite
 from tests.common.test_dimensions import create_uncompressed_text_dimension
+from tests.util.calculation_util import get_random_id
 from tests.verifiers.metric_verifier import MetricVerifier
 
 SENTRY_CONFIG_FILE = getenv('IMPALA_HOME') + '/fe/src/test/resources/sentry-site.xml'
@@ -98,6 +99,72 @@ class TestGrantRevoke(CustomClusterTestSuite, ImpalaTestSuite):
   @pytest.mark.execute_serially
   @CustomClusterTestSuite.with_args(
       impalad_args="--server_name=server1",
+      catalogd_args="--sentry_config=" + SENTRY_CONFIG_FILE +
+                    " --sentry_catalog_polling_frequency_s=1",
+      statestored_args="--statestore_update_frequency_ms=300")
+  def test_role_privilege_case(self, vector):
+    """IMPALA-5582: Store sentry privileges in lower case. This
+    test grants select privileges to roles assgined to tables/db
+    specified in lower, upper and mix cases. This test verifies
+    that these privileges do not vanish on a sentryProxy thread
+    update.
+    """
+    db_name = "test_role_privilege_case_x_" + get_random_id(5)
+    db_name_upper_case = "TEST_ROLE_PRIVILEGE_CASE_Y_" + get_random_id(5).upper()
+    db_name_mixed_case = "TesT_Role_PRIVIlege_case_z" + get_random_id(5)
+    role_name = "test_role_" + get_random_id(5)
+    try:
+      self.client.execute("create role {0}".format(role_name))
+      self.client.execute("grant all on server to {0}".format(role_name))
+      self.client.execute(
+          "grant role {0} to group {1}".format(
+           role_name, grp.getgrnam(getuser()).gr_name))
+
+      self.client.execute("create database " + db_name)
+      self.client.execute("create database " + db_name_upper_case)
+      self.client.execute("create database " + db_name_mixed_case)
+      self.client.execute(
+          "create table if not exists {0}.test1(i int)".format(db_name))
+      self.client.execute("create table if not exists {0}.TEST2(i int)".format(db_name))
+      self.client.execute("create table if not exists {0}.Test3(i int)".format(db_name))
+
+      self.client.execute(
+          "grant select on table {0}.test1 to {1}".format(db_name, role_name))
+      self.client.execute(
+          "grant select on table {0}.TEST2 to {1}".format(db_name, role_name))
+      self.client.execute(
+          "grant select on table {0}.TesT3 to {1}".format(db_name, role_name))
+      self.client.execute("grant all on database {0} to {1}".format(db_name, role_name))
+      self.client.execute(
+          "grant all on database {0} to {1}".format(db_name_upper_case, role_name))
+      self.client.execute(
+          "grant all on database {0} to {1}".format(db_name_mixed_case, role_name))
+      result = self.client.execute("show grant role {0}".format(role_name))
+      assert any('test1' in x for x in result.data)
+      assert any('test2' in x for x in result.data)
+      assert any('test3' in x for x in result.data)
+      assert any(db_name_upper_case.lower() in x for x in result.data)
+      assert any(db_name_mixed_case.lower() in x for x in result.data)
+      # Sleep for 2 seconds and make sure that the privileges
+      # on all 3 tables still persist on a sentryProxy thread
+      # update. sentry_catalog_polling_frequency_s is set to 1
+      # seconds.
+      sleep(2)
+      result = self.client.execute("show grant role {0}".format(role_name))
+      assert any('test1' in x for x in result.data)
+      assert any('test2' in x for x in result.data)
+      assert any('test3' in x for x in result.data)
+      assert any(db_name_upper_case.lower() in x for x in result.data)
+      assert any(db_name_mixed_case.lower() in x for x in result.data)
+    finally:
+      self.client.execute("drop database if exists {0}".format(db_name_upper_case))
+      self.client.execute("drop database if exists {0}".format(db_name_mixed_case))
+      self.client.execute("drop database if exists {0} cascade".format(db_name))
+      self.client.execute("drop role {0}".format(role_name))
+
+  @pytest.mark.execute_serially
+  @CustomClusterTestSuite.with_args(
+      impalad_args="--server_name=server1",
       catalogd_args="--sentry_config=" + SENTRY_CONFIG_FILE,
       statestored_args=("--statestore_heartbeat_frequency_ms=300 "
                         "--statestore_update_frequency_ms=300"))
@@ -106,25 +173,29 @@ class TestGrantRevoke(CustomClusterTestSuite, ImpalaTestSuite):
     reverse order if a role was modified, but not the associated privilege. Verify that
     Impala is able to handle this.
     """
-    self.client.execute("create role test_role")
-    self.client.execute("grant all on server to test_role")
-    # Wait a few seconds to make sure the update propagates to the statestore.
-    sleep(3)
-    # Update the role, increasing its catalog verion.
-    self.client.execute("grant role test_role to group {0}".format(
-        grp.getgrnam(getuser()).gr_name))
-    result = self.client.execute("show tables in functional")
-    assert 'alltypes' in result.data
-    privileges_before = self.client.execute("show grant role test_role")
-    # Wait a few seconds before restarting Impalad to make sure that the Catalog gets
-    # updated.
-    sleep(3)
-    self.restart_first_impalad()
-    verifier = MetricVerifier(self.cluster.impalads[0].service)
-    verifier.wait_for_metric("catalog.ready", True)
-    # Verify that we still have the right privileges after the first impalad was
-    # restarted.
-    result = self.client.execute("show tables in functional");
-    assert 'alltypes' in result.data
-    privileges_after = self.client.execute("show grant role test_role")
-    assert privileges_before.data == privileges_after.data
+    role_name = "test_role_" + get_random_id(5)
+    try:
+      self.client.execute("create role {0}".format(role_name))
+      self.client.execute("grant all on server to {0}".format(role_name))
+      # Wait a few seconds to make sure the update propagates to the statestore.
+      sleep(3)
+      # Update the role, increasing its catalog verion.
+      self.client.execute("grant role {0} to group {1}".format(
+          role_name, grp.getgrnam(getuser()).gr_name))
+      result = self.client.execute("show tables in functional")
+      assert 'alltypes' in result.data
+      privileges_before = self.client.execute("show grant role {0}".format(role_name))
+      # Wait a few seconds before restarting Impalad to make sure that the Catalog gets
+      # updated.
+      sleep(3)
+      self.restart_first_impalad()
+      verifier = MetricVerifier(self.cluster.impalads[0].service)
+      verifier.wait_for_metric("catalog.ready", True)
+      # Verify that we still have the right privileges after the first impalad was
+      # restarted.
+      result = self.client.execute("show tables in functional")
+      assert 'alltypes' in result.data
+      privileges_after = self.client.execute("show grant role {0}".format(role_name))
+      assert privileges_before.data == privileges_after.data
+    finally:
+      self.client.execute("drop role {0}".format(role_name))


Mime
View raw message