From cbc7a965f7aa66980fd042871aa9a128a65eeafb Mon Sep 17 00:00:00 2001 From: Hari Krishna Sunder Date: Mon, 1 Jul 2024 19:13:16 -0700 Subject: [PATCH] [BACKPORT pg15-cherrypicks][#22950] DocDB: Add ValidateFlagValue RPC Summary: Original commit: 507d287 / D36157 Added `ValidateFlagValue` RPC that will check if the given flag value is valid for use, without setting it. Make sure we do not log flag values that are sensitive, or send them back in Status messages. Added `yb-ts-cli validate_flag_value ` command to validate the flag value using the cli tool. Also fixed the `vmodule` flag validator. **Upgrade/Rollback safety:** Proto changes are new RPC which is only used by yb-ts-cli which is always run as the same yb server version, and YBA which will check db version numbers. Fixes #22950 Jira: DB-11865 Test Plan: FlagsTest.ValidateFlagValue ``` $ ./build/latest/bin/yb-ts-cli -server_address 127.0.0.1:9100 validate_flag_value v 3 Flag value is valid ``` ``` $ ./build/latest/bin/yb-ts-cli -server_address 127.0.0.1:9100 validate_flag_value na_flag na_value Invalid flag value: Remote error (yb/rpc/outbound_call.cc:561): Invalid argument (yb/util/flags/flags.cc:746): Unknown command line flag 'na_flag' : Bad value (rpc error 1) ``` ``` $ ./build/latest/bin/yb-ts-cli -server_address 127.0.0.1:9100 validate_flag_value v hari Invalid flag value: Remote error (yb/rpc/outbound_call.cc:561): Invalid argument (yb/util/flags/flags.cc:746): ERROR: illegal value 'hari' specified for int32 flag 'v' : Bad value (rpc error 1) ``` ``` $ ./build/latest/bin/yb-ts-cli -server_address 127.0.0.1:9100 validate_flag_value rpc_throttle_threshold_bytes 1 Invalid flag value: Remote error (yb/rpc/outbound_call.cc:561): Invalid argument (yb/util/flags/flags.cc:746): ERROR: failed validation of new value '1' for flag 'rpc_throttle_threshold_bytes' : common_flags.cc:155] Invalid value '1' for flag 'rpc_throttle_threshold_bytes': Must be at least 16 (rpc error 1) ``` ``` $ ./build/latest/bin/yb-ts-cli -server_address 127.0.0.1:9100 validate_flag_value vmodule asd- Invalid flag value: Remote error (yb/rpc/outbound_call.cc:561): Invalid argument (yb/util/flags/flags.cc:753): ERROR: failed validation of new value 'asd-' for flag 'vmodule' : flags.cc:478] Invalid value 'asd-' for flag 'vmodule': 'asd-' is not valid. vmodule should be a comma list of = (rpc error 1) ``` Reviewers: jason, tfoucher Reviewed By: jason Subscribers: ybase Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D36223 --- src/yb/server/generic_service.cc | 31 ++++----- src/yb/server/generic_service.h | 4 ++ src/yb/server/server_base.proto | 12 ++++ src/yb/tools/ts-cli.cc | 24 +++++++ src/yb/util/flags.h | 38 +++++++++-- src/yb/util/flags/flags-test.cc | 76 +++++++++++++++++++-- src/yb/util/flags/flags.cc | 112 ++++++++++++++++++++----------- 7 files changed, 227 insertions(+), 70 deletions(-) diff --git a/src/yb/server/generic_service.cc b/src/yb/server/generic_service.cc index 8b307ee96356..0d162193a5e0 100644 --- a/src/yb/server/generic_service.cc +++ b/src/yb/server/generic_service.cc @@ -31,32 +31,15 @@ // #include "yb/server/generic_service.h" -#include -#include -#include -#include -#include #include -#include -#include #include #include -#include "yb/util/logging.h" - -#include "yb/gutil/atomicops.h" -#include "yb/gutil/callback_forward.h" -#include "yb/gutil/dynamic_annotations.h" -#include "yb/gutil/macros.h" -#include "yb/gutil/map-util.h" -#include "yb/gutil/port.h" -#include "yb/gutil/strings/split.h" + #include "yb/rpc/rpc_context.h" #include "yb/server/clock.h" #include "yb/server/server_base.h" #include "yb/util/flags.h" -#include "yb/util/metrics_fwd.h" -#include "yb/util/monotime.h" #include "yb/util/status.h" #include "yb/util/status_format.h" #include "yb/util/status_fwd.h" @@ -137,6 +120,18 @@ void GenericServiceImpl::GetFlag(const GetFlagRequestPB* req, rpc.RespondSuccess(); } +void GenericServiceImpl::ValidateFlagValue( + const ValidateFlagValueRequestPB* req, ValidateFlagValueResponsePB* resp, rpc::RpcContext rpc) { + auto status = flags_internal::ValidateFlagValue(req->flag_name(), req->flag_value()); + if (status.ok()) { + rpc.RespondSuccess(); + return; + } + + LOG(WARNING) << "Flag validation failed: " << status; + rpc.RespondFailure(status); +} + void GenericServiceImpl::FlushCoverage(const FlushCoverageRequestPB* req, FlushCoverageResponsePB* resp, rpc::RpcContext rpc) { diff --git a/src/yb/server/generic_service.h b/src/yb/server/generic_service.h index 0638ad005005..823fa28809f2 100644 --- a/src/yb/server/generic_service.h +++ b/src/yb/server/generic_service.h @@ -56,6 +56,10 @@ class GenericServiceImpl : public GenericServiceIf { GetFlagResponsePB* resp, rpc::RpcContext rpc) override; + void ValidateFlagValue( + const ValidateFlagValueRequestPB* req, ValidateFlagValueResponsePB* resp, + rpc::RpcContext rpc) override; + void GetAutoFlagsConfigVersion( const GetAutoFlagsConfigVersionRequestPB* req, GetAutoFlagsConfigVersionResponsePB* resp, diff --git a/src/yb/server/server_base.proto b/src/yb/server/server_base.proto index a4defbccabc2..8ed29f299fa6 100644 --- a/src/yb/server/server_base.proto +++ b/src/yb/server/server_base.proto @@ -155,6 +155,15 @@ message GetAutoFlagsConfigVersionResponsePB { required uint32 config_version = 1; } +message ValidateFlagValueRequestPB { + required string flag_name = 1; + required string flag_value = 2; +} + +message ValidateFlagValueResponsePB { + // RPC will fail on errors. +} + service GenericService { rpc SetFlag(SetFlagRequestPB) returns (SetFlagResponsePB); @@ -162,6 +171,9 @@ service GenericService { rpc GetFlag(GetFlagRequestPB) returns (GetFlagResponsePB); + rpc ValidateFlagValue(ValidateFlagValueRequestPB) + returns (ValidateFlagValueResponsePB); + rpc RefreshFlags(RefreshFlagsRequestPB) returns (RefreshFlagsResponsePB); diff --git a/src/yb/tools/ts-cli.cc b/src/yb/tools/ts-cli.cc index 98a8f4e29c27..e0e893cb12fb 100644 --- a/src/yb/tools/ts-cli.cc +++ b/src/yb/tools/ts-cli.cc @@ -102,6 +102,7 @@ const char* const kVerifyTabletOp = "verify_tablet"; const char* const kAreTabletsRunningOp = "are_tablets_running"; const char* const kIsServerReadyOp = "is_server_ready"; const char* const kSetFlagOp = "set_flag"; +const char* const kValidateFlagValueOp = "validate_flag_value"; const char* const kRefreshFlagsOp = "refresh_flags"; const char* const kDumpTabletOp = "dump_tablet"; const char* const kTabletStateOp = "get_tablet_state"; @@ -200,6 +201,9 @@ class TsAdminClient { Status SetFlag(const string& flag, const string& val, bool force); + // Validates the value of a flag without actually setting it. + Status ValidateFlagValue(const string& flag, const string& val); + // Refreshes all gflags on the remote server to the flagfile, via RPC. Status RefreshFlags(); @@ -383,6 +387,18 @@ Status TsAdminClient::SetFlag(const string& flag, const string& val, } } +Status TsAdminClient::ValidateFlagValue(const string& flag, const string& val) { + server::ValidateFlagValueRequestPB req; + server::ValidateFlagValueResponsePB resp; + RpcController rpc; + + rpc.set_timeout(timeout_); + req.set_flag_name(flag); + req.set_flag_value(val); + + return generic_proxy_->ValidateFlagValue(req, &resp, &rpc); +} + Status TsAdminClient::RefreshFlags() { server::RefreshFlagsRequestPB req; server::RefreshFlagsResponsePB resp; @@ -727,6 +743,7 @@ void SetUsage(const char* argv0) { << " " << kAreTabletsRunningOp << "\n" << " " << kIsServerReadyOp << "\n" << " " << kSetFlagOp << " [-force] \n" + << " " << kValidateFlagValueOp << " \n" << " " << kRefreshFlagsOp << "\n" << " " << kTabletStateOp << " \n" << " " << kDumpTabletOp << " \n" @@ -860,6 +877,13 @@ static int TsCliMain(int argc, char** argv) { RETURN_NOT_OK_PREPEND_FROM_MAIN(client.SetFlag(argv[2], argv[3], FLAGS_force), "Unable to set flag"); + } else if (op == kValidateFlagValueOp) { + CHECK_ARGC_OR_RETURN_WITH_USAGE(op, 4); + + RETURN_NOT_OK_PREPEND_FROM_MAIN( + client.ValidateFlagValue(argv[2], argv[3]), "Invalid flag value"); + std::cout << "Flag value is valid" << std::endl; + } else if (op == kRefreshFlagsOp) { CHECK_ARGC_OR_RETURN_WITH_USAGE(op, 2); diff --git a/src/yb/util/flags.h b/src/yb/util/flags.h index ef89ceeb785d..ec558ddac0fc 100644 --- a/src/yb/util/flags.h +++ b/src/yb/util/flags.h @@ -38,15 +38,29 @@ #include "yb/util/flags/flags_callback.h" #include "yb/util/flags/auto_flags.h" -// Redefine the macro from gflags.h with an unused attribute. -// Note: This macro should be used in the same file that DEFINEs the flag. Using it any other file -// can result in segfault due to indeterminate order of static initialization. +// Macro for the registration of a flag validator. +// The validation function should return true if the flag value is valid, and false otherwise. If +// the function returns false for the new value of the flag, the flag will retain its current value. +// If it returns false for the default value, the process will die. Use LOG_FLAG_VALIDATION_ERROR to +// log error messages when returning false. +// +// Validator fuction should be of the form: +// bool ValidatorFunc(const char* flag_name, value); +// for strings the second argument should be `const std::string&`. +// +// Note: +// If the validation depends on the value of other flags then make sure to call +// DELAY_FLAG_VALIDATION_ON_STARTUP macro, so that we do not enforce a restriction on the order of +// flags in command line and flags file. +// +// This macro should be used in the same file that DEFINEs the flag. Using it any other file can +// result in segfault due to indeterminate order of static initialization. #ifdef DEFINE_validator #undef DEFINE_validator #endif #define DEFINE_validator(name, validator) \ static_assert( \ - sizeof(_DEFINE_FLAG_IN_FILE(name)), "validator must be DEFINED in the same file as the flag"); \ + sizeof(_DEFINE_FLAG_IN_FILE(name)), "validator must be DEFINED in the same file as the flag"); \ static const bool BOOST_PP_CAT(name, _validator_registered) __attribute__((unused)) = \ google::RegisterFlagValidator(&BOOST_PP_CAT(FLAGS_, name), (validator)) @@ -121,6 +135,17 @@ SetFlagResult SetFlag( const std::string& flag_name, const std::string& new_value, const SetFlagForce force, std::string* old_value, std::string* output_msg); +// If the flag is tagged as sensitive_info then returns a masked value('***'). +// Only string values are sensitive. +std::string GetMaskedValueIfSensitive(const std::string& flag_name, const std::string& value); +template +T GetMaskedValueIfSensitive(const std::string& flag_name, T value) { + return value; +} + +// Check if the flag can be set to the new value. Does not actually set the flag. +Status ValidateFlagValue(const std::string& flag_name, const std::string& value); + } // namespace flags_internal // In order to mark a flag as deprecated, use this macro: @@ -166,9 +191,12 @@ class FlagValidatorSink : public google::LogSink { FlagValidatorSink& GetFlagValidatorSink(); +// Log error message to the error log and the flag validator sink, which will ensure it is sent +// back to the user. Also masks any sensitive values. #define LOG_FLAG_VALIDATION_ERROR(flag_name, value) \ LOG_TO_SINK(&yb::GetFlagValidatorSink(), ERROR) \ - << "Invalid value '" << value << "' for flag '" << flag_name << "': " + << "Invalid value '" << yb::flags_internal::GetMaskedValueIfSensitive(flag_name, value) \ + << "' for flag '" << flag_name << "': " // Returns true if the flag was recorded for delayed validation, and the validation can be skipped. bool RecordFlagForDelayedValidation(const std::string& flag_name); diff --git a/src/yb/util/flags/flags-test.cc b/src/yb/util/flags/flags-test.cc index 2ae327fed9da..5c822be791f3 100644 --- a/src/yb/util/flags/flags-test.cc +++ b/src/yb/util/flags/flags-test.cc @@ -14,8 +14,9 @@ #include #include "yb/util/flags.h" -#include "yb/util/status.h" +#include "yb/util/logging_test_util.h" #include "yb/util/status_log.h" +#include "yb/util/status.h" #include "yb/util/test_util.h" using std::string; @@ -35,6 +36,19 @@ DECLARE_string(vmodule); DECLARE_string(allowed_preview_flags_csv); DEFINE_RUNTIME_PREVIEW_bool(preview_flag, false, "preview flag"); +DEFINE_NON_RUNTIME_string(flagstest_secret_flag, "", "This is a secret"); +TAG_FLAG(flagstest_secret_flag, sensitive_info); + +const auto kBadSecretValue = "yb_rox"; +bool ValidateSecretFlag(const char* flag_name, const std::string& new_val) { + if (new_val == kBadSecretValue) { + LOG_FLAG_VALIDATION_ERROR(flag_name, new_val) << "This is no secret. Everyone knows it!"; + return false; + } + return true; +} +DEFINE_validator(flagstest_secret_flag, &ValidateSecretFlag); + namespace yb { class FlagsTest : public YBTest { @@ -97,17 +111,23 @@ TEST_F(FlagsTest, TestVmodule) { // Set to invalid value string old_value, output_msg; - ASSERT_DEATH( + ASSERT_EQ( SetFlag("vmodule", "BadValue", SetFlagForce::kFalse, &old_value, &output_msg), - "'BadValue' is not valid"); + SetFlagResult::BAD_VALUE); + ASSERT_STR_CONTAINS(output_msg, "'BadValue' is not valid"); - ASSERT_DEATH( + ASSERT_EQ( SetFlag("vmodule", "files=", SetFlagForce::kFalse, &old_value, &output_msg), - "'files=' is not valid"); + SetFlagResult::BAD_VALUE); + ASSERT_STR_CONTAINS(output_msg, "'files=' is not valid"); - ASSERT_DEATH( + ASSERT_EQ( SetFlag("vmodule", "biggerThanInt=2147483648", SetFlagForce::kFalse, &old_value, &output_msg), - "'2147483648' is not a valid integer number"); + SetFlagResult::BAD_VALUE); + ASSERT_STR_CONTAINS( + output_msg, + "Invalid logging level '2147483648' for module 'biggerThanInt'. Only integer values between " + "-2147483648 and 2147483647 are allowed"); ASSERT_EQ(FLAGS_vmodule, expected_old); ASSERT_FALSE(VLOG_IS_ON(1)); @@ -218,4 +238,46 @@ TEST_F(FlagsTest, TestPreviewFlagsAllowList) { ASSERT_EQ(FLAGS_allowed_preview_flags_csv, ""); expected_old_allow_list = FLAGS_allowed_preview_flags_csv; } + +TEST_F(FlagsTest, ValidateFlagValue) { + StringVectorSink sink; + ScopedRegisterSink srs(&sink); + + // Test a valid value and make sure the flag is not changed. + ASSERT_OK(flags_internal::ValidateFlagValue("flagstest_testflag", "1")); + ASSERT_EQ(FLAGS_flagstest_testflag, 0); + + // Test an invalid value. + const auto kBadValueMessage = "Must be >= 0"; + ASSERT_NOK_STR_CONTAINS( + flags_internal::ValidateFlagValue("flagstest_testflag", "-1"), kBadValueMessage); + ASSERT_EQ(FLAGS_flagstest_testflag, 0); + + // We should have logged the error to the logs as well. + ASSERT_STR_CONTAINS(AsString(sink.logged_msgs()), kBadValueMessage); + + // Test validating a flag with tag sensitive_info. + const auto kSecretValue = "$secr3t#"; + ASSERT_OK(flags_internal::ValidateFlagValue("flagstest_secret_flag", kSecretValue)); + ASSERT_EQ(FLAGS_flagstest_secret_flag, ""); + + // Test an invalid value and make sure the status does not contain the secret value. + auto status = flags_internal::ValidateFlagValue("flagstest_secret_flag", kBadSecretValue); + ASSERT_NOK(status); + ASSERT_STR_NOT_CONTAINS(status.ToString(), kBadSecretValue); + ASSERT_EQ(FLAGS_flagstest_secret_flag, ""); + + // We should not have logged the secret values to the logs. + ASSERT_STR_NOT_CONTAINS(AsString(sink.logged_msgs()), kSecretValue); + ASSERT_STR_NOT_CONTAINS(AsString(sink.logged_msgs()), kBadSecretValue); + + // Test the vmodule flag. + const auto kVmoduleValue = "file=1"; + ASSERT_OK(SET_FLAG(vmodule, kVmoduleValue)); + ASSERT_EQ(FLAGS_vmodule, kVmoduleValue); + ASSERT_OK(flags_internal::ValidateFlagValue("vmodule", "files=1")); + ASSERT_NOK(flags_internal::ValidateFlagValue("vmodule", "files=")); + ASSERT_EQ(FLAGS_vmodule, kVmoduleValue); +} + } // namespace yb diff --git a/src/yb/util/flags/flags.cc b/src/yb/util/flags/flags.cc index 3226da4a43c1..4573c0564cf7 100644 --- a/src/yb/util/flags/flags.cc +++ b/src/yb/util/flags/flags.cc @@ -340,8 +340,6 @@ static string DescribeOneFlagInXML( return r; } -namespace { - struct sort_flags_by_name { inline bool operator()(const CommandLineFlagInfo& flag1, const CommandLineFlagInfo& flag2) { const auto& a = flag1.name; @@ -364,41 +362,19 @@ bool& CommandLineFlagsParsed() { return parsed; } -// Check if the flag can be set to the new value. Does not actually set the flag. -Status ValidateFlagValue(const std::string& flag_name, const std::string& value) { - auto flag_info = google::GetCommandLineFlagInfoOrDie(flag_name.c_str()); - - // Clear previous errors if any. - GetFlagValidatorSink().GetMessagesAndClear(); - - std::string error_msg; - if (google::ValidateCommandLineOption( - flag_name.c_str(), flag_info.current_value.c_str(), &error_msg)) { - return Status::OK(); - } - - auto validation_msgs = GetFlagValidatorSink().GetMessagesAndClear(); - - return STATUS_FORMAT( - InvalidArgument, "$0 : $1", error_msg, - validation_msgs.empty() ? "Bad value" : JoinStrings(validation_msgs, ";")); -} - Status ValidateFlagsRequiringDelayedValidation() { CommandLineFlagsParsed() = true; for (const auto& flag_name : FlagsWithDelayedValidation()) { auto flag_info = google::GetCommandLineFlagInfoOrDie(flag_name.c_str()); // Flag was already set without any validation. Check if the current value is valid. - RETURN_NOT_OK(ValidateFlagValue(flag_name, flag_info.current_value)); + RETURN_NOT_OK(flags_internal::ValidateFlagValue(flag_name, flag_info.current_value)); } FlagsWithDelayedValidation().clear(); return Status::OK(); } -} // namespace - void DumpFlagsXMLAndExit(OnlyDisplayDefaultFlagValue only_display_default_values) { vector flags; GetAllFlags(&flags); @@ -491,33 +467,35 @@ bool IsPreviewFlagAllowed(const CommandLineFlagInfo& flag_info, const string& ne } // Validates that the requested updates to vmodule can be made. -void ValidateVmodule() { - auto requested_settings = strings::Split(FLAGS_vmodule, ","); +bool ValidateVmodule(const char* flag_name, const std::string& value) { + auto requested_settings = strings::Split(value, ","); for (const auto& module_value : requested_settings) { if (module_value.empty()) { continue; } vector kv = strings::Split(module_value, "="); if (kv.size() != 2 || kv[0].empty() || kv[1].empty()) { - LOG(FATAL) << Format( - "'$0' is not valid. vmodule should be a comma list of =", - module_value); + LOG_FLAG_VALIDATION_ERROR(flag_name, value) + << "'" << module_value + << "' is not valid. vmodule should be a comma list of ="; + return false; } char* end; errno = 0; const int64 value = strtol(kv[1].c_str(), &end, 10); if (*end != '\0' || errno == ERANGE || value > INT_MAX || value < INT_MIN) { - LOG(FATAL) << Format( - "'$0' is not a valid integer number. Cannot update vmodule setting for module '$1'", - kv[1], kv[0]); + LOG_FLAG_VALIDATION_ERROR(flag_name, value) + << "Invalid logging level '" << kv[1] << "' for module '" << kv[0] + << "'. Only integer values between " << INT_MIN << " and " << INT_MAX << " are allowed"; + return false; } } -} -void ValidateAndUpdateVmodule() { - ValidateVmodule(); + return true; +} +void UpdateVmodule() { // glog behavior: The first time VLOG is invoked for a file it tries to find a matching pattern in // vmodule list. If found it links to that pattern for the rest of the program lifetime. If not // found it uses the default logging level from FLAGS_v and gets added to a list of files that @@ -580,9 +558,14 @@ void ValidateAndUpdateVmodule() { void ParseCommandLineFlags(int* argc, char*** argv, bool remove_flags) { static GoogleOnceType once_register_vmodule_callback = GOOGLE_ONCE_INIT; + // We cannot use DEFINE_validator and REGISTER_CALLBACK for vmodule since it is not DEFINED in any + // yb file, and we cannot guarantee the static initialization order. Instead we register them + // before we parse flags by which time static initialization is guaranteed to be complete. GoogleOnceInit(&once_register_vmodule_callback, []() { + CHECK(google::RegisterFlagValidator(&FLAGS_vmodule, &ValidateVmodule)); // NOLINT + flags_callback_internal::RegisterGlobalFlagUpdateCallback( - &FLAGS_vmodule, "ValidateAndUpdateVmodule", &ValidateAndUpdateVmodule); + &FLAGS_vmodule, "ValidateAndUpdateVmodule", &UpdateVmodule); }); { @@ -720,6 +703,55 @@ void WarnFlagDeprecated(const std::string& flagname, const std::string& date_mm_ } } +static const std::string kMaskedFlagValue = "***"; + +bool IsFlagSensitive(const unordered_set& tags) { + return ContainsKey(tags, FlagTag::kSensitive_info); +} + +bool IsFlagSensitive(const std::string& flag_name) { + unordered_set tags; + GetFlagTags(flag_name, &tags); + return IsFlagSensitive(tags); +} + +std::string GetMaskedValueIfSensitive( + const unordered_set& tags, const std::string& value) { + if (IsFlagSensitive(tags)) { + return kMaskedFlagValue; + } + return value; +} + +std::string GetMaskedValueIfSensitive(const std::string& flag_name, const std::string& value) { + unordered_set tags; + GetFlagTags(flag_name, &tags); + return GetMaskedValueIfSensitive(tags, value); +} + +Status ValidateFlagValue(const std::string& flag_name, const std::string& value) { + // Clear previous errors if any. + GetFlagValidatorSink().GetMessagesAndClear(); + + std::string error_msg; + if (google::ValidateCommandLineOption(flag_name.c_str(), value.c_str(), &error_msg)) { + return Status::OK(); + } + + auto validation_msgs = GetFlagValidatorSink().GetMessagesAndClear(); + + // error_msg originates from gflags, which may contain the value. Therefore, if it is sensitive, + // mask it. + // Ex: ERROR: failed validation of new value '1000' for flag 'ysql_conn_mgr_port' + if (!value.empty() && IsFlagSensitive(flag_name)) { + boost::replace_all(error_msg, Format("'$0'", value), Format("'$0'", kMaskedFlagValue)); + } + + return STATUS_FORMAT( + InvalidArgument, "$0 : $1", error_msg, + validation_msgs.empty() ? "Bad value" : JoinStrings(validation_msgs, ";")); +} + SetFlagResult SetFlag( const string& flag_name, const string& new_value, const SetFlagForce force, string* old_value, string* output_msg) { @@ -779,9 +811,9 @@ SetFlagResult SetFlag( // We have already validated the flag_name with GetCommandLineFlagInfo, so this should not fail. CHECK(google::GetCommandLineOption(flag_name.c_str(), &final_value)); - bool is_sensitive = ContainsKey(tags, FlagTag::kSensitive_info); - LOG(INFO) << "Changed flag '" << flag_name << "' from '" << (is_sensitive ? "***" : old_val) - << "' to '" << (is_sensitive ? "***" : final_value) << "'"; + LOG(INFO) << "Changed flag '" << flag_name << "' from '" + << GetMaskedValueIfSensitive(tags, old_val) << "' to '" + << GetMaskedValueIfSensitive(tags, final_value) << "'"; *output_msg = ret; *old_value = old_val;