From d813f2396f4166e88ead274db6e4279cdca19444 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Wed, 22 Oct 2025 17:42:52 +0000 Subject: [PATCH] Respect custom ports when discovering Proxmox clusters --- internal/api/config_handlers.go | 81 +++++++++++--- internal/api/config_handlers_cluster_test.go | 108 +++++++++++++++++++ 2 files changed, 174 insertions(+), 15 deletions(-) create mode 100644 internal/api/config_handlers_cluster_test.go diff --git a/internal/api/config_handlers.go b/internal/api/config_handlers.go index 0f4ea5d..5a4b05b 100644 --- a/internal/api/config_handlers.go +++ b/internal/api/config_handlers.go @@ -378,6 +378,62 @@ type NodeResponse struct { ClusterEndpoints []config.ClusterEndpoint `json:"clusterEndpoints,omitempty"` } +// deriveSchemeAndPort infers the scheme (without ://) and port from a base host URL. +// Defaults align with Proxmox expectations when details are omitted. +func deriveSchemeAndPort(baseHost string) (scheme string, port string) { + scheme = "https" + port = "8006" + + baseHost = strings.TrimSpace(baseHost) + if baseHost == "" { + return scheme, port + } + + candidate := baseHost + if !strings.Contains(candidate, "://") { + candidate = "https://" + candidate + } + + parsed, err := url.Parse(candidate) + if err != nil { + return scheme, port + } + + if parsed.Scheme != "" { + scheme = parsed.Scheme + } + + if parsed.Port() != "" { + port = parsed.Port() + } + + return scheme, port +} + +// ensureHostHasPort guarantees that a host string contains an explicit port. +func ensureHostHasPort(host, port string) string { + host = strings.TrimSpace(host) + if host == "" || port == "" { + return host + } + + if _, _, err := net.SplitHostPort(host); err == nil { + return host + } + + if parsed, err := url.Parse(host); err == nil && parsed.Host != "" { + if parsed.Port() != "" { + return parsed.Host + } + host = parsed.Host + } + + trimmed := strings.TrimPrefix(host, "[") + trimmed = strings.TrimSuffix(trimmed, "]") + + return net.JoinHostPort(trimmed, port) +} + // validateNodeAPI tests if a cluster node has a working Proxmox API // This helps filter out qdevice VMs and other non-Proxmox participants func validateNodeAPI(clusterNode proxmox.ClusterStatus, baseConfig proxmox.ClientConfig) bool { @@ -392,11 +448,14 @@ func validateNodeAPI(clusterNode proxmox.ClusterStatus, baseConfig proxmox.Clien return false } + scheme, defaultPort := deriveSchemeAndPort(baseConfig.Host) + // Create a test configuration for this specific node testConfig := baseConfig testConfig.Host = testHost if !strings.HasPrefix(testConfig.Host, "http") { - testConfig.Host = fmt.Sprintf("https://%s:8006", testConfig.Host) + hostWithPort := ensureHostHasPort(testConfig.Host, defaultPort) + testConfig.Host = fmt.Sprintf("%s://%s", scheme, hostWithPort) } // Use a very short timeout for validation - we just need to know if the API exists @@ -532,11 +591,8 @@ func detectPVECluster(clientConfig proxmox.ClientConfig, nodeName string) (isClu Str("node", nodeName). Int("nodes", len(clusterNodes)). Msg("Detected Proxmox cluster") - - scheme := "https://" - if strings.HasPrefix(strings.ToLower(clientConfig.Host), "http://") { - scheme = "http://" - } + scheme, defaultPort := deriveSchemeAndPort(clientConfig.Host) + schemePrefix := scheme + "://" var unvalidatedNodes []proxmox.ClusterStatus @@ -558,15 +614,12 @@ func detectPVECluster(clientConfig proxmox.ClientConfig, nodeName string) (isClu if nodeHost == "" { nodeHost = clusterNode.Name } - // Ensure host has port (PVE uses 8006) - if !strings.Contains(nodeHost, ":") { - nodeHost = nodeHost + ":8006" - } + nodeHost = ensureHostHasPort(nodeHost, defaultPort) endpoint := config.ClusterEndpoint{ NodeID: clusterNode.ID, NodeName: clusterNode.Name, - Host: scheme + nodeHost, + Host: schemePrefix + nodeHost, Online: clusterNode.Online == 1, LastSeen: time.Now(), } @@ -592,14 +645,12 @@ func detectPVECluster(clientConfig proxmox.ClientConfig, nodeName string) (isClu if nodeHost == "" { continue } - if !strings.Contains(nodeHost, ":") { - nodeHost = nodeHost + ":8006" - } + nodeHost = ensureHostHasPort(nodeHost, defaultPort) endpoint := config.ClusterEndpoint{ NodeID: clusterNode.ID, NodeName: clusterNode.Name, - Host: scheme + nodeHost, + Host: schemePrefix + nodeHost, Online: clusterNode.Online == 1, LastSeen: time.Now(), } diff --git a/internal/api/config_handlers_cluster_test.go b/internal/api/config_handlers_cluster_test.go new file mode 100644 index 0000000..e685718 --- /dev/null +++ b/internal/api/config_handlers_cluster_test.go @@ -0,0 +1,108 @@ +package api + +import "testing" + +func TestDeriveSchemeAndPort(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + host string + wantScheme string + wantPort string + }{ + { + name: "https with explicit port", + host: "https://pve1.local:8443", + wantScheme: "https", + wantPort: "8443", + }, + { + name: "http with explicit port", + host: "http://pve1.local:8006", + wantScheme: "http", + wantPort: "8006", + }, + { + name: "https without port falls back", + host: "https://pve1.local", + wantScheme: "https", + wantPort: "8006", + }, + { + name: "host without scheme", + host: "pve1.local:9000", + wantScheme: "https", + wantPort: "9000", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + gotScheme, gotPort := deriveSchemeAndPort(tt.host) + if gotScheme != tt.wantScheme || gotPort != tt.wantPort { + t.Fatalf("deriveSchemeAndPort(%q) = (%q, %q); want (%q, %q)", tt.host, gotScheme, gotPort, tt.wantScheme, tt.wantPort) + } + }) + } +} + +func TestEnsureHostHasPort(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + host string + port string + expected string + }{ + { + name: "hostname without port", + host: "pve1.local", + port: "8443", + expected: "pve1.local:8443", + }, + { + name: "hostname with port", + host: "pve1.local:8006", + port: "8443", + expected: "pve1.local:8006", + }, + { + name: "ipv6 without port", + host: "[2001:db8::1]", + port: "8006", + expected: "[2001:db8::1]:8006", + }, + { + name: "ipv6 with port", + host: "[2001:db8::1]:9000", + port: "8006", + expected: "[2001:db8::1]:9000", + }, + { + name: "host with scheme", + host: "https://pve1.local:8443", + port: "8006", + expected: "pve1.local:8443", + }, + { + name: "empty host", + host: "", + port: "8006", + expected: "", + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + if got := ensureHostHasPort(tt.host, tt.port); got != tt.expected { + t.Fatalf("ensureHostHasPort(%q, %q) = %q; want %q", tt.host, tt.port, got, tt.expected) + } + }) + } +}