You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by ta...@apache.org on 2018/10/27 05:19:00 UTC

[2/5] impala git commit: IMPALA-7710: test_owner_privileges_with_grant failed with AuthorizationException

IMPALA-7710: test_owner_privileges_with_grant failed with AuthorizationException

The problem was acache consistency issue between impalad and catalogd.
Because a Sentry refresh was occuring during an update to privileges
from the alter table set owner, impalad had the correct privileges,
which allowed the "show grant role" to succeed but the privileges in
catalogd were being overwritten from the sentry refresh. Added a delay
in the drop call to ensure privileges are updated. This is a
workaround to get the tests to pass with the existing behaviour and
should be reassessed if IMPALA-7763 is implemented.  This would add a
lock to possibly prevent this, but will need it's own assessment.

Testing:
- Ran custom cluster tests 50 times

Change-Id: I5a1babd3dcbb94ffaa1f3e6ef2cebf1a1d391219
Reviewed-on: http://gerrit.cloudera.org:8080/11786
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>


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

Branch: refs/heads/master
Commit: 8d628d7b62c4903bfd0236d2de150dae0752f0fe
Parents: f8b7f25
Author: Adam Holley <gi...@holleyism.com>
Authored: Thu Oct 18 06:43:29 2018 -0500
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Fri Oct 26 20:54:45 2018 +0000

----------------------------------------------------------------------
 tests/authorization/test_owner_privileges.py | 37 +++++++++++++----------
 1 file changed, 21 insertions(+), 16 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/8d628d7b/tests/authorization/test_owner_privileges.py
----------------------------------------------------------------------
diff --git a/tests/authorization/test_owner_privileges.py b/tests/authorization/test_owner_privileges.py
index 4cc2193..6520cd0 100644
--- a/tests/authorization/test_owner_privileges.py
+++ b/tests/authorization/test_owner_privileges.py
@@ -38,8 +38,6 @@ SENTRY_LONG_POLLING_FREQUENCY_S = 60
 SENTRY_POLLING_FREQUENCY_S = 1
 # The timeout, in seconds, when waiting for a refresh of Sentry privileges.
 SENTRY_REFRESH_TIMEOUT_S = SENTRY_POLLING_FREQUENCY_S * 2
-# The timeout needed because of statestore refresh.
-STATESTORE_TIMEOUT_S = 3
 
 SENTRY_CONFIG_DIR = getenv('IMPALA_HOME') + '/fe/src/test/resources/'
 SENTRY_BASE_LOG_DIR = getenv('IMPALA_CLUSTER_LOGS_DIR') + "/sentry"
@@ -93,13 +91,8 @@ class TestOwnerPrivileges(SentryCacheTestSuite):
     return total
 
   def _validate_user_privilege_count(self, client, query, user, delay_s, count):
-    start_time = time()
-    while time() - start_time < STATESTORE_TIMEOUT_S:
-      result = self.user_query(client, query, user=user, delay_s=delay_s)
-      if self.count_user_privileges(result) == count:
-        return True
-      sleep(1)
-    return False
+    result = self.user_query(client, query, user=user, delay_s=delay_s)
+    return self.count_user_privileges(result) == count
 
   def _test_cleanup(self):
     # Admin for manipulation and cleaning up.
@@ -191,14 +184,17 @@ class TestOwnerPrivileges(SentryCacheTestSuite):
         % (test_obj.grant_name, test_obj.obj_name), user="oo_user1")
 
     # Change the database owner and ensure oo_user1 does not have owner privileges.
+    # Use a delay to avoid cache consistency issue that could occur after create.
     self.user_query(self.oo_user1_impalad_client, "alter %s %s set owner user oo_user2"
-        % (test_obj.obj_type, test_obj.obj_name), user="oo_user1")
+        % (test_obj.obj_type, test_obj.obj_name), user="oo_user1",
+        delay_s=sentry_refresh_timeout_s)
     assert self._validate_user_privilege_count(self.oo_user1_impalad_client,
         "show grant user oo_user1", "oo_user1", sentry_refresh_timeout_s, 0)
 
     # Ensure oo_user1 cannot drop database after owner change.
+    # Use a delay to avoid cache consistency issue that could occur after alter.
     self.user_query(self.oo_user1_impalad_client, "drop %s %s" % (test_obj.obj_type,
-        test_obj.obj_name), user="oo_user1",
+        test_obj.obj_name), user="oo_user1", delay_s=sentry_refresh_timeout_s,
         error_msg="does not have privileges to execute 'DROP'")
 
     # oo_user2 should have privileges for object now.
