You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by tm...@apache.org on 2018/10/05 21:39:19 UTC
[7/8] impala git commit: IMPALA-7633: count_user_privilege isn't 0 at
the end of test_owner
IMPALA-7633: count_user_privilege isn't 0 at the end of test_owner
This patch adds a retry loop to validate the count of user privileges
in a SHOW GRANT USER statement after a DDL operation. The core of the
problem is cache consistency. When a DDL operation is executing, like
drop database, HMS is updated with the correct metadata, and Sentry is
updated to remove privileges from HMS. However, if a Sentry Refresh
happens between when HMS is updated CatalogOpExecutor:1322, and when
the local catalog privileges are updated CatalogOpExecutor:1341, then
the remove privilege call will fail and a log entry with "User does
not exist: foo_user" will be written to the log. The result is that
the response back to impalad with catalog updates will not contain
the user and privilege updates. Ultimately, when the "SHOW GRANT USER"
statement is run, it uses the local Impalad catalog which still
contains the privlege because it has not yet been updated from
statestore. This is not a security problem because the privilege
exists for a maximum of 2s by default, for an object that does not
exist. This is the same result as if the database was dropped from
Hive, except in that case it can be up to 62s by default that the
privilege exists for no object.
Testing:
- After retry was added, ran tests until log entry appeared and
validate test did not fail.
Change-Id: Ifbba0fbd0e24a24b3f2af82ad5209f3fb7fb387b
Reviewed-on: http://gerrit.cloudera.org:8080/11595
Reviewed-by: Fredy Wijaya <fw...@cloudera.com>
Reviewed-by: Vuk Ercegovac <ve...@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/37bee3ae
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/37bee3ae
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/37bee3ae
Branch: refs/heads/master
Commit: 37bee3aeca036f3eb96f55425627f02af78a1279
Parents: 23428dc
Author: Adam Holley <gi...@holleyism.com>
Authored: Fri Oct 5 08:53:33 2018 -0500
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Fri Oct 5 20:43:05 2018 +0000
----------------------------------------------------------------------
tests/authorization/test_owner_privileges.py | 41 +++++++++++++----------
1 file changed, 23 insertions(+), 18 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/impala/blob/37bee3ae/tests/authorization/test_owner_privileges.py
----------------------------------------------------------------------
diff --git a/tests/authorization/test_owner_privileges.py b/tests/authorization/test_owner_privileges.py
index ac229ad..a44e3e8 100644
--- a/tests/authorization/test_owner_privileges.py
+++ b/tests/authorization/test_owner_privileges.py
@@ -38,6 +38,8 @@ 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"
@@ -90,6 +92,15 @@ class TestOwnerPrivileges(SentryCacheTestSuite):
total += 1
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
+
def _test_cleanup(self):
# Admin for manipulation and cleaning up.
try:
@@ -178,9 +189,8 @@ class TestOwnerPrivileges(SentryCacheTestSuite):
# Change the database owner and ensure oo_user1 does not have owner privileges.
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")
- result = self.user_query(self.oo_user1_impalad_client, "show grant user oo_user1",
- user="oo_user1", delay_s=sentry_refresh_timeout_s)
- assert self.count_user_privileges(result) == 0
+ 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.
self.user_query(self.oo_user1_impalad_client, "drop %s %s" % (test_obj.obj_type,
@@ -196,16 +206,14 @@ class TestOwnerPrivileges(SentryCacheTestSuite):
# privileges on the underlying table.
self.execute_query("alter %s %s set owner user oo_user1" % (test_obj.obj_type,
test_obj.obj_name))
- result = self.user_query(self.oo_user2_impalad_client,
- "show grant user oo_user2", user="oo_user2", delay_s=sentry_refresh_timeout_s)
- assert self.count_user_privileges(result) == 0
+ assert self._validate_user_privilege_count(self.oo_user2_impalad_client,
+ "show grant user oo_user2", "oo_user2", sentry_refresh_timeout_s, 0)
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")
# Ensure oo_user1 does not have user privileges.
- result = self.user_query(self.oo_user1_impalad_client, "show grant user oo_user1",
- user="oo_user1", delay_s=sentry_refresh_timeout_s)
- assert self.count_user_privileges(result) == 0
+ assert self._validate_user_privilege_count(self.oo_user1_impalad_client,
+ "show grant user oo_user1", "oo_user1", sentry_refresh_timeout_s, 0)
# Ensure role has owner privileges.
self.validate_privileges(self.oo_user1_impalad_client,
@@ -215,9 +223,8 @@ class TestOwnerPrivileges(SentryCacheTestSuite):
# Drop the object and ensure no role privileges.
self.user_query(self.oo_user1_impalad_client, "drop %s %s " % (test_obj.obj_type,
test_obj.obj_name), user="oo_user1")
- result = self.user_query(self.oo_user1_impalad_client, "show grant role " +
- "owner_priv_test_owner_role", user="oo_user1", delay_s=sentry_refresh_timeout_s)
- assert self.count_user_privileges(result) == 0
+ 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.
self.user_query(self.oo_user1_impalad_client, "create %s if not exists %s %s %s"
@@ -225,9 +232,8 @@ class TestOwnerPrivileges(SentryCacheTestSuite):
test_obj.view_select), user="oo_user1")
self.user_query(self.oo_user1_impalad_client, "drop %s %s " % (test_obj.obj_type,
test_obj.obj_name), user="oo_user1")
- result = self.user_query(self.oo_user1_impalad_client, "show grant user oo_user1",
- user="oo_user1")
- assert self.count_user_privileges(result) == 0
+ assert self._validate_user_privilege_count(self.oo_user1_impalad_client,
+ "show grant user oo_user1", "oo_user1", sentry_refresh_timeout_s, 0)
@pytest.mark.execute_serially
@SentryCacheTestSuite.with_args(
@@ -373,6 +379,5 @@ class TestOwnerPrivileges(SentryCacheTestSuite):
self.user_query(self.oo_user1_impalad_client, "drop %s %s " % (test_obj.obj_type,
test_obj.obj_name), user="oo_user1")
- result = self.user_query(self.oo_user1_impalad_client, "show grant user oo_user1",
- user="oo_user1")
- assert self.count_user_privileges(result) == 0
+ assert self._validate_user_privilege_count(self.oo_user1_impalad_client,
+ "show grant user oo_user1", "oo_user1", sentry_refresh_timeout_s, 0)