kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From t...@apache.org
Subject [2/2] kudu git commit: Enable GSSAPI for servers and ExternalMiniCluster
Date Tue, 01 Nov 2016 22:24:39 GMT
Enable GSSAPI for servers and ExternalMiniCluster

This adds a flag --server_require_kerberos, which makes the server only
advertise the GSSAPI mechanism. It also adds the appropriate hook to the
client to enable GSSAPI as a supported GSSAPI mechanism.

With this I was able to start a kudu master with the new flag enabled,
and then use Kerberos to authenticate a client.

This also adds some basic support to ExternalMiniCluster to start a
kerberized cluster. This involves starting a KDC, creating keytabs for
each server, passing the appropriate environment down as environment
variables, and setting up a client principal for the test itself.

In order for these tests to work, I had to tweak some settings in
krb5.conf, and then make corresponding changes to sasl_rpc-test to use
IP addresses instead of hostnames.

Change-Id: I595469e9cc8b2b2f57e9726014eeeb8112595801
Reviewed-on: http://gerrit.cloudera.org:8080/4765
Tested-by: Kudu Jenkins
Reviewed-by: Dan Burkert <danburkert@apache.org>
Reviewed-by: Alexey Serbin <aserbin@cloudera.com>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/758a5c45
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/758a5c45
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/758a5c45

Branch: refs/heads/master
Commit: 758a5c45a525cda10a26f95f4ddca804897dd097
Parents: 71ba497
Author: Todd Lipcon <todd@apache.org>
Authored: Wed Oct 19 13:43:10 2016 -0700
Committer: Todd Lipcon <todd@apache.org>
Committed: Tue Nov 1 22:22:11 2016 +0000

----------------------------------------------------------------------
 src/kudu/integration-tests/CMakeLists.txt       |  3 +-
 .../external_mini_cluster-test.cc               | 25 ++++++++-
 .../integration-tests/external_mini_cluster.cc  | 54 ++++++++++++++++++--
 .../integration-tests/external_mini_cluster.h   | 25 ++++++++-
 src/kudu/rpc/connection.cc                      | 26 +++++++++-
 src/kudu/rpc/sasl_rpc-test.cc                   | 10 ++--
 src/kudu/security/mini_kdc-test.cc              |  5 ++
 src/kudu/security/mini_kdc.cc                   | 15 ++++++
 src/kudu/security/mini_kdc.h                    | 10 +++-
 src/kudu/util/subprocess-test.cc                | 14 +++++
 src/kudu/util/subprocess.cc                     | 16 +++++-
 src/kudu/util/subprocess.h                      | 12 +++++
 12 files changed, 198 insertions(+), 17 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/758a5c45/src/kudu/integration-tests/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/CMakeLists.txt b/src/kudu/integration-tests/CMakeLists.txt
index 3095d81..0fed394 100644
--- a/src/kudu/integration-tests/CMakeLists.txt
+++ b/src/kudu/integration-tests/CMakeLists.txt
@@ -34,7 +34,8 @@ target_link_libraries(integration-tests
   kudu_client
   kudu_client_test_util
   kudu_fs
-  kudu_test_util)
+  kudu_test_util
+  security-test)
 add_dependencies(integration-tests
   kudu-tserver
   kudu-master)

http://git-wip-us.apache.org/repos/asf/kudu/blob/758a5c45/src/kudu/integration-tests/external_mini_cluster-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/external_mini_cluster-test.cc b/src/kudu/integration-tests/external_mini_cluster-test.cc
index 944e958..fbc63ca 100644
--- a/src/kudu/integration-tests/external_mini_cluster-test.cc
+++ b/src/kudu/integration-tests/external_mini_cluster-test.cc
@@ -27,6 +27,7 @@
 #include "kudu/gutil/stl_util.h"
 #include "kudu/gutil/strings/substitute.h"
 #include "kudu/gutil/strings/util.h"
