MB-67857: Basic unit tests for menelaus_cbauth

Peter-Searby · Peter-Searby · commit 4e19e68a6759 · 2025-08-19T07:51:25.000Z
Adds unit tests covering menelaus_cbauth's interactions with the json_rpc_connection workers. Change-Id: I087bb68068ce46f2d7c2ba20817b810f71fe165a Reviewed-on: https://review.couchbase.org/c/ns_server/+/232216 Reviewed-by: Timofey Barmin <timofey.barmin@couchbase.com> Well-Formed: Restriction Checker Well-Formed: Build Bot <build@couchbase.com> Tested-by: Peter Searby <peter.searby@couchbase.com>
diff --git a/src/menelaus_cbauth.erl b/src/menelaus_cbauth.erl
@@ -37,7 +37,12 @@
 -include("ns_common.hrl").
 -include("cut.hrl").
 
+-ifdef(TEST).
+-include_lib("eunit/include/eunit.hrl").
+-endif.
+
 -define(VERSION_1, "v1").
+-define(WORKER_SYNC_TIMEOUT, 5000).
 
 handle_rpc_connect(?VERSION_1, Label, Req) ->
     case ns_config_auth:is_system_provisioned() of
@@ -76,6 +81,9 @@ sync() ->
     sync(node()).
 
 sync(Node) ->
+    sync(Node, ?WORKER_SYNC_TIMEOUT).
+
+sync(Node, WorkerSyncTimeout) ->
     InternalConnections =
         gen_server:call({?MODULE, Node}, get_internal_connections, infinity),
     %% If the above call succeeds, but a worker exits before we make the below
