Fix some git file storage issues (#3069)

Author: aleDsz

lib/livebook/file_system/git.ex

@@ -38,17 +38,6 @@ defmodule Livebook.FileSystem.Git do
       message: "must be a valid repo URL"
     )
     |> validate_required([:repo_url, :branch, :key, :hub_id])
-    |> update_change(:key, fn key ->
-      if key =~ "\n" do
-        key
-      else
-        String.replace(
-          key,
-          ~r/ (?!(?:OPENSSH |RSA |EC |DSA |PRIVATE )?(?:PRIVATE )?KEY-----)/,
-          "\n"
-        )
-      end
-    end)
     |> put_id()
   end
 

lib/livebook/file_system/git/client.ex

@@ -136,7 +136,7 @@ defmodule Livebook.FileSystem.Git.Client do
   end
 
   defp env_opts(key_path) do
-    [env: %{"GIT_SSH_COMMAND" => "ssh -i '#{key_path}'"}]
+    [env: %{"GIT_SSH_COMMAND" => "ssh -o StrictHostKeyChecking=no -i '#{key_path}'"}]
   end
 
   defp fetch_repository(file_system) do
@@ -151,10 +151,16 @@ defmodule Livebook.FileSystem.Git.Client do
 
   defp with_ssh_key_file(file_system, fun) when is_function(fun, 1) do
     File.mkdir_p!(Livebook.Config.tmp_path())
-
     key_path = FileSystem.Git.key_path(file_system)
-    pem_entry = :public_key.pem_decode(file_system.key)
-    ssh_key = :public_key.pem_encode(pem_entry)
+
+    ssh_key =
+      file_system.key
+      |> String.replace(
+        ~r/ (?!(?:OPENSSH |RSA |EC |DSA |PRIVATE )?(?:PRIVATE )?KEY-----)/,
+        "\n"
+      )
+      |> :public_key.pem_decode()
+      |> :public_key.pem_encode()
 
     File.write!(key_path, ssh_key)
     File.chmod!(key_path, 0o600)

lib/livebook/file_system/mounter.ex

@@ -1,6 +1,7 @@
 defmodule Livebook.FileSystem.Mounter do
   # This server is responsible to handle file systems that are mountable
   use GenServer
+  require Logger
 
   alias Livebook.{FileSystem, Hubs}
 
@@ -26,7 +27,7 @@ defmodule Livebook.FileSystem.Mounter do
 
   @impl GenServer
   def handle_continue(:boot, state) do
-    Hubs.Broadcasts.subscribe([:connection, :crud, :file_systems])
+    Hubs.Broadcasts.subscribe([:crud, :file_systems])
     Process.send_after(self(), :remount, state.loop_delay)
 
     {:noreply, mount_file_systems(state, Hubs.Personal.id())}
@@ -49,10 +50,6 @@ defmodule Livebook.FileSystem.Mounter do
     {:noreply, unmount_file_system(state, file_system)}
   end
 
-  def handle_info({:hub_changed, hub_id}, state) do
-    {:noreply, mount_file_systems(state, hub_id)}
-  end
-
   def handle_info({:hub_deleted, hub_id}, state) do
     {:noreply, unmount_file_systems(state, hub_id)}
   end
@@ -98,7 +95,10 @@ defmodule Livebook.FileSystem.Mounter do
         broadcast({:file_system_mounted, file_system})
         put_hub_file_system(state, file_system)
 
-      {:error, _reason} ->
+      {:error, reason} ->
+        Logger.error("[file_system=#{name(file_system)}] failed to mount: #{reason}")
+        Process.send_after(self(), {:file_system_created, file_system}, to_timeout(second: 10))
+
         state
     end
   end
@@ -109,7 +109,10 @@ defmodule Livebook.FileSystem.Mounter do
         broadcast({:file_system_unmounted, file_system})
         remove_hub_file_system(state, file_system)
 
-      {:error, _reason} ->
+      {:error, reason} ->
+        Logger.error("[file_system=#{name(file_system)}] failed to unmount: #{reason}")
+        Process.send_after(self(), {:file_system_deleted, file_system}, to_timeout(second: 10))
+
         state
     end
   end
@@ -148,6 +151,10 @@ defmodule Livebook.FileSystem.Mounter do
     put_in(hub_data.file_systems, file_systems)
   end
 
+  defp name(file_system) do
+    FileSystem.external_metadata(file_system).name
+  end
+
   if Mix.env() == :test do
     defp broadcast({_, %{external_id: _, hub_id: id}} = message) do
       Phoenix.PubSub.broadcast(Livebook.PubSub, "file_systems:#{id}", message)

test/livebook_teams/web/hub/edit_live_test.exs

@@ -229,15 +229,6 @@ defmodule LivebookWeb.Integration.Hub.EditLiveTest do
     test "creates a Git file system", %{conn: conn, team: team} do
       id = Livebook.FileSystem.Utils.id("git", team.id, "[email protected]:livebook-dev/test.git")
       file_system = build(:fs_git, id: id, hub_id: team.id)
-
-      # When the user paste the ssh key to the password input, it will remove the break lines.
-      # So, the changeset need to normalize it before persisting. That said, the ssh key from
-      # factory must be "denormalized" so we can test the user scenario.
-      file_system = %{
-        file_system
-        | key: file_system.key |> String.replace("\n", "\s") |> String.trim()
-      }
-
       attrs = %{file_system: Livebook.FileSystem.dump(file_system)}
 
       {:ok, view, _html} = live(conn, ~p"/hub/#{team.id}")

test/livebook_web/live/hub/edit_live_test.exs

@@ -201,15 +201,6 @@ defmodule LivebookWeb.Hub.EditLiveTest do
     test "creates a Git file system", %{conn: conn, hub: hub} do
       file_system = build(:fs_git)
       id = file_system.id
-
-      # When the user paste the ssh key to the password input, it will remove the break lines.
-      # So, the changeset need to normalize it before persisting. That said, the ssh key from
-      # factory must be "denormalized" so we can test the user scenario.
-      file_system = %{
-        file_system
-        | key: file_system.key |> String.replace("\n", "\s") |> String.trim()
-      }
-
       attrs = %{file_system: Livebook.FileSystem.dump(file_system)}
 
       {:ok, view, _html} = live(conn, ~p"/hub/#{hub.id}")

test/support/factory.ex

@@ -96,7 +96,14 @@ defmodule Livebook.Factory do
   def build(:fs_git) do
     repo_url = "[email protected]:livebook-dev/test.git"
     hub_id = Livebook.Hubs.Personal.id()
-    key = System.get_env("TEST_GIT_SSH_KEY")
+
+    # When the user paste the ssh key to the password input, it will remove the break lines.
+    # So, the changeset need to normalize it before persisting. That said, the ssh key from
+    # factory must be "denormalized" so we can test the user scenario.
+    key =
+      System.get_env("TEST_GIT_SSH_KEY")
+      |> String.replace("\n", "\s")
+      |> String.trim()
 
     %Livebook.FileSystem.Git{
       id: Livebook.FileSystem.Utils.id("git", hub_id, repo_url),