Fix Dockerfile generation with deployment group (#2475)

assets/css/components.css

@@ -70,7 +70,7 @@
   /* Form fields */
 
   .input {
-    @apply w-full px-3 py-2 bg-gray-50 text-sm border border-gray-200 rounded-lg placeholder-gray-400 text-gray-600 phx-form-error:border-red-300;
+    @apply w-full px-3 py-2 bg-gray-50 text-sm border border-gray-200 rounded-lg placeholder-gray-400 text-gray-600 phx-form-error:border-red-300 disabled:opacity-70 disabled:cursor-not-allowed;
   }
 
   .input[type="color"] {

lib/livebook/hubs/dockerfile.ex

@@ -14,20 +14,26 @@ defmodule Livebook.Hubs.Dockerfile do
         }
 
   @doc """
-  Builds a changeset for app Dockerfile configuration.
+  Builds the default Dockerfile configuration.
   """
-  @spec config_changeset(map()) :: Ecto.Changeset.t()
-  def config_changeset(attrs \\ %{}) do
+  @spec config_new() :: config()
+  def config_new() do
     default_image = Livebook.Config.docker_images() |> hd()
 
-    data = %{
+    %{
       deploy_all: false,
       docker_tag: default_image.tag,
       clustering: nil,
       zta_provider: nil,
       zta_key: nil
     }
+  end
 
+  @doc """
+  Builds a changeset for app Dockerfile configuration.
+  """
+  @spec config_changeset(config(), map()) :: Ecto.Changeset.t()
+  def config_changeset(config, attrs \\ %{}) do
     zta_types =
       for provider <- Livebook.Config.identity_providers(),
           do: provider.type
@@ -40,7 +46,7 @@ defmodule Livebook.Hubs.Dockerfile do
       zta_key: :string
     }
 
-    cast({data, types}, attrs, [:deploy_all, :docker_tag, :clustering, :zta_provider, :zta_key])
+    cast({config, types}, attrs, [:deploy_all, :docker_tag, :clustering, :zta_provider, :zta_key])
     |> validate_required([:deploy_all, :docker_tag])
   end
 

lib/livebook_web/components/app_components.ex

@@ -111,6 +111,7 @@ defmodule LivebookWeb.AppComponents do
   """
   attr :form, Phoenix.HTML.Form, required: true
   attr :hub, :map, required: true
+  attr :disabled, :boolean, default: false
 
   def deployment_group_form_content(assigns) do
     ~H"""
@@ -134,8 +135,7 @@ defmodule LivebookWeb.AppComponents do
           {"Single instance", ""},
           {"Fly.io", "fly_io"}
         ]}
-        disabled={@form[:ready_only].value}
-        class="disabled:cursor-not-allowed"
+        disabled={@disabled}
       />
     </div>
     <%= if Hubs.Provider.type(@hub) == "team" do %>
@@ -153,8 +153,7 @@ defmodule LivebookWeb.AppComponents do
             }
             prompt="None"
             options={zta_options()}
-            disabled={@form[:ready_only].value}
-            class="disabled:cursor-not-allowed"
+            disabled={@disabled}
           />
           <.text_field
             :if={zta_metadata = zta_metadata(@form[:zta_provider].value)}
@@ -162,8 +161,7 @@ defmodule LivebookWeb.AppComponents do
             label={zta_metadata.value}
             placeholder={zta_placeholder(zta_metadata)}
             phx-debounce
-            disabled={@form[:ready_only].value}
-            class="disabled:cursor-not-allowed"
+            disabled={@disabled}
           />
         </div>
         <div :if={zta_metadata = zta_metadata(@form[:zta_provider].value)} class="text-sm mt-1">

lib/livebook_web/components/form_components.ex

@@ -421,12 +421,11 @@ defmodule LivebookWeb.FormComponents do
   attr :class, :string, default: ""
   attr :field, Phoenix.HTML.FormField, doc: "a form field struct retrieved from the form"
   attr :help, :string, default: nil
-  attr :disabled, :boolean, default: false
 
   attr :options, :list, default: []
   attr :prompt, :string, default: nil
 
-  attr :rest, :global
+  attr :rest, :global, include: ~w(disabled)
 
   def select_field(assigns) do
     assigns = assigns_from_field(assigns)
