You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by gr...@apache.org on 2021/04/28 03:19:39 UTC

[kudu] branch master updated: [hms] KUDU-1884 Add support for custom SASL protocol name

This is an automated email from the ASF dual-hosted git repository.

granthenke pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git


The following commit(s) were added to refs/heads/master by this push:
     new d1a7044  [hms] KUDU-1884 Add support for custom SASL protocol name
d1a7044 is described below

commit d1a7044f8c76f6e1504459325afa516d3a4d5f4f
Author: Grant Henke <gr...@apache.org>
AuthorDate: Tue Apr 27 14:10:11 2021 -0500

    [hms] KUDU-1884 Add support for custom SASL protocol name
    
    This patch adds support to set a custom SASL protocol name
    when using the KuduMetastorePlugin in the HMS. Because
    there isn’t a good way to pass configuration the plugin
    today, it uses an environment variable instead.
    
    The patch updates the master_hms-itest to use the new
    custom SASL protocol name feature to validate functionality.
    
    Change-Id: Id8d9e208da1d767bb24470ab031a5266461d070b
    Reviewed-on: http://gerrit.cloudera.org:8080/17350
    Reviewed-by: Grant Henke <gr...@apache.org>
    Tested-by: Grant Henke <gr...@apache.org>
---
 .../kudu/hive/metastore/KuduMetastorePlugin.java   | 16 +++++++++++-
 src/kudu/integration-tests/master_hms-itest.cc     |  8 ++++++
 src/kudu/integration-tests/security-itest.cc       | 30 +++++++++++++---------
 src/kudu/mini-cluster/external_mini_cluster.cc     |  5 ++++
 4 files changed, 46 insertions(+), 13 deletions(-)

