You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Andrew Wong (Code Review)" <ge...@cloudera.org> on 2018/10/22 21:47:30 UTC
[kudu-CR] WIP authz: authorize ListTablets
Andrew Wong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11752
Change subject: WIP authz: authorize ListTablets
......................................................................
WIP authz: authorize ListTablets
WIP: add tests
See the comment in tablet_service.h for details.
Change-Id: I4bb2d09f23d7b77729e21060dad41c0501b17ded
---
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tablet_service.h
M src/kudu/tserver/tserver_service.proto
3 files changed, 20 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/52/11752/1
--
To view, visit http://gerrit.cloudera.org:8080/11752
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4bb2d09f23d7b77729e21060dad41c0501b17ded
Gerrit-Change-Number: 11752
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
[kudu-CR] authz: authorize ListTablets
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has removed a vote on this change.
Change subject: authz: authorize ListTablets
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/11752
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I4bb2d09f23d7b77729e21060dad41c0501b17ded
Gerrit-Change-Number: 11752
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] authz: authorize ListTablets
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11752 )
Change subject: authz: authorize ListTablets
......................................................................
Patch Set 4: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11752
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4bb2d09f23d7b77729e21060dad41c0501b17ded
Gerrit-Change-Number: 11752
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 09 Apr 2019 05:30:26 +0000
Gerrit-HasComments: No
[kudu-CR] authz: authorize ListTablets
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins, Adar Dembo, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11752
to look at the new patch set (#3).
Change subject: authz: authorize ListTablets
......................................................................
authz: authorize ListTablets
See the comment in tablet_service.h for details.
Change-Id: I4bb2d09f23d7b77729e21060dad41c0501b17ded
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tablet_service.h
M src/kudu/tserver/tserver_service.proto
4 files changed, 47 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/52/11752/3
--
To view, visit http://gerrit.cloudera.org:8080/11752
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4bb2d09f23d7b77729e21060dad41c0501b17ded
Gerrit-Change-Number: 11752
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] authz: authorize ListTablets
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11752
to look at the new patch set (#2).
Change subject: authz: authorize ListTablets
......................................................................
authz: authorize ListTablets
See the comment in tablet_service.h for details.
Change-Id: I4bb2d09f23d7b77729e21060dad41c0501b17ded
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tablet_service.h
M src/kudu/tserver/tserver_service.proto
4 files changed, 46 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/52/11752/2
--
To view, visit http://gerrit.cloudera.org:8080/11752
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4bb2d09f23d7b77729e21060dad41c0501b17ded
Gerrit-Change-Number: 11752
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] authz: authorize ListTablets
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Dan Burkert, Kudu Jenkins, Adar Dembo, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/11752
to look at the new patch set (#4).
Change subject: authz: authorize ListTablets
......................................................................
authz: authorize ListTablets
See the comment in tablet_service.h for details.
Change-Id: I4bb2d09f23d7b77729e21060dad41c0501b17ded
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tablet_service.h
M src/kudu/tserver/tserver_service.proto
4 files changed, 46 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/52/11752/4
--
To view, visit http://gerrit.cloudera.org:8080/11752
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4bb2d09f23d7b77729e21060dad41c0501b17ded
Gerrit-Change-Number: 11752
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] WIP authz: authorize ListTablets
Posted by "Dan Burkert (Code Review)" <ge...@cloudera.org>.
Dan Burkert has posted comments on this change. ( http://gerrit.cloudera.org:8080/11752 )
Change subject: WIP authz: authorize ListTablets
......................................................................
Patch Set 1:
LGTM
--
To view, visit http://gerrit.cloudera.org:8080/11752
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4bb2d09f23d7b77729e21060dad41c0501b17ded
Gerrit-Change-Number: 11752
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 23 Oct 2018 00:35:46 +0000
Gerrit-HasComments: No
[kudu-CR] authz: authorize ListTablets
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11752 )
Change subject: authz: authorize ListTablets
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11752/4/src/kudu/tserver/tablet_service.h
File src/kudu/tserver/tablet_service.h:
http://gerrit.cloudera.org:8080/#/c/11752/4/src/kudu/tserver/tablet_service.h@113
PS4, Line 113: Rather than authorizing multiple tables at
: // once, if enforcing access control, we require the super-user role and omit
: // checking table privileges, and authorize as a client otherwise.
Should we do the same for ListTables in the master? Why or why not?
--
To view, visit http://gerrit.cloudera.org:8080/11752
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4bb2d09f23d7b77729e21060dad41c0501b17ded
Gerrit-Change-Number: 11752
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 08 Apr 2019 05:19:22 +0000
Gerrit-HasComments: Yes
[kudu-CR] authz: authorize ListTablets
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11752 )
Change subject: authz: authorize ListTablets
......................................................................
authz: authorize ListTablets
See the comment in tablet_service.h for details.
Change-Id: I4bb2d09f23d7b77729e21060dad41c0501b17ded
Reviewed-on: http://gerrit.cloudera.org:8080/11752
Tested-by: Andrew Wong <aw...@cloudera.com>
Reviewed-by: Adar Dembo <ad...@cloudera.com>
Reviewed-by: Hao Hao <ha...@cloudera.com>
---
M src/kudu/integration-tests/security-itest.cc
M src/kudu/tserver/tablet_service.cc
M src/kudu/tserver/tablet_service.h
M src/kudu/tserver/tserver_service.proto
4 files changed, 46 insertions(+), 1 deletion(-)
Approvals:
Andrew Wong: Verified
Adar Dembo: Looks good to me, but someone else must approve
Hao Hao: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/11752
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I4bb2d09f23d7b77729e21060dad41c0501b17ded
Gerrit-Change-Number: 11752
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] authz: authorize ListTablets
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/11752 )
Change subject: authz: authorize ListTablets
......................................................................
Patch Set 4: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11752/4/src/kudu/tserver/tablet_service.h
File src/kudu/tserver/tablet_service.h:
http://gerrit.cloudera.org:8080/#/c/11752/4/src/kudu/tserver/tablet_service.h@113
PS4, Line 113: Rather than authorizing multiple tables at
: // once, if enforcing access control, we require the super-user role and omit
: // checking table privileges, and authorize as a client otherwise.
> The rationale for why this isn't contentious here is that, as far as Kudu's
Makes sense.
--
To view, visit http://gerrit.cloudera.org:8080/11752
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4bb2d09f23d7b77729e21060dad41c0501b17ded
Gerrit-Change-Number: 11752
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 08 Apr 2019 23:37:00 +0000
Gerrit-HasComments: Yes
[kudu-CR] authz: authorize ListTablets
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11752 )
Change subject: authz: authorize ListTablets
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11752/4/src/kudu/tserver/tablet_service.h
File src/kudu/tserver/tablet_service.h:
http://gerrit.cloudera.org:8080/#/c/11752/4/src/kudu/tserver/tablet_service.h@113
PS4, Line 113: Rather than authorizing multiple tables at
: // once, if enforcing access control, we require the super-user role and omit
: // checking table privileges, and authorize as a client otherwise.
> Should we do the same for ListTables in the master? Why or why not?
The rationale for why this isn't contentious here is that, as far as Kudu's codebase is concerned, this is used by tools only. Sure, there might be people using the endpoint directly, but that's quite unlikely.
This isn't the case for ListTables, which is a part of the public client API. I'd love for us to do this for ListTables, but it seems like a much higher-impact restriction. We could, and maybe that would be good enough. But it's less of a no-brainer IMO.
--
To view, visit http://gerrit.cloudera.org:8080/11752
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4bb2d09f23d7b77729e21060dad41c0501b17ded
Gerrit-Change-Number: 11752
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 08 Apr 2019 05:53:15 +0000
Gerrit-HasComments: Yes
[kudu-CR] authz: authorize ListTablets
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11752 )
Change subject: authz: authorize ListTablets
......................................................................
Patch Set 4: Verified+1
Flaky: KUDU-2718
--
To view, visit http://gerrit.cloudera.org:8080/11752
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4bb2d09f23d7b77729e21060dad41c0501b17ded
Gerrit-Change-Number: 11752
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <da...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 08 Apr 2019 06:07:05 +0000
Gerrit-HasComments: No