From 445c2d49ef3d917fe381c414421829241e7d6933 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Wed, 25 Mar 2026 23:40:22 +0000 Subject: [PATCH] Remove redundant inventory definition route --- .../v6/internal/subsystems/agent-lifecycle.md | 2 +- .../v6/internal/subsystems/api-contracts.md | 10 +-- .../internal/subsystems/storage-recovery.md | 8 +- internal/api/audit_reporting_scope_test.go | 1 - internal/api/contract_test.go | 88 ------------------- ...terprise_extension_reporting_admin_test.go | 21 ----- internal/api/rbac_reporting_auth_test.go | 1 - internal/api/reporting_handlers_test.go | 38 -------- internal/api/reporting_inventory_handlers.go | 13 --- internal/api/route_inventory_test.go | 1 - internal/api/router_routes_licensing.go | 14 --- internal/api/security_regression_test.go | 1 - pkg/extensions/reporting_admin.go | 1 - 13 files changed, 10 insertions(+), 189 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index 04aeced827..572e6779de 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -177,7 +177,7 @@ with that export, but `internal/api/reporting_inventory_handlers.go` and `internal/api/router_routes_licensing.go` remain API-owned reporting transport, not lifecycle-owned inventory or install behavior. That adjacent reporting transport now also includes a reporting catalog route -plus a VM inventory definition route that own panel copy, performance report +whose nested VM inventory definition owns panel copy, performance report options, export title, column schema, and filename prefixes. Lifecycle- adjacent install and fleet surfaces may read those facts, but they must not redefine reporting or inventory schema locally. diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index 7caaee44cd..1d59071803 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -226,11 +226,11 @@ first-class surface separate from historical metrics reports. `internal/api/reporting_inventory_handlers.go`, `internal/api/router_routes_licensing.go`, and the settings reporting shell now own `/api/admin/reports/catalog` as the canonical operator-facing reporting -catalog plus `/api/admin/reports/inventory/vms/definition` and -`/api/admin/reports/inventory/vms/export` as the stable VM inventory sub- -contract. The catalog endpoint owns the reporting panel title, description, -locked-shell teaser copy, historical performance report options, and nested VM inventory definition, -while the export endpoint remains the spreadsheet-shaped CSV transport. That +catalog plus `/api/admin/reports/inventory/vms/export` as the stable VM +inventory sub-contract. The catalog endpoint owns the reporting panel title, +description, locked-shell teaser copy, historical performance report options, +and nested VM inventory definition, while the export endpoint remains the +spreadsheet-shaped CSV transport. That export is intentionally not comment-prefixed like the legacy metrics CSV, and it now carries Proxmox pool membership from the canonical unified VM runtime model instead of inferring or reconstructing that field locally inside the diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index e38837a254..b27d5c83f9 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -223,10 +223,10 @@ state VM facts, but `internal/api/reporting_inventory_handlers.go` and `internal/api/router_routes_licensing.go` remain API/reporting transport ownership rather than storage/recovery contract ownership. That adjacent reporting transport now also includes a reporting catalog route -plus a VM inventory definition route that own panel copy, stable column -schema, and filename prefix. Storage and recovery flows may read those facts -when they need fleet context, but they must not fork their own reporting or -inventory column contract. +whose nested VM inventory definition owns panel copy, stable column schema, +and filename prefix. Storage and recovery flows may read those facts when they +need fleet context, but they must not fork their own reporting or inventory +column contract. That catalog route is intentionally metadata-readable without the `advanced_reporting` feature gate so locked admin reporting shells can stay on the same API-owned definition before upsell; storage- and recovery-adjacent diff --git a/internal/api/audit_reporting_scope_test.go b/internal/api/audit_reporting_scope_test.go index 57db3a7f1b..87b707a269 100644 --- a/internal/api/audit_reporting_scope_test.go +++ b/internal/api/audit_reporting_scope_test.go @@ -50,7 +50,6 @@ func TestReportingEndpointsRequireSettingsReadScope(t *testing.T) { "/api/admin/reports/catalog", "/api/admin/reports/generate", "/api/admin/reports/generate-multi", - "/api/admin/reports/inventory/vms/definition", "/api/admin/reports/inventory/vms/export", } diff --git a/internal/api/contract_test.go b/internal/api/contract_test.go index b500cb6e42..c49b7972f1 100644 --- a/internal/api/contract_test.go +++ b/internal/api/contract_test.go @@ -551,94 +551,6 @@ func TestContract_PerformanceReportTransportUsesCatalogDefaultRange(t *testing.T } } -func TestContract_VMInventoryExportDefinitionJSONSnapshot(t *testing.T) { - handler := NewReportingHandlers(nil, nil) - req := httptest.NewRequest(http.MethodGet, "/api/admin/reports/inventory/vms/definition", nil) - rec := httptest.NewRecorder() - - handler.HandleGetVMInventoryDefinition(rec, req) - - if rec.Code != http.StatusOK { - t.Fatalf("expected 200, got %d body=%s", rec.Code, rec.Body.String()) - } - if got := rec.Header().Get("Content-Type"); got != "application/json" { - t.Fatalf("expected json content type, got %q", got) - } - - const want = `{ - "id":"vm_inventory", - "title":"VM Inventory Export", - "description":"Export the current fleet-wide VM inventory as CSV using the canonical runtime model. Includes VM identity, placement, CPU, memory allocation, disk allocation, and disk usage columns.", - "format":"csv", - "exportEndpoint":"/api/admin/reports/inventory/vms/export", - "filenamePrefix":"vm-inventory", - "columns":[ - { - "key":"resource_id", - "label":"Resource ID", - "description":"Canonical Pulse resource ID for the VM." - }, - { - "key":"instance", - "label":"Instance", - "description":"Configured Proxmox instance or cluster name." - }, - { - "key":"node", - "label":"Node", - "description":"Proxmox node currently hosting the VM." - }, - { - "key":"pool", - "label":"Pool", - "description":"Canonical Proxmox pool membership when the platform reports one." - }, - { - "key":"vmid", - "label":"VMID", - "description":"Numeric Proxmox VM identifier." - }, - { - "key":"vm_name", - "label":"VM Name", - "description":"Current VM display name from the runtime model." - }, - { - "key":"status", - "label":"Status", - "description":"Canonical runtime status for the VM." - }, - { - "key":"cpu_cores", - "label":"CPU Cores", - "description":"Allocated virtual CPU core count." - }, - { - "key":"memory_allocated_bytes", - "label":"Memory Allocated Bytes", - "description":"Configured memory allocation in bytes." - }, - { - "key":"disk_allocated_bytes", - "label":"Disk Allocated Bytes", - "description":"Total allocated disk capacity in bytes across the VM." - }, - { - "key":"disk_used_bytes", - "label":"Disk Used Bytes", - "description":"Current used disk bytes from the canonical runtime disk view." - }, - { - "key":"disk_status_reason", - "label":"Disk Status Reason", - "description":"Reason disk usage is partial or unavailable when the runtime cannot provide a full guest view." - } - ] - }` - - assertJSONSnapshot(t, rec.Body.Bytes(), want) -} - func TestContract_HostedTenantAISettingsFallbackJSONSnapshot(t *testing.T) { t.Setenv("PULSE_HOSTED_MODE", "true") diff --git a/internal/api/enterprise_extension_reporting_admin_test.go b/internal/api/enterprise_extension_reporting_admin_test.go index 6a0ef7f112..2c1ea25411 100644 --- a/internal/api/enterprise_extension_reporting_admin_test.go +++ b/internal/api/enterprise_extension_reporting_admin_test.go @@ -11,7 +11,6 @@ import ( type testReportingAdminEndpoints struct { catalogCalls int generateCalls int - definitionCalls int exportInventoryCalls int } @@ -25,10 +24,6 @@ func (t *testReportingAdminEndpoints) HandleGenerateReport(http.ResponseWriter, func (t *testReportingAdminEndpoints) HandleGenerateMultiReport(http.ResponseWriter, *http.Request) {} -func (t *testReportingAdminEndpoints) HandleGetVMInventoryDefinition(http.ResponseWriter, *http.Request) { - t.definitionCalls++ -} - func (t *testReportingAdminEndpoints) HandleExportVMInventory(http.ResponseWriter, *http.Request) { t.exportInventoryCalls++ } @@ -105,19 +100,3 @@ func TestResolveReportingAdminEndpoints_UsesDefaultCatalogHandler(t *testing.T) t.Fatalf("expected default reporting catalog handler call, got %d", defaults.catalogCalls) } } - -func TestResolveReportingAdminEndpoints_UsesDefaultInventoryDefinitionHandler(t *testing.T) { - SetReportingAdminEndpointsBinder(nil) - t.Cleanup(func() { - SetReportingAdminEndpointsBinder(nil) - }) - - defaults := &testReportingAdminEndpoints{} - resolved := resolveReportingAdminEndpoints(defaults, extensions.ReportingAdminRuntime{}) - req := httptest.NewRequest(http.MethodGet, "/api/admin/reports/inventory/vms/definition", nil) - rec := httptest.NewRecorder() - resolved.HandleGetVMInventoryDefinition(rec, req) - if defaults.definitionCalls != 1 { - t.Fatalf("expected default VM inventory definition handler call, got %d", defaults.definitionCalls) - } -} diff --git a/internal/api/rbac_reporting_auth_test.go b/internal/api/rbac_reporting_auth_test.go index 1ca1e215a1..3748621f93 100644 --- a/internal/api/rbac_reporting_auth_test.go +++ b/internal/api/rbac_reporting_auth_test.go @@ -40,7 +40,6 @@ func TestReportingEndpointsRequireAuthInAPIMode(t *testing.T) { {method: http.MethodGet, path: "/api/admin/reports/catalog", body: ""}, {method: http.MethodGet, path: "/api/admin/reports/generate", body: ""}, {method: http.MethodPost, path: "/api/admin/reports/generate-multi", body: `{}`}, - {method: http.MethodGet, path: "/api/admin/reports/inventory/vms/definition", body: ""}, {method: http.MethodGet, path: "/api/admin/reports/inventory/vms/export", body: ""}, } diff --git a/internal/api/reporting_handlers_test.go b/internal/api/reporting_handlers_test.go index 178aeec65e..d178e177de 100644 --- a/internal/api/reporting_handlers_test.go +++ b/internal/api/reporting_handlers_test.go @@ -307,44 +307,6 @@ func TestReportingHandlers_GetReportingCatalog_ReturnsCanonicalDefinition(t *tes } } -func TestReportingHandlers_GetVMInventoryDefinition_MethodNotAllowed(t *testing.T) { - handler := NewReportingHandlers(nil, nil) - req := httptest.NewRequest(http.MethodPost, "/api/admin/reports/inventory/vms/definition", nil) - rr := httptest.NewRecorder() - - handler.HandleGetVMInventoryDefinition(rr, req) - - if rr.Code != http.StatusMethodNotAllowed { - t.Fatalf("expected status %d, got %d", http.StatusMethodNotAllowed, rr.Code) - } -} - -func TestReportingHandlers_GetVMInventoryDefinition_ReturnsCanonicalDefinition(t *testing.T) { - handler := NewReportingHandlers(nil, nil) - req := httptest.NewRequest(http.MethodGet, "/api/admin/reports/inventory/vms/definition", nil) - rr := httptest.NewRecorder() - - handler.HandleGetVMInventoryDefinition(rr, req) - - if rr.Code != http.StatusOK { - t.Fatalf("expected status %d, got %d body=%s", http.StatusOK, rr.Code, rr.Body.String()) - } - if contentType := rr.Header().Get("Content-Type"); !strings.Contains(contentType, "application/json") { - t.Fatalf("expected JSON content type, got %q", contentType) - } - - var payload reporting.VMInventoryExportDefinition - if err := json.NewDecoder(rr.Body).Decode(&payload); err != nil { - t.Fatalf("decode definition payload: %v", err) - } - if payload.ID != "vm_inventory" || payload.Format != reporting.FormatCSV { - t.Fatalf("unexpected VM inventory definition payload: %+v", payload) - } - if len(payload.Columns) == 0 || payload.Columns[3].Label != "Pool" { - t.Fatalf("expected canonical Pool column in definition payload, got %+v", payload.Columns) - } -} - func TestReportingHandlers_ExportVMInventory_InvalidFormat(t *testing.T) { handler := NewReportingHandlers(nil, nil) req := httptest.NewRequest(http.MethodGet, "/api/admin/reports/inventory/vms/export?format=pdf", nil) diff --git a/internal/api/reporting_inventory_handlers.go b/internal/api/reporting_inventory_handlers.go index b68a9ca9df..1e8121d7de 100644 --- a/internal/api/reporting_inventory_handlers.go +++ b/internal/api/reporting_inventory_handlers.go @@ -1,7 +1,6 @@ package api import ( - "encoding/json" "fmt" "net/http" "time" @@ -10,18 +9,6 @@ import ( "github.com/rcourtman/pulse-go-rewrite/pkg/reporting" ) -// HandleGetVMInventoryDefinition returns the canonical operator-facing -// definition for the VM inventory export surface. -func (h *ReportingHandlers) HandleGetVMInventoryDefinition(w http.ResponseWriter, r *http.Request) { - if r.Method != http.MethodGet { - http.Error(w, "Method not allowed", http.StatusMethodNotAllowed) - return - } - - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(reporting.DescribeVMInventoryExport()) -} - // HandleExportVMInventory exports the current VM inventory as spreadsheet-friendly CSV. func (h *ReportingHandlers) HandleExportVMInventory(w http.ResponseWriter, r *http.Request) { if r.Method != http.MethodGet { diff --git a/internal/api/route_inventory_test.go b/internal/api/route_inventory_test.go index 14ebe3cf45..0cceb81ed0 100644 --- a/internal/api/route_inventory_test.go +++ b/internal/api/route_inventory_test.go @@ -450,7 +450,6 @@ var allRouteAllowlist = []string{ "/api/admin/reports/generate", "/api/admin/reports/generate-multi", "/api/admin/reports/catalog", - "/api/admin/reports/inventory/vms/definition", "/api/admin/reports/inventory/vms/export", "/api/admin/webhooks/audit", "/api/security/change-password", diff --git a/internal/api/router_routes_licensing.go b/internal/api/router_routes_licensing.go index 1786d9774f..22e4c9d8f0 100644 --- a/internal/api/router_routes_licensing.go +++ b/internal/api/router_routes_licensing.go @@ -195,12 +195,6 @@ func (r *Router) registerOrgLicenseRoutesGroup(orgHandlers *OrgHandlers, rbacHan } RequireLicenseFeature(r.licenseHandlers, featureAdvancedReportingValue, RequireScope(config.ScopeSettingsRead, reportingAdminEndpoints.HandleGenerateMultiReport))(w, req) })) - r.mux.HandleFunc("/api/admin/reports/inventory/vms/definition", RequirePermission(r.config, r.authorizer, auth.ActionRead, auth.ResourceNodes, func(w http.ResponseWriter, req *http.Request) { - if !ensureAdminSession(r.config, w, req) { - return - } - RequireLicenseFeature(r.licenseHandlers, featureAdvancedReportingValue, RequireScope(config.ScopeSettingsRead, reportingAdminEndpoints.HandleGetVMInventoryDefinition))(w, req) - })) r.mux.HandleFunc("/api/admin/reports/inventory/vms/export", RequirePermission(r.config, r.authorizer, auth.ActionRead, auth.ResourceNodes, func(w http.ResponseWriter, req *http.Request) { if !ensureAdminSession(r.config, w, req) { return @@ -329,14 +323,6 @@ func (a reportingAdminEndpointAdapter) HandleGenerateMultiReport(w http.Response a.handlers.HandleGenerateMultiReport(w, req) } -func (a reportingAdminEndpointAdapter) HandleGetVMInventoryDefinition(w http.ResponseWriter, req *http.Request) { - if a.handlers == nil { - writeErrorResponse(w, http.StatusNotImplemented, "reporting_unavailable", "Reporting is not available", nil) - return - } - a.handlers.HandleGetVMInventoryDefinition(w, req) -} - func (a reportingAdminEndpointAdapter) HandleExportVMInventory(w http.ResponseWriter, req *http.Request) { if a.handlers == nil { writeErrorResponse(w, http.StatusNotImplemented, "reporting_unavailable", "Reporting is not available", nil) diff --git a/internal/api/security_regression_test.go b/internal/api/security_regression_test.go index 5108f5963b..bef0d1c23d 100644 --- a/internal/api/security_regression_test.go +++ b/internal/api/security_regression_test.go @@ -2917,7 +2917,6 @@ func TestReportingExecutionEndpointsRequireLicenseFeature(t *testing.T) { paths := []string{ "/api/admin/reports/generate", "/api/admin/reports/generate-multi", - "/api/admin/reports/inventory/vms/definition", "/api/admin/reports/inventory/vms/export", } diff --git a/pkg/extensions/reporting_admin.go b/pkg/extensions/reporting_admin.go index a9feee1af4..1dd5d3b030 100644 --- a/pkg/extensions/reporting_admin.go +++ b/pkg/extensions/reporting_admin.go @@ -13,7 +13,6 @@ type ReportingAdminEndpoints interface { HandleGetReportingCatalog(http.ResponseWriter, *http.Request) HandleGenerateReport(http.ResponseWriter, *http.Request) HandleGenerateMultiReport(http.ResponseWriter, *http.Request) - HandleGetVMInventoryDefinition(http.ResponseWriter, *http.Request) HandleExportVMInventory(http.ResponseWriter, *http.Request) }