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

Change subject: [sentry] add AuthzProvider
......................................................................


Patch Set 12:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/11659/11/src/kudu/master/authz_provider.h
File src/kudu/master/authz_provider.h:

http://gerrit.cloudera.org:8080/#/c/11659/11/src/kudu/master/authz_provider.h@63
PS11, Line 63:   // Checks if retrieving metadata about the table is authorized 
for the
> nit: "metadata" seems like it might be a Sentry construct, which is fine, b
Done


http://gerrit.cloudera.org:8080/#/c/11659/11/src/kudu/master/sentry_authz_provider.h
File src/kudu/master/sentry_authz_provider.h:

http://gerrit.cloudera.org:8080/#/c/11659/11/src/kudu/master/sentry_authz_provider.h@39
PS11, Line 39:
             : // An implementation of AuthzProvider that connects to the 
Sentry Service
             : // for authorization metadata and allow or deny the actions 
performed by
             : // users based on the metadata.
> nit: "An implementation of AuthzProvider that connects to Apache Sentry for
I would prefer to not use Apache Sentry here in order to match how we refer 
'Sentry' in other places.


http://gerrit.cloudera.org:8080/#/c/11659/11/src/kudu/master/sentry_authz_provider.cc
File src/kudu/master/sentry_authz_provider.cc:

http://gerrit.cloudera.org:8080/#/c/11659/11/src/kudu/master/sentry_authz_provider.cc@246
PS11, Line 246:   return Authorize(db_authorizable, db_action, user);
              : }
              :
              : Status SentryAuthzProvider::AuthorizeGetTableMetadata(const 
std::string& table_name,
              :                                                       const 
std::string& user) {
              :   // Retrieving table metadata requires 'METADATA ON TABLE' 
privilege.
              :   TSentryAuthorizable authorizable;
              :   RETURN_NOT_OK(GetAuthorizable(table_name, 
AuthorizableScope::TABLE, &authorizable));
              :   SentryAction action = 
SentryAction(SentryAction::Action::METADATA);
              :   return Authorize(authorizable, action, user);
              : }
              :
> Now that this is tested elsewhere, it doesn't need to be part of the class.
Done



--
To view, visit http://gerrit.cloudera.org:8080/11659
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I254828d640cd905e33dbaf0fe100d660bc9e6772
Gerrit-Change-Number: 11659
Gerrit-PatchSet: 12
Gerrit-Owner: Hao Hao <hao....@cloudera.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <danburk...@apache.org>
Gerrit-Reviewer: Hao Hao <hao....@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Fri, 02 Nov 2018 16:24:07 +0000
Gerrit-HasComments: Yes

Reply via email to