Skip to content

Commit

Permalink
test: do not use meck when testing metrics controller (#593)
Browse files Browse the repository at this point in the history
Not only it is not needed, it was harmful, as it was incorrect (the mock
wasn't removed after test), which was causing some other tests to fail
unexpectedly.
  • Loading branch information
hauleth authored Feb 4, 2025
1 parent 16224d6 commit ddcc26b
Show file tree
Hide file tree
Showing 8 changed files with 55 additions and 35 deletions.
2 changes: 1 addition & 1 deletion VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
2.2.0
2.2.1
18 changes: 16 additions & 2 deletions lib/supavisor/jwt.ex
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,24 @@ defmodule Supavisor.Jwt do
"""
require Logger

defmodule JwtAuthToken do
defmodule Token do
@moduledoc false
use Joken.Config

def default_signer(secret),
do: Joken.Signer.create("HS256", secret)

def gen!(claims \\ %{}, secret)

def gen!(claims, secret) when is_binary(secret),
do: gen!(claims, default_signer(secret))

def gen!(claims, signer) do
default = %{"exp" => Joken.current_time() + 3600}

generate_and_sign!(Map.merge(default, claims), signer)
end

@impl true
def token_config do
Application.fetch_env!(:supavisor, :jwt_claim_validators)
Expand Down Expand Up @@ -61,7 +75,7 @@ defmodule Supavisor.Jwt do
with {:ok, _claims} <- check_claims_format(token),
{:ok, header} <- check_header_format(token),
{:ok, signer} <- generate_signer(header, secret) do
JwtAuthToken.verify_and_validate(token, signer)
Token.verify_and_validate(token, signer)
end
end

Expand Down
2 changes: 1 addition & 1 deletion test/.formatter.exs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
[
import_deps: [:ecto, :stream_data],
inputs: ["**/*.{ex, exs}"]
inputs: ["**/*.{ex,exs}"]
]
6 changes: 3 additions & 3 deletions test/supavisor/helpers_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ defmodule Supavisor.HelpersTest do
describe "validate_name/1" do
test "Prisma migration databases are accepted" do
assert @subject.validate_name(
"prisma_migrate_shadow_db_dfe467a1-f7e4-4c27-87de-a930270f4622"
)
"prisma_migrate_shadow_db_dfe467a1-f7e4-4c27-87de-a930270f4622"
)
end

property "ASCII strings with length within 1..63 are valid" do
Expand All @@ -63,7 +63,7 @@ defmodule Supavisor.HelpersTest do
check all name <- string(:printable, min_length: 1, max_length: 63) do
# It is defined in weird way, as it is hard to generate strings with at
# most 63 bytes, but that test is functionally equivalend
assert @subject.validate_name(name) == (byte_size(name) < 64)
assert @subject.validate_name(name) == byte_size(name) < 64
end
end

Expand Down
18 changes: 8 additions & 10 deletions test/supavisor/jwt_test.exs
Original file line number Diff line number Diff line change
@@ -1,33 +1,31 @@
defmodule Supavisor.JwtTest do
use ExUnit.Case, async: true
alias Supavisor.Jwt

@subject Supavisor.Jwt

@secret "my_secret_key"
@wrong_secret "my_wrong_secret_key"

describe "authorize/2" do
test "returns claims for a valid token" do
{:ok, token, _} = create_valid_jwt_token()
assert {:ok, claims} = Jwt.authorize(token, @secret)
token = create_valid_jwt_token()
assert {:ok, claims} = @subject.authorize(token, @secret)
assert claims["role"] == "test"
end

test "raises an error for non-binary token" do
assert_raise FunctionClauseError, fn ->
Jwt.authorize(123, @secret)
@subject.authorize(123, @secret)
end
end

test "returns signature_error for a wrong secret" do
{:ok, token, _} = create_valid_jwt_token()
assert {:error, :signature_error} = Jwt.authorize(token, @wrong_secret)
token = create_valid_jwt_token()
assert {:error, :signature_error} = @subject.authorize(token, @wrong_secret)
end
end

defp create_valid_jwt_token do
exp = Joken.current_time() + 3600
claims = %{"role" => "test", "exp" => exp}
signer = Joken.Signer.create("HS256", @secret)
Joken.encode_and_sign(claims, signer)
@subject.Token.gen!(%{"role" => "test"}, @secret)
end
end
5 changes: 4 additions & 1 deletion test/supavisor/monitoring/prom_ex_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,10 @@ defmodule Supavisor.Monitoring.PromExTest do
end

@tag :tmp_dir
property "non-standard tenant names do not cause parsing issues", %{tmp_dir: dir, prom2json: exe} do
property "non-standard tenant names do not cause parsing issues", %{
tmp_dir: dir,
prom2json: exe
} do
db_name = "db_name"
user = "user"

Expand Down
35 changes: 20 additions & 15 deletions test/supavisor_web/controllers/metrics_controller_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -2,33 +2,38 @@ defmodule SupavisorWeb.MetricsControllerTest do
use SupavisorWeb.ConnCase
alias Supavisor.Support.Cluster

setup %{conn: conn} do
new_conn =
conn
|> put_req_header(
"authorization",
"Bearer auth_token"
)

{:ok, conn: new_conn}
end

@tag cluster: true
test "exporting metrics", %{conn: conn} do
{:ok, _pid, node2} = Cluster.start_node()

Node.connect(node2)

:meck.expect(Supavisor.Jwt, :authorize, fn _token, _secret -> {:ok, %{}} end)
conn = get(conn, Routes.metrics_path(conn, :index))
conn =
conn
|> auth
|> get(Routes.metrics_path(conn, :index))

assert conn.status == 200
assert conn.resp_body =~ "region=\"eu\""
assert conn.resp_body =~ "region=\"usa\""
end

test "invalid jwt", %{conn: conn} do
:meck.expect(Supavisor.Jwt, :authorize, fn _token, _secret -> {:error, nil} end)
conn = get(conn, Routes.metrics_path(conn, :index))
token = "invalid"

conn =
conn
|> auth(token)
|> get(Routes.metrics_path(conn, :index))

assert conn.status == 403
end

defp auth(conn, bearer \\ gen_token()) do
put_req_header(conn, "authorization", "Bearer " <> bearer)
end

defp gen_token(secret \\ Application.fetch_env!(:supavisor, :metrics_jwt_secret)) do
Supavisor.Jwt.Token.gen!(secret)

Check warning on line 37 in test/supavisor_web/controllers/metrics_controller_test.exs

View workflow job for this annotation

GitHub Actions / Code style

Nested modules could be aliased at the top of the invoking module.
end
end
4 changes: 2 additions & 2 deletions test/supavisor_web/controllers/tenant_controller_test.exs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
defmodule SupavisorWeb.TenantControllerTest do
use SupavisorWeb.ConnCase
use SupavisorWeb.ConnCase, async: false

import Supavisor.TenantsFixtures
import ExUnit.CaptureLog
require Logger

alias Supavisor.Tenants.Tenant

@jwt "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJzdXBhYmFzZSIsInJvbGUiOiJhbm9uIiwiaWF0IjoxNjQ1MTkyODI0LCJleHAiOjE5NjA3Njg4MjR9.M9jrxyvPLkUxWgOYSf5dNdJ8v_eRrq810ShFRT8N-6M"
Expand Down

0 comments on commit ddcc26b

Please sign in to comment.