You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by ab...@apache.org on 2023/01/31 07:30:01 UTC

[kudu] branch master updated: KUDU-3423 Add support for Ranger KMS HA

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

abukor 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 57aa48267 KUDU-3423 Add support for Ranger KMS HA
57aa48267 is described below

commit 57aa482673d97506c22273e5e0f37c4b43c7fb64
Author: Attila Bukor <ab...@apache.org>
AuthorDate: Wed Nov 23 12:33:59 2022 +0100

    KUDU-3423 Add support for Ranger KMS HA
    
    Ranger KMS supports high availability by listing multiple Ranger KMS
    servers (comma-separated, no spaces). Up until now, Kudu allowed
    configuring only one Ranger KMS server.
    
    This patch adds support for listing multiple KMS servers. This is done
    by extending EasyCurl to support multiple URLs. Kudu always attempts to
    connect to the first one in the list, and if the connection fails due to
    a network error or a timeout, it fails over to the next one. If a server
    responds with an error, that is considered a valid response and then it
    is returned immediately to the caller without trying the remaining URLs.
    
    Change-Id: Ibef941ed20eda1f4e624c2c7e16ca7955af570d8
    Reviewed-on: http://gerrit.cloudera.org:8080/19271
    Tested-by: Kudu Jenkins
    Reviewed-by: Alexey Serbin <al...@apache.org>
---
 src/kudu/fs/fs_manager.cc                    |  9 ++++++---
 src/kudu/integration-tests/security-itest.cc | 25 +++++++++++++++++++++++++
 src/kudu/ranger-kms/ranger_kms_client.cc     | 20 ++++++++++++++------
 src/kudu/ranger-kms/ranger_kms_client.h      |  8 +++++---
 src/kudu/util/curl_util.cc                   | 28 ++++++++++++++++++++++++++++
 src/kudu/util/curl_util.h                    | 26 ++++++++++++++++++++++++++
 6 files changed, 104 insertions(+), 12 deletions(-)

