Warn on Dockerfile when deploying a directory with personal hub and it has file systems (#2281)

lib/livebook/hubs/dockerfile.ex

@@ -59,7 +59,7 @@ defmodule Livebook.Hubs.Dockerfile do
     image_envs = format_envs(base_image.env)
 
     hub_type = Hubs.Provider.type(hub)
-    used_secrets = used_secrets(config, hub, secrets, hub_secrets) |> Enum.sort_by(& &1.name)
+    used_secrets = used_hub_secrets(config, hub_secrets, secrets) |> Enum.sort_by(& &1.name)
     hub_config = format_hub_config(hub_type, config, hub, hub_file_systems, used_secrets)
 
     apps_config = """
@@ -202,11 +202,29 @@ defmodule Livebook.Hubs.Dockerfile do
     |> Livebook.Teams.encrypt(secret_key)
   end
 
-  defp used_secrets(config, hub, secrets, hub_secrets) do
+  defp used_hub_secrets(config, hub_secrets, secrets) do
     if config.deploy_all do
       hub_secrets
     else
-      for {_, secret} <- secrets, secret.hub_id == hub.id, do: secret
+      Enum.filter(hub_secrets, fn hub_secret ->
+        if secret = secrets[hub_secret.name] do
+          secret.hub_id == hub_secret.hub_id
+        end
+      end)
+    end
+  end
+
+  defp used_hub_file_systems(config, hub_file_systems, file_entries) do
+    if config.deploy_all do
+      hub_file_systems
+    else
+      file_entry_file_system_ids =
+        for entry <- file_entries,
+            entry.type == :file,
+            do: entry.file.file_system_id,
+            into: MapSet.new()
+
+      Enum.filter(hub_file_systems, &(&1.id in file_entry_file_system_ids))
     end
   end
 
@@ -223,11 +241,12 @@ defmodule Livebook.Hubs.Dockerfile do
           config(),
           Hubs.Provider.t(),
           list(Livebook.Secrets.Secret.t()),
+          list(Livebook.FileSystem.t()),
           Livebook.Notebook.AppSettings.t(),
           list(Livebook.Notebook.file_entry()),
           list(Livebook.Session.Data.secrets())
         ) :: list(String.t())
-  def warnings(config, hub, hub_secrets, app_settings, file_entries, secrets) do
+  def warnings(config, hub, hub_secrets, hub_file_systems, app_settings, file_entries, secrets) do
     common_warnings =
       [
         if Livebook.Session.Data.session_secrets(secrets, hub.id) != [] do
@@ -239,14 +258,18 @@ defmodule Livebook.Hubs.Dockerfile do
     hub_warnings =
       case Hubs.Provider.type(hub) do
         "personal" ->
+          used_hub_secrets = used_hub_secrets(config, hub_secrets, secrets)
+          used_hub_file_systems = used_hub_file_systems(config, hub_file_systems, file_entries)
+
           [
-            if used_secrets(config, hub, secrets, hub_secrets) != [] do
+            if used_hub_secrets != [] do
               "You are deploying an app with secrets and the secrets are included in the Dockerfile" <>
                 " as environment variables. If someone else deploys this app, they must also set the" <>
                 " same secrets. Use Livebook Teams to automatically encrypt and synchronize secrets" <>
                 " across your team and deployments."
             end,
-            if module = find_hub_file_system(file_entries) do
+            if used_hub_file_systems != [] do
+              %module{} = hd(used_hub_file_systems)
               name = LivebookWeb.FileSystemHelpers.file_system_name(module)
 
               "The #{name} file storage, defined in your personal hub, will not be available in the Docker image." <>
@@ -274,11 +297,4 @@ defmodule Livebook.Hubs.Dockerfile do
 
     Enum.reject(common_warnings ++ hub_warnings, &is_nil/1)
   end
-
-  defp find_hub_file_system(file_entries) do
-    Enum.find_value(file_entries, fn entry ->
-      entry.type == :file && entry.file.file_system_module != FileSystem.Local &&
-        entry.file.file_system_module
-    end)
-  end
 end

lib/livebook_web/live/session_live/app_docker_component.ex

@@ -181,7 +181,15 @@ defmodule LivebookWeb.SessionLive.AppDockerComponent do
       )
 
     warnings =
-      Hubs.Dockerfile.warnings(config, hub, hub_secrets, app_settings, file_entries, secrets)
+      Hubs.Dockerfile.warnings(
+        config,
+        hub,
+        hub_secrets,
+        hub_file_systems,
+        app_settings,
+        file_entries,
+        secrets
+      )
 
     assign(socket, dockerfile: dockerfile, warnings: warnings)
   end

test/livebook/hubs/dockerfile_test.exs

@@ -206,7 +206,7 @@ defmodule Livebook.Hubs.DockerfileTest do
       session_secret = %Secret{name: "SESSION", value: "test", hub_id: nil}
       secrets = %{"SESSION" => session_secret}
 
-      assert [warning] = Dockerfile.warnings(config, hub, [], app_settings, [], secrets)
+      assert [warning] = Dockerfile.warnings(config, hub, [], [], app_settings, [], secrets)
       assert warning =~ "The notebook uses session secrets"
     end
 
@@ -220,7 +220,9 @@ defmodule Livebook.Hubs.DockerfileTest do
       hub_secrets = [secret]
       secrets = %{"TEST" => secret}
 
-      assert [warning] = Dockerfile.warnings(config, hub, hub_secrets, app_settings, [], secrets)
+      assert [warning] =
+               Dockerfile.warnings(config, hub, hub_secrets, [], app_settings, [], secrets)
+
       assert warning =~ "secrets are included in the Dockerfile"
     end
 
@@ -230,12 +232,28 @@ defmodule Livebook.Hubs.DockerfileTest do
       app_settings = Livebook.Notebook.AppSettings.new()
 
       file_system = Livebook.Factory.build(:fs_s3)
+      file_systems = [file_system]
 
       file_entries = [
         %{type: :file, file: Livebook.FileSystem.File.new(file_system, "/data.csv")}
       ]
 
-      assert [warning] = Dockerfile.warnings(config, hub, [], app_settings, file_entries, %{})
+      assert [warning] =
+               Dockerfile.warnings(config, hub, [], file_systems, app_settings, file_entries, %{})
+
+      assert warning =~
+               "The S3 file storage, defined in your personal hub, will not be available in the Docker image"
+    end
+
+    test "warns when deploying a directory in personal hub and it has any file systems" do
+      config = dockerfile_config(%{deploy_all: true})
+      hub = personal_hub()
+      app_settings = Livebook.Notebook.AppSettings.new()
+
+      file_system = Livebook.Factory.build(:fs_s3)
+      file_systems = [file_system]
+
+      assert [warning] = Dockerfile.warnings(config, hub, [], file_systems, app_settings, [], %{})
 
       assert warning =~
                "The S3 file storage, defined in your personal hub, will not be available in the Docker image"
@@ -246,7 +264,7 @@ defmodule Livebook.Hubs.DockerfileTest do
       hub = personal_hub()
       app_settings = %{Livebook.Notebook.AppSettings.new() | access_type: :public}
 
-      assert [warning] = Dockerfile.warnings(config, hub, [], app_settings, [], %{})
+      assert [warning] = Dockerfile.warnings(config, hub, [], [], app_settings, [], %{})
       assert warning =~ "This app has no password configuration"
     end
 
@@ -255,12 +273,12 @@ defmodule Livebook.Hubs.DockerfileTest do
       hub = team_hub()
       app_settings = %{Livebook.Notebook.AppSettings.new() | access_type: :public}
 
-      assert [warning] = Dockerfile.warnings(config, hub, [], app_settings, [], %{})
+      assert [warning] = Dockerfile.warnings(config, hub, [], [], app_settings, [], %{})
       assert warning =~ "This app has no password configuration"
 
       config = %{config | zta_provider: :cloudflare, zta_key: "key"}
 
-      assert [] = Dockerfile.warnings(config, hub, [], app_settings, [], %{})
+      assert [] = Dockerfile.warnings(config, hub, [], [], app_settings, [], %{})
     end
   end