[impala] 01/04: IMPALA-9977: Remove duplicate Ranger audit log entries for ALTER events

[ta...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

tarmstrong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git

commit d83074c5992d51d136a509639a038bde5c3393bf
Author: Fang-Yu Rao <fa...@cloudera.com>
AuthorDate: Tue Jul 21 17:11:23 2020 -0700

    IMPALA-9977: Remove duplicate Ranger audit log entries for ALTER events
    
    This JIRA could be considered as a follow-up to IMPALA-9625, where we
    converted the name of a TAccessEvent to lowercase to avoid duplicate
    audits in the Set used to maintain the collected TAccessEvent's so that
    there will not be duplicate TAccessEvent's in the file specified by the
    flag of "-audit_event_log_dir" when Impala is started.
    
    However, the patch for IMPALA-9625 only considered the audits that are
    exported to the specific file mentioned above but not the
    PrivilegeRequest's that will be processed by Ranger which in turn would
    produce the corresponding audit log entries. Therefore, the
    fully-qualified table name that is provided when
    Analyzer#registerPrivReq() is called in Analyzer#getTable() is not
    necessarily in lowercase, resulting in duplicate AuthzAuditEvent's
    stored in the corresponding RangerBufferAuditHandler because the
    full table names returned from registerAuthAndAuditEvent() and
    getTable() differ. Refer to IMPALA-9625 for more details.
    
    To resolve the inconsistencies, this patch converts the arguments of
    database and table names to lowercase when
    PrivilegeRequestBuilder#onTable() is building the corresponding
    PrivilegeRequest, which will later be added to the Set of
    PrivilegeRequest's for Ranger to process.
    
    Testing:
    - Added an FE test in RangerAuditLogTest.java to make sure no duplicate
      Ranger audit log entries are produced.
    - Verified that the patch passes the exhaustive tests in the DEBUG
      build.
    
    Change-Id: Iab9b664ad5ee9722182007ee67d14bf47bd03d8a
    Reviewed-on: http://gerrit.cloudera.org:8080/16231
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 .../impala/authorization/PrivilegeRequestBuilder.java      |  5 ++++-
 .../impala/authorization/ranger/RangerAuditLogTest.java    | 14 ++++++++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java b/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
index b115fb3..72127c8 100644
--- a/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
+++ b/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
@@ -79,7 +79,10 @@ public class PrivilegeRequestBuilder {
   public PrivilegeRequestBuilder onTable(
       String dbName, String tableName, String ownerUser) {
     Preconditions.checkState(authorizable_ == null);
-    authorizable_ = authzFactory_.newTable(dbName, tableName, ownerUser);
+    // Convert 'dbName' and 'tableName' to lowercase to avoid duplicate audit log entries
+    // for the PrivilegeRequest of ALTER.
+    authorizable_ = authzFactory_.newTable(dbName.toLowerCase(), tableName.toLowerCase(),
+        ownerUser);
     return this;
   }
 
diff --git a/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java b/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java
index 7de2acb..2b4775c 100644
--- a/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java
+++ b/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java
@@ -146,6 +146,20 @@ public class RangerAuditLogTest extends AuthorizationTestBase {
       assertEquals("show partitions functional.alltypes", events.get(0).getRequestData());
     }, "show partitions functional.alltypes",
         onTable("functional", "alltypes", TPrivilegeLevel.SELECT));
+
+    // COMPUTE STATS results in two registrations of the PrivilegeRequest for ALTER.
+    // Check we do not have duplicate ALTER events when the fully-qualified table name is
+    // not in lowercase.
+    authzOk(events -> {
+      assertEquals(2, events.size());
+      assertEventEquals("@table", "alter", "functional/alltypes", 1,
+          events.get(0));
+      assertEventEquals("@table", "select", "functional/alltypes", 1,
+          events.get(1));
+      assertEquals("compute stats FUNCTIONAL.ALLTYPES", events.get(0).getRequestData());
+    }, "compute stats FUNCTIONAL.ALLTYPES",
+        onTable("functional", "alltypes", TPrivilegeLevel.ALTER,
+            TPrivilegeLevel.SELECT));
   }
 
   @Test