rabbit_feature_flags: Take callback definition from correct node

[Why]
The feature flag controller that is responsible for enabling a feature
flag may be on a node that doesn't know this feature flag. This is
supported by there is a bug when it queries the callback definition for
that feature flag: it uses its own registry which does not have anything
about this feature flag.

This leads to a crash because the `run_callback/5` funtion tries to use
the `undefined` atom returned by the registry as a map:

    crasher:
      initial call: rabbit_ff_controller:init/1
      pid: <0.374.0>
      registered_name: rabbit_ff_controller
      exception error: bad map: undefined
        in function  rabbit_ff_controller:run_callback/5
        in call from rabbit_ff_controller:do_enable/3 (rabbit_ff_controller.erl, line 1244)
        in call from rabbit_ff_controller:update_feature_state_and_enable/2 (rabbit_ff_controller.erl, line 1180)
        in call from rabbit_ff_controller:enable_with_registry_locked/2 (rabbit_ff_controller.erl, line 1050)
        in call from rabbit_ff_controller:enable_many_locked/2 (rabbit_ff_controller.erl, line 991)
        in call from rabbit_ff_controller:enable_many/2 (rabbit_ff_controller.erl, line 979)
        in call from rabbit_ff_controller:updating_feature_flag_states/3 (rabbit_ff_controller.erl, line 307)
        in call from gen_statem:loop_state_callback/11 (gen_statem.erl, line 3735)

[How]
The callback definition is now queried from the first node in the list
given as argument. For the common use case where all nodes know about a
feature flag, the first node is the local one, so there should be no
latency caused by the RPC.

See #12963.

Author: dumbbell

deps/rabbit/src/rabbit_ff_controller.erl

@@ -1840,8 +1840,13 @@ enable_dependencies1(
                 rabbit_deprecated_features:is_feature_used_callback_ret() |
                 run_callback_error()}.
 
-run_callback(Nodes, FeatureName, Command, Extra, Timeout) ->
-    FeatureProps = rabbit_ff_registry_wrapper:get(FeatureName),
+run_callback([FirstNode | _] = Nodes, FeatureName, Command, Extra, Timeout) ->
+    %% We need to take the callback definition from a node in the `Nodes' list
+    %% because the local node may not know this feature flag and thus does not
+    %% have the callback definition.
+    FeatureProps = erpc:call(
+                     FirstNode,
+                     rabbit_ff_registry_wrapper, get, [FeatureName]),
     Callbacks = maps:get(callbacks, FeatureProps, #{}),
     case Callbacks of
         #{Command := {CallbackMod, CallbackFun}}

deps/rabbit/test/feature_flags_SUITE.erl

@@ -45,7 +45,8 @@
          clustering_ok_with_new_ff_enabled_from_plugin_on_some_nodes/1,
          clustering_ok_with_supported_required_ff/1,
          activating_plugin_with_new_ff_disabled/1,
-         activating_plugin_with_new_ff_enabled/1
+         activating_plugin_with_new_ff_enabled/1,
+         enable_plugin_feature_flag_after_deactivating_plugin/1
         ]).
 
 suite() ->
@@ -95,7 +96,8 @@ groups() ->
      {activating_plugin, [],
       [
        activating_plugin_with_new_ff_disabled,
-       activating_plugin_with_new_ff_enabled
+       activating_plugin_with_new_ff_enabled,
+       enable_plugin_feature_flag_after_deactivating_plugin
       ]}
     ],
 
@@ -1260,6 +1262,36 @@ activating_plugin_with_new_ff_enabled(Config) ->
     end,
     ok.
 
+enable_plugin_feature_flag_after_deactivating_plugin(Config) ->
+    FFSubsysOk = is_feature_flag_subsystem_available(Config),
+
+    log_feature_flags_of_all_nodes(Config),
+    case FFSubsysOk of
+        true ->
+            ?assertEqual([false, false],
+                         is_feature_flag_supported(Config, plugin_ff)),
+            ?assertEqual([false, false],
+                         is_feature_flag_enabled(Config, plugin_ff));
+        false ->
+            ok
+    end,
+
+    rabbit_ct_broker_helpers:enable_plugin(Config, 1, "my_plugin"),
+    rabbit_ct_broker_helpers:disable_plugin(Config, 1, "my_plugin"),
+
+    log_feature_flags_of_all_nodes(Config),
+    case FFSubsysOk of
+        true ->
+            enable_feature_flag_on(Config, 0, plugin_ff),
+            ?assertEqual([true, true],
+                         is_feature_flag_supported(Config, plugin_ff)),
+            ?assertEqual([false, true],
+                         is_feature_flag_enabled(Config, plugin_ff));
+        false ->
+            ok
+    end,
+    ok.
+
 %% -------------------------------------------------------------------
 %% Internal helpers.
 %% -------------------------------------------------------------------