[kudu-CR] docs: add Ranger integration

[Hao Hao (Code Review)]

Hao Hao has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/15897 )

Change subject: docs: add Ranger integration
..

docs: add Ranger integration

Staged version here:
https://github.com/haohaoc/kudu/blob/ranger-docs/docs/security.adoc

Change-Id: Iad9476f18267c1e14a73f893fd812674c955eee2
Reviewed-on: http://gerrit.cloudera.org:8080/15897
Tested-by: Kudu Jenkins
Reviewed-by: Grant Henke 
---
M docs/kudu_impala_integration.adoc
M docs/security.adoc
2 files changed, 218 insertions(+), 17 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Grant Henke: Looks good to me, approved

--
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: merged
Gerrit-Change-Id: Iad9476f18267c1e14a73f893fd812674c955eee2
Gerrit-Change-Number: 15897
Gerrit-PatchSet: 3
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] docs: add Ranger integration

[Hao Hao (Code Review)]

Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15897 )

Change subject: docs: add Ranger integration
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15897/2/docs/security.adoc
File docs/security.adoc:

http://gerrit.cloudera.org:8080/#/c/15897/2/docs/security.adoc@506
PS2, Line 506: [[privilege-caching]]
 : === Kudu Master Caching for Sentry
 :
 : To avoid overwhelming Sentry with requests to fetch user 
privileges, the Kudu
 : master can be configured to cache user privileges. A by-product 
of this caching
 : is that when privileges are changed in Sentry, they may not be 
reflected in
 : Kudu for a configurable amount of time, defined by the following 
Kudu master
 : configurations:
 :
 : `--sentry_privileges_cache_ttl_factor * 
--authz_token_validity_interval_secs`
 :
 : The default value is fifty minutes. If privilege updates need to 
be reflected
 : in Kudu sooner than this, the Kudu CLI tool can be used to 
invalidate the
 : cached privileges to force Kudu to fetch new ones from Sentry:
 :
 : [source,bash]
 : 
 : kudu master authz_cache reset 
> We need these docs to land in master to exist going forward. We can and sho
Ack



--
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: 2
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 13 May 2020 18:21:24 +
Gerrit-HasComments: Yes

[kudu-CR] docs: add Ranger integration

[Grant Henke (Code Review)]

Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15897 )

Change subject: docs: add Ranger integration
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15897/2/docs/security.adoc
File docs/security.adoc:

http://gerrit.cloudera.org:8080/#/c/15897/2/docs/security.adoc@506
PS2, Line 506: [[privilege-caching]]
 : === Kudu Master Caching for Sentry
 :
 : To avoid overwhelming Sentry with requests to fetch user 
privileges, the Kudu
 : master can be configured to cache user privileges. A by-product 
of this caching
 : is that when privileges are changed in Sentry, they may not be 
reflected in
 : Kudu for a configurable amount of time, defined by the following 
Kudu master
 : configurations:
 :
 : `--sentry_privileges_cache_ttl_factor * 
--authz_token_validity_interval_secs`
 :
 : The default value is fifty minutes. If privilege updates need to 
be reflected
 : in Kudu sooner than this, the Kudu CLI tool can be used to 
invalidate the
 : cached privileges to force Kudu to fetch new ones from Sentry:
 :
 : [source,bash]
 : 
 : kudu master authz_cache reset 
> Wait, but this changelist is for master branch, not kudu-1.12.  That's why
We need these docs to land in master to exist going forward. We can and should 
backport to 1.12 as well. A separate patch can remove the Sentry docs before 
the 1.13 release.



--
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: 2
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 13 May 2020 17:40:02 +
Gerrit-HasComments: Yes

[kudu-CR] docs: add Ranger integration

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15897 )

Change subject: docs: add Ranger integration
..


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15897/2/docs/security.adoc
File docs/security.adoc:

http://gerrit.cloudera.org:8080/#/c/15897/2/docs/security.adoc@506
PS2, Line 506: [[privilege-caching]]
 : === Kudu Master Caching for Sentry
 :
 : To avoid overwhelming Sentry with requests to fetch user 
privileges, the Kudu
 : master can be configured to cache user privileges. A by-product 
of this caching
 : is that when privileges are changed in Sentry, they may not be 
