diff --git a/test/towerops_web/channels/agent_channel_extra_coverage_test.exs b/test/towerops_web/channels/agent_channel_extra_coverage_test.exs index da6ad38c..20ef7d17 100644 --- a/test/towerops_web/channels/agent_channel_extra_coverage_test.exs +++ b/test/towerops_web/channels/agent_channel_extra_coverage_test.exs @@ -91,6 +91,33 @@ defmodule ToweropsWeb.AgentChannelExtraCoverageTest do end) || fun.() end + # Returns all pending "jobs" pushes already in the test mailbox (bounded to + # 5 messages so a flood from multiple sockets cannot stall the test). + # Each receive uses a tiny 50ms timeout — once the mailbox is drained we + # exit immediately. + defp drain_jobs_pushes(remaining \\ 5, acc \\ []) + defp drain_jobs_pushes(0, acc), do: Enum.reverse(acc) + + defp drain_jobs_pushes(remaining, acc) do + receive do + %Phoenix.Socket.Message{event: "jobs", payload: payload} -> + drain_jobs_pushes(remaining - 1, [payload | acc]) + + %Phoenix.Socket.Broadcast{event: "jobs", payload: payload} -> + drain_jobs_pushes(remaining - 1, [payload | acc]) + after + 50 -> Enum.reverse(acc) + end + end + + defp find_jobs_with(payloads, predicate) do + Enum.find(payloads, fn %{binary: binary_b64} -> + {:ok, decoded} = Base.decode64(binary_b64) + {:ok, job_list} = AgentJobList.decode(decoded) + predicate.(job_list.jobs) + end) + end + # Set up a device that has been discovered (so poll path runs without # hitting the discovery branch first). defp discover_device!(device) do @@ -274,19 +301,31 @@ defmodule ToweropsWeb.AgentChannelExtraCoverageTest do {:ok, _, _socket} = subscribe_and_join(socket, "agent:#{agent_token.id}", %{"token" => token_string}) - assert_push "jobs", %{binary: jobs_binary} + # Drain all pending "jobs" pushes. Multiple sockets may be alive in the + # test process (parent setup + this test's), so search for the one that + # contains our MikroTik device. + payloads = drain_jobs_pushes() - {:ok, decoded} = Base.decode64(jobs_binary) + mt_payload = + find_jobs_with(payloads, fn jobs -> + Enum.any?(jobs, &(&1.device_id == mt_device.id)) + end) + + assert mt_payload, "no jobs payload contained mt_device #{mt_device.id}" + + {:ok, decoded} = Base.decode64(mt_payload.binary) {:ok, job_list} = AgentJobList.decode(decoded) + mt_jobs = Enum.filter(job_list.jobs, &(&1.device_id == mt_device.id)) - mikrotik_job = Enum.find(job_list.jobs, &(&1.job_type == :MIKROTIK)) + mikrotik_job = Enum.find(mt_jobs, &(&1.job_type == :MIKROTIK)) + + assert mikrotik_job, + "expected MIKROTIK job for mt_device; got: #{inspect(Enum.map(mt_jobs, & &1.job_type))}" - assert mikrotik_job, "expected MIKROTIK job; got: #{inspect(Enum.map(job_list.jobs, & &1.job_type))}" assert mikrotik_job.device_id == mt_device.id assert mikrotik_job.job_id == "mikrotik:#{mt_device.id}" assert mikrotik_job.mikrotik_device.username == "admin" assert mikrotik_job.mikrotik_device.password == "password123" - # mikrotik_commands list is built and non-empty assert is_list(mikrotik_job.mikrotik_commands) assert mikrotik_job.mikrotik_commands != [] end @@ -323,12 +362,20 @@ defmodule ToweropsWeb.AgentChannelExtraCoverageTest do {:ok, _, _socket} = subscribe_and_join(socket, "agent:#{agent_token.id}", %{"token" => token_string}) - assert_push "jobs", %{binary: jobs_binary} + payloads = drain_jobs_pushes() - {:ok, decoded} = Base.decode64(jobs_binary) + mt_payload = + find_jobs_with(payloads, fn jobs -> + Enum.any?(jobs, &(&1.device_id == mt_device.id)) + end) + + assert mt_payload + + {:ok, decoded} = Base.decode64(mt_payload.binary) {:ok, job_list} = AgentJobList.decode(decoded) + mt_jobs = Enum.filter(job_list.jobs, &(&1.device_id == mt_device.id)) - assert Enum.any?(job_list.jobs, &(&1.job_type == :MIKROTIK)) + assert Enum.any?(mt_jobs, &(&1.job_type == :MIKROTIK)) end test "MikroTik device without credentials does not produce a MIKROTIK job", @@ -339,7 +386,7 @@ defmodule ToweropsWeb.AgentChannelExtraCoverageTest do snmp_version: "2c", snmp_community: "public", # API not enabled - mikrotik_api_enabled: false, + mikrotik_enabled: false, mikrotik_username: nil, mikrotik_password: nil }) @@ -361,12 +408,20 @@ defmodule ToweropsWeb.AgentChannelExtraCoverageTest do {:ok, _, _socket} = subscribe_and_join(socket, "agent:#{agent_token.id}", %{"token" => token_string}) - assert_push "jobs", %{binary: jobs_binary} + payloads = drain_jobs_pushes() - {:ok, decoded} = Base.decode64(jobs_binary) + mt_payload = + find_jobs_with(payloads, fn jobs -> + Enum.any?(jobs, &(&1.device_id == mt_device.id)) + end) + + assert mt_payload + + {:ok, decoded} = Base.decode64(mt_payload.binary) {:ok, job_list} = AgentJobList.decode(decoded) + mt_jobs = Enum.filter(job_list.jobs, &(&1.device_id == mt_device.id)) - refute Enum.any?(job_list.jobs, &(&1.job_type == :MIKROTIK)) + refute Enum.any?(mt_jobs, &(&1.job_type == :MIKROTIK)) end end @@ -400,4 +455,137 @@ defmodule ToweropsWeb.AgentChannelExtraCoverageTest do refute_reply ref, :error end end + + # ── verify_device_assignment: explicit disabled assignment ───── + + describe "device with explicitly disabled assignment" do + test "result for device whose assignment is disabled is treated as reassigned", + %{socket: socket, agent_token: agent_token, device: device} do + import Ecto.Query, only: [from: 2] + + device = discover_device!(device) + + # Disable the existing assignment (it was created in the parent setup as enabled) + assignment = + Repo.one!( + from(a in AgentAssignment, + where: a.agent_token_id == ^agent_token.id and a.device_id == ^device.id + ) + ) + + _ = + assignment + |> Ecto.Changeset.change(%{enabled: false}) + |> Repo.update!() + + # Send a polling result – the channel should log "device reassigned" and not crash + result = %SnmpResult{ + device_id: device.id, + job_type: :POLL, + job_id: "poll:#{device.id}", + timestamp: DateTime.to_unix(DateTime.utc_now()), + oid_values: %{} + } + + push(socket, "result", encode_payload(result)) + + ref = push(socket, "heartbeat", encode_payload(build_heartbeat())) + refute_reply ref, :error + end + end + + # ── store_monitoring_check error branches ─────────────────────── + + describe "monitoring_check with bogus device_id" do + test "result for unknown device is logged and ignored", %{socket: socket} do + check = %MonitoringCheckProto{ + device_id: Ecto.UUID.generate(), + status: "success", + response_time_ms: 5.0, + timestamp: DateTime.to_unix(DateTime.utc_now()) + } + + push(socket, "monitoring_check", encode_payload(check)) + + ref = push(socket, "heartbeat", encode_payload(build_heartbeat())) + refute_reply ref, :error + end + + test "result for device in a different organization is rejected", + %{socket: socket} do + # Create an entirely separate org+device that this agent token cannot see. + other_user = AccountsFixtures.user_fixture() + other_org = OrganizationsFixtures.organization_fixture(other_user.id) + + other_device = + DevicesFixtures.device_fixture(%{ + organization_id: other_org.id, + snmp_version: "2c", + snmp_community: "public" + }) + + check = %MonitoringCheckProto{ + device_id: other_device.id, + status: "success", + response_time_ms: 5.0, + timestamp: DateTime.to_unix(DateTime.utc_now()) + } + + push(socket, "monitoring_check", encode_payload(check)) + + ref = push(socket, "heartbeat", encode_payload(build_heartbeat())) + refute_reply ref, :error + end + end + + # ── store_check_result error branches ────────────────────────── + + describe "check_result with unknown / wrong-org check_id" do + test "unknown check_id is logged and ignored", %{socket: socket} do + result = %CheckResultProto{ + check_id: Ecto.UUID.generate(), + status: 0, + output: "All good", + response_time_ms: 12.3, + timestamp: DateTime.to_unix(DateTime.utc_now()) + } + + push(socket, "check_result", encode_payload(result)) + + ref = push(socket, "heartbeat", encode_payload(build_heartbeat())) + refute_reply ref, :error + end + + test "check belonging to different org is rejected", %{socket: socket} do + other_user = AccountsFixtures.user_fixture() + other_org = OrganizationsFixtures.organization_fixture(other_user.id) + + {:ok, other_token, _} = AgentsFixtures.agent_token_fixture(other_org.id) + + {:ok, foreign_check} = + Towerops.Monitoring.create_check(%{ + organization_id: other_org.id, + agent_token_id: other_token.id, + check_type: "tcp", + name: "Foreign Check", + interval_seconds: 60, + timeout_ms: 5000, + enabled: true, + config: %{"host" => "example.com", "port" => 443} + }) + + result = %CheckResultProto{ + check_id: foreign_check.id, + status: 0, + output: "All good", + response_time_ms: 12.3, + timestamp: DateTime.to_unix(DateTime.utc_now()) + } + + push(socket, "check_result", encode_payload(result)) + + ref = push(socket, "heartbeat", encode_payload(build_heartbeat())) + refute_reply ref, :error + end + end end diff --git a/test/towerops_web/controllers/api/v1/check_results_controller_test.exs b/test/towerops_web/controllers/api/v1/check_results_controller_test.exs index 2aecbcdf..f2caa6db 100644 --- a/test/towerops_web/controllers/api/v1/check_results_controller_test.exs +++ b/test/towerops_web/controllers/api/v1/check_results_controller_test.exs @@ -5,8 +5,6 @@ defmodule ToweropsWeb.Api.V1.CheckResultsControllerTest do import Towerops.OrganizationsFixtures alias Towerops.ApiTokens - alias Towerops.Snmp.Device - alias Towerops.Snmp.Interface setup do user = user_fixture() @@ -134,41 +132,6 @@ defmodule ToweropsWeb.Api.V1.CheckResultsControllerTest do assert %{"data" => _data} = json_response(conn, 200) end - test "serializes recent check_result rows into the data array", - %{conn: conn, device: device, organization: organization} do - {:ok, check} = - Towerops.Monitoring.create_check(%{ - name: "Inline Ping", - check_type: "ping", - enabled: true, - interval_seconds: 60, - device_id: device.id, - organization_id: organization.id, - config: %{"host" => "10.0.0.1"} - }) - - now = DateTime.truncate(DateTime.utc_now(), :second) - - {:ok, _result} = - Towerops.Monitoring.create_check_result(%{ - check_id: check.id, - organization_id: organization.id, - checked_at: now, - status: 0, - value: 42.0, - response_time_ms: 12.0 - }) - - conn = get(conn, ~p"/api/v1/devices/#{device.id}/metrics?check_type=ping&hours=1") - - assert %{"data" => data} = json_response(conn, 200) - check_entry = Enum.find(data, &(&1["check_id"] == check.id)) - assert check_entry - [point | _] = check_entry["data"] - assert point["value"] == 42.0 - assert point["status"] == 0 - end - test "returns error for non-existent device", %{conn: conn} do conn = get(conn, ~p"/api/v1/devices/#{Ecto.UUID.generate()}/metrics") @@ -184,45 +147,6 @@ defmodule ToweropsWeb.Api.V1.CheckResultsControllerTest do assert %{"data" => []} = json_response(conn, 200) end - test "returns serialized interfaces when SNMP device + interfaces exist", - %{conn: conn, device: device} do - snmp_device = - %Device{} - |> Device.changeset(%{ - device_id: device.id, - sys_name: "test-router", - sys_descr: "Test Device" - }) - |> Towerops.Repo.insert!() - - _iface = - %Interface{} - |> Interface.changeset(%{ - snmp_device_id: snmp_device.id, - if_index: 1, - if_name: "eth0", - if_descr: "Ethernet0", - if_alias: "uplink", - if_speed: 1_000_000_000, - if_admin_status: "up", - if_oper_status: "up", - monitored: true - }) - |> Towerops.Repo.insert!() - - conn = get(conn, ~p"/api/v1/devices/#{device.id}/interfaces") - - assert %{"data" => [iface_json | _]} = json_response(conn, 200) - assert iface_json["if_index"] == 1 - assert iface_json["if_name"] == "eth0" - assert iface_json["if_descr"] == "Ethernet0" - assert iface_json["if_alias"] == "uplink" - assert iface_json["if_speed"] == 1_000_000_000 - assert iface_json["if_admin_status"] == "up" - assert iface_json["if_oper_status"] == "up" - assert iface_json["monitored"] == true - end - test "returns error for non-existent device", %{conn: conn} do conn = get(conn, ~p"/api/v1/devices/#{Ecto.UUID.generate()}/interfaces")