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 2019/06/28 06:45:18 UTC
[kudu-CR] security: add docs for Sentry
Hello Alexey Serbin, Grant Henke, Hao Hao,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/13759
to review the following change.
Change subject: security: add docs for Sentry
......................................................................
security: add docs for Sentry
Staged version here:
https://github.com/andrwng/kudu/blob/sentry-docs/docs/security.adoc
Change-Id: Ie50bb11a9a5d2d2294cf0ac34ccd7d75aa2cbcdf
---
M docs/security.adoc
1 file changed, 184 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/59/13759/1
--
To view, visit http://gerrit.cloudera.org:8080/13759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie50bb11a9a5d2d2294cf0ac34ccd7d75aa2cbcdf
Gerrit-Change-Number: 13759
Gerrit-PatchSet: 1
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
[kudu-CR] docs: add info about Sentry
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13759 )
Change subject: docs: add info about Sentry
......................................................................
Patch Set 7: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/13759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie50bb11a9a5d2d2294cf0ac34ccd7d75aa2cbcdf
Gerrit-Change-Number: 13759
Gerrit-PatchSet: 7
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 03 Jul 2019 05:48:35 +0000
Gerrit-HasComments: No
[kudu-CR] docs: add info about Sentry
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13759 )
Change subject: docs: add info about Sentry
......................................................................
Patch Set 6:
(19 comments)
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc
File docs/security.adoc:
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@154
PS4, Line 154: Fine-Grained
> nit: Fine-grained ? Not sure whether they usually capitalize the after-hyp
after the hyphen in a title is fine
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@160
PS4, Line 160:
> Maybe, add a note for about exposing possibly sensitive information via deb
Done
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@165
PS4, Line 165: web-ui,i
> Maybe, add some information to specify what this semantically means (like t
Done
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@192
PS4, Line 192:
> nit: I'm not sure whether it's intentional, but I can see two different pre
Done
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@211
PS4, Line 211: example, in Sentry deployments that don't support `UPDATE` privileges, to
> Maybe, explicitly mention that all DDL requests are processed by Kudu maste
I list the authorization policy for Masters and Tablet Servers below. I'm hesitant to blanket all Master requests as DDL since not all of them are necessarily DDL (e.g. GetTabletLocations).
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@211
PS4, Line 211: ents that don't support `UP
> maybe just 'to perform or reject the requested action' ?
Done
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@211
PS4, Line 211: ple, in Sentry deploy
> Not sure I see where and what is described above. Drop?
Done
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@216
PS4, Line 216: er has. If
> propagated
Done
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@217
PS4, Line 217: action, the
> encapsulate
Done
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@218
PS4, Line 218:
:
> Why not present simple tense?
Done
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@221
PS4, Line 221:
> Does it make sense to mention that DDL operations are authorized not via au
Done
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@222
PS4, Line 222: ry,
: privileges are p
> ... the window of potential ...
Done
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@225
PS4, Line 225: pening a Kudu table. Kudu
> nit: maybe, use present simple tense instead?
Done
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@231
PS4, Line 231: tablet servers in a cluster is much higher than the number of Kudu masters,
> ... or if the token isn't valid.
Done
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@263
PS4, Line 263: tion>
> drop ?
Done
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@264
PS4, Line 264:
: --sentry_serv
> maybe, mention that this is one of the master's flag
Done
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@269
PS4, Line 269: own. The 'had
> nit: use present simple instead?
Done
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@271
PS4, Line 271:
> ... authorizes requests on its own, ...
Done
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@276
PS4, Line 276:
> nit: use present tense ?
Done
--
To view, visit http://gerrit.cloudera.org:8080/13759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie50bb11a9a5d2d2294cf0ac34ccd7d75aa2cbcdf
Gerrit-Change-Number: 13759
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 02 Jul 2019 02:56:35 +0000
Gerrit-HasComments: Yes
[kudu-CR] docs: add info about Sentry
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13759 )
Change subject: docs: add info about Sentry
......................................................................
Patch Set 7:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/13759/6/docs/security.adoc
File docs/security.adoc:
http://gerrit.cloudera.org:8080/#/c/13759/6/docs/security.adoc@218
PS6, Line 218: a
> a ?
Done
http://gerrit.cloudera.org:8080/#/c/13759/6/docs/security.adoc@291
PS6, Line 291:
> I'm not sure I understand this piece.
Done
--
To view, visit http://gerrit.cloudera.org:8080/13759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie50bb11a9a5d2d2294cf0ac34ccd7d75aa2cbcdf
Gerrit-Change-Number: 13759
Gerrit-PatchSet: 7
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 02 Jul 2019 05:55:28 +0000
Gerrit-HasComments: Yes
[kudu-CR] docs: add info about Sentry
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13759 )
Change subject: docs: add info about Sentry
......................................................................
Patch Set 4:
(6 comments)
LGTM, thanks a lot for documenting it! Though it would be good to call out disabling web UI as we discussed offline. +1 to let Alexey to review it as well.
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc
File docs/security.adoc:
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@199
PS2, Line 199: details about Sentry
> I already called out 2.2 elsewhere. I prefer this being generic and not tie
LGTM, thanks for the update!
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@219
PS2, Line 219: matically attach authorizat
> This is noted in the Caching section.
Ack
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@228
PS2, Line 228: a tablet server that has been configured to enforce fine-grained access
:
> That seems like it should be documented in the HMS docs, no? That doesn't h
Hmm, yeah, makes sense after thinking again.
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@272
PS2, Line 272: d extra
> I don't think so, if questions come up about it, they can ask on mailing li
Ack
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@301
PS2, Line 301:
> I would choose either "with no rename" or "without a rename"; I'm leaving t
Ack
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@321
PS2, Line 321: === Policy for Kudu Tablet Servers
> I've pointed at Impala authorization docs elsewhere. Is that not sufficient
Yeah, the note you added sounds good enough.
--
To view, visit http://gerrit.cloudera.org:8080/13759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie50bb11a9a5d2d2294cf0ac34ccd7d75aa2cbcdf
Gerrit-Change-Number: 13759
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 01 Jul 2019 21:21:11 +0000
Gerrit-HasComments: Yes
[kudu-CR] docs: add info about Sentry
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Alex Rodoni, Alexey Serbin, Kudu Jenkins, Grant Henke, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13759
to look at the new patch set (#6).
Change subject: docs: add info about Sentry
......................................................................
docs: add info about Sentry
I also removed a few security limitations that no longer apply.
Staged version here:
https://github.com/andrwng/kudu/blob/sentry-docs/docs/security.adoc
Change-Id: Ie50bb11a9a5d2d2294cf0ac34ccd7d75aa2cbcdf
---
M docs/security.adoc
1 file changed, 221 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/59/13759/6
--
To view, visit http://gerrit.cloudera.org:8080/13759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie50bb11a9a5d2d2294cf0ac34ccd7d75aa2cbcdf
Gerrit-Change-Number: 13759
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] docs: add info about Sentry
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13759 )
Change subject: docs: add info about Sentry
......................................................................
docs: add info about Sentry
I also removed a few security limitations that no longer apply.
Staged version here:
https://github.com/andrwng/kudu/blob/sentry-docs/docs/security.adoc
Change-Id: Ie50bb11a9a5d2d2294cf0ac34ccd7d75aa2cbcdf
Reviewed-on: http://gerrit.cloudera.org:8080/13759
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Grant Henke <gr...@apache.org>
---
M docs/security.adoc
1 file changed, 221 insertions(+), 9 deletions(-)
Approvals:
Kudu Jenkins: Verified
Alexey Serbin: Looks good to me, approved
Grant Henke: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/13759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie50bb11a9a5d2d2294cf0ac34ccd7d75aa2cbcdf
Gerrit-Change-Number: 13759
Gerrit-PatchSet: 8
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Priyanka Chheda <pc...@cloudera.com>
[kudu-CR] security: add docs for Sentry
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13759 )
Change subject: security: add docs for Sentry
......................................................................
Patch Set 2:
(14 comments)
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc
File docs/security.adoc:
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@175
PS2, Line 175: Privileges granted on a higher scope imply privileges on a lower
: scope
Maybe add a reference to Apache sentry docs? For example, https://cwiki.apache.org/confluence/display/SENTRY/Sentry+Privileges
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@193
PS2, Line 193: that user has `METADATA` privileges
Maybe mention `METADATA` privilege is only defined in Kudu. Unlike `UPDATE` and `DELETE`, I think there are few chances `METADATA` will become a Sentry privilege.
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@199
PS2, Line 199: in Sentry deployments
Maybe good to call out as of Sentry 2.2, `UPDATE` and `DELETE` are not supported yet.
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@206
PS2, Line 206: off of
nit: of?
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@219
PS2, Line 219: for five minutes by default
Maybe mention this can be adjusted by flag, since it controls how long it takes to have the newly privileges granted in sentry to take effect.
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@228
PS2, Line 228: Configuring the Integration with Apache Sentry
:
I think we should note that when Sentry HDFS sync feature in enabled, kudu needs to be in hive group.
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@234
PS2, Line 234: documentation
nit: add a link?
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@241
PS2, Line 241: server1
Mention this needs to match what is set in Hive?
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@267
PS2, Line 267: access requirements
nit: maybe use 'fine-grained authorization'?
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@272
PS2, Line 272: Caching
Do we need to talk about when and how to config the capacity of the cache (sentry_privileges_cache_capacity_mb)?
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@274
PS2, Line 274: requests
nit: maybe 'privilege retrieval requests' to be more specific.
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@299
PS2, Line 299: GRANT OPTION
Explain grant option a bit? Apache Sentry reference is https://cwiki.apache.org/confluence/display/SENTRY/Support+Delegated+GRANT+and+REVOKE+in+Hive+and+Impala.
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@301
PS2, Line 301: with no
nit: without
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@321
PS2, Line 321: `METADATA ON TABLE` and `SELECT ON COLUMN`
Note the differences of the required privileges in this scenario between Kudu and Impala?
--
To view, visit http://gerrit.cloudera.org:8080/13759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie50bb11a9a5d2d2294cf0ac34ccd7d75aa2cbcdf
Gerrit-Change-Number: 13759
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sat, 29 Jun 2019 00:58:53 +0000
Gerrit-HasComments: Yes
[kudu-CR] docs: add info about Sentry
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Alex Rodoni, Alexey Serbin, Kudu Jenkins, Grant Henke, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13759
to look at the new patch set (#5).
Change subject: docs: add info about Sentry
......................................................................
docs: add info about Sentry
I also removed a few security limitations that no longer apply.
Staged version here:
https://github.com/andrwng/kudu/blob/sentry-docs/docs/security.adoc
Change-Id: Ie50bb11a9a5d2d2294cf0ac34ccd7d75aa2cbcdf
---
M docs/security.adoc
1 file changed, 215 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/59/13759/5
--
To view, visit http://gerrit.cloudera.org:8080/13759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie50bb11a9a5d2d2294cf0ac34ccd7d75aa2cbcdf
Gerrit-Change-Number: 13759
Gerrit-PatchSet: 5
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] security: add docs for Sentry
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/13759 )
Change subject: security: add docs for Sentry
......................................................................
Patch Set 3:
(13 comments)
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc
File docs/security.adoc:
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@175
PS2, Line 175: Privileges granted on a higher scope imply privileges on a lower
: scope
> Maybe add a reference to Apache sentry docs? For example, https://cwiki.apa
Done
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@193
PS2, Line 193: l privilege per se, rather, it is a
> Maybe mention `METADATA` privilege is only defined in Kudu. Unlike `UPDATE`
Done
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@199
PS2, Line 199: details about Sentry
> Maybe good to call out as of Sentry 2.2, `UPDATE` and `DELETE` are not supp
I already called out 2.2 elsewhere. I prefer this being generic and not tied to a version if it doesn't need to be. WDYT
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@206
PS2, Line 206:
> nit: of?
Not quite, but fixed.
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@219
PS2, Line 219: matically attach authorizat
> Maybe mention this can be adjusted by flag, since it controls how long it t
This is noted in the Caching section.
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@228
PS2, Line 228: a tablet server that has been configured to enforce fine-grained access
:
> I think we should note that when Sentry HDFS sync feature in enabled, kudu
That seems like it should be documented in the HMS docs, no? That doesn't have anything to do with fine-grained access control IMO.
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@241
PS2, Line 241: re.adoc
> Mention this needs to match what is set in Hive?
Done
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@267
PS2, Line 267:
> nit: maybe use 'fine-grained authorization'?
Done
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@272
PS2, Line 272: d extra
> Do we need to talk about when and how to config the capacity of the cache (
I don't think so, if questions come up about it, they can ask on mailing lists or on Slack. AFAWCT the default is sufficient.
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@274
PS2, Line 274:
> nit: maybe 'privilege retrieval requests' to be more specific.
Done
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@299
PS2, Line 299:
> Explain grant option a bit? Apache Sentry reference is https://cwiki.apache
Just added a link.
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@301
PS2, Line 301:
> nit: without
I would choose either "with no rename" or "without a rename"; I'm leaving this as is.
http://gerrit.cloudera.org:8080/#/c/13759/2/docs/security.adoc@321
PS2, Line 321: === Policy for Kudu Tablet Servers
> Note the differences of the required privileges in this scenario between Ku
I've pointed at Impala authorization docs elsewhere. Is that not sufficient?
--
To view, visit http://gerrit.cloudera.org:8080/13759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie50bb11a9a5d2d2294cf0ac34ccd7d75aa2cbcdf
Gerrit-Change-Number: 13759
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Sun, 30 Jun 2019 21:53:03 +0000
Gerrit-HasComments: Yes
[kudu-CR] docs: add info about Sentry
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13759 )
Change subject: docs: add info about Sentry
......................................................................
Patch Set 7: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/13759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie50bb11a9a5d2d2294cf0ac34ccd7d75aa2cbcdf
Gerrit-Change-Number: 13759
Gerrit-PatchSet: 7
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Priyanka Chheda <pc...@cloudera.com>
Gerrit-Comment-Date: Wed, 03 Jul 2019 11:28:32 +0000
Gerrit-HasComments: No
[kudu-CR] security: add docs for Sentry
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Alex Rodoni, Alexey Serbin, Kudu Jenkins, Grant Henke, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13759
to look at the new patch set (#2).
Change subject: security: add docs for Sentry
......................................................................
security: add docs for Sentry
Staged version here:
https://github.com/andrwng/kudu/blob/sentry-docs/docs/security.adoc
Change-Id: Ie50bb11a9a5d2d2294cf0ac34ccd7d75aa2cbcdf
---
M docs/security.adoc
1 file changed, 184 insertions(+), 0 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/59/13759/2
--
To view, visit http://gerrit.cloudera.org:8080/13759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie50bb11a9a5d2d2294cf0ac34ccd7d75aa2cbcdf
Gerrit-Change-Number: 13759
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] docs: add info about Sentry
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13759 )
Change subject: docs: add info about Sentry
......................................................................
Patch Set 4:
(19 comments)
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc
File docs/security.adoc:
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@154
PS4, Line 154: Fine-Grained
nit: Fine-grained ? Not sure whether they usually capitalize the after-hyphen part
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@160
PS4, Line 160:
Maybe, add a note for about exposing possibly sensitive information via debug Web server even if fine-grained authz is configured in Kudu v1.10
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@165
PS4, Line 165: *Server*
Maybe, add some information to specify what this semantically means (like there a single Kudu table behind *Table*)
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@192
PS4, Line 192: for
nit: I'm not sure whether it's intentional, but I can see two different prepositions for describing privileges related to an object: 'on' and 'for'. Is there any difference between those? If not, maybe converge on to just one preposition (e.g., 'on')?
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@211
PS4, Line 211: to base access decisions on
maybe just 'to perform or reject the requested action' ?
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@211
PS4, Line 211: user, described in above, to base access decisions on.
Maybe, explicitly mention that all DDL requests are processed by Kudu master?
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@211
PS4, Line 211: , described in above,
Not sure I see where and what is described above. Drop?
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@216
PS4, Line 216: propogated
propagated
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@217
PS4, Line 217: enacapsulate
encapsulate
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@218
PS4, Line 218: Kudu
: clients will automatically attach
Why not present simple tense?
Kudu clients automatically attach ...
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@221
PS4, Line 221:
Does it make sense to mention that DDL operations are authorized not via authz tokens, but via direct authz calls to Sentry/privileges cache?
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@222
PS4, Line 222: the
: window potential
... the window of potential ...
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@225
PS4, Line 225: will automatically retrieve
nit: maybe, use present simple tense instead?
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@231
PS4, Line 231: operation.
... or if the token isn't valid.
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@263
PS4, Line 263: and all
drop ?
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@264
PS4, Line 264: `--trusted_user_acl`
: configuration
maybe, mention that this is one of the master's flag
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@269
PS4, Line 269: will authorize
nit: use present simple instead?
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@271
PS4, Line 271: can authorize requests, if using Impala,
... authorizes requests on its own, ...
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@276
PS4, Line 276: will be
nit: use present tense ?
--
To view, visit http://gerrit.cloudera.org:8080/13759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie50bb11a9a5d2d2294cf0ac34ccd7d75aa2cbcdf
Gerrit-Change-Number: 13759
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 02 Jul 2019 01:55:17 +0000
Gerrit-HasComments: Yes
[kudu-CR] security: add docs for Sentry
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Alex Rodoni, Alexey Serbin, Kudu Jenkins, Grant Henke, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13759
to look at the new patch set (#3).
Change subject: security: add docs for Sentry
......................................................................
security: add docs for Sentry
Staged version here:
https://github.com/andrwng/kudu/blob/sentry-docs/docs/security.adoc
Change-Id: Ie50bb11a9a5d2d2294cf0ac34ccd7d75aa2cbcdf
---
M docs/security.adoc
1 file changed, 200 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/59/13759/3
--
To view, visit http://gerrit.cloudera.org:8080/13759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie50bb11a9a5d2d2294cf0ac34ccd7d75aa2cbcdf
Gerrit-Change-Number: 13759
Gerrit-PatchSet: 3
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] docs: add info about Sentry
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/13759 )
Change subject: docs: add info about Sentry
......................................................................
Patch Set 6: Code-Review+1
(3 comments)
A couple of nits, otherwise LGTM
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc
File docs/security.adoc:
http://gerrit.cloudera.org:8080/#/c/13759/4/docs/security.adoc@154
PS4, Line 154: Fine-Grained
> after the hyphen in a title is fine
Ack
http://gerrit.cloudera.org:8080/#/c/13759/6/docs/security.adoc
File docs/security.adoc:
http://gerrit.cloudera.org:8080/#/c/13759/6/docs/security.adoc@218
PS6, Line 218: an
a ?
http://gerrit.cloudera.org:8080/#/c/13759/6/docs/security.adoc@291
PS6, Line 291: Since authorizes requests on its own, if using Impala,
I'm not sure I understand this piece.
Maybe, you meant 'Since Impala authorizes requests on its own, to avoid extraneous ... ' ?
--
To view, visit http://gerrit.cloudera.org:8080/13759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie50bb11a9a5d2d2294cf0ac34ccd7d75aa2cbcdf
Gerrit-Change-Number: 13759
Gerrit-PatchSet: 6
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 02 Jul 2019 04:25:52 +0000
Gerrit-HasComments: Yes
[kudu-CR] docs: add info about Sentry
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/13759 )
Change subject: docs: add info about Sentry
......................................................................
Patch Set 7: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/13759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie50bb11a9a5d2d2294cf0ac34ccd7d75aa2cbcdf
Gerrit-Change-Number: 13759
Gerrit-PatchSet: 7
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 02 Jul 2019 19:08:17 +0000
Gerrit-HasComments: No
[kudu-CR] docs: add info about Sentry
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Alex Rodoni, Alexey Serbin, Kudu Jenkins, Grant Henke, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13759
to look at the new patch set (#7).
Change subject: docs: add info about Sentry
......................................................................
docs: add info about Sentry
I also removed a few security limitations that no longer apply.
Staged version here:
https://github.com/andrwng/kudu/blob/sentry-docs/docs/security.adoc
Change-Id: Ie50bb11a9a5d2d2294cf0ac34ccd7d75aa2cbcdf
---
M docs/security.adoc
1 file changed, 221 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/59/13759/7
--
To view, visit http://gerrit.cloudera.org:8080/13759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie50bb11a9a5d2d2294cf0ac34ccd7d75aa2cbcdf
Gerrit-Change-Number: 13759
Gerrit-PatchSet: 7
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] docs: add info about Sentry
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Hello Alex Rodoni, Alexey Serbin, Kudu Jenkins, Grant Henke, Hao Hao,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13759
to look at the new patch set (#4).
Change subject: docs: add info about Sentry
......................................................................
docs: add info about Sentry
I also removed a few security limitations that no longer apply.
Staged version here:
https://github.com/andrwng/kudu/blob/sentry-docs/docs/security.adoc
Change-Id: Ie50bb11a9a5d2d2294cf0ac34ccd7d75aa2cbcdf
---
M docs/security.adoc
1 file changed, 200 insertions(+), 9 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/59/13759/4
--
To view, visit http://gerrit.cloudera.org:8080/13759
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie50bb11a9a5d2d2294cf0ac34ccd7d75aa2cbcdf
Gerrit-Change-Number: 13759
Gerrit-PatchSet: 4
Gerrit-Owner: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)