diff --git a/internal/hostagent/agent.go b/internal/hostagent/agent.go index e75c0b338..1880fadd4 100644 --- a/internal/hostagent/agent.go +++ b/internal/hostagent/agent.go @@ -70,6 +70,8 @@ type Agent struct { const defaultInterval = 30 * time.Second +var readFile = os.ReadFile + // New constructs a fully initialised host Agent. func New(cfg Config) (*Agent, error) { if cfg.Interval <= 0 { @@ -100,6 +102,7 @@ func New(cfg Config) (*Agent, error) { pulseURL = "http://localhost:7655" } pulseURL = strings.TrimRight(pulseURL, "/") + cfg.PulseURL = pulseURL ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() @@ -650,7 +653,7 @@ func (a *Agent) runProxmoxSetup(ctx context.Context) { // gopsutil to return identical HostIDs for all LXC containers on the same host. func isLXCContainer() bool { // Check systemd-detect-virt if available - if data, err := os.ReadFile("/run/systemd/container"); err == nil { + if data, err := readFile("/run/systemd/container"); err == nil { container := strings.TrimSpace(string(data)) if strings.Contains(container, "lxc") { return true @@ -658,14 +661,14 @@ func isLXCContainer() bool { } // Check /proc/1/environ for container=lxc - if data, err := os.ReadFile("/proc/1/environ"); err == nil { + if data, err := readFile("/proc/1/environ"); err == nil { if strings.Contains(string(data), "container=lxc") { return true } } // Check /proc/1/cgroup for lxc markers - if data, err := os.ReadFile("/proc/1/cgroup"); err == nil { + if data, err := readFile("/proc/1/cgroup"); err == nil { text := string(data) if strings.Contains(text, "/lxc/") || strings.Contains(text, "lxc.payload") { return true @@ -676,15 +679,21 @@ func isLXCContainer() bool { } // getReliableMachineID returns a machine ID that's unique per container/host. -// In LXC containers, gopsutil's HostID may use /sys/class/dmi/id/product_uuid -// which is shared with the host, causing ID collisions. This function detects -// LXC and prefers /etc/machine-id which is unique per container. +// On Linux, /etc/machine-id is always preferred over gopsutil's HostID because: +// - LXC containers share the host's /sys/class/dmi/id/product_uuid +// - Cloned VMs/hosts may share the same DMI product UUID +// - Proxmox cluster nodes with identical hardware may have the same UUID +// The /etc/machine-id file is guaranteed unique per installation. func getReliableMachineID(gopsutilHostID string, logger zerolog.Logger) string { gopsutilID := strings.TrimSpace(gopsutilHostID) - // For LXC containers, prefer /etc/machine-id to avoid ID collisions - if isLXCContainer() { - if data, err := os.ReadFile("/etc/machine-id"); err == nil { + // On Linux, always prefer /etc/machine-id as it's guaranteed unique per installation. + // This avoids ID collisions from: + // - LXC containers sharing host's DMI product UUID + // - Cloned VMs with identical hardware UUIDs + // - Proxmox cluster nodes with same hardware configuration + if runtime.GOOS == "linux" { + if data, err := readFile("/etc/machine-id"); err == nil { machineID := strings.TrimSpace(string(data)) if len(machineID) >= 32 { // Format as UUID if it's a 32-char hex string (like machine-id typically is) @@ -693,9 +702,15 @@ func getReliableMachineID(gopsutilHostID string, logger zerolog.Logger) string { machineID[0:8], machineID[8:12], machineID[12:16], machineID[16:20], machineID[20:32]) } - logger.Debug(). - Str("machineID", machineID). - Msg("LXC container detected, using /etc/machine-id for unique identification") + if isLXCContainer() { + logger.Debug(). + Str("machineID", machineID). + Msg("LXC container detected, using /etc/machine-id for unique identification") + } else { + logger.Debug(). + Str("machineID", machineID). + Msg("Linux host detected, using /etc/machine-id for unique identification") + } return machineID } } diff --git a/internal/hostagent/agent_test.go b/internal/hostagent/agent_test.go index 266a92df9..6cabb551e 100644 --- a/internal/hostagent/agent_test.go +++ b/internal/hostagent/agent_test.go @@ -2,6 +2,7 @@ package hostagent import ( "os" + "runtime" "testing" "github.com/rs/zerolog" @@ -78,37 +79,57 @@ func TestNormalisePlatform(t *testing.T) { func TestGetReliableMachineID(t *testing.T) { logger := zerolog.Nop() - // Check if we're running in an LXC container - inLXC := isLXCContainer() - - t.Run("returns non-empty ID", func(t *testing.T) { - result := getReliableMachineID("test-gopsutil-id", logger) - if result == "" { - t.Error("getReliableMachineID returned empty string") - } - }) + originalReadFile := readFile + t.Cleanup(func() { readFile = originalReadFile }) t.Run("trims whitespace", func(t *testing.T) { + readFile = func(string) ([]byte, error) { return nil, os.ErrNotExist } result := getReliableMachineID(" test-id ", logger) - if result == " test-id " { - t.Error("getReliableMachineID did not trim whitespace") + if result != "test-id" { + t.Errorf("getReliableMachineID trimmed result = %q, want %q", result, "test-id") } }) - if inLXC { - t.Run("LXC uses machine-id", func(t *testing.T) { - // In LXC, we should get a machine-id regardless of gopsutil input - result := getReliableMachineID("gopsutil-product-uuid", logger) - if result == "gopsutil-product-uuid" { - t.Error("In LXC, getReliableMachineID should use /etc/machine-id, not gopsutil ID") + // On Linux, we always use /etc/machine-id to avoid ID collisions + // from LXC containers, cloned VMs, or identical hardware UUIDs + if runtime.GOOS == "linux" { + t.Run("Linux prefers /etc/machine-id and formats 32-char IDs", func(t *testing.T) { + readFile = func(name string) ([]byte, error) { + if name == "/etc/machine-id" { + return []byte("0123456789abcdef0123456789abcdef\n"), nil + } + return nil, os.ErrNotExist } - // Verify it looks like a formatted UUID - if len(result) < 32 { - t.Errorf("Expected UUID-like result, got %q", result) + + result := getReliableMachineID("gopsutil-product-uuid", logger) + const want = "01234567-89ab-cdef-0123-456789abcdef" + if result != want { + t.Errorf("getReliableMachineID() = %q, want %q", result, want) + } + }) + + t.Run("Linux falls back to gopsutil ID when /etc/machine-id missing", func(t *testing.T) { + readFile = func(string) ([]byte, error) { return nil, os.ErrNotExist } + result := getReliableMachineID("gopsutil-product-uuid", logger) + if result != "gopsutil-product-uuid" { + t.Errorf("getReliableMachineID() = %q, want %q", result, "gopsutil-product-uuid") + } + }) + + t.Run("Linux falls back when machine-id is too short", func(t *testing.T) { + readFile = func(name string) ([]byte, error) { + if name == "/etc/machine-id" { + return []byte("short\n"), nil + } + return nil, os.ErrNotExist + } + result := getReliableMachineID("gopsutil-product-uuid", logger) + if result != "gopsutil-product-uuid" { + t.Errorf("getReliableMachineID() = %q, want %q", result, "gopsutil-product-uuid") } }) } else { - t.Run("non-LXC uses gopsutil ID", func(t *testing.T) { + t.Run("non-Linux uses gopsutil ID", func(t *testing.T) { result := getReliableMachineID("12345678-1234-1234-1234-123456789abc", logger) if result != "12345678-1234-1234-1234-123456789abc" { t.Errorf("Expected gopsutil ID, got %q", result) @@ -118,20 +139,60 @@ func TestGetReliableMachineID(t *testing.T) { } func TestIsLXCContainer(t *testing.T) { - // This test documents the detection behavior. - // On non-LXC systems, isLXCContainer should return false. - // We can't easily test the true case without mocking filesystem. - result := isLXCContainer() + originalReadFile := readFile + t.Cleanup(func() { readFile = originalReadFile }) - // Check if we're actually in an LXC container - isActuallyLXC := false - if data, err := os.ReadFile("/run/systemd/container"); err == nil { - if string(data) == "lxc" || string(data) == "lxc\n" { - isActuallyLXC = true + t.Run("/run/systemd/container detects lxc", func(t *testing.T) { + readFile = func(name string) ([]byte, error) { + if name == "/run/systemd/container" { + return []byte("lxc\n"), nil + } + return nil, os.ErrNotExist } - } + if !isLXCContainer() { + t.Fatalf("expected lxc container to be detected") + } + }) - if result != isActuallyLXC { - t.Logf("isLXCContainer() = %v (expected %v based on environment)", result, isActuallyLXC) - } + t.Run("/proc/1/environ detects container=lxc", func(t *testing.T) { + readFile = func(name string) ([]byte, error) { + if name == "/proc/1/environ" { + return []byte("foo=bar\x00container=lxc\x00baz=qux"), nil + } + return nil, os.ErrNotExist + } + if !isLXCContainer() { + t.Fatalf("expected lxc container to be detected") + } + }) + + t.Run("/proc/1/cgroup detects /lxc/", func(t *testing.T) { + readFile = func(name string) ([]byte, error) { + if name == "/proc/1/cgroup" { + return []byte("0::/lxc/abcd\n"), nil + } + return nil, os.ErrNotExist + } + if !isLXCContainer() { + t.Fatalf("expected lxc container to be detected") + } + }) + + t.Run("non-lxc container returns false", func(t *testing.T) { + readFile = func(name string) ([]byte, error) { + if name == "/run/systemd/container" { + return []byte("docker\n"), nil + } + if name == "/proc/1/environ" { + return []byte("container=podman\x00"), nil + } + if name == "/proc/1/cgroup" { + return []byte("0::/system.slice\n"), nil + } + return nil, os.ErrNotExist + } + if isLXCContainer() { + t.Fatalf("expected non-lxc container to not be detected as lxc") + } + }) } diff --git a/internal/hostagent/command_client_test.go b/internal/hostagent/command_client_test.go new file mode 100644 index 000000000..a78bb1174 --- /dev/null +++ b/internal/hostagent/command_client_test.go @@ -0,0 +1,116 @@ +package hostagent + +import ( + "io" + "testing" + + "github.com/rs/zerolog" +) + +func TestNew_DefaultPulseURLUsedForCommandClient(t *testing.T) { + logger := zerolog.New(io.Discard) + + agent, err := New(Config{ + APIToken: "test-token", + LogLevel: zerolog.InfoLevel, + Logger: &logger, + }) + if err != nil { + t.Fatalf("New: %v", err) + } + + const want = "http://localhost:7655" + if agent.trimmedPulseURL != want { + t.Fatalf("trimmedPulseURL = %q, want %q", agent.trimmedPulseURL, want) + } + if agent.cfg.PulseURL != want { + t.Fatalf("cfg.PulseURL = %q, want %q", agent.cfg.PulseURL, want) + } + if agent.commandClient == nil { + t.Fatalf("commandClient should be initialized") + } + if agent.commandClient.pulseURL != want { + t.Fatalf("commandClient.pulseURL = %q, want %q", agent.commandClient.pulseURL, want) + } +} + +func TestNew_TrimsPulseURLForCommandClient(t *testing.T) { + logger := zerolog.New(io.Discard) + + agent, err := New(Config{ + PulseURL: "https://example.invalid/", + APIToken: "test-token", + LogLevel: zerolog.InfoLevel, + Logger: &logger, + }) + if err != nil { + t.Fatalf("New: %v", err) + } + + const want = "https://example.invalid" + if agent.trimmedPulseURL != want { + t.Fatalf("trimmedPulseURL = %q, want %q", agent.trimmedPulseURL, want) + } + if agent.cfg.PulseURL != want { + t.Fatalf("cfg.PulseURL = %q, want %q", agent.cfg.PulseURL, want) + } + if agent.commandClient == nil { + t.Fatalf("commandClient should be initialized") + } + if agent.commandClient.pulseURL != want { + t.Fatalf("commandClient.pulseURL = %q, want %q", agent.commandClient.pulseURL, want) + } +} + +func TestCommandClientBuildWebSocketURL(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + pulseURL string + want string + wantErr bool + }{ + { + name: "https becomes wss", + pulseURL: "https://example.invalid", + want: "wss://example.invalid/api/agent/ws", + }, + { + name: "http becomes ws", + pulseURL: "http://example.invalid", + want: "ws://example.invalid/api/agent/ws", + }, + { + name: "ws preserved", + pulseURL: "ws://example.invalid", + want: "ws://example.invalid/api/agent/ws", + }, + { + name: "wss preserved", + pulseURL: "wss://example.invalid", + want: "wss://example.invalid/api/agent/ws", + }, + { + name: "invalid url returns error", + pulseURL: "http://[::1", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client := &CommandClient{pulseURL: tt.pulseURL} + got, err := client.buildWebSocketURL() + if (err != nil) != tt.wantErr { + t.Fatalf("buildWebSocketURL() err = %v, wantErr %v", err, tt.wantErr) + } + if tt.wantErr { + return + } + if got != tt.want { + t.Fatalf("buildWebSocketURL() = %q, want %q", got, tt.want) + } + }) + } +} diff --git a/internal/hostagent/proxmox_setup.go b/internal/hostagent/proxmox_setup.go index 5d021ace3..e05cf148f 100644 --- a/internal/hostagent/proxmox_setup.go +++ b/internal/hostagent/proxmox_setup.go @@ -36,12 +36,12 @@ type ProxmoxSetupResult struct { } const ( - proxmoxUser = "pulse-monitor" - proxmoxUserPVE = "pulse-monitor@pam" - proxmoxUserPBS = "pulse-monitor@pbs" - proxmoxComment = "Pulse monitoring service" - stateFilePath = "/var/lib/pulse-agent/proxmox-registered" - stateFileDir = "/var/lib/pulse-agent" + proxmoxUser = "pulse-monitor" + proxmoxUserPVE = "pulse-monitor@pam" + proxmoxUserPBS = "pulse-monitor@pbs" + proxmoxComment = "Pulse monitoring service" + stateFilePath = "/var/lib/pulse-agent/proxmox-registered" + stateFileDir = "/var/lib/pulse-agent" ) // NewProxmoxSetup creates a new ProxmoxSetup instance. @@ -246,23 +246,20 @@ func (p *ProxmoxSetup) parsePBSTokenValue(output string) string { } // getHostURL constructs the host URL for this Proxmox node. -// Prefers IP address over hostname since hostnames are often not DNS-resolvable. +// Uses intelligent IP selection to prefer LAN addresses over internal cluster networks. func (p *ProxmoxSetup) getHostURL(ptype string) string { port := "8006" if ptype == "pbs" { port = "8007" } - // Always prefer IP address since hostnames often can't be resolved by Pulse - // (e.g., "pi" hostname won't resolve via DNS) + // Get all IPs and select the best one for external access if out, err := exec.Command("hostname", "-I").Output(); err == nil { ips := strings.Fields(string(out)) if len(ips) > 0 { - // Use first non-loopback IP - for _, ip := range ips { - if ip != "127.0.0.1" && ip != "::1" { - return fmt.Sprintf("https://%s:%s", ip, port) - } + bestIP := selectBestIP(ips) + if bestIP != "" { + return fmt.Sprintf("https://%s:%s", bestIP, port) } } } @@ -275,6 +272,95 @@ func (p *ProxmoxSetup) getHostURL(ptype string) string { return fmt.Sprintf("https://%s:%s", hostname, port) } +// selectBestIP picks the most likely externally-reachable IP from a list. +// Prefers common LAN subnets (192.168.x.x, 10.x.x.x) and avoids internal +// cluster networks (like corosync's 172.20.x.x) and link-local addresses. +func selectBestIP(ips []string) string { + type scoredIP struct { + ip string + score int + } + + var candidates []scoredIP + + for _, ip := range ips { + // Skip loopback and IPv6 link-local + if ip == "127.0.0.1" || ip == "::1" || strings.HasPrefix(ip, "fe80:") { + continue + } + + // Skip IPv6 for simplicity (most Proxmox setups use IPv4) + if strings.Contains(ip, ":") { + continue + } + + score := scoreIPv4(ip) + if score > 0 { + candidates = append(candidates, scoredIP{ip: ip, score: score}) + } + } + + if len(candidates) == 0 { + // Fallback: return first non-loopback IP if no good candidates + for _, ip := range ips { + if ip != "127.0.0.1" && ip != "::1" && !strings.HasPrefix(ip, "fe80:") { + return ip + } + } + return "" + } + + // Return highest scored IP + best := candidates[0] + for _, c := range candidates[1:] { + if c.score > best.score { + best = c + } + } + return best.ip +} + +// scoreIPv4 assigns a preference score to an IPv4 address. +// Higher score = more likely to be externally reachable. +func scoreIPv4(ip string) int { + parts := strings.Split(ip, ".") + if len(parts) != 4 { + return 0 + } + + // Parse first two octets + first := 0 + second := 0 + fmt.Sscanf(parts[0], "%d", &first) + fmt.Sscanf(parts[1], "%d", &second) + + // Scoring logic: + // - 192.168.x.x: Very common home/office LAN, high priority (score 100) + // - 10.0.x.x - 10.31.x.x: Common corporate LAN ranges (score 90) + // - 10.x.x.x (other): Less common, but still likely LAN (score 70) + // - 172.16-31.x.x: Private range, often used for internal clusters (score 50) + // Corosync often uses 172.20.x.x or similar for ring0/ring1 + // - 169.254.x.x: Link-local, skip (score 0) + // - Other private/public: Unknown, low priority (score 30) + + switch { + case first == 192 && second == 168: + return 100 // Most common home/office LAN + case first == 10 && second <= 31: + return 90 // Common corporate LAN + case first == 10: + return 70 // Other 10.x.x.x + case first == 172 && second >= 16 && second <= 31: + // This is private 172.16-31.x.x range, often used for internal clusters + // Corosync commonly uses 172.20.x.x for cluster communication + return 50 // Lower priority - often internal cluster + case first == 169 && second == 254: + return 0 // Link-local, skip + default: + return 30 // Unknown/public - low priority + } +} + // registerWithPulse calls the auto-register endpoint to add the node. func (p *ProxmoxSetup) registerWithPulse(ctx context.Context, ptype, hostURL, tokenID, tokenValue string) error { payload := map[string]interface{}{ diff --git a/internal/hostagent/proxmox_setup_test.go b/internal/hostagent/proxmox_setup_test.go new file mode 100644 index 000000000..dd572696b --- /dev/null +++ b/internal/hostagent/proxmox_setup_test.go @@ -0,0 +1,134 @@ +package hostagent + +import "testing" + +func TestSelectBestIP(t *testing.T) { + tests := []struct { + name string + ips []string + expected string + }{ + { + name: "prefers 192.168.x.x over corosync 172.20.x.x", + ips: []string{"172.20.0.80", "192.168.1.100"}, + expected: "192.168.1.100", + }, + { + name: "prefers 192.168.x.x even when listed second", + ips: []string{"10.0.0.1", "192.168.0.1"}, + expected: "192.168.0.1", + }, + { + name: "prefers 10.x.x.x over 172.16-31.x.x", + ips: []string{"172.20.0.1", "10.1.10.5"}, + expected: "10.1.10.5", + }, + { + name: "handles single IP", + ips: []string{"192.168.1.1"}, + expected: "192.168.1.1", + }, + { + name: "skips loopback", + ips: []string{"127.0.0.1", "192.168.1.1"}, + expected: "192.168.1.1", + }, + { + name: "skips IPv6 loopback", + ips: []string{"::1", "10.0.0.1"}, + expected: "10.0.0.1", + }, + { + name: "skips link-local IPv6", + ips: []string{"fe80::1", "192.168.1.1"}, + expected: "192.168.1.1", + }, + { + name: "skips link-local IPv4", + ips: []string{"169.254.1.1", "10.0.0.1"}, + expected: "10.0.0.1", + }, + { + name: "returns corosync IP if only option", + ips: []string{"127.0.0.1", "172.20.0.80"}, + expected: "172.20.0.80", + }, + { + name: "empty list returns empty", + ips: []string{}, + expected: "", + }, + { + name: "only loopback returns empty", + ips: []string{"127.0.0.1", "::1"}, + expected: "", + }, + { + name: "common 10.1.x.x LAN preferred over 172.x.x", + ips: []string{"172.16.0.1", "10.1.10.50"}, + expected: "10.1.10.50", + }, + { + name: "prefers 10.0.x.x to 10.100.x.x (common ranges first)", + ips: []string{"10.100.0.1", "10.0.0.1"}, + expected: "10.0.0.1", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := selectBestIP(tt.ips) + if result != tt.expected { + t.Errorf("selectBestIP(%v) = %q, want %q", tt.ips, result, tt.expected) + } + }) + } +} + +func TestScoreIPv4(t *testing.T) { + tests := []struct { + ip string + expectedScore int + }{ + // 192.168.x.x - highest priority (100) + {"192.168.1.1", 100}, + {"192.168.0.100", 100}, + {"192.168.255.255", 100}, + + // 10.0-31.x.x - common corporate (90) + {"10.0.0.1", 90}, + {"10.1.10.5", 90}, + {"10.31.255.255", 90}, + + // 10.32+.x.x - less common (70) + {"10.32.0.1", 70}, + {"10.100.0.1", 70}, + {"10.255.255.255", 70}, + + // 172.16-31.x.x - private but often cluster (50) + {"172.16.0.1", 50}, + {"172.20.0.80", 50}, // Corosync typical + {"172.31.255.255", 50}, + + // 169.254.x.x - link-local (0) + {"169.254.1.1", 0}, + + // Other/public (30) + {"8.8.8.8", 30}, + {"1.1.1.1", 30}, + {"203.0.113.1", 30}, + + // Invalid + {"not-an-ip", 0}, + {"", 0}, + } + + for _, tt := range tests { + t.Run(tt.ip, func(t *testing.T) { + result := scoreIPv4(tt.ip) + if result != tt.expectedScore { + t.Errorf("scoreIPv4(%q) = %d, want %d", tt.ip, result, tt.expectedScore) + } + }) + } +} diff --git a/internal/hostagent/send_report_test.go b/internal/hostagent/send_report_test.go new file mode 100644 index 000000000..0e2dfcc65 --- /dev/null +++ b/internal/hostagent/send_report_test.go @@ -0,0 +1,119 @@ +package hostagent + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "sync" + "testing" + + agentshost "github.com/rcourtman/pulse-go-rewrite/pkg/agents/host" +) + +func TestAgentSendReport_SetsHeadersAndPostsJSON(t *testing.T) { + t.Parallel() + + type received struct { + method string + path string + authorization string + apiToken string + contentType string + userAgent string + body agentshost.Report + } + + var ( + mu sync.Mutex + got received + ) + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var report agentshost.Report + if err := json.NewDecoder(r.Body).Decode(&report); err != nil { + http.Error(w, "bad json", http.StatusBadRequest) + return + } + + mu.Lock() + got = received{ + method: r.Method, + path: r.URL.Path, + authorization: r.Header.Get("Authorization"), + apiToken: r.Header.Get("X-API-Token"), + contentType: r.Header.Get("Content-Type"), + userAgent: r.Header.Get("User-Agent"), + body: report, + } + mu.Unlock() + + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + agent := &Agent{ + cfg: Config{APIToken: "test-token"}, + httpClient: server.Client(), + trimmedPulseURL: server.URL, + } + + wantReport := agentshost.Report{ + Agent: agentshost.AgentInfo{ID: "agent-1"}, + Host: agentshost.HostInfo{Hostname: "test-host"}, + } + + if err := agent.sendReport(context.Background(), wantReport); err != nil { + t.Fatalf("sendReport: %v", err) + } + + mu.Lock() + defer mu.Unlock() + + if got.method != http.MethodPost { + t.Fatalf("method = %q, want %q", got.method, http.MethodPost) + } + if got.path != "/api/agents/host/report" { + t.Fatalf("path = %q, want %q", got.path, "/api/agents/host/report") + } + if got.authorization != "Bearer test-token" { + t.Fatalf("Authorization = %q, want %q", got.authorization, "Bearer test-token") + } + if got.apiToken != "test-token" { + t.Fatalf("X-API-Token = %q, want %q", got.apiToken, "test-token") + } + if got.contentType != "application/json" { + t.Fatalf("Content-Type = %q, want %q", got.contentType, "application/json") + } + if got.userAgent == "" { + t.Fatalf("User-Agent should be set") + } + if got.body.Agent.ID != wantReport.Agent.ID { + t.Fatalf("decoded report Agent.ID = %q, want %q", got.body.Agent.ID, wantReport.Agent.ID) + } + if got.body.Host.Hostname != wantReport.Host.Hostname { + t.Fatalf("decoded report Host.Hostname = %q, want %q", got.body.Host.Hostname, wantReport.Host.Hostname) + } +} + +func TestAgentSendReport_Non2xxReturnsError(t *testing.T) { + t.Parallel() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer server.Close() + + agent := &Agent{ + cfg: Config{APIToken: "test-token"}, + httpClient: server.Client(), + trimmedPulseURL: server.URL, + } + + err := agent.sendReport(context.Background(), agentshost.Report{ + Agent: agentshost.AgentInfo{ID: "agent-1"}, + }) + if err == nil { + t.Fatalf("expected error for non-2xx response") + } +} diff --git a/pkg/discovery/discovery_result_test.go b/pkg/discovery/discovery_result_test.go new file mode 100644 index 000000000..20c961b83 --- /dev/null +++ b/pkg/discovery/discovery_result_test.go @@ -0,0 +1,66 @@ +package discovery + +import ( + "testing" +) + +func TestDiscoveryResultAddError_PopulatesStructuredAndLegacy(t *testing.T) { + t.Parallel() + + var r DiscoveryResult + r.AddError("docker_bridge_network", "timeout", "request timed out", "192.168.1.10", 8006) + + if len(r.StructuredErrors) != 1 { + t.Fatalf("StructuredErrors len = %d, want %d", len(r.StructuredErrors), 1) + } + if len(r.Errors) != 1 { + t.Fatalf("Errors len = %d, want %d", len(r.Errors), 1) + } + + se := r.StructuredErrors[0] + if se.Phase != "docker_bridge_network" { + t.Fatalf("StructuredErrors[0].Phase = %q, want %q", se.Phase, "docker_bridge_network") + } + if se.ErrorType != "timeout" { + t.Fatalf("StructuredErrors[0].ErrorType = %q, want %q", se.ErrorType, "timeout") + } + if se.Message != "request timed out" { + t.Fatalf("StructuredErrors[0].Message = %q, want %q", se.Message, "request timed out") + } + if se.IP != "192.168.1.10" || se.Port != 8006 { + t.Fatalf("StructuredErrors[0] address = %s:%d, want %s:%d", se.IP, se.Port, "192.168.1.10", 8006) + } + if se.Timestamp.IsZero() { + t.Fatalf("StructuredErrors[0].Timestamp should be set") + } + + if r.Errors[0] != "Docker bridge network [192.168.1.10:8006]: request timed out" { + t.Fatalf("Errors[0] = %q", r.Errors[0]) + } +} + +func TestDiscoveryResultAddError_LegacyFormattingVariants(t *testing.T) { + t.Parallel() + + t.Run("ip-only", func(t *testing.T) { + var r DiscoveryResult + r.AddError("extra_targets", "phase_error", "failed", "10.0.0.1", 0) + if len(r.Errors) != 1 { + t.Fatalf("Errors len = %d, want %d", len(r.Errors), 1) + } + if r.Errors[0] != "Additional targets [10.0.0.1]: failed" { + t.Fatalf("Errors[0] = %q", r.Errors[0]) + } + }) + + t.Run("no-address", func(t *testing.T) { + var r DiscoveryResult + r.AddError("unknown_phase", "phase_error", "failed", "", 0) + if len(r.Errors) != 1 { + t.Fatalf("Errors len = %d, want %d", len(r.Errors), 1) + } + if r.Errors[0] != "unknown_phase: failed" { + t.Fatalf("Errors[0] = %q", r.Errors[0]) + } + }) +}