Ensure restores can target the backup id or alias (#243)

* Ensure restores can target the backup id or alias

* Fix test

* Suppress cyclomatic complexity warnings in test files

* specify test pattern in deepsource config

Author: davissp14

.deepsource.toml

@@ -1,5 +1,9 @@
 version = 1
 
+test_patterns = [
+  "**/*_test.go"
+]
+
 [[analyzers]]
 name = "shell"
 

cmd/flexctl/backups.go

@@ -36,7 +36,7 @@ var backupCreateCmd = &cobra.Command{
 		}
 
 		if err := createBackup(cmd); err != nil {
-			return err
+			return fmt.Errorf("failed to create backup: %v", err)
 		}
 
 		fmt.Println("Backup completed successfully!")
@@ -47,7 +47,7 @@ var backupCreateCmd = &cobra.Command{
 }
 
 var backupShowCmd = &cobra.Command{
-	Use:   "show",
+	Use:   "show <backup-id>",
 	Short: "Shows details about a specific backup",
 	Long:  `Shows details about a specific backup.`,
 	RunE: func(cmd *cobra.Command, args []string) error {
@@ -137,7 +137,7 @@ func createBackup(cmd *cobra.Command) error {
 	fmt.Println("Performing backup...")
 
 	if _, err := barman.Backup(ctx, cfg); err != nil {
-		return fmt.Errorf("failed to create backup: %v", err)
+		return err
 	}
 
 	return nil
@@ -190,7 +190,7 @@ func listBackups(cmd *cobra.Command) error {
 	}
 
 	table := tablewriter.NewWriter(os.Stdout)
-	table.SetHeader([]string{"ID", "Name", "Status", "End time", "Begin WAL"})
+	table.SetHeader([]string{"ID/Name", "Alias", "Status", "End time", "Begin WAL"})
 
 	// Set table alignment, borders, padding, etc. as needed
 	table.SetAlignment(tablewriter.ALIGN_LEFT)

cmd/flexctl/main.go

@@ -8,15 +8,19 @@ import (
 )
 
 func main() {
-	var rootCmd = &cobra.Command{Use: "flexctl"}
+	var rootCmd = &cobra.Command{
+		Use:           "flexctl",
+		SilenceErrors: true,
+		SilenceUsage:  true,
+	}
 
 	// Backup commands
 	var backupCmd = &cobra.Command{Use: "backup"}
 
 	rootCmd.AddCommand(backupCmd)
 	backupCmd.AddCommand(backupListCmd)
-	backupCmd.AddCommand(backupCreateCmd)
 	backupCmd.AddCommand(backupShowCmd)
+	backupCmd.AddCommand(backupCreateCmd)
 
 	if err := rootCmd.Execute(); err != nil {
 		fmt.Println(err)
@@ -25,12 +29,12 @@ func main() {
 }
 
 func init() {
-	// Backup commands
+	// Backup list
 	backupListCmd.Flags().StringP("status", "s", "", "Filter backups by status (Not applicable for JSON output)")
 	backupListCmd.Flags().BoolP("json", "", false, "Output in JSON format")
-
+	// Backup show
 	backupShowCmd.Flags().BoolP("json", "", false, "Output in JSON format")
-
+	// Backup create
 	backupCreateCmd.Flags().StringP("name", "n", "", "Name of the backup")
 	backupCreateCmd.Flags().BoolP("immediate-checkpoint", "", false, "Forces Postgres to perform an immediate checkpoint")
 }

internal/flypg/barman.go

@@ -127,21 +127,25 @@ func (b *Barman) Backup(ctx context.Context, cfg BackupConfig) ([]byte, error) {
 	}
 
 	if cfg.Name != "" {
+		// Ensure the alias is unique, otherwise we won't be able to restore using it.
+		if err := b.validateBackupName(ctx, cfg.Name); err != nil {
+			return nil, err
+		}
 		args = append(args, "-n", cfg.Name)
 	}
 
 	return utils.RunCmd(ctx, "postgres", "barman-cloud-backup", args...)
 }
 
 // RestoreBackup returns the command string used to restore a base backup.
-func (b *Barman) RestoreBackup(ctx context.Context, backupID string) ([]byte, error) {
+func (b *Barman) RestoreBackup(ctx context.Context, name string) ([]byte, error) {
 	args := []string{
 		"--cloud-provider", providerDefault,
 		"--endpoint-url", b.endpoint,
 		"--profile", b.authProfile,
 		b.BucketURL(),
 		b.bucketDirectory,
-		backupID,
+		name,
 		defaultRestoreDir,
 	}
 
@@ -287,6 +291,21 @@ func (*Barman) parseBackups(backupBytes []byte) (BackupList, error) {
 	return backupList, nil
 }
 
+func (b *Barman) validateBackupName(ctx context.Context, name string) error {
+	backupList, err := b.ListBackups(ctx)
+	if err != nil {
+		return fmt.Errorf("failed to list backups: %s", err)
+	}
+
+	for _, backup := range backupList.Backups {
+		if backup.Name == name {
+			return fmt.Errorf("backup name '%s' already exists", name)
+		}
+	}
+
+	return nil
+}
+
 func formatTimestamp(timestamp string) (string, error) {
 	if strings.HasSuffix(timestamp, "Z") {
 		timestamp = strings.TrimSuffix(timestamp, "Z") + "+00:00"

internal/flypg/barman_restore.go

@@ -164,9 +164,9 @@ func (b *BarmanRestore) restoreFromBackup(ctx context.Context) error {
 		}
 	case b.recoveryTargetName != "":
 		// Resolve the target base backup
-		backupID, err = b.resolveBackupFromID(backups, b.recoveryTargetName)
+		backupID, err = b.resolveBackupFromName(backups, b.recoveryTargetName)
 		if err != nil {
-			return fmt.Errorf("failed to resolve backup target by id: %s", err)
+			return fmt.Errorf("failed to resolve backup target by id/name: %s", err)
 		}
 	default:
 		backupID, err = b.resolveBackupFromTime(backups, time.Now().Format(time.RFC3339))
@@ -192,18 +192,19 @@ func (b *BarmanRestore) restoreFromBackup(ctx context.Context) error {
 	return nil
 }
 
-func (*BarmanRestore) resolveBackupFromID(backupList BackupList, id string) (string, error) {
+func (*BarmanRestore) resolveBackupFromName(backupList BackupList, name string) (string, error) {
 	if len(backupList.Backups) == 0 {
 		return "", fmt.Errorf("no backups found")
 	}
 
 	for _, backup := range backupList.Backups {
-		if backup.ID == id {
+		// Allow for either the backup ID or the backup name to be used.
+		if backup.ID == name || backup.Name == name {
 			return backup.ID, nil
 		}
 	}
 
-	return "", fmt.Errorf("no backup found with id %s", id)
+	return "", fmt.Errorf("no backup found with id/name %s", name)
 }
 
 func (*BarmanRestore) resolveBackupFromTime(backupList BackupList, restoreStr string) (string, error) {

internal/flypg/barman_restore_test.go

@@ -43,7 +43,6 @@ const backupsResponse = `{
         },
         {
             "backup_label": "'START WAL LOCATION: 0/8000028 (file 000000010000000000000008)\\nCHECKPOINT LOCATION: 0/8000098\\nBACKUP METHOD: streamed\\nBACKUP FROM: primary\\nSTART TIME: 2024-06-25 19:44:13 UTC\\nLABEL: Barman backup cloud 20240625T194412\\nSTART TIMELINE: 1\\n'",
-            "backup_name": "test-backup-2",
             "begin_offset": 40,
             "begin_time": "Tue Jun 25 19:44:12 2024",
             "begin_wal": "000000010000000000000008",
@@ -87,6 +86,7 @@ const backupsResponse = `{
             "timeline": 1,
             "version": 150006,
             "xlog_segment_size": 16777216,
+			"backup_name": "test-backup-1",
             "backup_id": "20240625T194412"
         },
         {
@@ -212,6 +212,19 @@ func TestNewBarmanRestore(t *testing.T) {
 		}
 	})
 
+	t.Run("target-name-with-alias", func(t *testing.T) {
+		t.Setenv("S3_ARCHIVE_REMOTE_RESTORE_CONFIG", "https://my-key:my-secret@fly.storage.tigris.dev/my-bucket/my-directory?targetName=test-backup-1")
+
+		restore, err := NewBarmanRestore(os.Getenv("S3_ARCHIVE_REMOTE_RESTORE_CONFIG"))
+		if err != nil {
+			t.Fatalf("NewBarmanRestore failed with: %v", err)
+		}
+
+		if restore.recoveryTargetName != "test-backup-1" {
+			t.Fatalf("expected recovery target name to be test-backup-1, got %s", restore.recoveryTargetName)
+		}
+	})
+
 	t.Run("target-name-with-options", func(t *testing.T) {
 		t.Setenv("S3_ARCHIVE_REMOTE_RESTORE_CONFIG", "https://my-key:my-secret@fly.storage.tigris.dev/my-bucket/my-directory?targetName=20240705T051010&targetAction=shutdown&targetTimeline=2&targetInclusive=false")
 
@@ -277,36 +290,57 @@ func TestParseBackups(t *testing.T) {
 			t.Fatalf("expected 2 backups, got %d", len(list.Backups))
 		}
 
-		firstBackup := list.Backups[0]
-		if firstBackup.ID != "20240702T210544" {
-			t.Fatalf("expected backup ID to be 20240625T194412, got %s", firstBackup.ID)
-		}
+		t.Run("first-backup", func(t *testing.T) {
+			backup := list.Backups[0]
+			if backup.ID != "20240702T210544" {
+				t.Fatalf("expected backup ID to be 20240625T194412, got %s", backup.ID)
+			}
 
-		if firstBackup.StartTime != "Tue Jun 24 19:44:20 2024" {
-			t.Fatalf("expected start time to be Tue Jun 24 19:44:20 2024, got %s", firstBackup.StartTime)
-		}
+			if backup.StartTime != "Tue Jun 24 19:44:20 2024" {
+				t.Fatalf("expected start time to be Tue Jun 24 19:44:20 2024, got %s", backup.StartTime)
+			}
 
-		if firstBackup.EndTime != "" {
-			t.Fatalf("expected end time to be empty, but got %s", firstBackup.EndTime)
-		}
+			if backup.EndTime != "" {
+				t.Fatalf("expected end time to be empty, but got %s", backup.EndTime)
+			}
 
-		if firstBackup.Status != "FAILED" {
-			t.Fatalf("expected status to be FAILED, got %s", firstBackup.Status)
-		}
+			if backup.Status != "FAILED" {
+				t.Fatalf("expected status to be FAILED, got %s", backup.Status)
+			}
 
-		secondBackup := list.Backups[2]
+			if backup.Name != "" {
+				t.Fatalf("expected name to be empty, but got %s", backup.Name)
+			}
 
-		if secondBackup.ID != "20240626T172443" {
-			t.Fatalf("expected backup ID to be 20240626T172443, got %s", secondBackup.ID)
-		}
+		})
 
-		if secondBackup.StartTime != "Wed Jun 26 17:24:43 2024" {
-			t.Fatalf("expected start time to be Wed Jun 26 17:24:43 2024, got %s", secondBackup.StartTime)
-		}
+		t.Run("second-backup", func(t *testing.T) {
+			backup := list.Backups[1]
+			if backup.Status != "DONE" {
+				t.Fatalf("expected status to be DONE, got %s", backup.Status)
+			}
 
-		if secondBackup.EndTime != "Wed Jun 26 17:27:02 2024" {
-			t.Fatalf("expected end time to be Wed Jun 26 17:27:02 2024, got %s", secondBackup.EndTime)
-		}
+			if backup.Name != "test-backup-1" {
+				t.Fatalf("expected name to be test-backup-1, got %s", backup.Name)
+			}
+		})
+
+		t.Run("third-backup", func(t *testing.T) {
+			backup := list.Backups[2]
+
+			if backup.ID != "20240626T172443" {
+				t.Fatalf("expected backup ID to be 20240626T172443, got %s", backup.ID)
+			}
+
+			if backup.StartTime != "Wed Jun 26 17:24:43 2024" {
+				t.Fatalf("expected start time to be Wed Jun 26 17:24:43 2024, got %s", backup.StartTime)
+			}
+
+			if backup.EndTime != "Wed Jun 26 17:27:02 2024" {
+				t.Fatalf("expected end time to be Wed Jun 26 17:27:02 2024, got %s", backup.EndTime)
+			}
+
+		})
 	})
 }
 
@@ -360,6 +394,29 @@ func TestResolveBackupTarget(t *testing.T) {
 			t.Fatalf("expected backup ID to be 20240626T172443, got %s", backupID)
 		}
 	})
+
+	t.Run("resolve-backup-by-name", func(t *testing.T) {
+		backupID, err := restore.resolveBackupFromName(list, "20240625T194412")
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+
+		if backupID != "20240625T194412" {
+			t.Fatalf("expected backup ID to be 20240625T194412, got %s", backupID)
+		}
+	})
+
+	t.Run("resolve-backup-by-name-with-alias", func(t *testing.T) {
+		// resolve backup by alias
+		backupID, err := restore.resolveBackupFromName(list, "test-backup-1")
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+
+		if backupID != "20240625T194412" {
+			t.Fatalf("expected backup ID to be 20240625T194412, got %s", backupID)
+		}
+	})
 }
 
 func setRestoreDefaultEnv(t *testing.T) {