[Impala-ASF-CR] IMPALA-8400: Implement Ranger audit event handler
Fredy Wijaya has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13309 Change subject: IMPALA-8400: Implement Ranger audit event handler .. IMPALA-8400: Implement Ranger audit event handler This patch implements Ranger audit event handler to behave similarly to the Hive/Ranger audit event handler, most notably: - Buffer the audit events during authorization and only flush them once the authorization is complete. - The audit will only add the event for the first failure. Testing: - Added test cases in RangerAuditLogTest - Ran FE tests - Ran all E2E authorization tests Change-Id: Ife93c3e708875ef5fc0117153ad8ee225a88518b --- M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java A fe/src/main/java/org/apache/impala/authorization/AuthorizationContext.java M fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java M fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java A fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java A fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaAuditHandler.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationTest.java A fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java A fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java 16 files changed, 1,403 insertions(+), 769 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/13309/4 -- To view, visit http://gerrit.cloudera.org:8080/13309 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Ife93c3e708875ef5fc0117153ad8ee225a88518b Gerrit-Change-Number: 13309 Gerrit-PatchSet: 4 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya
[Impala-ASF-CR] IMPALA-8400: Implement Ranger audit event handler
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13309 ) Change subject: IMPALA-8400: Implement Ranger audit event handler .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3224/ : 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/13309 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ife93c3e708875ef5fc0117153ad8ee225a88518b Gerrit-Change-Number: 13309 Gerrit-PatchSet: 4 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 14 May 2019 22:02:33 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8400: Implement Ranger audit event handler
Austin Nobis has posted comments on this change. (
http://gerrit.cloudera.org:8080/13309 )
Change subject: IMPALA-8400: Implement Ranger audit event handler
..
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13309/4/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
File fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java:
http://gerrit.cloudera.org:8080/#/c/13309/4/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java@435
PS4, Line 435: try {
: authzCtx = authzChecker.preAuthorize(analysisResult_,
catalog_);
: authzChecker.authorize(authzCtx, analysisResult_,
catalog_);
: } catch (AuthorizationException e) {
: authException = e;
: } finally {
: if (authzCtx != null) {
: authzChecker.postAuthorize(authzCtx, analysisResult_,
catalog_);
: }
: }
Had a discussion with Fredy because I didn't understand the purpose of having
`preAuthorize`, `authorize`, and then `postAuthorize` called sequentially.
The `BaseAuthorizationChecker` class has 2 `authorize` methods with different
signatures. The `abstract` one is the one that is being `override`'d. This is
why these 3 methods can't be merged into a single `authorize`.
Fredy and I came to an agreement that we should rename the `abstract authorize`
method to `authorizeResource` as it is an authorization that occurs per
resources, whereas the `authorize` being called here is the authorization for
the entire SQL statement.
The logic flow is as follows:
RangerAuthorizationChecker#preAuthorize
BaseAuthorizationChecker#authorize
foreach resource: RangerAuthorizationChecker#authorize (authorizeResource)
RangerAuthorizationChecker#postAuthorize
I still think the `preAuthorize`, `authorize`, `postAuthorize` sequence is a
little strange as the methods execute sequentially.
--
To view, visit http://gerrit.cloudera.org:8080/13309
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife93c3e708875ef5fc0117153ad8ee225a88518b
Gerrit-Change-Number: 13309
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Tue, 14 May 2019 22:44:14 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8400: Implement Ranger audit event handler
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/13309 )
Change subject: IMPALA-8400: Implement Ranger audit event handler
..
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/13309/4/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
File fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java:
http://gerrit.cloudera.org:8080/#/c/13309/4/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java@435
PS4, Line 435: authzChecker.authorize(analysisResult_, catalog_,
authzCtxConsumer);
: } catch (AuthorizationException e) {
: authException = e;
: }
:
: // AuthorizationExceptions take precedence over
AnalysisExceptions so as not
: // to reveal the existence/absence of objects the user is
not authorized to see.
: if (authException != null) throw authException;
: if (analysisException != null) throw analysisException;
: r
> Had a discussion with Fredy because I didn't understand the purpose of havi
Done. Rename the overloaded authorize to authorizeResource().
I removed the pre/postAutohrize(), but the interface is a bit leaky now with
Consumer since the authorize() is the one that handles
the creation of AuthorizationContext and creating the AuthorizationContext now
becomes a specific implementation of AuthorizationChecker
(BaseAuthorizationChecker in this case). Please take a look at the latest CR
and let me know what you think.
--
To view, visit http://gerrit.cloudera.org:8080/13309
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife93c3e708875ef5fc0117153ad8ee225a88518b
Gerrit-Change-Number: 13309
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Wed, 15 May 2019 01:19:41 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8400: Implement Ranger audit event handler
Fredy Wijaya has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/13309 ) Change subject: IMPALA-8400: Implement Ranger audit event handler .. IMPALA-8400: Implement Ranger audit event handler This patch implements Ranger audit event handler to behave similarly to the Hive/Ranger audit event handler, most notably: - Buffer the audit events during authorization and only flush them once the authorization is complete. - The audit will only add the event for the first failure. Testing: - Added test cases in RangerAuditLogTest - Ran FE tests - Ran all E2E authorization tests Change-Id: Ife93c3e708875ef5fc0117153ad8ee225a88518b --- M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java A fe/src/main/java/org/apache/impala/authorization/AuthorizationContext.java M fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java M fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java A fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java A fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaAuditHandler.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationTest.java A fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java A fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java 16 files changed, 1,448 insertions(+), 826 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/13309/6 -- To view, visit http://gerrit.cloudera.org:8080/13309 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ife93c3e708875ef5fc0117153ad8ee225a88518b Gerrit-Change-Number: 13309 Gerrit-PatchSet: 6 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-8400: Implement Ranger audit event handler
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13309 ) Change subject: IMPALA-8400: Implement Ranger audit event handler .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3229/ : 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/13309 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ife93c3e708875ef5fc0117153ad8ee225a88518b Gerrit-Change-Number: 13309 Gerrit-PatchSet: 6 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 15 May 2019 02:08:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8400: Implement Ranger audit event handler
Austin Nobis has posted comments on this change. (
http://gerrit.cloudera.org:8080/13309 )
Change subject: IMPALA-8400: Implement Ranger audit event handler
..
Patch Set 6:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/13309/4/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
File fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java:
http://gerrit.cloudera.org:8080/#/c/13309/4/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java@435
PS4, Line 435: authzChecker.authorize(analysisResult_, catalog_,
authzCtxConsumer);
: } catch (AuthorizationException e) {
: authException = e;
: }
:
: // AuthorizationExceptions take precedence over
AnalysisExceptions so as not
: // to reveal the existence/absence of objects the user is
not authorized to see.
: if (authException != null) throw authException;
: if (analysisException != null) throw analysisException;
: r
> Done. Rename the overloaded authorize to authorizeResource().
I think I prefer the implementation in Patch Set 4. The
preAuthorize/postAuthorize weren't actually removed in Patch Set 6 and adding
the Consumer<> makes everything even more confusing.
I believe the implementation is correct in patch set 4 so I'll +1 on the revert.
http://gerrit.cloudera.org:8080/#/c/13309/6/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java
File
fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java:
http://gerrit.cloudera.org:8080/#/c/13309/6/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java@64
PS6, Line 64: is used "show databases"
nit: is used by
--
To view, visit http://gerrit.cloudera.org:8080/13309
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife93c3e708875ef5fc0117153ad8ee225a88518b
Gerrit-Change-Number: 13309
Gerrit-PatchSet: 6
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Mon, 20 May 2019 18:14:19 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8400: Implement Ranger audit event handler
Fredy Wijaya has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/13309 ) Change subject: IMPALA-8400: Implement Ranger audit event handler .. IMPALA-8400: Implement Ranger audit event handler This patch implements Ranger audit event handler to behave similarly to the Hive/Ranger audit event handler, most notably: - Buffer the audit events during authorization and only flush them once the authorization is complete. - The audit will only add the event for the first failure. Testing: - Added test cases in RangerAuditLogTest - Ran FE tests - Ran all E2E authorization tests Change-Id: Ife93c3e708875ef5fc0117153ad8ee225a88518b --- M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java A fe/src/main/java/org/apache/impala/authorization/AuthorizationContext.java M fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java M fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java A fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java A fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaAuditHandler.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationTest.java A fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java A fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java 16 files changed, 1,405 insertions(+), 770 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/13309/7 -- To view, visit http://gerrit.cloudera.org:8080/13309 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ife93c3e708875ef5fc0117153ad8ee225a88518b Gerrit-Change-Number: 13309 Gerrit-PatchSet: 7 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-8400: Implement Ranger audit event handler
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/13309 )
Change subject: IMPALA-8400: Implement Ranger audit event handler
..
Patch Set 7:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/13309/4/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
File fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java:
http://gerrit.cloudera.org:8080/#/c/13309/4/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java@435
PS4, Line 435: try {
: authzCtx = authzChecker.preAuthorize(analysisResult_,
catalog_);
: authzChecker.authorize(authzCtx, analysisResult_,
catalog_);
: } catch (AuthorizationException e) {
: authException = e;
: } finally {
: if (authzCtx != null) {
: authzChecker.postAuthorize(authzCtx, analysisResult_,
catalog_);
: }
: }
> I think I prefer the implementation in Patch Set 4. The preAuthorize/postAu
Done
http://gerrit.cloudera.org:8080/#/c/13309/6/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java
File
fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java:
http://gerrit.cloudera.org:8080/#/c/13309/6/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java@64
PS6, Line 64: d database, table, or co
> nit: is used by
Done
--
To view, visit http://gerrit.cloudera.org:8080/13309
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife93c3e708875ef5fc0117153ad8ee225a88518b
Gerrit-Change-Number: 13309
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Tue, 21 May 2019 02:45:36 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8400: Implement Ranger audit event handler
Austin Nobis has posted comments on this change. ( http://gerrit.cloudera.org:8080/13309 ) Change subject: IMPALA-8400: Implement Ranger audit event handler .. Patch Set 7: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/13309 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ife93c3e708875ef5fc0117153ad8ee225a88518b Gerrit-Change-Number: 13309 Gerrit-PatchSet: 7 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 21 May 2019 02:45:40 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8400: Implement Ranger audit event handler
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13309 ) Change subject: IMPALA-8400: Implement Ranger audit event handler .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3302/ : 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/13309 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ife93c3e708875ef5fc0117153ad8ee225a88518b Gerrit-Change-Number: 13309 Gerrit-PatchSet: 7 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 21 May 2019 03:44:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8400: Implement Ranger audit event handler
Todd Lipcon has posted comments on this change. (
http://gerrit.cloudera.org:8080/13309 )
Change subject: IMPALA-8400: Implement Ranger audit event handler
..
Patch Set 7:
(13 comments)
http://gerrit.cloudera.org:8080/#/c/13309/7/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
File fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java:
http://gerrit.cloudera.org:8080/#/c/13309/7/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java@411
PS7, Line 411:* This method is meant for testing.
Is this _only_ for testing? Or visible for testing? I think it'd be a little
clearer to say that this is the implementation of the above function, but
providing the additional 'authzCtxConsumer'.
That said, it seems like the 'postAuthorize' hook already gets access to the
context, so couldn't you just spy on that call, eg by passing in a
Mockito.spy()-wrapped AuthorizationChecker in the tests?
http://gerrit.cloudera.org:8080/#/c/13309/7/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
File fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java:
http://gerrit.cloudera.org:8080/#/c/13309/7/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java@42
PS7, Line 42: AuthorizationContext preAuthorize(AnalysisResult result,
FeCatalog catalog)
It doesn't look like any of the implementations of preAuthorize() actually rely
on the parameters here. In fact, they basically just delegate to
'createAuthorizationContext()', with the only difference in behavior being
whether or not to enable auditing. What if you just moved the
'createAuthorizationContext()' function up into this interface, and added a
boolean parameter like 'enableAudits'?
http://gerrit.cloudera.org:8080/#/c/13309/7/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java@57
PS7, Line 57: postAuthorize
Doesn't look like 'result' or 'catalog' are used here in any of the impls either
http://gerrit.cloudera.org:8080/#/c/13309/7/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java
File
fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java:
http://gerrit.cloudera.org:8080/#/c/13309/7/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java@98
PS7, Line 98: endTime
this variable is misnamed -- should be something like 'durationMs'
http://gerrit.cloudera.org:8080/#/c/13309/7/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/13309/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@82
PS7, Line 82: authorization SQL statement
not sure what this means. Isn't this used for _all_ SQL statements? This makes
it sound like it's used only for GRANT/REVOKE
http://gerrit.cloudera.org:8080/#/c/13309/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@250
PS7, Line 250: curcuit
typo
http://gerrit.cloudera.org:8080/#/c/13309/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaAuditHandler.java
File
fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaAuditHandler.java:
http://gerrit.cloudera.org:8080/#/c/13309/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaAuditHandler.java@36
PS7, Line 36: */
Worth adding to the Javadoc that this is scoped once-per-statement, rather than
being a long-lived multi-threaded thing
http://gerrit.cloudera.org:8080/#/c/13309/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaAuditHandler.java@37
PS7, Line 37: public class RangerImpalaAuditHandler extends
RangerDefaultAuditHandler
Would it be possible to make this directly implement
RangerAccessResultProcessor and _wrap_ another instance (typically
RangerDefaultAuditHandler) instead of extending it? That would make the idea of
testing via spy/mock a bit easier elsewhere -- you just need to pass in a mock
RangerAccessResultProcessor instead of DefaultAuditHandler
http://gerrit.cloudera.org:8080/#/c/13309/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaAuditHandler.java@38
PS7, Line 38: implements AutoCloseable {
Related to the above comment, I see that this is used in some cases where it
actually is just a temporary holding area for a list of events, and not meant
to be flushed. If you make it AutoCloseable, I'm guessing that findbugs and
other static checkers are going to complain about usages that don't close it.
How about the following idea:
- make this implement RangerAccessResultProcessor
- make the ctor take no "wrapped class"
- add an explicit "flushTo(RangerAccessResultProcessor consumer)" method
- add a little wrapper like:
AutoCloseable autoFlushTo(Range
[Impala-ASF-CR] IMPALA-8400: Implement Ranger audit event handler
Fredy Wijaya has uploaded a new patch set (#9). ( http://gerrit.cloudera.org:8080/13309 ) Change subject: IMPALA-8400: Implement Ranger audit event handler .. IMPALA-8400: Implement Ranger audit event handler This patch implements Ranger audit event handler to behave similarly to the Hive/Ranger audit event handler, most notably: - Buffer the audit events during authorization and only flush them once the authorization is complete. - The audit will only add the event for the first failure. - Create an audit event handler per statement. Testing: - Added test cases in RangerAuditLogTest - Ran FE tests - Ran all E2E authorization tests Change-Id: Ife93c3e708875ef5fc0117153ad8ee225a88518b --- M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java A fe/src/main/java/org/apache/impala/authorization/AuthorizationContext.java M fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java M fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java A fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java A fe/src/main/java/org/apache/impala/authorization/ranger/RangerBufferAuditHandler.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationTest.java A fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java A fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java 16 files changed, 1,340 insertions(+), 769 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/13309/9 -- To view, visit http://gerrit.cloudera.org:8080/13309 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ife93c3e708875ef5fc0117153ad8ee225a88518b Gerrit-Change-Number: 13309 Gerrit-PatchSet: 9 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-8400: Implement Ranger audit event handler
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/13309 )
Change subject: IMPALA-8400: Implement Ranger audit event handler
..
Patch Set 9:
(13 comments)
http://gerrit.cloudera.org:8080/#/c/13309/7/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java
File fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java:
http://gerrit.cloudera.org:8080/#/c/13309/7/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java@411
PS7, Line 411:* This method is used by {@link
#analyzeAndAuthorize(StatementBase, StmtTableCache,
> Is this _only_ for testing? Or visible for testing? I think it'd be a littl
The comment is wrong. It is visible for testing and not only for testing. Will
update the comment
The analyzeAndAuthorize is called by
https://github.com/apache/impala/blob/f9bf62eefab7fb807f4e5d6900064b612b455a5e/fe/src/test/java/org/apache/impala/common/FrontendTestBase.java#L302.
So it's easier to just extend method to be more testable without using any
mocking framework and it feels a bit more functional :)
http://gerrit.cloudera.org:8080/#/c/13309/7/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
File fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java:
http://gerrit.cloudera.org:8080/#/c/13309/7/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java@42
PS7, Line 42: default AuthorizationContext preAuthorize()
> It doesn't look like any of the implementations of preAuthorize() actually
My original idea was for the case where a different implementation needed
access to the AnalysisResult and FeCatalog, but I guess we can just pass what
we need right now for this implementation.
http://gerrit.cloudera.org:8080/#/c/13309/7/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java@57
PS7, Line 57: is method is
> Doesn't look like 'result' or 'catalog' are used here in any of the impls e
Removed. Done.
http://gerrit.cloudera.org:8080/#/c/13309/7/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java
File
fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java:
http://gerrit.cloudera.org:8080/#/c/13309/7/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java@98
PS7, Line 98: void au
> this variable is misnamed -- should be something like 'durationMs'
Done
http://gerrit.cloudera.org:8080/#/c/13309/7/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/13309/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@82
PS7, Line 82: that goes through {@link au
> not sure what this means. Isn't this used for _all_ SQL statements? This ma
There are certain things where we don't want to audit the log, basically for
filtering out unauthorized columns, for example if we have a table:
create table t(i int, j int);
grant select(i) on table t to user foobar;
describe t; --> the describe will have an audit log
checking for whether i and j needs to be filtered out from the column output
will not be logged.
https://github.com/apache/impala/blob/7af981f7afdd25c6e0ed7e86c21bbc7710098a9e/fe/src/main/java/org/apache/impala/service/Frontend.java#L971-L984
Similarly for "show databases", "show tables", etc.
I will update the comment.
http://gerrit.cloudera.org:8080/#/c/13309/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@250
PS7, Line 250: s = tmp
> typo
Done
http://gerrit.cloudera.org:8080/#/c/13309/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaAuditHandler.java
File
fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaAuditHandler.java:
http://gerrit.cloudera.org:8080/#/c/13309/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaAuditHandler.java@36
PS7, Line 36:
> Worth adding to the Javadoc that this is scoped once-per-statement, rather
Done
http://gerrit.cloudera.org:8080/#/c/13309/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaAuditHandler.java@37
PS7, Line 37:
> Would it be possible to make this directly implement RangerAccessResultProc
Sure, composition is better than inheritance. Done.
http://gerrit.cloudera.org:8080/#/c/13309/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaAuditHandler.java@38
PS7, Line 38:
> Related to the above comment, I see that this is used in some cases where i
I implemented something similar to your idea. LMK what you think.
http://gerrit.cloudera.org:8080/#/c/13309/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaAuditHandler.java@56
PS7, Line 56:
> typo
Done
http://gerrit.cloudera.org:8080/#/c/13309/7/fe/src/main/ja
[Impala-ASF-CR] IMPALA-8400: Implement Ranger audit event handler
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13309 ) Change subject: IMPALA-8400: Implement Ranger audit event handler .. Patch Set 9: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3310/ : 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/13309 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ife93c3e708875ef5fc0117153ad8ee225a88518b Gerrit-Change-Number: 13309 Gerrit-PatchSet: 9 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 21 May 2019 19:47:18 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8400: Implement Ranger audit event handler
Fredy Wijaya has uploaded a new patch set (#11). ( http://gerrit.cloudera.org:8080/13309 ) Change subject: IMPALA-8400: Implement Ranger audit event handler .. IMPALA-8400: Implement Ranger audit event handler This patch implements Ranger audit event handler to behave similarly to the Hive/Ranger audit event handler, most notably: - Buffer the audit events during authorization and only flush them once the authorization is complete. - The audit will only add the event for the first failure. - Create an audit event handler per statement. Testing: - Added test cases in RangerAuditLogTest - Ran FE tests - Ran all E2E authorization tests Change-Id: Ife93c3e708875ef5fc0117153ad8ee225a88518b --- M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java A fe/src/main/java/org/apache/impala/authorization/AuthorizationContext.java M fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java M fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java A fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java A fe/src/main/java/org/apache/impala/authorization/ranger/RangerBufferAuditHandler.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationTest.java A fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java A fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java 16 files changed, 1,316 insertions(+), 768 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/13309/11 -- To view, visit http://gerrit.cloudera.org:8080/13309 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ife93c3e708875ef5fc0117153ad8ee225a88518b Gerrit-Change-Number: 13309 Gerrit-PatchSet: 11 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-8400: Implement Ranger audit event handler
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13309 ) Change subject: IMPALA-8400: Implement Ranger audit event handler .. Patch Set 11: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3312/ : 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/13309 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ife93c3e708875ef5fc0117153ad8ee225a88518b Gerrit-Change-Number: 13309 Gerrit-PatchSet: 11 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 21 May 2019 22:36:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8400: Implement Ranger audit event handler
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/13309 ) Change subject: IMPALA-8400: Implement Ranger audit event handler .. Patch Set 11: (6 comments) Thanks for making that refactor. Liking the new test setup much better. A few small remaining suggestions, but I think this is pretty close. http://gerrit.cloudera.org:8080/#/c/13309/11/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java File fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java: http://gerrit.cloudera.org:8080/#/c/13309/11/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java@44 PS11, Line 44: return createAuthorizationContext(); Could we get rid of preAuthorize entirely if we add an argument to 'createAuthorizationContext' like: AuthorizationContext createAuthorizationContext(boolean doAudits); where the default implementation ignores 'doAudits', but the Ranger implementation uses that to determine whether to set up the audit event buffer? That way we have a clear 'flow' that is always 'createAuthorizationContext(bool) -> authorize -> postAuthorize'. In the current state of the patch there are some paths where we use that flow and others where we use preAuthorize() -> authorize -> postAuthorize, with a slightly non-obvious side effect that one flow causes audits and the other doesn't. http://gerrit.cloudera.org:8080/#/c/13309/11/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/13309/11/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java@29 PS11, Line 29: to means nit: "to mean" or "meaning" http://gerrit.cloudera.org:8080/#/c/13309/11/fe/src/main/java/org/apache/impala/authorization/ranger/RangerBufferAuditHandler.java File fe/src/main/java/org/apache/impala/authorization/ranger/RangerBufferAuditHandler.java: http://gerrit.cloudera.org:8080/#/c/13309/11/fe/src/main/java/org/apache/impala/authorization/ranger/RangerBufferAuditHandler.java@112 PS11, Line 112: // If there's at least one access denied result, we want to only log the first per comment elsewhere, I don't think this optimization is worth the code complexity. Let's just do the filtering / "first failure" handling in flush(), or in getAuthzEvents() or somesuch if it's necessary to get the filtered results for tests. Otherwise we have more or less the same logic in two places which gets a bit confusing as to whether they compose in the expected way. http://gerrit.cloudera.org:8080/#/c/13309/11/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/13309/11/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java@89 PS11, Line 89: assertEquals(1, events.get(1).getAccessResult()); this is encompassed by the previous line, no? http://gerrit.cloudera.org:8080/#/c/13309/11/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java@156 PS11, Line 156: consumer mind renaming this to 'resultChecker' so it's clear what should be done with the consumed list? http://gerrit.cloudera.org:8080/#/c/13309/11/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java@164 PS11, Line 164: consumer same -- To view, visit http://gerrit.cloudera.org:8080/13309 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ife93c3e708875ef5fc0117153ad8ee225a88518b Gerrit-Change-Number: 13309 Gerrit-PatchSet: 11 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 23 May 2019 05:09:27 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8400: Implement Ranger audit event handler
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/13309 )
Change subject: IMPALA-8400: Implement Ranger audit event handler
..
Patch Set 12:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/13309/11/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
File fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java:
http://gerrit.cloudera.org:8080/#/c/13309/11/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java@44
PS11, Line 44:
> Could we get rid of preAuthorize entirely if we add an argument to 'createA
Done
http://gerrit.cloudera.org:8080/#/c/13309/11/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/13309/11/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java@29
PS11, Line 29: meaning
> nit: "to mean" or "meaning"
Done
http://gerrit.cloudera.org:8080/#/c/13309/11/fe/src/main/java/org/apache/impala/authorization/ranger/RangerBufferAuditHandler.java
File
fe/src/main/java/org/apache/impala/authorization/ranger/RangerBufferAuditHandler.java:
http://gerrit.cloudera.org:8080/#/c/13309/11/fe/src/main/java/org/apache/impala/authorization/ranger/RangerBufferAuditHandler.java@112
PS11, Line 112: }
> per comment elsewhere, I don't think this optimization is worth the code co
Done
http://gerrit.cloudera.org:8080/#/c/13309/11/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/13309/11/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java@89
PS11, Line 89: }, "create table functional.new_table(i int) locati
> this is encompassed by the previous line, no?
Good catch. Done.
http://gerrit.cloudera.org:8080/#/c/13309/11/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java@156
PS11, Line 156: n {
> mind renaming this to 'resultChecker' so it's clear what should be done wit
Done
http://gerrit.cloudera.org:8080/#/c/13309/11/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java@164
PS11, Line 164:
> same
Done
--
To view, visit http://gerrit.cloudera.org:8080/13309
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ife93c3e708875ef5fc0117153ad8ee225a88518b
Gerrit-Change-Number: 13309
Gerrit-PatchSet: 12
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Thu, 23 May 2019 16:31:31 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8400: Implement Ranger audit event handler
Fredy Wijaya has uploaded a new patch set (#12). ( http://gerrit.cloudera.org:8080/13309 ) Change subject: IMPALA-8400: Implement Ranger audit event handler .. IMPALA-8400: Implement Ranger audit event handler This patch implements Ranger audit event handler to behave similarly to the Hive/Ranger audit event handler, most notably: - Buffer the audit events during authorization and only flush them once the authorization is complete. - The audit will only add the event for the first failure. - Create an audit event handler per statement. Testing: - Added test cases in RangerAuditLogTest - Ran FE tests - Ran all E2E authorization tests Change-Id: Ife93c3e708875ef5fc0117153ad8ee225a88518b --- M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java A fe/src/main/java/org/apache/impala/authorization/AuthorizationContext.java M fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java M fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java A fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java A fe/src/main/java/org/apache/impala/authorization/ranger/RangerBufferAuditHandler.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationTest.java A fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java A fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java 16 files changed, 1,299 insertions(+), 768 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/09/13309/12 -- To view, visit http://gerrit.cloudera.org:8080/13309 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ife93c3e708875ef5fc0117153ad8ee225a88518b Gerrit-Change-Number: 13309 Gerrit-PatchSet: 12 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-8400: Implement Ranger audit event handler
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13309 ) Change subject: IMPALA-8400: Implement Ranger audit event handler .. Patch Set 12: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3357/ : 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/13309 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ife93c3e708875ef5fc0117153ad8ee225a88518b Gerrit-Change-Number: 13309 Gerrit-PatchSet: 12 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 23 May 2019 17:11:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8400: Implement Ranger audit event handler
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/13309 ) Change subject: IMPALA-8400: Implement Ranger audit event handler .. Patch Set 12: Code-Review+2 Thanks for the patience with the reviews. Happy with how it turned out! -- To view, visit http://gerrit.cloudera.org:8080/13309 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ife93c3e708875ef5fc0117153ad8ee225a88518b Gerrit-Change-Number: 13309 Gerrit-PatchSet: 12 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 24 May 2019 04:25:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8400: Implement Ranger audit event handler
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13309 ) Change subject: IMPALA-8400: Implement Ranger audit event handler .. Patch Set 13: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4312/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/13309 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ife93c3e708875ef5fc0117153ad8ee225a88518b Gerrit-Change-Number: 13309 Gerrit-PatchSet: 13 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 24 May 2019 04:26:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8400: Implement Ranger audit event handler
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13309 ) Change subject: IMPALA-8400: Implement Ranger audit event handler .. Patch Set 13: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13309 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ife93c3e708875ef5fc0117153ad8ee225a88518b Gerrit-Change-Number: 13309 Gerrit-PatchSet: 13 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 24 May 2019 04:25:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8400: Implement Ranger audit event handler
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13309 ) Change subject: IMPALA-8400: Implement Ranger audit event handler .. IMPALA-8400: Implement Ranger audit event handler This patch implements Ranger audit event handler to behave similarly to the Hive/Ranger audit event handler, most notably: - Buffer the audit events during authorization and only flush them once the authorization is complete. - The audit will only add the event for the first failure. - Create an audit event handler per statement. Testing: - Added test cases in RangerAuditLogTest - Ran FE tests - Ran all E2E authorization tests Change-Id: Ife93c3e708875ef5fc0117153ad8ee225a88518b Reviewed-on: http://gerrit.cloudera.org:8080/13309 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java A fe/src/main/java/org/apache/impala/authorization/AuthorizationContext.java M fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java M fe/src/main/java/org/apache/impala/authorization/NoopAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java A fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationContext.java A fe/src/main/java/org/apache/impala/authorization/ranger/RangerBufferAuditHandler.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java M fe/src/test/java/org/apache/impala/authorization/AuthorizationTest.java A fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java A fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java 16 files changed, 1,299 insertions(+), 768 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/13309 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: Ife93c3e708875ef5fc0117153ad8ee225a88518b Gerrit-Change-Number: 13309 Gerrit-PatchSet: 14 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-8400: Implement Ranger audit event handler
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13309 ) Change subject: IMPALA-8400: Implement Ranger audit event handler .. Patch Set 13: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/13309 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ife93c3e708875ef5fc0117153ad8ee225a88518b Gerrit-Change-Number: 13309 Gerrit-PatchSet: 13 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 24 May 2019 10:32:21 + Gerrit-HasComments: No
