feat: unify and improve Proxmox discovery/scanning architecture
Replaced inconsistent per-product detection logic with a unified probe architecture using confidence scoring and product-specific matchers. Key improvements: - PBS detection now inspects TLS certs, auth headers (401/403), and probes PBS-specific endpoints (/api2/json/status, /config/datastore) fixing false negatives for self-signed and auth-protected servers - PMG detection uses header analysis first, then conditional endpoint probing, working consistently regardless of port - Single unified probeProxmoxService() replaces separate checkPort8006() and checkServer() code paths, eliminating duplication - Confidence scoring (0.0-1.0+) with evidence tracking for debugging - Consolidated hostname resolution and version handling Technical changes: - Added ProxmoxProbeResult with structured evidence and scoring - Added product matchers: applyPVEHeuristics, applyPMGHeuristics, applyPBSHeuristics - Removed legacy methods: checkPort8006, checkServer, isPMGServer, detectProductFromEndpoint, and duplicate hostname helpers - Updated all tests to use new unified probe architecture - Added probe_test_helpers.go for test access to internal methods All tests passing. Fixes PBS detection issues and improves consistency across PVE/PMG/PBS discovery.
This commit is contained in:
parent
e0396c1362
commit
7c00055047
3 changed files with 766 additions and 464 deletions
File diff suppressed because it is too large
Load diff
|
|
@ -1,21 +1,21 @@
|
||||||
package discovery
|
package discovery
|
||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
"crypto/tls"
|
"crypto/tls"
|
||||||
"crypto/x509"
|
"crypto/x509"
|
||||||
"crypto/x509/pkix"
|
"crypto/x509/pkix"
|
||||||
"encoding/json"
|
"encoding/json"
|
||||||
"net"
|
"net"
|
||||||
"net/http"
|
"net/http"
|
||||||
"net/http/httptest"
|
"net/http/httptest"
|
||||||
"strconv"
|
"strconv"
|
||||||
"strings"
|
"strings"
|
||||||
"sync"
|
"sync"
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
"github.com/rcourtman/pulse-go-rewrite/pkg/discovery/envdetect"
|
"github.com/rcourtman/pulse-go-rewrite/pkg/discovery/envdetect"
|
||||||
)
|
)
|
||||||
|
|
||||||
func newTestScanner(client *http.Client) *Scanner {
|
func newTestScanner(client *http.Client) *Scanner {
|
||||||
|
|
@ -124,23 +124,26 @@ func TestDetectProductFromEndpoint(t *testing.T) {
|
||||||
}))
|
}))
|
||||||
defer ts.Close()
|
defer ts.Close()
|
||||||
|
|
||||||
scanner := newTestScanner(ts.Client())
|
scanner := newTestScanner(ts.Client())
|
||||||
|
|
||||||
address := strings.TrimPrefix(ts.URL, "https://")
|
address := strings.TrimPrefix(ts.URL, "https://")
|
||||||
if product := scanner.detectProductFromEndpoint(context.Background(), address, "api2/json/statistics/mail"); product != "pmg" {
|
finding := scanner.ProbeAPIEndpoint(context.Background(), address, "api2/json/statistics/mail")
|
||||||
t.Fatalf("detectProductFromEndpoint returned %q, want %q", product, "pmg")
|
if finding.ProductGuess != ProductPMG {
|
||||||
|
t.Fatalf("ProbeAPIEndpoint returned %q, want %q", finding.ProductGuess, ProductPMG)
|
||||||
}
|
}
|
||||||
|
|
||||||
if product := scanner.detectProductFromEndpoint(context.Background(), address, "api2/json/version"); product != "pbs" {
|
versionFinding := scanner.ProbeAPIEndpoint(context.Background(), address, "api2/json/version")
|
||||||
t.Fatalf("detectProductFromEndpoint returned %q, want %q", product, "pbs")
|
if versionFinding.ProductGuess != ProductPBS {
|
||||||
|
t.Fatalf("ProbeAPIEndpoint returned %q, want %q", versionFinding.ProductGuess, ProductPBS)
|
||||||
}
|
}
|
||||||
|
|
||||||
if product := scanner.detectProductFromEndpoint(context.Background(), address, "api2/json/unknown/path"); product != "" {
|
unknownFinding := scanner.ProbeAPIEndpoint(context.Background(), address, "api2/json/unknown/path")
|
||||||
t.Fatalf("expected empty result for unknown endpoint, got %q", product)
|
if unknownFinding.ProductGuess != "" || unknownFinding.Status != http.StatusNotFound {
|
||||||
|
t.Fatalf("expected empty result for unknown endpoint, got %+v", unknownFinding)
|
||||||
}
|
}
|
||||||
|
|
||||||
if len(requestPaths) == 0 {
|
if len(requestPaths) == 0 {
|
||||||
t.Fatalf("expected detectProductFromEndpoint to perform requests")
|
t.Fatalf("expected ProbeAPIEndpoint to perform requests")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -148,8 +151,10 @@ func TestIsPMGServer(t *testing.T) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
|
|
||||||
ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
ts := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
if strings.Contains(r.URL.Path, "statistics/mail") {
|
w.Header().Set("Proxmox-Product", "Proxmox Mail Gateway")
|
||||||
w.Header().Set("Proxmox-Product", "Proxmox Mail Gateway")
|
w.Header().Set("WWW-Authenticate", `PMGAuth realm="Proxmox Mail Gateway"`)
|
||||||
|
if strings.Contains(r.URL.Path, "statistics/mail") ||
|
||||||
|
strings.Contains(r.URL.Path, "api2/json/version") {
|
||||||
w.WriteHeader(http.StatusOK)
|
w.WriteHeader(http.StatusOK)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
@ -157,19 +162,39 @@ func TestIsPMGServer(t *testing.T) {
|
||||||
}))
|
}))
|
||||||
defer ts.Close()
|
defer ts.Close()
|
||||||
|
|
||||||
scanner := newTestScanner(ts.Client())
|
scanner := newTestScanner(ts.Client())
|
||||||
|
|
||||||
address := strings.TrimPrefix(ts.URL, "https://")
|
host, portStr, err := net.SplitHostPort(ts.Listener.Addr().String())
|
||||||
if !scanner.isPMGServer(context.Background(), address) {
|
if err != nil {
|
||||||
t.Fatalf("expected PMG detection to succeed")
|
t.Fatalf("SplitHostPort: %v", err)
|
||||||
|
}
|
||||||
|
port, err := strconv.Atoi(portStr)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("strconv.Atoi: %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
|
||||||
|
defer cancel()
|
||||||
|
|
||||||
|
probe := scanner.ProbeProxmoxService(ctx, host, port)
|
||||||
|
if probe == nil || !probe.Positive || probe.PrimaryProduct != ProductPMG {
|
||||||
|
t.Fatalf("expected PMG detection to succeed, got %+v", probe)
|
||||||
}
|
}
|
||||||
|
|
||||||
tsNoMatch := httptest.NewTLSServer(http.NotFoundHandler())
|
tsNoMatch := httptest.NewTLSServer(http.NotFoundHandler())
|
||||||
defer tsNoMatch.Close()
|
defer tsNoMatch.Close()
|
||||||
|
|
||||||
scanner.httpClient = tsNoMatch.Client()
|
scanner.httpClient = tsNoMatch.Client()
|
||||||
address = strings.TrimPrefix(tsNoMatch.URL, "https://")
|
host, portStr, err = net.SplitHostPort(tsNoMatch.Listener.Addr().String())
|
||||||
if scanner.isPMGServer(context.Background(), address) {
|
if err != nil {
|
||||||
|
t.Fatalf("SplitHostPort: %v", err)
|
||||||
|
}
|
||||||
|
port, err = strconv.Atoi(portStr)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("strconv.Atoi: %v", err)
|
||||||
|
}
|
||||||
|
probe = scanner.ProbeProxmoxService(ctx, host, port)
|
||||||
|
if probe != nil && probe.Positive {
|
||||||
t.Fatalf("expected PMG detection to fail for endpoints without markers")
|
t.Fatalf("expected PMG detection to fail for endpoints without markers")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -201,26 +226,26 @@ func TestCheckServerRetrievesVersion(t *testing.T) {
|
||||||
t.Fatalf("strconv.Atoi: %v", err)
|
t.Fatalf("strconv.Atoi: %v", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
scanner := newTestScanner(ts.Client())
|
scanner := newTestScanner(ts.Client())
|
||||||
|
|
||||||
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
|
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
|
||||||
defer cancel()
|
defer cancel()
|
||||||
|
|
||||||
server := scanner.checkServer(ctx, host, port, "pbs")
|
probe := scanner.ProbeProxmoxService(ctx, host, port)
|
||||||
if server == nil {
|
if probe == nil || !probe.Positive {
|
||||||
t.Fatalf("checkServer returned nil")
|
t.Fatalf("ProbeProxmoxService returned nil")
|
||||||
}
|
}
|
||||||
|
|
||||||
if server.Type != "pbs" {
|
if probe.PrimaryProduct != ProductPBS {
|
||||||
t.Fatalf("expected type pbs, got %q", server.Type)
|
t.Fatalf("expected product pbs, got %q", probe.PrimaryProduct)
|
||||||
}
|
}
|
||||||
|
|
||||||
if server.Version != "2.4.1" {
|
if probe.Version != "2.4.1" {
|
||||||
t.Fatalf("expected version 2.4.1, got %q", server.Version)
|
t.Fatalf("expected version 2.4.1, got %q", probe.Version)
|
||||||
}
|
}
|
||||||
|
|
||||||
if server.Release != "1" {
|
if probe.Release != "1" {
|
||||||
t.Fatalf("expected release 1, got %q", server.Release)
|
t.Fatalf("expected release 1, got %q", probe.Release)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -249,25 +274,25 @@ func TestCheckServerHandlesUnauthorized(t *testing.T) {
|
||||||
srv := startTLSServerOn(t, "127.0.0.1:9008", unauthorizedHandler)
|
srv := startTLSServerOn(t, "127.0.0.1:9008", unauthorizedHandler)
|
||||||
_ = srv
|
_ = srv
|
||||||
|
|
||||||
scanner := newTestScanner(&http.Client{
|
scanner := newTestScanner(&http.Client{
|
||||||
Timeout: 500 * time.Millisecond,
|
Timeout: 500 * time.Millisecond,
|
||||||
Transport: &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true}},
|
Transport: &http.Transport{TLSClientConfig: &tls.Config{InsecureSkipVerify: true}},
|
||||||
})
|
})
|
||||||
|
|
||||||
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
|
ctx, cancel := context.WithTimeout(context.Background(), time.Second)
|
||||||
defer cancel()
|
defer cancel()
|
||||||
|
|
||||||
server := scanner.checkServer(ctx, "127.0.0.1", 9008, "pve")
|
probe := scanner.ProbeProxmoxService(ctx, "127.0.0.1", 9008)
|
||||||
if server == nil {
|
if probe == nil || !probe.Positive {
|
||||||
t.Fatalf("expected server discovery despite unauthorized response")
|
t.Fatalf("expected server discovery despite unauthorized response: %+v", probe)
|
||||||
}
|
}
|
||||||
|
|
||||||
if server.Type != "pve" {
|
if probe.PrimaryProduct != ProductPVE {
|
||||||
t.Fatalf("expected type pve, got %q", server.Type)
|
t.Fatalf("expected product pve, got %q", probe.PrimaryProduct)
|
||||||
}
|
}
|
||||||
|
|
||||||
if server.Version != "Unknown" {
|
if probe.Version != "Unknown" {
|
||||||
t.Fatalf("expected version Unknown, got %q", server.Version)
|
t.Fatalf("expected version Unknown, got %q", probe.Version)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -338,10 +363,8 @@ func TestDiscoverServersWithCallback(t *testing.T) {
|
||||||
var callbacks []DiscoveredServer
|
var callbacks []DiscoveredServer
|
||||||
|
|
||||||
// Add a manual check for the TCP-only port.
|
// Add a manual check for the TCP-only port.
|
||||||
// We call the unexported helper directly inside the package to ensure it does not panic.
|
if probe := scanner.ProbeProxmoxService(ctx, "127.0.0.1", 9009); probe != nil && probe.Positive {
|
||||||
if server := scanner.checkServer(ctx, "127.0.0.1", 9009, "pve"); server == nil {
|
t.Fatalf("expected ProbeProxmoxService to ignore TCP-only host, got %+v", probe)
|
||||||
// keep discovery results unaffected but verify we survive the TCP-only host
|
|
||||||
t.Fatalf("expected checkServer to handle TCP-only host without panic")
|
|
||||||
}
|
}
|
||||||
|
|
||||||
result, err := scanner.DiscoverServersWithCallback(ctx, subnet, func(server DiscoveredServer, phase string) {
|
result, err := scanner.DiscoverServersWithCallback(ctx, subnet, func(server DiscoveredServer, phase string) {
|
||||||
|
|
|
||||||
17
pkg/discovery/probe_test_helpers.go
Normal file
17
pkg/discovery/probe_test_helpers.go
Normal file
|
|
@ -0,0 +1,17 @@
|
||||||
|
package discovery
|
||||||
|
|
||||||
|
import "context"
|
||||||
|
|
||||||
|
const (
|
||||||
|
ProductPVE = productPVE
|
||||||
|
ProductPMG = productPMG
|
||||||
|
ProductPBS = productPBS
|
||||||
|
)
|
||||||
|
|
||||||
|
func (s *Scanner) ProbeProxmoxService(ctx context.Context, ip string, port int) *ProxmoxProbeResult {
|
||||||
|
return s.probeProxmoxService(ctx, ip, port)
|
||||||
|
}
|
||||||
|
|
||||||
|
func (s *Scanner) ProbeAPIEndpoint(ctx context.Context, address, endpoint string) EndpointProbeFinding {
|
||||||
|
return s.probeAPIEndpoint(ctx, address, endpoint)
|
||||||
|
}
|
||||||
Loading…
Reference in a new issue