Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14894 )
Change subject: IMPALA-9009: Core support for Ranger column masking ...................................................................... Patch Set 11: (2 comments) Thanks for looking into the tests, Fang-Yu! I also updated the design doc for this patch: https://docs.google.com/document/d/1GC7au6F5Snp8zQisRopOhKSjKsI1XPPg8S2foQxfJrA/edit?usp=sharing Hope it can ease the review process! http://gerrit.cloudera.org:8080/#/c/14894/11/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java File fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java: http://gerrit.cloudera.org:8080/#/c/14894/11/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java@2918 PS11, Line 2918: .ok(onServer(TPrivilegeLevel.ALL)) I'm not sure whether we should delete this test (testColumnMaskEnabled). The main purpose of it is to verify the error message of Ranger column masking not supported. As now it's supported, looks like we don't have any error messages to verify. > On the other hand, I also found that any server-wide privilege other than > 'TPrivilegeLevel.ALL' and 'TPrivilegeLevel.OWNER' is insufficient to pass the > authorization for the query above, although I am not very sure if this is > also an expected behavior. Just found that even without any privileges the current user can pass these since it's the owner. Not sure how to make it failed as you mentioned. Do I miss anything?... http://gerrit.cloudera.org:8080/#/c/14894/11/tests/authorization/test_ranger.py File tests/authorization/test_ranger.py: http://gerrit.cloudera.org:8080/#/c/14894/11/tests/authorization/test_ranger.py@838 PS11, Line 838: finally: > I was wondering if it would be better to remove from the user 'user' the pr Good point! Added the REVOKE. For remaining empty policies after REVOKE, it's due to Ranger policies are resource based. Each policy corresponds to a resource, e.g. database db1. Revoking a privilege on this resource just removes an item of 'Allow Conditions' in the corresponding policy. The policy will be reused in later GRANT/REVOKE statements on this resource. So it's expected to have empty policies. -- To view, visit http://gerrit.cloudera.org:8080/14894 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4cad60e0e69ea573b7ecfc011b142c46ef52ed61 Gerrit-Change-Number: 14894 Gerrit-PatchSet: 11 Gerrit-Owner: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Fang-Yu Rao <fangyu....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Kurt Deschler <kdesc...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Vihang Karajgaonkar <vih...@cloudera.com> Gerrit-Comment-Date: Sun, 05 Jan 2020 12:57:40 +0000 Gerrit-HasComments: Yes
