You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org> on 2022/05/05 22:09:22 UTC
[Impala-ASF-CR] IMPALA-11269: Combine log entries of column accesses in the same table
Fang-Yu Rao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18498
Change subject: IMPALA-11269: Combine log entries of column accesses in the same table
......................................................................
IMPALA-11269: Combine log entries of column accesses in the same table
This patch consolidates the Ranger audit log entries of column accesses
granted by the same policy for columns in the same table when a query is
successfully authorized.
Testing:
- Revised the expected results in the existing FE tests related to
Ranger audit log.
- Added a new test case to verify we do not combine Ranger audit log
entries of column accesses granted by different policies even though
those columns belong to the same table.
Change-Id: Ib8c9522d3518f9523ab4539cdf51f5ee0b17cb7c
---
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java
M fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java
3 files changed, 142 insertions(+), 121 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/18498/1
--
To view, visit http://gerrit.cloudera.org:8080/18498
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib8c9522d3518f9523ab4539cdf51f5ee0b17cb7c
Gerrit-Change-Number: 18498
Gerrit-PatchSet: 1
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
[Impala-ASF-CR] IMPALA-11269: Combine log entries of column accesses in the same table
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18498 )
Change subject: IMPALA-11269: Combine log entries of column accesses in the same table
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/18498
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib8c9522d3518f9523ab4539cdf51f5ee0b17cb7c
Gerrit-Change-Number: 18498
Gerrit-PatchSet: 4
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 13 May 2022 00:39:43 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11269: Combine log entries of column accesses in the same table
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/18498 )
Change subject: IMPALA-11269: Combine log entries of column accesses in the same table
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18498/1/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java:
http://gerrit.cloudera.org:8080/#/c/18498/1/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@192
PS1, Line 192: ((RangerAuthorizationContext) authzCtx).consolidateAuthzEvents();
I think it's more clear to consolidate the events at the begining, i.e. at the place we generate them:
https://github.com/apache/impala/blob/1f20b7aa51b5c93dea03515423def2d800f43554/fe/src/main/java/org/apache/impala/authorization/ranger/RangerBufferAuditHandler.java#L109
https://github.com/apache/impala/blob/1f20b7aa51b5c93dea03515423def2d800f43554/fe/src/main/java/org/apache/impala/authorization/ranger/RangerBufferAuditHandler.java#L158-L166
Here is what Ranger does for Hive:
https://github.com/apache/ranger/blob/e5efe40dfa4c0e385cb04926ed230513f0ac6b0d/hive-agent/src/main/java/org/apache/ranger/authorization/hive/authorizer/RangerHiveAuditHandler.java#L200-L209
On the other hand, our current patch groups events by the table names. IIUC, the Hive audits are grouped by policy ids. I think we should switch to Hive's approach.
--
To view, visit http://gerrit.cloudera.org:8080/18498
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib8c9522d3518f9523ab4539cdf51f5ee0b17cb7c
Gerrit-Change-Number: 18498
Gerrit-PatchSet: 1
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 06 May 2022 02:57:43 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11269: Combine log entries of column accesses in the same table
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18498 )
Change subject: IMPALA-11269: Combine log entries of column accesses in the same table
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/10551/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/18498
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib8c9522d3518f9523ab4539cdf51f5ee0b17cb7c
Gerrit-Change-Number: 18498
Gerrit-PatchSet: 3
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 09 May 2022 23:59:12 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11269: Combine log entries of column accesses in the same table
Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/18498 )
Change subject: IMPALA-11269: Combine log entries of column accesses in the same table
......................................................................
IMPALA-11269: Combine log entries of column accesses in the same table
This patch consolidates the Ranger audit log entries of column accesses
granted by the same policy for columns in the same table when a query is
successfully authorized.
Testing:
- Revised the expected results in the existing FE tests related to
Ranger audit log due to this patch.
- Added a new test case to verify we do not combine Ranger audit log
entries of column accesses granted by different policies even though
those columns belong to the same table.
- Added two new test cases to verify we won't have duplicate Ranger
audit log entries if a column appears multiple times in a query.
Change-Id: Ib8c9522d3518f9523ab4539cdf51f5ee0b17cb7c
---
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java
M fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java
3 files changed, 178 insertions(+), 120 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/18498/2
--
To view, visit http://gerrit.cloudera.org:8080/18498
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib8c9522d3518f9523ab4539cdf51f5ee0b17cb7c
Gerrit-Change-Number: 18498
Gerrit-PatchSet: 2
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-11269: Combine log entries of column accesses in the same table
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18498 )
Change subject: IMPALA-11269: Combine log entries of column accesses in the same table
......................................................................
Patch Set 4: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/18498
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib8c9522d3518f9523ab4539cdf51f5ee0b17cb7c
Gerrit-Change-Number: 18498
Gerrit-PatchSet: 4
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 13 May 2022 05:08:21 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11269: Combine log entries of column accesses in the same table
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18498 )
Change subject: IMPALA-11269: Combine log entries of column accesses in the same table
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/10531/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/18498
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib8c9522d3518f9523ab4539cdf51f5ee0b17cb7c
Gerrit-Change-Number: 18498
Gerrit-PatchSet: 1
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 05 May 2022 22:29:30 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11269: Combine log entries of column accesses in the same table
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18498 )
Change subject: IMPALA-11269: Combine log entries of column accesses in the same table
......................................................................
IMPALA-11269: Combine log entries of column accesses in the same table
This patch consolidates the Ranger audit log entries of column accesses
granted by the same policy for columns in the same table when a query is
successfully authorized.
Testing:
- Revised the expected results in the existing FE tests related to
Ranger audit log due to this patch.
- Added a new test case to verify we do not combine Ranger audit log
entries of column accesses granted by different policies even though
those columns belong to the same table.
- Added two new test cases to verify we won't have duplicate Ranger
audit log entries if a column appears multiple times in a query.
Change-Id: Ib8c9522d3518f9523ab4539cdf51f5ee0b17cb7c
Reviewed-on: http://gerrit.cloudera.org:8080/18498
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java
M fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java
3 files changed, 179 insertions(+), 120 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/18498
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib8c9522d3518f9523ab4539cdf51f5ee0b17cb7c
Gerrit-Change-Number: 18498
Gerrit-PatchSet: 5
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-11269: Combine log entries of column accesses in the same table
Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/18498 )
Change subject: IMPALA-11269: Combine log entries of column accesses in the same table
......................................................................
IMPALA-11269: Combine log entries of column accesses in the same table
This patch consolidates the Ranger audit log entries of column accesses
granted by the same policy for columns in the same table when a query is
successfully authorized.
Testing:
- Revised the expected results in the existing FE tests related to
Ranger audit log due to this patch.
- Added a new test case to verify we do not combine Ranger audit log
entries of column accesses granted by different policies even though
those columns belong to the same table.
- Added two new test cases to verify we won't have duplicate Ranger
audit log entries if a column appears multiple times in a query.
Change-Id: Ib8c9522d3518f9523ab4539cdf51f5ee0b17cb7c
---
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java
M fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java
3 files changed, 179 insertions(+), 120 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/18498/3
--
To view, visit http://gerrit.cloudera.org:8080/18498
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib8c9522d3518f9523ab4539cdf51f5ee0b17cb7c
Gerrit-Change-Number: 18498
Gerrit-PatchSet: 3
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-11269: Combine log entries of column accesses in the same table
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18498 )
Change subject: IMPALA-11269: Combine log entries of column accesses in the same table
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/10548/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/18498
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib8c9522d3518f9523ab4539cdf51f5ee0b17cb7c
Gerrit-Change-Number: 18498
Gerrit-PatchSet: 2
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 09 May 2022 23:52:52 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11269: Combine log entries of column accesses in the same table
Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/18498 )
Change subject: IMPALA-11269: Combine log entries of column accesses in the same table
......................................................................
Patch Set 1:
Hi all, please let me know if you have any comment on the patch.
Thank you very much for the help!
--
To view, visit http://gerrit.cloudera.org:8080/18498
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib8c9522d3518f9523ab4539cdf51f5ee0b17cb7c
Gerrit-Change-Number: 18498
Gerrit-PatchSet: 1
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 05 May 2022 22:10:04 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11269: Combine log entries of column accesses in the same table
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/18498 )
Change subject: IMPALA-11269: Combine log entries of column accesses in the same table
......................................................................
Patch Set 3: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/18498
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib8c9522d3518f9523ab4539cdf51f5ee0b17cb7c
Gerrit-Change-Number: 18498
Gerrit-PatchSet: 3
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 10 May 2022 12:41:30 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11269: Combine log entries of column accesses in the same table
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18498 )
Change subject: IMPALA-11269: Combine log entries of column accesses in the same table
......................................................................
Patch Set 4:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/8100/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/18498
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib8c9522d3518f9523ab4539cdf51f5ee0b17cb7c
Gerrit-Change-Number: 18498
Gerrit-PatchSet: 4
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 13 May 2022 00:39:44 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11269: Combine log entries of column accesses in the same table
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/18498 )
Change subject: IMPALA-11269: Combine log entries of column accesses in the same table
......................................................................
Patch Set 1:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/18498/1/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java:
http://gerrit.cloudera.org:8080/#/c/18498/1/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@192
PS1, Line 192: ((RangerAuthorizationContext) authzCtx).consolidateAuthzEvents();
> Thanks for the pointers and feedback, Quanlong!
Thanks for digging into this! I checked some code paths in Hive & Ranger and found they do group requests for each table as you mentioned above:
https://github.com/apache/hive/blob/4c7dbd560f9034fbce77d17a82bd0a4abc3091fd/ql/src/java/org/apache/hadoop/hive/ql/security/authorization/command/CommandAuthorizerV2.java#L177-L180
https://github.com/apache/ranger/blob/a8c4b57abd11ca2742a390e4292905e34b2543bc/hive-agent/src/main/java/org/apache/ranger/authorization/hive/authorizer/RangerHiveAuthorizer.java#L1019-L1041
It's good to know that we are consistent with Hive. I think we can proceed with the current approach. :)
http://gerrit.cloudera.org:8080/#/c/18498/1/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java:
http://gerrit.cloudera.org:8080/#/c/18498/1/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java@100
PS1, Line 100: resourceType.compareTo("@column") == 0
nit: "@column".equals(resourceType)
http://gerrit.cloudera.org:8080/#/c/18498/1/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java@101
PS1, Line 101: split("/")
nit: split("/", 3)
http://gerrit.cloudera.org:8080/#/c/18498/1/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java@102
PS1, Line 102: long policyId = event.getPolicyId();
We'd better check the length of 'parsedNames' before using it. And skip illegal results with an error log.
http://gerrit.cloudera.org:8080/#/c/18498/1/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java@117
PS1, Line 117: columnName
Is it possible that we have duplicated column names here? E.g. for queries like
select min(id), max(id), avg(id) from functional.alltypes;
select min(id) from functional.alltypes union all select max(id) from functional.alltypes;
http://gerrit.cloudera.org:8080/#/c/18498/1/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java
File fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java:
http://gerrit.cloudera.org:8080/#/c/18498/1/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java@166
PS1, Line 166:
Can we add tests for duplicated columns? E.g.
select min(id), max(id), avg(id) from functional.alltypes;
select min(id) from functional.alltypes union all select max(id) from functional.alltypes;
http://gerrit.cloudera.org:8080/#/c/18498/1/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java@185
PS1, Line 185: get(1)
nit: get(0). let's keep the index align with the above line
http://gerrit.cloudera.org:8080/#/c/18498/1/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java@187
PS1, Line 187: get(2)
nit: get(1)
http://gerrit.cloudera.org:8080/#/c/18498/1/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java@189
PS1, Line 189: get(0)
nit: get(2)
--
To view, visit http://gerrit.cloudera.org:8080/18498
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib8c9522d3518f9523ab4539cdf51f5ee0b17cb7c
Gerrit-Change-Number: 18498
Gerrit-PatchSet: 1
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 09 May 2022 02:59:48 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11269: Combine log entries of column accesses in the same table
Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/18498 )
Change subject: IMPALA-11269: Combine log entries of column accesses in the same table
......................................................................
Patch Set 1:
(1 comment)
> Patch Set 1:
>
> (1 comment)
Hi all, I replied to Quanlong's comments in the previous iteration.
Let me know if you have additional ideas or suggestions about the patch or my analysis in my response to Quanlong's comments.
Thank you very much for the help!
http://gerrit.cloudera.org:8080/#/c/18498/1/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java:
http://gerrit.cloudera.org:8080/#/c/18498/1/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@192
PS1, Line 192: ((RangerAuthorizationContext) authzCtx).consolidateAuthzEvents();
> I think it's more clear to consolidate the events at the begining, i.e. at
Thanks for the pointers and feedback, Quanlong!
The current patch groups audit events by both the policy ID and the fully-qualified table name. I have also compared Hive with Impala on a Ranger-enabled cluster and found that the behavior of Hive and Impala is consistent with respect to audit event generation. That is, Hive won't consolidate audit events of different tables even though those audit events are associated with the same policy ID. This is possible since we are able to grant the privileges on multiple tables to a grantee in the same policy and a query could involve more than one table.
I try to see if it's possible to consolidate the events each time when an AuthzAuditEvent is generated. It's possible but it seems more changes have to be made. In what follows, a detailed analysis is also provided.
I took a closer look at how Hive and Impala produce the list of AuthzAuditEvent's for a given query. It looks like RangerHiveAuditHandler.java#createAuditEvents() (https://github.com/apache/ranger/blob/master/hive-agent/src/main/java/org/apache/ranger/authorization/hive/authorizer/RangerHiveAuditHandler.java#L188) and RangerBufferAuditHandler#createAuditEvents() (https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/authorization/ranger/RangerBufferAuditHandler.java#L158) are used in different contexts.
Specifically, RangerHiveAuditHandler#createAuditEvents() is called in RangerHiveAuditHandler#processResults() (https://github.com/apache/ranger/blob/master/hive-agent/src/main/java/org/apache/ranger/authorization/hive/authorizer/RangerHiveAuditHandler.java#L252-L257) to process a Collection of RangerAccessResult's produced due to the same table. I guess/think that is the reason why in RangerHiveAuditHandler#createAuditEvents() we do not need to consider whether each RangerAccessResult is associated with the same table. In practice, an administrator could grant the privileges on 2 different tables in one single Ranger policy. In such a case, when the grantee submits a query involving both tables, then RangerHiveAuditHandler#createAuditEvents() should be called once for RangerAccessResult's associated with each table.
On the other hand, even though a Collection of RangerAccessResult's is passed to RangerBufferAuditHandler#createAuditEvents(), it looks like this Collection can only contain at most one single RangerAccessResult. This is because every time when RangerAuthorizationChecker#authorizeAll() (https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java#L538) is called to authorize a request for a given resource, we call plugin_.isAccessAllowed() in RangerAuthorizationChecker#authorizeResource () (https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java#L617), which in turn calls RangerBufferAuditHandler#processResult() (https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/authorization/ranger/RangerBufferAuditHandler.java#L103) to process the RangerAccessResult for this resource. It can also be seen at https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/authorization/ranger/RangerBufferAuditHandler.java#L104 we pass a singleton List to RangerBufferAuditHandler#processResults(), which in turn passes the singleton List to RangerBufferAuditHandler#createAuditEvents(). Thus, the current implementation does not seem to allow us to consolidate RangerAccessResult's in RangerBufferAuditHandler#createAuditEvents() because of the way RangerBufferAuditHandler#processResults() is used.
If we do not want to consolidate the AuthzAuditEvent's in RangerAuthorizationChecker#postAuthorize(), it seems that the earliest point at which we could consolidate the AuthzAuditEvent's is in RangerAuthorizationChecker#authorizeAll() and RangerAuthorizationChecker#authorizeAny() when we call RangerAuthorizationChecker#updateAuditEvents() (https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java#L564) to copy the AuthzAuditEvent stored in 'tmpAuditHandler' (https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java#L551) to 'originalAuditHandler'. To consolidate AuthzAuditEvent's in RangerAuthorizationChecker#updateAuditEvents(), it may be more efficient to maintain a Map in RangerBufferAuditHandler for the currently collected AuthzAuditEvent's with the key being "<policy_id>/<database_name>/<table_name>" instead of just a List of AuthzAuditEvent's. In this way, every time we update 'originalAuditHandler', we can check whether the latest AuthzAuditEvent could be combined with some AuthzAuditEvent already added to the Map.
Another alternative I can think of is to add the RangerAccessResult in a Collection maintained in RangerAuthorizationChecker each time when plugin_.isAccessAllowed() returns (https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java#L617). When we collect all RangerAccessResult's involved in the current query, we start creating AuthzAuditEvent's and consolidate them when applicable.
Both alternatives described above seem to involve more changes to the way we create audit log entries in Impala and seem to be more complicated than this current patch.
However the code that produces AuthzAuditEvent's is indeed a bit convoluted. It's very possible that I missed something important in my analysis above and thus there could be approaches that are much cleaner. Let me know how you would like to proceed. Thanks!
--
To view, visit http://gerrit.cloudera.org:8080/18498
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib8c9522d3518f9523ab4539cdf51f5ee0b17cb7c
Gerrit-Change-Number: 18498
Gerrit-PatchSet: 1
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 06 May 2022 23:41:22 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11269: Combine log entries of column accesses in the same table
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/18498 )
Change subject: IMPALA-11269: Combine log entries of column accesses in the same table
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18498/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java:
http://gerrit.cloudera.org:8080/#/c/18498/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java@107
PS2, Line 107: LOG.error("The column resource path " + event.getResourcePath() + " could not " +
line too long (91 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/18498
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib8c9522d3518f9523ab4539cdf51f5ee0b17cb7c
Gerrit-Change-Number: 18498
Gerrit-PatchSet: 2
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 09 May 2022 23:33:14 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11269: Combine log entries of column accesses in the same table
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/18498 )
Change subject: IMPALA-11269: Combine log entries of column accesses in the same table
......................................................................
Patch Set 3: Code-Review+2
The patch LGTM. I think we are good to merge it.
--
To view, visit http://gerrit.cloudera.org:8080/18498
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib8c9522d3518f9523ab4539cdf51f5ee0b17cb7c
Gerrit-Change-Number: 18498
Gerrit-PatchSet: 3
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 12 May 2022 08:00:06 +0000
Gerrit-HasComments: No