reflected in
 : Kudu for a configurable amount of time, defined by the following 
Kudu master
 : configurations:
 :
 : `--sentry_privileges_cache_ttl_factor * 
--authz_token_validity_interval_secs`
 :
 : The default value is fifty minutes. If privilege updates need to 
be reflected
 : in Kudu sooner than this, the Kudu CLI tool can be used to 
invalidate the
 : cached privileges to force Kudu to fetch new ones from Sentry:
 :
 : [source,bash]
 : 
 : kudu master authz_cache reset 
> yes, that is for the next release (1.13.0). This release still supports bot
Wait, but this changelist is for master branch, not kudu-1.12.  That's why I 
asked.

Is that simply the wrong target branch for this patch?



--
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: 2
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 13 May 2020 16:15:43 +
Gerrit-HasComments: Yes

[kudu-CR] docs: add Ranger integration

[Grant Henke (Code Review)]

Grant Henke has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15897 )

Change subject: docs: add Ranger integration
..


Patch Set 2: Code-Review+2

(1 comment)

http://gerrit.cloudera.org:8080/#/c/15897/2/docs/security.adoc
File docs/security.adoc:

http://gerrit.cloudera.org:8080/#/c/15897/2/docs/security.adoc@506
PS2, Line 506: [[privilege-caching]]
 : === Kudu Master Caching for Sentry
 :
 : To avoid overwhelming Sentry with requests to fetch user 
privileges, the Kudu
 : master can be configured to cache user privileges. A by-product 
of this caching
 : is that when privileges are changed in Sentry, they may not be 
reflected in
 : Kudu for a configurable amount of time, defined by the following 
Kudu master
 : configurations:
 :
 : `--sentry_privileges_cache_ttl_factor * 
--authz_token_validity_interval_secs`
 :
 : The default value is fifty minutes. If privilege updates need to 
be reflected
 : in Kudu sooner than this, the Kudu CLI tool can be used to 
invalidate the
 : cached privileges to force Kudu to fetch new ones from Sentry:
 :
 : [source,bash]
 : 
 : kudu master authz_cache reset 
> Is this and other Sentry-related pieces relevant at all after removing Sent
yes, that is for the next release (1.13.0). This release still supports both.



--
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: 2
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 13 May 2020 13:37:32 +
Gerrit-HasComments: Yes

[kudu-CR] docs: add Ranger integration

[Alexey Serbin (Code Review)]

Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15897 )

Change subject: docs: add Ranger integration
..


Patch Set 2:

(1 comment)

Took a quick glance.

http://gerrit.cloudera.org:8080/#/c/15897/2/docs/security.adoc
File docs/security.adoc:

http://gerrit.cloudera.org:8080/#/c/15897/2/docs/security.adoc@506
PS2, Line 506: [[privilege-caching]]
 : === Kudu Master Caching for Sentry
 :
 : To avoid overwhelming Sentry with requests to fetch user 
privileges, the Kudu
 : master can be configured to cache user privileges. A by-product 
of this caching
 : is that when privileges are changed in Sentry, they may not be 
reflected in
 : Kudu for a configurable amount of time, defined by the following 
Kudu master
 : configurations:
 :
 : `--sentry_privileges_cache_ttl_factor * 
--authz_token_validity_interval_secs`
 :
 : The default value is fifty minutes. If privilege updates need to 
be reflected
 : in Kudu sooner than this, the Kudu CLI tool can be used to 
invalidate the
 : cached privileges to force Kudu to fetch new ones from Sentry:
 :
 : [source,bash]
 : 
 : kudu master authz_cache reset 
Is this and other Sentry-related pieces relevant at all after removing 
Sentry-related code in 
https://github.com/apache/kudu/commit/1ab6d50a891aa2efdb39d74af2ffe6f478c8b013 ?



--
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: 2
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Alexey Serbin 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 13 May 2020 04:02:40 +
Gerrit-HasComments: Yes

[kudu-CR] docs: add Ranger integration

[Hao Hao (Code Review)]

Hao Hao has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15897 )

Change subject: docs: add Ranger integration
..


Patch Set 1:

