fix(backup): hide storage paths and stabilize cron next run
- commit
- 63aabba7c6377e6befe770159463172ee5379203
- author
- brice · · 2026-02-07 16:52 UTC
- parent
- 66fbb46
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
7 files · +38 -9
M docs/cli/backup.md +7 -1
| @@ -48,7 +48,7 @@ gordon backup status | |||
| 48 | 48 | ### list | |
| 49 | 49 | ||
| 50 | 50 | ```text | |
| 51 | - | <domain>\t<db>\t<status>\t<started_at>\t<file_path> | |
| 51 | + | <domain>\t<db>\t<status>\t<started_at>\t<backup_id> | |
| 52 | 52 | ``` | |
| 53 | 53 | ||
| 54 | 54 | ### detect | |
| @@ -63,6 +63,12 @@ gordon backup status | |||
| 63 | 63 | <domain>\t<db>\t<status>\t<started_at> | |
| 64 | 64 | ``` | |
| 65 | 65 | ||
| 66 | + | ### run | |
| 67 | + | ||
| 68 | + | ```text | |
| 69 | + | <domain>\t<db>\t<status>\t<started_at>\t<backup_id>\t<size_bytes> | |
| 70 | + | ``` | |
| 71 | + | ||
| 66 | 72 | ## Required Permissions | |
| 67 | 73 | ||
| 68 | 74 | - Read operations (`list`, `status`, `detect`) require `admin:status:read`. | |
| @@ -48,7 +48,7 @@ gordon backup status | |||||
| 48 | ### list | 48 | ### list | ||
| 49 | 49 | ||||
| 50 | ```text | 50 | ```text | ||
| 51 | - | <domain>\t<db>\t<status>\t<started_at>\t<file_path> | 51 | + | <domain>\t<db>\t<status>\t<started_at>\t<backup_id> |
| 52 | ``` | 52 | ``` | ||
| 53 | 53 | ||||
| 54 | ### detect | 54 | ### detect | ||
| @@ -63,6 +63,12 @@ gordon backup status | |||||
| 63 | <domain>\t<db>\t<status>\t<started_at> | 63 | <domain>\t<db>\t<status>\t<started_at> | ||
| 64 | ``` | 64 | ``` | ||
| 65 | 65 | ||||
| 66 | + | ### run | |||
| 67 | + | ||||
| 68 | + | ```text | |||
| 69 | + | <domain>\t<db>\t<status>\t<started_at>\t<backup_id>\t<size_bytes> | |||
| 70 | + | ``` | |||
| 71 | + | ||||
| 66 | ## Required Permissions | 72 | ## Required Permissions | ||
| 67 | 73 | ||||
| 68 | - Read operations (`list`, `status`, `detect`) require `admin:status:read`. | 74 | - Read operations (`list`, `status`, `detect`) require `admin:status:read`. | ||
M internal/adapters/dto/admin_backups.go +0 -1
| @@ -13,7 +13,6 @@ type BackupJob struct { | |||
| 13 | 13 | StartedAt *time.Time `json:"started_at,omitempty"` | |
| 14 | 14 | CompletedAt *time.Time `json:"completed_at,omitempty"` | |
| 15 | 15 | SizeBytes int64 `json:"size_bytes"` | |
| 16 | - | FilePath string `json:"file_path"` | |
| 17 | 16 | Error string `json:"error,omitempty"` | |
| 18 | 17 | } | |
| 19 | 18 | ||
| @@ -13,7 +13,6 @@ type BackupJob struct { | |||||
| 13 | StartedAt *time.Time `json:"started_at,omitempty"` | 13 | StartedAt *time.Time `json:"started_at,omitempty"` | ||
| 14 | CompletedAt *time.Time `json:"completed_at,omitempty"` | 14 | CompletedAt *time.Time `json:"completed_at,omitempty"` | ||
| 15 | SizeBytes int64 `json:"size_bytes"` | 15 | SizeBytes int64 `json:"size_bytes"` | ||
| 16 | - | FilePath string `json:"file_path"` | |||
| 17 | Error string `json:"error,omitempty"` | 16 | Error string `json:"error,omitempty"` | ||
| 18 | } | 17 | } | ||
| 19 | 18 | ||||
M internal/adapters/in/cli/backup.go +4 -4
| @@ -53,12 +53,12 @@ func newBackupListCmd() *cobra.Command { | |||
| 53 | 53 | return nil | |
| 54 | 54 | } | |
| 55 | 55 | w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) | |
| 56 | - | if _, err := fmt.Fprintln(w, "DOMAIN\tDB\tSTATUS\tSTARTED_AT\tFILE_PATH"); err != nil { | |
| 56 | + | if _, err := fmt.Fprintln(w, "DOMAIN\tDB\tSTATUS\tSTARTED_AT\tBACKUP_ID"); err != nil { | |
| 57 | 57 | return err | |
| 58 | 58 | } | |
| 59 | 59 | ||
| 60 | 60 | for _, job := range jobs { | |
| 61 | - | if _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\n", job.Domain, job.DBName, job.Status, formatBackupTime(job.StartedAt), job.FilePath); err != nil { | |
| 61 | + | if _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\n", job.Domain, job.DBName, job.Status, formatBackupTime(job.StartedAt), job.ID); err != nil { | |
| 62 | 62 | return err | |
| 63 | 63 | } | |
| 64 | 64 | } | |
| @@ -94,10 +94,10 @@ func newBackupRunCmd() *cobra.Command { | |||
| 94 | 94 | } | |
| 95 | 95 | ||
| 96 | 96 | w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) | |
| 97 | - | if _, err := fmt.Fprintln(w, "DOMAIN\tDB\tSTATUS\tSTARTED_AT\tFILE_PATH\tSIZE_BYTES"); err != nil { | |
| 97 | + | if _, err := fmt.Fprintln(w, "DOMAIN\tDB\tSTATUS\tSTARTED_AT\tBACKUP_ID\tSIZE_BYTES"); err != nil { | |
| 98 | 98 | return err | |
| 99 | 99 | } | |
| 100 | - | if _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\t%d\n", result.Backup.Domain, result.Backup.DBName, result.Backup.Status, formatBackupTime(result.Backup.StartedAt), result.Backup.FilePath, result.Backup.SizeBytes); err != nil { | |
| 100 | + | if _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\t%d\n", result.Backup.Domain, result.Backup.DBName, result.Backup.Status, formatBackupTime(result.Backup.StartedAt), result.Backup.ID, result.Backup.SizeBytes); err != nil { | |
| 101 | 101 | return err | |
| 102 | 102 | } | |
| 103 | 103 | if err := w.Flush(); err != nil { | |
| @@ -53,12 +53,12 @@ func newBackupListCmd() *cobra.Command { | |||||
| 53 | return nil | 53 | return nil | ||
| 54 | } | 54 | } | ||
| 55 | w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) | 55 | w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) | ||
| 56 | - | if _, err := fmt.Fprintln(w, "DOMAIN\tDB\tSTATUS\tSTARTED_AT\tFILE_PATH"); err != nil { | 56 | + | if _, err := fmt.Fprintln(w, "DOMAIN\tDB\tSTATUS\tSTARTED_AT\tBACKUP_ID"); err != nil { |
| 57 | return err | 57 | return err | ||
| 58 | } | 58 | } | ||
| 59 | 59 | ||||
| 60 | for _, job := range jobs { | 60 | for _, job := range jobs { | ||
| 61 | - | if _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\n", job.Domain, job.DBName, job.Status, formatBackupTime(job.StartedAt), job.FilePath); err != nil { | 61 | + | if _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\n", job.Domain, job.DBName, job.Status, formatBackupTime(job.StartedAt), job.ID); err != nil { |
| 62 | return err | 62 | return err | ||
| 63 | } | 63 | } | ||
| 64 | } | 64 | } | ||
| @@ -94,10 +94,10 @@ func newBackupRunCmd() *cobra.Command { | |||||
| 94 | } | 94 | } | ||
| 95 | 95 | ||||
| 96 | w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) | 96 | w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) | ||
| 97 | - | if _, err := fmt.Fprintln(w, "DOMAIN\tDB\tSTATUS\tSTARTED_AT\tFILE_PATH\tSIZE_BYTES"); err != nil { | 97 | + | if _, err := fmt.Fprintln(w, "DOMAIN\tDB\tSTATUS\tSTARTED_AT\tBACKUP_ID\tSIZE_BYTES"); err != nil { |
| 98 | return err | 98 | return err | ||
| 99 | } | 99 | } | ||
| 100 | - | if _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\t%d\n", result.Backup.Domain, result.Backup.DBName, result.Backup.Status, formatBackupTime(result.Backup.StartedAt), result.Backup.FilePath, result.Backup.SizeBytes); err != nil { | 100 | + | if _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\t%s\t%d\n", result.Backup.Domain, result.Backup.DBName, result.Backup.Status, formatBackupTime(result.Backup.StartedAt), result.Backup.ID, result.Backup.SizeBytes); err != nil { |
| 101 | return err | 101 | return err | ||
| 102 | } | 102 | } | ||
| 103 | if err := w.Flush(); err != nil { | 103 | if err := w.Flush(); err != nil { | ||
M internal/adapters/in/http/admin/handler.go +0 -1
| @@ -104,7 +104,6 @@ func toBackupJobResponse(job domain.BackupJob) dto.BackupJob { | |||
| 104 | 104 | StartedAt: startedAt, | |
| 105 | 105 | CompletedAt: completedAt, | |
| 106 | 106 | SizeBytes: job.SizeBytes, | |
| 107 | - | FilePath: job.FilePath, | |
| 108 | 107 | Error: job.Error, | |
| 109 | 108 | } | |
| 110 | 109 | } | |
| @@ -104,7 +104,6 @@ func toBackupJobResponse(job domain.BackupJob) dto.BackupJob { | |||||
| 104 | StartedAt: startedAt, | 104 | StartedAt: startedAt, | ||
| 105 | CompletedAt: completedAt, | 105 | CompletedAt: completedAt, | ||
| 106 | SizeBytes: job.SizeBytes, | 106 | SizeBytes: job.SizeBytes, | ||
| 107 | - | FilePath: job.FilePath, | |||
| 108 | Error: job.Error, | 107 | Error: job.Error, | ||
| 109 | } | 108 | } | ||
| 110 | } | 109 | } | ||
M internal/adapters/in/http/admin/handler_test.go +2 -1
| @@ -1189,7 +1189,7 @@ func TestHandler_BackupsStatus(t *testing.T) { | |||
| 1189 | 1189 | secretSvc := inmocks.NewMockSecretService(t) | |
| 1190 | 1190 | backupSvc := inmocks.NewMockBackupService(t) | |
| 1191 | 1191 | ||
| 1192 | - | backupSvc.EXPECT().Status(mock.Anything).Return([]domain.BackupJob{{Domain: "app.example.com", DBName: "postgres", Status: domain.BackupStatusCompleted}}, nil) | |
| 1192 | + | backupSvc.EXPECT().Status(mock.Anything).Return([]domain.BackupJob{{Domain: "app.example.com", DBName: "postgres", Status: domain.BackupStatusCompleted, FilePath: "/var/lib/gordon/backups/private.bak"}}, nil) | |
| 1193 | 1193 | ||
| 1194 | 1194 | handler := NewHandler(configSvc, authSvc, containerSvc, inmocks.NewMockHealthService(t), secretSvc, nil, inmocks.NewMockRegistryService(t), nil, testLogger(), backupSvc) | |
| 1195 | 1195 | ||
| @@ -1201,6 +1201,7 @@ func TestHandler_BackupsStatus(t *testing.T) { | |||
| 1201 | 1201 | ||
| 1202 | 1202 | assert.Equal(t, http.StatusOK, rec.Code) | |
| 1203 | 1203 | assert.Contains(t, rec.Body.String(), "app.example.com") | |
| 1204 | + | assert.NotContains(t, rec.Body.String(), "file_path") | |
| 1204 | 1205 | } | |
| 1205 | 1206 | ||
| 1206 | 1207 | func TestHandler_BackupsListDomain(t *testing.T) { | |
| @@ -1189,7 +1189,7 @@ func TestHandler_BackupsStatus(t *testing.T) { | |||||
| 1189 | secretSvc := inmocks.NewMockSecretService(t) | 1189 | secretSvc := inmocks.NewMockSecretService(t) | ||
| 1190 | backupSvc := inmocks.NewMockBackupService(t) | 1190 | backupSvc := inmocks.NewMockBackupService(t) | ||
| 1191 | 1191 | ||||
| 1192 | - | backupSvc.EXPECT().Status(mock.Anything).Return([]domain.BackupJob{{Domain: "app.example.com", DBName: "postgres", Status: domain.BackupStatusCompleted}}, nil) | 1192 | + | backupSvc.EXPECT().Status(mock.Anything).Return([]domain.BackupJob{{Domain: "app.example.com", DBName: "postgres", Status: domain.BackupStatusCompleted, FilePath: "/var/lib/gordon/backups/private.bak"}}, nil) |
| 1193 | 1193 | ||||
| 1194 | handler := NewHandler(configSvc, authSvc, containerSvc, inmocks.NewMockHealthService(t), secretSvc, nil, inmocks.NewMockRegistryService(t), nil, testLogger(), backupSvc) | 1194 | handler := NewHandler(configSvc, authSvc, containerSvc, inmocks.NewMockHealthService(t), secretSvc, nil, inmocks.NewMockRegistryService(t), nil, testLogger(), backupSvc) | ||
| 1195 | 1195 | ||||
| @@ -1201,6 +1201,7 @@ func TestHandler_BackupsStatus(t *testing.T) { | |||||
| 1201 | 1201 | ||||
| 1202 | assert.Equal(t, http.StatusOK, rec.Code) | 1202 | assert.Equal(t, http.StatusOK, rec.Code) | ||
| 1203 | assert.Contains(t, rec.Body.String(), "app.example.com") | 1203 | assert.Contains(t, rec.Body.String(), "app.example.com") | ||
| 1204 | + | assert.NotContains(t, rec.Body.String(), "file_path") | |||
| 1204 | } | 1205 | } | ||
| 1205 | 1206 | ||||
| 1206 | func TestHandler_BackupsListDomain(t *testing.T) { | 1207 | func TestHandler_BackupsListDomain(t *testing.T) { | ||
M internal/usecase/cron/scheduler.go +6 -1
| @@ -227,7 +227,12 @@ func (s *Scheduler) executeEntry(ctx context.Context, e *entry) error { | |||
| 227 | 227 | jobErr = e.job(ctx) | |
| 228 | 228 | }() | |
| 229 | 229 | ||
| 230 | - | nextRun, nextErr := calculateNextRun(now, e.schedule) | |
| 230 | + | completedAt := s.nowFn() | |
| 231 | + | if completedAt.Before(now) { | |
| 232 | + | completedAt = now | |
| 233 | + | } | |
| 234 | + | ||
| 235 | + | nextRun, nextErr := calculateNextRun(completedAt, e.schedule) | |
| 231 | 236 | if nextErr != nil { | |
| 232 | 237 | if jobErr != nil { | |
| 233 | 238 | return fmt.Errorf("job failed: %w; also failed to calculate next run: %v", jobErr, nextErr) | |
| @@ -227,7 +227,12 @@ func (s *Scheduler) executeEntry(ctx context.Context, e *entry) error { | |||||
| 227 | jobErr = e.job(ctx) | 227 | jobErr = e.job(ctx) | ||
| 228 | }() | 228 | }() | ||
| 229 | 229 | ||||
| 230 | - | nextRun, nextErr := calculateNextRun(now, e.schedule) | 230 | + | completedAt := s.nowFn() |
| 231 | + | if completedAt.Before(now) { | |||
| 232 | + | completedAt = now | |||
| 233 | + | } | |||
| 234 | + | ||||
| 235 | + | nextRun, nextErr := calculateNextRun(completedAt, e.schedule) | |||
| 231 | if nextErr != nil { | 236 | if nextErr != nil { | ||
| 232 | if jobErr != nil { | 237 | if jobErr != nil { | ||
| 233 | return fmt.Errorf("job failed: %w; also failed to calculate next run: %v", jobErr, nextErr) | 238 | return fmt.Errorf("job failed: %w; also failed to calculate next run: %v", jobErr, nextErr) | ||
M internal/usecase/cron/scheduler_test.go +19 -0
| @@ -180,3 +180,22 @@ func TestSchedulerStartNoOpWhenStopped(t *testing.T) { | |||
| 180 | 180 | assert.Equal(t, 0, runs) | |
| 181 | 181 | assert.False(t, s.started.Load()) | |
| 182 | 182 | } | |
| 183 | + | ||
| 184 | + | func TestSchedulerRunNowComputesNextRunFromJobFinishTime(t *testing.T) { | |
| 185 | + | s := NewScheduler(zerowrap.Default()) | |
| 186 | + | current := time.Date(2026, 2, 7, 12, 59, 0, 0, time.UTC) | |
| 187 | + | s.nowFn = func() time.Time { return current } | |
| 188 | + | ||
| 189 | + | err := s.Add("backup-hourly", "hourly backup", domain.CronSchedule{Preset: domain.ScheduleHourly}, func(context.Context) error { | |
| 190 | + | current = time.Date(2026, 2, 7, 13, 1, 0, 0, time.UTC) | |
| 191 | + | return nil | |
| 192 | + | }) | |
| 193 | + | require.NoError(t, err) | |
| 194 | + | ||
| 195 | + | err = s.RunNow(context.Background(), "backup-hourly") | |
| 196 | + | require.NoError(t, err) | |
| 197 | + | ||
| 198 | + | entries := s.List() | |
| 199 | + | require.Len(t, entries, 1) | |
| 200 | + | assert.Equal(t, time.Date(2026, 2, 7, 14, 0, 0, 0, time.UTC), entries[0].NextRun) | |
| 201 | + | } | |
| @@ -180,3 +180,22 @@ func TestSchedulerStartNoOpWhenStopped(t *testing.T) { | |||||
| 180 | assert.Equal(t, 0, runs) | 180 | assert.Equal(t, 0, runs) | ||
| 181 | assert.False(t, s.started.Load()) | 181 | assert.False(t, s.started.Load()) | ||
| 182 | } | 182 | } | ||
| 183 | + | ||||
| 184 | + | func TestSchedulerRunNowComputesNextRunFromJobFinishTime(t *testing.T) { | |||
| 185 | + | s := NewScheduler(zerowrap.Default()) | |||
| 186 | + | current := time.Date(2026, 2, 7, 12, 59, 0, 0, time.UTC) | |||
| 187 | + | s.nowFn = func() time.Time { return current } | |||
| 188 | + | ||||
| 189 | + | err := s.Add("backup-hourly", "hourly backup", domain.CronSchedule{Preset: domain.ScheduleHourly}, func(context.Context) error { | |||
| 190 | + | current = time.Date(2026, 2, 7, 13, 1, 0, 0, time.UTC) | |||
| 191 | + | return nil | |||
| 192 | + | }) | |||
| 193 | + | require.NoError(t, err) | |||
| 194 | + | ||||
| 195 | + | err = s.RunNow(context.Background(), "backup-hourly") | |||
| 196 | + | require.NoError(t, err) | |||
| 197 | + | ||||
| 198 | + | entries := s.List() | |||
| 199 | + | require.Len(t, entries, 1) | |||
| 200 | + | assert.Equal(t, time.Date(2026, 2, 7, 14, 0, 0, 0, time.UTC), entries[0].NextRun) | |||
| 201 | + | } | |||