You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Fredy Wijaya (Code Review)" <ge...@cloudera.org> on 2019/06/26 23:40:00 UTC

[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event. 

Fredy Wijaya has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13744


Change subject: IMPALA-8716: Log a group of privileges into a single audit event.
......................................................................

IMPALA-8716: Log a group of privileges into a single audit event.

This patch updates the audit log handler to group a privilege that
consists of multiple privileges into a single audit event.

For example if we run "show partitions foo.bar" and we have
SELECT privilege on table "foo.bar", before this patch, we would be
creating 2 audit events:
- Attempt to check if there's INSERT privilege on table "foo.bar"
  Result: denied, access type: insert, resource: foo.bar
- Attempt to check if there's SELECT privilege on table "foo.bar"
  Result: allowed, access type: select, resource: foo.bar

After this patch, we will only create a single audit event, e.g.
Result: allowed, access type: view_metadata, resource: foo.bar

Testing:
- Updated tests in RangerAuditLogTest
- Ran FE tests

Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca
---
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java
2 files changed, 81 insertions(+), 20 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/13744/1
-- 
To view, visit http://gerrit.cloudera.org:8080/13744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca
Gerrit-Change-Number: 13744
Gerrit-PatchSet: 1
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>

[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.

Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13744 )

Change subject: IMPALA-8716: Log a group of privileges into a single audit event.
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13744/2/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/13744/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@408
PS2, Line 408:     RangerAccessResult result = plugin_.isAccessAllowed(request, auditHandler);
> Instead of modifying entries in a tmpAuditHandler, can we make it simple by
Tthe plugin._isAccessAllowed() does some logic that passes the correct RangerAccessResult including populating some default values. Calling it manually means we need to always keep the logic similar to the one implemented by isAccessAllowed.



-- 
To view, visit http://gerrit.cloudera.org:8080/13744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca
Gerrit-Change-Number: 13744
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 01 Jul 2019 14:16:22 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13744 )

Change subject: IMPALA-8716: Log a group of privileges into a single audit event.
......................................................................


Patch Set 3:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3793/ : 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/13744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca
Gerrit-Change-Number: 13744
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 01 Jul 2019 20:05:02 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.

Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13744 )

Change subject: IMPALA-8716: Log a group of privileges into a single audit event.
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13744/2/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/13744/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@408
PS2, Line 408:     RangerAccessResult result = plugin_.isAccessAllowed(request, auditHandler);
Instead of modifying entries in a tmpAuditHandler, can we make it simple by calling auditHandler.processEvents() ourselves (with an additional context of whether it is an implied privilege or not? Something like,

RangerAccessResult result = plugin_.isAccessAllowed(request, null);
authzCtx.getAuditHandler().processResultNew(result, privilege.isAny, originalPriv...) ?

Does that not work for some reason?



-- 
To view, visit http://gerrit.cloudera.org:8080/13744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca
Gerrit-Change-Number: 13744
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 28 Jun 2019 22:41:51 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.

Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13744 )

Change subject: IMPALA-8716: Log a group of privileges into a single audit event.
......................................................................


Patch Set 3: Code-Review+2

(2 comments)

http://gerrit.cloudera.org:8080/#/c/13744/2/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/13744/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@319
PS2, Line 319:      null : new RangerBufferAuditHandler(originalAuditHandler);
             :     for (Privilege impliedPrivilege: privilege.getImpliedPrivileges()) {
             :       if (authorizeResource(authzCtx, resource, user, impliedPrivilege,
             :           tmpAuditHandler)) {
             :         authorized = true;
> Good idea about copy constructor. Done.
I think you missed the comment.


http://gerrit.cloudera.org:8080/#/c/13744/2/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/13744/2/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java@141
PS2, Line 141: 1
> Those test cases have already been been covered:
Ah, i didn't see the other new additions in this file below, my bad.



-- 
To view, visit http://gerrit.cloudera.org:8080/13744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca
Gerrit-Change-Number: 13744
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 01 Jul 2019 20:07:03 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.

Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/13744 )

Change subject: IMPALA-8716: Log a group of privileges into a single audit event.
......................................................................

IMPALA-8716: Log a group of privileges into a single audit event.

This patch updates the audit log handler to group a privilege that
consists of multiple privileges into a single audit event.

For example if we run "show partitions foo.bar" and we have
SELECT privilege on table "foo.bar", before this patch, we would be
creating 2 audit events:
- Attempt to check if there's INSERT privilege on table "foo.bar"
  Result: denied, access type: insert, resource: foo.bar
- Attempt to check if there's SELECT privilege on table "foo.bar"
  Result: allowed, access type: select, resource: foo.bar

After this patch, we will only create a single audit event, e.g.
Result: allowed, access type: view_metadata, resource: foo.bar

Testing:
- Updated tests in RangerAuditLogTest
- Ran FE tests

Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca
---
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerBufferAuditHandler.java
M fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java
3 files changed, 95 insertions(+), 24 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/13744/3
-- 
To view, visit http://gerrit.cloudera.org:8080/13744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca
Gerrit-Change-Number: 13744
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.

Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13744 )

