You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by da...@apache.org on 2018/09/25 17:43:29 UTC

[2/2] kudu git commit: Add Hive Metastore service principal configuration

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 <ad...@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 <da...@apache.org>
Authored: Mon Sep 24 16:41:27 2018 -0700
Committer: Dan Burkert <da...@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());