From 17102706ae55aab475446ecc43bc562d305321cd Mon Sep 17 00:00:00 2001 From: rcourtman Date: Thu, 20 Nov 2025 16:35:08 +0000 Subject: [PATCH] Related to #727: restore default Proxmox ports --- internal/api/config_handlers.go | 9 ++- internal/api/config_handlers_host_test.go | 52 +++++++-------- internal/config/client_helpers.go | 50 ++++++++++++-- internal/config/client_helpers_test.go | 79 +++++++++++++++++++++++ 4 files changed, 153 insertions(+), 37 deletions(-) create mode 100644 internal/config/client_helpers_test.go diff --git a/internal/api/config_handlers.go b/internal/api/config_handlers.go index a0b82818e..a1e15abe1 100644 --- a/internal/api/config_handlers.go +++ b/internal/api/config_handlers.go @@ -1097,16 +1097,15 @@ func defaultPortForNodeType(nodeType string) string { } } -// normalizeNodeHost ensures hosts always include a scheme and only adds a default port -// when the user did not supply a scheme (e.g., bare hostname/IP inputs). If the user -// explicitly provides http/https, we respect their choice and avoid forcing a port. +// normalizeNodeHost ensures hosts always include a scheme and default port when one +// isn't provided. Defaults align with Proxmox APIs (PVE/PMG: 8006, PBS: 8007) while +// preserving any explicit scheme/port the user supplies. func normalizeNodeHost(rawHost, nodeType string) (string, error) { host := strings.TrimSpace(rawHost) if host == "" { return "", fmt.Errorf("host is required") } - userProvidedScheme := strings.HasPrefix(host, "http://") || strings.HasPrefix(host, "https://") scheme := "https" if strings.HasPrefix(host, "http://") { scheme = "http" @@ -1137,7 +1136,7 @@ func normalizeNodeHost(rawHost, nodeType string) (string, error) { parsed.RawQuery = "" parsed.Fragment = "" - if parsed.Port() == "" && !userProvidedScheme { + if parsed.Port() == "" { defaultPort := defaultPortForNodeType(nodeType) if defaultPort != "" { parsed.Host = net.JoinHostPort(parsed.Hostname(), defaultPort) diff --git a/internal/api/config_handlers_host_test.go b/internal/api/config_handlers_host_test.go index 13bc6f1e0..8bf17299b 100644 --- a/internal/api/config_handlers_host_test.go +++ b/internal/api/config_handlers_host_test.go @@ -6,19 +6,19 @@ func TestNormalizeNodeHost(t *testing.T) { tests := []struct { name string rawHost string - nodeType string - want string - }{ - { - name: "keeps explicit https without port", - rawHost: "https://example.com", - nodeType: "pve", - want: "https://example.com", - }, - { - name: "adds default port for bare pve host", - rawHost: "pve.lan", - nodeType: "pve", + nodeType string + want string +}{ + { + name: "adds default port to explicit https without port", + rawHost: "https://example.com", + nodeType: "pve", + want: "https://example.com:8006", + }, + { + name: "adds default port for bare pve host", + rawHost: "pve.lan", + nodeType: "pve", want: "https://pve.lan:8006", }, { @@ -39,19 +39,19 @@ func TestNormalizeNodeHost(t *testing.T) { nodeType: "pmg", want: "https://[2001:db8::1]:8006", }, - { - name: "drops path segments", - rawHost: "https://example.com/api", - nodeType: "pve", - want: "https://example.com", - }, - { - name: "respects explicit http without forcing a port", - rawHost: "http://example.com", - nodeType: "pve", - want: "http://example.com", - }, - } + { + name: "drops path segments", + rawHost: "https://example.com/api", + nodeType: "pve", + want: "https://example.com:8006", + }, + { + name: "adds default port to explicit http scheme", + rawHost: "http://example.com", + nodeType: "pve", + want: "http://example.com:8006", + }, +} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/internal/config/client_helpers.go b/internal/config/client_helpers.go index d3f598a31..d0f9ddbd7 100644 --- a/internal/config/client_helpers.go +++ b/internal/config/client_helpers.go @@ -1,6 +1,8 @@ package config import ( + "net" + "net/url" "strings" "github.com/rcourtman/pulse-go-rewrite/pkg/pbs" @@ -8,6 +10,42 @@ import ( "github.com/rcourtman/pulse-go-rewrite/pkg/proxmox" ) +const ( + defaultPVEPort = "8006" + defaultPBSPort = "8007" +) + +// normalizeHostPort ensures we always have a scheme and explicit port when talking to +// Proxmox APIs. It preserves existing ports/schemes and strips any path/query segments. +func normalizeHostPort(host, defaultPort string) string { + trimmed := strings.TrimSpace(host) + if trimmed == "" || defaultPort == "" { + return trimmed + } + + candidate := trimmed + if !strings.HasPrefix(candidate, "http://") && !strings.HasPrefix(candidate, "https://") { + candidate = "https://" + candidate + } + + parsed, err := url.Parse(candidate) + if err != nil || parsed.Host == "" { + return trimmed + } + + // Drop any path fragments so we only persist host:port + parsed.Path = "" + parsed.RawPath = "" + parsed.RawQuery = "" + parsed.Fragment = "" + + if parsed.Port() == "" { + parsed.Host = net.JoinHostPort(parsed.Hostname(), defaultPort) + } + + return parsed.Scheme + "://" + parsed.Host +} + // CreateProxmoxConfig creates a proxmox.ClientConfig from a PVEInstance func CreateProxmoxConfig(node *PVEInstance) proxmox.ClientConfig { user := node.User @@ -16,7 +54,7 @@ func CreateProxmoxConfig(node *PVEInstance) proxmox.ClientConfig { } return proxmox.ClientConfig{ - Host: node.Host, + Host: normalizeHostPort(node.Host, defaultPVEPort), User: user, Password: node.Password, TokenName: node.TokenName, @@ -29,7 +67,7 @@ func CreateProxmoxConfig(node *PVEInstance) proxmox.ClientConfig { // CreatePBSConfig creates a pbs.ClientConfig from a PBSInstance func CreatePBSConfig(node *PBSInstance) pbs.ClientConfig { return pbs.ClientConfig{ - Host: node.Host, + Host: normalizeHostPort(node.Host, defaultPBSPort), User: node.User, Password: node.Password, TokenName: node.TokenName, @@ -42,7 +80,7 @@ func CreatePBSConfig(node *PBSInstance) pbs.ClientConfig { // CreatePMGConfig creates a pmg.ClientConfig from a PMGInstance func CreatePMGConfig(node *PMGInstance) pmg.ClientConfig { return pmg.ClientConfig{ - Host: node.Host, + Host: normalizeHostPort(node.Host, defaultPVEPort), User: node.User, Password: node.Password, TokenName: node.TokenName, @@ -59,7 +97,7 @@ func CreateProxmoxConfigFromFields(host, user, password, tokenName, tokenValue, } return proxmox.ClientConfig{ - Host: host, + Host: normalizeHostPort(host, defaultPVEPort), User: user, Password: password, TokenName: tokenName, @@ -72,7 +110,7 @@ func CreateProxmoxConfigFromFields(host, user, password, tokenName, tokenValue, // CreatePBSConfigFromFields creates a pbs.ClientConfig from individual fields func CreatePBSConfigFromFields(host, user, password, tokenName, tokenValue, fingerprint string, verifySSL bool) pbs.ClientConfig { return pbs.ClientConfig{ - Host: host, + Host: normalizeHostPort(host, defaultPBSPort), User: user, Password: password, TokenName: tokenName, @@ -85,7 +123,7 @@ func CreatePBSConfigFromFields(host, user, password, tokenName, tokenValue, fing // CreatePMGConfigFromFields creates a pmg.ClientConfig from individual fields func CreatePMGConfigFromFields(host, user, password, tokenName, tokenValue, fingerprint string, verifySSL bool) pmg.ClientConfig { return pmg.ClientConfig{ - Host: host, + Host: normalizeHostPort(host, defaultPVEPort), User: user, Password: password, TokenName: tokenName, diff --git a/internal/config/client_helpers_test.go b/internal/config/client_helpers_test.go new file mode 100644 index 000000000..e414a05ea --- /dev/null +++ b/internal/config/client_helpers_test.go @@ -0,0 +1,79 @@ +package config + +import "testing" + +func TestNormalizeHostPort(t *testing.T) { + tests := []struct { + name string + host string + defaultPort string + want string + }{ + { + name: "adds default port to https host", + host: "https://example.com", + defaultPort: defaultPVEPort, + want: "https://example.com:8006", + }, + { + name: "adds default port when scheme missing", + host: "pbs.lan", + defaultPort: defaultPBSPort, + want: "https://pbs.lan:8007", + }, + { + name: "preserves existing port and scheme", + host: "http://example.com:8443", + defaultPort: defaultPVEPort, + want: "http://example.com:8443", + }, + { + name: "drops path segments before applying port", + host: "https://example.com/api", + defaultPort: defaultPVEPort, + want: "https://example.com:8006", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := normalizeHostPort(tt.host, tt.defaultPort); got != tt.want { + t.Fatalf("normalizeHostPort(%q, %q) = %q, want %q", tt.host, tt.defaultPort, got, tt.want) + } + }) + } +} + +func TestCreateConfigHelpersNormalizeHosts(t *testing.T) { + pveCfg := PVEInstance{ + Host: "https://pve.local", + User: "root@pam", + } + pveClientCfg := CreateProxmoxConfig(&pveCfg) + if pveClientCfg.Host != "https://pve.local:8006" { + t.Fatalf("expected default port on PVE host, got %q", pveClientCfg.Host) + } + + pbsCfg := PBSInstance{ + Host: "pbs.local", + User: "root@pam", + } + pbsClientCfg := CreatePBSConfig(&pbsCfg) + if pbsClientCfg.Host != "https://pbs.local:8007" { + t.Fatalf("expected default PBS port on host, got %q", pbsClientCfg.Host) + } + + pmgCfg := PMGInstance{ + Host: "https://pmg.local/api", + User: "root@pam", + } + pmgClientCfg := CreatePMGConfig(&pmgCfg) + if pmgClientCfg.Host != "https://pmg.local:8006" { + t.Fatalf("expected default PMG port on host, got %q", pmgClientCfg.Host) + } + + fromFields := CreateProxmoxConfigFromFields("https://cluster.local", "root@pam", "", "token!name", "value", "", true) + if fromFields.Host != "https://cluster.local:8006" { + t.Fatalf("expected default port on host built from fields, got %q", fromFields.Host) + } +}