Adar Dembo has posted comments on this change. (
http://gerrit.cloudera.org:8080/12833 )
Change subject: WIP [master] introduced SentryPrivilegesFetcher
......................................................................
Patch Set 9:
(2 comments)
The renaming of sentry_authz_privilege_fetcher.{h,cc} made the new files in
revisions after PS6 show up as new code, which made the review pretty
frustrating.
http://gerrit.cloudera.org:8080/#/c/12833/9/src/kudu/master/sentry_authz_provider-test.cc
File src/kudu/master/sentry_authz_provider-test.cc:
http://gerrit.cloudera.org:8080/#/c/12833/9/src/kudu/master/sentry_authz_provider-test.cc@426
PS9, Line 426: bool KerberosEnabled() const override {
: return std::get<1>(GetParam());
: }
> I chatted with Hao about this briefly, and it seems like non-Kerberos isn't
Yeah I had the same feedback in Andrew's patch. In my case, I didn't comment on
whether non-Kerberos is common and/or interesting, just that there didn't seem
to be any intersection between Kerberos functionality and Sentry functionality
(i.e. the former is used for authn and the latter for authz).
Certainly from a _product_ perspective we want users to enable authn (otherwise
their authz can be easily compromised), but from an integration testing
perspective, I think we'd be better off only testing the matrix where we expect
functional interactions.
http://gerrit.cloudera.org:8080/#/c/12833/9/src/kudu/master/sentry_privileges_fetcher.h
File src/kudu/master/sentry_privileges_fetcher.h:
http://gerrit.cloudera.org:8080/#/c/12833/9/src/kudu/master/sentry_privileges_fetcher.h@100
PS9, Line 100: // A representation of the Sentry privilege hierarchy branch for
a single table
I left some feedback in Andrew's patch about this comment: I think this
invariant would be clearer if enforced by SentryPrivilegesBranch itself. Right
now that's not the case; it's expected that when a SentryPrivilegesBranch is
constructed, the person constructing it remembers to enforce the invariant.
--
To view, visit http://gerrit.cloudera.org:8080/12833
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idaefacd50736f1f152dae34e76778e17b2e84cbe
Gerrit-Change-Number: 12833
Gerrit-PatchSet: 9
Gerrit-Owner: Alexey Serbin <[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: Mon, 08 Apr 2019 05:14:46 +0000
Gerrit-HasComments: Yes
