[kudu-CR] [sentry] add mini Sentry to the external mini cluster
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/11956 )
Change subject: [sentry] add mini Sentry to the external mini cluster
..
Patch Set 3:
(11 comments)
http://gerrit.cloudera.org:8080/#/c/11956/3/src/kudu/integration-tests/CMakeLists.txt
File src/kudu/integration-tests/CMakeLists.txt:
http://gerrit.cloudera.org:8080/#/c/11956/3/src/kudu/integration-tests/CMakeLists.txt@91
PS3, Line 91: ADD_KUDU_TEST(master_sentry-itest RUN_SERIAL true PROCESSORS 4)
> How did you arrive at the PROCESSORS value? And why use RUN_SERIAL here?
I think this is similar to the test in master_hms-itest, so I used its config
as a reference.
http://gerrit.cloudera.org:8080/#/c/11956/3/src/kudu/integration-tests/master_sentry-itest.cc
File src/kudu/integration-tests/master_sentry-itest.cc:
http://gerrit.cloudera.org:8080/#/c/11956/3/src/kudu/integration-tests/master_sentry-itest.cc@55
PS3, Line 55: const ExternalMiniCluster::BindMode bind_mode;
> Looks like you're not using this param in SetUp(). Nor do I think you need
Done
http://gerrit.cloudera.org:8080/#/c/11956/3/src/kudu/integration-tests/master_sentry-itest.cc@71
PS3, Line 71: opts.enable_sentry = true;
> What happens if enable_sentry=true but enable_kerberos=false? The MiniSentr
Yes. In the follow up patch I removed the situation when enable_kerberos=false
since it doesn't make much sense for Sentry integration test.
http://gerrit.cloudera.org:8080/#/c/11956/3/src/kudu/integration-tests/master_sentry-itest.cc@74
PS3, Line 74: opts.num_masters = 1;
: opts.num_tablet_servers = 1;
> These are the defaults; you can omit this.
Done
http://gerrit.cloudera.org:8080/#/c/11956/3/src/kudu/integration-tests/master_sentry-itest.cc@93
PS3, Line 93: TestFoo
> Rename to something more useful.
I revised this test to have a meaningful test in
https://gerrit.cloudera.org/#/c/11797/3/src/kudu/integration-tests/master_sentry-itest.cc
http://gerrit.cloudera.org:8080/#/c/11956/3/src/kudu/integration-tests/master_sentry-itest.cc@111
PS3, Line 111: exist
> Nit: exists
Done
http://gerrit.cloudera.org:8080/#/c/11956/3/src/kudu/integration-tests/master_sentry-itest.cc@116
PS3, Line 116: const auto& params = GetParam();
> Only used in one place below, so just inline GetParam().
Done
http://gerrit.cloudera.org:8080/#/c/11956/3/src/kudu/mini-cluster/mini_cluster.cc
File src/kudu/mini-cluster/mini_cluster.cc:
http://gerrit.cloudera.org:8080/#/c/11956/3/src/kudu/mini-cluster/mini_cluster.cc@52
PS3, Line 52: int idx;
: switch (type) {
: case MASTER:
: idx = kServersMaxNum - index;
: break;
: case TSERVER:
: idx = index + 1;
: break;
: case EXTERNAL_SERVER:
: idx = kServersMaxNum / 3 + index;
: break;
> Not really your fault, but could you add a comment here walking through wha
Done
http://gerrit.cloudera.org:8080/#/c/11956/3/src/kudu/util/test_util.cc
File src/kudu/util/test_util.cc:
http://gerrit.cloudera.org:8080/#/c/11956/3/src/kudu/util/test_util.cc@a421
PS3, Line 421:
:
> This structure of the pairs is lost in the new comment. Mind keeping it aro
Done
http://gerrit.cloudera.org:8080/#/c/11956/3/src/kudu/util/test_util.cc@402
PS3, Line 402: // The '-Ffn' flag gets lsof to output something like:
: // p5801
: // f548
: // n127.0.0.1:43954->127.0.0.1:43617
: // f549
: // n*:8038
> Hrm, I think I'm missing something. The command didn't change, right? Why d
The output format didn't change, but the way we parse the output is only good
for wildcard binding (n*).
http://gerrit.cloudera.org:8080/#/c/11956/3/src/kudu/util/test_util.cc@425
PS3, Line 425: Status s = Subprocess::Call(cmd, "", _out).AndThen([&]
() {
: StripTrailingNewline(_out);
: vector lines = strings::Split(lsof_out, "\n");
: for (int index = 2; index < lines.size(); index += 2) {
: StringPiece cur_line(lines[index]);
: if (HasPrefixString(cur_line.ToString(), addr_pattern) &&
: !cur_line.contains("->")) {
: if
(!safe_strto32(lines[index].substr(addr_pattern.size()), )) {
: return Status::RuntimeError("unexpected lsof
output", lsof_out);
: }
:
: return Status::OK();
: }
: }
:
: return Status::RuntimeError("unexpected lsof output",
lsof_out);
: });
> nit: merge these `if`s into one conditional block? Also mind adding a comme
The explanation is added in the previous
[kudu-CR] [sentry] add mini Sentry to the external mini cluster
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11956 to look at the new patch set (#8). Change subject: [sentry] add mini Sentry to the external mini cluster .. [sentry] add mini Sentry to the external mini cluster This commit enables external mini cluster to be able to start with both mini sentry and mini hms. A major challenge is the circular dependency between the HMS and Sentry in terms of configuring each with the other's address, including port which is currently discovered by polling lsof. To work around it, one approach would be to: 1. Start the Sentry service. Find out which port it's on. At this stage, the Sentry service has no knowledge of any HMS service. 2. Start the HMS, configured to talk to Sentry's port. Find out which port it's on. 3. Restart the Sentry service on the same port, reconfigured to talk to the HMS's port. However, there could be a race condition where another program binds to the same port during the restart in step 3. One option is to use SO_REUSEPORT socket option, which permits multiple sockets to be bound to an identical socket address. Although both Sentry and HMS are written in Java, only JDK9 (and above) supports this socket option[1]. Methods such as Java reflection can be used to invoke private methods to set up this option, but they are hacky. This patch extends the UNIQUE_LOOPBACK usage in mini cluster, so that external servers such as Sentry can pick up a unique bind address, knowing that in doing so there'll never be any danger of a port collision. [1]: https://docs.oracle.com/javase/9/docs/api/java/net/StandardSocketOptions.html#SO_REUSEPORT Change-Id: I7f02e6085bd239570d10ec629f48856d37ed6e59 --- M src/kudu/hms/mini_hms.cc M src/kudu/hms/mini_hms.h M src/kudu/integration-tests/CMakeLists.txt A src/kudu/integration-tests/master_sentry-itest.cc M src/kudu/mini-cluster/CMakeLists.txt M src/kudu/mini-cluster/external_mini_cluster.cc M src/kudu/mini-cluster/external_mini_cluster.h M src/kudu/mini-cluster/mini_cluster.cc M src/kudu/mini-cluster/mini_cluster.h M src/kudu/security/test/mini_kdc.cc M src/kudu/sentry/mini_sentry.cc M src/kudu/sentry/mini_sentry.h M src/kudu/sentry/sentry-test-base.h M src/kudu/tools/tool.proto M src/kudu/util/test_util.cc M src/kudu/util/test_util.h 16 files changed, 517 insertions(+), 69 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/11956/8 -- To view, visit http://gerrit.cloudera.org:8080/11956 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7f02e6085bd239570d10ec629f48856d37ed6e59 Gerrit-Change-Number: 11956 Gerrit-PatchSet: 8 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [sentry] add mini Sentry to the external mini cluster
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11956 ) Change subject: [sentry] add mini Sentry to the external mini cluster .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/11956/2/src/kudu/security/test/mini_kdc.cc File src/kudu/security/test/mini_kdc.cc: http://gerrit.cloudera.org:8080/#/c/11956/2/src/kudu/security/test/mini_kdc.cc@157 PS2, Line 157: kdc_ > Doc with inline comment. Done -- To view, visit http://gerrit.cloudera.org:8080/11956 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7f02e6085bd239570d10ec629f48856d37ed6e59 Gerrit-Change-Number: 11956 Gerrit-PatchSet: 7 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Mon, 10 Dec 2018 02:16:27 + Gerrit-HasComments: Yes
[kudu-CR] [sentry] Integrate AuthzProvider into CatalogManager
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/11797 ) Change subject: [sentry] Integrate AuthzProvider into CatalogManager .. Patch Set 2: (31 comments) http://gerrit.cloudera.org:8080/#/c/11797/2/src/kudu/integration-tests/hms_itest-base.h File src/kudu/integration-tests/hms_itest-base.h: http://gerrit.cloudera.org:8080/#/c/11797/2/src/kudu/integration-tests/hms_itest-base.h@56 PS2, Line 56: using client::KuduTableAlterer; > warning: using decl 'KuduTableAlterer' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/11797/2/src/kudu/integration-tests/hms_itest-base.h@59 PS2, Line 59: using cluster::ExternalMiniClusterOptions; > warning: using decl 'ExternalMiniClusterOptions' is unused [misc-unused-usi Done http://gerrit.cloudera.org:8080/#/c/11797/2/src/kudu/integration-tests/hms_itest-base.h@63 PS2, Line 63: using std::vector; > warning: using decl 'vector' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/11797/2/src/kudu/integration-tests/master_hms-itest.cc File src/kudu/integration-tests/master_hms-itest.cc: http://gerrit.cloudera.org:8080/#/c/11797/2/src/kudu/integration-tests/master_hms-itest.cc@49 PS2, Line 49: using client::KuduTableCreator; > warning: using decl 'KuduTableCreator' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/11797/2/src/kudu/integration-tests/master_sentry-itest.cc File src/kudu/integration-tests/master_sentry-itest.cc: http://gerrit.cloudera.org:8080/#/c/11797/2/src/kudu/integration-tests/master_sentry-itest.cc@49 PS2, Line 49: using sentry::TAlterSentryRoleAddGroupsRequest; > warning: using decl 'TAlterSentryRoleAddGroupsRequest' is unused [misc-unus Done http://gerrit.cloudera.org:8080/#/c/11797/2/src/kudu/integration-tests/master_sentry-itest.cc@50 PS2, Line 50: using sentry::TAlterSentryRoleAddGroupsResponse; > warning: using decl 'TAlterSentryRoleAddGroupsResponse' is unused [misc-unu Done http://gerrit.cloudera.org:8080/#/c/11797/2/src/kudu/integration-tests/master_sentry-itest.cc@51 PS2, Line 51: using sentry::TAlterSentryRoleGrantPrivilegeRequest; > warning: using decl 'TAlterSentryRoleGrantPrivilegeRequest' is unused [misc Done http://gerrit.cloudera.org:8080/#/c/11797/2/src/kudu/integration-tests/master_sentry-itest.cc@52 PS2, Line 52: using sentry::TAlterSentryRoleGrantPrivilegeResponse; > warning: using decl 'TAlterSentryRoleGrantPrivilegeResponse' is unused [mis Done http://gerrit.cloudera.org:8080/#/c/11797/2/src/kudu/integration-tests/master_sentry-itest.cc@53 PS2, Line 53: using sentry::TCreateSentryRoleRequest; > warning: using decl 'TCreateSentryRoleRequest' is unused [misc-unused-using Done http://gerrit.cloudera.org:8080/#/c/11797/2/src/kudu/integration-tests/master_sentry-itest.cc@55 PS2, Line 55: using sentry::TSentryGroup; > warning: using decl 'TSentryGroup' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/11797/2/src/kudu/integration-tests/master_sentry-itest.cc@61 PS2, Line 61: using client::KuduTable; > warning: using decl 'KuduTable' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/11797/2/src/kudu/integration-tests/master_sentry-itest.cc@63 PS2, Line 63: using client::KuduTableCreator; > warning: using decl 'KuduTableCreator' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/11797/2/src/kudu/integration-tests/master_sentry-itest.cc@68 PS2, Line 68: using std::set; > warning: using decl 'set' is unused [misc-unused-using-decls] Done http://gerrit.cloudera.org:8080/#/c/11797/2/src/kudu/master/authz_provider.cc File src/kudu/master/authz_provider.cc: http://gerrit.cloudera.org:8080/#/c/11797/2/src/kudu/master/authz_provider.cc@33 PS2, Line 33: "Comma-separated list of trusted users who may access the cluster "/master_sentry-itest.cc > error: use of undeclared identifier 'master_sentry' [clang-diagnostic-error Done http://gerrit.cloudera.org:8080/#/c/11797/2/src/kudu/master/authz_provider.cc@33 PS2, Line 33: "Comma-separated list of trusted users who may access the cluster "/master_sentry-itest.cc > error: use of undeclared identifier 'itest' [clang-diagnostic-error] Done http://gerrit.cloudera.org:8080/#/c/11797/2/src/kudu/master/authz_provider.cc@34 PS2, Line 34: "without being authorized."); > error: expected ')' [clang-diagnostic-error] Done http://gerrit.cloudera.org:8080/#/c/11797/1/src/kudu/master/catalog_manager.cc File src/kudu/master/catalog_manager.cc: http://gerrit.cloudera.org:8080/#/c/11797/1/src/kudu/master/catalog_manager.cc@101 PS1, Line 101: #include "kudu/master/default_authz_provider.h" > missing authz_provider.h? Done http://gerrit.cloudera.org:8080/#/c/11797/1/src/kudu/master/catalog_manager.cc@257 PS1, Line 257: DECLARE_bool(raft_prepare_replacement_before_eviction); > spitballing: in order to keep as much of
[kudu-CR] [sentry] Integrate AuthzProvider into CatalogManager
Hello Tidy Bot, Dan Burkert, Kudu Jenkins, Andrew Wong, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11797 to look at the new patch set (#3). Change subject: [sentry] Integrate AuthzProvider into CatalogManager .. [sentry] Integrate AuthzProvider into CatalogManager This commit enables master RPC authorization enforcement by connecting the CatalogManager to the Sentry service via the SentryAuthzProvider. When the Sentry integration is enabled (by setting the --sentry_service_rpc_addresses flag), DDLs as table creation, alteration, deletion are validated to see if the connected user has the permission to perform such operations. Note that the coarse-grained access control is still applied to these endpoints. A --trusted_user_acl flag is introduced to allow the trusted user, e.g. 'impala', to skip the authorization enforcement. More authorization endpoints, e.g ListTables RPC, will be addressed in a follow up patch. Testing: This commit adds a new integration test (master_sentry-itest) which tests that the integration works as expected with create/alter/drop table operations. More coverage on DDL stress tests with Sentry integration enabled will be in a follow up patch. Change-Id: Iab4aa027ae6eb4520db48ce348db552c9feec2a8 --- M src/kudu/client/client-test.cc M src/kudu/common/table_util-test.cc M src/kudu/integration-tests/consistency-itest.cc M src/kudu/integration-tests/create-table-stress-test.cc A src/kudu/integration-tests/hms_itest-base.h M src/kudu/integration-tests/master_hms-itest.cc M src/kudu/integration-tests/master_sentry-itest.cc M src/kudu/integration-tests/registration-test.cc M src/kudu/master/CMakeLists.txt A src/kudu/master/authz_provider.cc M src/kudu/master/authz_provider.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/default_authz_provider.h M src/kudu/master/master-test-util.h M src/kudu/master/master.proto M src/kudu/master/master_service.cc A src/kudu/master/sentry_authz_provider-test-base.h M src/kudu/master/sentry_authz_provider-test.cc M src/kudu/master/sentry_authz_provider.cc M src/kudu/master/sentry_authz_provider.h M src/kudu/sentry/mini_sentry.cc M src/kudu/sentry/sentry_policy_service.thrift 23 files changed, 1,030 insertions(+), 451 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/97/11797/3 -- To view, visit http://gerrit.cloudera.org:8080/11797 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Iab4aa027ae6eb4520db48ce348db552c9feec2a8 Gerrit-Change-Number: 11797 Gerrit-PatchSet: 3 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Dan Burkert Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR] [sentry] add mini Sentry to the external mini cluster
Hao Hao has posted comments on this change. (
http://gerrit.cloudera.org:8080/11956 )
Change subject: [sentry] add mini Sentry to the external mini cluster
..
Patch Set 7:
(42 comments)
http://gerrit.cloudera.org:8080/#/c/11956/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11956/2//COMMIT_MSG@13
PS2, Line 13: one appro
> nit: this reads a bit like this is what's implemented. Mind changing it to
Done
http://gerrit.cloudera.org:8080/#/c/11956/2//COMMIT_MSG@15
PS2, Line 15: 1. Start the Sentry service. Find out which port it's on. At
this
> Good explanation, but I'd make two small tweaks to improve readability:
Done
http://gerrit.cloudera.org:8080/#/c/11956/2//COMMIT_MSG@21
PS2, Line 21:
> Nit: where
Done
http://gerrit.cloudera.org:8080/#/c/11956/2//COMMIT_MSG@22
PS2, Line 22: ver, there could be
> Nit: "same port during the restart in step 3."
Done
http://gerrit.cloudera.org:8080/#/c/11956/2//COMMIT_MSG@24
PS2, Line 24: SO_REUSEPORT socket option, which permits multiple sockets to be
bound
: to an identical socket address
> Nit: I'd rewrite as "Although both Sentry and HMS are written in Java, only
Done
http://gerrit.cloudera.org:8080/#/c/11956/2//COMMIT_MSG@26
PS2, Line 26: [1]. Meth
> Nit: "they are hacky"
Done
http://gerrit.cloudera.org:8080/#/c/11956/2//COMMIT_MSG@33
PS2, Line 33: collision.
> Wrong link here; didn't you mean the SO_REUSEPORT anchor?
Done
http://gerrit.cloudera.org:8080/#/c/11956/2/src/kudu/hms/mini_hms.h
File src/kudu/hms/mini_hms.h:
http://gerrit.cloudera.org:8080/#/c/11956/2/src/kudu/hms/mini_hms.h@54
PS2, Line 54: d EnableSentry(const HostPort& sentry_address,
> nit: doc what 'sentry_service_principal' is? What happens when it's boost::
Done
http://gerrit.cloudera.org:8080/#/c/11956/2/src/kudu/hms/mini_hms.cc
File src/kudu/hms/mini_hms.cc:
http://gerrit.cloudera.org:8080/#/c/11956/2/src/kudu/hms/mini_hms.cc@81
PS2, Line 81: ess_ = sentry_address.ToSt
> nit: maybe add the sentry address?
Done
http://gerrit.cloudera.org:8080/#/c/11956/2/src/kudu/hms/mini_hms.cc@84
PS2, Line 84:
> If you're going to go through the trouble to wrap sentry_service_principal
Sorry for the confusion. I found it is more straightforward to always pass the
service_principal and use keytab_file as an indicator of if Kerberos is
enabled, the same as all other places are doing.
http://gerrit.cloudera.org:8080/#/c/11956/2/src/kudu/hms/mini_hms.cc@164
PS2, Line 164: if (!wait.ok()) {
> nit: Can you add a comment about why `none` is appropriate here?
Done
http://gerrit.cloudera.org:8080/#/c/11956/2/src/kudu/hms/mini_hms.cc@164
PS2, Line 164: if (!wait.ok()) {
> Should use /*foo=*/ inline comments to help explain what 'none' means.
Done
http://gerrit.cloudera.org:8080/#/c/11956/2/src/kudu/hms/mini_hms.cc@306
PS2, Line 306:
: string sentry_properties;
: if (IsAuthorizationEnabled()) {
:
> Shouldn't this also be conditioned on IsAuthorizationEnabled? Otherwise the
Done
http://gerrit.cloudera.org:8080/#/c/11956/2/src/kudu/hms/mini_hms.cc@318
PS2, Line 318: authorization validation.
: //
> I'm not sure I understand this. What is a "read result" in this context?
I am not super clear on this config as well, I cannot find any formal
documentations on this. By reading the code, it only adds filtering for
SHOWDATABASES and SHOWTABLES. I will make it more clear based on my
understanding.
http://gerrit.cloudera.org:8080/#/c/11956/2/src/kudu/hms/mini_hms.cc@322
PS2, Line 322: which synchronizes the notification logs with the Sentry
: // service. So that Sentry service is
> Just making sure I understand this, this is configured to ensure the HMS wi
Yeah.
For 'What is considered an authorization metadata operation?', do you mean what
kinds of operation are considered to consult for authorization metadata?
Similar to what we do in Kudu, mostly are DDL and DML operations. A complete
list can be found here:
https://www.cloudera.com/documentation/enterprise/latest/topics/sentry_privileges_hive_impala.html.
http://gerrit.cloudera.org:8080/#/c/11956/2/src/kudu/hms/mini_hms.cc@327
PS2, Line 327:
: hive.sentr
> Just making sure I understand this, this is synchronizing authz metadata en
This is to sync events in the HMS with Sentry service, so that Sentry knows
about the tables has been renamed, or the location of the table has changed.
And it updates these changes accordingly in Sentry. Update the comment to
explain in more detail.
http://gerrit.cloudera.org:8080/#/c/11956/2/src/kudu/hms/mini_hms.cc@328
PS2, Line 328: ve.sentr
> nit: missing space
Done
http://gerrit.cloudera.org:8080/#/c/11956/2/src/kudu/hms/mini_hms.cc@328
PS2, Line 328: hive.sentry.conf.url
> Nit: indentation
Done
http://gerrit.cloudera.org:8080/#/c/11956/2/src/kudu/hms/mini_hms.cc@355
PS2, Line 355:
[kudu-CR] [sentry] Integrate AuthzProvider into CatalogManager
Hao Hao has abandoned this change. ( http://gerrit.cloudera.org:8080/12058 ) Change subject: [sentry] Integrate AuthzProvider into CatalogManager .. Abandoned -- To view, visit http://gerrit.cloudera.org:8080/12058 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: I7377193abec7b8ed33d7c9455bcc9ef44da94941 Gerrit-Change-Number: 12058 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] [sentry] Integrate AuthzProvider into CatalogManager
Hao Hao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12058 Change subject: [sentry] Integrate AuthzProvider into CatalogManager .. [sentry] Integrate AuthzProvider into CatalogManager This commit enables master RPC authorization enforcement by connecting the CatalogManager to the Sentry service via the SentryAuthzProvider. When the Sentry integration is enabled (by setting the --sentry_service_rpc_addresses flag), DDLs as table creation, alteration, deletion are validated to see if the connected user has the permission to perform such operations. Note that the coarse-grained access control is still applied to these endpoints. A --trusted_user_acl flag is introduced to allow the trusted user, e.g. 'impala', to skip the authorization enforcement. More authorization endpoints, e.g ListTables RPC, will be addressed in a follow up patch. Testing: This commit adds a new integration test (master_sentry-itest) which tests that the integration works as expected with create/alter/drop table operations. More coverage on DDL stress tests with Sentry integration enabled will be in a follow up patch. Change-Id: I7377193abec7b8ed33d7c9455bcc9ef44da94941 --- M src/kudu/client/client-test.cc M src/kudu/common/table_util-test.cc M src/kudu/integration-tests/consistency-itest.cc M src/kudu/integration-tests/create-table-stress-test.cc A src/kudu/integration-tests/hms_itest-base.h M src/kudu/integration-tests/master_hms-itest.cc M src/kudu/integration-tests/master_sentry-itest.cc M src/kudu/integration-tests/registration-test.cc M src/kudu/master/CMakeLists.txt A src/kudu/master/authz_provider.cc M src/kudu/master/authz_provider.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/default_authz_provider.h M src/kudu/master/master-test-util.h M src/kudu/master/master.proto M src/kudu/master/master_service.cc A src/kudu/master/sentry_authz_provider-test-base.h M src/kudu/master/sentry_authz_provider-test.cc M src/kudu/master/sentry_authz_provider.cc M src/kudu/master/sentry_authz_provider.h M src/kudu/sentry/mini_sentry.cc M src/kudu/sentry/sentry_policy_service.thrift 23 files changed, 1,030 insertions(+), 451 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/58/12058/1 -- To view, visit http://gerrit.cloudera.org:8080/12058 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I7377193abec7b8ed33d7c9455bcc9ef44da94941 Gerrit-Change-Number: 12058 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao
[kudu-CR] [sentry] add mini Sentry to the external mini cluster
Hello Tidy Bot, Kudu Jenkins, Andrew Wong, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/11956 to look at the new patch set (#7). Change subject: [sentry] add mini Sentry to the external mini cluster .. [sentry] add mini Sentry to the external mini cluster This commit enables external mini cluster to be able to start with both mini sentry and mini hms. A major challenge is the circular dependency between the HMS and Sentry in terms of configuring each with the other's address, including port which is currently discovered by polling lsof. To work around it, one approach would be to: 1. Start the Sentry service. Find out which port it's on. At this stage, the Sentry service has no knowledge of any HMS service. 2. Start the HMS, configured to talk to Sentry's port. Find out which port it's on. 3. Restart the Sentry service on the same port, reconfigured to talk to the HMS's port. However, there could be a race condition where another program binds to the same port during the restart in step 3. One option is to use SO_REUSEPORT socket option, which permits multiple sockets to be bound to an identical socket address. Although both Sentry and HMS are written in Java, only JDK9 (and above) supports this socket option[1]. Methods such as Java reflection can be used to invoke private methods to set up this option, but they are hacky. This patch extends the UNIQUE_LOOPBACK usage in mini cluster, so that external servers such as Sentry can pick up a unique bind address, knowing that in doing so there'll never be any danger of a port collision. [1]: https://docs.oracle.com/javase/9/docs/api/java/net/StandardSocketOptions.html#SO_REUSEPORT Change-Id: I7f02e6085bd239570d10ec629f48856d37ed6e59 --- M src/kudu/hms/mini_hms.cc M src/kudu/hms/mini_hms.h M src/kudu/integration-tests/CMakeLists.txt A src/kudu/integration-tests/master_sentry-itest.cc M src/kudu/mini-cluster/CMakeLists.txt M src/kudu/mini-cluster/external_mini_cluster.cc M src/kudu/mini-cluster/external_mini_cluster.h M src/kudu/mini-cluster/mini_cluster.cc M src/kudu/mini-cluster/mini_cluster.h M src/kudu/security/test/mini_kdc.cc M src/kudu/sentry/mini_sentry.cc M src/kudu/sentry/mini_sentry.h M src/kudu/sentry/sentry-test-base.h M src/kudu/tools/tool.proto M src/kudu/util/test_util.cc M src/kudu/util/test_util.h 16 files changed, 537 insertions(+), 69 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/56/11956/7 -- To view, visit http://gerrit.cloudera.org:8080/11956 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7f02e6085bd239570d10ec629f48856d37ed6e59 Gerrit-Change-Number: 11956 Gerrit-PatchSet: 7 Gerrit-Owner: Hao Hao Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Andrew Wong Gerrit-Reviewer: Hao Hao Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241)
[kudu-CR](branch-1.7.x) kudu support ttl
YangSong has removed Kudu Jenkins from this change. ( http://gerrit.cloudera.org:8080/12057 ) Change subject: kudu support ttl .. Removed reviewer Kudu Jenkins. -- To view, visit http://gerrit.cloudera.org:8080/12057 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: deleteReviewer Gerrit-Change-Id: I6503ca90ebca66d154e7084e0c93e94c9b0d9c0c Gerrit-Change-Number: 12057 Gerrit-PatchSet: 1 Gerrit-Owner: YangSong
[kudu-CR](branch-1.7.x) kudu support ttl
YangSong has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12057 Change subject: kudu support ttl .. kudu support ttl Change-Id: I6503ca90ebca66d154e7084e0c93e94c9b0d9c0c --- M src/kudu/common/generic_iterators.cc M src/kudu/common/generic_iterators.h M src/kudu/common/row_operations.cc M src/kudu/common/row_operations.h M src/kudu/common/scan_spec.h M src/kudu/common/schema.cc M src/kudu/common/schema.h M src/kudu/common/types.h M src/kudu/master/catalog_manager.cc M src/kudu/master/catalog_manager.h M src/kudu/master/master.proto M src/kudu/master/master_path_handlers.cc M src/kudu/tablet/cfile_set.h M src/kudu/tablet/compaction-test.cc M src/kudu/tablet/compaction.cc M src/kudu/tablet/compaction.h M src/kudu/tablet/delta_applier.cc M src/kudu/tablet/delta_applier.h M src/kudu/tablet/delta_compaction.cc M src/kudu/tablet/delta_compaction.h M src/kudu/tablet/delta_iterator_merger.cc M src/kudu/tablet/delta_iterator_merger.h M src/kudu/tablet/delta_store.h M src/kudu/tablet/delta_tracker.cc M src/kudu/tablet/delta_tracker.h M src/kudu/tablet/deltafile.cc M src/kudu/tablet/deltafile.h M src/kudu/tablet/deltamemstore.cc M src/kudu/tablet/deltamemstore.h M src/kudu/tablet/diskrowset-test.cc M src/kudu/tablet/diskrowset.cc M src/kudu/tablet/diskrowset.h M src/kudu/tablet/memrowset.cc M src/kudu/tablet/memrowset.h M src/kudu/tablet/metadata.proto M src/kudu/tablet/mock-rowsets.h M src/kudu/tablet/rowset.cc M src/kudu/tablet/rowset.h M src/kudu/tablet/tablet.cc M src/kudu/tablet/tablet.h M src/kudu/tablet/tablet_metadata.cc M src/kudu/tablet/tablet_metadata.h M src/kudu/tablet/transactions/alter_schema_transaction.h M src/kudu/tools/tool_action_table.cc M src/kudu/tools/tool_action_tablet.cc M src/kudu/tserver/scanners.h M src/kudu/tserver/tablet_service.cc M src/kudu/tserver/tserver.proto M src/kudu/tserver/tserver_admin.proto 49 files changed, 768 insertions(+), 114 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/12057/1 -- To view, visit http://gerrit.cloudera.org:8080/12057 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: branch-1.7.x Gerrit-MessageType: newchange Gerrit-Change-Id: I6503ca90ebca66d154e7084e0c93e94c9b0d9c0c Gerrit-Change-Number: 12057 Gerrit-PatchSet: 1 Gerrit-Owner: YangSong
