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 2020/04/02 21:54:03 UTC

[kudu] 02/04: [ranger] Use unique loopback for MiniRanger

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

commit fc3db8fef17c600ea5cbe14f6f1fc2fe61a4063d
Author: Attila Bukor <ab...@apache.org>
AuthorDate: Tue Mar 31 20:43:25 2020 +0200

    [ranger] Use unique loopback for MiniRanger
    
    Other services use a unique loopback address on Linux systems to avoid
    conflicting ports. This patch changes Ranger and Postgres to use this
    unique loopback as well. Unfortunately as Ranger starts up Tomcat
    without specifying the bind address, it always listens to all
    interfaces.  Because of this the unique loopback trick is not enough in
    itself and the randomized port number is also used in this case.
    
    Change-Id: Id8833bb149fb2410f4f140b8fb67b1039a2f90d0
    Reviewed-on: http://gerrit.cloudera.org:8080/15625
    Tested-by: Kudu Jenkins
    Reviewed-by: Hao Hao <ha...@cloudera.com>
    Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
 src/kudu/mini-cluster/external_mini_cluster.cc |  6 ++++--
 src/kudu/postgres/mini_postgres-test.cc        |  5 +++++
 src/kudu/postgres/mini_postgres.cc             | 11 +++++++----
 src/kudu/postgres/mini_postgres.h              |  8 +++++---
 src/kudu/ranger/mini_ranger-test.cc            |  2 ++
 src/kudu/ranger/mini_ranger.cc                 | 17 ++++++++++-------
 src/kudu/ranger/mini_ranger.h                  | 10 ++++++----
 src/kudu/util/net/net_util-test.cc             |  2 +-
 src/kudu/util/net/net_util.cc                  |  8 ++++----
 src/kudu/util/net/net_util.h                   | 13 +++++++------
 10 files changed, 51 insertions(+), 31 deletions(-)

