From d40c004e83b48611ad4d92bb1013f8c27533967f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonatan=20K=C5=82osko?= Date: Tue, 17 Oct 2023 20:28:37 +0200 Subject: [PATCH] Warn on Dockerfile when deploying a directory with personal hub and it has file systems (#2281) --- lib/livebook/hubs/dockerfile.ex | 42 +++++++++++++------ .../live/session_live/app_docker_component.ex | 10 ++++- test/livebook/hubs/dockerfile_test.exs | 30 ++++++++++--- 3 files changed, 62 insertions(+), 20 deletions(-) diff --git a/lib/livebook/hubs/dockerfile.ex b/lib/livebook/hubs/dockerfile.ex index 18c544c08..49b2cc993 100644 --- a/lib/livebook/hubs/dockerfile.ex +++ b/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 diff --git a/lib/livebook_web/live/session_live/app_docker_component.ex b/lib/livebook_web/live/session_live/app_docker_component.ex index b963ccd61..dd3a0c152 100644 --- a/lib/livebook_web/live/session_live/app_docker_component.ex +++ b/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 diff --git a/test/livebook/hubs/dockerfile_test.exs b/test/livebook/hubs/dockerfile_test.exs index edb6c1ab3..238a09fc2 100644 --- a/test/livebook/hubs/dockerfile_test.exs +++ b/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