From 29db294da10d8488c2836b791576cf729a0f3aa4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C5=81ukasz=20Niemier?= <lukasz@niemier.pl>
Date: Fri, 12 Oct 2018 00:08:06 +0200
Subject: [PATCH] Use `__` prefix for internal endpoints

---
 lib/imager/config/store_transform.ex           |  4 ++--
 lib/imager_web/plug/metrics_exporter.ex        |  5 +----
 lib/imager_web/router.ex                       |  2 +-
 test/imager/config/store_transform_test.exs    | 13 ++++++++++---
 test/imager_web/controllers/health_test.exs    |  3 ++-
 test/imager_web/plug/metrics_exporter_test.exs |  8 ++++----
 6 files changed, 20 insertions(+), 15 deletions(-)

diff --git a/lib/imager/config/store_transform.ex b/lib/imager/config/store_transform.ex
index d3e0398..b8745fa 100644
--- a/lib/imager/config/store_transform.ex
+++ b/lib/imager/config/store_transform.ex
@@ -7,7 +7,6 @@ defmodule Imager.Config.StoreTransform do
 
   require Logger
 
-  @reserved ~w[health]
   @types ~w[S3 Local Blackhole]
 
   def transform(:stores, entries) do
@@ -16,7 +15,8 @@ defmodule Imager.Config.StoreTransform do
            {:ok, cache} <- get_cache(values) do
         path = to_string(path)
 
-        if path in @reserved, do: raise("'#{path}' is reserved name")
+        if String.starts_with?(path, "_"),
+          do: raise("'#{path}' cannot start with underscore")
 
         if String.contains?(path, "/"),
           do: raise("'#{path}' cannot contain '/'")
diff --git a/lib/imager_web/plug/metrics_exporter.ex b/lib/imager_web/plug/metrics_exporter.ex
index 6812d8a..87e59f7 100644
--- a/lib/imager_web/plug/metrics_exporter.ex
+++ b/lib/imager_web/plug/metrics_exporter.ex
@@ -25,13 +25,10 @@ defmodule ImagerWeb.Plug.MetricsExporter do
     opts =
       opts
       |> Keyword.merge(Application.get_env(:imager, :prometheus, []))
-      |> Keyword.put_new(:endpoint, "/metrics")
       |> Keyword.update(:format, :prometheus_text_format, &parse_format/1)
 
-    endpoint = Keyword.fetch!(opts, :endpoint)
-
     case conn.request_path do
-      ^endpoint -> send_stats(conn, opts)
+      "/__metrics" -> send_stats(conn, opts)
       _ -> conn
     end
   end
diff --git a/lib/imager_web/router.ex b/lib/imager_web/router.ex
index 6720de9..4d8dc32 100644
--- a/lib/imager_web/router.ex
+++ b/lib/imager_web/router.ex
@@ -12,7 +12,7 @@ defmodule ImagerWeb.Router do
   scope "/", ImagerWeb.Controllers do
     pipe_through(:api)
 
-    get("/health", Health, :get)
+    get("/__health", Health, :get)
 
     get("/:store/*path", Image, :get)
   end
diff --git a/test/imager/config/store_transform_test.exs b/test/imager/config/store_transform_test.exs
index 64333dc..3a31f5e 100644
--- a/test/imager/config/store_transform_test.exs
+++ b/test/imager/config/store_transform_test.exs
@@ -13,6 +13,7 @@ defmodule Imager.Config.StoreTransformTest do
 
   property "returns map with stringified atoms" do
     check all path <- atom(:alphanumeric),
+              not String.starts_with?("#{path}", "_"),
               config <- store_config() do
       assert Map.has_key?(
                Subject.transform(:stores, %{path => config}),
@@ -73,9 +74,15 @@ defmodule Imager.Config.StoreTransformTest do
     end
   end
 
-  test "raises when uses reserved name" do
-    assert_raise RuntimeError, "'health' is reserved name", fn ->
-      Subject.transform(:stores, %{health: %{type: "Blackhole"}})
+  property "raises when uses reserved name" do
+    check all name <- string(:alphanumeric) do
+      assert_raise RuntimeError,
+                   "'_#{name}' cannot start with underscore",
+                   fn ->
+                     Subject.transform(:stores, %{
+                       :"_#{name}" => %{type: "Blackhole"}
+                     })
+                   end
     end
   end
 
diff --git a/test/imager_web/controllers/health_test.exs b/test/imager_web/controllers/health_test.exs
index 247a953..4e1543c 100644
--- a/test/imager_web/controllers/health_test.exs
+++ b/test/imager_web/controllers/health_test.exs
@@ -2,6 +2,7 @@ defmodule ImagerWeb.Controllers.HealthTest do
   use ImagerWeb.ConnCase, async: true
 
   test "returns 200 on existing file", %{conn: conn} do
-    assert %{"status" => "pass"} = conn |> get("/health") |> json_response(200)
+    assert %{"status" => "pass"} =
+             conn |> get("/__health") |> json_response(200)
   end
 end
diff --git a/test/imager_web/plug/metrics_exporter_test.exs b/test/imager_web/plug/metrics_exporter_test.exs
index 3a9e951..b20632b 100644
--- a/test/imager_web/plug/metrics_exporter_test.exs
+++ b/test/imager_web/plug/metrics_exporter_test.exs
@@ -19,11 +19,11 @@ defmodule ImagerWeb.Plug.MetricsExporterTest do
   end
 
   property "non-metric endpoints are passed through" do
-    opts = Subject.init(endpoint: "/metric")
+    opts = Subject.init([])
 
     check all path <- path(),
               method <- http_method(),
-              path != "/metric" do
+              path != "/__metrics" do
       conn = conn(method, path)
 
       assert conn == Subject.call(conn, opts)
@@ -31,8 +31,8 @@ defmodule ImagerWeb.Plug.MetricsExporterTest do
   end
 
   test "metric endpoint return 200" do
-    opts = Subject.init(endpoint: "/metric")
-    conn = conn(:get, "/metric")
+    opts = Subject.init([])
+    conn = conn(:get, "/__metrics")
 
     assert {200, _, _} = sent_resp(Subject.call(conn, opts))
   end