You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Ashwani Raina (Code Review)" <ge...@cloudera.org> on 2025/08/06 16:12:22 UTC
[kudu-CR] KUDU-3676: Add support for Hadoop auth to local rule mapping
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](hue@EXAMPLE.COM)s/.*/hue/",
: .expected = {"2", "$1@]0", "hue@EXAMPLE.COM", "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 = "myotherprincipal@EXAMPLE.COM",
: .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 <je...@gmail.com>
Gerrit-Reviewer: Ashwani Raina <ar...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 06 Aug 2025 16:12:22 +0000
Gerrit-HasComments: Yes