@@ -86,7 +94,7 @@ sync(Node) ->
     %% caller
     misc:parallel_map(
       fun (Pid) ->
-              try menelaus_cbauth_worker:sync(Pid)
+              try menelaus_cbauth_worker:sync(Pid, WorkerSyncTimeout)
               catch exit:{noproc, _} ->
                       ?log_error("Process ~p no longer exists", [Pid])
               end
@@ -639,3 +647,182 @@ service_to_label(xdcr) ->
     "goxdcr-cbauth";
 service_to_label(Service) ->
     atom_to_list(Service) ++ "-cbauth".
+
+-ifdef(TEST).
+-define(SERVICE, "<service>").
+-define(LABEL, "<service>-cbauth").
+-define(HEARTBEAT_TIME_S, 1).
+%% Short timeout for waiting to confirm that something doesn't immediately
+%% happen, without waiting so long that it significantly increases test duration
+-define(SHORT_TIMEOUT, 500).
+%% Timeout for things that should eventually happen, but not necessarily
+%% immediately
+-define(LONG_TIMEOUT, 60_000).
+
+setup_t() ->
+    meck:expect(config_profile, get,
+                fun () ->
+                        ?DEFAULT_EMPTY_PROFILE_FOR_TESTS
+                end),
+
+    fake_ns_config:setup(),
+    fake_chronicle_kv:setup(),
+    %% Test setups return a map of pids for later shutdown in the teardown
+    PidMap = mock_helpers:setup_mocks([json_rpc_events,
+                                       ns_node_disco_events,
+                                       user_storage_events,
+                                       ssl_service_events,
+                                       json_rpc_connection_sup,
+                                       ns_ssl_services_setup,
+                                       menelaus_users,
+                                       ns_secrets,
+                                       testconditions]),
+
+    %% Set config values for a few keys, since these are needed for greater
+    %% coverage, and to avoid errors
+    fake_chronicle_kv:update_snapshot(#{nodes_wanted => [node()],
+                                        bucket_names => []}),
+    fake_ns_config:update_snapshot([{rest, [{port, 8091}]},
+                                    {rest_creds, placeholder},
+                                    {memcached, [{admin_user, "user"},
+                                                 {admin_pass, "pass"}]}]),
+
+    meck:new(menelaus_cbauth_worker, [passthrough]),
+
+    {ok, Pid} = menelaus_cbauth:start_link(),
+    start_fake_json_rpc_connection(?LABEL),
+    PidMap#{?MODULE => Pid}.
+
+teardown_t(PidMap) ->
+    Name = list_to_atom("json_rpc_connection-" ++ ?LABEL),
+    erlang:unregister(Name),
+    mock_helpers:shutdown_processes(PidMap),
+    fake_chronicle_kv:teardown(),
+    fake_ns_config:teardown(),
+    meck:unload().
+
+start_fake_json_rpc_connection(Label) ->
+    Name = list_to_atom("json_rpc_connection-" ++ Label),
+    Pid = self(),
+    true = erlang:register(Name, Pid),
+    meck:expect(json_rpc_connection, perform_call,
+                fun (_Label, _Call, _EJsonArg, _Opts) ->
+                        {ok, true}
+                end),
+    gen_event:notify(json_rpc_events,
+                     {started, Label, [internal,
+                                       {heartbeat, ?HEARTBEAT_TIME_S}],
+                      self()}).
+
+cbauth_init_t() ->
+    %% UpdateDB gets called once almost immediately
+    meck:wait(json_rpc_connection, perform_call,
+              ['_', "AuthCacheSvc.UpdateDB", '_', '_'], ?LONG_TIMEOUT),
+    %% Heartbeat gets called once
+    meck:wait(json_rpc_connection, perform_call,
+              ['_', "AuthCacheSvc.Heartbeat", '_', '_'],
+              2_000 * ?HEARTBEAT_TIME_S),
+    %% Heartbeat gets called again
+    meck:reset(json_rpc_connection),
+    meck:wait(json_rpc_connection, perform_call,
+              ['_', "AuthCacheSvc.Heartbeat", '_', '_'],
+              2_000 * ?HEARTBEAT_TIME_S),
+    %% UpdateDB isn't called immediately again
+    ?assertError(timeout,
+                 meck:wait(json_rpc_connection, perform_call,
+                           ['_', "AuthCacheSvc.UpdateDB", '_', '_'],
+                           ?SHORT_TIMEOUT)).
+
+cbauth_sync_t() ->
+    %% UpdateDB gets called once immediately
+    meck:wait(json_rpc_connection, perform_call,
+              ['_', "AuthCacheSvc.UpdateDB", '_', '_'],
+              ?LONG_TIMEOUT),
+    [ok] = sync(),
+    %% Ensure that the next perform_call doesn't immediately return until after
+    %% the sync call would time out
+    meck:expect(json_rpc_connection, perform_call,
+                fun (_, "AuthCacheSvc.UpdateDB", _, _) ->
+                        timer:sleep(2 * ?SHORT_TIMEOUT);
+                    (_, _, _, _) ->
+                        ok
+                end),
+    meck:reset(menelaus_cbauth_worker),
+    %% Force an update by updating the snapshot
+    fake_ns_config:update_snapshot(rest, [{port, 8092}]),
+    %% Wait for the update to start being handled
+    meck:wait(menelaus_cbauth_worker, notify, ['_', '_'], ?LONG_TIMEOUT),
+    %% Sync with timeout half the perform_call sleep time, to ensure it gets hit
+    ?assertExit({timeout, _}, sync(node(), ?SHORT_TIMEOUT)).
+
+cbauth_stats_t() ->
+    meck:expect(json_rpc_connection, perform_call,
+                fun (_Label, "AuthCacheSvc.GetStats", _EJsonArg, _Opts) ->
+                        {ok, {[ok]}};
+                    (_Label, _Call, _EJsonArg, _Opts) ->
+                        {ok, true}
+                end),
+    [{<<?SERVICE>>, ok}] = stats(),
+    meck:expect(json_rpc_connection, perform_call,
+                fun (_Label, "AuthCacheSvc.GetStats", _EJsonArg, _Opts) ->
+                        {error, error};
+                    (_Label, _Call, _EJsonArg, _Opts) ->
+                        {ok, true}
+                end),
+    [] = stats().
+
+trigger_notification(ns_node_disco_events) ->
+    %% To avoid needing to trick ns_node_disco into recognising fake nodes, just
+    %% make the node list different by removing the only node.
+    %% Note, this tests ns_node_disco_events as well as chronicle_kv, as the
+    %% event handler for chronicle_compat_events in this module does not cover
+    %% the nodes_wanted key (although the ns_node_disco handler does cover this
+    %% key, hence how this tests the ns_node_disco_events handler)
+    fake_chronicle_kv:update_snapshot(nodes_wanted, []);
+trigger_notification(ns_config_events) ->
+    %% This is just an arbitrary key in ns_config that we use for the cbauth
+    %% info, that is covered by the chronicle_compat_events handler
+    fake_ns_config:update_snapshot(rest, [{port, 8092}]);
+trigger_notification(chronicle_kv) ->
+    %% While the bucket_names key isn't subscribed to, the collections key is,
+    %% and we need to update both for the info to be updated
+    fake_chronicle_kv:update_snapshot(
+      #{bucket_names => ["test"],
+        {bucket, "test", collections} => [{uid, 0}],
+        {bucket, "test", props} => [{type, membase}]});
+trigger_notification(user_storage_events) ->
+    meck:expect(menelaus_users, get_users_version, 0, {1, 0}),
+    %% It isn't worth the complexity to trigger a user version change through
+    %% less artificial means, so just manually trigger the event
+    gen_event:notify(user_storage_events, event);
+trigger_notification(ssl_service_events) ->
+    %% It isn't worth the complexity to trigger an ssl_service_event through
+    %% less artificial means, so just manually trigger the event
+    gen_event:notify(ssl_service_events, client_cert_changed).
+
+cbauth_notify_tests() ->
+    %% Check that expected events cause notifications
+    [{"cbauth notify " ++ atom_to_list(EventManager) ++ " test",
+      fun () ->
+              %% UpdateDB gets called once almost immediately
+              meck:wait(json_rpc_connection, perform_call,
+                        ['_', "AuthCacheSvc.UpdateDB", '_', '_'],
+                        ?LONG_TIMEOUT),
+              meck:reset(json_rpc_connection),
+              trigger_notification(EventManager),
+              meck:wait(json_rpc_connection, perform_call,
+                        ['_', "AuthCacheSvc.UpdateDB", '_', '_'],
+                        ?LONG_TIMEOUT)
+      end} || EventManager <- [ns_node_disco_events,
+                               ns_config_events,
+                               chronicle_kv,
+                               user_storage_events,
+                               ssl_service_events]].
+
+cbauth_test_() ->
+    {foreach, fun setup_t/0, fun teardown_t/1,
+     [{"cbauth init test", fun cbauth_init_t/0},
+      {"cbauth sync test", fun cbauth_sync_t/0},
+      {"cbauth stats test", fun cbauth_stats_t/0}
+     | cbauth_notify_tests()]}.
+-endif.
diff --git a/src/menelaus_cbauth_worker.erl b/src/menelaus_cbauth_worker.erl
@@ -12,7 +12,7 @@
 
 -behaviour(gen_server).
 
