diff --git a/internal/ai/discovery_adapter.go b/internal/ai/discovery_adapter.go index 3a4b0769b..f404de502 100644 --- a/internal/ai/discovery_adapter.go +++ b/internal/ai/discovery_adapter.go @@ -191,11 +191,22 @@ func (a *discoveryStateAdapter) GetState() servicediscovery.StateSnapshot { } } + // Convert Nodes + nodes := make([]servicediscovery.Node, len(state.Nodes)) + for i, n := range state.Nodes { + nodes[i] = servicediscovery.Node{ + ID: n.ID, + Name: n.Name, + LinkedHostAgentID: n.LinkedHostAgentID, + } + } + return servicediscovery.StateSnapshot{ VMs: vms, Containers: containers, DockerHosts: dockerHosts, Hosts: hosts, + Nodes: nodes, } } diff --git a/internal/ai/discovery_adapter_test.go b/internal/ai/discovery_adapter_test.go index f6d4cd955..3cd1643c0 100644 --- a/internal/ai/discovery_adapter_test.go +++ b/internal/ai/discovery_adapter_test.go @@ -43,6 +43,13 @@ func TestDiscoveryStateAdapter(t *testing.T) { }, }, }, + Hosts: []models.Host{ + {ID: "host-agent-1", Hostname: "server1", Platform: "linux"}, + }, + Nodes: []models.Node{ + {ID: "node-1", Name: "pve1", LinkedHostAgentID: "host-agent-1"}, + {ID: "node-2", Name: "pve2", LinkedHostAgentID: ""}, // No linked agent + }, } provider := &MockStateProvider{state: mockState} @@ -86,6 +93,42 @@ func TestDiscoveryStateAdapter(t *testing.T) { } } } + + // Test Hosts conversion + if len(result.Hosts) != 1 { + t.Errorf("Expected 1 Host, got %d", len(result.Hosts)) + } else { + if result.Hosts[0].ID != "host-agent-1" { + t.Errorf("Expected Host ID 'host-agent-1', got %s", result.Hosts[0].ID) + } + if result.Hosts[0].Hostname != "server1" { + t.Errorf("Expected Host Hostname 'server1', got %s", result.Hosts[0].Hostname) + } + } + + // Test Nodes conversion (critical for discovery redirection) + if len(result.Nodes) != 2 { + t.Errorf("Expected 2 Nodes, got %d", len(result.Nodes)) + } else { + // Verify first node with linked agent + if result.Nodes[0].ID != "node-1" { + t.Errorf("Expected Node[0].ID 'node-1', got %s", result.Nodes[0].ID) + } + if result.Nodes[0].Name != "pve1" { + t.Errorf("Expected Node[0].Name 'pve1', got %s", result.Nodes[0].Name) + } + if result.Nodes[0].LinkedHostAgentID != "host-agent-1" { + t.Errorf("Expected Node[0].LinkedHostAgentID 'host-agent-1', got %s", result.Nodes[0].LinkedHostAgentID) + } + + // Verify second node without linked agent + if result.Nodes[1].Name != "pve2" { + t.Errorf("Expected Node[1].Name 'pve2', got %s", result.Nodes[1].Name) + } + if result.Nodes[1].LinkedHostAgentID != "" { + t.Errorf("Expected Node[1].LinkedHostAgentID to be empty, got %s", result.Nodes[1].LinkedHostAgentID) + } + } } func TestDiscoveryStateAdapter_NilProvider(t *testing.T) { diff --git a/internal/servicediscovery/service.go b/internal/servicediscovery/service.go index fda49be21..3329a5896 100644 --- a/internal/servicediscovery/service.go +++ b/internal/servicediscovery/service.go @@ -81,6 +81,14 @@ type StateSnapshot struct { DockerHosts []DockerHost KubernetesClusters []KubernetesCluster Hosts []Host + Nodes []Node +} + +// Node represents a Proxmox VE node. +type Node struct { + ID string + Name string + LinkedHostAgentID string } // Host represents a host system (via host-agent). @@ -1053,6 +1061,26 @@ func (s *Service) analyzeDockerContainer(ctx context.Context, analyzer AIAnalyze // - Runs discovery only when fingerprint changed or discovery is too old // - Prevents duplicate concurrent discoveries for the same resource func (s *Service) DiscoverResource(ctx context.Context, req DiscoveryRequest) (*ResourceDiscovery, error) { + // Redirect PVE node requests to linked host agent if available + // This ensures we always scan and store data under the canonical Host Agent ID + if req.ResourceType == ResourceTypeHost && s.stateProvider != nil { + state := s.stateProvider.GetState() + for _, node := range state.Nodes { + // Check if the requested ID matches the Node Name or ID + if node.Name == req.HostID || node.Name == req.ResourceID || node.ID == req.ResourceID { + if node.LinkedHostAgentID != "" { + log.Info(). + Str("from_host", req.HostID). + Str("to_agent", node.LinkedHostAgentID). + Msg("Redirecting discovery scan to linked host agent") + req.HostID = node.LinkedHostAgentID + req.ResourceID = node.LinkedHostAgentID + } + break + } + } + } + resourceID := MakeResourceID(req.ResourceType, req.HostID, req.ResourceID) // Get current fingerprint (if available) @@ -1726,9 +1754,48 @@ func (s *Service) GetDiscovery(id string) (*ResourceDiscovery, error) { return d, nil } -// GetDiscoveryByResource retrieves a discovery by resource type and ID. func (s *Service) GetDiscoveryByResource(resourceType ResourceType, hostID, resourceID string) (*ResourceDiscovery, error) { + originalHostID := hostID + originalResourceID := resourceID + redirected := false + + // Redirect PVE node lookups to linked host agent if available + // This ensures UI components looking up a PVE node by name (e.g. NodeDrawer) get the data associated with the Host Agent + if resourceType == ResourceTypeHost && s.stateProvider != nil { + state := s.stateProvider.GetState() + for _, node := range state.Nodes { + if node.Name == hostID || node.Name == resourceID || node.ID == resourceID { + if node.LinkedHostAgentID != "" { + log.Debug(). + Str("from_host", hostID). + Str("to_agent", node.LinkedHostAgentID). + Msg("Redirecting discovery lookup to linked host agent") + hostID = node.LinkedHostAgentID + resourceID = node.LinkedHostAgentID + redirected = true + } + break + } + } + } + d, err := s.store.GetByResource(resourceType, hostID, resourceID) + // If redirected and not found, try the original ID (fallback for unmigrated data) + if (err != nil || d == nil) && redirected { + log.Debug(). + Str("redirected_host", hostID). + Str("original_host", originalHostID). + Msg("Redirected lookup failed, trying fallback to original ID") + dOriginal, errOriginal := s.store.GetByResource(resourceType, originalHostID, originalResourceID) + if errOriginal == nil && dOriginal != nil { + log.Debug(). + Str("original_host", originalHostID). + Msg("Fallback lookup succeeded - returning legacy discovery") + s.upgradeCLIAccessIfNeeded(dOriginal) + return dOriginal, nil + } + } + if err != nil || d == nil { return d, err } @@ -1742,6 +1809,7 @@ func (s *Service) ListDiscoveries() ([]*ResourceDiscovery, error) { if err != nil { return nil, err } + discoveries = s.deduplicateDiscoveries(discoveries) for _, d := range discoveries { s.upgradeCLIAccessIfNeeded(d) } @@ -1754,6 +1822,7 @@ func (s *Service) ListDiscoveriesByType(resourceType ResourceType) ([]*ResourceD if err != nil { return nil, err } + discoveries = s.deduplicateDiscoveries(discoveries) for _, d := range discoveries { s.upgradeCLIAccessIfNeeded(d) } @@ -1766,12 +1835,80 @@ func (s *Service) ListDiscoveriesByHost(hostID string) ([]*ResourceDiscovery, er if err != nil { return nil, err } + discoveries = s.deduplicateDiscoveries(discoveries) for _, d := range discoveries { s.upgradeCLIAccessIfNeeded(d) } return discoveries, nil } +// deduplicateDiscoveries filters out redundant discoveries where a PVE node +// is represented by both its Node Name and its Linked Host Agent ID. +// The Host Agent ID is preferred. +func (s *Service) deduplicateDiscoveries(discoveries []*ResourceDiscovery) []*ResourceDiscovery { + if s.stateProvider == nil { + return discoveries + } + + state := s.stateProvider.GetState() + if len(state.Nodes) == 0 { + return discoveries + } + + // Map linked agent IDs to their PVE node source(s) + // AgentID -> NodeName + linkedAgents := make(map[string]string) + for _, node := range state.Nodes { + if node.LinkedHostAgentID != "" { + linkedAgents[node.LinkedHostAgentID] = node.Name + } + } + + if len(linkedAgents) == 0 { + return discoveries + } + + // Check which agents actually have discovery data + hasAgentDiscovery := make(map[string]bool) + for _, d := range discoveries { + if d.ResourceType == ResourceTypeHost { + // d.HostID is usually the agent ID for host resources + if _, ok := linkedAgents[d.HostID]; ok { + hasAgentDiscovery[d.HostID] = true + } + } + } + + // Filter out PVE node discoveries if the corresponding agent discovery exists + filtered := make([]*ResourceDiscovery, 0, len(discoveries)) + for _, d := range discoveries { + if d.ResourceType == ResourceTypeHost { + // If this discovery is for a PVE node (by name/ID) + // check if it maps to an agent that ALREADY has a discovery in this list + + // Is this discovery's ID satisfying a Node check? + isPVENode := false + var linkedAgentID string + + for _, node := range state.Nodes { + if d.HostID == node.Name || d.HostID == node.ID || d.ResourceID == node.Name { + isPVENode = true + linkedAgentID = node.LinkedHostAgentID + break + } + } + + if isPVENode && linkedAgentID != "" && hasAgentDiscovery[linkedAgentID] && d.HostID != linkedAgentID { + // We have the agent discovery, so skip this redundant PVE node discovery + continue + } + } + filtered = append(filtered, d) + } + + return filtered +} + // upgradeDiscoveryIfNeeded upgrades cached discovery fields to current versions. // This ensures cached discoveries get the new instructional CLI access format // and have hostname populated without requiring a full re-discovery. diff --git a/internal/servicediscovery/service_test.go b/internal/servicediscovery/service_test.go index 86ecf55fb..5666e8ded 100644 --- a/internal/servicediscovery/service_test.go +++ b/internal/servicediscovery/service_test.go @@ -957,3 +957,136 @@ func TestParseDockerMounts(t *testing.T) { }) } } + +func TestService_Redirection(t *testing.T) { + // Setup store + store, err := NewStore(t.TempDir()) + if err != nil { + t.Fatalf("NewStore error: %v", err) + } + store.crypto = nil + + // Setup state with a linked PVE node + state := StateSnapshot{ + Nodes: []Node{ + { + ID: "pve-id-1", + Name: "pve1", + LinkedHostAgentID: "agent-pve1", + }, + }, + Hosts: []Host{ + { + ID: "agent-pve1", + Hostname: "pve1-host", + }, + }, + } + + // Setup service + service := NewService(store, nil, stubStateProvider{state: state}, DefaultConfig()) + service.SetAIAnalyzer(&stubAnalyzer{ + response: `{"service_type":"proxmox","service_name":"Proxmox VE","service_version":"8.0","category":"virtualizer","cli_access":"ssh root@pve1","facts":[],"config_paths":[],"data_paths":[],"ports":[],"confidence":0.9,"reasoning":"test"}`, + }) + + ctx := context.Background() + + // 1. Test DiscoverResource redirection + // Trigger discovery for the PVE node "pve1" + req := DiscoveryRequest{ + ResourceType: ResourceTypeHost, + HostID: "pve1", + ResourceID: "pve1", + Force: true, + } + + discovery, err := service.DiscoverResource(ctx, req) + if err != nil { + t.Fatalf("DiscoverResource error: %v", err) + } + + // The discovery should be associated with the AGENT ID, not the NODE ID + expectedID := MakeResourceID(ResourceTypeHost, "agent-pve1", "agent-pve1") // Host resources usually have HostID == ResourceID + if discovery.ID != expectedID { + t.Errorf("DiscoverResource ID mismatch. Got %s, want %s (should have redirected to agent ID)", discovery.ID, expectedID) + } + if discovery.HostID != "agent-pve1" { + t.Errorf("DiscoverResource HostID mismatch. Got %s, want agent-pve1", discovery.HostID) + } + + // 2. Test GetDiscoveryByResource redirection (standard case) + // Try to get discovery using the PVE node name + got, err := service.GetDiscoveryByResource(ResourceTypeHost, "pve1", "pve1") + if err != nil { + t.Fatalf("GetDiscoveryByResource error: %v", err) + } + if got == nil { + t.Fatalf("GetDiscoveryByResource returned nil") + } + + // It should return the discovery we just created (which is under agent-pve1) + if got.ID != expectedID { + t.Errorf("GetDiscoveryByResource returned wrong discovery. Got ID %s, want %s", got.ID, expectedID) + } + + // 3. Test GetDiscoveryByResource fallback + // Create a "legacy" discovery that only exists under the node ID + legacyID := MakeResourceID(ResourceTypeHost, "pve1", "pve1") + legacyDiscovery := &ResourceDiscovery{ + ID: legacyID, + ResourceType: ResourceTypeHost, + HostID: "pve1", + ResourceID: "pve1", + ServiceName: "Legacy PVE", + } + if err := store.Save(legacyDiscovery); err != nil { + t.Fatalf("Failed to save legacy discovery: %v", err) + } + + // Temporarily remove the "agent" discovery to force fallback + if err := store.Delete(expectedID); err != nil { + t.Fatalf("Failed to delete agent discovery: %v", err) + } + + // Try to get "pve1" again. It should redirect to agent-pve1 (not found), then fallback to pve1 (found) + gotLegacy, err := service.GetDiscoveryByResource(ResourceTypeHost, "pve1", "pve1") + if err != nil { + t.Fatalf("GetDiscoveryByResource fallback error: %v", err) + } + if gotLegacy == nil { + t.Fatalf("GetDiscoveryByResource fallback returned nil") + } + if gotLegacy.ID != legacyID { + t.Errorf("GetDiscoveryByResource fallback returned wrong ID. Got %s, want %s", gotLegacy.ID, legacyID) + } + + // 4. Test Deduplication + // Restore the agent discovery, so we have BOTH legacy (pve1) and agent (agent-pve1) + if err := store.Save(discovery); err != nil { + t.Fatalf("Failed to restore agent discovery: %v", err) + } + + list, err := service.ListDiscoveries() + if err != nil { + t.Fatalf("ListDiscoveries error: %v", err) + } + + // We expect deduplication to remove the legacy pve1 entry because agent-pve1 exists + foundLegacy := false + foundAgent := false + for _, d := range list { + if d.ID == legacyID { + foundLegacy = true + } + if d.ID == expectedID { + foundAgent = true + } + } + + if foundLegacy { + t.Errorf("Deduplication failed: Legacy PVE node discovery should have been filtered out") + } + if !foundAgent { + t.Errorf("Deduplication failed: Agent discovery should be present") + } +}