From d9819bc5341330369fd16523f741e9fc5919a1ea Mon Sep 17 00:00:00 2001 From: Michal Kuratczyk Date: Wed, 14 Aug 2024 10:16:17 +0200 Subject: [PATCH 1/5] Prevent accidentally enabling experimental FFs --- .../priv/www/js/tmpl/feature-flags.ejs | 76 ++++++++++++++++++- 1 file changed, 75 insertions(+), 1 deletion(-) diff --git a/deps/rabbitmq_management/priv/www/js/tmpl/feature-flags.ejs b/deps/rabbitmq_management/priv/www/js/tmpl/feature-flags.ejs index ee704e453806..e4f0188ec07f 100644 --- a/deps/rabbitmq_management/priv/www/js/tmpl/feature-flags.ejs +++ b/deps/rabbitmq_management/priv/www/js/tmpl/feature-flags.ejs @@ -13,7 +13,7 @@

<% } %>
-

All Feature Flags

+

Feature Flags

<%= filter_ui(feature_flags) %>
@@ -30,6 +30,9 @@ <% for (var i = 0; i < feature_flags.length; i++) { var feature_flag = feature_flags[i]; + if (feature_flag.stability == "experimental") { + continue; + } var state_color = "grey"; if (feature_flag.state == "enabled") { state_color = "green"; @@ -76,3 +79,74 @@
+ + + +
+

Experimental Feature Flags

+
+<% if (feature_flags.length > 0) { %> +

+ Feature flags listed below are experimental. They should not be enabled in a produciton deployment. +

+ + + + + + + + + + <% + for (var i = 0; i < feature_flags.length; i++) { + var feature_flag = feature_flags[i]; + if (feature_flag.stability != "experimental") { + continue; + } + var state_color = "grey"; + if (feature_flag.state == "enabled") { + state_color = "green"; + } else if (feature_flag.state == "disabled") { + state_color = "yellow"; + } else if (feature_flag.state == "unsupported") { + state_color = "red"; + } + %> + > + + + + + <% } %> + +
<%= fmt_sort('Name', 'name') %><%= fmt_sort('State', 'state') %>Description
<%= fmt_string(feature_flag.name) %> + <% if (feature_flag.state == "disabled") { %> +
+ +
+
+
+ + +
+ + <% } else { %> + + <%= fmt_string(feature_flag.state) %> + + <% } %> + 		+

<%= fmt_string(feature_flag.desc) %>

+ <% if (feature_flag.doc_url) { %> +

[Learn more]

+ <% } %> +
+<% } else { %> +

... no feature_flags ...

+<% } %> +
+
+ From 58e0c1600f8d0aa6592fc9e231f35b6c243892ef Mon Sep 17 00:00:00 2001 From: Michal Kuratczyk Date: Wed, 14 Aug 2024 11:32:49 +0200 Subject: [PATCH 2/5] Require --force to enable experimental FF --- .../commands/enable_feature_flag_command.ex | 48 ++++++++++++------- .../test/ctl/enable_feature_flag_test.exs | 4 +- 2 files changed, 34 insertions(+), 18 deletions(-) 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 d4405f322891..90cbecdc8df8 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 @@ -7,15 +7,18 @@ defmodule RabbitMQ.CLI.Ctl.Commands.EnableFeatureFlagCommand do @behaviour RabbitMQ.CLI.CommandBehaviour - def merge_defaults(args, opts), do: {args, opts} + def switches(), do: [force: :boolean] + def aliases(), do: [f: :force] - def validate([], _), do: {:validation_failure, :not_enough_args} - def validate([_ | _] = args, _) when length(args) > 1, do: {:validation_failure, :too_many_args} + def merge_defaults(args, opts), do: { args, Map.merge(%{force: false}, opts) } - def validate([""], _), + 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."}} - def validate([_], _), do: :ok + def validate([_], _opts), do: :ok use RabbitMQ.CLI.Core.RequiresRabbitAppRunning @@ -29,32 +32,45 @@ defmodule RabbitMQ.CLI.Ctl.Commands.EnableFeatureFlagCommand do end end - def run([feature_flag], %{node: node_name}) do - case :rabbit_misc.rpc_call(node_name, :rabbit_feature_flags, :enable, [ - String.to_atom(feature_flag) - ]) do - # Server does not support feature flags, consider none are available. - # See rabbitmq/rabbitmq-cli#344 for context. MK. - {:badrpc, {:EXIT, {:undef, _}}} -> {:error, :unsupported} - {:badrpc, _} = err -> err - other -> other + def run([feature_flag], %{node: node_name, force: force}) do + case {force, :rabbit_misc.rpc_call(node_name, :rabbit_feature_flags, :get_stability, [ + String.to_atom(feature_flag) + ])} do + {_, {:badrpc, {:EXIT, {:undef, _}}}} -> {:error, :unsupported} + {_, {:badrpc, _} = err} -> err + {false, :experimental} -> + IO.puts("Feature flag #{feature_flag} is experimental and requires --force to enable it.") + _ -> + case :rabbit_misc.rpc_call(node_name, :rabbit_feature_flags, :enable, [ + String.to_atom(feature_flag) + ]) do + # Server does not support feature flags, consider none are available. + # See rabbitmq/rabbitmq-cli#344 for context. MK. + {:badrpc, {:EXIT, {:undef, _}}} -> {:error, :unsupported} + {:badrpc, _} = err -> err + other -> other + end end 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}"} + "This feature flag is not supported by node #{node_name}"} end use RabbitMQ.CLI.DefaultOutput - def usage, do: "enable_feature_flag " + def usage, do: "enable_feature_flag [--force] " def usage_additional() do [ [ "", "name of the feature flag to enable, or \"all\" to enable all supported flags" + ], + [ + "--force", + "required to enable experimental feature flags (make sure you understand the risks!))" ] ] end 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 2608751f404a..52ffeb196e08 100644 --- a/deps/rabbitmq_cli/test/ctl/enable_feature_flag_test.exs +++ b/deps/rabbitmq_cli/test/ctl/enable_feature_flag_test.exs @@ -35,7 +35,7 @@ defmodule EnableFeatureFlagCommandTest do { :ok, - opts: %{node: get_rabbit_hostname()}, feature_flag: @feature_flag + opts: %{node: get_rabbit_hostname(), force: false}, feature_flag: @feature_flag } end @@ -59,7 +59,7 @@ defmodule EnableFeatureFlagCommandTest do end test "run: attempt to use an unreachable node returns a nodedown" do - opts = %{node: :jake@thedog, timeout: 200} + opts = %{node: :jake@thedog, timeout: 200, force: false} assert match?({:badrpc, _}, @command.run(["na"], opts)) end From e3302f2f9a24fa00ab70a3e1fb2035c7e12e10a6 Mon Sep 17 00:00:00 2001 From: Michal Kuratczyk Date: Mon, 19 Aug 2024 09:01:08 +0200 Subject: [PATCH 3/5] Rename --force to --experimental Plus, a slightly more scary error message --- .../ctl/commands/enable_feature_flag_command.ex | 16 ++++++++-------- .../test/ctl/enable_feature_flag_test.exs | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) 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 90cbecdc8df8..fe7868145d1d 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 @@ -7,10 +7,10 @@ defmodule RabbitMQ.CLI.Ctl.Commands.EnableFeatureFlagCommand do @behaviour RabbitMQ.CLI.CommandBehaviour - def switches(), do: [force: :boolean] - def aliases(), do: [f: :force] + def switches(), do: [experimental: :boolean] + def aliases(), do: [f: :experimental] - def merge_defaults(args, opts), do: { args, Map.merge(%{force: false}, opts) } + def merge_defaults(args, opts), do: { args, Map.merge(%{experimental: 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} @@ -32,14 +32,14 @@ defmodule RabbitMQ.CLI.Ctl.Commands.EnableFeatureFlagCommand do end end - def run([feature_flag], %{node: node_name, force: force}) do - case {force, :rabbit_misc.rpc_call(node_name, :rabbit_feature_flags, :get_stability, [ + 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, {:EXIT, {:undef, _}}}} -> {:error, :unsupported} {_, {:badrpc, _} = err} -> err {false, :experimental} -> - IO.puts("Feature flag #{feature_flag} is experimental and requires --force to enable it.") + IO.puts("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) @@ -60,7 +60,7 @@ defmodule RabbitMQ.CLI.Ctl.Commands.EnableFeatureFlagCommand do use RabbitMQ.CLI.DefaultOutput - def usage, do: "enable_feature_flag [--force] " + def usage, do: "enable_feature_flag [--experimental] " def usage_additional() do [ @@ -69,7 +69,7 @@ defmodule RabbitMQ.CLI.Ctl.Commands.EnableFeatureFlagCommand do "name of the feature flag to enable, or \"all\" to enable all supported flags" ], [ - "--force", + "--experimental", "required to enable experimental feature flags (make sure you understand the risks!))" ] ] 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 52ffeb196e08..ad89e42024dc 100644 --- a/deps/rabbitmq_cli/test/ctl/enable_feature_flag_test.exs +++ b/deps/rabbitmq_cli/test/ctl/enable_feature_flag_test.exs @@ -35,7 +35,7 @@ defmodule EnableFeatureFlagCommandTest do { :ok, - opts: %{node: get_rabbit_hostname(), force: false}, feature_flag: @feature_flag + opts: %{node: get_rabbit_hostname(), experimental: false}, feature_flag: @feature_flag } end @@ -59,7 +59,7 @@ defmodule EnableFeatureFlagCommandTest do end test "run: attempt to use an unreachable node returns a nodedown" do - opts = %{node: :jake@thedog, timeout: 200, force: false} + opts = %{node: :jake@thedog, timeout: 200, experimental: false} assert match?({:badrpc, _}, @command.run(["na"], opts)) end From ddb117f810b45db14b8ebcd14148df28d5232186 Mon Sep 17 00:00:00 2001 From: Michal Kuratczyk Date: Mon, 19 Aug 2024 11:07:16 +0200 Subject: [PATCH 4/5] `-f` -> `-e`; drop unneeded cases; typos Also, remove the `undef` case which was only needed for RabbitMQ 3.7 and older. --- .../cli/ctl/commands/enable_feature_flag_command.ex | 11 ++--------- .../priv/www/js/tmpl/feature-flags.ejs | 2 +- 2 files changed, 3 insertions(+), 10 deletions(-) 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 fe7868145d1d..51f7f56fc7a3 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 @@ -8,7 +8,7 @@ defmodule RabbitMQ.CLI.Ctl.Commands.EnableFeatureFlagCommand do @behaviour RabbitMQ.CLI.CommandBehaviour def switches(), do: [experimental: :boolean] - def aliases(), do: [f: :experimental] + def aliases(), do: [e: :experimental] def merge_defaults(args, opts), do: { args, Map.merge(%{experimental: false}, opts) } @@ -24,9 +24,6 @@ defmodule RabbitMQ.CLI.Ctl.Commands.EnableFeatureFlagCommand do def run(["all"], %{node: node_name}) do case :rabbit_misc.rpc_call(node_name, :rabbit_feature_flags, :enable_all, []) do - # Server does not support feature flags, consider none are available. - # See rabbitmq/rabbitmq-cli#344 for context. MK. - {:badrpc, {:EXIT, {:undef, _}}} -> {:error, :unsupported} {:badrpc, _} = err -> err other -> other end @@ -36,7 +33,6 @@ defmodule RabbitMQ.CLI.Ctl.Commands.EnableFeatureFlagCommand do case {experimental, :rabbit_misc.rpc_call(node_name, :rabbit_feature_flags, :get_stability, [ String.to_atom(feature_flag) ])} do - {_, {:badrpc, {:EXIT, {:undef, _}}}} -> {:error, :unsupported} {_, {:badrpc, _} = err} -> err {false, :experimental} -> IO.puts("Feature flag #{feature_flag} is experimental. If you understand the risk, use --experimental to enable it.") @@ -44,9 +40,6 @@ defmodule RabbitMQ.CLI.Ctl.Commands.EnableFeatureFlagCommand do case :rabbit_misc.rpc_call(node_name, :rabbit_feature_flags, :enable, [ String.to_atom(feature_flag) ]) do - # Server does not support feature flags, consider none are available. - # See rabbitmq/rabbitmq-cli#344 for context. MK. - {:badrpc, {:EXIT, {:undef, _}}} -> {:error, :unsupported} {:badrpc, _} = err -> err other -> other end @@ -70,7 +63,7 @@ defmodule RabbitMQ.CLI.Ctl.Commands.EnableFeatureFlagCommand do ], [ "--experimental", - "required to enable experimental feature flags (make sure you understand the risks!))" + "required to enable experimental feature flags (make sure you understand the risks!)" ] ] end diff --git a/deps/rabbitmq_management/priv/www/js/tmpl/feature-flags.ejs b/deps/rabbitmq_management/priv/www/js/tmpl/feature-flags.ejs index e4f0188ec07f..03f036f06d43 100644 --- a/deps/rabbitmq_management/priv/www/js/tmpl/feature-flags.ejs +++ b/deps/rabbitmq_management/priv/www/js/tmpl/feature-flags.ejs @@ -87,7 +87,7 @@
<% if (feature_flags.length > 0) { %>

- Feature flags listed below are experimental. They should not be enabled in a produciton deployment. + Feature flags listed below are experimental. They should not be enabled in a production deployment.

From 396ad7af1a3e9bdb1d34c1d740157fc1e4489809 Mon Sep 17 00:00:00 2001 From: Michal Kuratczyk Date: Mon, 19 Aug 2024 11:39:24 +0200 Subject: [PATCH 5/5] Reject `--experimental all` --- .../cli/ctl/commands/enable_feature_flag_command.ex | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) 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 51f7f56fc7a3..76bbfd466b39 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 @@ -22,10 +22,15 @@ defmodule RabbitMQ.CLI.Ctl.Commands.EnableFeatureFlagCommand do use RabbitMQ.CLI.Core.RequiresRabbitAppRunning - def run(["all"], %{node: node_name}) do - case :rabbit_misc.rpc_call(node_name, :rabbit_feature_flags, :enable_all, []) do - {:badrpc, _} = err -> err - other -> other + 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."} + false -> + case :rabbit_misc.rpc_call(node_name, :rabbit_feature_flags, :enable_all, []) do + {:badrpc, _} = err -> err + other -> other + end end end