Merge pull request #12325 from rabbitmq/mk-cli-alias-experimental-flag

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)