(38 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
> +1, maybe even a "WARNING:" message.
Done


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.
Done


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 config
Done


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 update
Done


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,
Done


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. Ma
Done


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 Impa
Done


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
> Yeah, it's probably worth explicitly saying that "ALL" implies all other ac
Done


http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@265
PS1, Line 265: DATABASE
> lowercase
Done


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
Done


http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@271
PS1, Line 271: the following < nit: "the following section" usually means "the section immediately followi
Done


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.
Done


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
Removed as Grant suggested.


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"
Removed as Grant suggested.


http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@279
PS1, Line 279: remains
> "remain"
Done


http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@280
PS1, Line 280: are
> "is"
Done


http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@285
PS1, Line 285: e.g. Sentry or Ranger
> Fine-grained authz and coarse-grained authn use different tokens, so I don'
Ack


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.
Done


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?
No, it is not, but It is best practice to enable Kerberos in a Ranger secured 
cluster.


http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@384
PS1, Line 384: should not be enabled
> nit: "can not be" as it's actually enforced. Also consider adding a similar
Done


http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@388
PS1, Line 388: Note it down
> nit: "Note its path"?
Done


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.
Done


http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@391
PS1, Line 391: are
> nit: that are
Done

[kudu-CR] docs: add Ranger integration

[Hao Hao (Code Review)]

Hello Attila Bukor, Kudu Jenkins, Andrew Wong, Grant Henke,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/15897

to look at the new patch set (#2).

Change subject: docs: add Ranger integration
..

docs: add Ranger integration

Staged version here:
https://github.com/haohaoc/kudu/blob/ranger-docs/docs/security.adoc

Change-Id: Iad9476f18267c1e14a73f893fd812674c955eee2
---
M docs/kudu_impala_integration.adoc
M docs/security.adoc
2 files changed, 218 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/15897/2
--
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: newpatchset
Gerrit-Change-Id: Iad9476f18267c1e14a73f893fd812674c955eee2
Gerrit-Change-Number: 15897
Gerrit-PatchSet: 2
Gerrit-Owner: Hao Hao 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] docs: add Ranger integration

[Andrew Wong (Code Review)]

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  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 
Gerrit-Reviewer: Andrew Wo

[kudu-CR] docs: add Ranger integration

[Grant Henke (Code Review)]

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=
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 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 11 May 2020 14:44:29 +
Gerrit-HasComments: Yes

[kudu-CR] docs: add Ranger integration

[Attila Bukor (Code Review)]

Attila Bukor has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15897 )

Change subject: docs: add Ranger integration
..


Patch Set 1:

(6 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@384
PS1, Line 384: should not be enabled
nit: "can not be" as it's actually enforced. Also consider adding a similar 
note to the Sentry section.


http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@391
PS1, Line 391: are
nit: that are


http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@468
PS1, Line 468: may facilitate
nit: add an explanation when this is necessary (backup, checksum scan, etc), 
and what are the potential drawbacks (admins who can usually access the kudu 
keytab can do anything)


http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@473
PS1, Line 473: configurations
nit: also add an explanation what this is for


http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@486
PS1, Line 486: 
isn't tag.download.auth.users also necessary? or is that only for tag-based 
which we don't support for now anyway?


http://gerrit.cloudera.org:8080/#/c/15897/1/docs/security.adoc@518
PS1, Line 518: can determine
nit: to set



--
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 
Gerrit-Reviewer: Andrew Wong 
Gerrit-Reviewer: Attila Bukor 
Gerrit-Reviewer: Grant Henke 
Gerrit-Reviewer: Hao Hao 
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 11 May 2020 13:12:39 +
Gerrit-HasComments: Yes

[kudu-CR] docs: add Ranger integration

[Hao Hao (Code Review)]

Hao Hao has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/15897


Change subject: docs: add Ranger integration
..

docs: add Ranger integration

Staged version here:
https://github.com/haohaoc/kudu/blob/ranger-docs/docs/security.adoc

Change-Id: Iad9476f18267c1e14a73f893fd812674c955eee2
---
M docs/security.adoc
1 file changed, 201 insertions(+), 16 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/15897/1
--
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: newchange
Gerrit-Change-Id: Iad9476f18267c1e14a73f893fd812674c955eee2
Gerrit-Change-Number: 15897
Gerrit-PatchSet: 1
Gerrit-Owner: Hao Hao