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 <[email protected]>
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 <[email protected]>
Reviewed-by: Hao Hao <[email protected]>
Tested-by: Alexey Serbin <[email protected]>
---
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
