From ec28bb3314ec1ac4a23af7ea5d8dbad5bd86672e Mon Sep 17 00:00:00 2001 From: rcourtman Date: Sun, 19 Apr 2026 20:42:22 +0100 Subject: [PATCH] Accept aggregator semantic IDs on node mutation endpoints The unified /api/connections aggregator emits IDs as {type}:{name} (e.g. "pve:delly"), but the PUT/DELETE/refresh-cluster/test endpoints only parsed the legacy {type}-{index} array-position form. That left the new Connection surface unable to drive any mutation against the entries it lists. HandleUpdateNode, HandleDeleteNode, HandleRefreshClusterNodes, and HandleTestNode now route the incoming ID through a shared resolveNodeID helper: colon-form resolves by Name (404 on miss), dash-form keeps the existing index semantics. Frontend connection client gains setEnabled/remove that dispatch to the right per-type endpoint by ID prefix. --- frontend-modern/src/api/connections.ts | 77 +++++++++++ internal/api/config_handlers_delete_test.go | 19 +++ internal/api/config_handlers_update_test.go | 26 ++++ internal/api/config_node_handlers.go | 139 +++++++++++++------- 4 files changed, 217 insertions(+), 44 deletions(-) diff --git a/frontend-modern/src/api/connections.ts b/frontend-modern/src/api/connections.ts index 11267d0d9..019b3df87 100644 --- a/frontend-modern/src/api/connections.ts +++ b/frontend-modern/src/api/connections.ts @@ -1,4 +1,5 @@ import { apiFetchJSON } from '@/utils/apiClient'; +import { MonitoringAPI } from './monitoring'; export type ConnectionType = | 'pve' @@ -81,4 +82,80 @@ export class ConnectionsAPI { body: JSON.stringify({ address } satisfies ProbeRequest), }); } + + static async setEnabled(connectionId: string, enabled: boolean): Promise { + const { type, suffix } = splitConnectionId(connectionId); + switch (type) { + case 'pve': + case 'pbs': + case 'pmg': + await apiFetchJSON(`/api/config/nodes/${encodeURIComponent(connectionId)}`, { + method: 'PUT', + body: JSON.stringify({ enabled }), + }); + return; + case 'vmware': + await apiFetchJSON(`/api/vmware/connections/${encodeURIComponent(suffix)}`, { + method: 'PUT', + body: JSON.stringify({ enabled }), + }); + return; + case 'truenas': + await apiFetchJSON(`/api/truenas/connections/${encodeURIComponent(suffix)}`, { + method: 'PUT', + body: JSON.stringify({ enabled }), + }); + return; + case 'agent': + case 'docker': + case 'kubernetes': + throw new Error(`Pause is not supported for ${type} connections`); + default: + throw new Error(`Unknown connection type: ${type}`); + } + } + + static async remove(connectionId: string): Promise { + const { type, suffix } = splitConnectionId(connectionId); + switch (type) { + case 'pve': + case 'pbs': + case 'pmg': + await apiFetchJSON(`/api/config/nodes/${encodeURIComponent(connectionId)}`, { + method: 'DELETE', + }); + return; + case 'vmware': + await apiFetchJSON(`/api/vmware/connections/${encodeURIComponent(suffix)}`, { + method: 'DELETE', + }); + return; + case 'truenas': + await apiFetchJSON(`/api/truenas/connections/${encodeURIComponent(suffix)}`, { + method: 'DELETE', + }); + return; + case 'agent': + await MonitoringAPI.deleteAgent(suffix); + return; + case 'docker': + case 'kubernetes': + throw new Error(`Remove is not yet supported for ${type} connections`); + default: + throw new Error(`Unknown connection type: ${type}`); + } + } } + +const splitConnectionId = (id: string): { type: ConnectionType; suffix: string } => { + const colon = id.indexOf(':'); + if (colon <= 0) { + throw new Error(`Invalid connection id: ${id}`); + } + const type = id.slice(0, colon) as ConnectionType; + const suffix = id.slice(colon + 1); + if (!suffix) { + throw new Error(`Invalid connection id (empty suffix): ${id}`); + } + return { type, suffix }; +}; diff --git a/internal/api/config_handlers_delete_test.go b/internal/api/config_handlers_delete_test.go index 6d8f3d405..73e4c8c92 100644 --- a/internal/api/config_handlers_delete_test.go +++ b/internal/api/config_handlers_delete_test.go @@ -80,6 +80,25 @@ func TestHandleDeleteNode(t *testing.T) { expectedStatus: http.StatusNotFound, // Handler returns 404 for unknown types verifyDeletion: nil, }, + { + // Semantic ID form emitted by the unified connections aggregator. + // Looks up the remaining PVE ("pve2") by name after earlier deletes. + name: "success_semantic_id_pve_by_name", + nodeID: "pve:pve2", + expectedStatus: http.StatusOK, + verifyDeletion: func(t *testing.T, c *config.Config) { + if len(c.PVEInstances) != 0 { + t.Errorf("expected 0 PVE instances after semantic-id delete, got %d", len(c.PVEInstances)) + } + }, + }, + { + // Name that no longer exists after the previous delete. + name: "fail_semantic_id_unknown_name", + nodeID: "pve:pve2", + expectedStatus: http.StatusNotFound, + verifyDeletion: nil, + }, } for _, tt := range tests { diff --git a/internal/api/config_handlers_update_test.go b/internal/api/config_handlers_update_test.go index bb69488ef..cd284eace 100644 --- a/internal/api/config_handlers_update_test.go +++ b/internal/api/config_handlers_update_test.go @@ -124,6 +124,32 @@ func TestHandleUpdateNode(t *testing.T) { }, expectedStatus: http.StatusNotFound, }, + { + // The unified connections aggregator emits IDs of the form + // "pve:"; the update endpoint accepts this form and resolves + // by Name. + name: "success_semantic_id_pve_by_name", + nodeID: "pve:test-renamed-pve", + requestBody: map[string]interface{}{ + "name": "renamed-via-semantic-id", + }, + expectedStatus: http.StatusOK, + verifyConfig: func(t *testing.T, c *config.Config) { + if c.PVEInstances[0].Name != "renamed-via-semantic-id" { + t.Errorf("semantic-id update didn't take effect: got %q", c.PVEInstances[0].Name) + } + }, + }, + { + // Previous case just renamed to "renamed-via-semantic-id", so + // resolving by an outdated name should 404, not 400. + name: "fail_semantic_id_unknown_name", + nodeID: "pve:test-renamed-pve", + requestBody: map[string]interface{}{ + "name": "wont-work", + }, + expectedStatus: http.StatusNotFound, + }, } for _, tt := range tests { diff --git a/internal/api/config_node_handlers.go b/internal/api/config_node_handlers.go index c724cbba9..af6797d2a 100644 --- a/internal/api/config_node_handlers.go +++ b/internal/api/config_node_handlers.go @@ -3,6 +3,7 @@ package api import ( "context" "encoding/json" + "errors" "fmt" "net/http" "net/url" @@ -19,6 +20,72 @@ import ( "github.com/rs/zerolog/log" ) +// errNodeIDNotFound signals that resolveNodeID parsed a well-formed semantic +// ID ("pve:delly") but no matching instance is in the current config. Callers +// distinguish this from malformed IDs so they can emit 404 vs 400. +var errNodeIDNotFound = errors.New("node id not found") + +// resolveNodeID accepts either the legacy array-index form ("pve-0") or the +// semantic form the unified connections aggregator emits ("pve:delly") and +// returns (nodeType, slice-index) suitable for indexing into cfg's per-type +// instance slices. +// +// Semantic-form lookup is by Name. Duplicate names are prevented at write +// time; if one somehow sneaks in, the first match wins. +func resolveNodeID(cfg *config.Config, raw string) (string, int, error) { + if raw == "" { + return "", 0, fmt.Errorf("empty node id") + } + + if colonIdx := strings.Index(raw, ":"); colonIdx > 0 { + nodeType := raw[:colonIdx] + name := raw[colonIdx+1:] + if name == "" { + return "", 0, fmt.Errorf("empty name in node id %q", raw) + } + idx, found := findInstanceIndexByName(cfg, nodeType, name) + if !found { + return nodeType, 0, errNodeIDNotFound + } + return nodeType, idx, nil + } + + parts := strings.Split(raw, "-") + if len(parts) != 2 { + return "", 0, fmt.Errorf("invalid node id %q", raw) + } + nodeType := parts[0] + var index int + if _, err := fmt.Sscanf(parts[1], "%d", &index); err != nil { + return "", 0, fmt.Errorf("invalid index in node id %q: %w", raw, err) + } + return nodeType, index, nil +} + +func findInstanceIndexByName(cfg *config.Config, nodeType, name string) (int, bool) { + switch nodeType { + case "pve": + for i, inst := range cfg.PVEInstances { + if inst.Name == name { + return i, true + } + } + case "pbs": + for i, inst := range cfg.PBSInstances { + if inst.Name == name { + return i, true + } + } + case "pmg": + for i, inst := range cfg.PMGInstances { + if inst.Name == name { + return i, true + } + } + } + return 0, false +} + func (h *ConfigHandlers) handleGetNodes(w http.ResponseWriter, r *http.Request) { // Check if mock mode is enabled if mock.IsMockEnabled() { @@ -1113,17 +1180,13 @@ func (h *ConfigHandlers) handleUpdateNode(w http.ResponseWriter, r *http.Request Interface("temperatureMonitoringEnabled", req.TemperatureMonitoringEnabled). Msg("Received node update request") - // Parse node ID - parts := strings.Split(nodeID, "-") - if len(parts) != 2 { - http.Error(w, "Invalid node ID", http.StatusBadRequest) - return - } - - nodeType := parts[0] - index := 0 - if _, err := fmt.Sscanf(parts[1], "%d", &index); err != nil { - http.Error(w, "Invalid node ID", http.StatusBadRequest) + nodeType, index, err := resolveNodeID(h.getConfig(r.Context()), nodeID) + if err != nil { + if errors.Is(err, errNodeIDNotFound) { + http.Error(w, "Node not found", http.StatusNotFound) + } else { + http.Error(w, "Invalid node ID", http.StatusBadRequest) + } return } @@ -1530,17 +1593,13 @@ func (h *ConfigHandlers) handleDeleteNode(w http.ResponseWriter, r *http.Request return } - // Parse node ID - parts := strings.Split(nodeID, "-") - if len(parts) != 2 { - http.Error(w, "Invalid node ID", http.StatusBadRequest) - return - } - - nodeType := parts[0] - index := 0 - if _, err := fmt.Sscanf(parts[1], "%d", &index); err != nil { - http.Error(w, "Invalid node ID", http.StatusBadRequest) + nodeType, index, err := resolveNodeID(h.getConfig(r.Context()), nodeID) + if err != nil { + if errors.Is(err, errNodeIDNotFound) { + http.Error(w, "Node not found", http.StatusNotFound) + } else { + http.Error(w, "Invalid node ID", http.StatusBadRequest) + } return } @@ -1659,17 +1718,13 @@ func (h *ConfigHandlers) handleRefreshClusterNodes(w http.ResponseWriter, r *htt return } - // Parse node ID - parts := strings.Split(nodeID, "-") - if len(parts) != 2 { - http.Error(w, "Invalid node ID", http.StatusBadRequest) - return - } - - nodeType := parts[0] - index := 0 - if _, err := fmt.Sscanf(parts[1], "%d", &index); err != nil { - http.Error(w, "Invalid node ID", http.StatusBadRequest) + nodeType, index, err := resolveNodeID(h.getConfig(r.Context()), nodeID) + if err != nil { + if errors.Is(err, errNodeIDNotFound) { + http.Error(w, "Node not found", http.StatusNotFound) + } else { + http.Error(w, "Invalid node ID", http.StatusBadRequest) + } return } @@ -1847,17 +1902,13 @@ func (h *ConfigHandlers) handleTestNode(w http.ResponseWriter, r *http.Request) nodeID := parts[0] - // Parse node ID - idParts := strings.Split(nodeID, "-") - if len(idParts) != 2 { - http.Error(w, "Invalid node ID", http.StatusBadRequest) - return - } - - nodeType := idParts[0] - index := 0 - if _, err := fmt.Sscanf(idParts[1], "%d", &index); err != nil { - http.Error(w, "Invalid node ID", http.StatusBadRequest) + nodeType, index, err := resolveNodeID(h.getConfig(r.Context()), nodeID) + if err != nil { + if errors.Is(err, errNodeIDNotFound) { + http.Error(w, "Node not found", http.StatusNotFound) + } else { + http.Error(w, "Invalid node ID", http.StatusBadRequest) + } return }