cf5815b fix(backups): address follow-up review comments · gordon

M internal/adapters/in/cli/backup.go +10 -4

@@ -1,6 +1,7 @@
1	1		package cli
2	2
3	3		import (
	4	+	"context"
4	5		"fmt"
5	6		"io"
6	7		"os"

@@ -13,7 +14,12 @@ import (
13	14		"github.com/bnema/gordon/pkg/bytesize"
14	15		)
15	16
16		-	var backupResolveControlPlane = resolveControlPlane
	17	+	var backupResolveControlPlane = func(ctx context.Context, configPath, domainName string) (*controlPlaneHandle, error) {
	18	+	if domainName != "" {
	19	+	return resolveControlPlaneForRouteDomain(ctx, domainName)
	20	+	}
	21	+	return resolveControlPlane(configPath)
	22	+	}
17	23
18	24		// newBackupCmd creates the backup command group.
19	25		func newBackupCmd() *cobra.Command {

@@ -145,7 +151,7 @@ func newVolumeBackupListCmd() *cobra.Command {
145	151		if len(args) == 1 {
146	152		domainName = args[0]
147	153		}
148		-	handle, err := backupResolveControlPlane(configPath)
	154	+	handle, err := backupResolveControlPlane(cmd.Context(), configPath, domainName)
149	155		if err != nil {
150	156		return err
151	157		}

@@ -175,7 +181,7 @@ func newVolumeBackupRunCmd() *cobra.Command {
175	181		if len(args) == 1 {
176	182		domainName = args[0]
177	183		}
178		-	handle, err := backupResolveControlPlane(configPath)
	184	+	handle, err := backupResolveControlPlane(cmd.Context(), configPath, domainName)
179	185		if err != nil {
180	186		return err
181	187		}

@@ -204,7 +210,7 @@ func newVolumeBackupStatusCmd() *cobra.Command {
204	210		Use: "status",
205	211		Short: "Show volume backup status",
206	212		RunE: func(cmd *cobra.Command, args []string) error {
207		-	handle, err := backupResolveControlPlane(configPath)
	213	+	handle, err := backupResolveControlPlane(cmd.Context(), configPath, "")
208	214		if err != nil {
209	215		return err
210	216		}

 package cli
 import (
+	"context"
 	"fmt"
 	"io"
 	"os"
 	"github.com/bnema/gordon/pkg/bytesize"
 )
-var backupResolveControlPlane = resolveControlPlane
+var backupResolveControlPlane = func(ctx context.Context, configPath, domainName string) (*controlPlaneHandle, error) {
+	if domainName != "" {
+		return resolveControlPlaneForRouteDomain(ctx, domainName)
+	}
+	return resolveControlPlane(configPath)
+}
 // newBackupCmd creates the backup command group.
 func newBackupCmd() *cobra.Command {
 			if len(args) == 1 {
 				domainName = args[0]
 			}
-			handle, err := backupResolveControlPlane(configPath)
+			handle, err := backupResolveControlPlane(cmd.Context(), configPath, domainName)
 			if err != nil {
 				return err
 			}
 			if len(args) == 1 {
 				domainName = args[0]
 			}
-			handle, err := backupResolveControlPlane(configPath)
+			handle, err := backupResolveControlPlane(cmd.Context(), configPath, domainName)
 			if err != nil {
 				return err
 			}
 		Use:   "status",
 		Short: "Show volume backup status",
 		RunE: func(cmd *cobra.Command, args []string) error {
-			handle, err := backupResolveControlPlane(configPath)
+			handle, err := backupResolveControlPlane(cmd.Context(), configPath, "")
 			if err != nil {
 				return err
 			}

M internal/adapters/in/cli/backup_volume_test.go +20 -1


@@ -16,12 +16,31 @@ import (
16	16		func withBackupControlPlane(t *testing.T, plane ControlPlane) {
17	17		t.Helper()
18	18		old := backupResolveControlPlane
19		-	backupResolveControlPlane = func(string) (*controlPlaneHandle, error) {
	19	+	backupResolveControlPlane = func(context.Context, string, string) (*controlPlaneHandle, error) {
20	20		return &controlPlaneHandle{plane: plane}, nil
21	21		}
22	22		t.Cleanup(func() { backupResolveControlPlane = old })
23	23		}
24	24
	25	+	func TestVolumeBackupRun_UsesDomainAwareResolver(t *testing.T) {
	26	+	plane := climocks.NewMockControlPlane(t)
	27	+	old := backupResolveControlPlane
	28	+	var gotDomain string
	29	+	backupResolveControlPlane = func(_ context.Context, _ string, domainName string) (*controlPlaneHandle, error) {
	30	+	gotDomain = domainName
	31	+	return &controlPlaneHandle{plane: plane}, nil
	32	+	}
	33	+	t.Cleanup(func() { backupResolveControlPlane = old })
	34	+
	35	+	plane.EXPECT().RunVolumeBackups(mock.Anything, "app.example.com", "").Return(&dto.VolumeBackupRunResponse{Status: "ok"}, nil)
	36	+
	37	+	cmd := newVolumeBackupRunCmd()
	38	+	cmd.SetArgs([]string{"app.example.com"})
	39	+
	40	+	require.NoError(t, cmd.ExecuteContext(context.Background()))
	41	+	require.Equal(t, "app.example.com", gotDomain)
	42	+	}
	43	+
25	44		func TestVolumeBackupRun_JSONFlag(t *testing.T) {
26	45		plane := climocks.NewMockControlPlane(t)
27	46		withBackupControlPlane(t, plane)


@@ -16,12 +16,31 @@ import (
16		func withBackupControlPlane(t *testing.T, plane ControlPlane) {	16		func withBackupControlPlane(t *testing.T, plane ControlPlane) {
17		t.Helper()	17		t.Helper()
18		old := backupResolveControlPlane	18		old := backupResolveControlPlane
19	-	backupResolveControlPlane = func(string) (*controlPlaneHandle, error) {	19	+	backupResolveControlPlane = func(context.Context, string, string) (*controlPlaneHandle, error) {
20		return &controlPlaneHandle{plane: plane}, nil	20		return &controlPlaneHandle{plane: plane}, nil
21		}	21		}
22		t.Cleanup(func() { backupResolveControlPlane = old })	22		t.Cleanup(func() { backupResolveControlPlane = old })
23		}	23		}
24			24
			25	+	func TestVolumeBackupRun_UsesDomainAwareResolver(t *testing.T) {
			26	+	plane := climocks.NewMockControlPlane(t)
			27	+	old := backupResolveControlPlane
			28	+	var gotDomain string
			29	+	backupResolveControlPlane = func(_ context.Context, _ string, domainName string) (*controlPlaneHandle, error) {
			30	+	gotDomain = domainName
			31	+	return &controlPlaneHandle{plane: plane}, nil
			32	+	}
			33	+	t.Cleanup(func() { backupResolveControlPlane = old })
			34	+
			35	+	plane.EXPECT().RunVolumeBackups(mock.Anything, "app.example.com", "").Return(&dto.VolumeBackupRunResponse{Status: "ok"}, nil)
			36	+
			37	+	cmd := newVolumeBackupRunCmd()
			38	+	cmd.SetArgs([]string{"app.example.com"})
			39	+
			40	+	require.NoError(t, cmd.ExecuteContext(context.Background()))
			41	+	require.Equal(t, "app.example.com", gotDomain)
			42	+	}
			43	+
25		func TestVolumeBackupRun_JSONFlag(t *testing.T) {	44		func TestVolumeBackupRun_JSONFlag(t *testing.T) {
26		plane := climocks.NewMockControlPlane(t)	45		plane := climocks.NewMockControlPlane(t)
27		withBackupControlPlane(t, plane)	46		withBackupControlPlane(t, plane)

M internal/usecase/backup/volume_service.go +6 -2


@@ -45,7 +45,11 @@ func (s *VolumeService) ListVolumeBackups(ctx context.Context, domainName string
45	45		zerowrap.FieldUseCase: "ListVolumeBackups",
46	46		"domain": domainName,
47	47		})
48		-	return s.storage.ListVolumeArchives(ctx, domainName)
	48	+	jobs, err := s.storage.ListVolumeArchives(ctx, domainName)
	49	+	if err != nil {
	50	+	return nil, fmt.Errorf("list volume archives: %w", err)
	51	+	}
	52	+	return jobs, nil
49	53		}
50	54
51	55		// VolumeBackupStatus returns completed backup artifacts plus current/recent in-memory job state.

@@ -56,7 +60,7 @@ func (s *VolumeService) VolumeBackupStatus(ctx context.Context) ([]domain.Volume
56	60		})
57	61		jobs, err := s.storage.ListVolumeArchives(ctx, "")
58	62		if err != nil {
59		-	return nil, err
	63	+	return nil, fmt.Errorf("list volume backup status: %w", err)
60	64		}
61	65
62	66		s.mu.Lock()

 		zerowrap.FieldUseCase: "ListVolumeBackups",
 		"domain":              domainName,
 	})
-	return s.storage.ListVolumeArchives(ctx, domainName)
+	jobs, err := s.storage.ListVolumeArchives(ctx, domainName)
+	if err != nil {
+		return nil, fmt.Errorf("list volume archives: %w", err)
+	}
+	return jobs, nil
 }
 // VolumeBackupStatus returns completed backup artifacts plus current/recent in-memory job state.
 	})
 	jobs, err := s.storage.ListVolumeArchives(ctx, "")
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("list volume backup status: %w", err)
 	}
 	s.mu.Lock()

M internal/usecase/backup/volume_service_test.go +26 -0


@@ -35,6 +35,7 @@ func (f fakeVolumeArchiveExporter) ExportVolumeArchive(context.Context, domain.V
35	35		type fakeVolumeBackupStorage struct {
36	36		stored []domain.VolumeBackupJob
37	37		retention []string
	38	+	listErr error
38	39		}
39	40
40	41		func (f *fakeVolumeBackupStorage) StoreVolumeArchive(_ context.Context, job domain.VolumeBackupJob, data io.Reader) (string, error) {

@@ -50,6 +51,9 @@ func (f *fakeVolumeBackupStorage) GetVolumeArchive(context.Context, string) (io.
50	51		}
51	52
52	53		func (f *fakeVolumeBackupStorage) ListVolumeArchives(context.Context, string) ([]domain.VolumeBackupJob, error) {
	54	+	if f.listErr != nil {
	55	+	return nil, f.listErr
	56	+	}
53	57		return nil, nil
54	58		}
55	59

@@ -128,6 +132,28 @@ func TestVolumeServiceRunVolumeBackupsReturnsPartialFailure(t *testing.T) {
128	132		assert.Empty(t, storage.stored)
129	133		}
130	134
	135	+	func TestVolumeServiceListVolumeBackupsWrapsStorageError(t *testing.T) {
	136	+	storageErr := fmt.Errorf("s3 unavailable")
	137	+	svc := NewVolumeService(outmocks.NewMockContainerRuntime(t), fakeVolumeArchiveExporter{}, &fakeVolumeBackupStorage{listErr: storageErr}, domain.VolumeBackupConfig{}, testLogger())
	138	+
	139	+	jobs, err := svc.ListVolumeBackups(context.Background(), "app.example.com")
	140	+
	141	+	require.ErrorIs(t, err, storageErr)
	142	+	assert.Contains(t, err.Error(), "list volume archives")
	143	+	assert.Nil(t, jobs)
	144	+	}
	145	+
	146	+	func TestVolumeServiceStatusWrapsStorageError(t *testing.T) {
	147	+	storageErr := fmt.Errorf("s3 unavailable")
	148	+	svc := NewVolumeService(outmocks.NewMockContainerRuntime(t), fakeVolumeArchiveExporter{}, &fakeVolumeBackupStorage{listErr: storageErr}, domain.VolumeBackupConfig{}, testLogger())
	149	+
	150	+	jobs, err := svc.VolumeBackupStatus(context.Background())
	151	+
	152	+	require.ErrorIs(t, err, storageErr)
	153	+	assert.Contains(t, err.Error(), "list volume backup status")
	154	+	assert.Nil(t, jobs)
	155	+	}
	156	+
131	157		func TestVolumeServiceDisabledDoesNothing(t *testing.T) {
132	158		runtime := outmocks.NewMockContainerRuntime(t)
133	159		svc := NewVolumeService(runtime, fakeVolumeArchiveExporter{}, &fakeVolumeBackupStorage{}, domain.VolumeBackupConfig{}, testLogger())


@@ -35,6 +35,7 @@ func (f fakeVolumeArchiveExporter) ExportVolumeArchive(context.Context, domain.V
35	type fakeVolumeBackupStorage struct {	35		type fakeVolumeBackupStorage struct {
36	stored []domain.VolumeBackupJob	36		stored []domain.VolumeBackupJob
37	retention []string	37		retention []string
		38	+	listErr error
38	}	39		}
39		40
40	func (f *fakeVolumeBackupStorage) StoreVolumeArchive(_ context.Context, job domain.VolumeBackupJob, data io.Reader) (string, error) {	41		func (f *fakeVolumeBackupStorage) StoreVolumeArchive(_ context.Context, job domain.VolumeBackupJob, data io.Reader) (string, error) {

@@ -50,6 +51,9 @@ func (f *fakeVolumeBackupStorage) GetVolumeArchive(context.Context, string) (io.
50	}	51		}
51		52
52	func (f *fakeVolumeBackupStorage) ListVolumeArchives(context.Context, string) ([]domain.VolumeBackupJob, error) {	53		func (f *fakeVolumeBackupStorage) ListVolumeArchives(context.Context, string) ([]domain.VolumeBackupJob, error) {
		54	+	if f.listErr != nil {
		55	+	return nil, f.listErr
		56	+	}
53	return nil, nil	57		return nil, nil
54	}	58		}
55		59

@@ -128,6 +132,28 @@ func TestVolumeServiceRunVolumeBackupsReturnsPartialFailure(t *testing.T) {
128	assert.Empty(t, storage.stored)	132		assert.Empty(t, storage.stored)
129	}	133		}
130		134
		135	+	func TestVolumeServiceListVolumeBackupsWrapsStorageError(t *testing.T) {
		136	+	storageErr := fmt.Errorf("s3 unavailable")
		137	+	svc := NewVolumeService(outmocks.NewMockContainerRuntime(t), fakeVolumeArchiveExporter{}, &fakeVolumeBackupStorage{listErr: storageErr}, domain.VolumeBackupConfig{}, testLogger())
		138	+
		139	+	jobs, err := svc.ListVolumeBackups(context.Background(), "app.example.com")
		140	+
		141	+	require.ErrorIs(t, err, storageErr)
		142	+	assert.Contains(t, err.Error(), "list volume archives")
		143	+	assert.Nil(t, jobs)
		144	+	}
		145	+
		146	+	func TestVolumeServiceStatusWrapsStorageError(t *testing.T) {
		147	+	storageErr := fmt.Errorf("s3 unavailable")
		148	+	svc := NewVolumeService(outmocks.NewMockContainerRuntime(t), fakeVolumeArchiveExporter{}, &fakeVolumeBackupStorage{listErr: storageErr}, domain.VolumeBackupConfig{}, testLogger())
		149	+
		150	+	jobs, err := svc.VolumeBackupStatus(context.Background())
		151	+
		152	+	require.ErrorIs(t, err, storageErr)
		153	+	assert.Contains(t, err.Error(), "list volume backup status")
		154	+	assert.Nil(t, jobs)
		155	+	}
		156	+
131	func TestVolumeServiceDisabledDoesNothing(t *testing.T) {	157		func TestVolumeServiceDisabledDoesNothing(t *testing.T) {
132	runtime := outmocks.NewMockContainerRuntime(t)	158		runtime := outmocks.NewMockContainerRuntime(t)
133	svc := NewVolumeService(runtime, fakeVolumeArchiveExporter{}, &fakeVolumeBackupStorage{}, domain.VolumeBackupConfig{}, testLogger())	159		svc := NewVolumeService(runtime, fakeVolumeArchiveExporter{}, &fakeVolumeBackupStorage{}, domain.VolumeBackupConfig{}, testLogger())