cli enable_feature_flag: alias --experimental as --opt-in

--experimental is no longer particularly fair to Khepri,
which is not enabled by default because of its enormous
scope, and because once enabled, it cannot be disabled.

--opt-in would be a better name but --experimental
remains for backwards compatiblity.

When both are specified, we consider that the
user opts in if at least one of the flags is
set to true.

(cherry picked from commit 47210c8)

Author: michaelklishin

deps/rabbitmq_cli/lib/rabbitmq/cli/ctl/commands/enable_feature_flag_command.ex

@@ -7,58 +7,65 @@
 defmodule RabbitMQ.CLI.Ctl.Commands.EnableFeatureFlagCommand do
   @behaviour RabbitMQ.CLI.CommandBehaviour
 
-  def switches(), do: [experimental: :boolean]
-  def aliases(), do: [e: :experimental]
+  def switches(), do: [experimental: :boolean, opt_in: :boolean]
+  def aliases(), do: [e: :experimental, o: :opt_in]
 
-  def merge_defaults(args, opts), do: { args, Map.merge(%{experimental: false}, opts) }
+  def merge_defaults(args, opts), do: { args, Map.merge(%{experimental: false, opt_in: false}, opts) }
 
   def validate([], _opts), do: {:validation_failure, :not_enough_args}
   def validate([_ | _] = args, _opts) when length(args) > 1, do: {:validation_failure, :too_many_args}
 
   def validate([""], _opts),
-    do: {:validation_failure, {:bad_argument, "feature_flag cannot be an empty string."}}
+    do: {:validation_failure, {:bad_argument, "feature flag (or group) name cannot be an empty string"}}
 
   def validate([_], _opts), do: :ok
 
   use RabbitMQ.CLI.Core.RequiresRabbitAppRunning
 
-  def run(["all"], %{node: node_name, experimental: experimental}) do
-    case experimental do
-      true ->
-        {:error, RabbitMQ.CLI.Core.ExitCodes.exit_usage(), "`--experimental` flag is not allowed when enabling all feature flags.\nUse --experimental with a specific feature flag if you want to enable an experimental feature."}
-      false ->
-        case :rabbit_misc.rpc_call(node_name, :rabbit_feature_flags, :enable_all, []) do
-          {:badrpc, _} = err -> err
-          other -> other
-        end
-    end
+  def run(["all"], %{node: node_name, opt_in: opt_in, experimental: experimental}) do
+    has_opted_in = (opt_in || experimental)
+    enable_all(node_name, has_opted_in)
   end
 
-  def run([feature_flag], %{node: node_name, experimental: experimental}) do
-    case {experimental, :rabbit_misc.rpc_call(node_name, :rabbit_feature_flags, :get_stability, [
-      String.to_atom(feature_flag)
-    ])} do
-          {_, {:badrpc, _} = err} -> err
-          {false, :experimental} ->
-              {:error, RabbitMQ.CLI.Core.ExitCodes.exit_usage(), "Feature flag #{feature_flag} is experimental. If you understand the risk, use --experimental to enable it."}
-          _ ->
-                case :rabbit_misc.rpc_call(node_name, :rabbit_feature_flags, :enable, [
-                  String.to_atom(feature_flag)
-                ]) do
-                  {:badrpc, _} = err -> err
-                  other -> other
-                end
-    end
+  def run(["all"], %{node: node_name, opt_in: has_opted_in}) do
+    enable_all(node_name, has_opted_in)
   end
 
+  def run(["all"], %{node: node_name, experimental: has_opted_in}) do
+    enable_all(node_name, has_opted_in)
+  end
+
+  def run(["all"], %{node: node_name}) do
+    enable_all(node_name, false)
+  end
+
+
+  def run([feature_flag], %{node: node_name, opt_in: opt_in, experimental: experimental}) do
+    has_opted_in = (opt_in || experimental)
+    enable_one(node_name, feature_flag, has_opted_in)
+  end
+
+  def run([feature_flag], %{node: node_name, opt_in: has_opted_in}) do
+    enable_one(node_name, feature_flag, has_opted_in)
+  end
+
+  def run([feature_flag], %{node: node_name, experimental: has_opted_in}) do
+    enable_one(node_name, feature_flag, has_opted_in)
+  end
+
+  def run([feature_flag], %{node: node_name}) do
+    enable_one(node_name, feature_flag, false)
+  end
+
+
   def output({:error, :unsupported}, %{node: node_name}) do
     {:error, RabbitMQ.CLI.Core.ExitCodes.exit_usage(),
       "This feature flag is not supported by node #{node_name}"}
   end
 
   use RabbitMQ.CLI.DefaultOutput
 