diff --git a/src/kudu/mini-cluster/external_mini_cluster.cc b/src/kudu/mini-cluster/external_mini_cluster.cc
index d3cb42c..c249286 100644
--- a/src/kudu/mini-cluster/external_mini_cluster.cc
+++ b/src/kudu/mini-cluster/external_mini_cluster.cc
@@ -335,8 +335,10 @@ Status ExternalMiniCluster::Start() {
   }
 
   if (opts_.enable_ranger) {
-    ranger_.reset(new ranger::MiniRanger(cluster_root()));
-    string host = "127.0.0.1";
+    string host = GetBindIpForExternalServer(0);
+    ranger_.reset(new ranger::MiniRanger(cluster_root(), host));
+    // We're using the same service index as Sentry as they can't be both
+    // started at the same time.
     if (opts_.enable_kerberos) {
 
       // The SPNs match the ones defined in mini_ranger_configs.h.
diff --git a/src/kudu/postgres/mini_postgres-test.cc b/src/kudu/postgres/mini_postgres-test.cc
index c9efa72..4a42e82 100644
--- a/src/kudu/postgres/mini_postgres-test.cc
+++ b/src/kudu/postgres/mini_postgres-test.cc
@@ -17,6 +17,8 @@
 
 #include "kudu/postgres/mini_postgres.h"
 
+#include <string>
+
 #include <gtest/gtest.h>
 
 #include "kudu/util/status.h"
@@ -27,6 +29,9 @@ namespace kudu {
 namespace postgres {
 class PostgresTest : public KuduTest {
  public:
+  PostgresTest()
+    : postgres_("127.0.0.1") {}
+
   void SetUp() override {
     postgres_.Start();
   }
diff --git a/src/kudu/postgres/mini_postgres.cc b/src/kudu/postgres/mini_postgres.cc
index 9fe2c22..5b0e60d 100644
--- a/src/kudu/postgres/mini_postgres.cc
+++ b/src/kudu/postgres/mini_postgres.cc
@@ -70,7 +70,9 @@ Status MiniPostgres::Start() {
 
     // Postgres doesn't support binding to 0 so we need to get a random unused
     // port and persist that to the config file.
-    RETURN_NOT_OK(GetRandomPort(&port_));
+    if (port_ == 0) {
+      RETURN_NOT_OK(GetRandomPort(host_, &port_));
+    }
     RETURN_NOT_OK(CreateConfigs());
   }
 
@@ -84,8 +86,7 @@ Status MiniPostgres::Start() {
   });
   RETURN_NOT_OK(process_->Start());
 
-  const string ip = "127.0.0.1";
-  Status wait = WaitForTcpBind(process_->pid(), &port_, ip,
+  Status wait = WaitForTcpBind(process_->pid(), &port_, host_,
                                MonoDelta::FromMilliseconds(kPgStartTimeoutMs));
   if (!wait.ok()) {
     // TODO(abukor): implement retry with a different port if it can't bind
@@ -110,6 +111,7 @@ Status MiniPostgres::AddUser(const string& user, bool super) {
     user,
     Substitute("--$0superuser", super ? "" : "no-"),
     "-p", SimpleItoa(port_),
+    "-h", host_,
   });
   RETURN_NOT_OK(add.Start());
   return add.WaitAndCheckExitCode();
@@ -120,6 +122,7 @@ Status MiniPostgres::CreateDb(const string& db, const string& owner) {
     JoinPathSegments(bin_dir_, "postgres/createdb"),
     "-O", owner, db,
     "-p", SimpleItoa(port_),
+    "-h", host_,
   });
   RETURN_NOT_OK(createdb.Start());
   return createdb.WaitAndCheckExitCode();
@@ -132,7 +135,7 @@ Status MiniPostgres::CreateConfigs() {
   string config_file = JoinPathSegments(pg_root(), "postgresql.conf");
   faststring config;
   ReadFileToString(env, config_file, &config);
-  config.append(Substitute("\nport = $0\n", port_));
+  config.append(Substitute("\nlisten_addresses = '$0'\nport = $1\n", host_, port_));
   unique_ptr<WritableFile> file;
   RETURN_NOT_OK(env->NewWritableFile(config_file, &file));
   RETURN_NOT_OK(file->Append(config));
diff --git a/src/kudu/postgres/mini_postgres.h b/src/kudu/postgres/mini_postgres.h
index 7a7a136..c60a824 100644
--- a/src/kudu/postgres/mini_postgres.h
+++ b/src/kudu/postgres/mini_postgres.h
@@ -36,13 +36,14 @@ namespace postgres {
 // database connection (e.g. Apache Ranger).
 class MiniPostgres {
  public:
-  MiniPostgres()
-    : MiniPostgres(GetTestDataDirectory()) {}
+  explicit MiniPostgres(std::string host)
+    : MiniPostgres(GetTestDataDirectory(), std::move(host)) {}
 
   ~MiniPostgres();
 
-  explicit MiniPostgres(std::string data_root)
+  MiniPostgres(std::string data_root, std::string host)
     : data_root_(std::move(data_root)),
+      host_(std::move(host)),
       bin_dir_(GetBinDir()) {}
 
   Status Start();
@@ -88,6 +89,7 @@ class MiniPostgres {
 
   // Directory in which to put all our stuff.
   const std::string data_root_;
+  const std::string host_;
 
   // Directory that has the Postgres binary.
   // This may be in the thirdparty build, or may be shared across tests. As
diff --git a/src/kudu/ranger/mini_ranger-test.cc b/src/kudu/ranger/mini_ranger-test.cc
index 42fc271..9716938 100644
--- a/src/kudu/ranger/mini_ranger-test.cc
+++ b/src/kudu/ranger/mini_ranger-test.cc
@@ -35,6 +35,8 @@ namespace ranger {
 
 class MiniRangerTest : public KuduTest {
  public:
+  MiniRangerTest()
+    : ranger_("127.0.0.1") {}
   void SetUp() override {
     ASSERT_OK(ranger_.Start());
   }
diff --git a/src/kudu/ranger/mini_ranger.cc b/src/kudu/ranger/mini_ranger.cc
index 7c2f0a0..a880a97 100644
--- a/src/kudu/ranger/mini_ranger.cc
+++ b/src/kudu/ranger/mini_ranger.cc
@@ -94,20 +94,22 @@ Status MiniRanger::CreateConfigs() {
   //   command. We're not using this shutdown port as we simply send a SIGTERM,
   //   but it's necessary to set it to a random value to avoid collisions in
   //   parallel testing.
-  RETURN_NOT_OK(GetRandomPort(&port_));
+  if (port_ == 0) {
+    RETURN_NOT_OK(GetRandomPort(host_, &port_));
+  }
   uint16_t ranger_shutdown_port;
-  RETURN_NOT_OK(GetRandomPort(&ranger_shutdown_port));
+  RETURN_NOT_OK(GetRandomPort(host_, &ranger_shutdown_port));
   string admin_home = ranger_admin_home();
 
-  ranger_admin_url_ = Substitute("http://127.0.0.1:$0", port_);
+  ranger_admin_url_ = Substitute("http://$0:$1", host_, port_);
 
   // Write config files
   RETURN_NOT_OK(WriteStringToFile(
-      env_, GetRangerInstallProperties(bin_dir(), "127.0.0.1", mini_pg_.bound_port()),
+      env_, GetRangerInstallProperties(bin_dir(), host_, mini_pg_.bound_port()),
       JoinPathSegments(admin_home, "install.properties")));
 
   RETURN_NOT_OK(WriteStringToFile(
-      env_, GetRangerAdminSiteXml("127.0.0.1", port_, "127.0.0.1", mini_pg_.bound_port(),
+      env_, GetRangerAdminSiteXml(host_, port_, host_, mini_pg_.bound_port(),
                                   admin_ktpath_, lookup_ktpath_,
                                   spnego_ktpath_),
       JoinPathSegments(admin_home, "ranger-admin-site.xml")));
@@ -188,14 +190,15 @@ Status MiniRanger::StartRanger() {
 
     LOG(INFO) << "Using Ranger class path: " << classpath;
 
+    LOG(INFO) << "Using host: " << host_;
     std::vector<string> args({
         JoinPathSegments(java_home_, "bin/java"),
         "-Dproc_rangeradmin",
-        "-Dhostname=127.0.0.1",
+        Substitute("-Dhostname=$0", host_),
         Substitute("-Dlog4j.configuration=file:$0",
                    JoinPathSegments(kAdminHome, "log4j.properties")),
         "-Duser=miniranger",
-        "-Dranger.service.host=127.0.0.1",
+        Substitute("-Dranger.service.host=$0", host_),
         "-Dservername=miniranger",
         Substitute("-Dcatalina.base=$0", kEwsDir),
         Substitute("-Dlogdir=$0", JoinPathSegments(kAdminHome, "logs")),
diff --git a/src/kudu/ranger/mini_ranger.h b/src/kudu/ranger/mini_ranger.h
index b1d009b..e6ad364 100644
--- a/src/kudu/ranger/mini_ranger.h
+++ b/src/kudu/ranger/mini_ranger.h
@@ -63,14 +63,15 @@ struct AuthorizationPolicy {
 // Wrapper around Apache Ranger to be used in integration tests.
 class MiniRanger {
  public:
-  MiniRanger()
-    : MiniRanger(GetTestDataDirectory()) {}
+  explicit MiniRanger(std::string host)
+    : MiniRanger(GetTestDataDirectory(), std::move(host)) {}
 
   ~MiniRanger();
 
-  explicit MiniRanger(std::string data_root)
+  MiniRanger(std::string data_root, std::string host)
     : data_root_(std::move(data_root)),
-      mini_pg_(data_root_),
+      host_(std::move(host)),
+      mini_pg_(data_root_, host_),
       kerberos_(false),
       env_(Env::Default()) {
         curl_.set_auth(CurlAuthType::BASIC, "admin", "admin");
@@ -149,6 +150,7 @@ class MiniRanger {
 
   // Directory in which to put all our stuff.
   const std::string data_root_;
+  const std::string host_;
 
   postgres::MiniPostgres mini_pg_;
   std::unique_ptr<Subprocess> process_;
diff --git a/src/kudu/util/net/net_util-test.cc b/src/kudu/util/net/net_util-test.cc
index 26fbeec..5422c00 100644
--- a/src/kudu/util/net/net_util-test.cc
+++ b/src/kudu/util/net/net_util-test.cc
@@ -209,7 +209,7 @@ TEST_F(NetUtilTest, TestGetFQDN) {
 
 TEST_F(NetUtilTest, TestGetRandomPort) {
   uint16_t port;
-  ASSERT_OK(GetRandomPort(&port));
+  ASSERT_OK(GetRandomPort("127.0.0.1", &port));
   LOG(INFO) << "Random port is " << port;
 }
 
diff --git a/src/kudu/util/net/net_util.cc b/src/kudu/util/net/net_util.cc
index 81835c1..9bdf3c6 100644
--- a/src/kudu/util/net/net_util.cc
+++ b/src/kudu/util/net/net_util.cc
@@ -437,12 +437,12 @@ Status HostPortFromSockaddrReplaceWildcard(const Sockaddr& addr, HostPort* hp) {
   return Status::OK();
 }
 
-Status GetRandomPort(uint16_t* port) {
-  Sockaddr address;
-  address.ParseString("127.0.0.1", 0);
+Status GetRandomPort(const string& address, uint16_t* port) {
+  Sockaddr sockaddr;
+  sockaddr.ParseString(address, 0);
   Socket listener;
   RETURN_NOT_OK(listener.Init(0));
-  RETURN_NOT_OK(listener.Bind(address));
+  RETURN_NOT_OK(listener.Bind(sockaddr));
   Sockaddr listen_address;
   RETURN_NOT_OK(listener.GetSocketAddress(&listen_address));
   *port = listen_address.port();
diff --git a/src/kudu/util/net/net_util.h b/src/kudu/util/net/net_util.h
index 3a561c0..01546b6 100644
--- a/src/kudu/util/net/net_util.h
+++ b/src/kudu/util/net/net_util.h
@@ -232,12 +232,13 @@ enum class BindMode {
   LOOPBACK
 };
 
-// Gets a random port from the ephemeral range by binding to port 0 and letting
-// the kernel choose an unused one from the ephemeral port range. The socket is
-// then immediately closed and it remains in TIME_WAIT for 2*tcp_fin_timeout (by
-// default 2*60=120 seconds). The kernel won't assign this port until it's in
-// TIME_WAIT but it can still be used by binding it explicitly.
-Status GetRandomPort(uint16_t* port);
+// Gets a random port from the ephemeral range by binding to port 0 on address
+// 'address' and letting the kernel choose an unused one from the ephemeral port
+// range. The socket is then immediately closed and it remains in TIME_WAIT for
+// 2*tcp_fin_timeout (by default 2*60=120 seconds). The kernel won't assign this
+// port until it's in TIME_WAIT but it can still be used by binding it
+// explicitly.
+Status GetRandomPort(const std::string& address, uint16_t* port);
 
 #if defined(__APPLE__)
   static constexpr const BindMode kDefaultBindMode = BindMode::LOOPBACK;