fix(recovery): prefer canonical item names over raw ids

This commit is contained in:
rcourtman
2026-03-28 00:04:12 +00:00
parent ac6a52d534
commit 8a240f8312
7 changed files with 245 additions and 2 deletions
@@ -245,6 +245,12 @@ and `display.itemType` to recovery consumers, while legacy transport fields
such as `subjectLabel` and `subjectType` remain decode-only compatibility
aliases in the shared normalization layer instead of leaking into runtime
presenters.
That same canonical item-label boundary must prefer recognizable protected-item
names over raw entity IDs. When unresolved Proxmox-backed recovery points only
have a VMID/CTID in the subject ref but still carry a richer backup comment or
notes label, the canonical recovery index and store backfill must promote that
human-readable label into the persisted subject/item label instead of leaving
protected inventory rows to lead with bare numeric IDs.
That same shared presentation layer also owns the distinction between
aggregate recovery-method language and single-record recovery-method language.
Timeline legends and daily breakdowns must use aggregate labels such as
+64
View File
@@ -56,6 +56,61 @@ func detailsString(p RecoveryPoint, key string) string {
return trimString(p.Details[key])
}
func isNumericOnlyLabel(value string) bool {
value = strings.TrimSpace(value)
if value == "" {
return false
}
for _, r := range value {
if r < '0' || r > '9' {
return false
}
}
return true
}
func preferredProxmoxBackupCommentLabel(comment, entityID string) string {
comment = strings.TrimSpace(comment)
entityID = strings.TrimSpace(entityID)
if comment == "" {
return ""
}
if entityID != "" {
if comment == entityID {
return ""
}
parts := strings.Split(comment, ",")
if len(parts) >= 2 {
first := strings.TrimSpace(parts[0])
last := strings.TrimSpace(parts[len(parts)-1])
if last == entityID && first != "" && first != entityID {
return first
}
}
}
return comment
}
func preferredSubjectLabelFromDetails(p RecoveryPoint, currentLabel string) string {
currentLabel = strings.TrimSpace(currentLabel)
if !strings.HasPrefix(string(p.Provider), "proxmox-") {
return ""
}
entityID := entityIDLabel(p)
if currentLabel != "" && currentLabel != entityID && !isNumericOnlyLabel(currentLabel) {
return ""
}
for _, key := range []string{"comment", "notes"} {
if candidate := preferredProxmoxBackupCommentLabel(detailsString(p, key), entityID); candidate != "" {
return candidate
}
}
return ""
}
func NormalizeRecoveryItemType(value string) string {
t := strings.ToLower(strings.TrimSpace(value))
switch t {
@@ -121,9 +176,15 @@ func subjectLabel(p RecoveryPoint) string {
}
}
if candidate := preferredSubjectLabelFromDetails(p, name); candidate != "" {
return candidate
}
if name != "" {
return name
}
if candidate := preferredSubjectLabelFromDetails(p, id); candidate != "" {
return candidate
}
if ns != "" {
return ns
}
@@ -137,6 +198,9 @@ func subjectLabel(p RecoveryPoint) string {
if rid != "" {
return rid
}
if candidate := preferredSubjectLabelFromDetails(p, ""); candidate != "" {
return candidate
}
return strings.TrimSpace(p.ID)
}
+27 -1
View File
@@ -36,6 +36,28 @@ func guestLookupKey(instanceName, nodeName string, vmid int) string {
return fmt.Sprintf("%s|%s|%d", strings.TrimSpace(instanceName), strings.TrimSpace(nodeName), vmid)
}
func preferredPBSBackupSubjectName(comment, vmid string) string {
comment = strings.TrimSpace(comment)
vmid = strings.TrimSpace(vmid)
if comment == "" {
return ""
}
if vmid != "" {
if comment == vmid {
return ""
}
parts := strings.Split(comment, ",")
if len(parts) >= 2 {
last := strings.TrimSpace(parts[len(parts)-1])
first := strings.TrimSpace(parts[0])
if last == vmid && first != "" && first != vmid {
return first
}
}
}
return comment
}
func proxmoxSubjectRef(resourceType unifiedresources.ResourceType, info GuestInfo, instanceName, nodeName string, vmid int, sourceID string) *recovery.ExternalRef {
name := strings.TrimSpace(info.Name)
if name == "" && vmid > 0 {
@@ -344,10 +366,14 @@ func FromPBSBackups(backups []models.PBSBackup, candidatesByKey map[string][]Gue
} else if guestType == "ct" {
refType = "proxmox-lxc"
}
subjectName := preferredPBSBackupSubjectName(b.Comment, vmidStr)
if subjectName == "" {
subjectName = vmidStr
}
subjectRef = &recovery.ExternalRef{
Type: refType,
Namespace: strings.TrimSpace(b.Instance),
Name: vmidStr,
Name: subjectName,
ID: vmidStr,
Class: "",
}
@@ -249,6 +249,36 @@ func TestFromPBSBackups_Single(t *testing.T) {
}
}
func TestFromPBSBackups_PrefersCommentNameWhenGuestIsUnresolved(t *testing.T) {
backups := []models.PBSBackup{
{
ID: "pbs-backup-legacy",
VMID: "140",
Instance: "pbs-docker",
Namespace: "pimox",
Datastore: "main",
BackupType: "ct",
BackupTime: time.Date(2024, 1, 15, 10, 0, 0, 0, time.UTC),
Comment: "pulse-v4-prod, pi, 140",
},
}
result := FromPBSBackups(backups, nil)
if len(result) != 1 {
t.Fatalf("expected 1 point, got %d", len(result))
}
if result[0].SubjectRef == nil {
t.Fatal("expected SubjectRef to be set")
}
if got := result[0].SubjectRef.Name; got != "pulse-v4-prod" {
t.Fatalf("SubjectRef.Name = %q, want %q", got, "pulse-v4-prod")
}
if got := result[0].SubjectRef.ID; got != "140" {
t.Fatalf("SubjectRef.ID = %q, want %q", got, "140")
}
}
func TestFromPBSBackups_WithCandidates(t *testing.T) {
backups := []models.PBSBackup{
{
+29
View File
@@ -140,6 +140,35 @@ func TestDeriveIndex(t *testing.T) {
DetailsSummary: "local:backup/vm-100/2024-01-15_00:00:00",
},
},
{
name: "PBS falls back to backup comment when ref name is only vmid",
point: RecoveryPoint{
ID: "pbs-backup-legacy-140",
Provider: ProviderProxmoxPBS,
SubjectRef: &ExternalRef{
Type: "proxmox-lxc",
Namespace: "pbs-docker",
Name: "140",
ID: "140",
},
Details: map[string]any{
"comment": "pulse-v4-prod, pi, 140",
"vmid": "140",
},
},
expected: PointIndex{
SubjectLabel: "pulse-v4-prod",
SubjectType: "proxmox-lxc",
ItemType: "system-container",
IsWorkload: true,
ClusterLabel: "pbs-docker",
NodeHostLabel: "",
NamespaceLabel: "root",
EntityIDLabel: "140",
RepositoryLabel: "",
DetailsSummary: "pulse-v4-prod, pi, 140",
},
},
{
name: "TrueNAS with hostname",
point: RecoveryPoint{
@@ -37,7 +37,12 @@ func (s *Store) BackfillIndex(ctx context.Context) error {
namespace_label IS NULL OR
entity_id_label IS NULL OR
repository_label IS NULL OR
details_summary IS NULL)
details_summary IS NULL OR
(provider IN ('proxmox-pve', 'proxmox-pbs') AND
subject_label IS NOT NULL AND TRIM(subject_label) <> '' AND
entity_id_label IS NOT NULL AND TRIM(entity_id_label) <> '' AND
TRIM(subject_label) = TRIM(entity_id_label) AND
details_json IS NOT NULL AND TRIM(details_json) <> ''))
LIMIT `+fmt.Sprint(maxBackfillRows)+`
`)
if err != nil {
+83
View File
@@ -204,6 +204,89 @@ func TestStore_OpenMigratesLegacySchemaMissingItemType(t *testing.T) {
}
}
func TestStore_OpenBackfillsLegacyNumericPBSSubjectLabels(t *testing.T) {
t.Parallel()
dir := t.TempDir()
dbPath := filepath.Join(dir, "recovery.db")
store, err := Open(dbPath)
if err != nil {
t.Fatalf("Open() error = %v", err)
}
now := time.Date(2026, 3, 27, 12, 0, 0, 0, time.UTC)
point := recovery.RecoveryPoint{
ID: "pbs-backup-legacy-140",
Provider: recovery.ProviderProxmoxPBS,
Kind: recovery.KindBackup,
Mode: recovery.ModeRemote,
Outcome: recovery.OutcomeSuccess,
StartedAt: &now,
CompletedAt: &now,
SubjectRef: &recovery.ExternalRef{
Type: "proxmox-lxc",
Namespace: "pbs-docker",
Name: "pulse-v4-prod",
ID: "140",
},
Details: map[string]any{
"comment": "pulse-v4-prod, pi, 140",
"vmid": "140",
"backupType": "ct",
},
}
if err := store.UpsertPoints(context.Background(), []recovery.RecoveryPoint{point}); err != nil {
t.Fatalf("UpsertPoints() error = %v", err)
}
if _, err := store.db.ExecContext(
context.Background(),
`UPDATE recovery_points
SET subject_label = ?, entity_id_label = ?, details_summary = ?
WHERE id = ?`,
"140",
"140",
"pulse-v4-prod, pi, 140",
point.ID,
); err != nil {
t.Fatalf("degrade legacy numeric pbs label: %v", err)
}
if err := store.Close(); err != nil {
t.Fatalf("Close() error = %v", err)
}
reopened, err := Open(dbPath)
if err != nil {
t.Fatalf("reopen Open() error = %v", err)
}
t.Cleanup(func() { _ = reopened.Close() })
points, total, err := reopened.ListPoints(context.Background(), recovery.ListPointsOptions{Page: 1, Limit: 50})
if err != nil {
t.Fatalf("ListPoints() error = %v", err)
}
if total != 1 || len(points) != 1 {
t.Fatalf("ListPoints() total=%d len=%d, want 1/1", total, len(points))
}
if points[0].Display == nil || points[0].Display.SubjectLabel != "pulse-v4-prod" {
t.Fatalf("ListPoints() display = %#v, want backfilled subject label pulse-v4-prod", points[0].Display)
}
rollups, rollupTotal, err := reopened.ListRollups(context.Background(), recovery.ListPointsOptions{Page: 1, Limit: 50})
if err != nil {
t.Fatalf("ListRollups() error = %v", err)
}
if rollupTotal != 1 || len(rollups) != 1 {
t.Fatalf("ListRollups() total=%d len=%d, want 1/1", rollupTotal, len(rollups))
}
if rollups[0].Display == nil || rollups[0].Display.SubjectLabel != "pulse-v4-prod" {
t.Fatalf("ListRollups() display = %#v, want backfilled subject label pulse-v4-prod", rollups[0].Display)
}
}
func createLegacyRecoveryDBWithoutItemType(t *testing.T, dbPath string, point recovery.RecoveryPoint) {
t.Helper()