-  def usage, do: "enable_feature_flag [--experimental] <all | feature_flag>"
+  def usage, do: "enable_feature_flag [--opt-in] <all | feature_flag>"
 
   def usage_additional() do
     [
@@ -67,8 +74,8 @@ defmodule RabbitMQ.CLI.Ctl.Commands.EnableFeatureFlagCommand do
         "name of the feature flag to enable, or \"all\" to enable all supported flags"
       ],
       [
-        "--experimental",
-        "required to enable experimental feature flags (make sure you understand the risks!)"
+        "--opt-in",
+        "required to enable certain feature flags (those with vast scope or maturing)"
       ]
     ]
   end
@@ -81,4 +88,39 @@ defmodule RabbitMQ.CLI.Ctl.Commands.EnableFeatureFlagCommand do
   def banner(["all"], _), do: "Enabling all feature flags ..."
 
   def banner([feature_flag], _), do: "Enabling feature flag \"#{feature_flag}\" ..."
+
+  #
+  # Implementation
+  #
+
+  defp enable_all(node_name, has_opted_in) do
+    case has_opted_in do
+      true ->
+        msg = "`--opt-in` (aliased as `--experimental`) flag is not allowed when enabling all feature flags.\nUse --opt-in with a specific feature flag name if to enable an opt-in flag"
+        {:error, RabbitMQ.CLI.Core.ExitCodes.exit_usage(), msg}
+      _ ->
+        case :rabbit_misc.rpc_call(node_name, :rabbit_feature_flags, :enable_all, []) do
+          {:badrpc, _} = err -> err
+          other -> other
+        end
+    end
+  end
+
+  defp enable_one(node_name, feature_flag, has_opted_in) do
+    case {has_opted_in, :rabbit_misc.rpc_call(node_name, :rabbit_feature_flags, :get_stability, [
+      String.to_atom(feature_flag)
+    ])} do
+        {_, {:badrpc, _} = err} -> err
+        {false, :experimental} ->
+          msg = "Feature flag #{feature_flag} requires the user to explicitly opt-in.\nUse --opt-in with a specific feature flag name if to enable an opt-in flag"
+          {:error, RabbitMQ.CLI.Core.ExitCodes.exit_usage(), msg}
+        _ ->
+          case :rabbit_misc.rpc_call(node_name, :rabbit_feature_flags, :enable, [
+            String.to_atom(feature_flag)
+          ]) do
+            {:badrpc, _} = err -> err
+            other -> other
+          end
+    end
+  end
 end

deps/rabbitmq_cli/test/ctl/enable_feature_flag_test.exs

@@ -67,12 +67,27 @@ defmodule EnableFeatureFlagCommandTest do
     assert list_feature_flags(:enabled) |> Map.has_key?(context[:feature_flag])
   end
 
-  test "run: attempt to use an unreachable node returns a nodedown" do
+  test "run: attempt to use an unreachable node with --opt-in returns a nodedown" do
+    opts = %{node: :jake@thedog, timeout: 200, opt_in: false}
+    assert match?({:badrpc, _}, @command.run(["na"], opts))
+  end
+
+  test "run: attempt to use an unreachable node with --experimental returns a nodedown" do
     opts = %{node: :jake@thedog, timeout: 200, experimental: false}
     assert match?({:badrpc, _}, @command.run(["na"], opts))
   end
 
-  test "run: enabling an experimental flag requires '--experimental'", context do
+  test "run: enabling an experimental flag requires '--opt-in'", context do
+    experimental_flag = Atom.to_string(context[:experimental_flag])
+    assert match?(
+             {:error, @usage_exit_code, _},
+             @command.run([experimental_flag], context[:opts])
+           )
+    opts = Map.put(context[:opts], :opt_in, true)
+    assert @command.run([experimental_flag], opts) == :ok
+  end
+
+  test "run: enabling an experimental flag accepts '--experimental'", context do
     experimental_flag = Atom.to_string(context[:experimental_flag])
     assert match?(
              {:error, @usage_exit_code, _},
@@ -94,6 +109,12 @@ defmodule EnableFeatureFlagCommandTest do
     assert list_feature_flags(:enabled) |> Map.has_key?(context[:feature_flag])
   end
 
+  test "run: enabling all feature flags with '--opt-in' returns an error", context do
+    enable_feature_flag(context[:feature_flag])
+    opts = Map.put(context[:opts], :opt_in, true)
+    assert match?({:error, @usage_exit_code, _}, @command.run(["all"], opts))
+  end
+
   test "run: enabling all feature flags with '--experimental' returns an error", context do
     enable_feature_flag(context[:feature_flag])
     opts = Map.put(context[:opts], :experimental, true)