kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ale...@apache.org
Subject [kudu] branch master updated: [tools] command to reset authz cache in kudu CLI
Date Mon, 06 May 2019 18:16:27 GMT
This is an automated email from the ASF dual-hosted git repository.

alexey 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 f1c691e  [tools] command to reset authz cache in kudu CLI
f1c691e is described below

commit f1c691ead23b310e2506596b21409c29426f259f
Author: Alexey Serbin <alexey@apache.org>
AuthorDate: Fri Apr 26 21:19:53 2019 -0700

    [tools] command to reset authz cache in kudu CLI
    
    Added command to reset authz privileges cache into kudu CLI.
    Synopsis:
    
      kudu master authz_cache reset <all_master_addresses_in_cluster>
    
    The tool requires all masters' end-points to be specified in the command
    line.  This is to keep things as consistent as possible, since resetting
    the authz privileges cache only on a subset of masters could lead to
    unexpected surprises upon master leadership change.  It's possible
    to reset authz privileges cache at any subset of masters in a Kudu
    cluster adding --force flag.  For example, to reset authz cache only
    at one master out of three in multi-master Kudu cluster, run
    
      kudu master authz_cache reset --force <master_address>
    
    Added corresponding tests as well.  A scenario for a 'positive' case
    is not among the added ones: it will be a separate effort to bring in
    HMS+Sentry into the framework for testing kudu CLI tool.  However,
    there is coverage for a 'positive' case for cache reset elsewhere.
    In particular, the scenarios of the SentryAuthzProviderCacheITest
    give appropriate coverage using the ResetAuthzCache RPC endpoint.
    Those scenarios test and AdminCliTest.TestAuthzResetCacheNotImplemented
    should provide enough coverage.
    
    Change-Id: If49f3ebdea22ea0df3aaec313b4db949dc834943
    Reviewed-on: http://gerrit.cloudera.org:8080/13139
    Reviewed-by: Andrew Wong <awong@cloudera.com>
    Reviewed-by: Hao Hao <hao.hao@cloudera.com>
    Tested-by: Alexey Serbin <aserbin@cloudera.com>
---
 src/kudu/tools/kudu-admin-test.cc    |  87 ++++++++++
 src/kudu/tools/kudu-tool-test.cc     |  10 +-
 src/kudu/tools/tool_action_master.cc | 301 ++++++++++++++++++++++++++---------
 3 files changed, 326 insertions(+), 72 deletions(-)

diff --git a/src/kudu/tools/kudu-admin-test.cc b/src/kudu/tools/kudu-admin-test.cc
index d35e4c2..23d7b06 100644
--- a/src/kudu/tools/kudu-admin-test.cc
+++ b/src/kudu/tools/kudu-admin-test.cc
@@ -50,6 +50,7 @@
 #include "kudu/gutil/basictypes.h"
 #include "kudu/gutil/gscoped_ptr.h"
 #include "kudu/gutil/map-util.h"
+#include "kudu/gutil/strings/join.h"
 #include "kudu/gutil/strings/split.h"
 #include "kudu/gutil/strings/strip.h"
 #include "kudu/gutil/strings/substitute.h"
@@ -2164,5 +2165,91 @@ TEST_F(AdminCliTest, TestDumpMemTrackers) {
   ASSERT_STR_CONTAINS(stdout, Substitute("\"id\":\"$0\"", tablet_tracker_id));
   ASSERT_STR_CONTAINS(stdout, Substitute("\"parent_id\":\"$0\"", tablet_tracker_id));
 }
