Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15412 )
Change subject: IMPALA-9350: Produce Ranger audits for column masking ...................................................................... Patch Set 1: (4 comments) Hi Csaba, thanks for the suggestions! I will try to see first how automate the tests and then try to figure out how easy/difficult it is to pass an instance of AuthorizationContext to analyze() in AnalysisContext.java so that we only need one flush of the audits. http://gerrit.cloudera.org:8080/#/c/15412/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/15412/1//COMMIT_MSG@17 PS1, Line 17: Testing: > We should have some kind of automatic tests, e.g. in https://github.com/apa I agree with you! I think I will have to figure out 1) how to add column masking-related policies, and 2) how to verify the corresponding audit logs have been generated (there seem to be some examples in RangerAuditLogTest.java). http://gerrit.cloudera.org:8080/#/c/15412/1//COMMIT_MSG@20 PS1, Line 20: Solr > Can you explain why Solr is relevant here? Currently if we are trying to access the Ranger audits via its web UI in our upstream development environment, we will see an error message saying that "Error loading audit logs". According to my current understanding, one of the possible reasons is that Solr is not installed in our development environment, which is required to retrieve the Ranger audits. But it could also be the case that the corresponding logs are stored somewhere in the underlying HDFS and we do not know where they are. In either case, it is not that straightforward for us to verify in our development environment that the logs are indeed generated. http://gerrit.cloudera.org:8080/#/c/15412/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/15412/1/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@303 PS1, Line 303: RangerBufferAuditHandler auditHandler = (sqlStmt == null || hostname == null) ? null : : new RangerBufferAuditHandler(sqlStmt, plugin_.getClusterName(), hostname); > An alternative to always creating a new audit handler is to create a Author Thanks Csaba! I agree with you that it is possible we only need a single flush. If I understand you correctly, you are saying that we create an instance of AuthorizationContext and pass it to https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java#L415 when we call analyze(stmtTableCache) just like what we did at https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/analysis/AnalysisContext.java#L426-L430. I will investigate how easy or how difficult it is to implement this and get back to you. http://gerrit.cloudera.org:8080/#/c/15412/1/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@347 PS1, Line 347: > nit: extra line Thanks for catching this! -- To view, visit http://gerrit.cloudera.org:8080/15412 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9d8a1181234dcef580f68f56c24ad7e962cfe58e Gerrit-Change-Number: 15412 Gerrit-PatchSet: 1 Gerrit-Owner: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Fang-Yu Rao <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Thu, 12 Mar 2020 21:15:57 +0000 Gerrit-HasComments: Yes
