From 23e8029acc06bd2b665ee67d32a8adde823b7cb0 Mon Sep 17 00:00:00 2001 From: bmanifold Date: Tue, 7 Nov 2023 17:19:28 -0500 Subject: [PATCH] Add traffic filter feature flag (#2568) Why: * The traffic filter functionality is not quite ready in the system as a whole, so the web UI will give the ability to hide the section of the forms to allow for a better end user experience. --- elixir/apps/domain/lib/domain/config.ex | 20 ++++++-- .../domain/lib/domain/config/definitions.ex | 5 ++ .../web/lib/web/live/resources/components.ex | 16 ++++++ .../apps/web/lib/web/live/resources/edit.ex | 26 +++++----- elixir/apps/web/lib/web/live/resources/new.ex | 20 +++++--- .../web/test/web/live/resources/edit_test.exs | 51 +++++++++++++++++++ .../web/test/web/live/resources/new_test.exs | 44 ++++++++++++++++ .../test/web/live/sign_up/sign_up_test.exs | 2 +- elixir/config/config.exs | 6 +++ elixir/config/runtime.exs | 6 +++ 10 files changed, 171 insertions(+), 25 deletions(-) diff --git a/elixir/apps/domain/lib/domain/config.ex b/elixir/apps/domain/lib/domain/config.ex index 24d01f5c3..d22c4b0a4 100644 --- a/elixir/apps/domain/lib/domain/config.ex +++ b/elixir/apps/domain/lib/domain/config.ex @@ -115,16 +115,25 @@ defmodule Domain.Config do ## Feature flag helpers + defp feature_enabled?(feature) do + fetch_env!(:domain, :enabled_features) + |> Keyword.fetch!(feature) + end + def sign_up_enabled? do - compile_config!(Definitions, :feature_sign_up_enabled) + feature_enabled?(:signups) end def flow_activities_enabled? do - compile_config!(Definitions, :feature_flow_activities_enabled) + feature_enabled?(:flow_activities) end def todos_enabled? do - compile_config!(Definitions, :feature_todos_enabled) + feature_enabled?(:todos) + end + + def traffic_filters_enabled? do + feature_enabled?(:traffic_filters) end ## Test helpers @@ -180,6 +189,11 @@ defmodule Domain.Config do end end + def feature_flag_override(feature, value) do + enabled_features = fetch_env!(:domain, :enabled_features) |> Keyword.put(feature, value) + put_env_override(:enabled_features, enabled_features) + end + defp pdict_key_function(app, key), do: {app, key} end end diff --git a/elixir/apps/domain/lib/domain/config/definitions.ex b/elixir/apps/domain/lib/domain/config/definitions.ex index 93c7cd2c4..c00685323 100644 --- a/elixir/apps/domain/lib/domain/config/definitions.ex +++ b/elixir/apps/domain/lib/domain/config/definitions.ex @@ -623,4 +623,9 @@ defmodule Domain.Config.Definitions do Boolean flag to turn UI TODOs on/off. """ defconfig(:feature_todos_enabled, :boolean, default: false) + + @doc """ + Boolean flag to turn Resource traffic filters on/off. + """ + defconfig(:feature_traffic_filters_enabled, :boolean, default: false) end diff --git a/elixir/apps/web/lib/web/live/resources/components.ex b/elixir/apps/web/lib/web/live/resources/components.ex index ab671c726..f8ca892a3 100644 --- a/elixir/apps/web/lib/web/live/resources/components.ex +++ b/elixir/apps/web/lib/web/live/resources/components.ex @@ -5,6 +5,13 @@ defmodule Web.Resources.Components do defp pretty_print_ports(ports), do: Enum.join(ports, ", ") def map_filters_form_attrs(attrs) do + attrs = + if Domain.Config.traffic_filters_enabled?() do + attrs + else + Map.put(attrs, "filters", %{"all" => %{"enabled" => "true", "protocol" => "all"}}) + end + Map.update(attrs, "filters", [], fn filters -> filters = for {id, filter_attrs} <- filters, @@ -153,6 +160,15 @@ defmodule Web.Resources.Components do """ end + attr :form, :any, required: true + + def beta_filters_form(assigns) do + ~H""" + <.input type="hidden" name={"#{@form.name}[all][protocol]"} value="all" /> + <.input type="hidden" name={"#{@form.name}[all][enabled]"} value="true" /> + """ + end + def map_connections_form_attrs(attrs) do Map.update(attrs, "connections", [], fn connections -> for {id, connection_attrs} <- connections, diff --git a/elixir/apps/web/lib/web/live/resources/edit.ex b/elixir/apps/web/lib/web/live/resources/edit.ex index 105b241f2..0a17a71da 100644 --- a/elixir/apps/web/lib/web/live/resources/edit.ex +++ b/elixir/apps/web/lib/web/live/resources/edit.ex @@ -1,8 +1,7 @@ defmodule Web.Resources.Edit do use Web, :live_view import Web.Resources.Components - alias Domain.Gateways - alias Domain.Resources + alias Domain.{Gateways, Resources, Config} def mount(%{"id" => id} = params, _session, socket) do with {:ok, resource} <- @@ -10,16 +9,17 @@ defmodule Web.Resources.Edit do {:ok, gateway_groups} <- Gateways.list_groups(socket.assigns.subject) do form = Resources.change_resource(resource, socket.assigns.subject) |> to_form() - {:ok, - assign(socket, - resource: resource, - gateway_groups: gateway_groups, - form: form, - params: Map.take(params, ["site_id"]) - ), - temporary_assigns: [ - form: %Phoenix.HTML.Form{} - ]} + socket = + assign( + socket, + resource: resource, + gateway_groups: gateway_groups, + form: form, + params: Map.take(params, ["site_id"]), + traffic_filters_enabled?: Config.traffic_filters_enabled?() + ) + + {:ok, socket, temporary_assigns: [form: %Phoenix.HTML.Form{}]} else _other -> raise Web.LiveErrors.NotFoundError end @@ -53,7 +53,7 @@ defmodule Web.Resources.Edit do required /> - <.filters_form form={@form[:filters]} /> + <.filters_form :if={@traffic_filters_enabled?} form={@form[:filters]} /> <.connections_form :if={is_nil(@params["site_id"])} diff --git a/elixir/apps/web/lib/web/live/resources/new.ex b/elixir/apps/web/lib/web/live/resources/new.ex index 43597012c..1763498db 100644 --- a/elixir/apps/web/lib/web/live/resources/new.ex +++ b/elixir/apps/web/lib/web/live/resources/new.ex @@ -1,18 +1,22 @@ defmodule Web.Resources.New do use Web, :live_view import Web.Resources.Components - alias Domain.Gateways - alias Domain.Resources + alias Domain.{Gateways, Resources, Config} def mount(params, _session, socket) do with {:ok, gateway_groups} <- Gateways.list_groups(socket.assigns.subject) do changeset = Resources.new_resource(socket.assigns.account) - {:ok, assign(socket, params: Map.take(params, ["site_id"])), - temporary_assigns: [ - gateway_groups: gateway_groups, - form: to_form(changeset) - ]} + socket = + assign( + socket, + gateway_groups: gateway_groups, + form: to_form(changeset), + params: Map.take(params, ["site_id"]), + traffic_filters_enabled?: Config.traffic_filters_enabled?() + ) + + {:ok, socket, temporary_assigns: [form: %Phoenix.HTML.Form{}]} else _other -> raise Web.LiveErrors.NotFoundError end @@ -51,7 +55,7 @@ defmodule Web.Resources.New do phx-debounce="300" /> - <.filters_form form={@form[:filters]} /> + <.filters_form :if={@traffic_filters_enabled?} form={@form[:filters]} /> <.connections_form :if={is_nil(@params["site_id"])} diff --git a/elixir/apps/web/test/web/live/resources/edit_test.exs b/elixir/apps/web/test/web/live/resources/edit_test.exs index dcc19b83a..f59bb1697 100644 --- a/elixir/apps/web/test/web/live/resources/edit_test.exs +++ b/elixir/apps/web/test/web/live/resources/edit_test.exs @@ -314,4 +314,55 @@ defmodule Web.Live.Resources.EditTest do assert saved_filters == %{all: %{ports: ""}} end + + test "does not render traffic filter form when traffic filters disabled", %{ + account: account, + group: group, + identity: identity, + resource: resource, + conn: conn + } do + Domain.Config.feature_flag_override(:traffic_filters, false) + + {:ok, lv, _html} = + conn + |> authorize_conn(identity) + |> live(~p"/#{account}/resources/#{resource}/edit?site_id=#{group}") + + form = form(lv, "form") + + assert find_inputs(form) == ["resource[name]"] + end + + test "updates a resource on valid attrs when traffic filters disabled", %{ + account: account, + group: group, + identity: identity, + resource: resource, + conn: conn + } do + Domain.Config.feature_flag_override(:traffic_filters, false) + + attrs = %{ + name: "foobar.com" + } + + {:ok, lv, _html} = + conn + |> authorize_conn(identity) + |> live(~p"/#{account}/resources/#{resource}/edit?site_id=#{group}") + + assert lv + |> form("form", resource: attrs) + |> render_submit() == + {:error, + {:live_redirect, %{to: ~p"/#{account}/sites/#{group}?#resources", kind: :push}}} + + assert saved_resource = Repo.get_by(Domain.Resources.Resource, id: resource.id) + assert saved_resource.name == attrs.name + + assert saved_resource.filters == [ + %Domain.Resources.Resource.Filter{protocol: :all, ports: []} + ] + end end diff --git a/elixir/apps/web/test/web/live/resources/new_test.exs b/elixir/apps/web/test/web/live/resources/new_test.exs index 2c7883685..e8100c6c4 100644 --- a/elixir/apps/web/test/web/live/resources/new_test.exs +++ b/elixir/apps/web/test/web/live/resources/new_test.exs @@ -300,4 +300,48 @@ defmodule Web.Live.Resources.NewTest do assert assert_redirect(lv, ~p"/#{account}/sites/#{group}?#resources") end + + test "does not render traffic filter form", %{ + account: account, + group: group, + identity: identity, + conn: conn + } do + Domain.Config.feature_flag_override(:traffic_filters, false) + + {:ok, lv, _html} = + conn + |> authorize_conn(identity) + |> live(~p"/#{account}/resources/new?site_id=#{group}") + + form = form(lv, "form") + + assert find_inputs(form) == ["resource[address]", "resource[name]"] + end + + test "creates a resource on valid attrs when traffic filter form disabled", %{ + account: account, + group: group, + identity: identity, + conn: conn + } do + attrs = %{ + name: "foobar.com", + address: "foobar.com" + } + + {:ok, lv, _html} = + conn + |> authorize_conn(identity) + |> live(~p"/#{account}/resources/new?site_id=#{group}") + + lv + |> form("form", resource: attrs) + |> render_submit() + + resource = Repo.get_by(Domain.Resources.Resource, %{name: attrs.name, address: attrs.address}) + assert %{connections: [connection]} = Repo.preload(resource, :connections) + assert connection.gateway_group_id == group.id + assert assert_redirect(lv, ~p"/#{account}/sites/#{group}?#resources") + end end diff --git a/elixir/apps/web/test/web/live/sign_up/sign_up_test.exs b/elixir/apps/web/test/web/live/sign_up/sign_up_test.exs index 7f7d5ee89..18dba4897 100644 --- a/elixir/apps/web/test/web/live/sign_up/sign_up_test.exs +++ b/elixir/apps/web/test/web/live/sign_up/sign_up_test.exs @@ -80,7 +80,7 @@ defmodule Web.Live.SignUpTest do end test "renders signup disabled message", %{conn: conn} do - Domain.Config.put_system_env_override(:feature_sign_up_enabled, false) + Domain.Config.feature_flag_override(:signups, false) {:ok, _lv, html} = live(conn, ~p"/sign_up") diff --git a/elixir/config/config.exs b/elixir/config/config.exs index 8659e39cb..749c62432 100644 --- a/elixir/config/config.exs +++ b/elixir/config/config.exs @@ -71,6 +71,12 @@ config :domain, Domain.Instrumentation, client_logs_enabled: true, client_logs_bucket: "logs" +config :domain, :enabled_features, + traffic_filters: true, + signups: true, + flow_activities: true, + todos: true + ############################### ##### Web ##################### ############################### diff --git a/elixir/config/runtime.exs b/elixir/config/runtime.exs index 94869970e..7104e87b3 100644 --- a/elixir/config/runtime.exs +++ b/elixir/config/runtime.exs @@ -65,6 +65,12 @@ if config_env() == :prod do client_logs_enabled: compile_config!(:instrumentation_client_logs_enabled), client_logs_bucket: compile_config!(:instrumentation_client_logs_bucket) + config :domain, :enabled_features, + traffic_filters: compile_config!(:feature_traffic_filters_enabled), + signups: compile_config!(:feature_sign_up_enabled), + flow_activities: compile_config!(:feature_flow_activities_enabled), + todos: compile_config!(:feature_todos_enabled) + ############################### ##### Web ##################### ###############################