diff --git a/src/kudu/fs/fs_manager.cc b/src/kudu/fs/fs_manager.cc
index d2c7231cb..6d9b9f4b9 100644
--- a/src/kudu/fs/fs_manager.cc
+++ b/src/kudu/fs/fs_manager.cc
@@ -137,8 +137,8 @@ DEFINE_validator(encryption_key_provider, [](const char* /*n*/, const std::strin
 });
 
 DEFINE_string(ranger_kms_url, "",
-              "URL of the Ranger KMS server. Must be set when 'encryption_key_provider' "
-              "is set to 'ranger-kms'.");
+              "Comma-separated list of Ranger KMS server URLs. Must be set when "
+              "'encryption_key_provider' is set to 'ranger-kms'.");
 
 DEFINE_string(encryption_cluster_key_name, "kudu_cluster_key",
               "Name of the cluster key that is used to encrypt server encryption keys as "
@@ -146,7 +146,10 @@ DEFINE_string(encryption_cluster_key_name, "kudu_cluster_key",
 
 bool ValidateRangerKMSFlags() {
   if (FLAGS_encryption_key_provider == "ranger-kms") {
-    if (FLAGS_ranger_kms_url.empty() || FLAGS_encryption_cluster_key_name.empty()) {
+    if (FLAGS_ranger_kms_url.empty() ||
+        FLAGS_encryption_cluster_key_name.empty() ||
+        static_cast<std::vector<std::string>>(strings::Split(
+            FLAGS_ranger_kms_url, ",", strings::SkipEmpty())).empty()) {
       LOG(ERROR) << "If 'encryption_key_provider' is set to 'ranger-kms', then "
                     "'ranger_kms_url' and 'encryption_cluster_key_name' must also be set.";
       return false;
diff --git a/src/kudu/integration-tests/security-itest.cc b/src/kudu/integration-tests/security-itest.cc
index 9b702fe0a..f8a253ae2 100644
--- a/src/kudu/integration-tests/security-itest.cc
+++ b/src/kudu/integration-tests/security-itest.cc
@@ -26,6 +26,7 @@
 #include <ostream>
 #include <string>
 #include <tuple>
+#include <type_traits>
 #include <vector>
 
 #include <gflags/gflags_declare.h>
@@ -49,6 +50,7 @@
 #include "kudu/master/master.proxy.h"
 #include "kudu/master/sys_catalog.h"
 #include "kudu/mini-cluster/external_mini_cluster.h"
+#include "kudu/ranger-kms/mini_ranger_kms.h"
 #include "kudu/rpc/messenger.h"
 #include "kudu/rpc/rpc_controller.h"
 #include "kudu/security/kinit_context.h"
@@ -693,6 +695,29 @@ TEST_F(SecurityITest, TestEncryptionWithKMSIntegration) {
   SmokeTestCluster(client, /* transactional */ false);
 }
 
+TEST_F(SecurityITest, TestEncryptionWithKMSIntegrationMultipleServers) {
+  cluster_opts_.enable_ranger = true;
+  cluster_opts_.enable_ranger_kms = true;
+  ASSERT_OK(StartCluster());
+  cluster_->Shutdown();
+
+  const string& url = cluster_->ranger_kms()->url();
+  for (int i = 0; i < cluster_->num_masters(); ++i) {
+    cluster_->master(i)->mutable_flags()
+        ->emplace_back("--ranger_kms_url=invalid.host:1234/kms," + url);
+  }
+  for (int i = 0; i < cluster_->num_tablet_servers(); ++i) {
+    cluster_->tablet_server(i)->mutable_flags()
+        ->emplace_back("--ranger_kms_url=invalid.host:1234/kms," + url);
+  }
+  ASSERT_OK(cluster_->Restart());
+
+  shared_ptr<KuduClient> client;
+  KuduClientBuilder b;
+  ASSERT_OK(cluster_->CreateClient(&b, &client));
+  SmokeTestCluster(client, /*transactional=*/false);
+}
+
 class EncryptionPolicyTest :
     public SecurityITest,
     public ::testing::WithParamInterface<tuple<
diff --git a/src/kudu/ranger-kms/ranger_kms_client.cc b/src/kudu/ranger-kms/ranger_kms_client.cc
index 29b8c69db..d2e380432 100644
--- a/src/kudu/ranger-kms/ranger_kms_client.cc
+++ b/src/kudu/ranger-kms/ranger_kms_client.cc
@@ -58,11 +58,15 @@ Status RangerKMSClient::DecryptKey(const string& encrypted_key,
   payload.Set("material", eek_b64);
   EasyCurl curl;
   curl.set_auth(CurlAuthType::SPNEGO);
-  string url = Substitute("$0/v1/keyversion/$1/_eek?eek_op=decrypt",
-                          kms_url_, key_version);
+  vector<string> urls;
+  urls.reserve(kms_urls_.size());
+  for (const auto& url : kms_urls_) {
+    urls.emplace_back(Substitute("$0/v1/keyversion/$1/_eek?eek_op=decrypt",
+                                 url, key_version));
+  }
   faststring resp;
   RETURN_NOT_OK_PREPEND(
-      curl.PostToURL(url, payload.ToString(), &resp, {"Content-Type: application/json"}),
+      curl.PostToURL(urls, payload.ToString(), &resp, {"Content-Type: application/json"}),
       "failed to decrypt server key");
   JsonReader r(resp.ToString());
   RETURN_NOT_OK(r.Init());
@@ -79,10 +83,14 @@ Status RangerKMSClient::GenerateEncryptedServerKey(string* encrypted_key,
                                                    string* key_version) {
   EasyCurl curl;
   curl.set_auth(CurlAuthType::SPNEGO);
-  string url = Substitute("$0/v1/key/$1/_eek?eek_op=generate&num_keys=1",
-                          kms_url_, cluster_key_name_);
+  vector<string> urls;
+  urls.reserve(kms_urls_.size());
+  for (const auto& url : kms_urls_) {
+    urls.emplace_back(Substitute("$0/v1/key/$1/_eek?eek_op=generate&num_keys=1",
+                      url, cluster_key_name_));
+  }
   faststring resp;
-  RETURN_NOT_OK_PREPEND(curl.FetchURL(url, &resp), "failed to generate server key");
+  RETURN_NOT_OK_PREPEND(curl.FetchURL(urls, &resp), "failed to generate server key");
   JsonReader r(resp.ToString());
   RETURN_NOT_OK(r.Init());
   vector<const Value*> keys;
diff --git a/src/kudu/ranger-kms/ranger_kms_client.h b/src/kudu/ranger-kms/ranger_kms_client.h
index 52e2c5708..df144ffca 100644
--- a/src/kudu/ranger-kms/ranger_kms_client.h
+++ b/src/kudu/ranger-kms/ranger_kms_client.h
@@ -19,15 +19,17 @@
 
 #include <string>
 #include <utility>
+#include <vector>
 
 #include "kudu/util/status.h"
+#include "kudu/gutil/strings/split.h"
 
 namespace kudu {
 namespace security {
 class RangerKMSClient {
  public:
-  RangerKMSClient(std::string kms_url, std::string cluster_key_name)
-    : kms_url_(std::move(kms_url)),
+  RangerKMSClient(const std::string& kms_url, std::string cluster_key_name)
+    : kms_urls_(strings::Split(kms_url, ",", strings::SkipEmpty())),
       cluster_key_name_(std::move(cluster_key_name)) {}
 
   Status DecryptKey(const std::string& encrypted_key,
@@ -40,7 +42,7 @@ class RangerKMSClient {
                                     std::string* key_version);
 
  private:
-  std::string kms_url_;
+  std::vector<std::string> kms_urls_;
   std::string cluster_key_name_;
 
 };
diff --git a/src/kudu/util/curl_util.cc b/src/kudu/util/curl_util.cc
index 439ada168..6dc936c98 100644
--- a/src/kudu/util/curl_util.cc
+++ b/src/kudu/util/curl_util.cc
@@ -107,6 +107,11 @@ Status EasyCurl::FetchURL(const string& url, faststring* dst,
   return DoRequest(url, nullptr, dst, headers);
 }
 
+Status EasyCurl::FetchURL(const vector<string>& urls, faststring* dst,
+                          const vector<string>& headers) {
+  return DoRequest(urls, nullptr, dst, headers);
+}
+
 Status EasyCurl::PostToURL(const string& url,
                            const string& post_data,
                            faststring* dst,
@@ -114,6 +119,29 @@ Status EasyCurl::PostToURL(const string& url,
   return DoRequest(url, &post_data, dst, headers);
 }
 
+Status EasyCurl::PostToURL(const vector<string>& urls,
+                           const string& post_data,
+                           faststring* dst,
+                           const vector<string>& headers) {
+  return DoRequest(urls, &post_data, dst, headers);
+}
+
+Status EasyCurl::DoRequest(const vector<string>& urls,
+                           const string* post_data,
+                           faststring* dst,
+                           const vector<string>& headers) {
+  DCHECK(!urls.empty());
+  Status s;
+  for (const auto& url : urls) {
+    s = DoRequest(url, post_data, dst, headers);
+    if (s.IsNetworkError() || s.IsTimedOut()) {
+      continue;
+    }
+    break;
+  }
+  return s;
+}
+
 Status EasyCurl::DoRequest(const string& url,
                            const string* post_data,
                            faststring* dst,
diff --git a/src/kudu/util/curl_util.h b/src/kudu/util/curl_util.h
index 378168814..3910d6736 100644
--- a/src/kudu/util/curl_util.h
+++ b/src/kudu/util/curl_util.h
@@ -56,6 +56,14 @@ class EasyCurl {
                   faststring* dst,
                   const std::vector<std::string>& headers = {});
 
+  // Same as above, but with failover support. If the connection to the first
+  // URL in the vector fails or times out, it attempts to connect to the next
+  // one in the list. It stops at the first successful attempt and returns the
+  // result.
+  Status FetchURL(const std::vector<std::string>& urls,
+                  faststring* dst,
+                  const std::vector<std::string>& headers = {});
+
   // Issue an HTTP POST to the given URL with the given data.
   // Returns results in 'dst' as above.
   // The optional param 'headers' holds additional headers.
@@ -65,6 +73,15 @@ class EasyCurl {
                    faststring* dst,
                    const std::vector<std::string>& headers = {});
 
+  // Same as above, but with failover support. If the connection to the first
+  // URL in the vector fails or times out, it attempts to connect to the next
+  // one in the list. It stops at the first successful attempt and returns the
+  // result.
+  Status PostToURL(const std::vector<std::string>& urls,
+                   const std::string& post_data,
+                   faststring* dst,
+                   const std::vector<std::string>& headers = {});
+
   // Set whether to verify the server's SSL certificate in the case of an HTTPS
   // connection.
   void set_verify_peer(bool verify) {
@@ -141,6 +158,15 @@ class EasyCurl {
                    faststring* dst,
                    const std::vector<std::string>& headers = {});
 
+  // Same as above, but with failover support. If the connection to the first
+  // URL in the vector fails or times out, it attempts to connect to the next
+  // one in the list. It stops at the first successful attempt and returns the
+  // result.
+  Status DoRequest(const std::vector<std::string>& url,
+                   const std::string* post_data,
+                   faststring* dst,
+                   const std::vector<std::string>& headers = {});
+
   CURL* curl_;
 
   std::string custom_method_;