rabbit_feature_flags: Accept "+feature1,-feature2" in $RABBITMQ_FEATURE_FLAGS

[Why]
Before this patch, the $RABBITMQ_FEATURE_FLAGS environment variable took
an exhaustive list of feature flags to enable. This list overrode the
default of enabling all stable feature flags.

It made it inconvenient when a user wanted to enable an experimental
feature flag like `khepri_db` while still leaving the default behavior.

[How]
$RABBITMQ_FEATURE_FLAGS now acceps the following syntax:

    RABBITMQ_FEATURE_FLAGS=+feature1,-feature2

This will start RabbitMQ with all stable feature flags, plus `feature1`,
but without `feature2`.

For users setting `forced_feature_flags_on_init` in the config, the
corresponding syntax is:

    {forced_feature_flags_on_init, {rel, [feature1], [feature2]}}

Author: dumbbell

deps/rabbit/src/rabbit_ff_controller.erl

@@ -675,25 +675,14 @@ enable_task(FeatureNames) ->
     end.
 
 enable_default_task() ->
-    FeatureNames = get_forced_feature_flag_names(),
-    case FeatureNames of
+    case get_forced_feature_flag_names() of
         undefined ->
             ?LOG_DEBUG(
               "Feature flags: starting an unclustered node for the first "
               "time: all stable feature flags will be enabled by default",
               #{domain => ?RMQLOG_DOMAIN_FEAT_FLAGS}),
             {ok, Inventory} = collect_inventory_on_nodes([node()]),
-            #{feature_flags := FeatureFlags} = Inventory,
-            StableFeatureNames =
-            maps:fold(
-              fun(FeatureName, FeatureProps, Acc) ->
-                      Stability = rabbit_feature_flags:get_stability(
-                                    FeatureProps),
-                      case Stability of
-                          stable -> [FeatureName | Acc];
-                          _      -> Acc
-                      end
-              end, [], FeatureFlags),
+            StableFeatureNames = get_stable_feature_flags(Inventory),
             enable_many(Inventory, StableFeatureNames);
         [] ->
             ?LOG_DEBUG(
@@ -702,21 +691,67 @@ enable_default_task() ->
               "the $RABBITMQ_FEATURE_FLAGS environment variable",
               #{domain => ?RMQLOG_DOMAIN_FEAT_FLAGS}),
             ok;
-        _ ->
+        FeatureNames when is_list(FeatureNames) ->
             ?LOG_DEBUG(
                "Feature flags: starting an unclustered node for the first "
                "time: only the following feature flags specified in the "
                "$RABBITMQ_FEATURE_FLAGS environment variable will be enabled: "
-               "~tp",
+               "~0tp",
                [FeatureNames],
                #{domain => ?RMQLOG_DOMAIN_FEAT_FLAGS}),
             {ok, Inventory} = collect_inventory_on_nodes([node()]),
-            enable_many(Inventory, FeatureNames)
+            enable_many(Inventory, FeatureNames);
+        {rel, Plus, Minus} ->
+            ?LOG_DEBUG(
+              "Feature flags: starting an unclustered node for the first "
+              "time: all stable feature flags will be enabled, after "
+              "applying changes from $RABBITMQ_FEATURE_FLAGS: adding ~0tp, "
+              "skipping ~0tp",
+              [Plus, Minus],
+              #{domain => ?RMQLOG_DOMAIN_FEAT_FLAGS}),
+            {ok, Inventory} = collect_inventory_on_nodes([node()]),
+            StableFeatureNames = get_stable_feature_flags(Inventory),
+            Unsupported = lists:filter(
+                            fun(FeatureName) ->
+                                    not is_known_and_supported(
+                                          Inventory, FeatureName)
+                            end, Minus),
+            case Unsupported of
+                [] ->
+                    FeatureNames = (StableFeatureNames -- Minus) ++ Plus,
+                    enable_many(Inventory, FeatureNames);
+                _ ->
+                    ?LOG_ERROR(
+                       "Feature flags: unsupported feature flags to skip in "
+                       "$RABBITMQ_FEATURE_FLAGS: ~0tp",
+                       [Unsupported],
+                       #{domain => ?RMQLOG_DOMAIN_FEAT_FLAGS}),
+                    {error, unsupported}
+            end;
+        {error, syntax_error_in_envvar} = Error ->
+            ?LOG_DEBUG(
+               "Feature flags: invalid mix of `feature_flag` and "
+               "`+/-feature_flag` in $RABBITMQ_FEATURE_FLAGS",
+               #{domain => ?RMQLOG_DOMAIN_FEAT_FLAGS}),
+            Error
     end.
 
+get_stable_feature_flags(#{feature_flags := FeatureFlags}) ->
+    maps:fold(
+      fun(FeatureName, FeatureProps, Acc) ->
+              Stability = rabbit_feature_flags:get_stability(FeatureProps),
+              case Stability of
+                  stable -> [FeatureName | Acc];
+                  _      -> Acc
+              end
+      end, [], FeatureFlags).
+
 -spec get_forced_feature_flag_names() -> Ret when
-      Ret :: FeatureNames | undefined,
-      FeatureNames :: [rabbit_feature_flags:feature_name()].
+      Ret :: Abs | Rel | undefined,
+      Abs :: [rabbit_feature_flags:feature_name()],
+      Rel :: {rel,
+              [rabbit_feature_flags:feature_name()],
+              [rabbit_feature_flags:feature_name()]}.
 %% @doc Returns the (possibly empty) list of feature flags the user wants to
 %% enable out-of-the-box when starting a node for the first time.
 %%
@@ -737,59 +772,74 @@ enable_default_task() ->
 %% @private
 
 get_forced_feature_flag_names() ->
-    Ret = case get_forced_feature_flag_names_from_env() of
-              undefined -> get_forced_feature_flag_names_from_config();
-              List      -> List
-          end,
-    case Ret of
+    Value = case get_forced_feature_flag_names_from_env() of
+                undefined -> get_forced_feature_flag_names_from_config();
+                List      -> List
+            end,
+    case Value of
         undefined ->
-            ok;
+            Value;
         [] ->
-            ?LOG_INFO(
-               "Feature flags: automatic enablement of feature flags "
-               "disabled (i.e. none will be enabled automatically)",
-               #{domain => ?RMQLOG_DOMAIN_FEAT_FLAGS});
-        _ ->
-            ?LOG_INFO(
-               "Feature flags: automatic enablement of feature flags "
-               "limited to the following list: ~tp",
-               [Ret],
-               #{domain => ?RMQLOG_DOMAIN_FEAT_FLAGS})
-    end,
-    Ret.
+            Value;
+        [[Op | _] | _] when Op =:= $+ orelse Op =:= $- ->
+            lists:foldr(
+              fun
+                  (_, {error, _} = Error) ->
+                      Error;
+                  ([$+ | NameS], {rel, Plus, Minus}) ->
+                      Name = list_to_atom(NameS),
+                      Plus1 = [Name | Plus],
+                      {rel, Plus1, Minus};
+                  ([$- | NameS], {rel, Plus, Minus}) ->
+                      Name = list_to_atom(NameS),
+                      Minus1 = [Name | Minus],
+                      {rel, Plus, Minus1};
+                  (_, _) ->
+                      {error, syntax_error_in_envvar}
+              end, {rel, [], []}, Value);
+        _ when is_list(Value) ->
+            lists:foldr(
+              fun
+                  (_, {error, _} = Error) ->
+                      Error;
+                  (Name, Abs) when is_atom(Name) ->
+                      [Name | Abs];
+                  ([C | _] = NameS, Abs) when C =/= $+ andalso C =/= $- ->
+                      Name = list_to_atom(NameS),
+                      [Name | Abs];
+                  (_, _) ->
+                      {error, syntax_error_in_envvar}
+              end, [], Value)
+    end.
 
 -spec get_forced_feature_flag_names_from_env() -> Ret when
       Ret :: FeatureNames | undefined,
-      FeatureNames :: [rabbit_feature_flags:feature_name()].
+      FeatureNames :: [rabbit_feature_flags:feature_name() | string()].
 %% @private
 
 get_forced_feature_flag_names_from_env() ->
     case rabbit_prelaunch:get_context() of
         #{forced_feature_flags_on_init := ForcedFFs}
           when is_list(ForcedFFs) ->
             ForcedFFs;
+        #{forced_feature_flags_on_init := {'+', List} = ForcedFFs}
+          when is_list(List) ->
+            ForcedFFs;
         _ ->
             undefined
     end.
 
 -spec get_forced_feature_flag_names_from_config() -> Ret when
       Ret :: FeatureNames | undefined,
-      FeatureNames :: [rabbit_feature_flags:feature_name()].
+      FeatureNames :: [rabbit_feature_flags:feature_name() | string()].
 %% @private
 
 get_forced_feature_flag_names_from_config() ->
     Value = application:get_env(
               rabbit, forced_feature_flags_on_init, undefined),
     case Value of
-        undefined ->
-            Value;
-        _ when is_list(Value) ->
-            case lists:all(fun is_atom/1, Value) of
-                true  -> Value;
-                false -> undefined
-            end;
-        _ ->
-            undefined
+        undefined             -> Value;
+        _ when is_list(Value) -> Value
     end.
 
 -spec sync_cluster_task() -> Ret when
@@ -914,7 +964,7 @@ enable_if_supported(#{states_per_node := _} = Inventory, FeatureName) ->
                #{domain => ?RMQLOG_DOMAIN_FEAT_FLAGS}),
             enable_with_registry_locked(Inventory, FeatureName);
         false ->
-            ?LOG_DEBUG(
+            ?LOG_ERROR(
                "Feature flags: `~ts`: unsupported; aborting",
                [FeatureName],
                #{domain => ?RMQLOG_DOMAIN_FEAT_FLAGS}),

deps/rabbit_common/src/rabbit_env.erl

@@ -999,13 +999,15 @@ forced_feature_flags_on_init(Context) ->
     case Value of
         false ->
             %% get_prefixed_env_var() considers an empty string
-            %% is the same as an undefined environment variable.
-            update_context(Context,
-                           forced_feature_flags_on_init, undefined, default);
+            %% as an undefined environment variable.
+            update_context(
+              Context,
+              forced_feature_flags_on_init, undefined, default);
         _ ->
-            Flags = [list_to_atom(V) || V <- string:lexemes(Value, ",")],
-            update_context(Context,
-                           forced_feature_flags_on_init, Flags, environment)
+            FeatureNames = string:lexemes(Value, ","),
+            update_context(
+              Context,
+              forced_feature_flags_on_init, FeatureNames, environment)
     end.
 
 log_feature_flags_registry(Context) ->