+
+TEST_F(AdminCliTest, TestAuthzResetCacheIncorrectMasterAddressList) {
+  NO_FATALS(BuildAndStart());
+
+  const auto& master_addr = cluster_->master()->bound_rpc_hostport().ToString();
+  const vector<string> dup_master_addresses = { master_addr, master_addr, };
+  const auto& dup_master_addresses_str = JoinStrings(dup_master_addresses, ",");
+  string out;
+  string err;
+  Status s;
+
+  s = RunKuduTool({
+    "master",
+    "authz_cache",
+    "reset",
+    dup_master_addresses_str,
+  }, &out, &err);
+  ASSERT_TRUE(s.IsRuntimeError()) << ToolRunInfo(s, out, err);
+
+  const auto ref_err_msg = Substitute(
+      "Invalid argument: list of master addresses provided ($0) "
+      "does not match the actual cluster configuration ($1)",
+      dup_master_addresses_str, master_addr);
+  ASSERT_STR_CONTAINS(err, ref_err_msg);
+
+  // However, the '--force' option makes it possible to run the tool even
+  // if the specified list of master addresses does not match the actual
+  // list of master addresses in the cluster. The default authz provider
+  // doesn't have a cache of privileges, so the 'Not implemented' result status
+  // is exactly what's expected.
+  out.clear();
+  err.clear();
+  s = RunKuduTool({
+    "master",
+    "authz_cache",
+    "reset",
+    "--force",
+    dup_master_addresses_str,
+  }, &out, &err);
+  ASSERT_TRUE(s.IsRuntimeError()) << ToolRunInfo(s, out, err);
+  ASSERT_STR_CONTAINS(err,
+      "Not implemented: provider does not have privileges cache");
+}
+
+TEST_F(AdminCliTest, TestAuthzResetCacheNotAuthorized) {
+  vector<string> master_flags{ "--superuser_acl=no-such-user" };
+  NO_FATALS(BuildAndStart({}, master_flags));
+
+  // The tool should report an error: it's not possible to reset the cache
+  // since the OS user under which the tools is invoked is not a superuser/admin
+  // (the --superuser_acl flag is set to contain a non-existent user only).
+  string out;
+  string err;
+  Status s = RunKuduTool({
+    "master",
+    "authz_cache",
+    "reset",
+    cluster_->master()->bound_rpc_hostport().ToString(),
+  }, &out, &err);
+  ASSERT_TRUE(s.IsRuntimeError()) << ToolRunInfo(s, out, err);
+  ASSERT_STR_CONTAINS(err,
+      "Remote error: Not authorized: unauthorized access to method: "
+      "ResetAuthzCache");
+}
+
+TEST_F(AdminCliTest, TestAuthzResetCacheNotImplemented) {
+  NO_FATALS(BuildAndStart());
+
+  // Even if the OS user under which account the tool is running has admin Kudu
+  // credentials, the tool should report application error: it's not possible to
+  // reset the cache since the default authz provider doesn't have one. The
+  // system uses the default authz provider because the integration with
+  // HMS+Sentry is not enabled by default.
+  string out;
+  string err;
+  Status s = RunKuduTool({
+    "master",
+    "authz_cache",
+    "reset",
+    cluster_->master()->bound_rpc_hostport().ToString(),
+  }, &out, &err);
+  ASSERT_TRUE(s.IsRuntimeError()) << ToolRunInfo(s, out, err);
+  ASSERT_STR_CONTAINS(err,
+      "Not implemented: provider does not have privileges cache");
+}
+
 } // namespace tools
 } // namespace kudu
