IMPALA-7537: REVOKE GRANT OPTION regression This patch fixes several issues around granting and revoking of privileges. This includes: - REVOKE ALL ON SERVER where the privilege has the grant option was removing from the cache but not Sentry. - With the addition of the grantoption to the name in the catalog object, refactoring was required to make grants and revokes work correctly.
Assertions with regard to granting and revoking: - If there is a privilege that has the grant option, that privilege can be revoked simply with "REVOKE privilege..." or the grant option can be removed with "REVOKE GRANT OPTION ON..." - We should not limit the privilege being revoked simply because it has the grant option. - If a privilege already exists without the grant option, granting the privilege with the grant option should add the grant option to it. - If a privilege already exists with the grant option, granting the privilege without the grant option will not change anything as the expectation is if you want to remove the grant option, you should explicitly use the "REVOKE GRANT OPTION ON...". Testing: - Added new grant/revoke tests that validate cache and Sentry refresh - Ran all FE, E2E, and custom-cluster tests. Change-Id: I3be5c8f15e9bc53e9661347578832bf446abaedc Reviewed-on: http://gerrit.cloudera.org:8080/11483 Reviewed-by: Fredy Wijaya <fwij...@cloudera.com> Tested-by: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Project: http://git-wip-us.apache.org/repos/asf/impala/repo Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/c5dc6ded Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/c5dc6ded Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/c5dc6ded Branch: refs/heads/master Commit: c5dc6ded68c62f9f2138ab3376531c6292d1df78 Parents: 17bc980 Author: Adam Holley <g...@holleyism.com> Authored: Mon Sep 17 18:05:23 2018 -0500 Committer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Committed: Tue Sep 25 22:21:57 2018 +0000 ---------------------------------------------------------------------- .../impala/service/CatalogOpExecutor.java | 15 +- .../apache/impala/util/SentryPolicyService.java | 3 +- .../org/apache/impala/util/SentryProxy.java | 97 ++++++- .../impala/analysis/AuthorizationStmtTest.java | 6 +- tests/authorization/test_grant_revoke.py | 143 +++++++++- tests/authorization/test_owner_privileges.py | 276 +++++++------------ tests/common/custom_cluster_test_suite.py | 7 +- tests/common/sentry_cache_test_suite.py | 134 +++++++++ 8 files changed, 483 insertions(+), 198 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/impala/blob/c5dc6ded/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java index d9e85fc..97ed631 100644 --- a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java +++ b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java @@ -2863,8 +2863,7 @@ public class CatalogOpExecutor { /** * This is a helper method to take care of catalog related updates when removing - * a privilege. The lock is held to prevent race conditions between getting the owner - * and updating the privileges for that owner. + * a privilege. */ private void removePrivilegeFromCatalog(String ownerString, PrincipalType ownerType, TPrivilege filter, TDdlExecResponse response) { @@ -3160,7 +3159,8 @@ public class CatalogOpExecutor { Lists.newArrayListWithExpectedSize(privileges.size()); if (grantRevokePrivParams.isIs_grant()) { addedRolePrivileges = catalog_.getSentryProxy().grantRolePrivileges(requestingUser, - roleName, privileges); + roleName, privileges, grantRevokePrivParams.isHas_grant_opt(), + removedGrantOptPrivileges); addSummary(resp, "Privilege(s) have been granted."); } else { // If this is a revoke of a privilege that contains the grant option, the privileges @@ -3201,14 +3201,19 @@ public class CatalogOpExecutor { resp.result.setUpdated_catalog_objects(updatedPrivs); resp.result.setVersion( updatedPrivs.get(updatedPrivs.size() - 1).getCatalog_version()); + if (!removedPrivs.isEmpty()) { + resp.result.setRemoved_catalog_objects(removedPrivs); + resp.result.setVersion( + Math.max(getLastItemVersion(updatedPrivs), + getLastItemVersion(removedPrivs))); + } } } else if (privileges.get(0).isHas_grant_opt()) { if (!updatedPrivs.isEmpty() && !removedPrivs.isEmpty()) { resp.result.setUpdated_catalog_objects(updatedPrivs); resp.result.setRemoved_catalog_objects(removedPrivs); resp.result.setVersion( - getLastItemVersion(updatedPrivs) > getLastItemVersion(removedPrivs) ? - getLastItemVersion(updatedPrivs) : getLastItemVersion(removedPrivs)); + Math.max(getLastItemVersion(updatedPrivs), getLastItemVersion(removedPrivs))); } } else { if (!removedPrivs.isEmpty()) { http://git-wip-us.apache.org/repos/asf/impala/blob/c5dc6ded/fe/src/main/java/org/apache/impala/util/SentryPolicyService.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/org/apache/impala/util/SentryPolicyService.java b/fe/src/main/java/org/apache/impala/util/SentryPolicyService.java index 97e5d49..e693136 100644 --- a/fe/src/main/java/org/apache/impala/util/SentryPolicyService.java +++ b/fe/src/main/java/org/apache/impala/util/SentryPolicyService.java @@ -338,7 +338,8 @@ public class SentryPolicyService { switch (scope) { case SERVER: client.get().revokeServerPrivilege(requestingUser.getShortName(), roleName, - privilege.getServer_name(), privilege.getPrivilege_level().toString()); + privilege.getServer_name(), privilege.getPrivilege_level().toString(), + /* grant option */ null); break; case DATABASE: client.get().revokeDatabasePrivilege(requestingUser.getShortName(), roleName, http://git-wip-us.apache.org/repos/asf/impala/blob/c5dc6ded/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 dd34253..9bff096 100644 --- a/fe/src/main/java/org/apache/impala/util/SentryProxy.java +++ b/fe/src/main/java/org/apache/impala/util/SentryProxy.java @@ -23,6 +23,7 @@ import java.util.Set; import java.util.concurrent.Executors; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; import org.apache.impala.catalog.AuthorizationException; import org.apache.impala.catalog.CatalogException; @@ -365,15 +366,78 @@ public class SentryProxy { * be modified. Returns the granted privileges. * Throws exception if there was any error updating the Sentry Service or if the Impala * catalog does not contain the given role name. + * This code is odd because we need to avoid duplicate privileges in Sentry because + * the same privilege with and without grant option are two different privileges. + * https://issues.apache.org/jira/browse/SENTRY-2408 + * If the current privilege and the requested privilege have the same grant option + * state, then just execute the grant. This is necessary so that if the user does not + * have the ability to grant, Sentry will throw an exception. We don't want to + * expose data by skipping the grant if it already exists. + * For the case when the existing privilege does not have the grant option, but the + * request does, we need to first add the new privilege, then revoke the old one. + * If this is done in the wrong order, and an exception is thrown, the user will get + * a "REVOKE_PRIVILEGE" error on a grant. + * For the case when the existing privilege does have the grant option, but the + * request does not, we add the grant option to the request because the privilege + * should not be "downgraded", we don't want to have duplicates, and we still need + * Sentry to perform the "has grant ability" check. Downgraded indicates changing a + * privilege from one that has a grant option to one that does not. */ public synchronized List<PrincipalPrivilege> grantRolePrivileges(User user, - String roleName, List<TPrivilege> privileges) throws ImpalaException { + String roleName, List<TPrivilege> privileges, boolean hasGrantOption, + List<PrincipalPrivilege> removedPrivileges) throws ImpalaException { + // First find out what's in the catalog. All privileges will have the same grant + // option set. The only case there will be more than one privilege is in the case + // of multiple column privileges. + Preconditions.checkArgument(!privileges.isEmpty()); + TPrivilege tWithGrant = privileges.get(0).deepCopy().setHas_grant_opt(true); + tWithGrant = tWithGrant.setPrivilege_name(PrincipalPrivilege + .buildPrivilegeName(tWithGrant)); + PrincipalPrivilege catWithGrant = catalog_.getPrincipalPrivilege(roleName, + tWithGrant); + TPrivilege tNoGrant = privileges.get(0).deepCopy().setHas_grant_opt(false); + tNoGrant = tNoGrant.setPrivilege_name(PrincipalPrivilege + .buildPrivilegeName(tNoGrant)); + PrincipalPrivilege catNoGrant = catalog_.getPrincipalPrivilege(roleName, tNoGrant); + + // List of privileges that should be removed. If removed, they will be added to + // the removedPrivileges list. + List<TPrivilege> toRemove = null; + + if (catNoGrant != null && hasGrantOption) { + toRemove = privileges.stream().map(p -> { + TPrivilege tp = p.deepCopy(); + return tp.setPrivilege_name((PrincipalPrivilege + .buildPrivilegeName(tp.setHas_grant_opt(false)))); + }).collect(Collectors.toList()); + } else if (catWithGrant != null && !hasGrantOption) { + // Elevate the requested privileges. + privileges = privileges.stream().map(p -> p.setPrivilege_name(PrincipalPrivilege + .buildPrivilegeName(p.setHas_grant_opt(true)))).collect(Collectors.toList()); + } + + // This is a list of privileges that were added, to be returned. + List<PrincipalPrivilege> rolePrivileges = Lists.newArrayList(); + // Do the grants first sentryPolicyService_.grantRolePrivileges(user, roleName, privileges); // Update the catalog - List<PrincipalPrivilege> rolePrivileges = Lists.newArrayList(); for (TPrivilege privilege: privileges) { rolePrivileges.add(catalog_.addRolePrivilege(roleName, privilege)); } + + // Then the revokes + if (toRemove != null && !toRemove.isEmpty()) { + sentryPolicyService_.revokeRolePrivileges(user, roleName, toRemove); + for (TPrivilege privilege : toRemove) { + PrincipalPrivilege rolePriv = catalog_.removeRolePrivilege(roleName, privilege); + if (rolePriv == null) continue; + removedPrivileges.add(rolePriv); + } + // If we removed anything, it might have removed the grants, so redo the grants. + // TODO: https://issues.apache.org/jira/browse/SENTRY-2408 + // When Sentry adds API to modify privileges, this code can be refactored. + sentryPolicyService_.grantRolePrivileges(user, roleName, privileges); + } return rolePrivileges; } @@ -386,6 +450,13 @@ public class SentryProxy { * are granted, in the case where the revoke involves a grant option. In this case * privileges that contain the grant option are removed and the new privileges without * the grant option are added. + * + * The revoke code is confusing because of the various usages of "grant option". The + * parameter "hasGrantOption" indicates that the revoke should just remove the grant + * option from an existing privilege. The grant option on the privilege indicates + * whether the existing privilege has the grant option set which for the case of + * revokes, there's currently no SQL statement that will result in the grant option + * being set on the request. */ public synchronized List<PrincipalPrivilege> revokeRolePrivileges(User user, String roleName, List<TPrivilege> privileges, boolean hasGrantOption, @@ -393,11 +464,25 @@ public class SentryProxy { List<PrincipalPrivilege> rolePrivileges = Lists.newArrayList(); if (!hasGrantOption) { sentryPolicyService_.revokeRolePrivileges(user, roleName, privileges); - // Update the catalog + // Update the catalog. The catalog should only have one privilege whether it has + // the grant option or not. We need to remove it which ever one is set. Since the + // catalog object name for privileges contains the grantoption value, we need to + // check both. for (TPrivilege privilege: privileges) { - PrincipalPrivilege rolePriv = catalog_.removeRolePrivilege(roleName, privilege); - if (rolePriv == null) continue; - rolePrivileges.add(rolePriv); + TPrivilege privNotGrant = privilege.deepCopy(); + privNotGrant.setHas_grant_opt(!privilege.has_grant_opt); + privNotGrant.setPrivilege_name(PrincipalPrivilege + .buildPrivilegeName(privNotGrant)); + PrincipalPrivilege rolePrivNotGrant = catalog_.removeRolePrivilege(roleName, + privNotGrant); + PrincipalPrivilege rolePriv = catalog_.removeRolePrivilege(roleName, + privilege); + if (rolePrivNotGrant != null) { + rolePrivileges.add(rolePrivNotGrant); + } + if (rolePriv != null) { + rolePrivileges.add(rolePriv); + } } } else { // If the REVOKE GRANT OPTION has been specified, the privileges with grant must be http://git-wip-us.apache.org/repos/asf/impala/blob/c5dc6ded/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java ---------------------------------------------------------------------- diff --git a/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java b/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java index 8308a7d..c30a97a 100644 --- a/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java +++ b/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java @@ -2298,15 +2298,15 @@ public class AuthorizationStmtTest extends FrontendTestBase { TPrivilegeLevel.ALL, TPrivilegeLevel.OWNER))); } } finally { - authzCatalog_.removeRole("foo_owner"); + authzCatalog_.removeRole("foo"); } boolean exceptionThrown = false; try { - parseAndAnalyze("alter database functional set owner role foo_owner", + parseAndAnalyze("alter database functional set owner role foo", analysisContext_ , frontend_); } catch (AnalysisException e) { exceptionThrown = true; - assertEquals("Role 'foo_owner' does not exist.", e.getLocalizedMessage()); + assertEquals("Role 'foo' does not exist.", e.getLocalizedMessage()); } assertTrue(exceptionThrown); http://git-wip-us.apache.org/repos/asf/impala/blob/c5dc6ded/tests/authorization/test_grant_revoke.py ---------------------------------------------------------------------- diff --git a/tests/authorization/test_grant_revoke.py b/tests/authorization/test_grant_revoke.py index 8973026..39bd22d 100644 --- a/tests/authorization/test_grant_revoke.py +++ b/tests/authorization/test_grant_revoke.py @@ -16,6 +16,8 @@ # under the License. # # Client tests for SQL statement authorization +# In these tests we run all the tests twice. Once with just using cache, hence the +# long sentry poll, and once by ensuring refreshes happen from Sentry. import grp import pytest @@ -23,16 +25,27 @@ from getpass import getuser from os import getenv 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.common.sentry_cache_test_suite import SentryCacheTestSuite, TestObject 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' -class TestGrantRevoke(CustomClusterTestSuite, ImpalaTestSuite): +# The polling frequency used by catalogd to refresh Sentry privileges. +# The long polling is so we can check updates just for the cache. +# The other polling is so we can get the updates without having to wait. +SENTRY_POLLING_FREQUENCY_S = 1 +SENTRY_LONG_POLLING_FREQUENCY_S = 120 +# The timeout, in seconds, when waiting for a refresh of Sentry privileges. +# This is also used as a delay before executing a statement. The difference between +# the two usages is one is used to check results and once successful can return before +# the time is up. The other is to enforce a delay with no short circuit. +SENTRY_REFRESH_TIMEOUT_S = SENTRY_POLLING_FREQUENCY_S * 3 + + +class TestGrantRevoke(SentryCacheTestSuite): @classmethod def add_test_dimensions(cls): super(TestGrantRevoke, cls).add_test_dimensions() @@ -67,8 +80,8 @@ class TestGrantRevoke(CustomClusterTestSuite, ImpalaTestSuite): # Create a temporary admin user so we can actually view/clean up the test # db. - self.client.execute("create role grant_revoke_test_admin") try: + self.client.execute("create role grant_revoke_test_admin") self.client.execute("grant all on server to grant_revoke_test_admin") self.client.execute("grant role grant_revoke_test_admin to group `%s`" % group_name) self.cleanup_db('grant_rev_db', sync_ddl=0) @@ -82,7 +95,7 @@ class TestGrantRevoke(CustomClusterTestSuite, ImpalaTestSuite): cls.client = impalad.service.create_beeswax_client() @pytest.mark.execute_serially - @CustomClusterTestSuite.with_args( + @SentryCacheTestSuite.with_args( impalad_args="--server_name=server1", catalogd_args="--sentry_config=" + SENTRY_CONFIG_FILE, sentry_config=SENTRY_CONFIG_FILE) @@ -90,7 +103,7 @@ class TestGrantRevoke(CustomClusterTestSuite, ImpalaTestSuite): self.run_test_case('QueryTest/grant_revoke', vector, use_db="default") @pytest.mark.execute_serially - @CustomClusterTestSuite.with_args( + @SentryCacheTestSuite.with_args( impalad_args="--server_name=server1", catalogd_args="--sentry_config=" + SENTRY_CONFIG_FILE, sentry_config=SENTRY_CONFIG_FILE) @@ -100,7 +113,121 @@ class TestGrantRevoke(CustomClusterTestSuite, ImpalaTestSuite): self.run_test_case('QueryTest/grant_revoke_kudu', vector, use_db="default") @pytest.mark.execute_serially - @CustomClusterTestSuite.with_args( + @SentryCacheTestSuite.with_args( + impalad_args="--server_name=server1", + catalogd_args="--sentry_config=" + SENTRY_CONFIG_FILE + + " --sentry_catalog_polling_frequency_s=" + str(SENTRY_POLLING_FREQUENCY_S), + sentry_config=SENTRY_CONFIG_FILE) + def test_grant_revoke_with_sentry(self, vector): + self.__execute_with_grant_option_tests(TestObject(TestObject.SERVER), "all", + sentry_refresh_timeout_s=SENTRY_REFRESH_TIMEOUT_S) + self.__execute_with_grant_option_tests(TestObject(TestObject.DATABASE, + "grant_rev_db"), "all", sentry_refresh_timeout_s=SENTRY_REFRESH_TIMEOUT_S) + self.__execute_with_grant_option_tests(TestObject(TestObject.TABLE, + "grant_rev_db.tbl1"), "all", sentry_refresh_timeout_s=SENTRY_REFRESH_TIMEOUT_S) + self.__execute_with_grant_option_tests(TestObject(TestObject.VIEW, + "grant_rev_db.tbl1"), "all", sentry_refresh_timeout_s=SENTRY_REFRESH_TIMEOUT_S) + self.__execute_with_grant_option_tests(TestObject(TestObject.SERVER), "select", + sentry_refresh_timeout_s=SENTRY_REFRESH_TIMEOUT_S) + self.__execute_with_grant_option_tests(TestObject(TestObject.DATABASE, + "grant_rev_db"), "select", sentry_refresh_timeout_s=SENTRY_REFRESH_TIMEOUT_S) + self.__execute_with_grant_option_tests(TestObject(TestObject.TABLE, + "grant_rev_db.tbl1"), "select", sentry_refresh_timeout_s=SENTRY_REFRESH_TIMEOUT_S) + self.__execute_with_grant_option_tests(TestObject(TestObject.VIEW, + "grant_rev_db.tbl1"), "select", sentry_refresh_timeout_s=SENTRY_REFRESH_TIMEOUT_S) + + @pytest.mark.execute_serially + @SentryCacheTestSuite.with_args( + impalad_args="--server_name=server1", + catalogd_args="--sentry_config=" + SENTRY_CONFIG_FILE + + " --sentry_catalog_polling_frequency_s=" + str(SENTRY_LONG_POLLING_FREQUENCY_S), + sentry_config=SENTRY_CONFIG_FILE) + def test_grant_revoke_with_sentry_long_poll(self, vector): + self.__execute_with_grant_option_tests(TestObject(TestObject.SERVER), "all") + self.__execute_with_grant_option_tests(TestObject(TestObject.DATABASE, + "grant_rev_db"), "all") + self.__execute_with_grant_option_tests(TestObject(TestObject.TABLE, + "grant_rev_db.tbl1"), "all") + self.__execute_with_grant_option_tests(TestObject(TestObject.VIEW, + "grant_rev_db.tbl1"), "all") + self.__execute_with_grant_option_tests(TestObject(TestObject.SERVER), "select") + self.__execute_with_grant_option_tests(TestObject(TestObject.DATABASE, + "grant_rev_db"), "select") + self.__execute_with_grant_option_tests(TestObject(TestObject.TABLE, + "grant_rev_db.tbl1"), "select") + self.__execute_with_grant_option_tests(TestObject(TestObject.VIEW, + "grant_rev_db.tbl1"), "select") + + def __execute_with_grant_option_tests(self, test_obj, privilege, + sentry_refresh_timeout_s=0): + """ + Executes grant/revoke tests with grant option + """ + # Do some initial setup only for this test. + group_name = grp.getgrnam(getuser()).gr_name + try: + self.client.execute("create role grant_revoke_test_admin") + except Exception: + # Ignore this as it was already created on the last run. + pass + self.client.execute("grant all on server to grant_revoke_test_admin") + self.client.execute("grant role grant_revoke_test_admin to group `%s`" % group_name) + self.client.execute("create role grant_revoke_test_role") + if test_obj.obj_type != TestObject.SERVER: + self.user_query(self.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="root") + + # Grant a basic privilege + self.user_query(self.client, "grant %s on %s %s to role grant_revoke_test_role" + % (privilege, test_obj.grant_name, test_obj.obj_name), user="root") + + # Ensure role has privilege. + self.validate_privileges(self.client, "show grant role grant_revoke_test_role", + test_obj, timeout_sec=sentry_refresh_timeout_s) + + # Try with grant option on existing privilege. + test_obj.grant = True + self.user_query(self.client, "grant %s on %s %s " % (privilege, test_obj.grant_name, + test_obj.obj_name) + " to role grant_revoke_test_role with grant option", + user="root") + # Ensure role has updated privilege. + self.validate_privileges(self.client, "show grant role grant_revoke_test_role", + test_obj, timeout_sec=sentry_refresh_timeout_s) + + # Revoke the grant option + self.user_query(self.client, + "revoke grant option for %s on %s %s from role grant_revoke_test_role" + % (privilege, test_obj.grant_name, test_obj.obj_name)) + # Ensure role has updated privilege. + test_obj.grant = False + self.validate_privileges(self.client, "show grant role grant_revoke_test_role", + test_obj, delay_s=sentry_refresh_timeout_s) + + # Add the grant option back, then add a regular privilege + self.user_query(self.client, "grant %s on %s %s to role grant_revoke_test_role" + % (privilege, test_obj.grant_name, test_obj.obj_name) + " with grant option", + user="root") + self.user_query(self.client, "grant %s on %s %s to role grant_revoke_test_role" + % (privilege, test_obj.grant_name, test_obj.obj_name), user="root") + test_obj.grant = True + self.validate_privileges(self.client, "show grant role grant_revoke_test_role", + test_obj, timeout_sec=sentry_refresh_timeout_s) + + # Revoke the privilege + self.user_query(self.client, "revoke %s on %s %s from role grant_revoke_test_role" + % (privilege, test_obj.grant_name, test_obj.obj_name)) + result = self.user_query(self.client, "show grant role grant_revoke_test_role", + delay_s=sentry_refresh_timeout_s) + assert len(result.data) == 0 + + # Reset the grant value + test_obj.grant = False + # Remove the role + self.client.execute("drop role grant_revoke_test_role") + + @pytest.mark.execute_serially + @SentryCacheTestSuite.with_args( impalad_args="--server_name=server1", catalogd_args="--sentry_config=" + SENTRY_CONFIG_FILE + " --sentry_catalog_polling_frequency_s=1", @@ -166,7 +293,7 @@ class TestGrantRevoke(CustomClusterTestSuite, ImpalaTestSuite): self.client.execute("drop role {0}".format(role_name)) @pytest.mark.execute_serially - @CustomClusterTestSuite.with_args( + @SentryCacheTestSuite.with_args( impalad_args="--server_name=server1", catalogd_args="--sentry_config=" + SENTRY_CONFIG_FILE, sentry_config=SENTRY_CONFIG_FILE) http://git-wip-us.apache.org/repos/asf/impala/blob/c5dc6ded/tests/authorization/test_owner_privileges.py ---------------------------------------------------------------------- diff --git a/tests/authorization/test_owner_privileges.py b/tests/authorization/test_owner_privileges.py index 2469c3a..27ab36a 100644 --- a/tests/authorization/test_owner_privileges.py +++ b/tests/authorization/test_owner_privileges.py @@ -30,16 +30,16 @@ from getpass import getuser from os import getenv from time import sleep, time -from tests.common.custom_cluster_test_suite import CustomClusterTestSuite +from tests.common.sentry_cache_test_suite import SentryCacheTestSuite, TestObject from tests.common.test_dimensions import create_uncompressed_text_dimension # The polling frequency used by catalogd to refresh Sentry privileges. # The long polling is so we can check updates just for the cache. # The other polling is so we can get the updates without having to wait. SENTRY_LONG_POLLING_FREQUENCY_S = 60 -SENTRY_POLLING_FREQUENCY_S = 3 +SENTRY_POLLING_FREQUENCY_S = 1 # The timeout, in seconds, when waiting for a refresh of Sentry privileges. -SENTRY_REFRESH_TIMEOUT_S = 9 +SENTRY_REFRESH_TIMEOUT_S = SENTRY_POLLING_FREQUENCY_S * 2 SENTRY_CONFIG_DIR = getenv('IMPALA_HOME') + '/fe/src/test/resources/' SENTRY_BASE_LOG_DIR = getenv('IMPALA_CLUSTER_LOGS_DIR') + "/sentry" @@ -48,33 +48,7 @@ SENTRY_CONFIG_FILE_OO_NOGRANT = SENTRY_CONFIG_DIR + 'sentry-site_oo_nogrant.xml' SENTRY_CONFIG_FILE_NO_OO = SENTRY_CONFIG_DIR + 'sentry-site_no_oo.xml' -class TestObject(): - DATABASE = "database" - TABLE = "table" - VIEW = "view" - - def __init__(self, obj_type, obj_name, grant=False): - self.obj_name = obj_name - self.obj_type = obj_type - parts = obj_name.split(".") - self.db_name = parts[0] - self.table_name = None - self.table_def = "" - self.view_select = "" - if len(parts) > 1: - self.table_name = parts[1] - if obj_type == TestObject.VIEW: - self.grant_name = TestObject.TABLE - self.view_select = "as select * from functional.alltypes" - elif obj_type == TestObject.TABLE: - self.grant_name = TestObject.TABLE - self.table_def = "(col1 int)" - else: - self.grant_name = obj_type - self.grant = grant - - -class TestOwnerPrivileges(CustomClusterTestSuite): +class TestOwnerPrivileges(SentryCacheTestSuite): @classmethod def add_test_dimensions(cls): super(TestOwnerPrivileges, cls).add_test_dimensions() @@ -130,7 +104,7 @@ class TestOwnerPrivileges(CustomClusterTestSuite): self.execute_query("drop role %s" % role_name) @pytest.mark.execute_serially - @CustomClusterTestSuite.with_args( + @SentryCacheTestSuite.with_args( impalad_args="--server_name=server1 --sentry_config=" + SENTRY_CONFIG_FILE_OO, catalogd_args="--sentry_config=" + SENTRY_CONFIG_FILE_OO + " --sentry_catalog_polling_frequency_s=" + str(SENTRY_LONG_POLLING_FREQUENCY_S), @@ -144,22 +118,22 @@ class TestOwnerPrivileges(CustomClusterTestSuite): unique_database + ".owner_priv_view", grant=True)) @pytest.mark.execute_serially - @CustomClusterTestSuite.with_args( + @SentryCacheTestSuite.with_args( impalad_args="--server_name=server1 --sentry_config=" + SENTRY_CONFIG_FILE_OO, catalogd_args="--sentry_config=" + SENTRY_CONFIG_FILE_OO + " --sentry_catalog_polling_frequency_s=" + str(SENTRY_POLLING_FREQUENCY_S), sentry_config=SENTRY_CONFIG_FILE_OO) def test_owner_privileges_with_grant(self, vector, unique_database): self.__execute_owner_privilege_tests(TestObject(TestObject.DATABASE, "owner_priv_db", - grant=True), sentry_refresh_timeout_sec=SENTRY_REFRESH_TIMEOUT_S) + grant=True), sentry_refresh_timeout_s=SENTRY_REFRESH_TIMEOUT_S) self.__execute_owner_privilege_tests(TestObject(TestObject.TABLE, unique_database + ".owner_priv_tbl", grant=True), - sentry_refresh_timeout_sec=SENTRY_REFRESH_TIMEOUT_S) + sentry_refresh_timeout_s=SENTRY_REFRESH_TIMEOUT_S) self.__execute_owner_privilege_tests(TestObject(TestObject.VIEW, unique_database + ".owner_priv_view", grant=True), - sentry_refresh_timeout_sec=SENTRY_REFRESH_TIMEOUT_S) + sentry_refresh_timeout_s=SENTRY_REFRESH_TIMEOUT_S) - def __execute_owner_privilege_tests(self, test_obj, sentry_refresh_timeout_sec=0): + def __execute_owner_privilege_tests(self, test_obj, sentry_refresh_timeout_s=0): """ Executes all the statements required to validate owner privileges work correctly for a specific database, table, or view. @@ -167,111 +141,76 @@ class TestOwnerPrivileges(CustomClusterTestSuite): # Create object and ensure root gets owner privileges. self.root_impalad_client = self.create_impala_client() self.test_user_impalad_client = self.create_impala_client() - self.__root_query("create %s if not exists %s %s %s" % (test_obj.obj_type, - test_obj.obj_name, test_obj.table_def, test_obj.view_select)) - self.__validate_privileges(self.root_impalad_client, "root", "show grant user root", - test_obj, sentry_refresh_timeout_sec) + self.user_query(self.root_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="root") + self.validate_privileges(self.root_impalad_client, "show grant user root", test_obj, + sentry_refresh_timeout_s, "root") # Ensure grant works. - self.__root_query("grant all on %s %s to role owner_priv_test_all_role" - % (test_obj.grant_name, test_obj.obj_name)) - self.__root_query("revoke all on %s %s from role owner_priv_test_all_role" - % (test_obj.grant_name, test_obj.obj_name)) + self.user_query(self.root_impalad_client, + "grant all on %s %s to role owner_priv_test_all_role" + % (test_obj.grant_name, test_obj.obj_name), user="root") + self.user_query(self.root_impalad_client, + "revoke all on %s %s from role owner_priv_test_all_role" + % (test_obj.grant_name, test_obj.obj_name), user="root") # Change the database owner and ensure root does not have owner privileges. - self.__root_query("alter %s %s set owner user test_user" % (test_obj.obj_type, - test_obj.obj_name)) - result = self.__root_query("show grant user root") + self.user_query(self.root_impalad_client, "alter %s %s set owner user test_user" + % (test_obj.obj_type, test_obj.obj_name), user="root") + result = self.user_query(self.root_impalad_client, "show grant user root", + user="root", delay_s=sentry_refresh_timeout_s) assert len(result.data) == 0 # Ensure root cannot drop database after owner change. - self.__root_query_fail("drop %s %s" % (test_obj.obj_type, test_obj.obj_name), - "does not have privileges to execute 'DROP'") + self.user_query(self.root_impalad_client, "drop %s %s" % (test_obj.obj_type, + test_obj.obj_name), user="root", + error_msg="does not have privileges to execute 'DROP'") # test_user should have privileges for object now. - self.__validate_privileges(self.test_user_impalad_client, "test_user", - "show grant user test_user", test_obj, sentry_refresh_timeout_sec) + self.validate_privileges(self.test_user_impalad_client, "show grant user test_user", + test_obj, sentry_refresh_timeout_s, "test_user") # Change the owner to a role and ensure test_user doesn't have privileges. # Set the owner back to root since for views, test_user doesn't have select # privileges on the underlying table. self.execute_query("alter %s %s set owner user root" % (test_obj.obj_type, test_obj.obj_name)) - result = self.__test_user_query("show grant user test_user") + result = self.user_query(self.test_user_impalad_client, + "show grant user test_user", user="test_user", delay_s=sentry_refresh_timeout_s) assert len(result.data) == 0 - self.__root_query("alter %s %s set owner role owner_priv_test_owner_role" - % (test_obj.obj_type, test_obj.obj_name)) + self.user_query(self.root_impalad_client, + "alter %s %s set owner role owner_priv_test_owner_role" + % (test_obj.obj_type, test_obj.obj_name), user="root") # Ensure root does not have user privileges. - result = self.__root_query("show grant user root") + result = self.user_query(self.root_impalad_client, "show grant user root", + user="root", delay_s=sentry_refresh_timeout_s) assert len(result.data) == 0 # Ensure role has owner privileges. - self.__validate_privileges(self.root_impalad_client, "root", - "show grant role owner_priv_test_owner_role", test_obj, - sentry_refresh_timeout_sec) + self.validate_privileges(self.root_impalad_client, + "show grant role owner_priv_test_owner_role", test_obj, sentry_refresh_timeout_s, + "root") # Drop the object and ensure no role privileges. - self.__root_query("drop %s %s " % (test_obj.obj_type, test_obj.obj_name)) - result = self.__root_query("show grant role owner_priv_test_owner_role") + self.user_query(self.root_impalad_client, "drop %s %s " % (test_obj.obj_type, + test_obj.obj_name), user="root") + result = self.user_query(self.root_impalad_client, "show grant role " + + "owner_priv_test_owner_role", user="root", delay_s=sentry_refresh_timeout_s) assert len(result.data) == 0 # Ensure user privileges are gone after drop. - self.__root_query("create %s if not exists %s %s %s" % (test_obj.obj_type, - test_obj.obj_name, test_obj.table_def, test_obj.view_select)) - self.__root_query("drop %s %s " % (test_obj.obj_type, test_obj.obj_name)) - result = self.__root_query("show grant user root") + self.user_query(self.root_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="root") + self.user_query(self.root_impalad_client, "drop %s %s " % (test_obj.obj_type, + test_obj.obj_name), user="root") + result = self.user_query(self.root_impalad_client, "show grant user root", + user="root") assert len(result.data) == 0 - def __str_to_bool(self, val): - if val.lower() == 'true': - return True - return False - - def __check_privileges(self, result, test_obj, null_create_date=True): - # results should have the following columns - # scope, database, table, column, uri, privilege, grant_option, create_time - for row in result.data: - col = row.split('\t') - assert col[0] == test_obj.grant_name - assert col[1] == test_obj.db_name - if test_obj.table_name is not None and len(test_obj.table_name) > 0: - assert col[2] == test_obj.table_name - assert self.__str_to_bool(col[6]) == test_obj.grant - if not null_create_date: - assert str(col[7]) != 'NULL' - - def __root_query(self, query): - return self.execute_query_expect_success(self.root_impalad_client, query, user="root") - - def __root_query_fail(self, query, error_msg): - e = self.execute_query_expect_failure(self.root_impalad_client, query, user="root") - self.__verify_exceptions(error_msg, str(e)) - - def __validate_privileges(self, client, user, query, test_obj, timeout_sec): - """Validate privileges. If timeout_sec is > 0 then retry until create_date is not null - or the timeout_sec is reached. - """ - if timeout_sec is None or timeout_sec <= 0: - self.__check_privileges(self.execute_query_expect_success(client, query, - user=user), test_obj) - else: - start_time = time() - while time() - start_time < timeout_sec: - result = self.execute_query_expect_success(client, query, user=user) - try: - self.__check_privileges(result, test_obj, null_create_date=False) - return True - except Exception: - pass - sleep(1) - return False - - def __test_user_query(self, query): - return self.execute_query_expect_success(self.test_user_impalad_client, query, - user="test_user") - @pytest.mark.execute_serially - @CustomClusterTestSuite.with_args( + @SentryCacheTestSuite.with_args( impalad_args="--server_name=server1 --sentry_config=" + SENTRY_CONFIG_FILE_NO_OO, catalogd_args="--sentry_config=" + SENTRY_CONFIG_FILE_NO_OO + " --sentry_catalog_polling_frequency_s=" + str(SENTRY_LONG_POLLING_FREQUENCY_S), @@ -285,55 +224,60 @@ class TestOwnerPrivileges(CustomClusterTestSuite): unique_database + ".owner_priv_view")) @pytest.mark.execute_serially - @CustomClusterTestSuite.with_args( + @SentryCacheTestSuite.with_args( impalad_args="--server_name=server1 --sentry_config=" + SENTRY_CONFIG_FILE_NO_OO, catalogd_args="--sentry_config=" + SENTRY_CONFIG_FILE_NO_OO + " --sentry_catalog_polling_frequency_s=" + str(SENTRY_POLLING_FREQUENCY_S), sentry_config=SENTRY_CONFIG_FILE_NO_OO) def test_owner_privileges_disabled(self, vector, unique_database): self.__execute_owner_privilege_tests_no_oo(TestObject(TestObject.DATABASE, - "owner_priv_db"), sentry_refresh_timeout_sec=SENTRY_REFRESH_TIMEOUT_S) + "owner_priv_db"), sentry_refresh_timeout_s=SENTRY_REFRESH_TIMEOUT_S) self.__execute_owner_privilege_tests_no_oo(TestObject(TestObject.TABLE, unique_database + ".owner_priv_tbl"), - sentry_refresh_timeout_sec=SENTRY_REFRESH_TIMEOUT_S) + sentry_refresh_timeout_s=SENTRY_REFRESH_TIMEOUT_S) self.__execute_owner_privilege_tests_no_oo(TestObject(TestObject.VIEW, unique_database + ".owner_priv_view"), - sentry_refresh_timeout_sec=SENTRY_REFRESH_TIMEOUT_S) + sentry_refresh_timeout_s=SENTRY_REFRESH_TIMEOUT_S) - def __execute_owner_privilege_tests_no_oo(self, test_obj, sentry_refresh_timeout_sec=0): + def __execute_owner_privilege_tests_no_oo(self, test_obj, sentry_refresh_timeout_s=0): """ Executes all the statements required to validate owner privileges work correctly for a specific database, table, or view. """ # Create object and ensure root gets owner privileges. self.root_impalad_client = self.create_impala_client() - self.test_user_impalad_client = self.create_impala_client() - self.__root_query("create %s if not exists %s %s %s" % (test_obj.obj_type, - test_obj.obj_name, test_obj.table_def, test_obj.view_select)) + self.user_query(self.root_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="root") # For user privileges, if the user has no privileges, the user will not exist # in the privileges list. - self.__root_query_fail("show grant user root", "User 'root' does not exist") + self.user_query(self.root_impalad_client, "show grant user root", + user="root", error_msg="User 'root' does not exist") # Ensure grant doesn't work. - self.__root_query_fail("grant all on %s %s to role owner_priv_test_all_role" - % (test_obj.grant_name, test_obj.obj_name), - "does not have privileges to execute: GRANT_PRIVILEGE") + self.user_query(self.root_impalad_client, + "grant all on %s %s to role owner_priv_test_all_role" + % (test_obj.grant_name, test_obj.obj_name), user="root", + error_msg="does not have privileges to execute: GRANT_PRIVILEGE") - self.__root_query_fail("revoke all on %s %s from role owner_priv_test_all_role" - % (test_obj.grant_name, test_obj.obj_name), - "does not have privileges to execute: REVOKE_PRIVILEGE") + self.user_query(self.root_impalad_client, + "revoke all on %s %s from role owner_priv_test_all_role" + % (test_obj.grant_name, test_obj.obj_name), user="root", + error_msg="does not have privileges to execute: REVOKE_PRIVILEGE") # Ensure changing the database owner doesn't work. - self.__root_query_fail("alter %s %s set owner user test_user" - % (test_obj.obj_type, test_obj.obj_name), - "does not have privileges with 'GRANT OPTION'") + self.user_query(self.root_impalad_client, "alter %s %s set owner user test_user" + % (test_obj.obj_type, test_obj.obj_name), user="root", + error_msg="does not have privileges with 'GRANT OPTION'") # Ensure root cannot drop database. - self.__root_query_fail("drop %s %s" % (test_obj.obj_type, test_obj.obj_name), - "does not have privileges to execute 'DROP'") + self.user_query(self.root_impalad_client, "drop %s %s" % (test_obj.obj_type, + test_obj.obj_name), user="root", + error_msg="does not have privileges to execute 'DROP'", + delay_s=sentry_refresh_timeout_s) @pytest.mark.execute_serially - @CustomClusterTestSuite.with_args( + @SentryCacheTestSuite.with_args( impalad_args="--server_name=server1 --sentry_config=" + SENTRY_CONFIG_FILE_OO_NOGRANT, catalogd_args="--sentry_config=" + SENTRY_CONFIG_FILE_OO_NOGRANT + " --sentry_catalog_polling_frequency_s=" + str(SENTRY_LONG_POLLING_FREQUENCY_S), @@ -347,60 +291,52 @@ class TestOwnerPrivileges(CustomClusterTestSuite): unique_database + ".owner_priv_view")) @pytest.mark.execute_serially - @CustomClusterTestSuite.with_args( + @SentryCacheTestSuite.with_args( impalad_args="--server_name=server1 --sentry_config=" + SENTRY_CONFIG_FILE_OO_NOGRANT, catalogd_args="--sentry_config=" + SENTRY_CONFIG_FILE_OO_NOGRANT + " --sentry_catalog_polling_frequency_s=" + str(SENTRY_POLLING_FREQUENCY_S), sentry_config=SENTRY_CONFIG_FILE_OO_NOGRANT) def test_owner_privileges_without_grant(self, vector, unique_database): self.__execute_owner_privilege_tests_oo_nogrant(TestObject(TestObject.DATABASE, - "owner_priv_db"), sentry_refresh_timeout_sec=SENTRY_REFRESH_TIMEOUT_S) + "owner_priv_db"), sentry_refresh_timeout_s=SENTRY_REFRESH_TIMEOUT_S) self.__execute_owner_privilege_tests_oo_nogrant(TestObject(TestObject.TABLE, unique_database + ".owner_priv_tbl"), - sentry_refresh_timeout_sec=SENTRY_REFRESH_TIMEOUT_S) + sentry_refresh_timeout_s=SENTRY_REFRESH_TIMEOUT_S) self.__execute_owner_privilege_tests_oo_nogrant(TestObject(TestObject.VIEW, unique_database + ".owner_priv_view"), - sentry_refresh_timeout_sec=SENTRY_REFRESH_TIMEOUT_S) + sentry_refresh_timeout_s=SENTRY_REFRESH_TIMEOUT_S) def __execute_owner_privilege_tests_oo_nogrant(self, test_obj, - sentry_refresh_timeout_sec=0): + sentry_refresh_timeout_s=0): """ Executes all the statements required to validate owner privileges work correctly for a specific database, table, or view. """ # Create object and ensure root gets owner privileges. self.root_impalad_client = self.create_impala_client() - self.test_user_impalad_client = self.create_impala_client() - self.__root_query("create %s if not exists %s %s %s" % (test_obj.obj_type, - test_obj.obj_name, test_obj.table_def, test_obj.view_select)) - self.__validate_privileges(self.root_impalad_client, "root", "show grant user root", - test_obj, sentry_refresh_timeout_sec) + self.user_query(self.root_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="root") + self.validate_privileges(self.root_impalad_client, "show grant user root", test_obj, + sentry_refresh_timeout_s, "root") # Ensure grant doesn't work. - self.__root_query_fail("grant all on %s %s to role owner_priv_test_all_role" - % (test_obj.grant_name, test_obj.obj_name), - "does not have privileges to execute: GRANT_PRIVILEGE") - - self.__root_query_fail("revoke all on %s %s from role owner_priv_test_all_role" - % (test_obj.grant_name, test_obj.obj_name), - "does not have privileges to execute: REVOKE_PRIVILEGE") - - self.__root_query_fail("alter %s %s set owner user test_user" - % (test_obj.obj_type, test_obj.obj_name), - "does not have privileges with 'GRANT OPTION'") - - self.__root_query("drop %s %s " % (test_obj.obj_type, test_obj.obj_name)) - result = self.__root_query("show grant user root") + self.user_query(self.root_impalad_client, + "grant all on %s %s to role owner_priv_test_all_role" + % (test_obj.grant_name, test_obj.obj_name), user="root", + error_msg="does not have privileges to execute: GRANT_PRIVILEGE") + + self.user_query(self.root_impalad_client, + "revoke all on %s %s from role owner_priv_test_all_role" + % (test_obj.grant_name, test_obj.obj_name), user="root", + error_msg="does not have privileges to execute: REVOKE_PRIVILEGE") + + self.user_query(self.root_impalad_client, "alter %s %s set owner user test_user" + % (test_obj.obj_type, test_obj.obj_name), user="root", + error_msg="does not have privileges with 'GRANT OPTION'") + + self.user_query(self.root_impalad_client, "drop %s %s " % (test_obj.obj_type, + test_obj.obj_name), user="root") + result = self.user_query(self.root_impalad_client, "show grant user root", + user="root") assert len(result.data) == 0 - - def __verify_exceptions(self, expected_str, actual_str): - """ - Verifies that 'expected_str' is a substring of the actual exception string - 'actual_str'. - """ - actual_str = actual_str.replace('\n', '') - # Strip newlines so we can split error message into multiple lines - expected_str = expected_str.replace('\n', '') - if expected_str in actual_str: return - assert False, 'Unexpected exception string. Expected: %s\nNot found in actual: %s' % \ - (expected_str, actual_str) http://git-wip-us.apache.org/repos/asf/impala/blob/c5dc6ded/tests/common/custom_cluster_test_suite.py ---------------------------------------------------------------------- diff --git a/tests/common/custom_cluster_test_suite.py b/tests/common/custom_cluster_test_suite.py index 6e14274..ed3d671 100644 --- a/tests/common/custom_cluster_test_suite.py +++ b/tests/common/custom_cluster_test_suite.py @@ -148,12 +148,9 @@ class CustomClusterTestSuite(ImpalaTestSuite): sentry_env = dict(os.environ) sentry_env['SENTRY_SERVICE_CONFIG'] = sentry_service_config call = subprocess.Popen( - [os.path.join(IMPALA_HOME, 'testdata/bin/run-sentry-service.sh')], - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - stdin=file("/dev/null"), + ['/bin/bash', '-c', os.path.join(IMPALA_HOME, + 'testdata/bin/run-sentry-service.sh')], env=sentry_env) - (stdout, stderr) = call.communicate() call.wait() if call.returncode != 0: raise RuntimeError("unable to start sentry") http://git-wip-us.apache.org/repos/asf/impala/blob/c5dc6ded/tests/common/sentry_cache_test_suite.py ---------------------------------------------------------------------- diff --git a/tests/common/sentry_cache_test_suite.py b/tests/common/sentry_cache_test_suite.py new file mode 100644 index 0000000..ad6f10d --- /dev/null +++ b/tests/common/sentry_cache_test_suite.py @@ -0,0 +1,134 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# +# Base class for test that need to run with both just privilege +# cache as well as Sentry privilege refresh. +# There are two variables (delay_s timeout) used in these methods. +# The first is the timeout when validating privileges. This is +# needed to ensure that the privileges returned have been updated +# from Sentry. The second is the delay_s before executing a query. +# This is needed to ensure Sentry has been updated before running +# the query. The reason for both is because the timeout can +# be short circuited upon successful results. Using the delay_s +# for every query and test would add significant time. As an +# example, if a revoke is called, the expectation is the privilege +# would not be in the result. If the cache is updated correctly, +# but Sentry was not, using the timeout check, we would miss that +# Sentry was not updated correctly. + +from time import sleep, time +from tests.common.custom_cluster_test_suite import CustomClusterTestSuite + + +class SentryCacheTestSuite(CustomClusterTestSuite): + @staticmethod + def str_to_bool(val): + return val.lower() == 'true' + + @staticmethod + def __check_privileges(result, test_obj, null_create_date=True): + """ + This method validates privileges. Most validations are assertions, but for + null_create_date, we just return False to indicate the privilege cannot + be validated because it has not been refreshed from Sentry yet. + """ + # results should have the following columns + # scope, database, table, column, uri, privilege, grant_option, create_time + for row in result.data: + col = row.split('\t') + assert col[0] == test_obj.grant_name + assert col[1] == test_obj.db_name + if test_obj.table_name is not None and len(test_obj.table_name) > 0: + assert col[2] == test_obj.table_name + assert SentryCacheTestSuite.str_to_bool(col[6]) == test_obj.grant + if not null_create_date and str(col[7]) == 'NULL': + return False + return True + + def validate_privileges(self, client, query, test_obj, timeout_sec=None, user=None, + delay_s=0): + """Validate privileges. If timeout_sec is > 0 then retry until create_date is not null + or the timeout_sec is reached. If delay_s is > 0 then wait that long before running. + """ + if delay_s > 0: + sleep(delay_s) + if timeout_sec is None or timeout_sec <= 0: + self.__check_privileges(self.execute_query_expect_success(client, query, + user=user), test_obj) + else: + start_time = time() + while time() - start_time < timeout_sec: + result = self.execute_query_expect_success(client, query, user=user) + success = self.__check_privileges(result, test_obj, null_create_date=False) + if success: + return True + sleep(1) + return False + + def user_query(self, client, query, user=None, delay_s=0, error_msg=None): + """ + Executes a query with the root user client. If delay_s is > 0 then wait before + running the query. This is used to wait for Sentry refresh. If error_msg is + set, then expect a failure. Returns None when there is no error_msg. + """ + if delay_s > 0: + sleep(delay_s) + if error_msg is not None: + e = self.execute_query_expect_failure(client, query, user=user) + self.verify_exceptions(error_msg, str(e)) + return None + return self.execute_query_expect_success(client, query, user=user) + + @staticmethod + def verify_exceptions(expected_str, actual_str): + """ + Verifies that 'expected_str' is a substring of the actual exception string + 'actual_str'. + """ + actual_str = actual_str.replace('\n', '') + # Strip newlines so we can split error message into multiple lines + expected_str = expected_str.replace('\n', '') + if expected_str in actual_str: return + assert False, 'Unexpected exception string. Expected: %s\nNot found in actual: %s' % \ + (expected_str, actual_str) + + +class TestObject(object): + SERVER = "server" + DATABASE = "database" + TABLE = "table" + VIEW = "view" + + def __init__(self, obj_type, obj_name="", grant=False): + self.obj_name = obj_name + self.obj_type = obj_type + parts = obj_name.split(".") + self.db_name = parts[0] + self.table_name = None + self.table_def = "" + self.view_select = "" + if len(parts) > 1: + self.table_name = parts[1] + if obj_type == TestObject.VIEW: + self.grant_name = TestObject.TABLE + self.view_select = "as select * from functional.alltypes" + elif obj_type == TestObject.TABLE: + self.grant_name = TestObject.TABLE + self.table_def = "(col1 int)" + else: + self.grant_name = obj_type + self.grant = grant
