From 535c26406c9e13edc273bf4ecb58670cad6a8890 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Tue, 20 Aug 2024 15:25:20 -0400 Subject: [PATCH 1/3] enable_feature_flag CLI: Fix typo in usage message (cherry picked from commit f0c0cf8052a3d458e1acd9278627194a1c17e8b8) --- .../rabbitmq/cli/ctl/commands/enable_feature_flag_command.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deps/rabbitmq_cli/lib/rabbitmq/cli/ctl/commands/enable_feature_flag_command.ex b/deps/rabbitmq_cli/lib/rabbitmq/cli/ctl/commands/enable_feature_flag_command.ex index 76bbfd466b39..bc56cd1c6655 100644 --- a/deps/rabbitmq_cli/lib/rabbitmq/cli/ctl/commands/enable_feature_flag_command.ex +++ b/deps/rabbitmq_cli/lib/rabbitmq/cli/ctl/commands/enable_feature_flag_command.ex @@ -25,7 +25,7 @@ defmodule RabbitMQ.CLI.Ctl.Commands.EnableFeatureFlagCommand do def run(["all"], %{node: node_name, experimental: experimental}) do case experimental do true -> - {:error, RabbitMQ.CLI.Core.ExitCodes.exit_usage(), "`--experiemntal` flag is not allowed when enabling all feature flags.\nUse --experimental with a specific feature flag if you want to enable an experimental feature."} + {: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 From a5216db06fdc4bce5e3894a0b8de21d9e4c4fa7d Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Tue, 20 Aug 2024 16:03:29 -0400 Subject: [PATCH 2/3] Non-zero exit code for failing to enable an experimental feature flag With the prior behavior it can be unclear whether the text was a warning and the feature flag was enabled anyways. We can use a non-zero exit code and the `{:error, code, text}` return value to make it clear that the flag wasn't enabled. (cherry picked from commit dc611dd45c2612d3fe33e52ad21761d15efae1da) --- .../rabbitmq/cli/ctl/commands/enable_feature_flag_command.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deps/rabbitmq_cli/lib/rabbitmq/cli/ctl/commands/enable_feature_flag_command.ex b/deps/rabbitmq_cli/lib/rabbitmq/cli/ctl/commands/enable_feature_flag_command.ex index bc56cd1c6655..b94074056070 100644 --- a/deps/rabbitmq_cli/lib/rabbitmq/cli/ctl/commands/enable_feature_flag_command.ex +++ b/deps/rabbitmq_cli/lib/rabbitmq/cli/ctl/commands/enable_feature_flag_command.ex @@ -40,7 +40,7 @@ defmodule RabbitMQ.CLI.Ctl.Commands.EnableFeatureFlagCommand do ])} do {_, {:badrpc, _} = err} -> err {false, :experimental} -> - IO.puts("Feature flag #{feature_flag} is experimental. If you understand the risk, use --experimental to enable it.") + {: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) From 9bea607752384bd43deae874a1bb140859c50402 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Tue, 20 Aug 2024 16:06:13 -0400 Subject: [PATCH 3/3] Add test cases for the '--experimental' flag (cherry picked from commit dce8135d303ce034e47a2c2bc491d47680370020) --- .../test/ctl/enable_feature_flag_test.exs | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/deps/rabbitmq_cli/test/ctl/enable_feature_flag_test.exs b/deps/rabbitmq_cli/test/ctl/enable_feature_flag_test.exs index ad89e42024dc..92264641344d 100644 --- a/deps/rabbitmq_cli/test/ctl/enable_feature_flag_test.exs +++ b/deps/rabbitmq_cli/test/ctl/enable_feature_flag_test.exs @@ -10,6 +10,8 @@ defmodule EnableFeatureFlagCommandTest do @command RabbitMQ.CLI.Ctl.Commands.EnableFeatureFlagCommand @feature_flag :ff_from_enable_ff_testsuite + @experimental_flag :ff_from_enable_ff_testsuite_experimental + @usage_exit_code RabbitMQ.CLI.Core.ExitCodes.exit_usage() setup_all do RabbitMQ.CLI.Core.Distribution.start() @@ -22,6 +24,11 @@ defmodule EnableFeatureFlagCommandTest do desc: ~c"My feature flag", provided_by: :EnableFeatureFlagCommandTest, stability: :stable + }, + @experimental_flag => %{ + desc: ~c"An **experimental** feature!", + provided_by: :EnableFeatureFlagCommandTest, + stability: :experimental } } @@ -35,7 +42,9 @@ defmodule EnableFeatureFlagCommandTest do { :ok, - opts: %{node: get_rabbit_hostname(), experimental: false}, feature_flag: @feature_flag + opts: %{node: get_rabbit_hostname(), experimental: false}, + feature_flag: @feature_flag, + experimental_flag: @experimental_flag } end @@ -63,6 +72,16 @@ defmodule EnableFeatureFlagCommandTest do assert match?({:badrpc, _}, @command.run(["na"], opts)) end + test "run: enabling an experimental flag requires '--experimental'", 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], :experimental, true) + assert @command.run([experimental_flag], opts) == :ok + end + test "run: enabling the same feature flag twice is idempotent", context do enable_feature_flag(context[:feature_flag]) assert @command.run([Atom.to_string(context[:feature_flag])], context[:opts]) == :ok @@ -75,6 +94,12 @@ defmodule EnableFeatureFlagCommandTest do assert list_feature_flags(:enabled) |> Map.has_key?(context[:feature_flag]) 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) + assert match?({:error, @usage_exit_code, _}, @command.run(["all"], opts)) + end + test "banner", context do assert @command.banner([context[:feature_flag]], context[:opts]) =~ ~r/Enabling feature flag \"#{context[:feature_flag]}\" \.\.\./