Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12919 )
Change subject: sentry: sanitize and parse privileges from Sentry ...................................................................... Patch Set 11: (3 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@83 PS11, Line 83: // The scope of the authorizable being granted privileges. : const sentry::SentryAuthorizableScope::Scope scope; > Can't we figure out the scope by checking which of our strings is empty? Th Defensive programming and avoiding some excessive branching at the cost of a stored enum, yeah. 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@424 PS11, Line 424: if (!privilege.__isset.columnName || privilege.columnName.empty()) { > Why do we need to validate __isset as well as the strings themselves? This is also just defensive. http://gerrit.cloudera.org:8080/#/c/12919/11/src/kudu/master/sentry_authz_provider.cc@524 PS11, Line 524: privilege.all_with_grant = true; > What should we do with privileges whose grant option was enabled but whose It should be fine to have a grant option on any action; but for Kudu we only really use it for "all with grant" or "owner with grant". In all other cases, the grant option can be ignored, but the privilege is still valid. -- 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: Sat, 06 Apr 2019 17:53:58 +0000 Gerrit-HasComments: Yes