+#include "kudu/security/mini_kdc.h"
 #include "kudu/util/metrics.h"
 #include "kudu/util/net/net_util.h"
 #include "kudu/util/test_util.h"
@@ -39,8 +40,20 @@ namespace kudu {
 using std::string;
 using strings::Substitute;
 
-TEST_F(KuduTest, TestBasicOperation) {
+enum KerberosMode {
+  WITHOUT_KERBEROS, WITH_KERBEROS
+};
+
+class ExternalMiniClusterTest : public KuduTest,
+                                public testing::WithParamInterface<KerberosMode> {};
+
+INSTANTIATE_TEST_CASE_P(KerberosOnAndOff,
+                        ExternalMiniClusterTest,
+                        ::testing::Values(WITHOUT_KERBEROS, WITH_KERBEROS));
+
+TEST_P(ExternalMiniClusterTest, TestBasicOperation) {
   ExternalMiniClusterOptions opts;
+  opts.enable_kerberos = GetParam() == WITH_KERBEROS;
 
   // Hard-coded RPC ports for the masters. This is safe, as this unit test
   // runs under a resource lock (see CMakeLists.txt in this directory).
@@ -94,6 +107,9 @@ TEST_F(KuduTest, TestBasicOperation) {
     EXPECT_GT(value, 0);
   }
 
+  // Ensure that all of the tablet servers can register with the masters.
+  ASSERT_OK(cluster.WaitForTabletServerCount(opts.num_tablet_servers, MonoDelta::FromSeconds(30)));
+
   // Restart a master and a tablet server. Make sure they come back up with the same ports.
   ExternalMaster* master = cluster.master(0);
   HostPort master_rpc = master->bound_rpc_hostport();
@@ -116,6 +132,13 @@ TEST_F(KuduTest, TestBasicOperation) {
   ASSERT_EQ(ts_rpc.ToString(), ts->bound_rpc_hostport().ToString());
   ASSERT_EQ(ts_http.ToString(), ts->bound_http_hostport().ToString());
 
+  // Verify that, in a Kerberized cluster, if we drop our Kerberos environment,
+  // we can't make RPCs to a server.
+  if (opts.enable_kerberos) {
+    ASSERT_OK(cluster.kdc()->Kdestroy());
+    Status s = cluster.SetFlag(ts, "foo", "bar");
+    ASSERT_STR_MATCHES(s.ToString(), "Not authorized.*No Kerberos credentials");
+  }
   cluster.Shutdown();
 }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/758a5c45/src/kudu/integration-tests/external_mini_cluster.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/external_mini_cluster.cc b/src/kudu/integration-tests/external_mini_cluster.cc
index 63eaff6..4e4420a 100644
--- a/src/kudu/integration-tests/external_mini_cluster.cc
+++ b/src/kudu/integration-tests/external_mini_cluster.cc
@@ -32,6 +32,7 @@
 #include "kudu/gutil/strings/util.h"
 #include "kudu/master/master.proxy.h"
 #include "kudu/master/master_rpc.h"
+#include "kudu/security/mini_kdc.h"
 #include "kudu/server/server_base.pb.h"
 #include "kudu/server/server_base.proxy.h"
 #include "kudu/tserver/tserver_service.proxy.h"
@@ -82,7 +83,8 @@ static bool kBindToUniqueLoopbackAddress = true;
 ExternalMiniClusterOptions::ExternalMiniClusterOptions()
     : num_masters(1),
       num_tablet_servers(1),
-      bind_to_unique_loopback_addresses(kBindToUniqueLoopbackAddress) {
+      bind_to_unique_loopback_addresses(kBindToUniqueLoopbackAddress),
+      enable_kerberos(false) {
 }
 
 ExternalMiniClusterOptions::~ExternalMiniClusterOptions() {
@@ -136,6 +138,17 @@ Status ExternalMiniCluster::Start() {
     RETURN_NOT_OK_PREPEND(s, "Could not create root dir " + data_root_);
   }
 
+  if (opts_.enable_kerberos) {
+    kdc_.reset(new MiniKdc());
+    RETURN_NOT_OK(kdc_->Start());
+    RETURN_NOT_OK_PREPEND(kdc_->CreateUserPrincipal("testuser"),
+                          "could not create client principal");
+    RETURN_NOT_OK_PREPEND(kdc_->Kinit("testuser"),
+                          "could not kinit as client");
+    RETURN_NOT_OK_PREPEND(kdc_->SetKrb5Environment(),
+                          "could not set krb5 client env");
+  }
+
   if (opts_.num_masters != 1) {
     RETURN_NOT_OK_PREPEND(StartDistributedMasters(),
                           "Failed to add distributed masters");
@@ -228,9 +241,16 @@ vector<string> SubstituteInFlags(const vector<string>&
orig_flags,
 
 Status ExternalMiniCluster::StartSingleMaster() {
   string exe = GetBinaryPath(kMasterBinaryName);
+  vector<string> flags = SubstituteInFlags(opts_.extra_master_flags, 0);
+
   scoped_refptr<ExternalMaster> master =
-    new ExternalMaster(messenger_, exe, GetDataPath("master-0"),
-                       SubstituteInFlags(opts_.extra_master_flags, 0));
+      new ExternalMaster(messenger_, exe, GetDataPath("master-0"), flags);
+
+  if (opts_.enable_kerberos) {
+    RETURN_NOT_OK_PREPEND(master->EnableKerberos(kdc_.get()),
+                          "could not enable Kerberos");
+  }
+
   RETURN_NOT_OK(master->Start());
   masters_.push_back(master);
   return Status::OK();
@@ -261,6 +281,10 @@ Status ExternalMiniCluster::StartDistributedMasters() {
                            GetDataPath(Substitute("master-$0", i)),
                            peer_addrs[i],
                            SubstituteInFlags(flags, i));
+    if (opts_.enable_kerberos) {
+      RETURN_NOT_OK_PREPEND(peer->EnableKerberos(kdc_.get()),
+                            "could not enable Kerberos");
+    }
     RETURN_NOT_OK_PREPEND(peer->Start(),
                           Substitute("Unable to start Master at index $0", i));
     masters_.push_back(peer);
@@ -296,6 +320,11 @@ Status ExternalMiniCluster::AddTabletServer() {
                              GetBindIpForTabletServer(idx),
                              master_hostports,
                              SubstituteInFlags(opts_.extra_tserver_flags, idx));
+  if (opts_.enable_kerberos) {
+    RETURN_NOT_OK_PREPEND(ts->EnableKerberos(kdc_.get()),
+                          "could not enable Kerberos");
+  }
+
   RETURN_NOT_OK(ts->Start());
   tablet_servers_.push_back(ts);
   return Status::OK();
@@ -547,6 +576,19 @@ ExternalDaemon::ExternalDaemon(std::shared_ptr<rpc::Messenger>
messenger,
 ExternalDaemon::~ExternalDaemon() {
 }
 
+Status ExternalDaemon::EnableKerberos(MiniKdc* kdc) {
+  string ktpath;
+  RETURN_NOT_OK_PREPEND(kdc->CreateServiceKeytab("kudu/127.0.0.1", &ktpath),
+                        "could not create keytab");
+  extra_env_ = kdc->GetEnvVars();
+  extra_env_["KRB5_KTNAME"] = ktpath;
+  extra_env_["KRB5_CLIENT_KTNAME"] = ktpath;
+  // Have the daemons use an in-memory ticket cache, so they don't accidentally
+  // pick up credentials from the test case itself or from any other daemon.
+  extra_env_["KRB5CCNAME"] = "MEMORY:foo";
+  extra_flags_.push_back("--server_require_kerberos");
+  return Status::OK();
+}
 
 Status ExternalDaemon::StartProcess(const vector<string>& user_flags) {
   CHECK(!process_);
@@ -593,7 +635,11 @@ Status ExternalDaemon::StartProcess(const vector<string>& user_flags)
{
 
   gscoped_ptr<Subprocess> p(new Subprocess(exe_, argv));
   p->ShareParentStdout(false);
-  LOG(INFO) << "Running " << exe_ << "\n" << JoinStrings(argv, "\n");
+  p->SetEnvVars(extra_env_);
+  string env_str;
+  JoinMapKeysAndValues(extra_env_, "=", ",", &env_str);
+  LOG(INFO) << "Running " << exe_ << "\n" << JoinStrings(argv, "\n")
+            << " with env {" << env_str << "}";
   RETURN_NOT_OK_PREPEND(p->Start(),
                         Substitute("Failed to start subprocess $0", exe_));
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/758a5c45/src/kudu/integration-tests/external_mini_cluster.h
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/external_mini_cluster.h b/src/kudu/integration-tests/external_mini_cluster.h
index d24d12e..537c676 100644
--- a/src/kudu/integration-tests/external_mini_cluster.h
+++ b/src/kudu/integration-tests/external_mini_cluster.h
@@ -17,6 +17,7 @@
 #ifndef KUDU_INTEGRATION_TESTS_EXTERNAL_MINI_CLUSTER_H
 #define KUDU_INTEGRATION_TESTS_EXTERNAL_MINI_CLUSTER_H
 
+#include <map>
 #include <memory>
 #include <string>
 #include <sys/types.h>
@@ -39,6 +40,7 @@ class ExternalTabletServer;
 class HostPort;
 class MetricPrototype;
 class MetricEntityPrototype;
+class MiniKdc;
 class NodeInstancePB;
 class Sockaddr;
 class Subprocess;
@@ -111,6 +113,14 @@ struct ExternalMiniClusterOptions {
   // masters in a consensus configuration. Port at index 0 is used for the leader
   // master.
   std::vector<uint16_t> master_rpc_ports;
+
+  // If true, set up a KDC as part of this MiniCluster, generate keytabs for
+  // the servers, and require Kerberos authentication from clients.
+  //
+  // Additionally, when the cluster is started, the environment of the
+  // test process will be modified to include Kerberos credentials for
+  // a principal named 'testuser'.
+  bool enable_kerberos;
 };
 
 // A mini-cluster made up of subprocesses running each of the daemons
@@ -195,6 +205,10 @@ class ExternalMiniCluster : public MiniClusterBase {
   // Return all tablet servers and masters.
   std::vector<ExternalDaemon*> daemons() const;
 
+  MiniKdc* kdc() const {
+    return CHECK_NOTNULL(kdc_.get());
+  }
+
   int num_tablet_servers() const override {
     return tablet_servers_.size();
   }
@@ -283,6 +297,7 @@ class ExternalMiniCluster : public MiniClusterBase {
 
   std::vector<scoped_refptr<ExternalMaster> > masters_;
   std::vector<scoped_refptr<ExternalTabletServer> > tablet_servers_;
+  std::unique_ptr<MiniKdc> kdc_;
 
   std::shared_ptr<rpc::Messenger> messenger_;
 
@@ -309,6 +324,13 @@ class ExternalDaemon : public RefCountedThreadSafe<ExternalDaemon>
{
   // The daemon must be shut down before calling this method.
   void SetExePath(std::string exe);
 
+  // Enable Kerberos for this daemon. This creates a Kerberos principal
+  // and keytab, and sets the appropriate environment variables in the
+  // subprocess such that the server will use Kerberos authentication.
+  //
+  // Must be called before 'StartProcess()'.
+  Status EnableKerberos(MiniKdc* kdc);
+
   // Sends a SIGSTOP signal to the daemon.
   Status Pause();
 
@@ -354,7 +376,7 @@ class ExternalDaemon : public RefCountedThreadSafe<ExternalDaemon>
{
   friend class RefCountedThreadSafe<ExternalDaemon>;
   virtual ~ExternalDaemon();
 
-  Status StartProcess(const std::vector<std::string>& flags);
+  Status StartProcess(const std::vector<std::string>& user_flags);
 
   // In a code-coverage build, try to flush the coverage data to disk.
   // In a non-coverage build, this does nothing.
@@ -364,6 +386,7 @@ class ExternalDaemon : public RefCountedThreadSafe<ExternalDaemon>
{
   const std::string data_dir_;
   std::string exe_;
   std::vector<std::string> extra_flags_;
+  std::map<std::string, std::string> extra_env_;
 
   gscoped_ptr<Subprocess> process_;
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/758a5c45/src/kudu/rpc/connection.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/connection.cc b/src/kudu/rpc/connection.cc
index bdc4f9d..8e64e1b 100644
--- a/src/kudu/rpc/connection.cc
+++ b/src/kudu/rpc/connection.cc
@@ -40,6 +40,7 @@
 #include "kudu/rpc/sasl_server.h"
 #include "kudu/rpc/transfer.h"
 #include "kudu/util/debug-util.h"
+#include "kudu/util/flag_tags.h"
 #include "kudu/util/net/sockaddr.h"
 #include "kudu/util/status.h"
 #include "kudu/util/trace.h"
@@ -51,6 +52,14 @@ using std::shared_ptr;
 using std::vector;
 using strings::Substitute;
 
+DEFINE_bool(server_require_kerberos, false,
+            "Whether to force all inbound RPC connections to authenticate "
+            "with Kerberos");
+// TODO(todd): this flag is too coarse-grained, since secure servers still
+// need to allow non-kerberized connections authenticated by tokens. But
+// it's a useful stop-gap.
+TAG_FLAG(server_require_kerberos, experimental);
+
 namespace kudu {
 namespace rpc {
 
@@ -619,6 +628,14 @@ std::string Connection::ToString() const {
 }
 
 Status Connection::InitSaslClient() {
+  // Note that remote_.host() is an IP address here: we've already lost
+  // whatever DNS name the client was attempting to use. Unless krb5
+  // is configured with 'rdns = false', it will automatically take care
+  // of reversing this address to its canonical hostname to determine
+  // the expected server principal.
+  sasl_client().set_server_fqdn(remote_.host());
+  RETURN_NOT_OK(sasl_client().EnableGSSAPI());
+  // TODO(todd): we dont seem to ever use ANONYMOUS. Should we remove it?
   RETURN_NOT_OK(sasl_client().EnableAnonymous());
   RETURN_NOT_OK(sasl_client().EnablePlain(user_credentials().real_user(), ""));
   RETURN_NOT_OK(sasl_client().Init(kSaslProtoName));
@@ -626,7 +643,14 @@ Status Connection::InitSaslClient() {
 }
 
 Status Connection::InitSaslServer() {
-  RETURN_NOT_OK(sasl_server().EnablePlain());
+  if (FLAGS_server_require_kerberos) {
+    // TODO(todd): should we use krb5 APIs directly to verify that we have a valid
+    // keytab when we start up, rather than starting up and then rejecting
+    // connections?
+    RETURN_NOT_OK(sasl_server().EnableGSSAPI());
+  } else {
+    RETURN_NOT_OK(sasl_server().EnablePlain());
+  }
   RETURN_NOT_OK(sasl_server().Init(kSaslProtoName));
   return Status::OK();
 }

http://git-wip-us.apache.org/repos/asf/kudu/blob/758a5c45/src/kudu/rpc/sasl_rpc-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/sasl_rpc-test.cc b/src/kudu/rpc/sasl_rpc-test.cc
index 61b3377..7592901 100644
--- a/src/kudu/rpc/sasl_rpc-test.cc
+++ b/src/kudu/rpc/sasl_rpc-test.cc
@@ -188,7 +188,7 @@ TEST_F(TestSaslRpc, TestRestrictiveServer_NonRestrictiveClient) {
 
   // Create the server principal and keytab.
   string kt_path;
-  ASSERT_OK(kdc.CreateServiceKeytab("kudu/localhost", &kt_path));
+  ASSERT_OK(kdc.CreateServiceKeytab("kudu/127.0.0.1", &kt_path));
   CHECK_ERR(setenv("KRB5_KTNAME", kt_path.c_str(), 1 /*replace*/));
 
   // Create and kinit as a client user.
@@ -277,7 +277,7 @@ TEST_F(TestSaslRpc, TestGSSAPINegotiation) {
 
   // Create the server principal and keytab.
   string kt_path;
-  ASSERT_OK(kdc.CreateServiceKeytab("kudu/localhost", &kt_path));
+  ASSERT_OK(kdc.CreateServiceKeytab("kudu/127.0.0.1", &kt_path));
   CHECK_ERR(setenv("KRB5_KTNAME", kt_path.c_str(), 1 /*replace*/));
 
   // Try to negotiate with no krb5 credentials on the client. It should fail on both
@@ -320,7 +320,7 @@ TEST_F(TestSaslRpc, TestGSSAPINegotiation) {
   // Change the server's keytab file so that it has inappropriate
   // credentials.
   // Authentication should now fail.
-  ASSERT_OK(kdc.CreateServiceKeytab("kudu/other-host", &kt_path));
+  ASSERT_OK(kdc.CreateServiceKeytab("otherservice/127.0.0.1", &kt_path));
   CHECK_ERR(setenv("KRB5_KTNAME", kt_path.c_str(), 1 /*replace*/));
 
   RunNegotiationTest(
@@ -328,13 +328,13 @@ TEST_F(TestSaslRpc, TestGSSAPINegotiation) {
                 [](const Status& s, SaslServer& server) {
                   CHECK(s.IsNotAuthorized());
                   ASSERT_STR_CONTAINS(s.ToString(),
-                                      "No key table entry found matching kudu/localhost");
+                                      "No key table entry found matching kudu/127.0.0.1");
                 }),
       std::bind(RunGSSAPINegotiationClient, std::placeholders::_1,
                 [](const Status& s, SaslClient& client) {
                   CHECK(s.IsNotAuthorized());
                   ASSERT_STR_CONTAINS(s.ToString(),
-                                      "No key table entry found matching kudu/localhost");
+                                      "No key table entry found matching kudu/127.0.0.1");
                 }));
 
 }

http://git-wip-us.apache.org/repos/asf/kudu/blob/758a5c45/src/kudu/security/mini_kdc-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/mini_kdc-test.cc b/src/kudu/security/mini_kdc-test.cc
index 5e93b07..66fcb4c 100644
--- a/src/kudu/security/mini_kdc-test.cc
+++ b/src/kudu/security/mini_kdc-test.cc
@@ -47,6 +47,11 @@ TEST(MiniKdcTest, TestBasicOperation) {
   ASSERT_STR_CONTAINS(klist, "bob@KRBTEST.COM");
   ASSERT_STR_CONTAINS(klist, "krbtgt/KRBTEST.COM@KRBTEST.COM");
 
+  // Drop 'bob' credentials. We'll get a RuntimeError because klist
+  // exits with a non-zero exit code if there are no cached credentials.
+  ASSERT_OK(kdc.Kdestroy());
+  ASSERT_TRUE(kdc.Klist(&klist).IsRuntimeError());
+
   // Test keytab creation.
   string kt_path;
   ASSERT_OK(kdc.CreateServiceKeytab("kudu/foo.example.com", &kt_path));

http://git-wip-us.apache.org/repos/asf/kudu/blob/758a5c45/src/kudu/security/mini_kdc.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/mini_kdc.cc b/src/kudu/security/mini_kdc.cc
index 3cc88f1..a5fd196 100644
--- a/src/kudu/security/mini_kdc.cc
+++ b/src/kudu/security/mini_kdc.cc
@@ -223,6 +223,15 @@ Status MiniKdc::CreateKrb5Conf() const {
     renew_lifetime = 7d
     ticket_lifetime = 24h
 
+    # In miniclusters, we start daemons on local loopback IPs that
+    # have no reverse DNS entries. So, disable reverse DNS.
+    rdns = false
+
+    # The server side will start its GSSAPI server using the local FQDN.
+    # However, in tests, we connect to it via a non-matching loopback IP.
+    # This enables us to connect despite that mismatch.
+    ignore_acceptor_hostname = true
+
     # The KDC is configured to only use TCP, so the client should not prefer UDP.
     udp_preference_limit = 0
 
@@ -318,6 +327,12 @@ Status MiniKdc::Kinit(const string& username) {
   return Status::OK();
 }
 
+Status MiniKdc::Kdestroy() {
+  string kdestroy;
+  RETURN_NOT_OK(GetBinaryPath("kdestroy", &kdestroy));
+  return Subprocess::Call(MakeArgv({ kdestroy, "-A" }));
+}
+
 Status MiniKdc::Klist(string* output) {
   string klist;
   RETURN_NOT_OK(GetBinaryPath("klist", &klist));

http://git-wip-us.apache.org/repos/asf/kudu/blob/758a5c45/src/kudu/security/mini_kdc.h
----------------------------------------------------------------------
diff --git a/src/kudu/security/mini_kdc.h b/src/kudu/security/mini_kdc.h
index aa24b5f..bb5dede 100644
--- a/src/kudu/security/mini_kdc.h
+++ b/src/kudu/security/mini_kdc.h
@@ -84,6 +84,10 @@ class MiniKdc {
   // Kinit a user to the mini KDC.
   Status Kinit(const std::string& username) WARN_UNUSED_RESULT;
 
+  // Destroy any credentials in the current ticket cache.
+  // Equivalent to 'kdestroy -A'.
+  Status Kdestroy() WARN_UNUSED_RESULT;
+
   // Call the 'klist' utility.  This is useful for logging the local ticket
   // cache state.
   Status Klist(std::string* output) WARN_UNUSED_RESULT;
@@ -97,10 +101,12 @@ class MiniKdc {
   // configuration associated with this KDC.
   Status SetKrb5Environment() const;
 
- private:
-  // Returns a map of the necessary Kerberos environment variables.
+  // Returns a map of the Kerberos environment variables which configure
+  // a process to use this KDC.
   std::map<std::string, std::string> GetEnvVars() const;
 
+ private:
+
   // Prepends required Kerberos environment variables to the process arguments.
   std::vector<std::string> MakeArgv(const std::vector<std::string>& in_argv);
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/758a5c45/src/kudu/util/subprocess-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/subprocess-test.cc b/src/kudu/util/subprocess-test.cc
index f08802f..86238a8 100644
--- a/src/kudu/util/subprocess-test.cc
+++ b/src/kudu/util/subprocess-test.cc
@@ -131,6 +131,20 @@ TEST_F(SubprocessTest, TestReadFromStdoutAndStderr) {
   }, "", &stdout, &stderr));
 }
 
+// Test that environment variables can be passed to the subprocess.
+TEST_F(SubprocessTest, TestEnvVars) {
+  Subprocess p("bash", {"/bin/bash", "-c", "echo $FOO"});
+  p.SetEnvVars({{"FOO", "bar"}});
+  p.ShareParentStdout(false);
+  ASSERT_OK(p.Start());
+  FILE* in = fdopen(p.from_child_stdout_fd(), "r");
+  PCHECK(in);
+  char buf[1024];
+  ASSERT_EQ(buf, fgets(buf, sizeof(buf), in));
+  ASSERT_STREQ("bar\n", &buf[0]);
+  ASSERT_OK(p.Wait());
+}
+
 // Tests writing to the subprocess stdin.
 TEST_F(SubprocessTest, TestCallWithStdin) {
   string stdout;

http://git-wip-us.apache.org/repos/asf/kudu/blob/758a5c45/src/kudu/util/subprocess.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/subprocess.cc b/src/kudu/util/subprocess.cc
index 603cf99..5de4e90 100644
--- a/src/kudu/util/subprocess.cc
+++ b/src/kudu/util/subprocess.cc
@@ -345,7 +345,7 @@ Status Subprocess::Start() {
     PCHECK(pipe2(child_stderr, O_CLOEXEC) == 0);
   }
   // The synchronization pipe: this trick is to make sure the parent returns
-  // control only after the child process has invoked execve().
+  // control only after the child process has invoked execvp().
   int sync_pipe[2];
   PCHECK(pipe2(sync_pipe, O_CLOEXEC) == 0);
 
@@ -401,6 +401,14 @@ Status Subprocess::Start() {
 
     CloseNonStandardFDs(fd_dir);
 
+    // Set the environment for the subprocess. This is more portable than
+    // using execvpe(), which doesn't exist on OS X. We rely on the 'p'
+    // variant of exec to do $PATH searching if the executable specified
+    // by the caller isn't an absolute path.
+    for (const auto& env : env_) {
+      ignore_result(setenv(env.first.c_str(), env.second.c_str(), 1 /* overwrite */));
+    }
+
     execvp(program_.c_str(), &argv_ptrs[0]);
     int err = errno;
     PLOG(ERROR) << "Couldn't exec " << program_;
@@ -417,7 +425,7 @@ Status Subprocess::Start() {
     child_fds_[STDOUT_FILENO] = child_stdout[0];
     child_fds_[STDERR_FILENO] = child_stderr[0];
 
-    // Wait for the child process to invoke execve(). The trick involves
+    // Wait for the child process to invoke execvp(). The trick involves
     // a pipe with O_CLOEXEC option for its descriptors. The parent process
     // performs blocking read from the pipe while the write side of the pipe
     // is kept open by the child (it does not write any data, though). The write
@@ -621,6 +629,10 @@ Status Subprocess::DoWait(int* wait_status, WaitMode mode) {
   return Status::OK();
 }
 
+void Subprocess::SetEnvVars(std::map<std::string, std::string> env) {
+  env_ = std::move(env);
+}
+
 void Subprocess::SetFdShared(int stdfd, bool share) {
   CHECK_EQ(state_, kNotStarted);
   CHECK_NE(fd_state_[stdfd], DISABLED);

http://git-wip-us.apache.org/repos/asf/kudu/blob/758a5c45/src/kudu/util/subprocess.h
----------------------------------------------------------------------
diff --git a/src/kudu/util/subprocess.h b/src/kudu/util/subprocess.h
index 2ec9468..73c0497 100644
--- a/src/kudu/util/subprocess.h
+++ b/src/kudu/util/subprocess.h
@@ -17,6 +17,7 @@
 #ifndef KUDU_UTIL_SUBPROCESS_H
 #define KUDU_UTIL_SUBPROCESS_H
 
+#include <map>
 #include <string>
 #include <vector>
 
@@ -59,6 +60,16 @@ class Subprocess {
   void ShareParentStdout(bool share = true) { SetFdShared(STDOUT_FILENO, share); }
   void ShareParentStderr(bool share = true) { SetFdShared(STDERR_FILENO, share); }
 
+  // Add environment variables to be set before executing the subprocess.
+  //
+  // These environment variables are merged into the existing environment
+  // of the parent process. In other words, there is no need to prime this
+  // map with the current environment; instead, just specify any variables
+  // that should be overridden.
+  //
+  // Repeated calls to this function replace earlier calls.
+  void SetEnvVars(std::map<std::string, std::string> env);
+
   // Start the subprocess. Can only be called once.
   //
   // This returns a bad Status if the fork() fails. However,
@@ -145,6 +156,7 @@ class Subprocess {
 
   std::string program_;
   std::vector<std::string> argv_;
+  std::map<std::string, std::string> env_;
   State state_;
   int child_pid_;
   enum StreamMode fd_state_[3];


Mime
View raw message