diff --git a/.gitignore b/.gitignore index 5b807d8..9327c11 100644 --- a/.gitignore +++ b/.gitignore @@ -10,6 +10,9 @@ # Where 3rd-party dependencies like ExDoc output generated docs. /doc/ +# Where ExUnit adds temporary directories during test runs. +/tmp/ + # Ignore .fetch files in case you like to edit your project deps locally. /.fetch diff --git a/lib/nerves_hub_link/socket.ex b/lib/nerves_hub_link/socket.ex index 246e6dc..1503dc8 100644 --- a/lib/nerves_hub_link/socket.ex +++ b/lib/nerves_hub_link/socket.ex @@ -55,6 +55,10 @@ defmodule NervesHubLink.Socket do GenServer.call(pid, {:finish_uploading, filename}) end + def check_update_available(pid \\ __MODULE__) do + GenServer.call(pid, :check_update_available) + end + @doc """ Cancel an ongoing upload @@ -164,6 +168,12 @@ defmodule NervesHubLink.Socket do {:reply, connected?(socket), socket} end + def handle_call(:check_update_available, _from, socket) do + {:ok, ref} = push(socket, @device_topic, "check_update_available", socket.assigns.params) + + {:reply, await_reply(ref), socket} + end + def handle_call(:console_active?, _from, socket) do {:reply, socket.assigns.iex_pid != nil, socket} end diff --git a/lib/nerves_hub_link/update_manager.ex b/lib/nerves_hub_link/update_manager.ex index 347e16c..926795d 100644 --- a/lib/nerves_hub_link/update_manager.ex +++ b/lib/nerves_hub_link/update_manager.ex @@ -9,7 +9,7 @@ defmodule NervesHubLink.UpdateManager do """ use GenServer - alias NervesHubLink.{Downloader, FwupConfig} + alias NervesHubLink.{Downloader, FwupConfig, Socket} alias NervesHubLink.Message.UpdateInfo require Logger @@ -27,13 +27,17 @@ defmodule NervesHubLink.UpdateManager do | :update_rescheduled | {:updating, integer()} + @type previous_update :: :complete | :failed + @type t :: %__MODULE__{ status: status(), update_reschedule_timer: nil | :timer.tref(), download: nil | GenServer.server(), fwup: nil | GenServer.server(), fwup_config: FwupConfig.t(), - update_info: nil | UpdateInfo.t() + update_info: nil | UpdateInfo.t(), + retrying: boolean(), + previous_update: nil | previous_update() } defstruct status: :idle, @@ -41,7 +45,9 @@ defmodule NervesHubLink.UpdateManager do fwup: nil, download: nil, fwup_config: nil, - update_info: nil + update_info: nil, + retrying: false, + previous_update: nil end @doc """ @@ -61,6 +67,14 @@ defmodule NervesHubLink.UpdateManager do GenServer.call(manager, :status) end + @doc """ + Returns the previous status of the update manager + """ + @spec previous_update(GenServer.server()) :: State.previous_update() + def previous_update(manager \\ __MODULE__) do + GenServer.call(manager, :previous_update) + end + @doc """ Returns the UUID of the currently downloading firmware, or nil. """ @@ -113,6 +127,10 @@ defmodule NervesHubLink.UpdateManager do {:reply, state.status, state} end + def handle_call(:previous_update, _from, %State{} = state) do + {:reply, state.previous_update, state} + end + @impl GenServer def handle_info({:update_reschedule, response, fwup_public_keys}, state) do {:noreply, @@ -145,22 +163,55 @@ defmodule NervesHubLink.UpdateManager do end # messages from Downloader - def handle_info({:download, :complete}, state) do + def handle_info({:download, :complete, _fwup_public_keys}, state) do Logger.info("[NervesHubLink] Firmware Download complete") - {:noreply, %State{state | download: nil}} + + {:noreply, %State{state | status: :idle, retrying: false, previous_update: :complete}} end - def handle_info({:download, {:error, reason}}, state) do - Logger.error("[NervesHubLink] Nonfatal HTTP download error: #{inspect(reason)}") + def handle_info( + {:download, {:error, %Mint.HTTPError{reason: {:http_error, 401}}}, _fwup_public_keys}, + %State{retrying: true} = state + ) do + Logger.error("[NervesHubLink] Firmware download error: 401 after retry") + {:noreply, %State{state | status: :idle, retrying: false, previous_update: :failed}} + end + + def handle_info( + {:download, {:error, %Mint.HTTPError{reason: {:http_error, 401}}}, fwup_public_keys}, + state + ) do + Logger.error( + "[NervesHubLink] Firmware download URL signature may have expired. Requesting new URL and retrying." + ) + + GenServer.stop(Fwup.Stream) + + update_info = Socket.check_update_available() + updated_state = %State{state | status: :idle, retrying: true, fwup: nil} + state = maybe_update_firmware(update_info, fwup_public_keys, updated_state) {:noreply, state} end + def handle_info({:download, {:error, reason}, _fwup_public_keys}, state) do + Logger.error("[NervesHubLink] Nonfatal HTTP download error: #{inspect(reason)}") + {:noreply, %State{state | status: :idle, retrying: false}} + end + # Data from the downloader is sent to fwup - def handle_info({:download, {:data, data}}, state) do + def handle_info({:download, {:data, data}, _fwup_public_keys}, state) do _ = Fwup.Stream.send_chunk(state.fwup, data) {:noreply, state} end + def handle_info({:EXIT, _pid, {:http_error, 401}}, state) do + {:noreply, %State{state | download: nil}} + end + + def handle_info({:EXIT, _pid, :normal}, state) do + {:noreply, state} + end + @spec maybe_update_firmware(UpdateInfo.t(), [binary()], State.t()) :: State.t() defp maybe_update_firmware( %UpdateInfo{} = _update_info, @@ -215,7 +266,10 @@ defmodule NervesHubLink.UpdateManager do @spec start_fwup_stream(UpdateInfo.t(), [binary()], State.t()) :: State.t() defp start_fwup_stream(%UpdateInfo{} = update_info, [] = fwup_public_keys, state) do pid = self() - fun = &send(pid, {:download, &1}) + + Process.flag(:trap_exit, true) + + fun = &send(pid, {:download, &1, fwup_public_keys}) {:ok, download} = Downloader.start_download(update_info.firmware_url, fun) {:ok, fwup} = diff --git a/mix.exs b/mix.exs index 918fa8c..6940278 100644 --- a/mix.exs +++ b/mix.exs @@ -89,6 +89,7 @@ defmodule NervesHubLink.MixProject do {:fwup, "~> 1.0"}, {:jason, "~> 1.0"}, {:mint, "~> 1.2"}, + {:mock, "~> 0.3.0", only: :test}, {:mox, "~> 1.0", only: :test}, {:nerves_key, "~> 1.0 or ~> 0.5", optional: true}, {:nerves_runtime, "~> 0.8"}, diff --git a/mix.lock b/mix.lock index c23dac8..d6a83ca 100644 --- a/mix.lock +++ b/mix.lock @@ -21,9 +21,11 @@ "makeup": {:hex, :makeup, "1.1.2", "9ba8837913bdf757787e71c1581c21f9d2455f4dd04cfca785c70bbfff1a76a3", [:mix], [{:nimble_parsec, "~> 1.2.2 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "cce1566b81fbcbd21eca8ffe808f33b221f9eee2cbc7a1706fc3da9ff18e6cac"}, "makeup_elixir": {:hex, :makeup_elixir, "0.16.2", "627e84b8e8bf22e60a2579dad15067c755531fea049ae26ef1020cad58fe9578", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}, {:nimble_parsec, "~> 1.2.3 or ~> 1.3", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "41193978704763f6bbe6cc2758b84909e62984c7752b3784bd3c218bb341706b"}, "makeup_erlang": {:hex, :makeup_erlang, "1.0.0", "6f0eff9c9c489f26b69b61440bf1b238d95badae49adac77973cbacae87e3c2e", [:mix], [{:makeup, "~> 1.0", [hex: :makeup, repo: "hexpm", optional: false]}], "hexpm", "ea7a9307de9d1548d2a72d299058d1fd2339e3d398560a0e46c27dab4891e4d2"}, + "meck": {:hex, :meck, "0.9.2", "85ccbab053f1db86c7ca240e9fc718170ee5bda03810a6292b5306bf31bae5f5", [:rebar3], [], "hexpm", "81344f561357dc40a8344afa53767c32669153355b626ea9fcbc8da6b3045826"}, "mime": {:hex, :mime, "2.0.5", "dc34c8efd439abe6ae0343edbb8556f4d63f178594894720607772a041b04b02", [:mix], [], "hexpm", "da0d64a365c45bc9935cc5c8a7fc5e49a0e0f9932a761c55d6c52b142780a05c"}, "mint": {:hex, :mint, "1.6.0", "88a4f91cd690508a04ff1c3e28952f322528934be541844d54e0ceb765f01d5e", [:mix], [{:castore, "~> 0.1.0 or ~> 1.0", [hex: :castore, repo: "hexpm", optional: true]}, {:hpax, "~> 0.1.1 or ~> 0.2.0", [hex: :hpax, repo: "hexpm", optional: false]}], "hexpm", "3c5ae85d90a5aca0a49c0d8b67360bbe407f3b54f1030a111047ff988e8fefaa"}, "mint_web_socket": {:hex, :mint_web_socket, "1.0.3", "aab42fff792a74649916236d0b01f560a0b3f03ca5dea693c230d1c44736b50e", [:mix], [{:mint, ">= 1.4.1 and < 2.0.0-0", [hex: :mint, repo: "hexpm", optional: false]}], "hexpm", "ca3810ca44cc8532e3dce499cc17f958596695d226bb578b2fbb88c09b5954b0"}, + "mock": {:hex, :mock, "0.3.8", "7046a306b71db2488ef54395eeb74df0a7f335a7caca4a3d3875d1fc81c884dd", [:mix], [{:meck, "~> 0.9.2", [hex: :meck, repo: "hexpm", optional: false]}], "hexpm", "7fa82364c97617d79bb7d15571193fc0c4fe5afd0c932cef09426b3ee6fe2022"}, "mox": {:hex, :mox, "1.1.0", "0f5e399649ce9ab7602f72e718305c0f9cdc351190f72844599545e4996af73c", [:mix], [], "hexpm", "d44474c50be02d5b72131070281a5d3895c0e7a95c780e90bc0cfe712f633a13"}, "muontrap": {:hex, :muontrap, "1.5.0", "bf5c273872379968615a39974458328209ac97fa1f588396192131ff973d1ca2", [:make, :mix], [{:elixir_make, "~> 0.6", [hex: :elixir_make, repo: "hexpm", optional: false]}], "hexpm", "daf605e877f60b5be9215e3420d7971fc468677b29921e40915b15fd928273d4"}, "nerves_key": {:hex, :nerves_key, "1.2.0", "2cd65b7f474582d0af7bcd59664df16d0c58d1673dce573072d7dbe971dbd663", [:mix], [{:atecc508a, "~> 0.3.0 or ~> 1.1", [hex: :atecc508a, repo: "hexpm", optional: false]}, {:nerves_key_pkcs11, "~> 0.2 or ~> 1.0", [hex: :nerves_key_pkcs11, repo: "hexpm", optional: false]}], "hexpm", "ef49b26f38728b554cca2a0b16d198989a571e753d427ead2b22a8ea10a06000"}, diff --git a/test/nerves_hub_link/downloader_test.exs b/test/nerves_hub_link/downloader_test.exs index 3e20071..8120c95 100644 --- a/test/nerves_hub_link/downloader_test.exs +++ b/test/nerves_hub_link/downloader_test.exs @@ -3,6 +3,7 @@ defmodule NervesHubLink.DownloaderTest do alias NervesHubLink.Support.{ HTTPErrorPlug, + HTTPUnauthorizedErrorPlug, IdleTimeoutPlug, RangeRequestPlug, RedirectPlug, @@ -101,6 +102,35 @@ defmodule NervesHubLink.DownloaderTest do end end + describe "unauthorized http error" do + setup do + port = 7025 + + {:ok, plug} = + start_supervised( + {Plug.Cowboy, + scheme: :http, + plug: {HTTPUnauthorizedErrorPlug, report_pid: self()}, + options: [port: port]} + ) + + {:ok, [plug: plug, url: "http://localhost:#{port}/test"]} + end + + test "exits when an HTTP error occurs", %{url: url} do + test_pid = self() + handler_fun = &send(test_pid, &1) + + Process.flag(:trap_exit, true) + + {:ok, download} = Downloader.start_download(url, handler_fun) + + assert_receive :request_error, 1000 + assert_receive {:error, %Mint.HTTPError{reason: {:http_error, 401}}}, 1000 + assert_receive {:EXIT, ^download, {:http_error, 401}} + end + end + describe "range" do setup do port = 7030 diff --git a/test/nerves_hub_link/update_manager_test.exs b/test/nerves_hub_link/update_manager_test.exs index ef4ca91..5619609 100644 --- a/test/nerves_hub_link/update_manager_test.exs +++ b/test/nerves_hub_link/update_manager_test.exs @@ -1,13 +1,15 @@ defmodule NervesHubLink.UpdateManagerTest do - use ExUnit.Case + use ExUnit.Case, async: false + alias NervesHubLink.{FwupConfig, UpdateManager} alias NervesHubLink.Message.{FirmwareMetadata, UpdateInfo} alias NervesHubLink.Support.FWUPStreamPlug + import Mock + describe "fwup stream" do setup do - port = 6000 - devpath = "/tmp/fwup_output" + port = Enum.random(6000..6999) update_payload = %UpdateInfo{ firmware_url: "http://localhost:#{port}/test.fw", @@ -19,13 +21,12 @@ defmodule NervesHubLink.UpdateManagerTest do {Plug.Cowboy, scheme: :http, plug: FWUPStreamPlug, options: [port: port]} ) - File.rm(devpath) - - {:ok, [plug: plug, update_payload: update_payload, devpath: "/tmp/fwup_output"]} + {:ok, [plug: plug, update_payload: update_payload]} end - test "apply", %{update_payload: update_payload, devpath: devpath} do - fwup_config = %{default_config() | fwup_devpath: devpath} + @tag :tmp_dir + test "apply", %{update_payload: update_payload, tmp_dir: tmp_dir} do + fwup_config = %{default_config() | fwup_devpath: Path.join(tmp_dir, "fwup_output")} {:ok, manager} = UpdateManager.start_link(fwup_config) assert UpdateManager.apply_update(manager, update_payload, []) == {:updating, 0} @@ -35,7 +36,8 @@ defmodule NervesHubLink.UpdateManagerTest do assert_receive {:fwup, {:ok, 0, ""}} end - test "reschedule", %{update_payload: update_payload, devpath: devpath} do + @tag :tmp_dir + test "reschedule", %{update_payload: update_payload, tmp_dir: tmp_dir} do test_pid = self() update_available_fun = fn _ -> @@ -52,7 +54,7 @@ defmodule NervesHubLink.UpdateManagerTest do fwup_config = %{ default_config() - | fwup_devpath: devpath, + | fwup_devpath: Path.join(tmp_dir, "fwup_output"), update_available: update_available_fun } @@ -66,10 +68,11 @@ defmodule NervesHubLink.UpdateManagerTest do assert_receive {:fwup, {:ok, 0, ""}} end - test "apply with fwup environment", %{update_payload: update_payload, devpath: devpath} do + @tag :tmp_dir + test "apply with fwup environment", %{update_payload: update_payload, tmp_dir: tmp_dir} do fwup_config = %{ default_config() - | fwup_devpath: devpath, + | fwup_devpath: Path.join(tmp_dir, "fwup_output"), fwup_task: "secret_upgrade", fwup_env: [ {"SUPER_SECRET", "1234567890123456789012345678901234567890123456789012345678901234"} @@ -87,6 +90,49 @@ defmodule NervesHubLink.UpdateManagerTest do end end + describe "401 retry" do + setup do + port = 6543 + + update_payload = %UpdateInfo{ + firmware_url: "http://localhost:#{port}/test.fw", + firmware_meta: %FirmwareMetadata{} + } + + {:ok, plug} = + start_supervised( + {Plug.Cowboy, + scheme: :http, + plug: {NervesHubLink.Support.HTTPUnauthorizedErrorPlug, report_pid: self()}, + options: [port: port]} + ) + + {:ok, [plug: plug, update_payload: update_payload]} + end + + @tag :tmp_dir + test "retries firmware updates once", %{ + update_payload: update_payload, + tmp_dir: tmp_dir + } do + fwup_config = %{default_config() | fwup_devpath: Path.join(tmp_dir, "fwup_output")} + + {:ok, manager} = UpdateManager.start_link(fwup_config) + + with_mock NervesHubLink.Socket, check_update_available: fn -> update_payload end do + assert UpdateManager.apply_update(manager, update_payload, []) == {:updating, 0} + + assert_receive {:fwup, {:progress, 0}}, 1_000 + assert_receive :request_error, 1_000 + assert_receive :request_error, 1_000 + + Process.sleep(300) + + assert UpdateManager.previous_update(manager) == :failed + end + end + end + defp default_config() do test_pid = self() fwup_fun = &send(test_pid, {:fwup, &1}) diff --git a/test/support/http_error_plug.ex b/test/support/http_error_plug.ex index b479a6c..ec13812 100644 --- a/test/support/http_error_plug.ex +++ b/test/support/http_error_plug.ex @@ -10,7 +10,6 @@ defmodule NervesHubLink.Support.HTTPErrorPlug do @impl Plug def call(conn, _opts) do - conn - |> send_resp(416, "Range Not Satisfiable") + send_resp(conn, 416, "Range Not Satisfiable") end end diff --git a/test/support/http_unauthorized_error_plug.ex b/test/support/http_unauthorized_error_plug.ex new file mode 100644 index 0000000..3f876d1 --- /dev/null +++ b/test/support/http_unauthorized_error_plug.ex @@ -0,0 +1,17 @@ +defmodule NervesHubLink.Support.HTTPUnauthorizedErrorPlug do + @moduledoc false + + @behaviour Plug + + import Plug.Conn + + @impl Plug + def init(options), do: options + + @impl Plug + def call(conn, opts) do + send(opts[:report_pid], :request_error) + + send_resp(conn, 401, "Unauthorized") + end +end