This is an automated email from the ASF dual-hosted git repository. alexey pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 6e831433239f213f6592b50f15da2317bfc0f84a Author: Ádám Bakai <[email protected]> AuthorDate: Tue Nov 14 14:49:25 2023 +0100 [messenger] Initialize SASL proto name to principal Adding master in case of custom kerberos principal name didn't work because the non-default principal name was not propagated to the MessengerBuilder every time and it used "kudu" instead of the one provided by the kerberos service. If GetLoggedInUsernameFromKeytab is available it provided to the MessengerBuilder KuduClientBuilder objects. If it is not set the sasl_proto_name remains at the default "kudu" value. However this change is not enough because ServerBase::ShutdownImpl called DestroyKerberosForServer which destroyed the global state which GetLoggedInUsernameFromKeytab uses. The DestroyKerberosForServer call was moved to ServerBase::~ServerBase(). The new test is using non default principals, adds two new masters, runs a smoke test. After that one master is shut down to simulate recoverable master error, and the smoke test is executed again. Change-Id: Ie8decbd0b3e54df42bb0b9b14fc5ec291cd70b8b Reviewed-on: http://gerrit.cloudera.org:8080/20708 Tested-by: Kudu Jenkins Reviewed-by: Alexey Serbin <[email protected]> --- src/kudu/integration-tests/security-itest.cc | 31 ++++++++++++++++++++++++++++ src/kudu/master/auto_leader_rebalancer.cc | 7 ++++++- src/kudu/master/auto_rebalancer.cc | 7 ++++++- src/kudu/master/master_runner.cc | 13 ++++++++++-- src/kudu/server/server_base.cc | 8 +++++-- 5 files changed, 60 insertions(+), 6 deletions(-) diff --git a/src/kudu/integration-tests/security-itest.cc b/src/kudu/integration-tests/security-itest.cc index adea89025..e4c6e5b91 100644 --- a/src/kudu/integration-tests/security-itest.cc +++ b/src/kudu/integration-tests/security-itest.cc @@ -873,6 +873,37 @@ TEST_F(SecurityITest, TestCorruptKerberosCC) { } } +TEST_F(SecurityITest, TestNonDefaultPrincipalMultipleMaster) { + SKIP_IF_SLOW_NOT_ALLOWED(); + cluster_opts_.principal = "oryx"; + + // Start with a single master setup and add two masters + ASSERT_OK(StartCluster()); + ASSERT_OK(cluster_->master(0)->WaitForCatalogManager()); + ASSERT_OK(cluster_->AddMaster()); + ASSERT_OK(cluster_->master(1)->WaitForCatalogManager()); + ASSERT_OK(cluster_->AddMaster()); + ASSERT_OK(cluster_->master(2)->WaitForCatalogManager()); + + // Tablet servers need to be restarted in order to update their master list. + for (int i = 0; i < cluster_->num_tablet_servers(); i++) { + cluster_->tablet_server(i)->Shutdown(); + ASSERT_OK(cluster_->tablet_server(i)->Restart()); + } + + // Run a smoke test to verify that the cluster is working properly. + shared_ptr<KuduClient> client; + ASSERT_OK(cluster_->CreateClient(nullptr, &client)); + SmokeTestCluster(client); + + // Shut down the leader master to simulate recoverable error. + cluster_->leader_master()->Shutdown(); + + // Run a smoke test to verify that the cluster is still working properly. + ASSERT_OK(cluster_->CreateClient(nullptr, &client)); + SmokeTestCluster(client); +} + TEST_F(SecurityITest, TestNonDefaultPrincipal) { const string kPrincipal = "oryx"; cluster_opts_.principal = kPrincipal; diff --git a/src/kudu/master/auto_leader_rebalancer.cc b/src/kudu/master/auto_leader_rebalancer.cc index 1ee94e5cb..629afb77d 100644 --- a/src/kudu/master/auto_leader_rebalancer.cc +++ b/src/kudu/master/auto_leader_rebalancer.cc @@ -48,6 +48,7 @@ #include "kudu/master/ts_manager.h" #include "kudu/rpc/messenger.h" #include "kudu/rpc/rpc_controller.h" +#include "kudu/security/init.h" #include "kudu/util/cow_object.h" #include "kudu/util/flag_tags.h" #include "kudu/util/monotime.h" @@ -104,7 +105,11 @@ AutoLeaderRebalancerTask::~AutoLeaderRebalancerTask() { Shutdown(); } Status AutoLeaderRebalancerTask::Init() { DCHECK(!thread_) << "AutoleaderRebalancerTask is already initialized"; - RETURN_NOT_OK(MessengerBuilder("auto-leader-rebalancer").Build(&messenger_)); + MessengerBuilder builder("auto-leader-rebalancer"); + if (auto username = kudu::security::GetLoggedInUsernameFromKeytab()) { + builder.set_sasl_proto_name(username.value()); + } + RETURN_NOT_OK(std::move(builder).Build(&messenger_)); return Thread::Create("catalog manager", "auto-leader-rebalancer", [this]() { this->RunLoop(); }, &thread_); } diff --git a/src/kudu/master/auto_rebalancer.cc b/src/kudu/master/auto_rebalancer.cc index aa3d1a7fc..518c3831e 100644 --- a/src/kudu/master/auto_rebalancer.cc +++ b/src/kudu/master/auto_rebalancer.cc @@ -52,6 +52,7 @@ #include "kudu/rebalance/rebalancer.h" #include "kudu/rpc/messenger.h" #include "kudu/rpc/rpc_controller.h" +#include "kudu/security/init.h" #include "kudu/tserver/tserver.pb.h" #include "kudu/util/cow_object.h" #include "kudu/util/monotime.h" @@ -175,7 +176,11 @@ AutoRebalancerTask::~AutoRebalancerTask() { Status AutoRebalancerTask::Init() { DCHECK(!thread_) << "AutoRebalancerTask is already initialized"; - RETURN_NOT_OK(MessengerBuilder("auto-rebalancer").Build(&messenger_)); + MessengerBuilder builder("auto-rebalancer"); + if (auto username = kudu::security::GetLoggedInUsernameFromKeytab()) { + builder.set_sasl_proto_name(username.value()); + } + RETURN_NOT_OK(std::move(builder).Build(&messenger_)); return Thread::Create("catalog manager", "auto-rebalancer", [this]() { this->RunLoop(); }, &thread_); } diff --git a/src/kudu/master/master_runner.cc b/src/kudu/master/master_runner.cc index 7395ba2f4..00f23d769 100644 --- a/src/kudu/master/master_runner.cc +++ b/src/kudu/master/master_runner.cc @@ -55,6 +55,7 @@ #include "kudu/rpc/messenger.h" #include "kudu/rpc/rpc.h" #include "kudu/rpc/rpc_controller.h" +#include "kudu/security/init.h" #include "kudu/tablet/metadata.pb.h" #include "kudu/tablet/tablet_metadata.h" #include "kudu/tablet/tablet_replica.h" @@ -313,7 +314,11 @@ Status ClearLocalSystemCatalogAndCopy(const HostPort& src_hp) { /*last_logged_opid*/std::nullopt)); LOG(INFO) << "Copying system tablet from " << src_hp.ToString(); std::shared_ptr<rpc::Messenger> messenger; - RETURN_NOT_OK(rpc::MessengerBuilder("tablet_copy_client").Build(&messenger)); + rpc::MessengerBuilder builder("tablet_copy_client"); + if (auto username = kudu::security::GetLoggedInUsernameFromKeytab()) { + builder.set_sasl_proto_name(username.value()); + } + RETURN_NOT_OK(builder.Build(&messenger)); RemoteTabletCopyClient copy_client(SysCatalogTable::kSysCatalogTabletId, &fs_manager, cmeta_manager, messenger, nullptr /* no metrics */); @@ -430,7 +435,11 @@ Status RunMasterServer() { master_addrs.emplace_back(hp.ToString()); } client::sp::shared_ptr<KuduClient> client; - RETURN_NOT_OK(KuduClientBuilder() + KuduClientBuilder builder; + if (auto username = kudu::security::GetLoggedInUsernameFromKeytab()) { + builder.sasl_protocol_name(username.value()); + } + RETURN_NOT_OK(builder .master_server_addrs(master_addrs) .Build(&client)); AddMasterRequestPB add_req; diff --git a/src/kudu/server/server_base.cc b/src/kudu/server/server_base.cc index 0d9bd20fd..84d6a7b24 100644 --- a/src/kudu/server/server_base.cc +++ b/src/kudu/server/server_base.cc @@ -671,6 +671,7 @@ ServerBase::ServerBase(string name, const ServerBaseOptions& options, ServerBase::~ServerBase() { ShutdownImpl(); + security::DestroyKerberosForServer(); } Sockaddr ServerBase::first_rpc_address() const { @@ -832,6 +833,11 @@ Status ServerBase::Init() { .set_hostname(hostname) .enable_inbound_tls(); + auto username = kudu::security::GetLoggedInUsernameFromKeytab(); + if (username.has_value()) { + builder.set_sasl_proto_name(username.value()); + } + if (options_.rpc_opts.rpc_reuseport) { builder.set_reuseport(); } @@ -1118,8 +1124,6 @@ void ServerBase::ShutdownImpl() { tcmalloc_memory_gc_thread_->Join(); } #endif - - security::DestroyKerberosForServer(); } #ifdef TCMALLOC_ENABLED