@@ -438,7 +437,7 @@ defmodule LivebookWeb.FormComponents do
           id={@id}
           name={@name}
           class={[
-            "w-full px-3 py-2 pr-7 appearance-none bg-gray-50 text-sm border border-gray-200 rounded-lg placeholder-gray-400 text-gray-600 phx-form-error:border-red-300",
+            "w-full px-3 py-2 pr-7 appearance-none bg-gray-50 text-sm border border-gray-200 rounded-lg placeholder-gray-400 text-gray-600 phx-form-error:border-red-300 disabled:opacity-70 disabled:cursor-not-allowed",
             @class
           ]}
           {@rest}

lib/livebook_web/live/hub/teams/deployment_group_live.ex

@@ -57,7 +57,9 @@ defmodule LivebookWeb.Hub.Teams.DeploymentGroupLive do
        secrets: secrets,
        agent_keys: agent_keys
      )
-     |> assign_new(:config_changeset, fn -> Hubs.Dockerfile.config_changeset() end)
+     |> assign_new(:config_changeset, fn ->
+       Hubs.Dockerfile.config_changeset(Hubs.Dockerfile.config_new())
+     end)
      |> update_dockerfile()}
   end
 
@@ -206,7 +208,7 @@ defmodule LivebookWeb.Hub.Teams.DeploymentGroupLive do
   def handle_event("validate_dockerfile", %{"data" => data}, socket) do
     changeset =
       data
