You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by aw...@apache.org on 2020/04/06 06:58:48 UTC
[kudu] branch master updated: ranger: allow overwriting of the
log4j2 properties file
This is an automated email from the ASF dual-hosted git repository.
awong 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 b08fb26 ranger: allow overwriting of the log4j2 properties file
b08fb26 is described below
commit b08fb2615ba58024b60fb177c2cc311ba96f189f
Author: Andrew Wong <aw...@cloudera.com>
AuthorDate: Fri Apr 3 20:27:56 2020 -0700
ranger: allow overwriting of the log4j2 properties file
It seems important for users to have the ability to have subsequent runs
of the master honor any new logging configurations specified via gflag.
As such, it seems important to allow users to recreate the log4j2
properties file used by the Ranger client, even if one exists.
This patch enables this by introducing the --ranger_overwrite_log_config
gflag, which is set to true by default.
Change-Id: I4a06f8a1b3328cfd4029295527b5ba61a03efbfa
Reviewed-on: http://gerrit.cloudera.org:8080/15650
Tested-by: Andrew Wong <aw...@cloudera.com>
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Hao Hao <ha...@cloudera.com>
---
src/kudu/ranger/ranger_client-test.cc | 85 +++++++++++++++++++++++++++++++----
src/kudu/ranger/ranger_client.cc | 78 +++++++++++++++++++++++---------
2 files changed, 132 insertions(+), 31 deletions(-)
diff --git a/src/kudu/ranger/ranger_client-test.cc b/src/kudu/ranger/ranger_client-test.cc
index 73d0128..02fe0cd 100644
--- a/src/kudu/ranger/ranger_client-test.cc
+++ b/src/kudu/ranger/ranger_client-test.cc
@@ -33,6 +33,7 @@
#include "kudu/gutil/map-util.h"
#include "kudu/gutil/ref_counted.h"
#include "kudu/gutil/strings/join.h"
+#include "kudu/gutil/strings/split.h"
#include "kudu/gutil/strings/substitute.h"
#include "kudu/ranger/mini_ranger.h"
#include "kudu/ranger/ranger.pb.h"
@@ -40,6 +41,7 @@
#include "kudu/subprocess/subprocess.pb.h"
#include "kudu/util/env.h"
#include "kudu/util/env_util.h"
+#include "kudu/util/faststring.h"
#include "kudu/util/metrics.h"
#include "kudu/util/path_util.h"
#include "kudu/util/status.h"
@@ -51,9 +53,7 @@ DECLARE_string(ranger_config_path);
DECLARE_string(ranger_log_config_dir);
DECLARE_string(ranger_log_level);
DECLARE_bool(ranger_logtostdout);
-
-namespace kudu {
-namespace ranger {
+DECLARE_bool(ranger_overwrite_log_config);
using boost::hash_combine;
using kudu::env_util::ListFilesInDir;
@@ -65,8 +65,13 @@ using std::move;
using std::string;
using std::unordered_set;
using std::vector;
+using strings::SkipEmpty;
+using strings::Split;
using strings::Substitute;
+namespace kudu {
+namespace ranger {
+
struct AuthorizedAction {
string user_name;
ActionPB action;
@@ -356,6 +361,28 @@ class RangerClientTestBase : public KuduTest {
std::unique_ptr<RangerClient> client_;
};
+namespace {
+
+// Looks in --log_dir and returns the full Kudu Ranger subprocess log filename
+// and the lines contained therein.
+Status GetLinesFromLogFile(Env* env, string* file, vector<string>* lines) {
+ vector<string> matches;
+ RETURN_NOT_OK(env->Glob(JoinPathSegments(FLAGS_log_dir, "kudu-ranger-subprocess*.log"),
+ &matches));
+ if (matches.size() != 1) {
+ return Status::Corruption(Substitute("Expected one file but got $0: $1",
+ matches.size(), JoinStrings(matches, ", ")));
+ }
+ faststring contents;
+ const string& full_log_file = matches[0];
+ RETURN_NOT_OK(ReadFileToString(env, full_log_file, &contents));
+ *file = full_log_file;
+ *lines = Split(contents.ToString(), "\n", SkipEmpty());
+ return Status::OK();
+}
+
+} // anonymous namespace
+
TEST_F(RangerClientTestBase, TestLogging) {
{
// Check that a logging configuration was produced by the Ranger client.
@@ -368,12 +395,52 @@ TEST_F(RangerClientTestBase, TestLogging) {
// should include info about each request.
Status s = client_->AuthorizeAction("user", ActionPB::ALL, "table");
ASSERT_TRUE(s.IsNotAuthorized());
-
- // Check that the Ranger client logs some things.
- vector<string> files;
- ASSERT_OK(ListFilesInDir(env_, FLAGS_log_dir, &files));
- SCOPED_TRACE(JoinStrings(files, "\n"));
- ASSERT_STRINGS_ANY_MATCH(files, "kudu-ranger-subprocess.*log");
+ {
+ // Check that the Ranger client logs some DEBUG messages.
+ vector<string> lines;
+ string log_file;
+ ASSERT_OK(GetLinesFromLogFile(env_, &log_file, &lines));
+ ASSERT_STRINGS_ANY_MATCH(lines, ".*DEBUG.*");
+ // Delete the log file so we can start a fresh log.
+ ASSERT_OK(env_->DeleteFile(log_file));
+ }
+ // Try reconfiguring our logging to spit out info-level logs and start a new
+ // log file by restarting the Ranger client. Since we're set up to not
+ // overwrite the logging config, this won't be effective -- we should still
+ // see DEBUG messages.
+ FLAGS_ranger_log_level = "info";
+ FLAGS_ranger_overwrite_log_config = false;
+ client_.reset(new RangerClient(env_, metric_entity_));
+ ASSERT_OK(client_->Start());
+ s = client_->AuthorizeAction("user", ActionPB::ALL, "table");
+ ASSERT_TRUE(s.IsNotAuthorized());
+ {
+ // Our logs should still contain DEBUG messages since we didn't update the
+ // logging configuration.
+ vector<string> lines;
+ string log_file;
+ ASSERT_OK(GetLinesFromLogFile(env_, &log_file, &lines));
+ ASSERT_STRINGS_ANY_MATCH(lines, ".*DEBUG.*");
+ // Delete the log file so we can start a fresh log.
+ ASSERT_OK(env_->DeleteFile(log_file));
+ }
+ // Now try again but this time overwrite the logging configuration.
+ FLAGS_ranger_overwrite_log_config = true;
+ client_.reset(new RangerClient(env_, metric_entity_));
+ ASSERT_OK(client_->Start());
+ s = client_->AuthorizeAction("user", ActionPB::ALL, "table");
+ ASSERT_TRUE(s.IsNotAuthorized());
+ {
+ // We shouldn't see any DEBUG messages since the client is configured to
+ // use INFO-level logging.
+ vector<string> lines;
+ string unused;
+ ASSERT_OK(GetLinesFromLogFile(env_, &unused, &lines));
+ // We should see no DEBUG messages.
+ for (const auto& l : lines) {
+ ASSERT_STR_NOT_CONTAINS(l, "DEBUG");
+ }
+ }
}
} // namespace ranger
diff --git a/src/kudu/ranger/ranger_client.cc b/src/kudu/ranger/ranger_client.cc
index 07baa40..ff78112 100644
--- a/src/kudu/ranger/ranger_client.cc
+++ b/src/kudu/ranger/ranger_client.cc
@@ -20,6 +20,7 @@
#include <algorithm>
#include <cstdint>
#include <cstdlib>
+#include <memory>
#include <ostream>
#include <string>
#include <utility>
@@ -42,6 +43,7 @@
#include "kudu/util/metrics.h"
#include "kudu/util/net/net_util.h"
#include "kudu/util/path_util.h"
+#include "kudu/util/scoped_cleanup.h"
#include "kudu/util/slice.h"
#include "kudu/util/status.h"
#include "kudu/util/string_case.h"
@@ -83,6 +85,11 @@ DEFINE_string(ranger_log_config_dir, "",
TAG_FLAG(ranger_log_config_dir, advanced);
TAG_FLAG(ranger_log_config_dir, evolving);
+DEFINE_bool(ranger_overwrite_log_config, true,
+ "Whether to overwrite any existing logging configuration file, if found.");
+TAG_FLAG(ranger_overwrite_log_config, advanced);
+TAG_FLAG(ranger_overwrite_log_config, evolving);
+
DEFINE_string(ranger_log_level, "info",
"Log level to use in the Ranger Java subprocess. Supports \"all\", \"trace\", "
"\"debug\", \"info\", \"warn\", \"error\", \"fatal\", and \"off\"");
@@ -167,6 +174,7 @@ using kudu::subprocess::SubprocessMetrics;
using kudu::subprocess::SubprocessServer;
using std::move;
using std::string;
+using std::unique_ptr;
using std::unordered_set;
using std::vector;
using strings::Substitute;
@@ -274,44 +282,70 @@ bool ValidateLog4jLevel(const char* /*flagname*/, const string& value) {
}
DEFINE_validator(ranger_log_level, &ValidateLog4jLevel);
-Status GetOrCreateLog4j2PropertiesFile(Env* env, string* logging_properties_file) {
+Status GetOrCreateLog4j2PropertiesFile(Env* env, string* logging_properties_path) {
const string log_conf_dir = FLAGS_ranger_log_config_dir.empty() ?
FLAGS_log_dir : FLAGS_ranger_log_config_dir;
// It's generally expected that --log_dir has already been created elsewhere.
if (!FLAGS_ranger_log_config_dir.empty() && !env->FileExists(log_conf_dir)) {
RETURN_NOT_OK(env->CreateDir(log_conf_dir));
}
- const string log4j2_properties_file = JoinPathSegments(log_conf_dir,
+ const string log4j2_properties_path = JoinPathSegments(log_conf_dir,
kRangerClientPropertiesFilename);
- string new_or_existing;
- if (env->FileExists(log4j2_properties_file)) {
- new_or_existing = "existing";
+ string file_state;
+ bool should_create_file = true;
+ if (env->FileExists(log4j2_properties_path)) {
+ if (FLAGS_ranger_overwrite_log_config) {
+ file_state = "overwritten";
+ } else {
+ file_state = "existing";
+ should_create_file = false;
+ }
} else {
+ file_state = "new";
+ }
+ if (should_create_file) {
+ // Write our new properties file to a tmp file first so other processes
+ // don't read a partial file (not expected, but just in case).
+ unique_ptr<WritableFile> tmp_file;
+ string tmp_path;
+ RETURN_NOT_OK(env->NewTempWritableFile(WritableFileOptions(),
+ Substitute("$0.XXXXXX", log4j2_properties_path),
+ &tmp_path, &tmp_file));
+ // If anything fails, clean up the tmp file.
+ auto tmp_deleter = MakeScopedCleanup([&] {
+ WARN_NOT_OK(env->DeleteFile(tmp_path),
+ Substitute("Couldn't clean up tmp file $0", tmp_path));
+ });
string exe;
RETURN_NOT_OK(env->GetExecutablePath(&exe));
const string program_name = BaseName(exe);
string hostname;
RETURN_NOT_OK(GetHostname(&hostname));
const string log_filename = Substitute("$0.$1", kRangerClientLogFilename, hostname);
- RETURN_NOT_OK(WriteStringToFileSync(
- env, subprocess::Log4j2Properties(program_name, FLAGS_log_dir, log_filename,
- FLAGS_max_log_size, FLAGS_max_log_files,
- FLAGS_ranger_log_level,
- FLAGS_ranger_logtostdout),
- log4j2_properties_file));
- new_or_existing = "new";
+ RETURN_NOT_OK(tmp_file->Append(
+ subprocess::Log4j2Properties(program_name, FLAGS_log_dir, log_filename,
+ FLAGS_max_log_size, FLAGS_max_log_files,
+ FLAGS_ranger_log_level,
+ FLAGS_ranger_logtostdout)));
+ RETURN_NOT_OK(tmp_file->Sync());
+ RETURN_NOT_OK(tmp_file->Close());
+ // Now atomically swap in our file.
+ RETURN_NOT_OK_PREPEND(env->RenameFile(tmp_path, log4j2_properties_path),
+ Substitute("Failed to rename tmp file $0 to $1", tmp_path, log4j2_properties_path));
+ tmp_deleter.cancel();
}
LOG(INFO) << Substitute("Using $0 properties file: $1",
- new_or_existing, log4j2_properties_file);
- *logging_properties_file = log4j2_properties_file;
+ file_state, log4j2_properties_path);
+ *logging_properties_path = log4j2_properties_path;
return Status::OK();
}
// Builds the arguments to start the Ranger subprocess with the given receiver
-// fifo path. Specifically pass the principal and keytab file that the Ranger
-// subprocess will log in with if Kerberos is enabled. 'args' has the final
-// arguments. Returns 'OK' if arguments successfully created, error otherwise.
-Status BuildArgv(const string& fifo_path, const string& log_properties_file,
+// fifo path and logging properties file. Specifically pass the principal and
+// keytab file that the Ranger subprocess will log in with if Kerberos is
+// enabled. 'args' has the final arguments. Returns 'OK' if arguments
+// successfully created, error otherwise.
+Status BuildArgv(const string& fifo_path, const string& log_properties_path,
vector<string>* argv) {
DCHECK(argv);
DCHECK(!FLAGS_ranger_config_path.empty());
@@ -319,7 +353,7 @@ Status BuildArgv(const string& fifo_path, const string& log_properties_file,
vector<string> ret = {
JavaPath(),
Substitute("-Djava.security.krb5.conf=$0", GetKrb5ConfigFile()),
- Substitute("-Dlog4j2.configurationFile=$0", log_properties_file),
+ Substitute("-Dlog4j2.configurationFile=$0", log_properties_path),
"-cp", JavaClasspath(), kMainClass,
};
// When Kerberos is enabled in Kudu, pass both Kudu principal and keytab file
@@ -362,11 +396,11 @@ RangerClient::RangerClient(Env* env, const scoped_refptr<MetricEntity>& metric_e
Status RangerClient::Start() {
VLOG(1) << "Initializing Ranger subprocess server";
- string log_properties_file;
- RETURN_NOT_OK(GetOrCreateLog4j2PropertiesFile(env_, &log_properties_file));
+ string log_properties_path;
+ RETURN_NOT_OK(GetOrCreateLog4j2PropertiesFile(env_, &log_properties_path));
const string fifo_path = SubprocessServer::FifoPath(RangerFifoBase());
vector<string> argv;
- RETURN_NOT_OK(BuildArgv(fifo_path, log_properties_file, &argv));
+ RETURN_NOT_OK(BuildArgv(fifo_path, log_properties_path, &argv));
subprocess_.reset(new RangerSubprocess(env_, fifo_path, std::move(argv), metric_entity_));
return subprocess_->Start();
}