--export([start_monitor/4, notify/2, collect_stats/1, sync/1,
+-export([start_monitor/4, notify/2, collect_stats/1, sync/2,
          strip_cbauth_suffix/1]).
 
 -export([init/1, handle_call/3, handle_cast/2,
@@ -50,8 +50,8 @@ notify(Pid, Info) ->
 collect_stats(Pid) ->
     gen_server:call(Pid, collect_stats).
 
-sync(Pid) ->
-    gen_server:call(Pid, sync).
+sync(Pid, Timeout) ->
+    gen_server:call(Pid, sync, Timeout).
 
 init([Label, Version, Pid, Params]) ->
     MRef = erlang:monitor(process, Pid),
diff --git a/test/fake_helpers/fake_ns_config.erl b/test/fake_helpers/fake_ns_config.erl
@@ -137,12 +137,18 @@ meck_setup() ->
     meck_setup_getters(),
     meck_setup_setters(),
 
-    %% This function is slightly interesting. It uses some "fold" function in
-    %% ns_config that requires the snapshot to be in a specific format.
+    %% These functions are slightly interesting - they require the snapshot to
+    %% be in a specific format.
     %% The snapshot format is trivial to map to, so rather than re-implement
     %% (simple as the logic is) just map the snapshot to the expected format
     %% and pass it through to the base function, which should stop this from
     %% ever getting out of sync.
+    meck:expect(ns_config, fold,
+                fun(Fun, Acc, ?NS_CONFIG_LATEST_MARKER) ->
+                        meck:passthrough([Fun, Acc, [get_ets_snapshot()]]);
+                   (Fun, Acc, Snapshot) ->
+                        meck:passthrough([Fun, Acc, [Snapshot]])
+                end),
     meck:expect(ns_config, get_node_uuid_map,
                 fun(?NS_CONFIG_LATEST_MARKER) ->
                         meck:passthrough([[get_ets_snapshot()]]);
@@ -197,13 +203,20 @@ meck_setup_getters() ->
                         end
                 end),
 
-
+    meck:expect(ns_config, search_node,
+                fun(Snapshot, Key) ->
+                        fetch_node(node(), Snapshot, Key)
+                end),
     meck:expect(ns_config, search_node,
                 fun(Node, Snapshot, Key) ->
                         fetch_node(Node, Snapshot, Key)
                 end),
 
 
+    meck:expect(ns_config, search_prop,
+                fun (Snapshot, Key, SubKey) ->
+                        fetch_prop(Snapshot, Key, SubKey, undefined)
+                end),
     meck:expect(ns_config, search_prop,
                 fun(Snapshot, Key, SubKey, DefaultSubVal) ->
                         fetch_prop(Snapshot, Key, SubKey, DefaultSubVal)
@@ -230,6 +243,10 @@ meck_setup_getters() ->
     meck:expect(ns_config, search_node_with_default,
                 fun(Key, Default) ->
                         fetch_with_default_from_latest_snapshot(Key, Default)
+                end),
+    meck:expect(ns_config, search_node_with_default,
+                fun(Node, Snapshot, Key, Default) ->
+                        fetch_node_with_default(Node, Snapshot, Key, Default)
                 end).
 
 meck_setup_setters() ->
@@ -301,12 +318,20 @@ fetch_node_prop(Node, Snapshot, Key, SubKey, Default) ->
         false -> Default
     end.
 
+fetch_node(Node, ?NS_CONFIG_LATEST_MARKER, Key) ->
+    fetch_node(Node, get_ets_snapshot(), Key);
 fetch_node(Node, Snapshot, Key) ->
     case fetch_from_snapshot(Snapshot, {node, Node, Key}) of
         {value, _} = V -> V;
         false -> fetch_from_snapshot(Snapshot, Key)
     end.
 
+fetch_node_with_default(Node, Snapshot, Key, Default) ->
+    case fetch_from_snapshot(Snapshot, {node, Node, Key}) of
+        {value, V} -> V;
+        false -> fetch_with_default(Snapshot, Key, Default)
+    end.
+
 fetch_prop(?NS_CONFIG_LATEST_MARKER, Key, SubKey, DefaultSubVal) ->
     fetch_prop(get_ets_snapshot(), Key, SubKey, DefaultSubVal);
 fetch_prop(Snapshot, Key, SubKey, DefaultSubVal) ->
diff --git a/test/mock_helpers.erl b/test/mock_helpers.erl
@@ -150,10 +150,28 @@ compat_mode_events(PidMap) ->
                                                       ?FUNCTION_NAME}),
     PidMap#{?FUNCTION_NAME => CompatModeEventsPid}.
 
+ns_node_disco(PidMap) ->
+    setup_mocks([dist_manager], PidMap),
+    {ok, NsNodeDiscoPid} = ns_node_disco:start_link(),
+    PidMap#{?FUNCTION_NAME => NsNodeDiscoPid}.
+
 ns_node_disco_events(PidMap) ->
+    PidMap0 = setup_mocks([ns_node_disco], PidMap),
     {ok, NSNodeDiscoEventsPid} = gen_event:start_link({local,
                                                        ?FUNCTION_NAME}),
-    PidMap#{?FUNCTION_NAME => NSNodeDiscoEventsPid}.
+    PidMap0#{?FUNCTION_NAME => NSNodeDiscoEventsPid}.
+
+json_rpc_events(PidMap) ->
+    {ok, JsonRpcEventsPid} = gen_event:start_link({local, ?FUNCTION_NAME}),
+    PidMap#{?FUNCTION_NAME => JsonRpcEventsPid}.
+
+user_storage_events(PidMap) ->
+    {ok, UserStorageEventsPid} = gen_event:start_link({local, ?FUNCTION_NAME}),
+    PidMap#{?FUNCTION_NAME => UserStorageEventsPid}.
+
+ssl_service_events(PidMap) ->
+    {ok, SslServiceEventsPid} = gen_event:start_link({local, ?FUNCTION_NAME}),
+    PidMap#{?FUNCTION_NAME => SslServiceEventsPid}.
 
 %%%===================================================================
 %%% Mock setup functions
@@ -249,5 +267,31 @@ rebalance_quirks(PidMap) ->
 
 testconditions(PidMap) ->
     meck:new(?FUNCTION_NAME, [passthrough]),
-    meck:expect(?FUNCTION_NAME, get, fun(_) -> ok end),
+    meck:expect(?FUNCTION_NAME, get, fun(_) -> false end),
+    PidMap#{?FUNCTION_NAME => mocked}.
+
+json_rpc_connection_sup(PidMap) ->
+    meck:expect(?FUNCTION_NAME, reannounce, fun () -> ok end),
+    PidMap#{?FUNCTION_NAME => mocked}.
+
+ns_ssl_services_setup(PidMap) ->
+    meck:expect(?FUNCTION_NAME, client_cert_auth, fun () -> [] end),
+    PidMap#{?FUNCTION_NAME => mocked}.
+
+menelaus_users(PidMap) ->
+    meck:expect(?FUNCTION_NAME, get_auth_version,
+                fun () -> {0, 0} end),
+    meck:expect(?FUNCTION_NAME, get_users_version,
+                fun () -> {0, 0} end),
+    meck:expect(?FUNCTION_NAME, get_groups_version,
+                fun () -> {0, 0} end),
+    PidMap#{?FUNCTION_NAME => mocked}.
+
+ns_secrets(PidMap) ->
+    meck:expect(?FUNCTION_NAME, get_pkey_pass,
+                fun (_) -> ?HIDE(undefined) end),
+    PidMap#{?FUNCTION_NAME => mocked}.
+
+dist_manager(PidMap) ->
+    meck:expect(?FUNCTION_NAME, get_rename_txn_pid, fun () -> undefined end),
     PidMap#{?FUNCTION_NAME => mocked}.
