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: graham/towerops-web#101

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
 

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