From 56643dde8651a6a96004e4d8887e6efb871f1c57 Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Tue, 24 Mar 2026 12:04:48 -0500 Subject: [PATCH] fix: use correct struct field names for Check protobuf (#141) The Check protobuf has a `oneof config` field which generates separate fields in the Elixir struct: :http, :tcp, :dns, :ssl. However, check_type_config/2 was returning [config: {:http, ...}] which doesn't match the struct definition, causing a KeyError crash in build_and_push_check_jobs/1. This bug was originally fixed in e0a74e2f but was accidentally reintroduced in f703e61b when attempting to use "tagged tuple config". Fix: Return field-specific keyword lists (e.g., [http: %HttpCheckConfig{}]) instead of [config: {:http, ...}]. Also added comprehensive test that creates all 4 check types and verifies no KeyError crash occurs when sending check jobs to agent. Reviewed-on: https://git.mcintire.me/graham/towerops-web/pulls/141 --- .pre-commit-config.yaml | 2 +- lib/towerops_web/channels/agent_channel.ex | 58 ++++++------- test/towerops/settings_test.exs | 2 +- .../channels/agent_channel_test.exs | 86 +++++++++++++++++++ 4 files changed, 113 insertions(+), 35 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 3498c83d..173daa95 120000 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1 +1 @@ -/nix/store/4xlkmdd947h5qlhj2rmbyavq08grkz27-pre-commit-config.json \ No newline at end of file +/nix/store/zx636v118rzzqzwafgrw2aybh7l0szrl-pre-commit-config.json \ No newline at end of file diff --git a/lib/towerops_web/channels/agent_channel.ex b/lib/towerops_web/channels/agent_channel.ex index 2874a528..afc396c1 100644 --- a/lib/towerops_web/channels/agent_channel.ex +++ b/lib/towerops_web/channels/agent_channel.ex @@ -910,54 +910,46 @@ defmodule ToweropsWeb.AgentChannel do defp check_type_config("http", config) do [ - config: - {:http, - %HttpCheckConfig{ - url: config["url"] || "", - method: config["method"] || "GET", - expected_status: config["expected_status"] || 200, - verify_ssl: config["verify_ssl"] != false, - regex: config["regex"] || "", - follow_redirects: config["follow_redirects"] != false - }} + http: %HttpCheckConfig{ + url: config["url"] || "", + method: config["method"] || "GET", + expected_status: config["expected_status"] || 200, + verify_ssl: config["verify_ssl"] != false, + regex: config["regex"] || "", + follow_redirects: config["follow_redirects"] != false + } ] end defp check_type_config("tcp", config) do [ - config: - {:tcp, - %TcpCheckConfig{ - host: config["host"] || "", - port: config["port"] || 0, - send: config["send"] || "", - expect: config["expect"] || "" - }} + tcp: %TcpCheckConfig{ + host: config["host"] || "", + port: config["port"] || 0, + send: config["send"] || "", + expect: config["expect"] || "" + } ] end defp check_type_config("dns", config) do [ - config: - {:dns, - %DnsCheckConfig{ - hostname: config["hostname"] || "", - server: config["server"] || "", - record_type: config["record_type"] || "A", - expected: config["expected"] || "" - }} + dns: %DnsCheckConfig{ + hostname: config["hostname"] || "", + server: config["server"] || "", + record_type: config["record_type"] || "A", + expected: config["expected"] || "" + } ] end defp check_type_config("ssl", config) do [ - config: - {:ssl, - %SslCheckConfig{ - host: config["host"] || "", - port: config["port"] || 443, - warning_days: config["warning_days"] || 30 - }} + ssl: %SslCheckConfig{ + host: config["host"] || "", + port: config["port"] || 443, + warning_days: config["warning_days"] || 30 + } ] end diff --git a/test/towerops/settings_test.exs b/test/towerops/settings_test.exs index d18ddb11..756d205a 100644 --- a/test/towerops/settings_test.exs +++ b/test/towerops/settings_test.exs @@ -6,7 +6,7 @@ defmodule Towerops.SettingsTest do # Ensure the migration-seeded setting exists (CI may have stale test DB) setup do - unless Settings.get_setting_record("global_default_cloud_poller_id") do + if !Settings.get_setting_record("global_default_cloud_poller_id") do {:ok, _} = create_setting("global_default_cloud_poller_id", nil, "uuid") end diff --git a/test/towerops_web/channels/agent_channel_test.exs b/test/towerops_web/channels/agent_channel_test.exs index b06197f1..1eb86607 100644 --- a/test/towerops_web/channels/agent_channel_test.exs +++ b/test/towerops_web/channels/agent_channel_test.exs @@ -464,6 +464,92 @@ defmodule ToweropsWeb.AgentChannelTest do {:ok, job_list} = AgentJobList.decode(decoded_binary) assert is_list(job_list.jobs) end + + test "sends check jobs to agent when checks exist", %{ + socket: socket, + organization: organization, + agent_token: agent_token + } do + # Create HTTP check + {:ok, _http_check} = + Towerops.Monitoring.create_check(%{ + organization_id: organization.id, + agent_token_id: agent_token.id, + check_type: "http", + name: "Test HTTP Check", + interval_seconds: 60, + timeout_ms: 5000, + enabled: true, + config: %{ + "url" => "https://example.com", + "method" => "GET", + "expected_status" => 200, + "verify_ssl" => true + } + }) + + # Create TCP check + {:ok, _tcp_check} = + Towerops.Monitoring.create_check(%{ + organization_id: organization.id, + agent_token_id: agent_token.id, + check_type: "tcp", + name: "Test TCP Check", + interval_seconds: 60, + timeout_ms: 5000, + enabled: true, + config: %{ + "host" => "example.com", + "port" => 80 + } + }) + + # Create DNS check + {:ok, _dns_check} = + Towerops.Monitoring.create_check(%{ + organization_id: organization.id, + agent_token_id: agent_token.id, + check_type: "dns", + name: "Test DNS Check", + interval_seconds: 60, + timeout_ms: 5000, + enabled: true, + config: %{ + "hostname" => "example.com", + "record_type" => "A" + } + }) + + # Create SSL check + {:ok, _ssl_check} = + Towerops.Monitoring.create_check(%{ + organization_id: organization.id, + agent_token_id: agent_token.id, + check_type: "ssl", + name: "Test SSL Check", + interval_seconds: 60, + timeout_ms: 5000, + enabled: true, + config: %{ + "host" => "example.com", + "port" => 443, + "warning_days" => 30 + } + }) + + # Trigger check jobs - this will crash if check_type_config returns :config field + send(socket.channel_pid, :send_jobs) + + # Should receive check_jobs message (proves no KeyError crash) + assert_push "check_jobs", %{binary: check_jobs_binary} + + # Verify it's valid base64 + assert {:ok, _decoded} = Base.decode64(check_jobs_binary) + + # Test that the channel is still alive (no crash from KeyError) + ref = push(socket, "heartbeat", encode_payload(build_heartbeat())) + refute_reply ref, :error + end end describe "handle_info :check_heartbeat" do