Allow specifying S3 region explicitly (#1638)

lib/livebook/file_system/s3.ex

@@ -3,28 +3,46 @@ defmodule Livebook.FileSystem.S3 do
 
   # File system backed by an S3 bucket.
 
-  defstruct [:bucket_url, :access_key_id, :secret_access_key]
+  defstruct [:bucket_url, :region, :access_key_id, :secret_access_key]
 
   @type t :: %__MODULE__{
           bucket_url: String.t(),
+          region: String.t(),
           access_key_id: String.t(),
           secret_access_key: String.t()
         }
 
   @doc """
   Returns a new file system struct.
+
+  ## Options
+
+    * `:region` - the bucket region. By default the URL is assumed
+      to have the format `*.[region].[rootdomain].com` and the region
+      is inferred from that URL
+
   """
-  @spec new(String.t(), String.t(), String.t()) :: t()
-  def new(bucket_url, access_key_id, secret_access_key) do
+  @spec new(String.t(), String.t(), String.t(), keyword()) :: t()
+  def new(bucket_url, access_key_id, secret_access_key, opts \\ []) do
+    opts = Keyword.validate!(opts, [:region])
+
     bucket_url = String.trim_trailing(bucket_url, "/")
+    region = opts[:region] || region_from_uri(bucket_url)
 
     %__MODULE__{
       bucket_url: bucket_url,
+      region: region,
       access_key_id: access_key_id,
       secret_access_key: secret_access_key
     }
   end
 
+  defp region_from_uri(uri) do
+    # For many services the API host is of the form *.[region].[rootdomain].com
+    %{host: host} = URI.parse(uri)
+    host |> String.split(".") |> Enum.reverse() |> Enum.at(2, "auto")
+  end
+
   @doc """
   Parses file system from a configuration map.
   """
@@ -36,7 +54,7 @@ defmodule Livebook.FileSystem.S3 do
         access_key_id: access_key_id,
         secret_access_key: secret_access_key
       } ->
-        {:ok, new(bucket_url, access_key_id, secret_access_key)}
+        {:ok, new(bucket_url, access_key_id, secret_access_key, region: config[:region])}
 
       _config ->
         {:error,
@@ -46,7 +64,7 @@ defmodule Livebook.FileSystem.S3 do
 
   @spec to_config(t()) :: map()
   def to_config(%__MODULE__{} = s3) do
-    Map.take(s3, [:bucket_url, :access_key_id, :secret_access_key])
+    Map.take(s3, [:bucket_url, :region, :access_key_id, :secret_access_key])
   end
 end
 
@@ -373,7 +391,7 @@ defimpl Livebook.FileSystem, for: Livebook.FileSystem.S3 do
       :aws_signature.sign_v4(
         file_system.access_key_id,
         file_system.secret_access_key,
-        region_from_uri(file_system.bucket_url),
+        file_system.region,
         "s3",
         now,
         Atom.to_string(method),
@@ -387,12 +405,6 @@ defimpl Livebook.FileSystem, for: Livebook.FileSystem.S3 do
     HTTP.request(method, url, headers: headers, body: body)
   end
 
-  defp region_from_uri(uri) do
-    # For many services the API host is of the form *.[region].[rootdomain].com
-    %{host: host} = URI.parse(uri)
-    host |> String.split(".") |> Enum.reverse() |> Enum.at(2, "auto")
-  end
-
   defp decode({:ok, status, headers, body}) do
     case HTTP.fetch_content_type(headers) do
       {:ok, content_type} when content_type in ["text/xml", "application/xml"] ->

lib/livebook_web/live/settings_live/add_file_system_component.ex

@@ -43,6 +43,13 @@ defmodule LivebookWeb.SettingsLive.AddFileSystemComponent do
               placeholder: "https://s3.[region].amazonaws.com/[bucket]"
             ) %>
           </div>
+          <div>
+            <div class="input-label">Region (optional)</div>
+            <%= text_input(f, :region,
+              value: @data["region"],
+              class: "input"
+            ) %>
+          </div>
           <div>
             <div class="input-label">Access Key ID</div>
             <.with_password_toggle id="access-key-password-toggle">
@@ -97,7 +104,7 @@ defmodule LivebookWeb.SettingsLive.AddFileSystemComponent do
   end
 
   defp empty_data() do
-    %{"bucket_url" => "", "access_key_id" => "", "secret_access_key" => ""}
+    %{"bucket_url" => "", "region" => "", "access_key_id" => "", "secret_access_key" => ""}
   end
 
   defp data_valid?(data) do
@@ -106,6 +113,10 @@ defmodule LivebookWeb.SettingsLive.AddFileSystemComponent do
   end
 
   defp data_to_file_system(data) do
-    FileSystem.S3.new(data["bucket_url"], data["access_key_id"], data["secret_access_key"])
+    region = if(data["region"] != "", do: data["region"])
+
+    FileSystem.S3.new(data["bucket_url"], data["access_key_id"], data["secret_access_key"],
+      region: region
+    )
   end
 end

test/livebook/file_system/s3_test.exs

@@ -14,6 +14,18 @@ defmodule Livebook.FileSystem.S3Test do
       assert %{bucket_url: "https://example.com/mybucket"} =
                S3.new("https://example.com/mybucket/", "key", "secret")
     end
+
+    test "determines region based on the URL by default" do
+      assert %{region: "eu-central-1"} =
+               S3.new("https://s3.eu-central-1.amazonaws.com/mybucket", "key", "secret")
+    end
+
+    test "accepts explicit region as an option" do
+      assert %{region: "auto"} =
+               S3.new("https://s3.eu-central-1.amazonaws.com/mybucket", "key", "secret",
+                 region: "auto"
+               )
+    end
   end
 
   describe "FileSystem.default_path/1" do