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: graham/towerops-web#141
This commit is contained in:
Graham McIntire 2026-03-24 12:04:48 -05:00 committed by graham
parent f703e61b12
commit 56643dde86
4 changed files with 113 additions and 35 deletions

View file

@ -1 +1 @@
/nix/store/4xlkmdd947h5qlhj2rmbyavq08grkz27-pre-commit-config.json
/nix/store/zx636v118rzzqzwafgrw2aybh7l0szrl-pre-commit-config.json

View file

@ -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

View file

@ -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

View file

@ -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