-      |> Hubs.Dockerfile.config_changeset()
+      |> Hubs.Dockerfile.config_changeset(Hubs.Dockerfile.config_new())
       |> Map.replace!(:action, :validate)
 
     {:noreply,
@@ -252,12 +254,23 @@ defmodule LivebookWeb.Hub.Teams.DeploymentGroupLive do
     Hubs.get_default_hub().id == hub.id
   end
 
+  defp update_dockerfile(socket) when socket.assigns.deployment_group == nil, do: socket
+
   defp update_dockerfile(socket) do
     config =
       socket.assigns.config_changeset
       |> Ecto.Changeset.apply_changes()
       |> Map.replace!(:deploy_all, true)
 
+    deployment_group = socket.assigns.deployment_group
+
+    config = %{
+      config
+      | clustering: deployment_group.clustering,
+        zta_provider: deployment_group.zta_provider,
+        zta_key: deployment_group.zta_key
+    }
+
     %{hub: hub, secrets: deployment_group_secrets} = socket.assigns
 
     hub_secrets = Hubs.get_secrets(hub)

lib/livebook_web/live/session_live/app_docker_component.ex

@@ -10,22 +10,48 @@ defmodule LivebookWeb.SessionLive.AppDockerComponent do
 
   @impl true
   def update(assigns, socket) do
+    deployment_group_changed? =
+      not Map.has_key?(socket.assigns, :deployment_group_id) or
+        socket.assigns.deployment_group_id != assigns.deployment_group_id
+
     socket = assign(socket, assigns)
     deployment_groups = Provider.deployment_groups(assigns.hub)
 
-    {:ok,
-     socket
-     |> assign(settings_valid?: Livebook.Notebook.AppSettings.valid?(socket.assigns.settings))
-     |> assign(
-       hub_secrets: Hubs.get_secrets(assigns.hub),
-       hub_file_systems: Hubs.get_file_systems(assigns.hub, hub_only: true),
-       deployment_groups: deployment_groups,
-       deployment_group_form: %{"deployment_group_id" => assigns.deployment_group_id},
-       deployment_group_id: assigns.deployment_group_id
-     )
-     |> assign_new(:changeset, fn -> Hubs.Dockerfile.config_changeset() end)
-     |> assign_new(:save_result, fn -> nil end)
-     |> update_dockerfile()}
+    socket =
+      socket
+      |> assign(settings_valid?: Livebook.Notebook.AppSettings.valid?(socket.assigns.settings))
+      |> assign(
+        hub_secrets: Hubs.get_secrets(assigns.hub),
+        hub_file_systems: Hubs.get_file_systems(assigns.hub, hub_only: true),
+        deployment_groups: deployment_groups,
+        deployment_group_form: %{"deployment_group_id" => assigns.deployment_group_id},
+        deployment_group_id: assigns.deployment_group_id
+      )
+      |> assign_new(:save_result, fn -> nil end)
+
+    socket =
+      if deployment_group_changed? do
+        assign(socket, :changeset, Hubs.Dockerfile.config_changeset(base_config(socket)))
+      else
+        socket
+      end
+
+    {:ok, update_dockerfile(socket)}
+  end
+
+  defp base_config(socket) do
+    if id = socket.assigns.deployment_group_id do
+      deployment_group = Enum.find(socket.assigns.deployment_groups, &(&1.id == id))
+
+      %{
+        Hubs.Dockerfile.config_new()
+        | clustering: deployment_group.clustering,
+          zta_provider: deployment_group.zta_provider,
+          zta_key: deployment_group.zta_key
+      }
+    else
+      Hubs.Dockerfile.config_new()
+    end
   end
 
   @impl true
@@ -128,19 +154,15 @@ defmodule LivebookWeb.SessionLive.AppDockerComponent do
           <%= raw(warning) %>
         </.message_box>
       </div>
-      <.form
-        :let={f}
-        for={deployment_group_form_content(assigns)}
-        as={:data}
-        phx-change="validate"
-        phx-target={@myself}
-      >
-        <div class="flex flex-col space-y-4">
-          <AppComponents.deployment_group_form_content hub={@hub} form={f} />
-        </div>
-      </.form>
       <.form :let={f} for={@changeset} as={:data} phx-change="validate" phx-target={@myself}>
-        <AppComponents.docker_config_form_content hub={@hub} form={f} />
+        <div class="flex flex-col space-y-4">
+          <AppComponents.deployment_group_form_content
+            hub={@hub}
+            form={f}
+            disabled={@deployment_group_id != nil}
+          />
+          <AppComponents.docker_config_form_content hub={@hub} form={f} />
+        </div>
       </.form>
       <.save_result :if={@save_result} save_result={@save_result} />
       <AppComponents.docker_instructions
@@ -184,8 +206,9 @@ defmodule LivebookWeb.SessionLive.AppDockerComponent do
   @impl true
   def handle_event("validate", %{"data" => data}, socket) do
     changeset =
-      data
-      |> Hubs.Dockerfile.config_changeset()
+      socket
+      |> base_config()
+      |> Hubs.Dockerfile.config_changeset(data)
       |> Map.replace!(:action, :validate)
 
     {:noreply, assign(socket, changeset: changeset) |> update_dockerfile()}
@@ -204,6 +227,7 @@ defmodule LivebookWeb.SessionLive.AppDockerComponent do
   end
 
   def handle_event("select_deployment_group", %{"deployment_group_id" => id}, socket) do
+    id = if(id != "", do: id)
     Livebook.Session.set_notebook_deployment_group(socket.assigns.session.pid, id)
 
     {:noreply, socket}
@@ -268,22 +292,6 @@ defmodule LivebookWeb.SessionLive.AppDockerComponent do
         do: {deployment_group.name, deployment_group.id}
   end
 
-  defp deployment_group_form_content(%{deployment_group_id: id} = assigns) do
-    deployment_group = if id, do: Enum.find(assigns.deployment_groups, &(&1.id == id))
-
-    if deployment_group do
-      %{
-        "deployment_group_id" => deployment_group.id,
-        "clustering" => deployment_group.clustering,
-        "zta_provider" => deployment_group.zta_provider,
-        "zta_key" => deployment_group.zta_key,
-        "ready_only" => true
-      }
-    else
-      assigns.changeset
-    end
-  end
-
   defp deployment_group_help() do
     "Share deployment credentials, secrets, and configuration with deployment groups."
   end

test/livebook/hubs/dockerfile_test.exs

@@ -292,8 +292,8 @@ defmodule Livebook.Hubs.DockerfileTest do
   end
 
   defp dockerfile_config(attrs \\ %{}) do
-    attrs
-    |> Dockerfile.config_changeset()
+    Dockerfile.config_new()
+    |> Dockerfile.config_changeset(attrs)
     |> Ecto.Changeset.apply_changes()
   end