diff options
author | iilyak <iilyak@users.noreply.github.com> | 2021-03-24 06:46:45 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-03-24 06:46:45 -0700 |
commit | 663b31467befa0c3c1c81a5b7e6328b0de431d0e (patch) | |
tree | ae57c343513f4931c0074adc68fbac16843ecb3a | |
parent | 1b7efadd7317ab66b1f7140c31be81c4e1c77d0b (diff) | |
parent | 19625a537d63b035d1014704ca2f23dda3dd9a9a (diff) | |
download | couchdb-663b31467befa0c3c1c81a5b7e6328b0de431d0e.tar.gz |
Merge pull request #3380 from cloudant/clean-up-logs-3.x.v2
Clean up logs 3.x.v2
-rw-r--r-- | rebar.config.script | 2 | ||||
-rw-r--r-- | src/chttpd/src/chttpd_node.erl | 4 | ||||
-rw-r--r-- | src/couch_log/src/couch_log_config.erl | 11 | ||||
-rw-r--r-- | src/couch_log/src/couch_log_config_dyn.erl | 3 | ||||
-rw-r--r-- | src/couch_log/src/couch_log_formatter.erl | 24 | ||||
-rw-r--r-- | src/couch_log/src/couch_log_sup.erl | 2 | ||||
-rw-r--r-- | src/couch_log/test/eunit/couch_log_config_test.erl | 37 | ||||
-rw-r--r-- | src/couch_log/test/eunit/couch_log_formatter_test.erl | 114 | ||||
-rw-r--r-- | src/couch_replicator/src/couch_replicator_httpc_pool.erl | 31 | ||||
-rw-r--r-- | src/setup/src/setup.erl | 2 | ||||
-rw-r--r-- | src/setup/src/setup_httpd.erl | 15 |
11 files changed, 223 insertions, 22 deletions
diff --git a/rebar.config.script b/rebar.config.script index e37a43257..f12ef3842 100644 --- a/rebar.config.script +++ b/rebar.config.script @@ -144,7 +144,7 @@ SubDirs = [ DepDescs = [ %% Independent Apps -{config, "config", {tag, "2.1.7"}}, +{config, "config", {tag, "2.1.8"}}, {b64url, "b64url", {tag, "1.0.2"}}, {ets_lru, "ets-lru", {tag, "1.1.0"}}, {khash, "khash", {tag, "1.1.0"}}, diff --git a/src/chttpd/src/chttpd_node.erl b/src/chttpd/src/chttpd_node.erl index 734c49bd7..c48dfc09a 100644 --- a/src/chttpd/src/chttpd_node.erl +++ b/src/chttpd/src/chttpd_node.erl @@ -71,7 +71,9 @@ handle_node_req(#httpd{method='PUT', path_parts=[_, Node, <<"_config">>, Section Value = couch_util:trim(chttpd:json_body(Req)), Persist = chttpd:header_value(Req, "X-Couch-Persist") /= "false", OldValue = call_node(Node, config, get, [Section, Key, ""]), - case call_node(Node, config, set, [Section, Key, ?b2l(Value), Persist]) of + IsSensitive = Section == <<"admins">>, + Opts = #{persist => Persist, sensitive => IsSensitive}, + case call_node(Node, config, set, [Section, Key, ?b2l(Value), Opts]) of ok -> send_json(Req, 200, list_to_binary(OldValue)); {error, Reason} -> diff --git a/src/couch_log/src/couch_log_config.erl b/src/couch_log/src/couch_log_config.erl index 766d068a4..ab076cc69 100644 --- a/src/couch_log/src/couch_log_config.erl +++ b/src/couch_log/src/couch_log_config.erl @@ -49,7 +49,8 @@ entries() -> [ {level, "level", "info"}, {level_int, "level", "info"}, - {max_message_size, "max_message_size", "16000"} + {max_message_size, "max_message_size", "16000"}, + {strip_last_msg, "strip_last_msg", "true"} ]. @@ -97,4 +98,10 @@ transform(max_message_size, SizeStr) -> Size -> Size catch _:_ -> 16000 - end.
\ No newline at end of file + end; + +transform(strip_last_msg, "false") -> + false; + +transform(strip_last_msg, _) -> + true. diff --git a/src/couch_log/src/couch_log_config_dyn.erl b/src/couch_log/src/couch_log_config_dyn.erl index f7541f61f..b39dcf2f5 100644 --- a/src/couch_log/src/couch_log_config_dyn.erl +++ b/src/couch_log/src/couch_log_config_dyn.erl @@ -25,4 +25,5 @@ get(level) -> info; get(level_int) -> 2; -get(max_message_size) -> 16000. +get(max_message_size) -> 16000; +get(strip_last_msg) -> true. diff --git a/src/couch_log/src/couch_log_formatter.erl b/src/couch_log/src/couch_log_formatter.erl index 4d81f184f..26997a8a6 100644 --- a/src/couch_log/src/couch_log_formatter.erl +++ b/src/couch_log/src/couch_log_formatter.erl @@ -68,7 +68,13 @@ format(Event) -> do_format({error, _GL, {Pid, "** Generic server " ++ _, Args}}) -> %% gen_server terminate - [Name, LastMsg, State, Reason | Extra] = Args, + [Name, LastMsg0, State, Reason | Extra] = Args, + LastMsg = case couch_log_config:get(strip_last_msg) of + true -> + redacted; + false -> + LastMsg0 + end, MsgFmt = "gen_server ~w terminated with reason: ~s~n" ++ " last msg: ~p~n state: ~p~n extra: ~p", MsgArgs = [Name, format_reason(Reason), LastMsg, State, Extra], @@ -76,7 +82,13 @@ do_format({error, _GL, {Pid, "** Generic server " ++ _, Args}}) -> do_format({error, _GL, {Pid, "** State machine " ++ _, Args}}) -> %% gen_fsm terminate - [Name, LastMsg, StateName, State, Reason | Extra] = Args, + [Name, LastMsg0, StateName, State, Reason | Extra] = Args, + LastMsg = case couch_log_config:get(strip_last_msg) of + true -> + redacted; + false -> + LastMsg0 + end, MsgFmt = "gen_fsm ~w in state ~w terminated with reason: ~s~n" ++ " last msg: ~p~n state: ~p~n extra: ~p", MsgArgs = [Name, StateName, format_reason(Reason), LastMsg, State, Extra], @@ -84,7 +96,13 @@ do_format({error, _GL, {Pid, "** State machine " ++ _, Args}}) -> do_format({error, _GL, {Pid, "** gen_event handler" ++ _, Args}}) -> %% gen_event handler terminate - [ID, Name, LastMsg, State, Reason] = Args, + [ID, Name, LastMsg0, State, Reason] = Args, + LastMsg = case couch_log_config:get(strip_last_msg) of + true -> + redacted; + false -> + LastMsg0 + end, MsgFmt = "gen_event ~w installed in ~w terminated with reason: ~s~n" ++ " last msg: ~p~n state: ~p", MsgArgs = [ID, Name, format_reason(Reason), LastMsg, State], diff --git a/src/couch_log/src/couch_log_sup.erl b/src/couch_log/src/couch_log_sup.erl index 6219a36e9..fc1ac7812 100644 --- a/src/couch_log/src/couch_log_sup.erl +++ b/src/couch_log/src/couch_log_sup.erl @@ -63,6 +63,8 @@ handle_config_change("log", Key, _, _, S) -> couch_log_config:reconfigure(); "max_message_size" -> couch_log_config:reconfigure(); + "strip_last_msg" -> + couch_log_config:reconfigure(); _ -> % Someone may have changed the config for % the writer so we need to re-initialize. diff --git a/src/couch_log/test/eunit/couch_log_config_test.erl b/src/couch_log/test/eunit/couch_log_config_test.erl index c4677f37f..a4c4bcff2 100644 --- a/src/couch_log/test/eunit/couch_log_config_test.erl +++ b/src/couch_log/test/eunit/couch_log_config_test.erl @@ -25,7 +25,9 @@ couch_log_config_test_() -> fun check_level/0, fun check_max_message_size/0, fun check_bad_level/0, - fun check_bad_max_message_size/0 + fun check_bad_max_message_size/0, + fun check_strip_last_msg/0, + fun check_bad_strip_last_msg/0 ] }. @@ -108,3 +110,36 @@ check_bad_max_message_size() -> couch_log_test_util:wait_for_config(), ?assertEqual(16000, couch_log_config:get(max_message_size)) end). + + +check_strip_last_msg() -> + % Default is true + ?assertEqual(true, couch_log_config:get(strip_last_msg)), + + couch_log_test_util:with_config_listener(fun() -> + config:set("log", "strip_last_msg", "false"), + couch_log_test_util:wait_for_config(), + ?assertEqual(false, couch_log_config:get(strip_last_msg)), + + config:delete("log", "strip_last_msg"), + couch_log_test_util:wait_for_config(), + ?assertEqual(true, couch_log_config:get(strip_last_msg)) + end). + +check_bad_strip_last_msg() -> + % Default is true + ?assertEqual(true, couch_log_config:get(strip_last_msg)), + + couch_log_test_util:with_config_listener(fun() -> + config:set("log", "strip_last_msg", "false"), + couch_log_test_util:wait_for_config(), + ?assertEqual(false, couch_log_config:get(strip_last_msg)), + + config:set("log", "strip_last_msg", "this is not a boolean"), + couch_log_test_util:wait_for_config(), + ?assertEqual(true, couch_log_config:get(strip_last_msg)), + + config:delete("log", "strip_last_msg"), + couch_log_test_util:wait_for_config(), + ?assertEqual(true, couch_log_config:get(strip_last_msg)) + end). diff --git a/src/couch_log/test/eunit/couch_log_formatter_test.erl b/src/couch_log/test/eunit/couch_log_formatter_test.erl index 795efcf29..24de346c6 100644 --- a/src/couch_log/test/eunit/couch_log_formatter_test.erl +++ b/src/couch_log/test/eunit/couch_log_formatter_test.erl @@ -81,7 +81,7 @@ gen_server_error_test() -> do_matches(do_format(Event), [ "gen_server a_gen_server terminated", "with reason: some_reason", - "last msg: {foo,bar}", + "last msg: redacted", "state: server_state", "extra: \\[\\]" ]). @@ -108,7 +108,7 @@ gen_server_error_with_extra_args_test() -> do_matches(do_format(Event), [ "gen_server a_gen_server terminated", "with reason: some_reason", - "last msg: {foo,bar}", + "last msg: redacted", "state: server_state", "extra: \\[sad,args\\]" ]). @@ -135,7 +135,7 @@ gen_fsm_error_test() -> do_matches(do_format(Event), [ "gen_fsm a_gen_fsm in state state_name", "with reason: barf", - "last msg: {ohai,there}", + "last msg: redacted", "state: curr_state", "extra: \\[\\]" ]). @@ -162,7 +162,7 @@ gen_fsm_error_with_extra_args_test() -> do_matches(do_format(Event), [ "gen_fsm a_gen_fsm in state state_name", "with reason: barf", - "last msg: {ohai,there}", + "last msg: redacted", "state: curr_state", "extra: \\[sad,args\\]" ]). @@ -195,7 +195,7 @@ gen_event_error_test() -> do_matches(do_format(Event), [ "gen_event handler_id installed in a_gen_event", "reason: barf", - "last msg: {ohai,there}", + "last msg: redacted", "state: curr_state" ]). @@ -850,6 +850,110 @@ coverage_test() -> }) ). +gen_server_error_with_last_msg_test() -> + Pid = self(), + Event = { + error, + erlang:group_leader(), + { + Pid, + "** Generic server and some stuff", + [a_gen_server, {foo, bar}, server_state, some_reason] + } + }, + ?assertMatch( + #log_entry{ + level = error, + pid = Pid + }, + do_format(Event) + ), + with_last(fun() -> + do_matches(do_format(Event), [ + "gen_server a_gen_server terminated", + "with reason: some_reason", + "last msg: {foo,bar}", + "state: server_state", + "extra: \\[\\]" + ]) + end). + +gen_event_error_with_last_msg_test() -> + Pid = self(), + Event = { + error, + erlang:group_leader(), + { + Pid, + "** gen_event handler did a thing", + [ + handler_id, + a_gen_event, + {ohai,there}, + curr_state, + barf + ] + } + }, + ?assertMatch( + #log_entry{ + level = error, + pid = Pid + }, + do_format(Event) + ), + with_last(fun() -> + do_matches(do_format(Event), [ + "gen_event handler_id installed in a_gen_event", + "reason: barf", + "last msg: {ohai,there}", + "state: curr_state" + ]) + end). + + +gen_fsm_error_with_last_msg_test() -> + Pid = self(), + Event = { + error, + erlang:group_leader(), + { + Pid, + "** State machine did a thing", + [a_gen_fsm, {ohai,there}, state_name, curr_state, barf] + } + }, + ?assertMatch( + #log_entry{ + level = error, + pid = Pid + }, + do_format(Event) + ), + with_last(fun() -> + do_matches(do_format(Event), [ + "gen_fsm a_gen_fsm in state state_name", + "with reason: barf", + "last msg: {ohai,there}", + "state: curr_state", + "extra: \\[\\]" + ]) + end). + + +with_last(Fun) -> + meck:new(couch_log_config_dyn, [passthrough]), + try + meck:expect(couch_log_config_dyn, get, fun(Case) -> + case Case of + strip_last_msg -> false; + Case -> meck:passthrough([Case]) + end + end), + Fun() + after + meck:unload(couch_log_config_dyn) + end. do_format(Event) -> E = couch_log_formatter:format(Event), diff --git a/src/couch_replicator/src/couch_replicator_httpc_pool.erl b/src/couch_replicator/src/couch_replicator_httpc_pool.erl index 90234a6a0..dc9a749ac 100644 --- a/src/couch_replicator/src/couch_replicator_httpc_pool.erl +++ b/src/couch_replicator/src/couch_replicator_httpc_pool.erl @@ -20,7 +20,7 @@ % gen_server API -export([init/1, handle_call/3, handle_info/2, handle_cast/2]). --export([code_change/3, terminate/2]). +-export([code_change/3, terminate/2, format_status/2]). -include_lib("couch/include/couch_db.hrl"). @@ -145,6 +145,16 @@ code_change(_OldVsn, #state{}=State, _Extra) -> terminate(_Reason, _State) -> ok. +format_status(_Opt, [_PDict, State]) -> + #state{ + url = Url, + proxy_url = ProxyUrl + } = State, + [{data, [{"State", State#state{ + url = couch_util:url_strip_password(Url), + proxy_url = couch_util:url_strip_password(ProxyUrl) + }}]}]. + monitor_client(Callers, Worker, {ClientPid, _}) -> [{Worker, erlang:monitor(process, ClientPid)} | Callers]. @@ -182,3 +192,22 @@ release_worker_internal(Worker, State) -> false -> State#state{callers = NewCallers0} end. + + +-ifdef(TEST). + +-include_lib("couch/include/couch_eunit.hrl"). + +format_status_test_() -> + ?_test(begin + State = #state{ + url = "https://username1:password1@$ACCOUNT2.cloudant.com/db", + proxy_url = "https://username2:password2@proxy.thing.com:8080/" + }, + [{data, [{"State", ScrubbedN}]}] = format_status(normal, [[], State]), + ?assertEqual("https://username1:*****@$ACCOUNT2.cloudant.com/db", ScrubbedN#state.url), + ?assertEqual("https://username2:*****@proxy.thing.com:8080/", ScrubbedN#state.proxy_url), + ok + end). + +-endif.
\ No newline at end of file diff --git a/src/setup/src/setup.erl b/src/setup/src/setup.erl index e681864c7..5129765da 100644 --- a/src/setup/src/setup.erl +++ b/src/setup/src/setup.erl @@ -165,7 +165,7 @@ enable_cluster_int(Options, false) -> couch_log:debug("Enable Cluster: ~p~n", [Options]). set_admin(Username, Password) -> - config:set("admins", binary_to_list(Username), binary_to_list(Password)). + config:set("admins", binary_to_list(Username), binary_to_list(Password), #{sensitive => true}). setup_node(NewCredentials, NewBindAddress, NodeCount, Port) -> case NewCredentials of diff --git a/src/setup/src/setup_httpd.erl b/src/setup/src/setup_httpd.erl index 949675b6a..44ea5d1a7 100644 --- a/src/setup/src/setup_httpd.erl +++ b/src/setup/src/setup_httpd.erl @@ -19,7 +19,7 @@ handle_setup_req(#httpd{method='POST'}=Req) -> ok = chttpd:verify_is_server_admin(Req), couch_httpd:validate_ctype(Req, "application/json"), Setup = get_body(Req), - couch_log:notice("Setup: ~p~n", [Setup]), + couch_log:notice("Setup: ~p~n", [remove_sensitive(Setup)]), Action = binary_to_list(couch_util:get_value(<<"action">>, Setup, <<"missing">>)), case handle_action(Action, Setup) of ok -> @@ -91,7 +91,7 @@ handle_action("enable_cluster", Setup) -> handle_action("finish_cluster", Setup) -> - couch_log:notice("finish_cluster: ~p~n", [Setup]), + couch_log:notice("finish_cluster: ~p~n", [remove_sensitive(Setup)]), Options = get_options([ {ensure_dbs_exist, <<"ensure_dbs_exist">>} @@ -105,7 +105,7 @@ handle_action("finish_cluster", Setup) -> end; handle_action("enable_single_node", Setup) -> - couch_log:notice("enable_single_node: ~p~n", [Setup]), + couch_log:notice("enable_single_node: ~p~n", [remove_sensitive(Setup)]), Options = get_options([ {ensure_dbs_exist, <<"ensure_dbs_exist">>}, @@ -125,7 +125,7 @@ handle_action("enable_single_node", Setup) -> handle_action("add_node", Setup) -> - couch_log:notice("add_node: ~p~n", [Setup]), + couch_log:notice("add_node: ~p~n", [remove_sensitive(Setup)]), Options = get_options([ {username, <<"username">>}, @@ -147,10 +147,10 @@ handle_action("add_node", Setup) -> end; handle_action("remove_node", Setup) -> - couch_log:notice("remove_node: ~p~n", [Setup]); + couch_log:notice("remove_node: ~p~n", [remove_sensitive(Setup)]); handle_action("receive_cookie", Setup) -> - couch_log:notice("receive_cookie: ~p~n", [Setup]), + couch_log:notice("receive_cookie: ~p~n", [remove_sensitive(Setup)]), Options = get_options([ {cookie, <<"cookie">>} ], Setup), @@ -173,3 +173,6 @@ get_body(Req) -> couch_log:notice("Body Fail: ~p~n", [Else]), couch_httpd:send_error(Req, 400, <<"bad_request">>, <<"Missing JSON body'">>) end. + +remove_sensitive(KVList) -> + lists:keyreplace(<<"password">>, 1, KVList, {<<"password">>, <<"****">>}). |