@@ -212,9 +208,11 @@ class TestOwnerPrivileges(SentryCacheTestSuite):
         test_obj.obj_name))
     assert self._validate_user_privilege_count(self.oo_user2_impalad_client,
         "show grant user oo_user2", "oo_user2", sentry_refresh_timeout_s, 0)
+    # Use a delay to avoid cache consistency issue that could occur after alter.
     self.user_query(self.oo_user1_impalad_client,
         "alter %s %s set owner role owner_priv_test_owner_role"
-        % (test_obj.obj_type, test_obj.obj_name), user="oo_user1")
+        % (test_obj.obj_type, test_obj.obj_name), user="oo_user1",
+        delay_s=sentry_refresh_timeout_s)
     # Ensure oo_user1 does not have user privileges.
     assert self._validate_user_privilege_count(self.oo_user1_impalad_client,
         "show grant user oo_user1", "oo_user1", sentry_refresh_timeout_s, 0)
@@ -225,17 +223,20 @@ class TestOwnerPrivileges(SentryCacheTestSuite):
         "oo_user1")
 
     # Drop the object and ensure no role privileges.
+    # Use a delay to avoid cache consistency issue that could occur after alter.
     self.user_query(self.oo_user1_impalad_client, "drop %s %s " % (test_obj.obj_type,
-        test_obj.obj_name), user="oo_user1")
+        test_obj.obj_name), user="oo_user1", delay_s=sentry_refresh_timeout_s)
     assert self._validate_user_privilege_count(self.oo_user1_impalad_client,
         "show grant user oo_user1", "oo_user1", sentry_refresh_timeout_s, 0)
 
     # Ensure user privileges are gone after drop.
+    # Use a delay to avoid cache consistency issue that could occur after drop.
     self.user_query(self.oo_user1_impalad_client, "create %s if not exists %s %s %s"
         % (test_obj.obj_type, test_obj.obj_name, test_obj.table_def,
-        test_obj.view_select), user="oo_user1")
+        test_obj.view_select), user="oo_user1", delay_s=sentry_refresh_timeout_s)
+    # Use a delay to avoid cache consistency issue that could occur after create.
     self.user_query(self.oo_user1_impalad_client, "drop %s %s " % (test_obj.obj_type,
-        test_obj.obj_name), user="oo_user1")
+        test_obj.obj_name), user="oo_user1", delay_s=sentry_refresh_timeout_s)
     assert self._validate_user_privilege_count(self.oo_user1_impalad_client,
         "show grant user oo_user1", "oo_user1", sentry_refresh_timeout_s, 0)
 
@@ -312,6 +313,7 @@ class TestOwnerPrivileges(SentryCacheTestSuite):
         error_msg="does not have privileges with 'GRANT OPTION'")
 
     # Ensure oo_user1 cannot drop database.
+    # Use a delay to avoid cache consistency issue that could occur after alter.
     self.user_query(self.oo_user1_impalad_client, "drop %s %s" % (test_obj.obj_type,
         test_obj.obj_name), user="oo_user1",
         error_msg="does not have privileges to execute 'DROP'",
@@ -388,11 +390,14 @@ class TestOwnerPrivileges(SentryCacheTestSuite):
         % (test_obj.grant_name, test_obj.obj_name), user="oo_user1",
         error_msg="does not have privileges to execute: REVOKE_PRIVILEGE")
 
+    # Use a delay to avoid cache consistency issue that could occur after create.
     self.user_query(self.oo_user1_impalad_client, "alter %s %s set owner user oo_user2"
         % (test_obj.obj_type, test_obj.obj_name), user="oo_user1",
+        delay_s=sentry_refresh_timeout_s,
         error_msg="does not have privileges with 'GRANT OPTION'")
 
+    # Use a delay to avoid cache consistency issue that could occur after alter.
     self.user_query(self.oo_user1_impalad_client, "drop %s %s " % (test_obj.obj_type,
-        test_obj.obj_name), user="oo_user1")
+        test_obj.obj_name), user="oo_user1", delay_s=sentry_refresh_timeout_s)
     assert self._validate_user_privilege_count(self.oo_user1_impalad_client,
         "show grant user oo_user1", "oo_user1", sentry_refresh_timeout_s, 0)