Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/12877 )
Change subject: [sentry] enable sentry integration for master stress test ...................................................................... Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/12877/4/src/kudu/integration-tests/cluster_itest_util.h File src/kudu/integration-tests/cluster_itest_util.h: http://gerrit.cloudera.org:8080/#/c/12877/4/src/kudu/integration-tests/cluster_itest_util.h@454 PS4, Line 454: : // When Sentry is enabled, set up super privilege for 'admin' group (via : // creating an admin role and grant privilege 'ALL' on server to the role), : // so that 'test-admin' user, as who all integration tests are running, : // belong to 'admin' group can operate on the cluster. As a user of this function, it isn't clear what super privileges are. Also the second part of the sentence doesn't read quite right. This also doesn't document what 'kdc' or 'address' are, or the fact that a user should expect 'test-admin' to be logged in after calling this. Consider: "Grants the 'test-admin' user Sentry privileges to perform any operation, using 'kdc' to authenticate with the Sentry instance at 'address'. Once called, the 'test-admin' user will be logged in." http://gerrit.cloudera.org:8080/#/c/12877/2/src/kudu/integration-tests/cluster_itest_util.h File src/kudu/integration-tests/cluster_itest_util.h: http://gerrit.cloudera.org:8080/#/c/12877/2/src/kudu/integration-tests/cluster_itest_util.h@458 PS2, Line 458: // belong to 'admin' group can operate on the cluster. : Status SetupAdministratorPrivileges(MiniKdc* kdc, : const HostPort& address); : > Added more clarification. If this is tied to ExternalMiniCluster::Start(), did you consider putting it there? Why does this need to be a stand-alone piece? Wouldn't we always want to do this when Sentry is enabled with an EMC? -- To view, visit http://gerrit.cloudera.org:8080/12877 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic48aa1bfd0947c645bb81137bb34e6cdfc088cf4 Gerrit-Change-Number: 12877 Gerrit-PatchSet: 4 Gerrit-Owner: Hao Hao <[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: Tue, 23 Apr 2019 05:36:42 +0000 Gerrit-HasComments: Yes
