From 45a12ed18113f7f5e317c2b8f28e581736c9fad1 Mon Sep 17 00:00:00 2001 From: Graham McIntire Date: Sat, 21 Mar 2026 06:56:48 -0500 Subject: [PATCH] Fix Check protobuf oneof field encoding for service checks (#101) Protobuf oneof fields use a single :config key with tagged tuples like {:dns, %DnsCheckConfig{}} rather than individual keys. The check_type_config functions were setting e.g. dns: %DnsCheckConfig{} directly, causing KeyError crashes in Check.__struct__/1. Reviewed-on: https://git.mcintire.me/graham/towerops-web/pulls/101 --- lib/towerops_web/channels/agent_channel.ex | 58 +++++---- .../agent_channel_check_proto_test.exs | 110 ++++++++++++++++++ 2 files changed, 143 insertions(+), 25 deletions(-) create mode 100644 test/towerops_web/channels/agent_channel_check_proto_test.exs diff --git a/lib/towerops_web/channels/agent_channel.ex b/lib/towerops_web/channels/agent_channel.ex index cdc7edb5..31e2a656 100644 --- a/lib/towerops_web/channels/agent_channel.ex +++ b/lib/towerops_web/channels/agent_channel.ex @@ -907,46 +907,54 @@ defmodule ToweropsWeb.AgentChannel do defp check_type_config("http", config) do [ - 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 - } + 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 + }} ] end defp check_type_config("tcp", config) do [ - tcp: %TcpCheckConfig{ - host: config["host"] || "", - port: config["port"] || 0, - send: config["send"] || "", - expect: config["expect"] || "" - } + config: + {:tcp, + %TcpCheckConfig{ + host: config["host"] || "", + port: config["port"] || 0, + send: config["send"] || "", + expect: config["expect"] || "" + }} ] end defp check_type_config("dns", config) do [ - dns: %DnsCheckConfig{ - hostname: config["hostname"] || "", - server: config["server"] || "", - record_type: config["record_type"] || "A", - expected: config["expected"] || "" - } + config: + {: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 [ - ssl: %SslCheckConfig{ - host: config["host"] || "", - port: config["port"] || 443, - warning_days: config["warning_days"] || 30 - } + config: + {:ssl, + %SslCheckConfig{ + host: config["host"] || "", + port: config["port"] || 443, + warning_days: config["warning_days"] || 30 + }} ] end diff --git a/test/towerops_web/channels/agent_channel_check_proto_test.exs b/test/towerops_web/channels/agent_channel_check_proto_test.exs new file mode 100644 index 00000000..464fbb04 --- /dev/null +++ b/test/towerops_web/channels/agent_channel_check_proto_test.exs @@ -0,0 +1,110 @@ +defmodule ToweropsWeb.AgentChannelCheckProtoTest do + use ExUnit.Case, async: true + + alias Towerops.Agent.Check, as: CheckProto + alias Towerops.Agent.CheckList + alias Towerops.Agent.DnsCheckConfig + alias Towerops.Agent.HttpCheckConfig + alias Towerops.Agent.SslCheckConfig + alias Towerops.Agent.TcpCheckConfig + + describe "Check protobuf encoding" do + test "encodes HTTP check correctly" do + check = %CheckProto{ + id: "test-id", + check_type: "http", + interval_seconds: 60, + timeout_ms: 5000, + config: + {:http, + %HttpCheckConfig{ + url: "https://example.com", + method: "GET", + expected_status: 200, + verify_ssl: true, + regex: "", + follow_redirects: true + }} + } + + binary = CheckList.encode(%CheckList{checks: [check]}) + decoded = CheckList.decode(binary) + [decoded_check] = decoded.checks + + assert decoded_check.id == "test-id" + assert decoded_check.check_type == "http" + assert {:http, %HttpCheckConfig{url: "https://example.com"}} = decoded_check.config + end + + test "encodes TCP check correctly" do + check = %CheckProto{ + id: "test-id", + check_type: "tcp", + interval_seconds: 60, + timeout_ms: 5000, + config: + {:tcp, + %TcpCheckConfig{ + host: "example.com", + port: 443, + send: "", + expect: "" + }} + } + + binary = CheckList.encode(%CheckList{checks: [check]}) + decoded = CheckList.decode(binary) + [decoded_check] = decoded.checks + + assert decoded_check.check_type == "tcp" + assert {:tcp, %TcpCheckConfig{host: "example.com", port: 443}} = decoded_check.config + end + + test "encodes DNS check correctly" do + check = %CheckProto{ + id: "test-id", + check_type: "dns", + interval_seconds: 60, + timeout_ms: 5000, + config: + {:dns, + %DnsCheckConfig{ + hostname: "example.com", + server: "8.8.8.8", + record_type: "A", + expected: "1.2.3.4" + }} + } + + binary = CheckList.encode(%CheckList{checks: [check]}) + decoded = CheckList.decode(binary) + [decoded_check] = decoded.checks + + assert decoded_check.check_type == "dns" + assert {:dns, %DnsCheckConfig{hostname: "example.com", server: "8.8.8.8"}} = decoded_check.config + end + + test "encodes SSL check correctly" do + check = %CheckProto{ + id: "test-id", + check_type: "ssl", + interval_seconds: 60, + timeout_ms: 5000, + config: + {:ssl, + %SslCheckConfig{ + host: "example.com", + port: 443, + warning_days: 30 + }} + } + + binary = CheckList.encode(%CheckList{checks: [check]}) + decoded = CheckList.decode(binary) + [decoded_check] = decoded.checks + + assert decoded_check.check_type == "ssl" + assert {:ssl, %SslCheckConfig{host: "example.com", port: 443}} = decoded_check.config + end + end +end