[kudu-CR] [WIP] Ranger authorization provider
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15207 to look at the new patch set (#27). Change subject: [WIP] Ranger authorization provider .. [WIP] Ranger authorization provider Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 --- M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc A src/kudu/master/ranger_authz_provider-test.cc A src/kudu/master/ranger_authz_provider.cc A src/kudu/master/ranger_authz_provider.h M src/kudu/master/sentry_authz_provider.cc 6 files changed, 348 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/15207/27 -- To view, visit http://gerrit.cloudera.org:8080/15207 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 Gerrit-Change-Number: 15207 Gerrit-PatchSet: 27 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [WIP] Ranger authorization provider
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/15207 ) Change subject: [WIP] Ranger authorization provider .. Patch Set 27: (1 comment) http://gerrit.cloudera.org:8080/#/c/15207/26/src/kudu/master/ranger_authz_provider.cc File src/kudu/master/ranger_authz_provider.cc: http://gerrit.cloudera.org:8080/#/c/15207/26/src/kudu/master/ranger_authz_provider.cc@186 PS26, Line 186: &co > nit: indent. Done -- To view, visit http://gerrit.cloudera.org:8080/15207 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 Gerrit-Change-Number: 15207 Gerrit-PatchSet: 27 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 05 Mar 2020 11:11:34 + Gerrit-HasComments: Yes
[kudu-CR] [WIP] Ranger authorization provider
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15207 ) Change subject: [WIP] Ranger authorization provider .. Patch Set 26: (1 comment) http://gerrit.cloudera.org:8080/#/c/15207/26/src/kudu/master/ranger_authz_provider.cc File src/kudu/master/ranger_authz_provider.cc: http://gerrit.cloudera.org:8080/#/c/15207/26/src/kudu/master/ranger_authz_provider.cc@186 PS26, Line 186: &column_names).ok() nit: indent. -- To view, visit http://gerrit.cloudera.org:8080/15207 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 Gerrit-Change-Number: 15207 Gerrit-PatchSet: 26 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Thu, 05 Mar 2020 05:12:04 + Gerrit-HasComments: Yes
[kudu-CR] [WIP] Ranger authorization provider
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15207 to look at the new patch set (#24). Change subject: [WIP] Ranger authorization provider .. [WIP] Ranger authorization provider Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 --- M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc A src/kudu/master/ranger_authz_provider-test.cc A src/kudu/master/ranger_authz_provider.cc A src/kudu/master/ranger_authz_provider.h M src/kudu/master/sentry_authz_provider.cc 6 files changed, 348 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/15207/24 -- To view, visit http://gerrit.cloudera.org:8080/15207 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 Gerrit-Change-Number: 15207 Gerrit-PatchSet: 24 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [WIP] Ranger authorization provider
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15207 to look at the new patch set (#23). Change subject: [WIP] Ranger authorization provider .. [WIP] Ranger authorization provider Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 --- M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc A src/kudu/master/ranger_authz_provider-test.cc A src/kudu/master/ranger_authz_provider.cc A src/kudu/master/ranger_authz_provider.h M src/kudu/master/sentry_authz_provider.cc 6 files changed, 349 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/15207/23 -- To view, visit http://gerrit.cloudera.org:8080/15207 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 Gerrit-Change-Number: 15207 Gerrit-PatchSet: 23 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [WIP] Ranger authorization provider
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/15207 ) Change subject: [WIP] Ranger authorization provider .. Patch Set 22: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/15207 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 Gerrit-Change-Number: 15207 Gerrit-PatchSet: 22 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 04 Mar 2020 17:11:11 + Gerrit-HasComments: No
[kudu-CR] [WIP] Ranger authorization provider
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15207 to look at the new patch set (#22). Change subject: [WIP] Ranger authorization provider .. [WIP] Ranger authorization provider Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 --- M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc A src/kudu/master/ranger_authz_provider-test.cc A src/kudu/master/ranger_authz_provider.cc A src/kudu/master/ranger_authz_provider.h M src/kudu/master/sentry_authz_provider.cc 6 files changed, 351 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/15207/22 -- To view, visit http://gerrit.cloudera.org:8080/15207 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 Gerrit-Change-Number: 15207 Gerrit-PatchSet: 22 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [WIP] Ranger authorization provider
Attila Bukor has posted comments on this change. (
http://gerrit.cloudera.org:8080/15207 )
Change subject: [WIP] Ranger authorization provider
..
Patch Set 21:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/15207/16/src/kudu/master/ranger_authz_provider.cc
File src/kudu/master/ranger_authz_provider.cc:
http://gerrit.cloudera.org:8080/#/c/15207/16/src/kudu/master/ranger_authz_provider.cc@41
PS16, Line 41: using kudu::security::TablePrivilegePB;
> Not sure. As it stands, there isn't enough information for a casual user to
Hao you have more experience with this side, do you have any suggestions how to
handle this?
http://gerrit.cloudera.org:8080/#/c/15207/16/src/kudu/master/ranger_authz_provider.cc@184
PS16, Line 184: // Otherwise we populate schema_pb and return OK.
> I guess we can leave it this way for now if the alternative is more complic
Done
http://gerrit.cloudera.org:8080/#/c/15207/20/src/kudu/ranger/ranger_client-test.cc
File src/kudu/ranger/ranger_client-test.cc:
http://gerrit.cloudera.org:8080/#/c/15207/20/src/kudu/ranger/ranger_client-test.cc@71
PS20, Line 71: struct AuthorizedActionHash {
: public:
: size_t operator()(const AuthorizedAction& action) const {
: size_t seed = 0;
: hash_combine(seed, action.action);
: hash_combine(seed, action.column_name);
: hash_combine(seed, action.database_name);
> Use boost::hash_combine for stuff like this, a la ConnectionId:
Done
--
To view, visit http://gerrit.cloudera.org:8080/15207
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012
Gerrit-Change-Number: 15207
Gerrit-PatchSet: 21
Gerrit-Owner: Attila Bukor
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 04 Mar 2020 11:16:50 +
Gerrit-HasComments: Yes
[kudu-CR] [WIP] Ranger authorization provider
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15207 to look at the new patch set (#21). Change subject: [WIP] Ranger authorization provider .. [WIP] Ranger authorization provider Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 --- M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc A src/kudu/master/ranger_authz_provider-test.cc A src/kudu/master/ranger_authz_provider.cc A src/kudu/master/ranger_authz_provider.h M src/kudu/master/sentry_authz_provider.cc 6 files changed, 348 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/15207/21 -- To view, visit http://gerrit.cloudera.org:8080/15207 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 Gerrit-Change-Number: 15207 Gerrit-PatchSet: 21 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [WIP] Ranger authorization provider
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/15207 )
Change subject: [WIP] Ranger authorization provider
..
Patch Set 20:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/15207/16/src/kudu/master/ranger_authz_provider.cc
File src/kudu/master/ranger_authz_provider.cc:
http://gerrit.cloudera.org:8080/#/c/15207/16/src/kudu/master/ranger_authz_provider.cc@41
PS16, Line 41:
> We need to add this directory to the Java classpath. Should I list all file
Not sure. As it stands, there isn't enough information for a casual user to
figure out what needs to be configured and how, and we need to fix that.
http://gerrit.cloudera.org:8080/#/c/15207/16/src/kudu/master/ranger_authz_provider.cc@184
PS16, Line 184: column_names.emplace(col.name());
> We could, but that would be a lot of extra complexity and I'm not sure it's
I guess we can leave it this way for now if the alternative is more
complicated. Could you a TODO? Basically we should revisit when we have a
clearer sense of the sort of Ranger policies our users will create.
http://gerrit.cloudera.org:8080/#/c/15207/20/src/kudu/ranger/ranger_client-test.cc
File src/kudu/ranger/ranger_client-test.cc:
http://gerrit.cloudera.org:8080/#/c/15207/20/src/kudu/ranger/ranger_client-test.cc@71
PS20, Line 71: int h_action =
std::hash{}(static_cast(action.action));
: int h_column = std::hash{}(action.column_name) << 1;
: int h_database = std::hash{}(action.database_name)
<< 2;
: int h_table = std::hash{}(action.table_name) << 3;
: int h_user = std::hash{}(action.user_name) << 4;
:
: return h_action ^ h_column ^ h_database ^ h_table ^ h_user;
Use boost::hash_combine for stuff like this, a la ConnectionId:
size_t ConnectionId::HashCode() const {
size_t seed = 0;
boost::hash_combine(seed, remote_.HashCode());
boost::hash_combine(seed, hostname_);
boost::hash_combine(seed, user_credentials_.HashCode());
boost::hash_combine(seed, network_plane_);
return seed;
}
--
To view, visit http://gerrit.cloudera.org:8080/15207
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012
Gerrit-Change-Number: 15207
Gerrit-PatchSet: 20
Gerrit-Owner: Attila Bukor
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 04 Mar 2020 05:25:52 +
Gerrit-HasComments: Yes
[kudu-CR] [WIP] Ranger authorization provider
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15207 to look at the new patch set (#20). Change subject: [WIP] Ranger authorization provider .. [WIP] Ranger authorization provider Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 --- M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc A src/kudu/master/ranger_authz_provider-test.cc A src/kudu/master/ranger_authz_provider.cc A src/kudu/master/ranger_authz_provider.h M src/kudu/master/sentry_authz_provider.cc M src/kudu/ranger/CMakeLists.txt 7 files changed, 362 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/15207/20 -- To view, visit http://gerrit.cloudera.org:8080/15207 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 Gerrit-Change-Number: 15207 Gerrit-PatchSet: 20 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [WIP] Ranger authorization provider
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15207 to look at the new patch set (#18). Change subject: [WIP] Ranger authorization provider .. [WIP] Ranger authorization provider Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 --- M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc A src/kudu/master/ranger_authz_provider-test.cc A src/kudu/master/ranger_authz_provider.cc A src/kudu/master/ranger_authz_provider.h M src/kudu/master/sentry_authz_provider.cc M src/kudu/ranger/CMakeLists.txt M src/kudu/ranger/ranger_client-test.cc 8 files changed, 387 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/15207/18 -- To view, visit http://gerrit.cloudera.org:8080/15207 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 Gerrit-Change-Number: 15207 Gerrit-PatchSet: 18 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [WIP] Ranger authorization provider
Attila Bukor has posted comments on this change. (
http://gerrit.cloudera.org:8080/15207 )
Change subject: [WIP] Ranger authorization provider
..
Patch Set 17:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/15207/16/src/kudu/master/ranger_authz_provider.h
File src/kudu/master/ranger_authz_provider.h:
http://gerrit.cloudera.org:8080/#/c/15207/16/src/kudu/master/ranger_authz_provider.h@84
PS16, Line 84: // Returns the location of kudu-ranger.jar
> Should doc what this does.
Done
http://gerrit.cloudera.org:8080/#/c/15207/16/src/kudu/master/ranger_authz_provider.cc
File src/kudu/master/ranger_authz_provider.cc:
http://gerrit.cloudera.org:8080/#/c/15207/16/src/kudu/master/ranger_authz_provider.cc@41
PS16, Line 41: DEFINE_string(ranger_config_path, "",
> Shouldn't we link to a specific filename? If not, we should indicate what f
We need to add this directory to the Java classpath. Should I list all files
the subprocess will need, including optional ones? Or should I just point to
the Ranger docs maybe?
http://gerrit.cloudera.org:8080/#/c/15207/16/src/kudu/master/ranger_authz_provider.cc@151
PS16, Line 151:
: unordered_set actions = {
: ActionPB::DELETE,
: ActionPB::INSERT,
: ActionPB::UPDATE,
> Can use an initializer_list?
Done
http://gerrit.cloudera.org:8080/#/c/15207/16/src/kudu/master/ranger_authz_provider.cc@161
PS16, Line 161: auto s = client_.AuthorizeActions(user, &actions, table_name);
: if (s.ok()) {
: for (const ActionPB& action : actions) {
: switch (action) {
: case ActionPB::DELETE:
: pb->set_delete_privilege(true);
: break;
: case ActionPB::UPDATE:
> I think it'd be more performant to iterate over 'actions' rather than do fo
Done
http://gerrit.cloudera.org:8080/#/c/15207/16/src/kudu/master/ranger_authz_provider.cc@184
PS16, Line 184: }
> So it seems as if we are making two distinct kinds of authz checks here:
We could, but that would be a lot of extra complexity and I'm not sure it's
worth it. This way we don't need to check column-level privileges if SELECT is
allowed on the table, so basically we are optimizing for the case where the
whole table is readable and not only specific columns by a user. Let me know
what you think, I can rewrite it.
http://gerrit.cloudera.org:8080/#/c/15207/16/src/kudu/master/ranger_authz_provider.cc@206
PS16, Line 206: if (ContainsKey(column_names, col.name())) {
> Return value should be checked.
Done
--
To view, visit http://gerrit.cloudera.org:8080/15207
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012
Gerrit-Change-Number: 15207
Gerrit-PatchSet: 17
Gerrit-Owner: Attila Bukor
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 03 Mar 2020 13:18:13 +
Gerrit-HasComments: Yes
[kudu-CR] [WIP] Ranger authorization provider
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15207 to look at the new patch set (#17). Change subject: [WIP] Ranger authorization provider .. [WIP] Ranger authorization provider Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 --- M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc A src/kudu/master/ranger_authz_provider-test.cc A src/kudu/master/ranger_authz_provider.cc A src/kudu/master/ranger_authz_provider.h M src/kudu/master/sentry_authz_provider.cc M src/kudu/ranger/CMakeLists.txt M src/kudu/ranger/ranger_client-test.cc 8 files changed, 386 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/15207/17 -- To view, visit http://gerrit.cloudera.org:8080/15207 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 Gerrit-Change-Number: 15207 Gerrit-PatchSet: 17 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [WIP] Ranger authorization provider
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/15207 )
Change subject: [WIP] Ranger authorization provider
..
Patch Set 16:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/15207/16/src/kudu/master/ranger_authz_provider.h
File src/kudu/master/ranger_authz_provider.h:
http://gerrit.cloudera.org:8080/#/c/15207/16/src/kudu/master/ranger_authz_provider.h@84
PS16, Line 84: static std::string GetJar();
Should doc what this does.
http://gerrit.cloudera.org:8080/#/c/15207/16/src/kudu/master/ranger_authz_provider.cc
File src/kudu/master/ranger_authz_provider.cc:
http://gerrit.cloudera.org:8080/#/c/15207/16/src/kudu/master/ranger_authz_provider.cc@41
PS16, Line 41: DEFINE_string(ranger_config_path, "",
Shouldn't we link to a specific filename? If not, we should indicate what files
we'll be looking for in this directory.
http://gerrit.cloudera.org:8080/#/c/15207/16/src/kudu/master/ranger_authz_provider.cc@151
PS16, Line 151: unordered_set actions;
: actions.emplace(ActionPB::DELETE);
: actions.emplace(ActionPB::INSERT);
: actions.emplace(ActionPB::UPDATE);
: actions.emplace(ActionPB::SELECT);
Can use an initializer_list?
unoredered_set actions = { ActionPB::DELETE, ActionPB::INSERT, ...
};
http://gerrit.cloudera.org:8080/#/c/15207/16/src/kudu/master/ranger_authz_provider.cc@161
PS16, Line 161: pb->set_delete_privilege(ContainsKey(actions,
ActionPB::DELETE));
: pb->set_update_privilege(ContainsKey(actions,
ActionPB::UPDATE));
: pb->set_insert_privilege(ContainsKey(actions,
ActionPB::INSERT));
: if (ContainsKey(actions, ActionPB::SELECT)) {
: pb->set_scan_privilege(true);
: return Status::OK();
: }
: pb->set_scan_privilege(false);
I think it'd be more performant to iterate over 'actions' rather than do four
discrete lookups.
http://gerrit.cloudera.org:8080/#/c/15207/16/src/kudu/master/ranger_authz_provider.cc@184
PS16, Line 184: if (!client_.AuthorizeAction(user, ActionPB::SELECT,
table_name,
So it seems as if we are making two distinct kinds of authz checks here:
1. One table, N actions, no columns. --> tell me which of the N actions I'm
allowed to perform on this table.
2. One table, one action, N columns. --> tell me which of the table's N columns
I'm allowed to SELECT.
Can we combine them into one request to Ranger?
http://gerrit.cloudera.org:8080/#/c/15207/16/src/kudu/master/ranger_authz_provider.cc@206
PS16, Line 206: env->GetExecutablePath(&exe);
Return value should be checked.
--
To view, visit http://gerrit.cloudera.org:8080/15207
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012
Gerrit-Change-Number: 15207
Gerrit-PatchSet: 16
Gerrit-Owner: Attila Bukor
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 03 Mar 2020 07:26:22 +
Gerrit-HasComments: Yes
[kudu-CR] [WIP] Ranger authorization provider
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15207 to look at the new patch set (#15). Change subject: [WIP] Ranger authorization provider .. [WIP] Ranger authorization provider Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 --- M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc A src/kudu/master/ranger_authz_provider-test.cc A src/kudu/master/ranger_authz_provider.cc A src/kudu/master/ranger_authz_provider.h M src/kudu/master/sentry_authz_provider.cc M src/kudu/ranger/CMakeLists.txt M src/kudu/ranger/ranger_client-test.cc M src/kudu/ranger/ranger_client.h 9 files changed, 370 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/15207/15 -- To view, visit http://gerrit.cloudera.org:8080/15207 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 Gerrit-Change-Number: 15207 Gerrit-PatchSet: 15 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [WIP] Ranger authorization provider
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15207 to look at the new patch set (#14). Change subject: [WIP] Ranger authorization provider .. [WIP] Ranger authorization provider Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 --- M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc A src/kudu/master/ranger_authz_provider-test.cc A src/kudu/master/ranger_authz_provider.cc A src/kudu/master/ranger_authz_provider.h M src/kudu/master/sentry_authz_provider.cc M src/kudu/ranger/ranger_client-test.cc M src/kudu/ranger/ranger_client.cc M src/kudu/ranger/ranger_client.h 9 files changed, 344 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/15207/14 -- To view, visit http://gerrit.cloudera.org:8080/15207 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 Gerrit-Change-Number: 15207 Gerrit-PatchSet: 14 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [WIP] Ranger authorization provider
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/15207 ) Change subject: [WIP] Ranger authorization provider .. Patch Set 13: (2 comments) http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.cc File src/kudu/master/ranger_authz_provider.cc: http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.cc@131 PS12, Line 131: const SchemaPB& schema_pb, > This looks like a lot of round-trips to the plugin and, if the cache is col Done http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.cc@168 PS12, Line 168: unordered_set column_names; > This is O(n^2) given that column_names is a vector. Done -- To view, visit http://gerrit.cloudera.org:8080/15207 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 Gerrit-Change-Number: 15207 Gerrit-PatchSet: 13 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Mon, 02 Mar 2020 18:32:27 + Gerrit-HasComments: Yes
[kudu-CR] [WIP] Ranger authorization provider
Attila Bukor has posted comments on this change. (
http://gerrit.cloudera.org:8080/15207 )
Change subject: [WIP] Ranger authorization provider
..
Patch Set 13:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/catalog_manager.cc@754
PS12, Line 754: return Status::InvalidArgument(Substitute(
: "column '$0' has write_default field set but no
read_default", col->name()));
: }
: return Status::OK();
> Handle this as a GROUP_FLAG_VALIDATOR instead.
Done
http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.h
File src/kudu/master/ranger_authz_provider.h:
http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.h@38
PS12, Line 38: // An implementation of AuthzProvider that connects to Ranger
and translates
> Provide a high level class doc.
Done
http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.h@40
PS12, Line 40: // the received response
> Can this be omitted? Don't you get a public constructor by default?
I think I need it if I want to provide a constructor definition.
http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.h@48
PS12, Line 48:
: Status ResetCache() ove
> You can implement this function inline and remove the docs since it'll be o
Done
http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.cc
File src/kudu/master/ranger_authz_provider.cc:
http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.cc@40
PS12, Line 40: "Ranger authorization.
sentry_service_rpc_addresses must not be "
> We should update the equivalent flag in Sentry to indicate this incompatibi
Done
http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.cc@57
PS12, Line 57: vector({"ranger"
> Don't need std prefixes.
Done
http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.cc@65
PS12, Line 65: Status RangerAuthzProvider::AuthorizeCreateTable(const string&
table_name,
:
> Can inline in the header.
Done
http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.cc@95
PS12, Line 95: RETURN_NOT_OK(client_.AuthorizeAction(user, ActionPB::ALL,
old_table));
: return client_.AuthorizeAction(user, ActionPB::CREATE,
new_table);
: }
:
: Status RangerAuthzProvider::AuthorizeGetTableMetadata(const
string& table_name,
: const
string
> Could you write a comment similar to what's in SentryAuthzProvider to expla
Done
http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.cc@128
PS12, Line 128:
> Why not METADATA? Please add a comment.
Done
--
To view, visit http://gerrit.cloudera.org:8080/15207
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012
Gerrit-Change-Number: 15207
Gerrit-PatchSet: 13
Gerrit-Owner: Attila Bukor
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 02 Mar 2020 18:32:12 +
Gerrit-HasComments: Yes
[kudu-CR] [WIP] Ranger authorization provider
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15207 to look at the new patch set (#13). Change subject: [WIP] Ranger authorization provider .. [WIP] Ranger authorization provider Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 --- M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc A src/kudu/master/ranger_authz_provider-test.cc A src/kudu/master/ranger_authz_provider.cc A src/kudu/master/ranger_authz_provider.h M src/kudu/master/sentry_authz_provider.cc 6 files changed, 335 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/15207/13 -- To view, visit http://gerrit.cloudera.org:8080/15207 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 Gerrit-Change-Number: 15207 Gerrit-PatchSet: 13 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [WIP] Ranger authorization provider
Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/15207 )
Change subject: [WIP] Ranger authorization provider
..
Patch Set 12:
(11 comments)
http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/catalog_manager.cc
File src/kudu/master/catalog_manager.cc:
http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/catalog_manager.cc@754
PS12, Line 754: if (SentryAuthzProvider::IsEnabled() &&
: RangerAuthzProvider::IsEnabled()) {
: LOG(FATAL) << "Ranger and Sentry authorization cannot be
enabled at the "
: "same time.";
Handle this as a GROUP_FLAG_VALIDATOR instead.
http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.h
File src/kudu/master/ranger_authz_provider.h:
http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.h@38
PS12, Line 38: class RangerAuthzProvider : public AuthzProvider {
Provide a high level class doc.
Also I don't think you need to provide per-function docs for the overrides,
unless the behavior deviates in some interesting way from the docs in
authz_provider.h.
http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.h@40
PS12, Line 40: RangerAuthzProvider();
Can this be omitted? Don't you get a public constructor by default?
http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.h@48
PS12, Line 48: // Returns Status::NotSupported() as Ranger authz provider
doesn't support
: // resetting its cache.
You can implement this function inline and remove the docs since it'll be
obvious what's going on.
http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.cc
File src/kudu/master/ranger_authz_provider.cc:
http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.cc@40
PS12, Line 40: "Ranger authorization.
sentry_service_rpc_addresses must not be "
We should update the equivalent flag in Sentry to indicate this incompatibility
too.
http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.cc@57
PS12, Line 57: std::vector
Don't need std prefixes.
http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.cc@65
PS12, Line 65: void RangerAuthzProvider::Stop() {
: }
Can inline in the header.
http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.cc@95
PS12, Line 95: if (old_table == new_table) {
: return client_.AuthorizeAction(user, Action::ALTER,
old_table);
: }
:
: RETURN_NOT_OK(client_.AuthorizeAction(user, Action::ALL,
old_table));
: return client_.AuthorizeAction(user, Action::CREATE,
new_table);
Could you write a comment similar to what's in SentryAuthzProvider to explain
this?
http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.cc@128
PS12, Line 128: return client_.AuthorizeAction(user, Action::SELECT,
table_name);
Why not METADATA? Please add a comment.
http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.cc@131
PS12, Line 131: Status RangerAuthzProvider::FillTablePrivilegePB(const string&
table_name,
This looks like a lot of round-trips to the plugin and, if the cache is cold,
to the Ranger server. Can we do it in bulk instead?
http://gerrit.cloudera.org:8080/#/c/15207/12/src/kudu/master/ranger_authz_provider.cc@168
PS12, Line 168: if (find(column_names.begin(), column_names.end(),
col.name()) != column_names.end()) {
This is O(n^2) given that column_names is a vector.
--
To view, visit http://gerrit.cloudera.org:8080/15207
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012
Gerrit-Change-Number: 15207
Gerrit-PatchSet: 12
Gerrit-Owner: Attila Bukor
Gerrit-Reviewer: Adar Dembo
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Wed, 26 Feb 2020 21:59:52 +
Gerrit-HasComments: Yes
[kudu-CR] [WIP] Ranger authorization provider
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15207 to look at the new patch set (#10). Change subject: [WIP] Ranger authorization provider .. [WIP] Ranger authorization provider Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 --- M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc A src/kudu/master/ranger_authz_provider-test.cc A src/kudu/master/ranger_authz_provider.cc A src/kudu/master/ranger_authz_provider.h 5 files changed, 343 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/15207/10 -- To view, visit http://gerrit.cloudera.org:8080/15207 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 Gerrit-Change-Number: 15207 Gerrit-PatchSet: 10 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [WIP] Ranger authorization provider
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15207 to look at the new patch set (#9). Change subject: [WIP] Ranger authorization provider .. [WIP] Ranger authorization provider Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 --- M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc A src/kudu/master/ranger_authz_provider-test.cc A src/kudu/master/ranger_authz_provider.cc A src/kudu/master/ranger_authz_provider.h 5 files changed, 345 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/15207/9 -- To view, visit http://gerrit.cloudera.org:8080/15207 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 Gerrit-Change-Number: 15207 Gerrit-PatchSet: 9 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [WIP] Ranger authorization provider
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15207 ) Change subject: [WIP] Ranger authorization provider .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/15207/5/src/kudu/master/ranger_authz_provider.cc File src/kudu/master/ranger_authz_provider.cc: http://gerrit.cloudera.org:8080/#/c/15207/5/src/kudu/master/ranger_authz_provider.cc@120 PS5, Line 120: Status RangerAuthzProvider::AuthorizeGetTableStatistics(const string& tabl > I'm not sure what you mean, do you mean the C++ Ranger client should cache I meant that we should validate that the Ranger plugin actually caches privileges in an acceptable way. For instance, will the below 5 calls to AuthorizeAction() actually result in 5 calls to the Ranger server? If so, is that the best we can do? Or are there things we can do to improve our hit rates? For instance, it's unclear to me whether the current single-action, single-table API for the RangerClient here is better than passing multiple Actions to the RangerClient for this table so that the Ranger plugin can authorize in bulk. If Ranger's bulk-authorization API means it can cache more efficiently, we might even want to consider passing the requests for column privileges together with the rest of the requests for table privileges. -- To view, visit http://gerrit.cloudera.org:8080/15207 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 Gerrit-Change-Number: 15207 Gerrit-PatchSet: 8 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Mon, 24 Feb 2020 03:43:33 + Gerrit-HasComments: Yes
[kudu-CR] [WIP] Ranger authorization provider
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15207 to look at the new patch set (#7). Change subject: [WIP] Ranger authorization provider .. [WIP] Ranger authorization provider Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 --- M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc A src/kudu/master/ranger_authz_provider-test.cc A src/kudu/master/ranger_authz_provider.cc A src/kudu/master/ranger_authz_provider.h 5 files changed, 344 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/15207/7 -- To view, visit http://gerrit.cloudera.org:8080/15207 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 Gerrit-Change-Number: 15207 Gerrit-PatchSet: 7 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [WIP] Ranger authorization provider
Attila Bukor has posted comments on this change. (
http://gerrit.cloudera.org:8080/15207 )
Change subject: [WIP] Ranger authorization provider
..
Patch Set 6:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/15207/5/src/kudu/master/ranger_authz_provider.h
File src/kudu/master/ranger_authz_provider.h:
http://gerrit.cloudera.org:8080/#/c/15207/5/src/kudu/master/ranger_authz_provider.h@37
PS5, Line 37: class RangerAuthzProvider : public AuthzProvider {
> The public methods in this class should be marked 'override'.
Done
http://gerrit.cloudera.org:8080/#/c/15207/5/src/kudu/master/ranger_authz_provider.h@46
PS5, Line 46:
: // Returns Status::NotSupported() as Ranger authz provider
doesn't support
: // resetting its cache.
: Status ResetC
> These comments should be updated to reflect the RangerAuthzProvider's seman
Done
http://gerrit.cloudera.org:8080/#/c/15207/5/src/kudu/master/ranger_authz_provider.cc
File src/kudu/master/ranger_authz_provider.cc:
http://gerrit.cloudera.org:8080/#/c/15207/5/src/kudu/master/ranger_authz_provider.cc@120
PS5, Line 120: Status RangerAuthzProvider::AuthorizeGetTableStatistics(const
string& tabl
> It's probably worth validating that the Ranger client will cache privileges
I'm not sure what you mean, do you mean the C++ Ranger client should cache
privileges? The Ranger plugin caches privileges automatically and it shouldn't
be much overhead calling the subprocess as it's all stdio.
http://gerrit.cloudera.org:8080/#/c/15207/5/src/kudu/master/ranger_authz_provider.cc@152
PS5, Line 152: if (client_.AuthorizeAction(user, Act
> nit: could use EmplaceOrDie here
changed column_names to vector in the meantime.
http://gerrit.cloudera.org:8080/#/c/15207/5/src/kudu/master/ranger_authz_provider.cc@157
PS5, Line 157:
> nit: use InsertOrDie or EmplaceOrDie? The column IDs should never already b
I think so, I copied this part from the Sentry authz provider actually.
--
To view, visit http://gerrit.cloudera.org:8080/15207
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012
Gerrit-Change-Number: 15207
Gerrit-PatchSet: 6
Gerrit-Owner: Attila Bukor
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 17 Feb 2020 07:32:42 +
Gerrit-HasComments: Yes
[kudu-CR] [WIP] Ranger authorization provider
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15207 to look at the new patch set (#6). Change subject: [WIP] Ranger authorization provider .. [WIP] Ranger authorization provider Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 --- M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc A src/kudu/master/ranger_authz_provider-test.cc A src/kudu/master/ranger_authz_provider.cc A src/kudu/master/ranger_authz_provider.h 5 files changed, 344 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/15207/6 -- To view, visit http://gerrit.cloudera.org:8080/15207 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 Gerrit-Change-Number: 15207 Gerrit-PatchSet: 6 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [WIP] Ranger authorization provider
Andrew Wong has posted comments on this change. (
http://gerrit.cloudera.org:8080/15207 )
Change subject: [WIP] Ranger authorization provider
..
Patch Set 5:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/15207/5/src/kudu/master/ranger_authz_provider.h
File src/kudu/master/ranger_authz_provider.h:
http://gerrit.cloudera.org:8080/#/c/15207/5/src/kudu/master/ranger_authz_provider.h@37
PS5, Line 37: class RangerAuthzProvider : public AuthzProvider {
The public methods in this class should be marked 'override'.
http://gerrit.cloudera.org:8080/#/c/15207/5/src/kudu/master/ranger_authz_provider.h@46
PS5, Line 46:
: // Reset the underlying cache (if any), invalidating all cached
entries.
: // Returns Status::NotSupported() if the provider doesn't
support resetting
: // its cache.
These comments should be updated to reflect the RangerAuthzProvider's
semantics, if they differ from the default.
http://gerrit.cloudera.org:8080/#/c/15207/5/src/kudu/master/ranger_authz_provider.cc
File src/kudu/master/ranger_authz_provider.cc:
http://gerrit.cloudera.org:8080/#/c/15207/5/src/kudu/master/ranger_authz_provider.cc@120
PS5, Line 120: Status RangerAuthzProvider::FillTablePrivilegePB(const string&
table_name,
It's probably worth validating that the Ranger client will cache privileges as
appropriate.
http://gerrit.cloudera.org:8080/#/c/15207/5/src/kudu/master/ranger_authz_provider.cc@152
PS5, Line 152: column_names.emplace(col.name());
nit: could use EmplaceOrDie here
http://gerrit.cloudera.org:8080/#/c/15207/5/src/kudu/master/ranger_authz_provider.cc@157
PS5, Line 157: InsertIfNotPresent
nit: use InsertOrDie or EmplaceOrDie? The column IDs should never already be in
present, right?
--
To view, visit http://gerrit.cloudera.org:8080/15207
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012
Gerrit-Change-Number: 15207
Gerrit-PatchSet: 5
Gerrit-Owner: Attila Bukor
Gerrit-Reviewer: Andrew Wong
Gerrit-Reviewer: Attila Bukor
Gerrit-Reviewer: Hao Hao
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Mon, 17 Feb 2020 03:54:55 +
Gerrit-HasComments: Yes
[kudu-CR] [WIP] Ranger authorization provider
Hello Tidy Bot, Kudu Jenkins, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15207 to look at the new patch set (#4). Change subject: [WIP] Ranger authorization provider .. [WIP] Ranger authorization provider Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 --- M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc A src/kudu/master/ranger_authz_provider-test.cc A src/kudu/master/ranger_authz_provider.cc A src/kudu/master/ranger_authz_provider.h 5 files changed, 334 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/15207/4 -- To view, visit http://gerrit.cloudera.org:8080/15207 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 Gerrit-Change-Number: 15207 Gerrit-PatchSet: 4 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [WIP] Ranger authorization provider
Hello Tidy Bot, Kudu Jenkins, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15207 to look at the new patch set (#3). Change subject: [WIP] Ranger authorization provider .. [WIP] Ranger authorization provider Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 --- M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc A src/kudu/master/ranger_authz_provider-test.cc A src/kudu/master/ranger_authz_provider.cc A src/kudu/master/ranger_authz_provider.h 5 files changed, 334 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/15207/3 -- To view, visit http://gerrit.cloudera.org:8080/15207 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 Gerrit-Change-Number: 15207 Gerrit-PatchSet: 3 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [WIP] Ranger authorization provider
Hello Tidy Bot, Kudu Jenkins, Hao Hao, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/15207 to look at the new patch set (#2). Change subject: [WIP] Ranger authorization provider .. [WIP] Ranger authorization provider Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 --- M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc A src/kudu/master/ranger_authz_provider-test.cc A src/kudu/master/ranger_authz_provider.cc A src/kudu/master/ranger_authz_provider.h 5 files changed, 335 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/15207/2 -- To view, visit http://gerrit.cloudera.org:8080/15207 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 Gerrit-Change-Number: 15207 Gerrit-PatchSet: 2 Gerrit-Owner: Attila Bukor Gerrit-Reviewer: Attila Bukor Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [WIP] Ranger authorization provider
Attila Bukor has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15207 Change subject: [WIP] Ranger authorization provider .. [WIP] Ranger authorization provider Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 --- M src/kudu/master/CMakeLists.txt M src/kudu/master/catalog_manager.cc A src/kudu/master/ranger_authz_provider-test.cc A src/kudu/master/ranger_authz_provider.cc A src/kudu/master/ranger_authz_provider.h 5 files changed, 333 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/07/15207/1 -- To view, visit http://gerrit.cloudera.org:8080/15207 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I6e7672a5947d6406e0cad83a0c900bf5b2c03012 Gerrit-Change-Number: 15207 Gerrit-PatchSet: 1 Gerrit-Owner: Attila Bukor