Change subject: IMPALA-8716: Log a group of privileges into a single audit event.
......................................................................


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/13744/2/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/13744/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@318
PS2, Line 318: fferAu
> nit: authorized ? (same below)
Done


http://gerrit.cloudera.org:8080/#/c/13744/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@319
PS2, Line 319:      null : new RangerBufferAuditHandler(originalAuditHandler);
             :     for (Privilege impliedPrivilege: privilege.getImpliedPrivileges()) {
             :       if (authorizeResource(authzCtx, resource, user, impliedPrivilege,
             :           tmpAuditHandler)) {
             :         authorized = true;
> I think this could use a comment. Also, create a copy c'tor? (same below)
Good idea about copy constructor. Done.


http://gerrit.cloudera.org:8080/#/c/13744/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@385
PS2, Line 385:       RangerBufferAuditHandler auditHandler) throws InternalException {
> just checking, is the accessResult automatically inferred from newAuditEven
Yeah since we're assigning newAuditEvent to an existing instance and mutating it, so it will have the accessResult.


http://gerrit.cloudera.org:8080/#/c/13744/2/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/13744/2/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java@141
PS2, Line 141: 1
> Mind adding a couple more tests?
Those test cases have already been been covered:

accessResult = 0, isAny = true in L193-L199
accessResult = 1, isAny = false in L63-L136 --> isAny = false is basically all privileges in https://github.com/apache/impala/blob/c353cf7a648651244ac39677d0cb028e704281d0/fe/src/main/java/org/apache/impala/authorization/Privilege.java#L28-L35 (anyOf_ = false)



-- 
To view, visit http://gerrit.cloudera.org:8080/13744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca
Gerrit-Change-Number: 13744
Gerrit-PatchSet: 3
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 01 Jul 2019 19:23:20 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13744 )

Change subject: IMPALA-8716: Log a group of privileges into a single audit event.
......................................................................


Patch Set 6: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/13744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca
Gerrit-Change-Number: 13744
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 01 Jul 2019 21:41:53 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.

Posted by "Bharath Vissapragada (Code Review)" <ge...@cloudera.org>.
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/13744 )

Change subject: IMPALA-8716: Log a group of privileges into a single audit event.
......................................................................


Patch Set 2:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/13744/2/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/13744/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@318
PS2, Line 318: result
nit: authorized ? (same below)


http://gerrit.cloudera.org:8080/#/c/13744/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@319
PS2, Line 319:  RangerBufferAuditHandler originalAuditHandler = authzCtx.getAuditHandler();
             :     RangerBufferAuditHandler tmpAuditHandler =
             :         originalAuditHandler == null ? null : new RangerBufferAuditHandler(
             :         originalAuditHandler.getSqlStmt(), originalAuditHandler.getClusterName(),
             :         originalAuditHandler.getClientIp());
I think this could use a comment. Also, create a copy c'tor? (same below)


http://gerrit.cloudera.org:8080/#/c/13744/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@385
PS2, Line 385:       dest.getAuthzEvents().add(newAuditEvent);
just checking, is the accessResult automatically inferred from newAuditEvent?


http://gerrit.cloudera.org:8080/#/c/13744/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@408
PS2, Line 408:     RangerAccessResult result = plugin_.isAccessAllowed(request, auditHandler);
> Tthe plugin._isAccessAllowed() does some logic that passes the correct Rang
I see, ok!


http://gerrit.cloudera.org:8080/#/c/13744/2/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/13744/2/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java@141
PS2, Line 141: 1
Mind adding a couple more tests?

- accessResult = 0 isAny = true
- accessResult = 1 isAny = false



-- 
To view, visit http://gerrit.cloudera.org:8080/13744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca
Gerrit-Change-Number: 13744
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 01 Jul 2019 18:24:04 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.

Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13744 )

Change subject: IMPALA-8716: Log a group of privileges into a single audit event.
......................................................................


Patch Set 5: Code-Review+2

Carry Bharath's +2.


-- 
To view, visit http://gerrit.cloudera.org:8080/13744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca
Gerrit-Change-Number: 13744
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 01 Jul 2019 21:41:39 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13744 )

Change subject: IMPALA-8716: Log a group of privileges into a single audit event.
......................................................................


Patch Set 5:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3796/ : 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/13744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca
Gerrit-Change-Number: 13744
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 01 Jul 2019 22:21:49 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13744 )

Change subject: IMPALA-8716: Log a group of privileges into a single audit event.
......................................................................


Patch Set 6: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/13744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca
Gerrit-Change-Number: 13744
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 02 Jul 2019 03:26:07 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13744 )