diff --git a/src/kudu/tools/kudu-tool-test.cc b/src/kudu/tools/kudu-tool-test.cc
index 2c9a002..d7b864f 100644
--- a/src/kudu/tools/kudu-tool-test.cc
+++ b/src/kudu/tools/kudu-tool-test.cc
@@ -1011,15 +1011,23 @@ TEST_F(ToolTest, TestModeHelp) {
   }
   {
     const vector<string> kMasterModeRegexes = {
+        "authz_cache.*Operate on the authz cache of a Kudu Master",
         "dump_memtrackers.*Dump the memtrackers",
         "get_flags.*Get the gflags",
         "set_flag.*Change a gflag value",
         "status.*Get the status",
-        "timestamp.*Get the current timestamp"
+        "timestamp.*Get the current timestamp",
+        "list.*List masters in a Kudu cluster",
     };
     NO_FATALS(RunTestHelp("master", kMasterModeRegexes));
   }
   {
+    const vector<string> kMasterAuthzCacheModeRegexes = {
+        "reset.*Reset the privileges cache",
+    };
+    NO_FATALS(RunTestHelp("master authz_cache", kMasterAuthzCacheModeRegexes));
+  }
+  {
     const vector<string> kPbcModeRegexes = {
         "dump.*Dump a PBC",
     };
diff --git a/src/kudu/tools/tool_action_master.cc b/src/kudu/tools/tool_action_master.cc
index e74c1b7..352ace1 100644
--- a/src/kudu/tools/tool_action_master.cc
+++ b/src/kudu/tools/tool_action_master.cc
@@ -18,12 +18,16 @@
 #include <algorithm>
 #include <iostream>
 #include <iterator>
+#include <map>
 #include <memory>
+#include <set>
 #include <string>
 #include <unordered_map>
+#include <utility>
 #include <vector>
 
 #include <boost/algorithm/string/predicate.hpp>
+#include <boost/optional/optional.hpp>
 #include <gflags/gflags_declare.h>
 #include <glog/logging.h>
 
@@ -38,22 +42,35 @@
 #include "kudu/master/master.h"
 #include "kudu/master/master.pb.h"
 #include "kudu/master/master.proxy.h"
+#include "kudu/rpc/rpc_controller.h"
 #include "kudu/tools/tool_action.h"
 #include "kudu/tools/tool_action_common.h"
+#include "kudu/util/monotime.h"
 #include "kudu/util/status.h"
 
+DECLARE_bool(force);
+DECLARE_int64(timeout_ms);
 DECLARE_string(columns);
 
-namespace kudu {
-
-using master::ListMastersRequestPB;
-using master::ListMastersResponsePB;
-using master::MasterServiceProxy;
+using kudu::master::ConnectToMasterRequestPB;
+using kudu::master::ConnectToMasterResponsePB;
+using kudu::master::ListMastersRequestPB;
+using kudu::master::ListMastersResponsePB;
+using kudu::master::Master;
+using kudu::master::MasterServiceProxy;
+using kudu::master::MasterServiceProxy;
+using kudu::master::ResetAuthzCacheRequestPB;
+using kudu::master::ResetAuthzCacheResponsePB;
+using kudu::rpc::RpcController;
 using std::cout;
+using std::map;
+using std::set;
 using std::string;
 using std::unique_ptr;
 using std::vector;
+using strings::Substitute;
 
+namespace kudu {
 namespace tools {
 namespace {
 
@@ -66,24 +83,24 @@ const char* const kValueArg = "value";
 
 Status MasterGetFlags(const RunnerContext& context) {
   const string& address = FindOrDie(context.required_args, kMasterAddressArg);
-  return PrintServerFlags(address, master::Master::kDefaultPort);
+  return PrintServerFlags(address, Master::kDefaultPort);
 }
 
 Status MasterSetFlag(const RunnerContext& context) {
   const string& address = FindOrDie(context.required_args, kMasterAddressArg);
   const string& flag = FindOrDie(context.required_args, kFlagArg);
   const string& value = FindOrDie(context.required_args, kValueArg);
-  return SetServerFlag(address, master::Master::kDefaultPort, flag, value);
+  return SetServerFlag(address, Master::kDefaultPort, flag, value);
 }
 
 Status MasterStatus(const RunnerContext& context) {
   const string& address = FindOrDie(context.required_args, kMasterAddressArg);
-  return PrintServerStatus(address, master::Master::kDefaultPort);
+  return PrintServerStatus(address, Master::kDefaultPort);
 }
 
 Status MasterTimestamp(const RunnerContext& context) {
   const string& address = FindOrDie(context.required_args, kMasterAddressArg);
-  return PrintServerTimestamp(address, master::Master::kDefaultPort);
+  return PrintServerTimestamp(address, Master::kDefaultPort);
 }
 
 Status ListMasters(const RunnerContext& context) {
@@ -114,7 +131,7 @@ Status ListMasters(const RunnerContext& context) {
                });
 
   auto hostport_to_string = [] (const HostPortPB& hostport) {
-    return strings::Substitute("$0:$1", hostport.host(), hostport.port());
+    return Substitute("$0:$1", hostport.host(), hostport.port());
   };
 
   for (const auto& column : strings::Split(FLAGS_columns, ",", strings::SkipEmpty()))
{
@@ -159,72 +176,214 @@ Status ListMasters(const RunnerContext& context) {
 
 Status MasterDumpMemTrackers(const RunnerContext& context) {
   const auto& address = FindOrDie(context.required_args, kMasterAddressArg);
-  return DumpMemTrackers(address, master::Master::kDefaultPort);
+  return DumpMemTrackers(address, Master::kDefaultPort);
+}
+
+// Make sure the list of master addresses specified in 'master_addresses'
+// corresponds to the actual list of masters addresses in the cluster,
+// as reported in ConnectToMasterResponsePB::master_addrs.
+Status VerifyMasterAddressList(const vector<string>& master_addresses) {
+  map<string, set<string>> addresses_per_master;
+  for (const auto& address : master_addresses) {
+    unique_ptr<MasterServiceProxy> proxy;
+    RETURN_NOT_OK(BuildProxy(address, Master::kDefaultPort, &proxy));
+
+    RpcController ctl;
+    ctl.set_timeout(MonoDelta::FromMilliseconds(FLAGS_timeout_ms));
+    ConnectToMasterRequestPB req;
+    ConnectToMasterResponsePB resp;
+    RETURN_NOT_OK(proxy->ConnectToMaster(req, &resp, &ctl));
+    const auto& resp_master_addrs = resp.master_addrs();
+    if (resp_master_addrs.size() != master_addresses.size()) {
+      const auto addresses_provided = JoinStrings(master_addresses, ",");
+      const auto addresses_cluster_config = JoinMapped(
+          resp_master_addrs,
+          [](const HostPortPB& pb) {
+            return Substitute("$0:$1", pb.host(), pb.port());
+          }, ",");
+      return Status::InvalidArgument(Substitute(
+          "list of master addresses provided ($0) "
+          "does not match the actual cluster configuration ($1) ",
+          addresses_provided, addresses_cluster_config));
+    }
+    set<string> addr_set;
+    for (const auto& hp : resp_master_addrs) {
+      addr_set.emplace(Substitute("$0:$1", hp.host(), hp.port()));
+    }
+    addresses_per_master.emplace(address, std::move(addr_set));
+  }
+
+  bool mismatch = false;
+  if (addresses_per_master.size() > 1) {
+    const auto it_0 = addresses_per_master.cbegin();
+    auto it_1 = addresses_per_master.begin();
+    ++it_1;
+    for (auto it = it_1; it != addresses_per_master.end(); ++it) {
+      if (it->second != it_0->second) {
+        mismatch = true;
+        break;
+      }
+    }
+  }
+
+  if (mismatch) {
+    string err_msg = Substitute("specified: ($0);",
+                                JoinStrings(master_addresses, ","));
+    for (const auto& e : addresses_per_master) {
+      err_msg += Substitute(" from master $0: ($1);",
+                            e.first, JoinStrings(e.second, ","));
+    }
+    return Status::ConfigurationError(
+        Substitute("master address lists mismatch: $0", err_msg));
+  }
+
+  return Status::OK();
+}
+
+Status ResetAuthzCacheAtMaster(const string& master_address) {
+  unique_ptr<MasterServiceProxy> proxy;
+  RETURN_NOT_OK(BuildProxy(master_address, Master::kDefaultPort, &proxy));
+
+  RpcController ctl;
+  ctl.set_timeout(MonoDelta::FromMilliseconds(FLAGS_timeout_ms));
+
+  ResetAuthzCacheRequestPB req;
+  ResetAuthzCacheResponsePB resp;
+  RETURN_NOT_OK(proxy->ResetAuthzCache(req, &resp, &ctl));
+  if (resp.has_error()) {
+    return StatusFromPB(resp.error().status());
+  }
+  return Status::OK();
+}
+
+Status ResetAuthzCache(const RunnerContext& context) {
+  const string& master_addresses_str =
+      FindOrDie(context.required_args, kMasterAddressesArg);
+  const vector<string>& master_addresses =
+      strings::Split(master_addresses_str, ",");
+
+  if (!FLAGS_force) {
+    // Make sure the list of master addresses specified for the command
+    // matches the actual list of masters in the cluster.
+    RETURN_NOT_OK(VerifyMasterAddressList(master_addresses));
+  }
+
+  // It makes sense to reset privileges cache at every master in the cluster.
+  // Otherwise, SentryAuthzProvider might return inconsistent results for authz
+  // requests upon master leadership change.
+  vector<Status> statuses;
+  statuses.reserve(master_addresses.size());
+  for (const auto& address : master_addresses) {
+    auto status = ResetAuthzCacheAtMaster(address);
+    statuses.emplace_back(std::move(status));
+  }
+  DCHECK_EQ(master_addresses.size(), statuses.size());
+  string err_str;
+  for (auto i = 0; i < statuses.size(); ++i) {
+    const auto& s = statuses[i];
+    if (s.ok()) {
+      continue;
+    }
+    err_str += Substitute(" error from master at $0: $1",
+                          master_addresses[i], s.ToString());
+  }
+  if (err_str.empty()) {
+    return Status::OK();
+  }
+  return Status::Incomplete(err_str);
 }
 
 } // anonymous namespace
 
 unique_ptr<Mode> BuildMasterMode() {
-  unique_ptr<Action> dump_memtrackers =
-      ActionBuilder("dump_memtrackers", &MasterDumpMemTrackers)
-      .Description("Dump the memtrackers from a Kudu Master")
-      .AddRequiredParameter({ kMasterAddressArg, kMasterAddressDesc })
-      .AddOptionalParameter("format")
-      .AddOptionalParameter("memtracker_output")
-      .AddOptionalParameter("timeout_ms")
-      .Build();
-
-  unique_ptr<Action> get_flags =
-      ActionBuilder("get_flags", &MasterGetFlags)
-      .Description("Get the gflags for a Kudu Master")
-      .AddRequiredParameter({ kMasterAddressArg, kMasterAddressDesc })
-      .AddOptionalParameter("all_flags")
-      .AddOptionalParameter("flag_tags")
-      .Build();
-
-  unique_ptr<Action> set_flag =
-      ActionBuilder("set_flag", &MasterSetFlag)
-      .Description("Change a gflag value on a Kudu Master")
-      .AddRequiredParameter({ kMasterAddressArg, kMasterAddressDesc })
-      .AddRequiredParameter({ kFlagArg, "Name of the gflag" })
-      .AddRequiredParameter({ kValueArg, "New value for the gflag" })
-      .AddOptionalParameter("force")
-      .Build();
-
-  unique_ptr<Action> status =
-      ActionBuilder("status", &MasterStatus)
-      .Description("Get the status of a Kudu Master")
-      .AddRequiredParameter({ kMasterAddressArg, kMasterAddressDesc })
-      .Build();
-
-  unique_ptr<Action> timestamp =
-      ActionBuilder("timestamp", &MasterTimestamp)
-      .Description("Get the current timestamp of a Kudu Master")
-      .AddRequiredParameter({ kMasterAddressArg, kMasterAddressDesc })
-      .Build();
-
-  unique_ptr<Action> list_masters =
-      ActionBuilder("list", &ListMasters)
-      .Description("List masters in a Kudu cluster")
-      .AddRequiredParameter({ kMasterAddressesArg, kMasterAddressesArgDesc })
-      .AddOptionalParameter("columns", string("uuid,rpc-addresses"),
-                            string("Comma-separated list of master info fields to "
-                                   "include in output.\nPossible values: uuid, "
-                                   "rpc-addresses, http-addresses, version, seqno "
-                                   "and start_time"))
-      .AddOptionalParameter("format")
-      .AddOptionalParameter("timeout_ms")
-      .Build();
-
-  return ModeBuilder("master")
-      .Description("Operate on a Kudu Master")
-      .AddAction(std::move(dump_memtrackers))
-      .AddAction(std::move(get_flags))
-      .AddAction(std::move(set_flag))
-      .AddAction(std::move(status))
-      .AddAction(std::move(timestamp))
-      .AddAction(std::move(list_masters))
-      .Build();
+  ModeBuilder builder("master");
+  builder.Description("Operate on a Kudu Master");
+
+  {
+    unique_ptr<Action> action_reset =
+        ActionBuilder("reset", &ResetAuthzCache)
+        .Description("Reset the privileges cache")
+        .AddRequiredParameter({ kMasterAddressesArg, kMasterAddressesArgDesc })
+        .AddOptionalParameter(
+            "force",
+            boost::none,
+            string("Ignore mismatches of the specified and the actual lists "
+                   "of master addresses in the cluster"))
+        .Build();
+
+    unique_ptr<Mode> mode_authz_cache = ModeBuilder("authz_cache")
+        .Description("Operate on the authz cache of a Kudu Master")
+        .AddAction(std::move(action_reset))
+        .Build();
+    builder.AddMode(std::move(mode_authz_cache));
+  }
+  {
+    unique_ptr<Action> dump_memtrackers =
+        ActionBuilder("dump_memtrackers", &MasterDumpMemTrackers)
+        .Description("Dump the memtrackers from a Kudu Master")
+        .AddRequiredParameter({ kMasterAddressArg, kMasterAddressDesc })
+        .AddOptionalParameter("format")
+        .AddOptionalParameter("memtracker_output")
+        .AddOptionalParameter("timeout_ms")
+        .Build();
+    builder.AddAction(std::move(dump_memtrackers));
+  }
+  {
+    unique_ptr<Action> get_flags =
+        ActionBuilder("get_flags", &MasterGetFlags)
+        .Description("Get the gflags for a Kudu Master")
+        .AddRequiredParameter({ kMasterAddressArg, kMasterAddressDesc })
+        .AddOptionalParameter("all_flags")
+        .AddOptionalParameter("flag_tags")
+        .Build();
+    builder.AddAction(std::move(get_flags));
+  }
+  {
+    unique_ptr<Action> set_flag =
+        ActionBuilder("set_flag", &MasterSetFlag)
+        .Description("Change a gflag value on a Kudu Master")
+        .AddRequiredParameter({ kMasterAddressArg, kMasterAddressDesc })
+        .AddRequiredParameter({ kFlagArg, "Name of the gflag" })
+        .AddRequiredParameter({ kValueArg, "New value for the gflag" })
+        .AddOptionalParameter("force")
+        .Build();
+    builder.AddAction(std::move(set_flag));
+  }
+  {
+    unique_ptr<Action> status =
+        ActionBuilder("status", &MasterStatus)
+        .Description("Get the status of a Kudu Master")
+        .AddRequiredParameter({ kMasterAddressArg, kMasterAddressDesc })
+        .Build();
+    builder.AddAction(std::move(status));
+  }
+  {
+    unique_ptr<Action> timestamp =
+        ActionBuilder("timestamp", &MasterTimestamp)
+        .Description("Get the current timestamp of a Kudu Master")
+        .AddRequiredParameter({ kMasterAddressArg, kMasterAddressDesc })
+        .Build();
+    builder.AddAction(std::move(timestamp));
+  }
+  {
+    unique_ptr<Action> list_masters =
+        ActionBuilder("list", &ListMasters)
+        .Description("List masters in a Kudu cluster")
+        .AddRequiredParameter({ kMasterAddressesArg, kMasterAddressesArgDesc })
+        .AddOptionalParameter(
+            "columns",
+            string("uuid,rpc-addresses"),
+            string("Comma-separated list of master info fields to "
+                   "include in output.\nPossible values: uuid, "
+                   "rpc-addresses, http-addresses, version, seqno "
+                   "and start_time"))
+        .AddOptionalParameter("format")
+        .AddOptionalParameter("timeout_ms")
+        .Build();
+    builder.AddAction(std::move(list_masters));
+  }
+
+  return builder.Build();
 }
 
 } // namespace tools


Mime
View raw message