Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make it hard to accidentally enable an experimental feature flag #11998

Merged
merged 5 commits into from
Aug 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -7,54 +7,68 @@
defmodule RabbitMQ.CLI.Ctl.Commands.EnableFeatureFlagCommand do
@behaviour RabbitMQ.CLI.CommandBehaviour

def merge_defaults(args, opts), do: {args, opts}
def switches(), do: [experimental: :boolean]
def aliases(), do: [e: :experimental]

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(%{experimental: 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

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
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

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, 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} ->
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)
]) do
{: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 <all | feature_flag>"
def usage, do: "enable_feature_flag [--experimental] <all | feature_flag>"

def usage_additional() do
[
[
"<feature_flag>",
"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!)"
]
]
end
Expand Down
4 changes: 2 additions & 2 deletions deps/rabbitmq_cli/test/ctl/enable_feature_flag_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ defmodule EnableFeatureFlagCommandTest do

{
:ok,
opts: %{node: get_rabbit_hostname()}, feature_flag: @feature_flag
opts: %{node: get_rabbit_hostname(), experimental: false}, feature_flag: @feature_flag
}
end

Expand All @@ -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, experimental: false}
assert match?({:badrpc, _}, @command.run(["na"], opts))
end

Expand Down
76 changes: 75 additions & 1 deletion deps/rabbitmq_management/priv/www/js/tmpl/feature-flags.ejs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
</p>
<% } %>
<div class="section">
<h2>All Feature Flags</h2>
<h2>Feature Flags</h2>
<div class="hider">
<%= filter_ui(feature_flags) %>
<div class="updatable">
Expand All @@ -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";
Expand Down Expand Up @@ -76,3 +79,74 @@
</div>
</div>
</div>



<div class="section">
<h2>Experimental Feature Flags</h2>
<div class="hider">
<% if (feature_flags.length > 0) { %>
<p class="warning">
Feature flags listed below are experimental. They should not be enabled in a production deployment.
</p>
<table class="list">
<thead>
<tr>
<th><%= fmt_sort('Name', 'name') %></th>
<th class="c"><%= fmt_sort('State', 'state') %></th>
<th>Description</th>
</tr>
</thead>
<tbody>
<%
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";
}
%>
<tr<%= alt_rows(i)%>>
<td><%= fmt_string(feature_flag.name) %></td>
<td class="c">
<% if (feature_flag.state == "disabled") { %>
<div>
<input id="<%= feature_flag.name %>" type="checkbox" class="riskCheckbox" onclick="this.parentNode.querySelector('.enable-feature-flag input[type=submit]').disabled = !this.checked;">
<label for="<%= feature_flag.name %>"> I understand the risk</label><br>
<br>
<form action="#/feature-flags-enable" method="put" style="display: inline-block" class="enable-feature-flag">
<input type="hidden" name="name" value="<%= fmt_string(feature_flag.name) %>"/>
<input type="submit" value="Enable" class="c" disabled="disabled"/>
</div>
</form>
<% } else { %>
<abbr class="status-<%= fmt_string(state_color) %>"
style="text-transform: capitalize"
title="Feature flag state: <%= fmt_string(feature_flag.state) %>">
<%= fmt_string(feature_flag.state) %>
</abbr>
<% } %>
</td>
<td>
<p><%= fmt_string(feature_flag.desc) %></p>
<% if (feature_flag.doc_url) { %>
<p><a href="<%= fmt_string(feature_flag.doc_url) %>">[Learn more]</a></p>
<% } %>
</td>
</tr>
<% } %>
</tbody>
</table>
<% } else { %>
<p>... no feature_flags ...</p>
<% } %>
</div>
</div>
</div>
Loading