Change subject: IMPALA-8716: Log a group of privileges into a single audit event.
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3762/ : 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/13744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca
Gerrit-Change-Number: 13744
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 27 Jun 2019 03:02:10 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13744 )

Change subject: IMPALA-8716: Log a group of privileges into a single audit event.
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/3760/ : 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/13744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca
Gerrit-Change-Number: 13744
Gerrit-PatchSet: 1
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Thu, 27 Jun 2019 00:28:08 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.

Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/13744 )

Change subject: IMPALA-8716: Log a group of privileges into a single audit event.
......................................................................

IMPALA-8716: Log a group of privileges into a single audit event.

This patch updates the audit log handler to group a privilege that
consists of multiple privileges into a single audit event.

For example if we run "show partitions foo.bar" and we have
SELECT privilege on table "foo.bar", before this patch, we would be
creating 2 audit events:
- Attempt to check if there's INSERT privilege on table "foo.bar"
  Result: denied, access type: insert, resource: foo.bar
- Attempt to check if there's SELECT privilege on table "foo.bar"
  Result: allowed, access type: select, resource: foo.bar

After this patch, we will only create a single audit event, e.g.
Result: allowed, access type: view_metadata, resource: foo.bar

Testing:
- Updated tests in RangerAuditLogTest
- Ran FE tests

Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca
---
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java
2 files changed, 87 insertions(+), 20 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/13744/2
-- 
To view, visit http://gerrit.cloudera.org:8080/13744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca
Gerrit-Change-Number: 13744
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.

Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13744 )

Change subject: IMPALA-8716: Log a group of privileges into a single audit event.
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/13744/2/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/13744/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@319
PS2, Line 319:  // so that we know all the audit events generated by the temporary audit
             :     // handler are contained to the given resource.
             :     RangerBufferAuditHandler tmpAuditHandler = originalAuditHandler == null ?
             :         null : new RangerBufferAuditHandler(originalAuditHandler);
             :     for (Privilege impliedPrivilege: privile
> I think you missed the comment.
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/13744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca
Gerrit-Change-Number: 13744
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 01 Jul 2019 21:41:32 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13744 )

Change subject: IMPALA-8716: Log a group of privileges into a single audit event.
......................................................................


Patch Set 6:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4573/ DRY_RUN=false


-- 
To view, visit http://gerrit.cloudera.org:8080/13744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca
Gerrit-Change-Number: 13744
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Mon, 01 Jul 2019 21:41:54 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.

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/13744 )

Change subject: IMPALA-8716: Log a group of privileges into a single audit event.
......................................................................

IMPALA-8716: Log a group of privileges into a single audit event.

This patch updates the audit log handler to group a privilege that
consists of multiple privileges into a single audit event.

For example if we run "show partitions foo.bar" and we have
SELECT privilege on table "foo.bar", before this patch, we would be
creating 2 audit events:
- Attempt to check if there's INSERT privilege on table "foo.bar"
  Result: denied, access type: insert, resource: foo.bar
- Attempt to check if there's SELECT privilege on table "foo.bar"
  Result: allowed, access type: select, resource: foo.bar

After this patch, we will only create a single audit event, e.g.
Result: allowed, access type: view_metadata, resource: foo.bar

Testing:
- Updated tests in RangerAuditLogTest
- Ran FE tests

Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca
Reviewed-on: http://gerrit.cloudera.org:8080/13744
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/RangerBufferAuditHandler.java
M fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java
3 files changed, 101 insertions(+), 24 deletions(-)

Approvals:
  Impala Public Jenkins: Looks good to me, approved; Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/13744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca
Gerrit-Change-Number: 13744
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[Impala-ASF-CR] IMPALA-8716: Log a group of privileges into a single audit event.

Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/13744 )

Change subject: IMPALA-8716: Log a group of privileges into a single audit event.
......................................................................

IMPALA-8716: Log a group of privileges into a single audit event.

This patch updates the audit log handler to group a privilege that
consists of multiple privileges into a single audit event.

For example if we run "show partitions foo.bar" and we have
SELECT privilege on table "foo.bar", before this patch, we would be
creating 2 audit events:
- Attempt to check if there's INSERT privilege on table "foo.bar"
  Result: denied, access type: insert, resource: foo.bar
- Attempt to check if there's SELECT privilege on table "foo.bar"
  Result: allowed, access type: select, resource: foo.bar

After this patch, we will only create a single audit event, e.g.
Result: allowed, access type: view_metadata, resource: foo.bar

Testing:
- Updated tests in RangerAuditLogTest
- Ran FE tests

Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca
---
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerBufferAuditHandler.java
M fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java
3 files changed, 101 insertions(+), 24 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/44/13744/5
-- 
To view, visit http://gerrit.cloudera.org:8080/13744
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib53102bc1ceaf9d62544090dc00f3231fae0efca
Gerrit-Change-Number: 13744
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bh...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>