diff --git a/java/kudu-hive/src/main/java/org/apache/kudu/hive/metastore/KuduMetastorePlugin.java b/java/kudu-hive/src/main/java/org/apache/kudu/hive/metastore/KuduMetastorePlugin.java
index 814107a..d304146 100644
--- a/java/kudu-hive/src/main/java/org/apache/kudu/hive/metastore/KuduMetastorePlugin.java
+++ b/java/kudu-hive/src/main/java/org/apache/kudu/hive/metastore/KuduMetastorePlugin.java
@@ -104,6 +104,10 @@ public class KuduMetastorePlugin extends MetaStoreEventListener {
   // are too many requests to the master.
   static final String SYNC_ENABLED_ENV = "KUDU_HMS_SYNC_ENABLED";
 
+  // System env to set a custom sasl protocol name for the Kudu client.
+  // TODO(ghenke): Use a Hive config parameter from the KuduStorageHandler instead.
+  static final String SASL_PROTOCOL_NAME_ENV = "KUDU_SASL_PROTOCOL_NAME";
+
   // Maps lists of master addresses to KuduClients to cache clients.
   private static final Map<String, KuduClient> KUDU_CLIENTS =
       new ConcurrentHashMap<String, KuduClient>();
@@ -512,7 +516,9 @@ public class KuduMetastorePlugin extends MetaStoreEventListener {
       try {
         client = UserGroupInformation.getLoginUser().doAs(
             (PrivilegedExceptionAction<KuduClient>) () ->
-                new KuduClient.KuduClientBuilder(kuduMasters).build()
+                new KuduClient.KuduClientBuilder(kuduMasters)
+                    .saslProtocolName(getSaslProtocolName())
+                    .build()
         );
       } catch (IOException | InterruptedException e) {
         throw new RuntimeException("Failed to create the Kudu client");
@@ -521,4 +527,12 @@ public class KuduMetastorePlugin extends MetaStoreEventListener {
     }
     return client;
   }
+
+  private static String getSaslProtocolName() {
+    String saslProtocolName = System.getenv(SASL_PROTOCOL_NAME_ENV);
+    if (saslProtocolName == null || saslProtocolName.isEmpty()) {
+      saslProtocolName = "kudu";
+    }
+    return saslProtocolName;
+  }
 }
diff --git a/src/kudu/integration-tests/master_hms-itest.cc b/src/kudu/integration-tests/master_hms-itest.cc
index bcb3911..c0ae05e 100644
--- a/src/kudu/integration-tests/master_hms-itest.cc
+++ b/src/kudu/integration-tests/master_hms-itest.cc
@@ -75,6 +75,7 @@ class MasterHmsTest : public ExternalMiniClusterITestBase {
     opts.num_masters = 1;
     opts.num_tablet_servers = 1;
     opts.enable_kerberos = EnableKerberos();
+    opts.principal = Principal();
     // Tune down the notification log poll period in order to speed up catalog convergence.
     opts.extra_master_flags.emplace_back("--hive_metastore_notification_log_poll_period_seconds=1");
     StartClusterWithOpts(std::move(opts));
@@ -164,6 +165,10 @@ class MasterHmsTest : public ExternalMiniClusterITestBase {
   virtual bool EnableKerberos() {
     return false;
   }
+
+  virtual string Principal() {
+    return "kudu";
+  }
 };
 
 TEST_F(MasterHmsTest, TestCreateTable) {
@@ -769,6 +774,9 @@ class MasterHmsKerberizedTest : public MasterHmsTest {
   bool EnableKerberos() override {
     return true;
   }
+  string Principal() override {
+    return "oryx";
+  }
 };
 
 // Checks that table ownership in a Kerberized cluster is set to the user
diff --git a/src/kudu/integration-tests/security-itest.cc b/src/kudu/integration-tests/security-itest.cc
index d8e1aed..aeb88e2 100644
--- a/src/kudu/integration-tests/security-itest.cc
+++ b/src/kudu/integration-tests/security-itest.cc
@@ -456,18 +456,24 @@ TEST_F(SecurityITest, TestNonDefaultPrincipal) {
   ASSERT_OK(StartCluster());
 
   // A client with the default SASL proto shouldn't be able to connect
-  client::sp::shared_ptr<KuduClient> default_client;
-  Status s = cluster_->CreateClient(nullptr, &default_client);
-  ASSERT_TRUE(s.IsNotAuthorized());
-  ASSERT_STR_CONTAINS(s.ToString(), "not found in Kerberos database");
-
-  // Create a client with the matching SASL proto name and verify it's able to
-  // connect to the cluster and perform basic actions.
-  KuduClientBuilder b;
-  b.sasl_protocol_name(kPrincipal);
-  client::sp::shared_ptr<KuduClient> client;
-  ASSERT_OK(cluster_->CreateClient(&b, &client));
-  SmokeTestCluster(client);
+  {
+    client::sp::shared_ptr<KuduClient> client;
+    KuduClientBuilder builder;
+    for (auto i = 0; i < cluster_->num_masters(); ++i) {
+      builder.add_master_server_addr(cluster_->master(i)->bound_rpc_addr().ToString());
+    }
+    const auto s = builder.Build(&client);
+    ASSERT_TRUE(s.IsNotAuthorized());
+    ASSERT_STR_CONTAINS(s.ToString(), "not found in Kerberos database");
+  }
+
+  {
+    // Create a client with the matching SASL proto name and verify it's able to
+    // connect to the cluster and perform basic actions.
+    client::sp::shared_ptr<KuduClient> client;
+    ASSERT_OK(cluster_->CreateClient(nullptr, &client));
+    SmokeTestCluster(client);
+  }
 }
 
 TEST_F(SecurityITest, TestNonExistentPrincipal) {
diff --git a/src/kudu/mini-cluster/external_mini_cluster.cc b/src/kudu/mini-cluster/external_mini_cluster.cc
index 0fec6b8..6a59811 100644
--- a/src/kudu/mini-cluster/external_mini_cluster.cc
+++ b/src/kudu/mini-cluster/external_mini_cluster.cc
@@ -334,6 +334,10 @@ Status ExternalMiniCluster::Start() {
                             "could not create keytab");
       hms_->EnableKerberos(kdc_->GetEnvVars()["KRB5_CONFIG"], spn, ktpath,
                            rpc::SaslProtection::kAuthentication);
+
+      // Set the protocol name in the environment so that the KuduMetastorePlugin
+      // can communicate with Kudu when a custom name is used.
+      hms_->AddEnvVar("KUDU_SASL_PROTOCOL_NAME", opts_.principal);
     }
 
     RETURN_NOT_OK_PREPEND(hms_->Start(),
@@ -917,6 +921,7 @@ Status ExternalMiniCluster::CreateClient(client::KuduClientBuilder* builder,
   for (const scoped_refptr<ExternalMaster>& master : masters_) {
     builder->add_master_server_addr(master->bound_rpc_hostport().ToString());
   }
+  builder->sasl_protocol_name(opts_.principal);
   return builder->Build(client);
 }