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/17 17:03:05 UTC

[10/11] impala git commit: IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled

IMPALA-7713: Add test coverage for catalogd restart when authorization is enabled

This patch adds a test coverage for catalogd restart when authorization
is enabled to ensure all privileges in the impalad's catalogs get reset
after the catalogd restart to avoid stale privileges in the impalad's
catalogs, which can pose a security issue.

Testing:
- Ran all E2E authorization tests
- Added a new test

Change-Id: Ib9a168697401cf0b83c7a193fa477888b48cb369
Reviewed-on: http://gerrit.cloudera.org:8080/11696
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/0cd91518
Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/0cd91518
Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/0cd91518

Branch: refs/heads/master
Commit: 0cd9151801cf446330b129b0609d6cedd4b98f06
Parents: 2b2cf8d
Author: Fredy Wijaya <fw...@cloudera.com>
Authored: Tue Oct 16 12:30:34 2018 -0700
Committer: Impala Public Jenkins <im...@cloudera.com>
Committed: Wed Oct 17 05:33:05 2018 +0000

----------------------------------------------------------------------
 tests/authorization/test_authorization.py | 55 ++++++++++++++++++++++++++
 tests/conftest.py                         | 12 ++++++
 2 files changed, 67 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/impala/blob/0cd91518/tests/authorization/test_authorization.py
----------------------------------------------------------------------
diff --git a/tests/authorization/test_authorization.py b/tests/authorization/test_authorization.py
index a508440..cf5b2e9 100644
--- a/tests/authorization/test_authorization.py
+++ b/tests/authorization/test_authorization.py
@@ -383,3 +383,58 @@ class TestAuthorization(CustomClusterTestSuite):
   def test_deprecated_flags(self):
     assert_file_in_dir_contains(self.impala_log_dir, "authorization_policy_file flag" +
         " is deprecated. Object Ownership feature is not supported")
+
+  @pytest.mark.execute_serially
+  @CustomClusterTestSuite.with_args(
+    impalad_args="--server_name=server1 --sentry_config=%s" % SENTRY_CONFIG_FILE,
+    catalogd_args="--sentry_config=%s" % SENTRY_CONFIG_FILE,
+    impala_log_dir=tempfile.mkdtemp(prefix="test_catalog_restart_",
+                                    dir=os.getenv("LOG_DIR")))
+  def test_catalog_restart(self, unique_role):
+    """IMPALA-7713: Tests that a catalogd restart when authorization is enabled should
+    reset the previous privileges stored in impalad's catalog to avoid stale privilege
+    data in the impalad's catalog."""
+    def assert_privileges():
+      result = self.client.execute("show grant role %s_foo" % unique_role)
+      TestAuthorization._check_privileges(result, [["database", "functional",
+                                                    "", "", "", "all", "false"]])
+
+      result = self.client.execute("show grant role %s_bar" % unique_role)
+      TestAuthorization._check_privileges(result, [["database", "functional_kudu",
+                                                    "", "", "", "all", "false"]])
+
+      result = self.client.execute("show grant role %s_baz" % unique_role)
+      TestAuthorization._check_privileges(result, [["database", "functional_avro",
+                                                    "", "", "", "all", "false"]])
+
+    self.role_cleanup(unique_role)
+    try:
+      self.client.execute("create role %s_foo" % unique_role)
+      self.client.execute("create role %s_bar" % unique_role)
+      self.client.execute("create role %s_baz" % unique_role)
+      self.client.execute("grant all on database functional to role %s_foo" %
+                          unique_role)
+      self.client.execute("grant all on database functional_kudu to role %s_bar" %
+                          unique_role)
+      self.client.execute("grant all on database functional_avro to role %s_baz" %
+                          unique_role)
+
+      assert_privileges()
+      self._start_impala_cluster(["--catalogd_args=--sentry_config=%s" %
+                                  SENTRY_CONFIG_FILE, "--restart_catalogd_only"])
+      assert_privileges()
+    finally:
+      self.role_cleanup(unique_role)
+
+  def role_cleanup(self, role_name_match):
+    """Cleans up any roles that match the given role name."""
+    for role_name in self.client.execute("show roles").data:
+      if role_name_match in role_name:
+        self.client.execute("drop role %s" % role_name)
+
+  @staticmethod
+  def _check_privileges(result, expected):
+    def columns(row):
+      cols = row.split("\t")
+      return cols[0:len(cols) - 1]
+    assert map(columns, result.data) == expected

http://git-wip-us.apache.org/repos/asf/impala/blob/0cd91518/tests/conftest.py
----------------------------------------------------------------------
diff --git a/tests/conftest.py b/tests/conftest.py
index 1eb5c76..4e1c837 100644
--- a/tests/conftest.py
+++ b/tests/conftest.py
@@ -329,6 +329,18 @@ def unique_database(request, testid_checksum):
   return first_db_name
 
 
+@pytest.fixture
+def unique_role(request, testid_checksum):
+  """Returns a unique role to any test using the fixture. The fixture does not create
+  a role."""
+  role_name_prefix = request.function.__name__
+  fixture_params = getattr(request, 'param', None)
+  if fixture_params is not None:
+    if 'name_prefix' in fixture_params:
+      role_name_prefix = fixture_params['name_prefix']
+  return '{0}_{1}_role'.format(role_name_prefix, testid_checksum)
+
+
 @pytest.yield_fixture
 def kudu_client():
   """Provides a new Kudu client as a pytest fixture. The client only exists for the