Ashwani Raina has posted comments on this change. ( http://gerrit.cloudera.org:8080/23191 )
Change subject: KUDU-3676: Add support for Hadoop auth to local rule mapping ...................................................................... Patch Set 1: (8 comments) Thank you for adding this support. I had a cursory look. With my limited knowledge about hadoop auth mapping, I have a few questions. Apart from those, there are a few nits. http://gerrit.cloudera.org:8080/#/c/23191/1/src/kudu/security/hadoop.h File src/kudu/security/hadoop.h: http://gerrit.cloudera.org:8080/#/c/23191/1/src/kudu/security/hadoop.h@40 PS1, Line 40: ; nit: here and wherever applicable - can do away with semicolon http://gerrit.cloudera.org:8080/#/c/23191/1/src/kudu/security/hadoop.cc File src/kudu/security/hadoop.cc: http://gerrit.cloudera.org:8080/#/c/23191/1/src/kudu/security/hadoop.cc@35 PS1, Line 35: hadoop.h nit: here and wherever else applicable - Add directory path (kudu/security/hadoop.h) http://gerrit.cloudera.org:8080/#/c/23191/1/src/kudu/security/hadoop.cc@116 PS1, Line 116: HadoopAuthToLocal::SetRules Is it possible that two different principals map to a same local user? In other words, is it required to catch such situations or responsibility lies on the input rules provider? http://gerrit.cloudera.org:8080/#/c/23191/1/src/kudu/security/hadoop.cc@890 PS1, Line 890: ; nit: Here and wherever else applicable - can do away with semicolon http://gerrit.cloudera.org:8080/#/c/23191/1/src/kudu/security/hadoop.cc@924 PS1, Line 924: Otherwise check the default rule How is default rule in terms of permissions? Is it possible to have too much permission for a principal with default rule? http://gerrit.cloudera.org:8080/#/c/23191/1/src/kudu/security/hadoop_tests.cc File src/kudu/security/hadoop_tests.cc: http://gerrit.cloudera.org:8080/#/c/23191/1/src/kudu/security/hadoop_tests.cc@113 PS1, Line 113: {.input = "RULE:[2:$1@\\]0]([email protected])s/.*/hue/", : .expected = {"2", "$1@]0", "[email protected]", "s/.*/hue/"}}, nit: Fix indentation http://gerrit.cloudera.org:8080/#/c/23191/1/src/kudu/security/hadoop_tests.cc@282 PS1, Line 282: { : "host", : "part1", : "part2", : }, nit: Move this to line 281 for consistency http://gerrit.cloudera.org:8080/#/c/23191/1/src/kudu/security/hadoop_tests.cc@707 PS1, Line 707: { : .principal = "[email protected]", : .expected = "myotherprincipal", : }, nit: Move it into one line if possible, for consistency. Ditto for line 697-700. -- To view, visit http://gerrit.cloudera.org:8080/23191 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e99639248fce01b9af0778074ecf6aa43660742 Gerrit-Change-Number: 23191 Gerrit-PatchSet: 1 Gerrit-Owner: Jeffrey Smith <[email protected]> Gerrit-Reviewer: Ashwani Raina <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Wed, 06 Aug 2025 16:12:22 +0000 Gerrit-HasComments: Yes
