Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/12919 )
Change subject: sentry: sanitize and parse privileges from Sentry ...................................................................... Patch Set 11: (4 comments) http://gerrit.cloudera.org:8080/#/c/12919/11/src/kudu/master/sentry_authz_provider.h File src/kudu/master/sentry_authz_provider.h: http://gerrit.cloudera.org:8080/#/c/12919/11/src/kudu/master/sentry_authz_provider.h@51 PS11, Line 51: AuthorizablePrivileges(sentry::SentryAuthorizableScope::Scope scope, > My eye is immediately drawn to the lack of a 'server' field; a comment expl Done http://gerrit.cloudera.org:8080/#/c/12919/11/src/kudu/master/sentry_authz_provider.h@87 PS11, Line 87: sentry::SentryActionsSet granted_privileges; > I don't feel strongly, but I also don't understand your response. This seems to be the most bike-shadeable point in this changelist :) Thank you for the the explanation. Yep, I think it's very important to make sure we understand each other. Frankly, the Sentry terminology is confusing to me. I'm more used to Postgresql concept of privileges, and there CREATE, ALTER, etc. are privileges: https://www.postgresql.org/docs/9.1/ddl-priv.html https://www.postgresql.org/docs/9.0/sql-grant.html Probably, I should get used to the Sentry terminology and move on. However, how is it possible to grant an action? What is that? My point is that the semantics of 'action' doesn't imply it can be granted (but a privilege or a permission can). If so, what 'granted_actions' would mean? At least I could not find a notion of granting an action in the user-facing documentation: https://www.cloudera.com/documentation/enterprise/latest/topics/cm_sg_sentry_service.html Should we then rename 'granted_privileges' --> 'actions' and be fine? http://gerrit.cloudera.org:8080/#/c/12919/11/src/kudu/master/sentry_authz_provider.h@102 PS11, Line 102: the : // given table. > Yeah, as I commented in the other review, I think _encapsulating_ the enfor Yep, that code is now in SentryPrivilegesBranch::Init(...) http://gerrit.cloudera.org:8080/#/c/12919/11/src/kudu/master/sentry_authz_provider.cc File src/kudu/master/sentry_authz_provider.cc: http://gerrit.cloudera.org:8080/#/c/12919/11/src/kudu/master/sentry_authz_provider.cc@431 PS11, Line 431: !boost::iequals(privilege.tableName, requested_authorizable.table))) { > > I didn't understand the 'kServer' part of your comment. Ah, I see. I fixed the misprint: SentryAuthorizableScope::kSever --> SentryAuthorizableScope::kServer Thanks! -- To view, visit http://gerrit.cloudera.org:8080/12919 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib6de6814f99abfbee4f030298b74f21f4e7c729b Gerrit-Change-Number: 12919 Gerrit-PatchSet: 11 Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Wed, 10 Apr 2019 06:47:37 +0000 Gerrit-HasComments: Yes
