From 234a8f84621c5ece7169f79c561b8a62a2902b3a Mon Sep 17 00:00:00 2001 From: Paul Payne Date: Thu, 9 Jul 2026 21:07:12 +0000 Subject: [PATCH] test: Add 22 unit tests across dnsmasq, haproxy, services MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit dnsmasq (6 new, 30.8% → 32%): - Empty instances, multiple instances, nil config, configured IP - No leaked empty entries (address=//, local=//) - Empty fields don't produce broken entries Also fixed a bug: empty Domain/InternalDomain fields in commented-out entries (no LB IP case) now guarded against producing # local=// and # address=// directives. haproxy (7 new): - No HTTPS frontend with no routes - HTTP-only and L4-only route scenarios - CertsDir customization and default - Custom TCP route ports - Health check directive in L7 backends services (8 new, 72.7% → 75.8%): - Not-found errors for Get, Deregister, Update - DeregisterBySource partial match (source AND backend) - Overwrite preserves file count - Empty dir and non-YAML file handling Co-Authored-By: Claude Opus 4.6 (1M context) --- internal/dnsmasq/config.go | 8 +- internal/dnsmasq/config_modular.go | 10 +- internal/dnsmasq/config_test.go | 129 +++++++++++++++++++++ internal/haproxy/config_test.go | 144 ++++++++++++++++++++++++ internal/services/manager_test.go | 173 +++++++++++++++++++++++++++++ 5 files changed, 459 insertions(+), 5 deletions(-) diff --git a/internal/dnsmasq/config.go b/internal/dnsmasq/config.go index 5ab0f83..26e9770 100644 --- a/internal/dnsmasq/config.go +++ b/internal/dnsmasq/config.go @@ -60,8 +60,12 @@ func (g *ConfigGenerator) Generate(cfg *config.GlobalConfig, clouds []config.Ins slog.Info("no load balancer IP configured, adding commented DNS config", "component", "dnsmasq", "instance", cloud.Cluster.Name) // Add commented out entries for instances without load balancer resolution_section += fmt.Sprintf("# No load balancer IP configured for instance %s\n", cloud.Cluster.Name) - resolution_section += fmt.Sprintf("# local=/%s/\n# address=/%s/\n", cloud.Cloud.InternalDomain, cloud.Cloud.InternalDomain) - resolution_section += fmt.Sprintf("# address=/%s/\n", cloud.Cloud.Domain) + if cloud.Cloud.InternalDomain != "" { + resolution_section += fmt.Sprintf("# local=/%s/\n# address=/%s/\n", cloud.Cloud.InternalDomain, cloud.Cloud.InternalDomain) + } + if cloud.Cloud.Domain != "" { + resolution_section += fmt.Sprintf("# address=/%s/\n", cloud.Cloud.Domain) + } continue } // Internal domain (.internal.cloud.example.tld) - local only, no external DNS diff --git a/internal/dnsmasq/config_modular.go b/internal/dnsmasq/config_modular.go index 8d011dd..c3e839b 100644 --- a/internal/dnsmasq/config_modular.go +++ b/internal/dnsmasq/config_modular.go @@ -112,9 +112,13 @@ func (g *ConfigGenerator) GenerateInstanceConfig(instance config.InstanceConfig) if loadBalancerIP == "" { sb.WriteString("# WARNING: No load balancer IP configured for this instance\n") sb.WriteString("# DNS entries are commented out until load balancer IP is configured\n\n") - fmt.Fprintf(&sb, "# local=/%s/\n", instance.Cloud.InternalDomain) - fmt.Fprintf(&sb, "# address=/%s/\n\n", instance.Cloud.InternalDomain) - fmt.Fprintf(&sb, "# address=/%s/\n", instance.Cloud.Domain) + if instance.Cloud.InternalDomain != "" { + fmt.Fprintf(&sb, "# local=/%s/\n", instance.Cloud.InternalDomain) + fmt.Fprintf(&sb, "# address=/%s/\n\n", instance.Cloud.InternalDomain) + } + if instance.Cloud.Domain != "" { + fmt.Fprintf(&sb, "# address=/%s/\n", instance.Cloud.Domain) + } } else { // Internal domain (.internal.cloud.example.tld) - local only, no external DNS if instance.Cloud.InternalDomain != "" { diff --git a/internal/dnsmasq/config_test.go b/internal/dnsmasq/config_test.go index dc0fe07..1deb03c 100644 --- a/internal/dnsmasq/config_test.go +++ b/internal/dnsmasq/config_test.go @@ -1,6 +1,7 @@ package dnsmasq import ( + "fmt" "strings" "testing" @@ -438,3 +439,131 @@ func TestGenerate_MixedServicesAndInstances(t *testing.T) { t.Errorf("unexpected empty local=// in output:\n%s", out) } } + +// Test: empty instances list produces base config with no address= entries +func TestGenerate_EmptyInstances(t *testing.T) { + g := NewConfigGenerator("/tmp/test-dnsmasq.conf") + globalCfg := &config.GlobalConfig{} + + out := g.Generate(globalCfg, []config.InstanceConfig{}) + + // Should have base config skeleton + if !strings.Contains(out, "bind-interfaces") { + t.Errorf("expected bind-interfaces in config, got:\n%s", out) + } + if !strings.Contains(out, "server=1.1.1.1") { + t.Errorf("expected upstream DNS server, got:\n%s", out) + } + // Should not have any address= entries + if strings.Contains(out, "address=/") { + t.Errorf("expected no address= entries with empty instances, got:\n%s", out) + } + // Should not have any local=/ entries + if strings.Contains(out, "local=/") { + t.Errorf("expected no local=/ entries with empty instances, got:\n%s", out) + } +} + +// Test: multiple instances each get their own entries +func TestGenerate_MultipleInstances(t *testing.T) { + g := NewConfigGenerator("/tmp/test-dnsmasq.conf") + globalCfg := &config.GlobalConfig{} + + instances := []config.InstanceConfig{ + instanceWith("cloud1.example.com", "internal1.example.com", "10.0.0.1"), + instanceWith("cloud2.example.com", "internal2.example.com", "10.0.0.2"), + instanceWith("cloud3.example.com", "internal3.example.com", "10.0.0.3"), + } + + out := g.Generate(globalCfg, instances) + + for i, inst := range instances { + ip := []string{"10.0.0.1", "10.0.0.2", "10.0.0.3"}[i] + if !strings.Contains(out, fmt.Sprintf("address=/%s/%s", inst.Cloud.Domain, ip)) { + t.Errorf("expected address entry for %s, got:\n%s", inst.Cloud.Domain, out) + } + if !strings.Contains(out, fmt.Sprintf("address=/%s/%s", inst.Cloud.InternalDomain, ip)) { + t.Errorf("expected address entry for %s, got:\n%s", inst.Cloud.InternalDomain, out) + } + if !strings.Contains(out, fmt.Sprintf("local=/%s/", inst.Cloud.InternalDomain)) { + t.Errorf("expected local=/ entry for %s, got:\n%s", inst.Cloud.InternalDomain, out) + } + } +} + +// Test: nil config doesn't panic, uses auto-detect for IP +func TestGenerate_NilConfig(t *testing.T) { + g := NewConfigGenerator("/tmp/test-dnsmasq.conf") + + // Should not panic with nil config + out := g.Generate(nil, []config.InstanceConfig{}) + + if !strings.Contains(out, "bind-interfaces") { + t.Errorf("expected valid config with nil cfg, got:\n%s", out) + } + if !strings.Contains(out, "server=1.1.1.1") { + t.Errorf("expected upstream DNS server with nil cfg, got:\n%s", out) + } +} + +// Test: when cfg.Cloud.Dnsmasq.IP is set, uses that instead of auto-detect +func TestGenerate_ConfiguredDnsmasqIP(t *testing.T) { + g := NewConfigGenerator("/tmp/test-dnsmasq.conf") + cfg := &config.GlobalConfig{} + cfg.Cloud.Dnsmasq.IP = "192.168.8.100" + + out := g.Generate(cfg, []config.InstanceConfig{}) + + if !strings.Contains(out, "listen-address=192.168.8.100") { + t.Errorf("expected configured dnsmasq IP in listen-address, got:\n%s", out) + } +} + +// Test: verify no address=// or local=// entries appear in any test output +func TestGenerate_NoLeakedEmptyEntries(t *testing.T) { + g := NewConfigGenerator("/tmp/test-dnsmasq.conf") + cfg := &config.GlobalConfig{} + + cases := []struct { + name string + instances []config.InstanceConfig + }{ + {"empty", nil}, + {"domain_only", []config.InstanceConfig{instanceWith("example.com", "", "10.0.0.1")}}, + {"internal_only", []config.InstanceConfig{instanceWith("", "internal.example.com", "10.0.0.1")}}, + {"both", []config.InstanceConfig{instanceWith("example.com", "internal.example.com", "10.0.0.1")}}, + {"no_lb_ip", []config.InstanceConfig{instanceWith("example.com", "internal.example.com", "")}}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + out := g.Generate(cfg, tc.instances) + if strings.Contains(out, "address=//") { + t.Errorf("leaked empty address=// in output:\n%s", out) + } + if strings.Contains(out, "local=//") { + t.Errorf("leaked empty local=// in output:\n%s", out) + } + }) + } +} + +// Test: GenerateInstanceConfig with all fields empty doesn't produce broken entries +func TestGenerateInstanceConfig_EmptyFields(t *testing.T) { + g := NewConfigGenerator("/tmp/test-dnsmasq.conf") + var inst config.InstanceConfig + // All fields empty: no domain, no internal domain, no LB IP + + out := g.GenerateInstanceConfig(inst) + + if strings.Contains(out, "address=//") { + t.Errorf("empty fields produced broken address=// entry:\n%s", out) + } + if strings.Contains(out, "local=//") { + t.Errorf("empty fields produced broken local=// entry:\n%s", out) + } + // Should still contain the header comment + if !strings.Contains(out, "DNS configuration for instance") { + t.Errorf("expected header comment in output, got:\n%s", out) + } +} diff --git a/internal/haproxy/config_test.go b/internal/haproxy/config_test.go index f32e645..8fab099 100644 --- a/internal/haproxy/config_test.go +++ b/internal/haproxy/config_test.go @@ -424,3 +424,147 @@ func TestGenerateWithOpts_BackwardCompatible(t *testing.T) { t.Errorf("backward compat: unexpected L7 frontend, got:\n%s", out) } } + +func TestGenerateWithOpts_NoRoutes_NoHTTPSFrontend(t *testing.T) { + m := NewManager("") + + // No instances, no HTTP routes + out := m.GenerateWithOpts(nil, nil, GenerateOpts{}) + + if strings.Contains(out, "frontend https_in") { + t.Errorf("expected no HTTPS frontend with empty instances and empty HTTP routes, got:\n%s", out) + } + if strings.Contains(out, "frontend l7_https") { + t.Errorf("expected no L7 frontend with no HTTP routes, got:\n%s", out) + } +} + +func TestGenerateWithOpts_OnlyHTTPRoutes(t *testing.T) { + m := NewManager("") + + httpRoutes := []HTTPRoute{ + {Name: "my-api", Domain: "api.example.com", Backend: "192.168.1.10:9001"}, + } + + out := m.GenerateWithOpts(nil, nil, GenerateOpts{HTTPRoutes: httpRoutes}) + + // HTTPS frontend should be present (needed for SNI dispatch to L7) + if !strings.Contains(out, "frontend https_in") { + t.Errorf("expected HTTPS frontend when HTTP routes present, got:\n%s", out) + } + // L7 frontend should be present + if !strings.Contains(out, "frontend l7_https") { + t.Errorf("expected L7 frontend for HTTP routes, got:\n%s", out) + } + // L7 backend should be present + if !strings.Contains(out, "backend be_l7_my_api") { + t.Errorf("expected L7 backend for my-api, got:\n%s", out) + } + // No L4 instance backends + if strings.Contains(out, "be_https_") { + t.Errorf("unexpected L4 instance backend when only HTTP routes, got:\n%s", out) + } +} + +func TestGenerateWithOpts_OnlyL4Routes(t *testing.T) { + m := NewManager("") + + instances := []InstanceRoute{ + {Name: "cloud", Domain: "cloud.example.com", BackendIP: "10.0.0.1", Subdomains: true}, + } + + out := m.GenerateWithOpts(instances, nil, GenerateOpts{}) + + // HTTPS frontend should exist for L4 SNI routing + if !strings.Contains(out, "frontend https_in") { + t.Errorf("expected HTTPS frontend for L4 routes, got:\n%s", out) + } + // L4 backend should exist + if !strings.Contains(out, "backend be_https_cloud") { + t.Errorf("expected L4 backend, got:\n%s", out) + } + // No L7 frontend when no HTTP routes + if strings.Contains(out, "frontend l7_https") { + t.Errorf("unexpected L7 frontend with only L4 routes, got:\n%s", out) + } + // No L7 termination backend + if strings.Contains(out, "be_l7_termination") { + t.Errorf("unexpected L7 termination backend with only L4 routes, got:\n%s", out) + } +} + +func TestGenerateWithOpts_CertsDir(t *testing.T) { + m := NewManager("") + + httpRoutes := []HTTPRoute{ + {Name: "app", Domain: "app.example.com", Backend: "127.0.0.1:8080"}, + } + + out := m.GenerateWithOpts(nil, nil, GenerateOpts{ + HTTPRoutes: httpRoutes, + CertsDir: "/opt/certs/wildcard/", + }) + + if !strings.Contains(out, "ssl crt /opt/certs/wildcard/") { + t.Errorf("expected custom CertsDir in L7 bind, got:\n%s", out) + } +} + +func TestGenerateWithOpts_DefaultCertsDir(t *testing.T) { + m := NewManager("") + + httpRoutes := []HTTPRoute{ + {Name: "app", Domain: "app.example.com", Backend: "127.0.0.1:8080"}, + } + + // Empty CertsDir should default to /etc/haproxy/certs/ + out := m.GenerateWithOpts(nil, nil, GenerateOpts{ + HTTPRoutes: httpRoutes, + CertsDir: "", + }) + + if !strings.Contains(out, "ssl crt /etc/haproxy/certs/") { + t.Errorf("expected default CertsDir /etc/haproxy/certs/ in L7 bind, got:\n%s", out) + } +} + +func TestGenerate_CustomRoutePorts(t *testing.T) { + m := NewManager("") + + custom := []CustomRoute{ + {Name: "ssh", Port: 2222, Backend: "192.168.1.10:22"}, + {Name: "mqtt", Port: 1883, Backend: "192.168.1.20:1883"}, + } + + out := m.Generate(nil, custom) + + if !strings.Contains(out, "bind *:2222") { + t.Errorf("expected bind on port 2222, got:\n%s", out) + } + if !strings.Contains(out, "bind *:1883") { + t.Errorf("expected bind on port 1883, got:\n%s", out) + } + if !strings.Contains(out, "server s0 192.168.1.10:22") { + t.Errorf("expected SSH backend, got:\n%s", out) + } + if !strings.Contains(out, "server s0 192.168.1.20:1883") { + t.Errorf("expected MQTT backend, got:\n%s", out) + } +} + +func TestGenerateWithOpts_HealthCheckInL7Backend(t *testing.T) { + m := NewManager("") + + httpRoutes := []HTTPRoute{ + {Name: "api", Domain: "api.example.com", Backend: "10.0.0.5:8080", HealthPath: "/healthz"}, + } + + out := m.GenerateWithOpts(nil, nil, GenerateOpts{HTTPRoutes: httpRoutes}) + + if !strings.Contains(out, "option httpchk GET /healthz") { + t.Errorf("expected httpchk directive with health path, got:\n%s", out) + } + if !strings.Contains(out, "server s0 10.0.0.5:8080 check") { + t.Errorf("expected server line with 'check' flag for health check, got:\n%s", out) + } +} diff --git a/internal/services/manager_test.go b/internal/services/manager_test.go index a013452..a2f1f4d 100644 --- a/internal/services/manager_test.go +++ b/internal/services/manager_test.go @@ -1,6 +1,8 @@ package services import ( + "os" + "path/filepath" "testing" ) @@ -324,3 +326,174 @@ func TestDomainToFilename(t *testing.T) { } } } + +func TestGet_NotFound(t *testing.T) { + mgr := NewManager(t.TempDir()) + + _, err := mgr.Get("nonexistent.example.com") + if err == nil { + t.Error("expected error for non-existent domain, got nil") + } +} + +func TestDeregister_NotFound(t *testing.T) { + mgr := NewManager(t.TempDir()) + + err := mgr.Deregister("nonexistent.example.com") + if err == nil { + t.Error("expected error when deregistering non-existent domain, got nil") + } +} + +func TestDeregisterBySource_NoMatch(t *testing.T) { + mgr := NewManager(t.TempDir()) + + // Register some services + mgr.Register(Service{ + Domain: "a.example.com", + Source: "wild-cloud", + Backend: Backend{Address: "10.0.0.1:443", Type: BackendTCPPassthrough}, + Reach: ReachPublic, + }) + mgr.Register(Service{ + Domain: "b.example.com", + Source: "wild-works", + Backend: Backend{Address: "10.0.0.2:8080", Type: BackendHTTP}, + Reach: ReachInternal, + }) + + // Deregister with a source that doesn't match anything + err := mgr.DeregisterBySource("unknown-source", "10.0.0.99:443") + if err != nil { + t.Fatalf("DeregisterBySource with no matches should not error: %v", err) + } + + // All services should still be present + svcs, _ := mgr.List() + if len(svcs) != 2 { + t.Errorf("expected 2 services after no-match deregister, got %d", len(svcs)) + } +} + +func TestDeregisterBySource_PartialMatch(t *testing.T) { + mgr := NewManager(t.TempDir()) + + // Same source, different backends + mgr.Register(Service{ + Domain: "a.example.com", + Source: "wild-cloud", + Backend: Backend{Address: "10.0.0.1:443", Type: BackendTCPPassthrough}, + Reach: ReachPublic, + }) + mgr.Register(Service{ + Domain: "b.example.com", + Source: "wild-cloud", + Backend: Backend{Address: "10.0.0.2:443", Type: BackendTCPPassthrough}, + Reach: ReachPublic, + }) + + // Deregister only the ones matching source AND backend + err := mgr.DeregisterBySource("wild-cloud", "10.0.0.1:443") + if err != nil { + t.Fatalf("DeregisterBySource failed: %v", err) + } + + svcs, _ := mgr.List() + if len(svcs) != 1 { + t.Errorf("expected 1 remaining service, got %d", len(svcs)) + } + if svcs[0].Domain != "b.example.com" { + t.Errorf("expected b.example.com to remain, got %s", svcs[0].Domain) + } +} + +func TestUpdate_NotFound(t *testing.T) { + mgr := NewManager(t.TempDir()) + + err := mgr.Update("nonexistent.example.com", map[string]any{ + "reach": "public", + }) + if err == nil { + t.Error("expected error when updating non-existent domain, got nil") + } +} + +func TestRegister_OverwritePreservesFile(t *testing.T) { + dir := t.TempDir() + mgr := NewManager(dir) + + svc := Service{ + Domain: "overwrite.example.com", + Source: "test", + Backend: Backend{Address: "127.0.0.1:8080", Type: BackendHTTP}, + Reach: ReachInternal, + } + + // Register first time + if err := mgr.Register(svc); err != nil { + t.Fatalf("first Register failed: %v", err) + } + + // Register again with different backend + svc.Backend.Address = "127.0.0.1:9090" + if err := mgr.Register(svc); err != nil { + t.Fatalf("second Register failed: %v", err) + } + + // File count should be exactly 1 + entries, err := os.ReadDir(filepath.Join(dir, "services")) + if err != nil { + t.Fatalf("ReadDir failed: %v", err) + } + if len(entries) != 1 { + t.Errorf("expected 1 file after overwrite, got %d", len(entries)) + } + + // Value should be updated + got, _ := mgr.Get("overwrite.example.com") + if got.Backend.Address != "127.0.0.1:9090" { + t.Errorf("expected updated backend 127.0.0.1:9090, got %s", got.Backend.Address) + } +} + +func TestList_EmptyDir(t *testing.T) { + mgr := NewManager(t.TempDir()) + + svcs, err := mgr.List() + if err != nil { + t.Fatalf("List on empty dir should not error: %v", err) + } + if len(svcs) != 0 { + t.Errorf("expected 0 services from empty dir, got %d", len(svcs)) + } +} + +func TestList_IgnoresNonYAML(t *testing.T) { + dir := t.TempDir() + mgr := NewManager(dir) + + // Register one real service + mgr.Register(Service{ + Domain: "real.example.com", + Source: "test", + Backend: Backend{Address: "127.0.0.1:8080", Type: BackendHTTP}, + Reach: ReachInternal, + }) + + // Create non-YAML files in the services directory + servicesDir := filepath.Join(dir, "services") + os.WriteFile(filepath.Join(servicesDir, "README.md"), []byte("# ignore me"), 0644) + os.WriteFile(filepath.Join(servicesDir, "notes.txt"), []byte("some notes"), 0644) + os.WriteFile(filepath.Join(servicesDir, ".gitkeep"), []byte(""), 0644) + + svcs, err := mgr.List() + if err != nil { + t.Fatalf("List failed: %v", err) + } + if len(svcs) != 1 { + t.Errorf("expected 1 service (ignoring non-YAML files), got %d", len(svcs)) + } + if svcs[0].Domain != "real.example.com" { + t.Errorf("expected real.example.com, got %s", svcs[0].Domain) + } +}