Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/23569 )
Change subject: IMPALA-14507: Register column-level privilege requests for INSERT ...................................................................... Patch Set 24: (4 comments) I have tried to address Csaba's and Quanlong's comments. Let me know if there are additional ideas. http://gerrit.cloudera.org:8080/#/c/23569/23//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/23569/23//COMMIT_MSG@9 PS23, Line 9: This patch registers column-level privilege requests for columns : involved in the INSERT statement so that the requesting user does not : need to be granted the INSERT privilege on the entire table. > What happens if the DML inserts into all columns? Will Impala register requ Yes. In this case, Impala will register 1 table-level INSERT request, and 1 column-level INSERT request for each column in the table. This is consistent with what we have seen in the test case at https://gerrit.cloudera.org/c/23569/23/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java#169. http://gerrit.cloudera.org:8080/#/c/23569/23//COMMIT_MSG@56 PS23, Line 56: columns given that there is no deny policy on those columns and : no column masking policy on any column of the > Can you mention this change also earlier in the commit message? Yes I will mention this earlier in the commit message in the next patch. > BTW can't this increase the number of audit log entries drastically? When an INSERT query fails the authorization, there will be only one single Ranger audit log entry produced. When the authorization is successful, it depends on the number of Ranger policies covering the columns of the target table. - If we define 1 Ranger policy for each column in the target table, then the number of audit log entries would be the number of columns in the target table. This is verified by the test case at https://gerrit.cloudera.org/c/23569/23/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java#187. - But if we define 1 single Ranger policy covering all the columns in the target table, then there will only be one Ranger audit log entry. I could add to testAuditLogSuccess() a test case verifying this in the next patch. - Alternatively, we could just grant the INSERT privilege on the entire target table to the requesting user. In this case, there will be 2 Ranger audit log entries produced. One corresponds to the table-level INSERT request, and the other corresponds to all the column-level INSERT requests of the target table. This is seen at https://gerrit.cloudera.org/c/23569/23/fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java#125. > Do you know if Hive works the same? The short answer is not exactly the same but the difference should be acceptable. I briefly verified this on a cluster where Hive used a RangerHiveAuthorizer similar to https://github.com/apache/ranger/blob/5ea59e0/hive-agent/src/main/java/org/apache/ranger/authorization/hive/authorizer/RangerHiveAuthorizer.java to perform authorization. - If we define 1 Ranger policy for each column in the target table, then the number of audit log entries produced by Hive would be the number of columns in the target table. Note that each produced audit log entry has its respective policy id. This is the same as what is seen in Impala. - If we define 1 single Ranger policy covering all the columns in the target table, then there will only be one Ranger audit log entry produced by Hive. This is the same as what is seen in Impala. - If we define 1 single Ranger policy granting the INSERT privilege on the entire target table to the requesting user, then there will only be one Ranger audit log entry produced by Hive. This is different from what we have seen in Impala. http://gerrit.cloudera.org:8080/#/c/23569/23/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/23569/23/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@763 PS23, Line 763: trac > nit: this might be too verbose if we log it for all the columns. Sure. I will change this to trace() in the next patch. http://gerrit.cloudera.org:8080/#/c/23569/23/fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java File fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java: http://gerrit.cloudera.org:8080/#/c/23569/23/fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java@660 PS23, Line 660: " \"name\": \"%s\",\n" + > optional: using multiline strings (""") would make this more readable I am not very sure if we could use multi-line strings/text blocks without changing any pom.xml files. My $JAVA_HOME is /usr/lib/jvm/java-17-openjdk-amd64, which seems to be the same as the $JAVA_HOME at https://jenkins.impala.io/job/ubuntu-20.04-from-scratch/7980/consoleFull against https://gerrit.cloudera.org/c/23916/ (IMPALA-13921: Make Java 17 the default in tests). Maybe I missed something important? The error message I saw was the following. 18:57:37 [ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.14.0:testCompile (default-testCompile) on project impala-frontend: Compilation failure 18:57:37 [ERROR] /home/fangyurao/Impala_for_FE/fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java:[695,25] text blocks are not supported in -source 8 18:57:37 [ERROR] (use -source 15 or higher to enable text blocks) I already prepared a text block like the following. So if it's not too difficult to add this text block, then I will do so. If it turns out updating some pom.xml files is required, then maybe that should be done in a separate patch since using multi-line strings is orthogonal to the purpose of this patch. String policyJson = """ { "name": "%s", "policyType": 0, "serviceType": "%s" "service": "%s" "resources": { "database": { "values": [ "%s" ], "isExcludes": false, "isRecursive": false }, "table": { "values": [ "%s" ], "isExcludes": false, "isRecursive": false }, "column": { "values": [ "%s" ], "isExcludes": false, "isRecursive": false }, "denyPolicyItems": [ { "accesses": [ { "type": "%s", "isAllowed": true } ], "users": [ "%s" ] } ] } """.formatted(policyName, RANGER_SERVICE_TYPE, RANGER_SERVICE_NAME, databaseName, tableName, columnName, accessType, user); -- To view, visit http://gerrit.cloudera.org:8080/23569 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2ef61801d3b394c56702b193c250492a62b111df Gerrit-Change-Number: 23569 Gerrit-PatchSet: 24 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: Noemi Pap-Takacs <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Comment-Date: Fri, 06 Feb 2026 03:30:08 +0000 Gerrit-HasComments: Yes
