Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/15897 )
Change subject: docs: add Ranger integration ...................................................................... Patch Set 1: (17 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 Ranger is preferred going forward. http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@223 PS1, Line 223: === Apache Ranger nit: Put this section above the Sentry section since it is preferred. http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@226 PS1, Line 226: fashion as Apache Sentry, but without `Server` scope in the hierarchy. We should not define things by relating them to Apache Sentry. Users configuring Ranger may not be familiar with Sentry and have no reason to read the Sentry docs. We will also be removing the Sentry docs at some point. Even if we duplicate a bunch of information from the Sentry docs, I think that is preferred. http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@233 PS1, Line 233: * *Database* - indicated as a prefix of table names with the format May preface with. "Kudu does not have the concept of a database. Therefore, a database is indicated as a prefix..." http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@237 PS1, Line 237: `impala::db.table` will be `impala::db`. Is there more description of this "impala::" oddity in the HMS sync or Impala docs? We might want to link to it for more context. 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. http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@275 PS1, Line 275: NOTE: Enabling of Ranger integration does not require the cluster to be integrated I don't think this note is required since a new user wouldn't expect this. This is more appropriate for a Sentry migration guide. 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? http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@379 PS1, Line 379: === Configuring the Integration with Apache Ranger nit: Put this section above the Sentry section since it is preferred. http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@381 PS1, Line 381: NOTE: Ranger is often configured with Kerberos authentication. See Is Kerberos authentication required? http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@392 PS1, Line 392: not Ranger-compatible, which are names begin or end with a period. Also check What happens if a user enables Ranger with incompatible table names? http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@397 PS1, Line 397: * Create Ranger client `ranger-hive-security.xml` configuration file, and note down Are there Ranger docs we can link to for this configuration? http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@459 PS1, Line 459: master nit: Kudu master 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 description of the flags. http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@463 PS1, Line 463: --ranger_java_path=<Path where the Java binary was installed> If `--ranger_java_path` isn't set will it use $JAVA_HOME, look in the PATH or "search" for it? http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@476 PS1, Line 476: --tserver_enforce_access_control=true Not related to this patch, but it might be a good idea to add some validation (maybe in ksck or something) that this is set if an authz integration is enabled on the master. http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@493 PS1, Line 493: === Caching Maybe these should live under the respective configuring sections for each service instead of in a section of its own. -- 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 14:44:29 +0000 Gerrit-HasComments: Yes
