Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/15897 )
Change subject: docs: add Ranger integration ...................................................................... Patch Set 1: (19 comments) http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc File docs/security.adoc: http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@160 PS1, Line 160: defined in Apache Sentry 2.2 and later. In addition, starting from Kudu > This is probably a good place to mention that Sentry is deprecated and Rang +1, maybe even a "WARNING:" message. http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@228 PS1, Line 228: NOTE: Ranger allows you to add separate service repositories to manage nit: maybe also mention the Ranger config file that would need to be updated to point to a different privilege repository? http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@234 PS1, Line 234: Since Kudu's only restriction on table names is : that they be a valid UTF-8 encoded string, which remains the same with : Ranger integration, the database in this case for tables named such as : `impala::db.table` will be `impala::db`. nit: this is a bit awkward because it's half guidance, and half example. Maybe separate it as: "Since Kudu's only restriction on table names is that they be valid UTF-8 encoded strings, Kudu considers special characters to be valid parts of database or table names. For example, if a table is named `impala::db.table`, its database will be `impala::db`." http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@256 PS1, Line 256: All of the actions have the same semantics as in Sentry with the exception that > Same comment about relating to Sentry here. Yeah, it's probably worth explicitly saying that "ALL" implies all other actions, and any action implies "METADATA". http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@265 PS1, Line 265: DATABASE lowercase http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@266 PS1, Line 266: `SELECT` privilege on `db=a->table=*->column=*` : to match the semantics of `SELECT` "the `SELECT` privilege" for both of these http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@271 PS1, Line 271: the following <<security.adoc#policy-for-kudu-masters,section nit: "the following section" usually means "the section immediately following this one", which isn't the case here. Maybe just refer to it as "the Policy for Kudu Masters" http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@277 PS1, Line 277: Apache Hive Metastore. nit: maybe also mention Impala, which uses the same Ranger policies as Hive? http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@277 PS1, Line 277: is Kudu specific, and are not in sync "are Kudu specific, and are not synchronized" http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@279 PS1, Line 279: remains "remain" http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@280 PS1, Line 280: are "is" http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@285 PS1, Line 285: e.g. Sentry or Ranger > Coarse grained auth as well? Fine-grained authz and coarse-grained authn use different tokens, so I don't think it's worth brining up coarse-grained authn here. http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@388 PS1, Line 388: Note it down nit: "Note its path"? http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@389 PS1, Line 389: Ranger subprocess Users probably don't know what this is, so it's probably worth elaborating. Maybe: "the Ranger subprocess, which houses the Ranger client that Kudu will use to communicate with the Ranger server" http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@392 PS1, Line 392: names begin or end "names that begin" http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@393 PS1, Line 393: there are no multiple Kudu tables whose names only differ by case since : policies authorization are case-insensitive "that there are no two table names that only differ by case, since authorization is case-insensitive." http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@394 PS1, Line 394: comply : the requirements "comply with the requirements" http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@397 PS1, Line 397: hive Shouldn't this be `ranger-kudu-security.xml`? http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@461 PS1, Line 461: ``` > It could be useful to provide example values here along with the descriptio +1 It'd also be nice to give examples of the full path of various important files that exist so users can cross-reference the flags with what files they see. -- To view, visit http://gerrit.cloudera.org:8080/15897 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iad9476f18267c1e14a73f893fd812674c955eee2 Gerrit-Change-Number: 15897 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Attila Bukor <abu...@apache.org> Gerrit-Reviewer: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 11 May 2020 19:29:17 +0000 Gerrit-HasComments: Yes
