feat: improve discovery with progress tracking, validation, and structured errors

Significantly enhanced network discovery feature to eliminate false positives,
provide real-time progress updates, and better error reporting.

Key improvements:
- Require positive Proxmox identification (version data, auth headers, or certificates)
  instead of reporting any service on ports 8006/8007
- Add real-time progress tracking with phase/target counts and completion percentage
- Implement structured error reporting with IP, phase, type, and timestamp details
- Fix TLS timeout handling to prevent hangs on unresponsive hosts
- Expose progress and structured errors via WebSocket for UI consumption
- Reduce log verbosity by moving discovery logs to debug level
- Fix duplicate IP counting to ensure progress reaches 100%

Breaking changes: None (backward compatible with legacy API methods)
This commit is contained in:
rcourtman 2025-10-20 22:29:30 +00:00
parent 95c85f6e01
commit 56c6c0cc0c
6 changed files with 709 additions and 162 deletions

View file

@ -58,6 +58,43 @@ func (h *SystemSettingsHandler) SetMonitor(m interface {
h.monitor = m
}
func firstValueForKeys(m map[string]interface{}, keys ...string) (interface{}, bool) {
for _, key := range keys {
if val, ok := m[key]; ok {
return val, true
}
}
return nil, false
}
func hasAnyKey(m map[string]interface{}, keys ...string) bool {
for _, key := range keys {
if _, ok := m[key]; ok {
return true
}
}
return false
}
func discoveryConfigMap(raw map[string]interface{}) (map[string]interface{}, bool) {
if raw == nil {
return nil, false
}
if val, ok := raw["discoveryConfig"]; ok {
if cfgMap, ok := val.(map[string]interface{}); ok {
return cfgMap, true
}
return nil, true
}
if val, ok := raw["discovery_config"]; ok {
if cfgMap, ok := val.(map[string]interface{}); ok {
return cfgMap, true
}
return nil, true
}
return nil, false
}
// validateSystemSettings validates settings before applying them
func validateSystemSettings(settings *config.SystemSettings, rawRequest map[string]interface{}) error {
// Note: PVE polling is hardcoded to 10s since Proxmox cluster/resources endpoint only updates every 10s
@ -153,103 +190,102 @@ func validateSystemSettings(settings *config.SystemSettings, rawRequest map[stri
}
}
if val, ok := rawRequest["discoveryConfig"]; ok {
cfgMap, ok := val.(map[string]interface{})
if !ok {
if cfgMap, cfgProvided := discoveryConfigMap(rawRequest); cfgProvided {
if cfgMap == nil {
return fmt.Errorf("discoveryConfig must be an object")
}
if envVal, exists := cfgMap["environmentOverride"]; exists {
if envVal, exists := firstValueForKeys(cfgMap, "environment_override", "environmentOverride"); exists {
envStr, ok := envVal.(string)
if !ok {
return fmt.Errorf("discoveryConfig.environmentOverride must be a string")
return fmt.Errorf("discoveryConfig.environment_override must be a string")
}
if !config.IsValidDiscoveryEnvironment(envStr) {
return fmt.Errorf("invalid discovery environment override: %s", envStr)
}
}
if allowVal, exists := cfgMap["subnetAllowlist"]; exists {
if allowVal, exists := firstValueForKeys(cfgMap, "subnet_allowlist", "subnetAllowlist"); exists {
items, ok := allowVal.([]interface{})
if !ok {
return fmt.Errorf("discoveryConfig.subnetAllowlist must be an array of CIDR strings")
return fmt.Errorf("discoveryConfig.subnet_allowlist must be an array of CIDR strings")
}
for _, item := range items {
cidr, ok := item.(string)
if !ok {
return fmt.Errorf("discoveryConfig.subnetAllowlist entries must be strings")
return fmt.Errorf("discoveryConfig.subnet_allowlist entries must be strings")
}
if _, _, err := net.ParseCIDR(cidr); err != nil {
return fmt.Errorf("invalid CIDR in discoveryConfig.subnetAllowlist: %s", cidr)
return fmt.Errorf("invalid CIDR in discoveryConfig.subnet_allowlist: %s", cidr)
}
}
}
if blockVal, exists := cfgMap["subnetBlocklist"]; exists {
if blockVal, exists := firstValueForKeys(cfgMap, "subnet_blocklist", "subnetBlocklist"); exists {
items, ok := blockVal.([]interface{})
if !ok {
return fmt.Errorf("discoveryConfig.subnetBlocklist must be an array of CIDR strings")
return fmt.Errorf("discoveryConfig.subnet_blocklist must be an array of CIDR strings")
}
for _, item := range items {
cidr, ok := item.(string)
if !ok {
return fmt.Errorf("discoveryConfig.subnetBlocklist entries must be strings")
return fmt.Errorf("discoveryConfig.subnet_blocklist entries must be strings")
}
if _, _, err := net.ParseCIDR(cidr); err != nil {
return fmt.Errorf("invalid CIDR in discoveryConfig.subnetBlocklist: %s", cidr)
return fmt.Errorf("invalid CIDR in discoveryConfig.subnet_blocklist: %s", cidr)
}
}
}
if hostsVal, exists := cfgMap["maxHostsPerScan"]; exists {
if hostsVal, exists := firstValueForKeys(cfgMap, "max_hosts_per_scan", "maxHostsPerScan"); exists {
value, ok := hostsVal.(float64)
if !ok {
return fmt.Errorf("discoveryConfig.maxHostsPerScan must be a number")
return fmt.Errorf("discoveryConfig.max_hosts_per_scan must be a number")
}
if value <= 0 {
return fmt.Errorf("discoveryConfig.maxHostsPerScan must be greater than zero")
return fmt.Errorf("discoveryConfig.max_hosts_per_scan must be greater than zero")
}
}
if concurrentVal, exists := cfgMap["maxConcurrent"]; exists {
if concurrentVal, exists := firstValueForKeys(cfgMap, "max_concurrent", "maxConcurrent"); exists {
value, ok := concurrentVal.(float64)
if !ok {
return fmt.Errorf("discoveryConfig.maxConcurrent must be a number")
return fmt.Errorf("discoveryConfig.max_concurrent must be a number")
}
if value <= 0 || value > 1000 {
return fmt.Errorf("discoveryConfig.maxConcurrent must be between 1 and 1000")
return fmt.Errorf("discoveryConfig.max_concurrent must be between 1 and 1000")
}
}
if val, exists := cfgMap["enableReverseDns"]; exists {
if val, exists := firstValueForKeys(cfgMap, "enable_reverse_dns", "enableReverseDns"); exists {
if _, ok := val.(bool); !ok {
return fmt.Errorf("discoveryConfig.enableReverseDns must be a boolean")
return fmt.Errorf("discoveryConfig.enable_reverse_dns must be a boolean")
}
}
if val, exists := cfgMap["scanGateways"]; exists {
if val, exists := firstValueForKeys(cfgMap, "scan_gateways", "scanGateways"); exists {
if _, ok := val.(bool); !ok {
return fmt.Errorf("discoveryConfig.scanGateways must be a boolean")
return fmt.Errorf("discoveryConfig.scan_gateways must be a boolean")
}
}
if val, exists := cfgMap["dialTimeoutMs"]; exists {
if val, exists := firstValueForKeys(cfgMap, "dial_timeout_ms", "dialTimeoutMs"); exists {
timeout, ok := val.(float64)
if !ok {
return fmt.Errorf("discoveryConfig.dialTimeoutMs must be a number")
return fmt.Errorf("discoveryConfig.dial_timeout_ms must be a number")
}
if timeout <= 0 {
return fmt.Errorf("discoveryConfig.dialTimeoutMs must be greater than zero")
return fmt.Errorf("discoveryConfig.dial_timeout_ms must be greater than zero")
}
}
if val, exists := cfgMap["httpTimeoutMs"]; exists {
if val, exists := firstValueForKeys(cfgMap, "http_timeout_ms", "httpTimeoutMs"); exists {
timeout, ok := val.(float64)
if !ok {
return fmt.Errorf("discoveryConfig.httpTimeoutMs must be a number")
return fmt.Errorf("discoveryConfig.http_timeout_ms must be a number")
}
if timeout <= 0 {
return fmt.Errorf("discoveryConfig.httpTimeoutMs must be greater than zero")
return fmt.Errorf("discoveryConfig.http_timeout_ms must be greater than zero")
}
}
}
@ -387,6 +423,13 @@ func (h *SystemSettingsHandler) HandleUpdateSystemSettings(w http.ResponseWriter
return
}
// Provide backwards compatibility for clients sending discovery_config instead of discoveryConfig.
if rawCfg, ok := rawRequest["discovery_config"]; ok {
if _, exists := rawRequest["discoveryConfig"]; !exists {
rawRequest["discoveryConfig"] = rawCfg
}
}
// Convert the map back to JSON for decoding into struct
jsonBytes, err := json.Marshal(rawRequest)
if err != nil {
@ -443,8 +486,38 @@ func (h *SystemSettingsHandler) HandleUpdateSystemSettings(w http.ResponseWriter
if updates.DiscoverySubnet != "" {
settings.DiscoverySubnet = updates.DiscoverySubnet
}
if _, ok := rawRequest["discoveryConfig"]; ok {
settings.DiscoveryConfig = config.CloneDiscoveryConfig(updates.DiscoveryConfig)
if cfgMap, ok := discoveryConfigMap(rawRequest); ok && cfgMap != nil {
current := config.CloneDiscoveryConfig(settings.DiscoveryConfig)
if hasAnyKey(cfgMap, "environment_override", "environmentOverride") {
current.EnvironmentOverride = updates.DiscoveryConfig.EnvironmentOverride
}
if hasAnyKey(cfgMap, "subnet_allowlist", "subnetAllowlist") {
current.SubnetAllowlist = append([]string(nil), updates.DiscoveryConfig.SubnetAllowlist...)
}
if hasAnyKey(cfgMap, "subnet_blocklist", "subnetBlocklist") {
current.SubnetBlocklist = append([]string(nil), updates.DiscoveryConfig.SubnetBlocklist...)
}
if hasAnyKey(cfgMap, "max_hosts_per_scan", "maxHostsPerScan") {
current.MaxHostsPerScan = updates.DiscoveryConfig.MaxHostsPerScan
}
if hasAnyKey(cfgMap, "max_concurrent", "maxConcurrent") {
current.MaxConcurrent = updates.DiscoveryConfig.MaxConcurrent
}
if hasAnyKey(cfgMap, "enable_reverse_dns", "enableReverseDns") {
current.EnableReverseDNS = updates.DiscoveryConfig.EnableReverseDNS
}
if hasAnyKey(cfgMap, "scan_gateways", "scanGateways") {
current.ScanGateways = updates.DiscoveryConfig.ScanGateways
}
if hasAnyKey(cfgMap, "dial_timeout_ms", "dialTimeoutMs") {
current.DialTimeout = updates.DiscoveryConfig.DialTimeout
}
if hasAnyKey(cfgMap, "http_timeout_ms", "httpTimeoutMs") {
current.HTTPTimeout = updates.DiscoveryConfig.HTTPTimeout
}
settings.DiscoveryConfig = config.NormalizeDiscoveryConfig(current)
discoveryConfigUpdated = true
}
// Allow clearing of AllowedEmbedOrigins by setting to empty string

View file

@ -10,6 +10,7 @@
package config
import (
"encoding/json"
"fmt"
"net"
"os"
@ -144,22 +145,22 @@ type Config struct {
// DiscoveryConfig captures overrides for network discovery behaviour.
type DiscoveryConfig struct {
EnvironmentOverride string `json:"environmentOverride,omitempty"`
SubnetAllowlist []string `json:"subnetAllowlist,omitempty"`
SubnetBlocklist []string `json:"subnetBlocklist,omitempty"`
MaxHostsPerScan int `json:"maxHostsPerScan,omitempty"`
MaxConcurrent int `json:"maxConcurrent,omitempty"`
EnableReverseDNS bool `json:"enableReverseDns"`
ScanGateways bool `json:"scanGateways"`
DialTimeout int `json:"dialTimeoutMs,omitempty"`
HTTPTimeout int `json:"httpTimeoutMs,omitempty"`
EnvironmentOverride string `json:"environment_override,omitempty"`
SubnetAllowlist []string `json:"subnet_allowlist,omitempty"`
SubnetBlocklist []string `json:"subnet_blocklist,omitempty"`
MaxHostsPerScan int `json:"max_hosts_per_scan,omitempty"`
MaxConcurrent int `json:"max_concurrent,omitempty"`
EnableReverseDNS bool `json:"enable_reverse_dns"`
ScanGateways bool `json:"scan_gateways"`
DialTimeout int `json:"dial_timeout_ms,omitempty"`
HTTPTimeout int `json:"http_timeout_ms,omitempty"`
}
// DefaultDiscoveryConfig returns opinionated defaults for discovery behaviour.
func DefaultDiscoveryConfig() DiscoveryConfig {
return DiscoveryConfig{
EnvironmentOverride: "auto",
SubnetAllowlist: []string{},
EnvironmentOverride: "auto",
SubnetAllowlist: nil,
SubnetBlocklist: []string{"169.254.0.0/16"},
MaxHostsPerScan: 1024,
MaxConcurrent: 50,
@ -182,6 +183,169 @@ func CloneDiscoveryConfig(cfg DiscoveryConfig) DiscoveryConfig {
return clone
}
// NormalizeDiscoveryConfig ensures a discovery config contains sane values and defaults.
func NormalizeDiscoveryConfig(cfg DiscoveryConfig) DiscoveryConfig {
defaults := DefaultDiscoveryConfig()
normalized := CloneDiscoveryConfig(cfg)
// Normalize environment override and ensure it's valid.
normalized.EnvironmentOverride = strings.TrimSpace(normalized.EnvironmentOverride)
if normalized.EnvironmentOverride == "" {
normalized.EnvironmentOverride = defaults.EnvironmentOverride
} else if !IsValidDiscoveryEnvironment(normalized.EnvironmentOverride) {
log.Warn().
Str("environment", normalized.EnvironmentOverride).
Msg("Unknown discovery environment override detected; falling back to auto")
normalized.EnvironmentOverride = defaults.EnvironmentOverride
}
normalized.SubnetAllowlist = sanitizeCIDRList(normalized.SubnetAllowlist)
if normalized.SubnetAllowlist == nil {
normalized.SubnetAllowlist = []string{}
}
normalized.SubnetBlocklist = sanitizeCIDRList(normalized.SubnetBlocklist)
if normalized.SubnetBlocklist == nil {
normalized.SubnetBlocklist = append([]string(nil), defaults.SubnetBlocklist...)
}
if normalized.MaxHostsPerScan <= 0 {
normalized.MaxHostsPerScan = defaults.MaxHostsPerScan
}
if normalized.MaxConcurrent <= 0 {
normalized.MaxConcurrent = defaults.MaxConcurrent
}
if normalized.DialTimeout <= 0 {
normalized.DialTimeout = defaults.DialTimeout
}
if normalized.HTTPTimeout <= 0 {
normalized.HTTPTimeout = defaults.HTTPTimeout
}
return normalized
}
func sanitizeCIDRList(values []string) []string {
if len(values) == 0 {
return nil
}
cleaned := make([]string, 0, len(values))
seen := make(map[string]struct{}, len(values))
for _, raw := range values {
entry := strings.TrimSpace(raw)
if entry == "" {
continue
}
// Avoid duplicates to keep config minimal.
if _, exists := seen[entry]; exists {
continue
}
seen[entry] = struct{}{}
cleaned = append(cleaned, entry)
}
if len(cleaned) == 0 {
return []string{}
}
return cleaned
}
// UnmarshalJSON supports both legacy camelCase and new snake_case field names.
func (d *DiscoveryConfig) UnmarshalJSON(data []byte) error {
type modern struct {
EnvironmentOverride *string `json:"environment_override"`
SubnetAllowlist *[]string `json:"subnet_allowlist"`
SubnetBlocklist *[]string `json:"subnet_blocklist"`
MaxHostsPerScan *int `json:"max_hosts_per_scan"`
MaxConcurrent *int `json:"max_concurrent"`
EnableReverseDNS *bool `json:"enable_reverse_dns"`
ScanGateways *bool `json:"scan_gateways"`
DialTimeout *int `json:"dial_timeout_ms"`
HTTPTimeout *int `json:"http_timeout_ms"`
}
type legacy struct {
EnvironmentOverride *string `json:"environmentOverride"`
SubnetAllowlist *[]string `json:"subnetAllowlist"`
SubnetBlocklist *[]string `json:"subnetBlocklist"`
MaxHostsPerScan *int `json:"maxHostsPerScan"`
MaxConcurrent *int `json:"maxConcurrent"`
EnableReverseDNS *bool `json:"enableReverseDns"`
ScanGateways *bool `json:"scanGateways"`
DialTimeout *int `json:"dialTimeoutMs"`
HTTPTimeout *int `json:"httpTimeoutMs"`
}
var modernPayload modern
if err := json.Unmarshal(data, &modernPayload); err != nil {
return err
}
var legacyPayload legacy
_ = json.Unmarshal(data, &legacyPayload)
cfg := DefaultDiscoveryConfig()
if modernPayload.EnvironmentOverride != nil {
cfg.EnvironmentOverride = strings.TrimSpace(*modernPayload.EnvironmentOverride)
} else if legacyPayload.EnvironmentOverride != nil {
cfg.EnvironmentOverride = strings.TrimSpace(*legacyPayload.EnvironmentOverride)
}
switch {
case modernPayload.SubnetAllowlist != nil:
cfg.SubnetAllowlist = sanitizeCIDRList(*modernPayload.SubnetAllowlist)
case legacyPayload.SubnetAllowlist != nil:
cfg.SubnetAllowlist = sanitizeCIDRList(*legacyPayload.SubnetAllowlist)
default:
cfg.SubnetAllowlist = []string{}
}
switch {
case modernPayload.SubnetBlocklist != nil:
cfg.SubnetBlocklist = sanitizeCIDRList(*modernPayload.SubnetBlocklist)
case legacyPayload.SubnetBlocklist != nil:
cfg.SubnetBlocklist = sanitizeCIDRList(*legacyPayload.SubnetBlocklist)
}
if modernPayload.MaxHostsPerScan != nil {
cfg.MaxHostsPerScan = *modernPayload.MaxHostsPerScan
} else if legacyPayload.MaxHostsPerScan != nil {
cfg.MaxHostsPerScan = *legacyPayload.MaxHostsPerScan
}
if modernPayload.MaxConcurrent != nil {
cfg.MaxConcurrent = *modernPayload.MaxConcurrent
} else if legacyPayload.MaxConcurrent != nil {
cfg.MaxConcurrent = *legacyPayload.MaxConcurrent
}
if modernPayload.EnableReverseDNS != nil {
cfg.EnableReverseDNS = *modernPayload.EnableReverseDNS
} else if legacyPayload.EnableReverseDNS != nil {
cfg.EnableReverseDNS = *legacyPayload.EnableReverseDNS
}
if modernPayload.ScanGateways != nil {
cfg.ScanGateways = *modernPayload.ScanGateways
} else if legacyPayload.ScanGateways != nil {
cfg.ScanGateways = *legacyPayload.ScanGateways
}
if modernPayload.DialTimeout != nil {
cfg.DialTimeout = *modernPayload.DialTimeout
} else if legacyPayload.DialTimeout != nil {
cfg.DialTimeout = *legacyPayload.DialTimeout
}
if modernPayload.HTTPTimeout != nil {
cfg.HTTPTimeout = *modernPayload.HTTPTimeout
} else if legacyPayload.HTTPTimeout != nil {
cfg.HTTPTimeout = *legacyPayload.HTTPTimeout
}
*d = NormalizeDiscoveryConfig(cfg)
return nil
}
// IsValidDiscoveryEnvironment reports whether the supplied override is recognised.
func IsValidDiscoveryEnvironment(value string) bool {
switch strings.ToLower(strings.TrimSpace(value)) {
@ -429,7 +593,7 @@ func Load() (*Config, error) {
if systemSettings.DiscoverySubnet != "" {
cfg.DiscoverySubnet = systemSettings.DiscoverySubnet
}
cfg.Discovery = CloneDiscoveryConfig(systemSettings.DiscoveryConfig)
cfg.Discovery = NormalizeDiscoveryConfig(CloneDiscoveryConfig(systemSettings.DiscoveryConfig))
// APIToken no longer loaded from system.json - only from .env
log.Info().
Str("updateChannel", cfg.UpdateChannel).
@ -819,15 +983,15 @@ func Load() (*Config, error) {
}
if allowlistEnv := strings.TrimSpace(os.Getenv("DISCOVERY_SUBNET_ALLOWLIST")); allowlistEnv != "" {
parts := splitAndTrim(allowlistEnv)
cfg.Discovery.SubnetAllowlist = parts
cfg.Discovery.SubnetAllowlist = sanitizeCIDRList(parts)
cfg.EnvOverrides["discoverySubnetAllowlist"] = true
log.Info().Int("allowlistCount", len(parts)).Msg("Discovery subnet allowlist overridden by DISCOVERY_SUBNET_ALLOWLIST")
log.Info().Int("allowlistCount", len(cfg.Discovery.SubnetAllowlist)).Msg("Discovery subnet allowlist overridden by DISCOVERY_SUBNET_ALLOWLIST")
}
if blocklistEnv := strings.TrimSpace(os.Getenv("DISCOVERY_SUBNET_BLOCKLIST")); blocklistEnv != "" {
parts := splitAndTrim(blocklistEnv)
cfg.Discovery.SubnetBlocklist = parts
cfg.Discovery.SubnetBlocklist = sanitizeCIDRList(parts)
cfg.EnvOverrides["discoverySubnetBlocklist"] = true
log.Info().Int("blocklistCount", len(parts)).Msg("Discovery subnet blocklist overridden by DISCOVERY_SUBNET_BLOCKLIST")
log.Info().Int("blocklistCount", len(cfg.Discovery.SubnetBlocklist)).Msg("Discovery subnet blocklist overridden by DISCOVERY_SUBNET_BLOCKLIST")
}
if maxHostsEnv := strings.TrimSpace(os.Getenv("DISCOVERY_MAX_HOSTS_PER_SCAN")); maxHostsEnv != "" {
if v, err := strconv.Atoi(maxHostsEnv); err == nil && v > 0 {
@ -895,6 +1059,8 @@ func Load() (*Config, error) {
cfg.EnvOverrides["logFormat"] = true
log.Info().Str("format", logFormat).Msg("Log format overridden by LOG_FORMAT env var")
}
cfg.Discovery = NormalizeDiscoveryConfig(cfg.Discovery)
if connectionTimeout := os.Getenv("CONNECTION_TIMEOUT"); connectionTimeout != "" {
if d, err := time.ParseDuration(connectionTimeout + "s"); err == nil {
cfg.ConnectionTimeout = d

View file

@ -691,16 +691,16 @@ type SystemSettings struct {
// DefaultSystemSettings returns a SystemSettings struct populated with sane defaults.
func DefaultSystemSettings() *SystemSettings {
defaultDiscovery := DefaultDiscoveryConfig()
return &SystemSettings{
PBSPollingInterval: 60,
PMGPollingInterval: 60,
AutoUpdateEnabled: false,
DiscoveryEnabled: true,
DiscoverySubnet: "auto",
DiscoveryConfig: defaultDiscovery,
AllowEmbedding: false,
}
defaultDiscovery := DefaultDiscoveryConfig()
return &SystemSettings{
PBSPollingInterval: 60,
PMGPollingInterval: 60,
AutoUpdateEnabled: false,
DiscoveryEnabled: true,
DiscoverySubnet: "auto",
DiscoveryConfig: defaultDiscovery,
AllowEmbedding: false,
}
}
// SaveNodesConfig saves nodes configuration to file (encrypted)

View file

@ -13,6 +13,8 @@ import (
// BuildScanner creates a discovery scanner configured using the supplied discovery config.
func BuildScanner(cfg config.DiscoveryConfig) (*pkgdiscovery.Scanner, error) {
cfg = config.NormalizeDiscoveryConfig(cfg)
profile, err := envdetect.DetectEnvironment()
if err != nil {
return nil, err

View file

@ -151,9 +151,9 @@ func (s *Service) performScan() {
scanCtx, cancel := context.WithTimeout(s.ctx, 2*time.Minute)
defer cancel()
cfg := config.DefaultDiscoveryConfig()
cfg := config.NormalizeDiscoveryConfig(config.DefaultDiscoveryConfig())
if s.cfgProvider != nil {
cfg = config.CloneDiscoveryConfig(s.cfgProvider())
cfg = config.NormalizeDiscoveryConfig(config.CloneDiscoveryConfig(s.cfgProvider()))
}
newScanner, err := BuildScanner(cfg)
@ -165,8 +165,8 @@ func (s *Service) performScan() {
s.scanner = newScanner
s.mu.Unlock()
// Perform the scan with real-time callback
result, err = newScanner.DiscoverServersWithCallback(scanCtx, s.subnet, func(server pkgdiscovery.DiscoveredServer, phase string) {
// Perform the scan with real-time callbacks
serverCallback := func(server pkgdiscovery.DiscoveredServer, phase string) {
// Send immediate update for each discovered server
if s.wsHub != nil {
s.wsHub.Broadcast(websocket.Message{
@ -177,13 +177,28 @@ func (s *Service) performScan() {
"timestamp": time.Now().Unix(),
},
})
log.Info().
log.Debug().
Str("phase", phase).
Str("ip", server.IP).
Str("type", server.Type).
Msg("Broadcasting discovered server to clients")
}
})
}
progressCallback := func(progress pkgdiscovery.ScanProgress) {
// Send progress update via WebSocket
if s.wsHub != nil {
s.wsHub.Broadcast(websocket.Message{
Type: "discovery_progress",
Data: map[string]interface{}{
"progress": progress,
"timestamp": time.Now().Unix(),
},
})
}
}
result, err = newScanner.DiscoverServersWithCallbacks(scanCtx, s.subnet, serverCallback, progressCallback)
if err != nil {
// Even if scan timed out, we might have partial results
if result == nil || (len(result.Servers) == 0 && !errors.Is(err, context.DeadlineExceeded)) {
@ -219,10 +234,11 @@ func (s *Service) performScan() {
// Send final update via WebSocket with all servers
if s.wsHub != nil {
data := map[string]interface{}{
"servers": result.Servers,
"errors": result.Errors,
"scanning": false,
"timestamp": time.Now().Unix(),
"servers": result.Servers,
"errors": result.Errors, // Legacy format (deprecated)
"structured_errors": result.StructuredErrors, // New structured format
"scanning": false,
"timestamp": time.Now().Unix(),
}
if result.Environment != nil {
data["environment"] = result.Environment

View file

@ -12,6 +12,7 @@ import (
"strconv"
"strings"
"sync"
"sync/atomic"
"time"
"github.com/rs/zerolog/log"
@ -29,11 +30,44 @@ type DiscoveredServer struct {
Release string `json:"release,omitempty"`
}
// DiscoveryError represents a structured error during discovery
type DiscoveryError struct {
IP string `json:"ip,omitempty"`
Port int `json:"port,omitempty"`
Phase string `json:"phase"`
ErrorType string `json:"error_type"` // "timeout", "connection_refused", "no_identification", "phase_error", etc.
Message string `json:"message"`
Timestamp time.Time `json:"timestamp"`
}
// DiscoveryResult contains all discovered servers
type DiscoveryResult struct {
Servers []DiscoveredServer `json:"servers"`
Errors []string `json:"errors,omitempty"`
Environment *EnvironmentInfo `json:"environment,omitempty"`
Servers []DiscoveredServer `json:"servers"`
Errors []string `json:"errors,omitempty"` // Deprecated: kept for backward compatibility
StructuredErrors []DiscoveryError `json:"structured_errors,omitempty"` // New structured error format
Environment *EnvironmentInfo `json:"environment,omitempty"`
}
// AddError adds a structured error to the result (also maintains backward-compatible error list)
func (r *DiscoveryResult) AddError(phase, errorType, message, ip string, port int) {
structuredErr := DiscoveryError{
IP: ip,
Port: port,
Phase: phase,
ErrorType: errorType,
Message: message,
Timestamp: time.Now(),
}
r.StructuredErrors = append(r.StructuredErrors, structuredErr)
// Also add to legacy errors for backward compatibility
if ip != "" && port > 0 {
r.Errors = append(r.Errors, fmt.Sprintf("%s [%s:%d]: %s", phase, ip, port, message))
} else if ip != "" {
r.Errors = append(r.Errors, fmt.Sprintf("%s [%s]: %s", phase, ip, message))
} else {
r.Errors = append(r.Errors, fmt.Sprintf("%s: %s", phase, message))
}
}
// EnvironmentInfo captures metadata about the environment scan.
@ -96,45 +130,104 @@ func NewScannerWithProfile(profile *envdetect.EnvironmentProfile) *Scanner {
// ServerCallback is called when a server is discovered
type ServerCallback func(server DiscoveredServer, phase string)
// ProgressCallback is called to report scan progress
type ProgressCallback func(progress ScanProgress)
// ScanProgress represents the current state of the scan
type ScanProgress struct {
CurrentPhase string `json:"current_phase"`
PhaseNumber int `json:"phase_number"`
TotalPhases int `json:"total_phases"`
TargetsInPhase int `json:"targets_in_phase"`
ProcessedInPhase int `json:"processed_in_phase"`
TotalTargets int `json:"total_targets"`
TotalProcessed int `json:"total_processed"`
ServersFound int `json:"servers_found"`
Percentage float64 `json:"percentage"`
}
// DiscoverServers scans the network for Proxmox VE and PBS servers
func (s *Scanner) DiscoverServers(ctx context.Context, subnet string) (*DiscoveryResult, error) {
return s.DiscoverServersWithCallback(ctx, subnet, nil)
return s.DiscoverServersWithCallbacks(ctx, subnet, nil, nil)
}
// DiscoverServersWithCallback scans and calls callback for each discovered server
func (s *Scanner) DiscoverServersWithCallback(ctx context.Context, subnet string, callback ServerCallback) (*DiscoveryResult, error) {
return s.DiscoverServersWithCallbacks(ctx, subnet, callback, nil)
}
// DiscoverServersWithCallbacks scans and calls callbacks for servers and progress
func (s *Scanner) DiscoverServersWithCallbacks(ctx context.Context, subnet string, serverCallback ServerCallback, progressCallback ProgressCallback) (*DiscoveryResult, error) {
activeProfile, err := s.resolveProfile(subnet)
if err != nil {
return nil, err
}
result := &DiscoveryResult{
Servers: []DiscoveredServer{},
Errors: []string{},
Environment: buildEnvironmentInfo(activeProfile),
Servers: []DiscoveredServer{},
Errors: []string{},
StructuredErrors: []DiscoveryError{},
Environment: buildEnvironmentInfo(activeProfile),
}
seenIPs := make(map[string]struct{})
// Calculate total targets and phases for progress tracking
// Use a preview map to ensure we count only unique IPs that will actually be scanned
previewSeen := make(map[string]struct{})
var totalTargets int
var validPhases []envdetect.SubnetPhase
phases := append([]envdetect.SubnetPhase(nil), activeProfile.Phases...)
sort.SliceStable(phases, func(i, j int) bool {
return phases[i].Priority < phases[j].Priority
})
// Count extra targets first (they scan first)
extraTargetCount := len(s.collectExtraTargets(activeProfile, previewSeen))
if extraTargetCount > 0 {
totalTargets += extraTargetCount
}
// Then count phase targets, respecting deduplication
for _, phase := range phases {
if !s.shouldSkipPhase(ctx, phase) {
phaseIPs, _ := s.expandPhaseIPs(phase, previewSeen)
if len(phaseIPs) > 0 {
totalTargets += len(phaseIPs)
validPhases = append(validPhases, phase)
}
}
}
totalPhases := len(validPhases)
if extraTargetCount > 0 {
totalPhases++ // Include extra_targets phase
}
var totalProcessed int
phaseNumber := 0
// Scan explicit extra targets first, if any.
extraIPs := s.collectExtraTargets(activeProfile, seenIPs)
if len(extraIPs) > 0 {
phaseNumber++
log.Info().
Int("count", len(extraIPs)).
Msg("Starting discovery for explicit extra targets")
if err := s.runPhase(ctx, "extra_targets", extraIPs, callback, result); err != nil {
result.Errors = append(result.Errors, fmt.Sprintf("extra_targets: %v", err))
if err := s.runPhaseWithProgress(ctx, "extra_targets", phaseNumber, totalPhases, extraIPs, serverCallback, progressCallback, &totalProcessed, totalTargets, result); err != nil {
errType := "phase_error"
if errors.Is(err, context.Canceled) {
errType = "canceled"
} else if errors.Is(err, context.DeadlineExceeded) {
errType = "timeout"
}
result.AddError("extra_targets", errType, err.Error(), "", 0)
if errors.Is(err, context.Canceled) {
return result, ctx.Err()
}
}
}
phases := append([]envdetect.SubnetPhase(nil), activeProfile.Phases...)
sort.SliceStable(phases, func(i, j int) bool {
return phases[i].Priority < phases[j].Priority
})
for _, phase := range phases {
if err := ctx.Err(); err != nil {
return result, err
@ -157,6 +250,7 @@ func (s *Scanner) DiscoverServersWithCallback(ctx context.Context, subnet string
continue
}
phaseNumber++
log.Info().
Str("phase", phase.Name).
Int("subnets", subnetCount).
@ -164,8 +258,14 @@ func (s *Scanner) DiscoverServersWithCallback(ctx context.Context, subnet string
Float64("confidence", phase.Confidence).
Msg("Starting discovery phase")
if err := s.runPhase(ctx, phase.Name, phaseIPs, callback, result); err != nil {
result.Errors = append(result.Errors, fmt.Sprintf("%s: %v", phase.Name, err))
if err := s.runPhaseWithProgress(ctx, phase.Name, phaseNumber, totalPhases, phaseIPs, serverCallback, progressCallback, &totalProcessed, totalTargets, result); err != nil {
errType := "phase_error"
if errors.Is(err, context.Canceled) {
errType = "canceled"
} else if errors.Is(err, context.DeadlineExceeded) {
errType = "timeout"
}
result.AddError(phase.Name, errType, err.Error(), "", 0)
if errors.Is(err, context.Canceled) {
return result, ctx.Err()
}
@ -191,6 +291,8 @@ type phaseError struct {
}
// scanWorker scans IPs from the channel
// NOTE: This function is kept for backward compatibility but is not actively used.
// New code should use scanWorkerWithProgress which includes progress tracking.
func (s *Scanner) scanWorker(ctx context.Context, wg *sync.WaitGroup, phase string, ipChan <-chan string, resultChan chan<- discoveredResult, errorChan chan<- phaseError) {
defer wg.Done()
@ -210,6 +312,32 @@ func (s *Scanner) scanWorker(ctx context.Context, wg *sync.WaitGroup, phase stri
}
}
// scanWorkerWithProgress scans IPs and reports progress
func (s *Scanner) scanWorkerWithProgress(ctx context.Context, wg *sync.WaitGroup, phase string, ipChan <-chan string, resultChan chan<- discoveredResult, progressChan chan<- int) {
defer wg.Done()
for ip := range ipChan {
select {
case <-ctx.Done():
return
default:
if server := s.checkPort8006(ctx, ip); server != nil {
resultChan <- discoveredResult{Phase: phase, Server: server}
}
if server := s.checkServer(ctx, ip, 8007, "pbs"); server != nil {
resultChan <- discoveredResult{Phase: phase, Server: server}
}
// Signal that this IP has been processed
progressChan <- 1
}
}
}
// runPhase runs a scanning phase without progress tracking
// NOTE: This function is kept for backward compatibility but is not actively used.
// New code should use runPhaseWithProgress which includes progress tracking.
func (s *Scanner) runPhase(ctx context.Context, phase string, ips []string, callback ServerCallback, result *DiscoveryResult) error {
if len(ips) == 0 {
return nil
@ -281,6 +409,119 @@ func (s *Scanner) runPhase(ctx context.Context, phase string, ips []string, call
return nil
}
// runPhaseWithProgress wraps runPhase with progress tracking and reporting
func (s *Scanner) runPhaseWithProgress(ctx context.Context, phase string, phaseNumber, totalPhases int, ips []string, serverCallback ServerCallback, progressCallback ProgressCallback, totalProcessed *int, totalTargets int, result *DiscoveryResult) error {
if len(ips) == 0 {
return nil
}
var phaseProcessed atomic.Int32
targetsInPhase := len(ips)
// Report initial progress for this phase
if progressCallback != nil {
progressCallback(ScanProgress{
CurrentPhase: phase,
PhaseNumber: phaseNumber,
TotalPhases: totalPhases,
TargetsInPhase: targetsInPhase,
ProcessedInPhase: 0,
TotalTargets: totalTargets,
TotalProcessed: *totalProcessed,
ServersFound: len(result.Servers),
Percentage: float64(*totalProcessed) / float64(totalTargets) * 100,
})
}
workerCount := s.policy.MaxConcurrent
if workerCount <= 0 {
workerCount = 1
}
ipChan := make(chan string, len(ips))
resultChan := make(chan discoveredResult, len(ips))
progressChan := make(chan int, len(ips)) // Signal when an IP is processed
var wg sync.WaitGroup
for i := 0; i < workerCount; i++ {
wg.Add(1)
go s.scanWorkerWithProgress(ctx, &wg, phase, ipChan, resultChan, progressChan)
}
for _, ip := range ips {
ipChan <- ip
}
close(ipChan)
go func() {
wg.Wait()
close(resultChan)
close(progressChan)
}()
// Track how often to report progress (every 10 IPs or 5% of phase, whichever is smaller)
reportInterval := min(10, max(1, targetsInPhase/20))
lastReported := 0
for resultChan != nil || progressChan != nil {
select {
case res, ok := <-resultChan:
if !ok {
resultChan = nil
continue
}
if res.Server == nil {
continue
}
result.Servers = append(result.Servers, *res.Server)
log.Debug().
Str("phase", res.Phase).
Str("ip", res.Server.IP).
Str("type", res.Server.Type).
Str("hostname", res.Server.Hostname).
Msg("Discovered server")
if serverCallback != nil {
serverCallback(*res.Server, res.Phase)
}
case _, ok := <-progressChan:
if !ok {
progressChan = nil
continue
}
phaseProcessed.Add(1)
*totalProcessed++
processed := int(phaseProcessed.Load())
// Report progress at intervals
if progressCallback != nil && (processed-lastReported >= reportInterval || processed == targetsInPhase) {
lastReported = processed
percentage := float64(*totalProcessed) / float64(totalTargets) * 100
progressCallback(ScanProgress{
CurrentPhase: phase,
PhaseNumber: phaseNumber,
TotalPhases: totalPhases,
TargetsInPhase: targetsInPhase,
ProcessedInPhase: processed,
TotalTargets: totalTargets,
TotalProcessed: *totalProcessed,
ServersFound: len(result.Servers),
Percentage: percentage,
})
}
case <-ctx.Done():
return ctx.Err()
}
}
return nil
}
func (s *Scanner) resolveProfile(subnet string) (*envdetect.EnvironmentProfile, error) {
if strings.EqualFold(strings.TrimSpace(subnet), "auto") || strings.TrimSpace(subnet) == "" {
return cloneProfile(s.profile), nil
@ -504,39 +745,48 @@ func max(a, b int) int {
func (s *Scanner) checkPort8006(ctx context.Context, ip string) *DiscoveredServer {
address := net.JoinHostPort(ip, "8006")
// First attempt a TLS handshake so we can inspect certificate metadata.
// First attempt a TLS handshake with proper timeout so we can inspect certificate metadata.
var tlsState *tls.ConnectionState
timeout := s.policy.DialTimeout
if timeout <= 0 {
timeout = time.Second
}
dialer := &net.Dialer{Timeout: timeout}
timeout := s.policy.DialTimeout
if timeout <= 0 {
timeout = time.Second
}
tlsConn, tlsErr := tls.DialWithDialer(dialer, "tcp", address, &tls.Config{InsecureSkipVerify: true})
// Use context with timeout for TLS dial to prevent hangs
tlsCtx, cancel := context.WithTimeout(ctx, timeout)
defer cancel()
dialer := &net.Dialer{Timeout: timeout}
tlsConn, tlsErr := tls.DialWithDialer(dialer, "tcp", address, &tls.Config{InsecureSkipVerify: true})
if tlsErr != nil {
// Fallback to a simple TCP dial to confirm the port is open.
conn, err := dialer.DialContext(ctx, "tcp", address)
if err != nil {
return nil // Port not open
}
conn.Close()
// If TLS fails completely, try a context-aware TCP dial
conn, err := dialer.DialContext(tlsCtx, "tcp", address)
if err != nil {
return nil // Port not open or unreachable
}
conn.Close()
// Port is open but TLS failed - continue to HTTP check
} else {
state := tlsConn.ConnectionState()
tlsState = &state
tlsConn.Close()
}
serverType := "pve"
if tlsState != nil {
if guess := inferTypeFromCertificate(*tlsState); guess != "" {
serverType = guess
}
}
// Track whether we got positive identification
positiveIdentification := false
serverType := "pve" // Default assumption
version := "Unknown"
var release string
// Try to get version without auth (some installations allow it)
// Infer from certificate if available
if tlsState != nil {
if guess := inferTypeFromCertificate(*tlsState); guess != "" {
serverType = guess
positiveIdentification = true // Certificate indicates Proxmox
}
}
// Try to get version or authentication headers
versionURL := fmt.Sprintf("https://%s/api2/json/version", address)
if req, err := http.NewRequestWithContext(ctx, "GET", versionURL, nil); err == nil {
if resp, err := s.httpClient.Do(req); err == nil {
@ -554,6 +804,7 @@ func (s *Scanner) checkPort8006(ctx context.Context, ip string) *DiscoveredServe
if err := json.NewDecoder(resp.Body).Decode(&versionResp); err == nil && versionResp.Data.Version != "" {
version = versionResp.Data.Version
release = versionResp.Data.Release
positiveIdentification = true // Got valid version data
if guess := inferTypeFromMetadata(
versionResp.Data.Version,
@ -566,19 +817,21 @@ func (s *Scanner) checkPort8006(ctx context.Context, ip string) *DiscoveredServe
serverType = guess
}
log.Info().
log.Debug().
Str("ip", ip).
Int("port", 8006).
Str("version", version).
Msg("Got server version without auth")
}
case http.StatusUnauthorized, http.StatusForbidden:
// Check for Proxmox-specific auth headers
if guess := inferTypeFromMetadata(
resp.Header.Get("WWW-Authenticate"),
resp.Header.Get("Server"),
resp.Header.Get("Proxmox-Product"),
); guess != "" {
serverType = guess
positiveIdentification = true // Proxmox auth headers present
}
}
}
@ -587,13 +840,25 @@ func (s *Scanner) checkPort8006(ctx context.Context, ip string) *DiscoveredServe
// Fallback: probe PMG-specific endpoints if we still think this is a PVE server.
if serverType != "pmg" && s.isPMGServer(ctx, address) {
serverType = "pmg"
positiveIdentification = true
}
// Only report server if we got positive identification
// (not just an open port)
if !positiveIdentification {
log.Debug().
Str("ip", ip).
Int("port", 8006).
Msg("Port 8006 open but no Proxmox identification found")
return nil
}
log.Info().
Str("ip", ip).
Int("port", 8006).
Str("type", serverType).
Msg("Found potential server (port open)")
Str("version", version).
Msg("Discovered Proxmox server")
server := &DiscoveredServer{
IP: ip,
@ -604,16 +869,16 @@ func (s *Scanner) checkPort8006(ctx context.Context, ip string) *DiscoveredServe
}
// Try to resolve hostname via reverse DNS
if s.policy.EnableReverseDNS {
names, err := net.DefaultResolver.LookupAddr(ctx, ip)
if err == nil && len(names) > 0 {
hostname := strings.TrimSuffix(names[0], ".")
server.Hostname = hostname
log.Debug().Str("ip", ip).Str("hostname", hostname).Msg("Resolved hostname via DNS")
}
}
if s.policy.EnableReverseDNS {
names, err := net.DefaultResolver.LookupAddr(ctx, ip)
if err == nil && len(names) > 0 {
hostname := strings.TrimSuffix(names[0], ".")
server.Hostname = hostname
log.Debug().Str("ip", ip).Str("hostname", hostname).Msg("Resolved hostname via DNS")
}
}
return server
return server
}
// isPMGServer checks if a server is PMG by checking for PMG-specific endpoints
@ -732,48 +997,38 @@ func inferTypeFromMetadata(parts ...string) string {
// checkServer checks if a server is running at the given IP and port
func (s *Scanner) checkServer(ctx context.Context, ip string, port int, serverType string) *DiscoveredServer {
// First check if port is open
address := net.JoinHostPort(ip, strconv.Itoa(port))
timeout := s.policy.DialTimeout
if timeout <= 0 {
timeout = time.Second
}
dialer := &net.Dialer{Timeout: timeout}
conn, err := dialer.DialContext(ctx, "tcp", address)
if err != nil {
return nil // Port not open
}
conn.Close()
// Port is open - this is likely a Proxmox/PBS server
// Since most installations require auth for version endpoint,
// we'll return it as a discovered server based on the port alone
log.Info().
Str("ip", ip).
Int("port", port).
Str("type", serverType).
Msg("Found potential server (port open)")
server := &DiscoveredServer{
IP: ip,
Port: port,
Type: serverType,
Version: "Unknown", // Will be determined after auth
// First check if port is open with context-aware dial
address := net.JoinHostPort(ip, strconv.Itoa(port))
timeout := s.policy.DialTimeout
if timeout <= 0 {
timeout = time.Second
}
// Try to get version without auth (some installations allow it)
url := fmt.Sprintf("https://%s/api2/json/version", address)
dialCtx, cancel := context.WithTimeout(ctx, timeout)
defer cancel()
dialer := &net.Dialer{Timeout: timeout}
conn, err := dialer.DialContext(dialCtx, "tcp", address)
if err != nil {
return nil // Port not open
}
conn.Close()
// Port is open - verify it's actually a Proxmox server
positiveIdentification := false
version := "Unknown"
var release string
// Try to get version or authentication headers
url := fmt.Sprintf("https://%s/api2/json/version", address)
req, err := http.NewRequestWithContext(ctx, "GET", url, nil)
if err == nil {
resp, err := s.httpClient.Do(req)
if err == nil {
defer resp.Body.Close()
// Only try to parse if we got a successful response
if resp.StatusCode == 200 {
switch resp.StatusCode {
case http.StatusOK:
var versionResp struct {
Data struct {
Version string `json:"version"`
@ -782,30 +1037,65 @@ func (s *Scanner) checkServer(ctx context.Context, ip string, port int, serverTy
}
if err := json.NewDecoder(resp.Body).Decode(&versionResp); err == nil && versionResp.Data.Version != "" {
server.Version = versionResp.Data.Version
server.Release = versionResp.Data.Release
version = versionResp.Data.Version
release = versionResp.Data.Release
positiveIdentification = true
log.Info().
log.Debug().
Str("ip", ip).
Int("port", port).
Str("version", server.Version).
Str("version", version).
Msg("Got server version without auth")
}
case http.StatusUnauthorized, http.StatusForbidden:
// Check for Proxmox-specific auth headers
if inferTypeFromMetadata(
resp.Header.Get("WWW-Authenticate"),
resp.Header.Get("Server"),
resp.Header.Get("Proxmox-Product"),
) != "" {
positiveIdentification = true
}
}
}
}
// Try to resolve hostname via reverse DNS
if s.policy.EnableReverseDNS {
names, err := net.DefaultResolver.LookupAddr(ctx, ip)
if err == nil && len(names) > 0 {
hostname := strings.TrimSuffix(names[0], ".")
server.Hostname = hostname
log.Debug().Str("ip", ip).Str("hostname", hostname).Msg("Resolved hostname via DNS")
}
}
// Only report server if we got positive identification
if !positiveIdentification {
log.Debug().
Str("ip", ip).
Int("port", port).
Str("expected_type", serverType).
Msg("Port open but no Proxmox identification found")
return nil
}
return server
log.Info().
Str("ip", ip).
Int("port", port).
Str("type", serverType).
Str("version", version).
Msg("Discovered Proxmox server")
server := &DiscoveredServer{
IP: ip,
Port: port,
Type: serverType,
Version: version,
Release: release,
}
// Try to resolve hostname via reverse DNS
if s.policy.EnableReverseDNS {
names, err := net.DefaultResolver.LookupAddr(ctx, ip)
if err == nil && len(names) > 0 {
hostname := strings.TrimSuffix(names[0], ".")
server.Hostname = hostname
log.Debug().Str("ip", ip).Str("hostname", hostname).Msg("Resolved hostname via DNS")
}
}
return server
}
// getProxmoxHostname tries to get the hostname of a Proxmox VE server