Add Hive Metastore service principal configuration This commit adds a new flag, --hive_metastore_kerberos_principal flag which corresponds to the HMS 'hive.metastore.kerberos.principal' configuration. This configuration is rarely overridden, but in cases where it is, having a way to match it in Kudu is critical.
Change-Id: I6c5c56423a62cba0b696f97d866077b35845ce26 Reviewed-on: http://gerrit.cloudera.org:8080/11503 Tested-by: Kudu Jenkins Reviewed-by: Adar Dembo <a...@cloudera.com> Project: http://git-wip-us.apache.org/repos/asf/kudu/repo Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/a09c89ab Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/a09c89ab Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/a09c89ab Branch: refs/heads/master Commit: a09c89abc1c0df62cffc9bdf3b36eede9150c4b9 Parents: 24b38f7 Author: Dan Burkert <danburk...@apache.org> Authored: Mon Sep 24 16:41:27 2018 -0700 Committer: Dan Burkert <danburk...@apache.org> Committed: Tue Sep 25 17:42:49 2018 +0000 ---------------------------------------------------------------------- src/kudu/hms/hms_catalog-test.cc | 1 + src/kudu/hms/hms_catalog.cc | 7 +++++++ src/kudu/hms/hms_client-test.cc | 5 ++++- src/kudu/integration-tests/master_hms-itest.cc | 1 + src/kudu/mini-cluster/external_mini_cluster-test.cc | 1 + src/kudu/thrift/client.cc | 4 +++- src/kudu/thrift/client.h | 6 ++++++ src/kudu/thrift/sasl_client_transport.cc | 9 +++++---- src/kudu/thrift/sasl_client_transport.h | 6 +++++- src/kudu/tools/kudu-tool-test.cc | 3 +++ 10 files changed, 36 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kudu/blob/a09c89ab/src/kudu/hms/hms_catalog-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/hms/hms_catalog-test.cc b/src/kudu/hms/hms_catalog-test.cc index a2bfabf..83369fb 100644 --- a/src/kudu/hms/hms_catalog-test.cc +++ b/src/kudu/hms/hms_catalog-test.cc @@ -184,6 +184,7 @@ class HmsCatalogTest : public KuduTest { ASSERT_OK(kdc_->Kinit("alice")); ASSERT_OK(kdc_->SetKrb5Environment()); hms_client_opts.enable_kerberos = true; + hms_client_opts.service_principal = "hive"; // Configure the HmsCatalog flags. FLAGS_hive_metastore_sasl_enabled = true; http://git-wip-us.apache.org/repos/asf/kudu/blob/a09c89ab/src/kudu/hms/hms_catalog.cc ---------------------------------------------------------------------- diff --git a/src/kudu/hms/hms_catalog.cc b/src/kudu/hms/hms_catalog.cc index aeaf226..ae4d998 100644 --- a/src/kudu/hms/hms_catalog.cc +++ b/src/kudu/hms/hms_catalog.cc @@ -73,6 +73,12 @@ DEFINE_bool(hive_metastore_sasl_enabled, false, "When enabled, the --keytab_file flag must be provided."); TAG_FLAG(hive_metastore_sasl_enabled, experimental); +DEFINE_string(hive_metastore_kerberos_principal, "hive", + "The service principal of the Hive Metastore server. Must match " + "the primary (user) portion of hive.metastore.kerberos.principal option " + "in the Hive Metastore configuration."); +TAG_FLAG(hive_metastore_kerberos_principal, experimental); + DEFINE_int32(hive_metastore_retry_count, 1, "The number of times that HMS operations will retry after " "encountering retriable failures, such as network errors."); @@ -416,6 +422,7 @@ Status HmsCatalog::Reconnect() { options.recv_timeout = MonoDelta::FromSeconds(FLAGS_hive_metastore_recv_timeout); options.conn_timeout = MonoDelta::FromSeconds(FLAGS_hive_metastore_conn_timeout); options.enable_kerberos = FLAGS_hive_metastore_sasl_enabled; + options.service_principal = FLAGS_hive_metastore_kerberos_principal; options.max_buf_size = FLAGS_hive_metastore_max_message_size; // Try reconnecting to each HMS in sequence, returning the first one which http://git-wip-us.apache.org/repos/asf/kudu/blob/a09c89ab/src/kudu/hms/hms_client-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/hms/hms_client-test.cc b/src/kudu/hms/hms_client-test.cc index e677cd0..9091a46 100644 --- a/src/kudu/hms/hms_client-test.cc +++ b/src/kudu/hms/hms_client-test.cc @@ -127,6 +127,7 @@ TEST_P(HmsClientTest, TestHmsOperations) { ASSERT_OK(kdc.Kinit("alice")); ASSERT_OK(kdc.SetKrb5Environment()); hms_client_opts.enable_kerberos = true; + hms_client_opts.service_principal = "hive"; } ASSERT_OK(hms.Start()); @@ -288,7 +289,8 @@ TEST_P(HmsClientTest, TestLargeObjects) { if (protection) { ASSERT_OK(kdc.Start()); - string spn = "hive/127.0.0.1"; + // Try a non-standard service principal to ensure it works correctly. + string spn = "hive_alternate_sp/127.0.0.1"; string ktpath; ASSERT_OK(kdc.CreateServiceKeytab(spn, &ktpath)); @@ -302,6 +304,7 @@ TEST_P(HmsClientTest, TestLargeObjects) { ASSERT_OK(kdc.Kinit("alice")); ASSERT_OK(kdc.SetKrb5Environment()); hms_client_opts.enable_kerberos = true; + hms_client_opts.service_principal = "hive_alternate_sp"; } ASSERT_OK(hms.Start()); http://git-wip-us.apache.org/repos/asf/kudu/blob/a09c89ab/src/kudu/integration-tests/master_hms-itest.cc ---------------------------------------------------------------------- diff --git a/src/kudu/integration-tests/master_hms-itest.cc b/src/kudu/integration-tests/master_hms-itest.cc index 1cd33b8..cd148e7 100644 --- a/src/kudu/integration-tests/master_hms-itest.cc +++ b/src/kudu/integration-tests/master_hms-itest.cc @@ -81,6 +81,7 @@ class MasterHmsTest : public ExternalMiniClusterITestBase { thrift::ClientOptions hms_opts; hms_opts.enable_kerberos = EnableKerberos(); + hms_opts.service_principal = "hive"; hms_client_.reset(new HmsClient(cluster_->hms()->address(), hms_opts)); ASSERT_OK(hms_client_->Start()); } http://git-wip-us.apache.org/repos/asf/kudu/blob/a09c89ab/src/kudu/mini-cluster/external_mini_cluster-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/mini-cluster/external_mini_cluster-test.cc b/src/kudu/mini-cluster/external_mini_cluster-test.cc index e337499..8517354 100644 --- a/src/kudu/mini-cluster/external_mini_cluster-test.cc +++ b/src/kudu/mini-cluster/external_mini_cluster-test.cc @@ -196,6 +196,7 @@ TEST_P(ExternalMiniClusterTest, TestBasicOperation) { if (opts.hms_mode == HmsMode::ENABLE_HIVE_METASTORE) { thrift::ClientOptions hms_client_opts; hms_client_opts.enable_kerberos = opts.enable_kerberos; + hms_client_opts.service_principal = "hive"; hms::HmsClient hms_client(cluster.hms()->address(), hms_client_opts); ASSERT_OK(hms_client.Start()); vector<string> tables; http://git-wip-us.apache.org/repos/asf/kudu/blob/a09c89ab/src/kudu/thrift/client.cc ---------------------------------------------------------------------- diff --git a/src/kudu/thrift/client.cc b/src/kudu/thrift/client.cc index 7c109db..9f4c29c 100644 --- a/src/kudu/thrift/client.cc +++ b/src/kudu/thrift/client.cc @@ -71,7 +71,9 @@ shared_ptr<TProtocol> CreateClientProtocol(const HostPort& address, const Client shared_ptr<TTransport> transport; if (options.enable_kerberos) { - transport = make_shared<SaslClientTransport>(address.host(), + DCHECK(!options.service_principal.empty()); + transport = make_shared<SaslClientTransport>(options.service_principal, + address.host(), std::move(socket), options.max_buf_size); } else { http://git-wip-us.apache.org/repos/asf/kudu/blob/a09c89ab/src/kudu/thrift/client.h ---------------------------------------------------------------------- diff --git a/src/kudu/thrift/client.h b/src/kudu/thrift/client.h index 8b6a7d8..b1e21aa 100644 --- a/src/kudu/thrift/client.h +++ b/src/kudu/thrift/client.h @@ -21,6 +21,7 @@ #include <cstdint> #include <memory> +#include <string> #include "kudu/util/monotime.h" @@ -53,6 +54,11 @@ struct ClientOptions { // Whether to use SASL Kerberos authentication. bool enable_kerberos = false; + // The registered name of the service (Kerberos principal). + // + // Must be set if kerberos is enabled. + std::string service_principal; + // Maximum size of objects which can be received on the Thrift connection. // Defaults to 100MiB to match Thrift TSaslTransport.receiveSaslMessage. int32_t max_buf_size = 100 * 1024 * 1024; http://git-wip-us.apache.org/repos/asf/kudu/blob/a09c89ab/src/kudu/thrift/sasl_client_transport.cc ---------------------------------------------------------------------- diff --git a/src/kudu/thrift/sasl_client_transport.cc b/src/kudu/thrift/sasl_client_transport.cc index 8a8d5a9..80ea0c2 100644 --- a/src/kudu/thrift/sasl_client_transport.cc +++ b/src/kudu/thrift/sasl_client_transport.cc @@ -92,7 +92,8 @@ int UserCb(void* /*context*/, int id, const char** result, unsigned* len) { } } // anonymous namespace -SaslClientTransport::SaslClientTransport(const string& server_fqdn, +SaslClientTransport::SaslClientTransport(string service_principal, + const string& server_fqdn, shared_ptr<TTransport> transport, size_t max_recv_buf_size) : transport_(std::move(transport)), @@ -109,7 +110,8 @@ SaslClientTransport::SaslClientTransport(const string& server_fqdn, max_recv_buf_size_(max_recv_buf_size), // Set a reasonable max send buffer size for negotiation. Once negotiation // is complete the negotiated value will be used. - max_send_buf_size_(64 * 1024) { + max_send_buf_size_(64 * 1024), + service_principal_(std::move(service_principal)) { sasl_helper_.set_server_fqdn(server_fqdn); sasl_helper_.EnableGSSAPI(); ResetWriteBuf(); @@ -373,8 +375,7 @@ void SaslClientTransport::SetupSaslContext() { sasl_conn_t* sasl_conn = nullptr; Status s = WrapSaslCall(nullptr /* no conn */, [&] { return sasl_client_new( - // TODO(dan): make the service name configurable. - "hive", // Registered name of the service using SASL. Required. + service_principal_.c_str(), // Registered name of the service using SASL. Required. sasl_helper_.server_fqdn(), // The fully qualified domain name of the remote server. nullptr, // Local and remote IP address strings. (we don't use nullptr, // any mechanisms which require this info.) http://git-wip-us.apache.org/repos/asf/kudu/blob/a09c89ab/src/kudu/thrift/sasl_client_transport.h ---------------------------------------------------------------------- diff --git a/src/kudu/thrift/sasl_client_transport.h b/src/kudu/thrift/sasl_client_transport.h index ef9936f..360e41f 100644 --- a/src/kudu/thrift/sasl_client_transport.h +++ b/src/kudu/thrift/sasl_client_transport.h @@ -81,7 +81,8 @@ enum NegotiationStatus { class SaslClientTransport : public apache::thrift::transport::TVirtualTransport<SaslClientTransport> { public: - SaslClientTransport(const std::string& server_fqdn, + SaslClientTransport(std::string service_principal, + const std::string& server_fqdn, std::shared_ptr<TTransport> transport, size_t max_recv_buf_size); @@ -170,6 +171,9 @@ class SaslClientTransport // The write buffer. faststring write_buf_; + + // The principal of the service being connected to. + std::string service_principal_; }; } // namespace thrift http://git-wip-us.apache.org/repos/asf/kudu/blob/a09c89ab/src/kudu/tools/kudu-tool-test.cc ---------------------------------------------------------------------- diff --git a/src/kudu/tools/kudu-tool-test.cc b/src/kudu/tools/kudu-tool-test.cc index 4df871a..f6fabc9 100644 --- a/src/kudu/tools/kudu-tool-test.cc +++ b/src/kudu/tools/kudu-tool-test.cc @@ -2412,6 +2412,7 @@ TEST_P(ToolTestKerberosParameterized, TestHmsDowngrade) { string master_addr = cluster_->master()->bound_rpc_addr().ToString(); thrift::ClientOptions hms_opts; hms_opts.enable_kerberos = EnableKerberos(); + hms_opts.service_principal = "hive"; HmsClient hms_client(cluster_->hms()->address(), hms_opts); ASSERT_OK(hms_client.Start()); ASSERT_TRUE(hms_client.IsConnected()); @@ -2457,6 +2458,7 @@ TEST_P(ToolTestKerberosParameterized, TestCheckAndAutomaticFixHmsMetadata) { string master_addr = cluster_->master()->bound_rpc_addr().ToString(); thrift::ClientOptions hms_opts; hms_opts.enable_kerberos = EnableKerberos(); + hms_opts.service_principal = "hive"; HmsClient hms_client(cluster_->hms()->address(), hms_opts); ASSERT_OK(hms_client.Start()); ASSERT_TRUE(hms_client.IsConnected()); @@ -2728,6 +2730,7 @@ TEST_P(ToolTestKerberosParameterized, TestCheckAndManualFixHmsMetadata) { string master_addr = cluster_->master()->bound_rpc_addr().ToString(); thrift::ClientOptions hms_opts; hms_opts.enable_kerberos = EnableKerberos(); + hms_opts.service_principal = "hive"; HmsClient hms_client(cluster_->hms()->address(), hms_opts); ASSERT_OK(hms_client.Start()); ASSERT_TRUE(hms_client.IsConnected());