fix(backup): harden backup flows, scheduler safety, and admin/cli handling
- commit
- 1ac3076eec7557489663400f8b9e6db05c66356e
- author
- brice · · 2026-02-07 16:03 UTC
- parent
- 28497b7
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
19 files · +358 -45
M docs/concepts.md +1 -0
| @@ -224,6 +224,7 @@ Current design: | |||
| 224 | 224 | ||
| 225 | 225 | This is intentionally scoped for operational safety and predictable behavior. | |
| 226 | 226 | Future extensions can add physical backups, PITR, and remote object storage adapters. | |
| 227 | + | For configuration details and usage examples, see the [Backups Configuration guide](./config/backups.md), [Backup CLI reference](./cli/backup.md), and [Configuration Reference](./config/reference.md). | |
| 227 | 228 | ||
| 228 | 229 | ## Container Labels | |
| 229 | 230 | ||
| @@ -224,6 +224,7 @@ Current design: | |||||
| 224 | 224 | ||||
| 225 | This is intentionally scoped for operational safety and predictable behavior. | 225 | This is intentionally scoped for operational safety and predictable behavior. | ||
| 226 | Future extensions can add physical backups, PITR, and remote object storage adapters. | 226 | Future extensions can add physical backups, PITR, and remote object storage adapters. | ||
| 227 | + | For configuration details and usage examples, see the [Backups Configuration guide](./config/backups.md), [Backup CLI reference](./cli/backup.md), and [Configuration Reference](./config/reference.md). | |||
| 227 | 228 | ||||
| 228 | ## Container Labels | 229 | ## Container Labels | ||
| 229 | 230 | ||||
M internal/adapters/dto/admin_backups.go +2 -2
| @@ -29,8 +29,8 @@ type BackupRunRequest struct { | |||
| 29 | 29 | ||
| 30 | 30 | // BackupRunResponse is returned after triggering a backup. | |
| 31 | 31 | type BackupRunResponse struct { | |
| 32 | - | Status string `json:"status"` | |
| 33 | - | Backup BackupJob `json:"backup"` | |
| 32 | + | Status string `json:"status"` | |
| 33 | + | Backup *BackupJob `json:"backup,omitempty"` | |
| 34 | 34 | } | |
| 35 | 35 | ||
| 36 | 36 | // DatabaseInfo represents a detected database attachment. | |
| @@ -29,8 +29,8 @@ type BackupRunRequest struct { | |||||
| 29 | 29 | ||||
| 30 | // BackupRunResponse is returned after triggering a backup. | 30 | // BackupRunResponse is returned after triggering a backup. | ||
| 31 | type BackupRunResponse struct { | 31 | type BackupRunResponse struct { | ||
| 32 | - | Status string `json:"status"` | 32 | + | Status string `json:"status"` |
| 33 | - | Backup BackupJob `json:"backup"` | 33 | + | Backup *BackupJob `json:"backup,omitempty"` |
| 34 | } | 34 | } | ||
| 35 | 35 | ||||
| 36 | // DatabaseInfo represents a detected database attachment. | 36 | // DatabaseInfo represents a detected database attachment. | ||
M internal/adapters/in/cli/backup.go +46 -8
| @@ -3,6 +3,8 @@ package cli | |||
| 3 | 3 | import ( | |
| 4 | 4 | "context" | |
| 5 | 5 | "fmt" | |
| 6 | + | "os" | |
| 7 | + | "text/tabwriter" | |
| 6 | 8 | "time" | |
| 7 | 9 | ||
| 8 | 10 | "github.com/spf13/cobra" | |
| @@ -51,10 +53,18 @@ func newBackupListCmd() *cobra.Command { | |||
| 51 | 53 | fmt.Println("No backups found") | |
| 52 | 54 | return nil | |
| 53 | 55 | } | |
| 54 | - | fmt.Println("DOMAIN\tDB\tSTATUS\tSTARTED_AT\tFILE_PATH") | |
| 56 | + | w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) | |
| 57 | + | if _, err := fmt.Fprintln(w, "DOMAIN\tDB\tSTATUS\tSTARTED_AT\tFILE_PATH"); err != nil { | |
| 58 | + | return err | |
| 59 | + | } | |
| 55 | 60 | ||
| 56 | 61 | for _, job := range jobs { | |
| 57 | - | fmt.Printf("%s\t%s\t%s\t%s\t%s\n", job.Domain, job.DBName, job.Status, formatBackupTime(job.StartedAt), job.FilePath) | |
| 62 | + | 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 { | |
| 63 | + | return err | |
| 64 | + | } | |
| 65 | + | } | |
| 66 | + | if err := w.Flush(); err != nil { | |
| 67 | + | return err | |
| 58 | 68 | } | |
| 59 | 69 | ||
| 60 | 70 | return nil | |
| @@ -80,8 +90,20 @@ func newBackupRunCmd() *cobra.Command { | |||
| 80 | 90 | return fmt.Errorf("failed to run backup: %w", err) | |
| 81 | 91 | } | |
| 82 | 92 | ||
| 83 | - | fmt.Println("DOMAIN\tDB\tSTATUS\tSTARTED_AT\tFILE_PATH\tSIZE_BYTES") | |
| 84 | - | fmt.Printf("%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) | |
| 93 | + | if result.Backup == nil { | |
| 94 | + | return fmt.Errorf("backup run completed without backup payload") | |
| 95 | + | } | |
| 96 | + | ||
| 97 | + | w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) | |
| 98 | + | if _, err := fmt.Fprintln(w, "DOMAIN\tDB\tSTATUS\tSTARTED_AT\tFILE_PATH\tSIZE_BYTES"); err != nil { | |
| 99 | + | return err | |
| 100 | + | } | |
| 101 | + | 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 { | |
| 102 | + | return err | |
| 103 | + | } | |
| 104 | + | if err := w.Flush(); err != nil { | |
| 105 | + | return err | |
| 106 | + | } | |
| 85 | 107 | return nil | |
| 86 | 108 | }, | |
| 87 | 109 | } | |
| @@ -110,10 +132,18 @@ func newBackupDetectCmd() *cobra.Command { | |||
| 110 | 132 | fmt.Println("No supported databases detected") | |
| 111 | 133 | return nil | |
| 112 | 134 | } | |
| 113 | - | fmt.Println("NAME\tTYPE\tHOST\tPORT\tIMAGE") | |
| 135 | + | w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) | |
| 136 | + | if _, err := fmt.Fprintln(w, "NAME\tTYPE\tHOST\tPORT\tIMAGE"); err != nil { | |
| 137 | + | return err | |
| 138 | + | } | |
| 114 | 139 | ||
| 115 | 140 | for _, db := range dbs { | |
| 116 | - | fmt.Printf("%s\t%s\t%s\t%d\t%s\n", db.Name, db.Type, db.Host, db.Port, db.ImageName) | |
| 141 | + | if _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%d\t%s\n", db.Name, db.Type, db.Host, db.Port, db.ImageName); err != nil { | |
| 142 | + | return err | |
| 143 | + | } | |
| 144 | + | } | |
| 145 | + | if err := w.Flush(); err != nil { | |
| 146 | + | return err | |
| 117 | 147 | } | |
| 118 | 148 | ||
| 119 | 149 | return nil | |
| @@ -140,10 +170,18 @@ func newBackupStatusCmd() *cobra.Command { | |||
| 140 | 170 | fmt.Println("No backup status available") | |
| 141 | 171 | return nil | |
| 142 | 172 | } | |
| 143 | - | fmt.Println("DOMAIN\tDB\tSTATUS\tSTARTED_AT") | |
| 173 | + | w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) | |
| 174 | + | if _, err := fmt.Fprintln(w, "DOMAIN\tDB\tSTATUS\tSTARTED_AT"); err != nil { | |
| 175 | + | return err | |
| 176 | + | } | |
| 144 | 177 | ||
| 145 | 178 | for _, job := range jobs { | |
| 146 | - | fmt.Printf("%s\t%s\t%s\t%s\n", job.Domain, job.DBName, job.Status, formatBackupTime(job.StartedAt)) | |
| 179 | + | if _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\n", job.Domain, job.DBName, job.Status, formatBackupTime(job.StartedAt)); err != nil { | |
| 180 | + | return err | |
| 181 | + | } | |
| 182 | + | } | |
| 183 | + | if err := w.Flush(); err != nil { | |
| 184 | + | return err | |
| 147 | 185 | } | |
| 148 | 186 | ||
| 149 | 187 | return nil | |
| @@ -3,6 +3,8 @@ package cli | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "context" | 4 | "context" | ||
| 5 | "fmt" | 5 | "fmt" | ||
| 6 | + | "os" | |||
| 7 | + | "text/tabwriter" | |||
| 6 | "time" | 8 | "time" | ||
| 7 | 9 | ||||
| 8 | "github.com/spf13/cobra" | 10 | "github.com/spf13/cobra" | ||
| @@ -51,10 +53,18 @@ func newBackupListCmd() *cobra.Command { | |||||
| 51 | fmt.Println("No backups found") | 53 | fmt.Println("No backups found") | ||
| 52 | return nil | 54 | return nil | ||
| 53 | } | 55 | } | ||
| 54 | - | fmt.Println("DOMAIN\tDB\tSTATUS\tSTARTED_AT\tFILE_PATH") | 56 | + | w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) |
| 57 | + | if _, err := fmt.Fprintln(w, "DOMAIN\tDB\tSTATUS\tSTARTED_AT\tFILE_PATH"); err != nil { | |||
| 58 | + | return err | |||
| 59 | + | } | |||
| 55 | 60 | ||||
| 56 | for _, job := range jobs { | 61 | for _, job := range jobs { | ||
| 57 | - | fmt.Printf("%s\t%s\t%s\t%s\t%s\n", job.Domain, job.DBName, job.Status, formatBackupTime(job.StartedAt), job.FilePath) | 62 | + | 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 { |
| 63 | + | return err | |||
| 64 | + | } | |||
| 65 | + | } | |||
| 66 | + | if err := w.Flush(); err != nil { | |||
| 67 | + | return err | |||
| 58 | } | 68 | } | ||
| 59 | 69 | ||||
| 60 | return nil | 70 | return nil | ||
| @@ -80,8 +90,20 @@ func newBackupRunCmd() *cobra.Command { | |||||
| 80 | return fmt.Errorf("failed to run backup: %w", err) | 90 | return fmt.Errorf("failed to run backup: %w", err) | ||
| 81 | } | 91 | } | ||
| 82 | 92 | ||||
| 83 | - | fmt.Println("DOMAIN\tDB\tSTATUS\tSTARTED_AT\tFILE_PATH\tSIZE_BYTES") | 93 | + | if result.Backup == nil { |
| 84 | - | fmt.Printf("%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) | 94 | + | return fmt.Errorf("backup run completed without backup payload") |
| 95 | + | } | |||
| 96 | + | ||||
| 97 | + | w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) | |||
| 98 | + | if _, err := fmt.Fprintln(w, "DOMAIN\tDB\tSTATUS\tSTARTED_AT\tFILE_PATH\tSIZE_BYTES"); err != nil { | |||
| 99 | + | return err | |||
| 100 | + | } | |||
| 101 | + | 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 { | |||
| 102 | + | return err | |||
| 103 | + | } | |||
| 104 | + | if err := w.Flush(); err != nil { | |||
| 105 | + | return err | |||
| 106 | + | } | |||
| 85 | return nil | 107 | return nil | ||
| 86 | }, | 108 | }, | ||
| 87 | } | 109 | } | ||
| @@ -110,10 +132,18 @@ func newBackupDetectCmd() *cobra.Command { | |||||
| 110 | fmt.Println("No supported databases detected") | 132 | fmt.Println("No supported databases detected") | ||
| 111 | return nil | 133 | return nil | ||
| 112 | } | 134 | } | ||
| 113 | - | fmt.Println("NAME\tTYPE\tHOST\tPORT\tIMAGE") | 135 | + | w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) |
| 136 | + | if _, err := fmt.Fprintln(w, "NAME\tTYPE\tHOST\tPORT\tIMAGE"); err != nil { | |||
| 137 | + | return err | |||
| 138 | + | } | |||
| 114 | 139 | ||||
| 115 | for _, db := range dbs { | 140 | for _, db := range dbs { | ||
| 116 | - | fmt.Printf("%s\t%s\t%s\t%d\t%s\n", db.Name, db.Type, db.Host, db.Port, db.ImageName) | 141 | + | if _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%d\t%s\n", db.Name, db.Type, db.Host, db.Port, db.ImageName); err != nil { |
| 142 | + | return err | |||
| 143 | + | } | |||
| 144 | + | } | |||
| 145 | + | if err := w.Flush(); err != nil { | |||
| 146 | + | return err | |||
| 117 | } | 147 | } | ||
| 118 | 148 | ||||
| 119 | return nil | 149 | return nil | ||
| @@ -140,10 +170,18 @@ func newBackupStatusCmd() *cobra.Command { | |||||
| 140 | fmt.Println("No backup status available") | 170 | fmt.Println("No backup status available") | ||
| 141 | return nil | 171 | return nil | ||
| 142 | } | 172 | } | ||
| 143 | - | fmt.Println("DOMAIN\tDB\tSTATUS\tSTARTED_AT") | 173 | + | w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0) |
| 174 | + | if _, err := fmt.Fprintln(w, "DOMAIN\tDB\tSTATUS\tSTARTED_AT"); err != nil { | |||
| 175 | + | return err | |||
| 176 | + | } | |||
| 144 | 177 | ||||
| 145 | for _, job := range jobs { | 178 | for _, job := range jobs { | ||
| 146 | - | fmt.Printf("%s\t%s\t%s\t%s\n", job.Domain, job.DBName, job.Status, formatBackupTime(job.StartedAt)) | 179 | + | if _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\n", job.Domain, job.DBName, job.Status, formatBackupTime(job.StartedAt)); err != nil { |
| 180 | + | return err | |||
| 181 | + | } | |||
| 182 | + | } | |||
| 183 | + | if err := w.Flush(); err != nil { | |||
| 184 | + | return err | |||
| 147 | } | 185 | } | ||
| 148 | 186 | ||||
| 149 | return nil | 187 | return nil | ||
M internal/adapters/in/http/admin/handler.go +5 -6
| @@ -851,11 +851,9 @@ func (h *Handler) handleBackupsDomainRun(w http.ResponseWriter, r *http.Request, | |||
| 851 | 851 | ||
| 852 | 852 | var req dto.BackupRunRequest | |
| 853 | 853 | r.Body = http.MaxBytesReader(w, r.Body, maxAdminRequestSize) | |
| 854 | - | if r.ContentLength > 0 { | |
| 855 | - | if err := json.NewDecoder(r.Body).Decode(&req); err != nil && !errors.Is(err, io.EOF) { | |
| 856 | - | h.sendError(w, http.StatusBadRequest, "invalid JSON") | |
| 857 | - | return | |
| 858 | - | } | |
| 854 | + | if err := json.NewDecoder(r.Body).Decode(&req); err != nil && !errors.Is(err, io.EOF) { | |
| 855 | + | h.sendError(w, http.StatusBadRequest, "invalid JSON") | |
| 856 | + | return | |
| 859 | 857 | } | |
| 860 | 858 | ||
| 861 | 859 | result, err := h.backupSvc.RunBackup(ctx, backupDomain, req.DB) | |
| @@ -868,9 +866,10 @@ func (h *Handler) handleBackupsDomainRun(w http.ResponseWriter, r *http.Request, | |||
| 868 | 866 | log := zerowrap.FromCtx(ctx) | |
| 869 | 867 | log.Info().Str("domain", backupDomain).Str("db", req.DB).Str("job_id", result.Job.ID).Msg("backup completed via admin API") | |
| 870 | 868 | ||
| 869 | + | job := toBackupJobResponse(result.Job) | |
| 871 | 870 | h.sendJSON(w, http.StatusOK, dto.BackupRunResponse{ | |
| 872 | 871 | Status: "completed", | |
| 873 | - | Backup: toBackupJobResponse(result.Job), | |
| 872 | + | Backup: &job, | |
| 874 | 873 | }) | |
| 875 | 874 | } | |
| 876 | 875 | ||
| @@ -851,11 +851,9 @@ func (h *Handler) handleBackupsDomainRun(w http.ResponseWriter, r *http.Request, | |||||
| 851 | 851 | ||||
| 852 | var req dto.BackupRunRequest | 852 | var req dto.BackupRunRequest | ||
| 853 | r.Body = http.MaxBytesReader(w, r.Body, maxAdminRequestSize) | 853 | r.Body = http.MaxBytesReader(w, r.Body, maxAdminRequestSize) | ||
| 854 | - | if r.ContentLength > 0 { | 854 | + | if err := json.NewDecoder(r.Body).Decode(&req); err != nil && !errors.Is(err, io.EOF) { |
| 855 | - | if err := json.NewDecoder(r.Body).Decode(&req); err != nil && !errors.Is(err, io.EOF) { | 855 | + | h.sendError(w, http.StatusBadRequest, "invalid JSON") |
| 856 | - | h.sendError(w, http.StatusBadRequest, "invalid JSON") | 856 | + | return |
| 857 | - | return | |||
| 858 | - | } | |||
| 859 | } | 857 | } | ||
| 860 | 858 | ||||
| 861 | result, err := h.backupSvc.RunBackup(ctx, backupDomain, req.DB) | 859 | result, err := h.backupSvc.RunBackup(ctx, backupDomain, req.DB) | ||
| @@ -868,9 +866,10 @@ func (h *Handler) handleBackupsDomainRun(w http.ResponseWriter, r *http.Request, | |||||
| 868 | log := zerowrap.FromCtx(ctx) | 866 | log := zerowrap.FromCtx(ctx) | ||
| 869 | log.Info().Str("domain", backupDomain).Str("db", req.DB).Str("job_id", result.Job.ID).Msg("backup completed via admin API") | 867 | log.Info().Str("domain", backupDomain).Str("db", req.DB).Str("job_id", result.Job.ID).Msg("backup completed via admin API") | ||
| 870 | 868 | ||||
| 869 | + | job := toBackupJobResponse(result.Job) | |||
| 871 | h.sendJSON(w, http.StatusOK, dto.BackupRunResponse{ | 870 | h.sendJSON(w, http.StatusOK, dto.BackupRunResponse{ | ||
| 872 | Status: "completed", | 871 | Status: "completed", | ||
| 873 | - | Backup: toBackupJobResponse(result.Job), | 872 | + | Backup: &job, |
| 874 | }) | 873 | }) | ||
| 875 | } | 874 | } | ||
| 876 | 875 | ||||
M internal/adapters/in/http/admin/handler_test.go +23 -0
| @@ -1246,6 +1246,29 @@ func TestHandler_BackupsRunDomain(t *testing.T) { | |||
| 1246 | 1246 | assert.Contains(t, rec.Body.String(), "completed") | |
| 1247 | 1247 | } | |
| 1248 | 1248 | ||
| 1249 | + | func TestHandler_BackupsRunDomain_ChunkedBodyIsDecoded(t *testing.T) { | |
| 1250 | + | configSvc := inmocks.NewMockConfigService(t) | |
| 1251 | + | authSvc := inmocks.NewMockAuthService(t) | |
| 1252 | + | containerSvc := inmocks.NewMockContainerService(t) | |
| 1253 | + | secretSvc := inmocks.NewMockSecretService(t) | |
| 1254 | + | backupSvc := inmocks.NewMockBackupService(t) | |
| 1255 | + | ||
| 1256 | + | backupSvc.EXPECT().RunBackup(mock.Anything, "app.example.com", "postgres").Return(&domain.BackupResult{Job: domain.BackupJob{Domain: "app.example.com", DBName: "postgres", Status: domain.BackupStatusCompleted}}, nil) | |
| 1257 | + | ||
| 1258 | + | handler := NewHandler(configSvc, authSvc, containerSvc, inmocks.NewMockHealthService(t), secretSvc, nil, inmocks.NewMockRegistryService(t), nil, testLogger(), backupSvc) | |
| 1259 | + | ||
| 1260 | + | body := bytes.NewBufferString(`{"db":"postgres"}`) | |
| 1261 | + | req := httptest.NewRequest("POST", "/admin/backups/app.example.com", body) | |
| 1262 | + | req.ContentLength = -1 | |
| 1263 | + | req = req.WithContext(ctxWithScopes("admin:config:write")) | |
| 1264 | + | rec := httptest.NewRecorder() | |
| 1265 | + | ||
| 1266 | + | handler.ServeHTTP(rec, req) | |
| 1267 | + | ||
| 1268 | + | assert.Equal(t, http.StatusOK, rec.Code) | |
| 1269 | + | assert.Contains(t, rec.Body.String(), "completed") | |
| 1270 | + | } | |
| 1271 | + | ||
| 1249 | 1272 | func TestHandler_BackupsDetectDomain(t *testing.T) { | |
| 1250 | 1273 | configSvc := inmocks.NewMockConfigService(t) | |
| 1251 | 1274 | authSvc := inmocks.NewMockAuthService(t) | |
| @@ -1246,6 +1246,29 @@ func TestHandler_BackupsRunDomain(t *testing.T) { | |||||
| 1246 | assert.Contains(t, rec.Body.String(), "completed") | 1246 | assert.Contains(t, rec.Body.String(), "completed") | ||
| 1247 | } | 1247 | } | ||
| 1248 | 1248 | ||||
| 1249 | + | func TestHandler_BackupsRunDomain_ChunkedBodyIsDecoded(t *testing.T) { | |||
| 1250 | + | configSvc := inmocks.NewMockConfigService(t) | |||
| 1251 | + | authSvc := inmocks.NewMockAuthService(t) | |||
| 1252 | + | containerSvc := inmocks.NewMockContainerService(t) | |||
| 1253 | + | secretSvc := inmocks.NewMockSecretService(t) | |||
| 1254 | + | backupSvc := inmocks.NewMockBackupService(t) | |||
| 1255 | + | ||||
| 1256 | + | backupSvc.EXPECT().RunBackup(mock.Anything, "app.example.com", "postgres").Return(&domain.BackupResult{Job: domain.BackupJob{Domain: "app.example.com", DBName: "postgres", Status: domain.BackupStatusCompleted}}, nil) | |||
| 1257 | + | ||||
| 1258 | + | handler := NewHandler(configSvc, authSvc, containerSvc, inmocks.NewMockHealthService(t), secretSvc, nil, inmocks.NewMockRegistryService(t), nil, testLogger(), backupSvc) | |||
| 1259 | + | ||||
| 1260 | + | body := bytes.NewBufferString(`{"db":"postgres"}`) | |||
| 1261 | + | req := httptest.NewRequest("POST", "/admin/backups/app.example.com", body) | |||
| 1262 | + | req.ContentLength = -1 | |||
| 1263 | + | req = req.WithContext(ctxWithScopes("admin:config:write")) | |||
| 1264 | + | rec := httptest.NewRecorder() | |||
| 1265 | + | ||||
| 1266 | + | handler.ServeHTTP(rec, req) | |||
| 1267 | + | ||||
| 1268 | + | assert.Equal(t, http.StatusOK, rec.Code) | |||
| 1269 | + | assert.Contains(t, rec.Body.String(), "completed") | |||
| 1270 | + | } | |||
| 1271 | + | ||||
| 1249 | func TestHandler_BackupsDetectDomain(t *testing.T) { | 1272 | func TestHandler_BackupsDetectDomain(t *testing.T) { | ||
| 1250 | configSvc := inmocks.NewMockConfigService(t) | 1273 | configSvc := inmocks.NewMockConfigService(t) | ||
| 1251 | authSvc := inmocks.NewMockAuthService(t) | 1274 | authSvc := inmocks.NewMockAuthService(t) | ||
M internal/adapters/out/docker/runtime.go +6 -5
| @@ -1113,19 +1113,20 @@ func (r *Runtime) DisconnectContainerFromNetwork(ctx context.Context, containerN | |||
| 1113 | 1113 | ||
| 1114 | 1114 | // ExecInContainer executes a command in a running container. | |
| 1115 | 1115 | func (r *Runtime) ExecInContainer(ctx context.Context, containerID string, cmd []string) (*out.ExecResult, error) { | |
| 1116 | + | if len(cmd) == 0 { | |
| 1117 | + | return nil, fmt.Errorf("command cannot be empty") | |
| 1118 | + | } | |
| 1119 | + | ||
| 1116 | 1120 | ctx = zerowrap.CtxWithFields(ctx, map[string]any{ | |
| 1117 | 1121 | zerowrap.FieldLayer: "adapter", | |
| 1118 | 1122 | zerowrap.FieldAdapter: "docker", | |
| 1119 | 1123 | zerowrap.FieldAction: "ExecInContainer", | |
| 1120 | 1124 | zerowrap.FieldEntityID: containerID, | |
| 1121 | - | "command": strings.Join(cmd, " "), | |
| 1125 | + | "command": cmd[0], | |
| 1126 | + | "arg_count": len(cmd) - 1, | |
| 1122 | 1127 | }) | |
| 1123 | 1128 | log := zerowrap.FromCtx(ctx) | |
| 1124 | 1129 | ||
| 1125 | - | if len(cmd) == 0 { | |
| 1126 | - | return nil, fmt.Errorf("command cannot be empty") | |
| 1127 | - | } | |
| 1128 | - | ||
| 1129 | 1130 | execResp, err := r.client.ContainerExecCreate(ctx, containerID, container.ExecOptions{ | |
| 1130 | 1131 | Cmd: cmd, | |
| 1131 | 1132 | AttachStdout: true, | |
| @@ -1113,19 +1113,20 @@ func (r *Runtime) DisconnectContainerFromNetwork(ctx context.Context, containerN | |||||
| 1113 | 1113 | ||||
| 1114 | // ExecInContainer executes a command in a running container. | 1114 | // ExecInContainer executes a command in a running container. | ||
| 1115 | func (r *Runtime) ExecInContainer(ctx context.Context, containerID string, cmd []string) (*out.ExecResult, error) { | 1115 | func (r *Runtime) ExecInContainer(ctx context.Context, containerID string, cmd []string) (*out.ExecResult, error) { | ||
| 1116 | + | if len(cmd) == 0 { | |||
| 1117 | + | return nil, fmt.Errorf("command cannot be empty") | |||
| 1118 | + | } | |||
| 1119 | + | ||||
| 1116 | ctx = zerowrap.CtxWithFields(ctx, map[string]any{ | 1120 | ctx = zerowrap.CtxWithFields(ctx, map[string]any{ | ||
| 1117 | zerowrap.FieldLayer: "adapter", | 1121 | zerowrap.FieldLayer: "adapter", | ||
| 1118 | zerowrap.FieldAdapter: "docker", | 1122 | zerowrap.FieldAdapter: "docker", | ||
| 1119 | zerowrap.FieldAction: "ExecInContainer", | 1123 | zerowrap.FieldAction: "ExecInContainer", | ||
| 1120 | zerowrap.FieldEntityID: containerID, | 1124 | zerowrap.FieldEntityID: containerID, | ||
| 1121 | - | "command": strings.Join(cmd, " "), | 1125 | + | "command": cmd[0], |
| 1126 | + | "arg_count": len(cmd) - 1, | |||
| 1122 | }) | 1127 | }) | ||
| 1123 | log := zerowrap.FromCtx(ctx) | 1128 | log := zerowrap.FromCtx(ctx) | ||
| 1124 | 1129 | ||||
| 1125 | - | if len(cmd) == 0 { | |||
| 1126 | - | return nil, fmt.Errorf("command cannot be empty") | |||
| 1127 | - | } | |||
| 1128 | - | ||||
| 1129 | execResp, err := r.client.ContainerExecCreate(ctx, containerID, container.ExecOptions{ | 1130 | execResp, err := r.client.ContainerExecCreate(ctx, containerID, container.ExecOptions{ | ||
| 1130 | Cmd: cmd, | 1131 | Cmd: cmd, | ||
| 1131 | AttachStdout: true, | 1132 | AttachStdout: true, | ||
M internal/adapters/out/filesystem/backup_storage.go +1 -0
| @@ -42,6 +42,7 @@ func (s *BackupStorage) Store(_ context.Context, domainName, dbName string, sche | |||
| 42 | 42 | if schedulePart == "" { | |
| 43 | 43 | schedulePart = "manual" | |
| 44 | 44 | } | |
| 45 | + | schedulePart = sanitizeBackupPathComponent(schedulePart) | |
| 45 | 46 | ||
| 46 | 47 | backupDir := filepath.Join(s.rootDir, domainPart, dbPart, schedulePart) | |
| 47 | 48 | if err := os.MkdirAll(backupDir, 0750); err != nil { | |
| @@ -42,6 +42,7 @@ func (s *BackupStorage) Store(_ context.Context, domainName, dbName string, sche | |||||
| 42 | if schedulePart == "" { | 42 | if schedulePart == "" { | ||
| 43 | schedulePart = "manual" | 43 | schedulePart = "manual" | ||
| 44 | } | 44 | } | ||
| 45 | + | schedulePart = sanitizeBackupPathComponent(schedulePart) | |||
| 45 | 46 | ||||
| 46 | backupDir := filepath.Join(s.rootDir, domainPart, dbPart, schedulePart) | 47 | backupDir := filepath.Join(s.rootDir, domainPart, dbPart, schedulePart) | ||
| 47 | if err := os.MkdirAll(backupDir, 0750); err != nil { | 48 | if err := os.MkdirAll(backupDir, 0750); err != nil { | ||
M internal/adapters/out/filesystem/backup_storage_test.go +16 -0
| @@ -4,6 +4,7 @@ import ( | |||
| 4 | 4 | "bytes" | |
| 5 | 5 | "context" | |
| 6 | 6 | "io" | |
| 7 | + | "path/filepath" | |
| 7 | 8 | "testing" | |
| 8 | 9 | "time" | |
| 9 | 10 | ||
| @@ -93,3 +94,18 @@ func TestBackupStorage_StoreSanitizesDotOnlyPathComponents(t *testing.T) { | |||
| 93 | 94 | require.NoError(t, err) | |
| 94 | 95 | assert.NotContains(t, path, "..") | |
| 95 | 96 | } | |
| 97 | + | ||
| 98 | + | func TestBackupStorage_StoreSanitizesSchedulePathComponent(t *testing.T) { | |
| 99 | + | rootDir := t.TempDir() | |
| 100 | + | storage, err := NewBackupStorage(rootDir, testLogger()) | |
| 101 | + | require.NoError(t, err) | |
| 102 | + | ||
| 103 | + | path, err := storage.Store(context.Background(), "app.example.com", "postgres", domain.BackupSchedule("../../escape"), time.Now().UTC(), bytes.NewReader([]byte("data"))) | |
| 104 | + | require.NoError(t, err) | |
| 105 | + | ||
| 106 | + | rel, err := filepath.Rel(rootDir, path) | |
| 107 | + | require.NoError(t, err) | |
| 108 | + | assert.NotEqual(t, "..", rel) | |
| 109 | + | assert.NotContains(t, rel, "../") | |
| 110 | + | assert.NotContains(t, rel, "..\\") | |
| 111 | + | } | |
| @@ -4,6 +4,7 @@ import ( | |||||
| 4 | "bytes" | 4 | "bytes" | ||
| 5 | "context" | 5 | "context" | ||
| 6 | "io" | 6 | "io" | ||
| 7 | + | "path/filepath" | |||
| 7 | "testing" | 8 | "testing" | ||
| 8 | "time" | 9 | "time" | ||
| 9 | 10 | ||||
| @@ -93,3 +94,18 @@ func TestBackupStorage_StoreSanitizesDotOnlyPathComponents(t *testing.T) { | |||||
| 93 | require.NoError(t, err) | 94 | require.NoError(t, err) | ||
| 94 | assert.NotContains(t, path, "..") | 95 | assert.NotContains(t, path, "..") | ||
| 95 | } | 96 | } | ||
| 97 | + | ||||
| 98 | + | func TestBackupStorage_StoreSanitizesSchedulePathComponent(t *testing.T) { | |||
| 99 | + | rootDir := t.TempDir() | |||
| 100 | + | storage, err := NewBackupStorage(rootDir, testLogger()) | |||
| 101 | + | require.NoError(t, err) | |||
| 102 | + | ||||
| 103 | + | path, err := storage.Store(context.Background(), "app.example.com", "postgres", domain.BackupSchedule("../../escape"), time.Now().UTC(), bytes.NewReader([]byte("data"))) | |||
| 104 | + | require.NoError(t, err) | |||
| 105 | + | ||||
| 106 | + | rel, err := filepath.Rel(rootDir, path) | |||
| 107 | + | require.NoError(t, err) | |||
| 108 | + | assert.NotEqual(t, "..", rel) | |||
| 109 | + | assert.NotContains(t, rel, "../") | |||
| 110 | + | assert.NotContains(t, rel, "..\\") | |||
| 111 | + | } | |||
M internal/app/run.go +28 -6
| @@ -1095,15 +1095,15 @@ func createBackupService(cfg Config, svc *services, log zerowrap.Logger) (*files | |||
| 1095 | 1095 | return nil, nil, log.WrapErr(err, "failed to create backup storage") | |
| 1096 | 1096 | } | |
| 1097 | 1097 | ||
| 1098 | + | retention, err := validateBackupRetention(cfg) | |
| 1099 | + | if err != nil { | |
| 1100 | + | return nil, nil, log.WrapErr(err, "invalid backup retention policy") | |
| 1101 | + | } | |
| 1102 | + | ||
| 1098 | 1103 | backupCfg := domain.BackupConfig{ | |
| 1099 | 1104 | Enabled: cfg.Backups.Enabled, | |
| 1100 | 1105 | StorageDir: storageDir, | |
| 1101 | - | Retention: domain.RetentionPolicy{ | |
| 1102 | - | Hourly: cfg.Backups.Retention.Hourly, | |
| 1103 | - | Daily: cfg.Backups.Retention.Daily, | |
| 1104 | - | Weekly: cfg.Backups.Retention.Weekly, | |
| 1105 | - | Monthly: cfg.Backups.Retention.Monthly, | |
| 1106 | - | }, | |
| 1106 | + | Retention: retention, | |
| 1107 | 1107 | } | |
| 1108 | 1108 | ||
| 1109 | 1109 | backupSvc := backup.NewService(svc.runtime, backupStorage, svc.containerSvc, backupCfg, log) | |
| @@ -1115,6 +1115,28 @@ func createBackupService(cfg Config, svc *services, log zerowrap.Logger) (*files | |||
| 1115 | 1115 | return backupStorage, backupSvc, nil | |
| 1116 | 1116 | } | |
| 1117 | 1117 | ||
| 1118 | + | func validateBackupRetention(cfg Config) (domain.RetentionPolicy, error) { | |
| 1119 | + | if cfg.Backups.Retention.Hourly < 0 { | |
| 1120 | + | return domain.RetentionPolicy{}, fmt.Errorf("backups.retention.hourly cannot be negative") | |
| 1121 | + | } | |
| 1122 | + | if cfg.Backups.Retention.Daily < 0 { | |
| 1123 | + | return domain.RetentionPolicy{}, fmt.Errorf("backups.retention.daily cannot be negative") | |
| 1124 | + | } | |
| 1125 | + | if cfg.Backups.Retention.Weekly < 0 { | |
| 1126 | + | return domain.RetentionPolicy{}, fmt.Errorf("backups.retention.weekly cannot be negative") | |
| 1127 | + | } | |
| 1128 | + | if cfg.Backups.Retention.Monthly < 0 { | |
| 1129 | + | return domain.RetentionPolicy{}, fmt.Errorf("backups.retention.monthly cannot be negative") | |
| 1130 | + | } | |
| 1131 | + | ||
| 1132 | + | return domain.RetentionPolicy{ | |
| 1133 | + | Hourly: cfg.Backups.Retention.Hourly, | |
| 1134 | + | Daily: cfg.Backups.Retention.Daily, | |
| 1135 | + | Weekly: cfg.Backups.Retention.Weekly, | |
| 1136 | + | Monthly: cfg.Backups.Retention.Monthly, | |
| 1137 | + | }, nil | |
| 1138 | + | } | |
| 1139 | + | ||
| 1118 | 1140 | // registerEventHandlers registers all event handlers. | |
| 1119 | 1141 | func registerEventHandlers(ctx context.Context, svc *services, cfg Config) error { | |
| 1120 | 1142 | imagePushedHandler := container.NewImagePushedHandler(ctx, svc.containerSvc, svc.configSvc) | |
| @@ -1095,15 +1095,15 @@ func createBackupService(cfg Config, svc *services, log zerowrap.Logger) (*files | |||||
| 1095 | return nil, nil, log.WrapErr(err, "failed to create backup storage") | 1095 | return nil, nil, log.WrapErr(err, "failed to create backup storage") | ||
| 1096 | } | 1096 | } | ||
| 1097 | 1097 | ||||
| 1098 | + | retention, err := validateBackupRetention(cfg) | |||
| 1099 | + | if err != nil { | |||
| 1100 | + | return nil, nil, log.WrapErr(err, "invalid backup retention policy") | |||
| 1101 | + | } | |||
| 1102 | + | ||||
| 1098 | backupCfg := domain.BackupConfig{ | 1103 | backupCfg := domain.BackupConfig{ | ||
| 1099 | Enabled: cfg.Backups.Enabled, | 1104 | Enabled: cfg.Backups.Enabled, | ||
| 1100 | StorageDir: storageDir, | 1105 | StorageDir: storageDir, | ||
| 1101 | - | Retention: domain.RetentionPolicy{ | 1106 | + | Retention: retention, |
| 1102 | - | Hourly: cfg.Backups.Retention.Hourly, | |||
| 1103 | - | Daily: cfg.Backups.Retention.Daily, | |||
| 1104 | - | Weekly: cfg.Backups.Retention.Weekly, | |||
| 1105 | - | Monthly: cfg.Backups.Retention.Monthly, | |||
| 1106 | - | }, | |||
| 1107 | } | 1107 | } | ||
| 1108 | 1108 | ||||
| 1109 | backupSvc := backup.NewService(svc.runtime, backupStorage, svc.containerSvc, backupCfg, log) | 1109 | backupSvc := backup.NewService(svc.runtime, backupStorage, svc.containerSvc, backupCfg, log) | ||
| @@ -1115,6 +1115,28 @@ func createBackupService(cfg Config, svc *services, log zerowrap.Logger) (*files | |||||
| 1115 | return backupStorage, backupSvc, nil | 1115 | return backupStorage, backupSvc, nil | ||
| 1116 | } | 1116 | } | ||
| 1117 | 1117 | ||||
| 1118 | + | func validateBackupRetention(cfg Config) (domain.RetentionPolicy, error) { | |||
| 1119 | + | if cfg.Backups.Retention.Hourly < 0 { | |||
| 1120 | + | return domain.RetentionPolicy{}, fmt.Errorf("backups.retention.hourly cannot be negative") | |||
| 1121 | + | } | |||
| 1122 | + | if cfg.Backups.Retention.Daily < 0 { | |||
| 1123 | + | return domain.RetentionPolicy{}, fmt.Errorf("backups.retention.daily cannot be negative") | |||
| 1124 | + | } | |||
| 1125 | + | if cfg.Backups.Retention.Weekly < 0 { | |||
| 1126 | + | return domain.RetentionPolicy{}, fmt.Errorf("backups.retention.weekly cannot be negative") | |||
| 1127 | + | } | |||
| 1128 | + | if cfg.Backups.Retention.Monthly < 0 { | |||
| 1129 | + | return domain.RetentionPolicy{}, fmt.Errorf("backups.retention.monthly cannot be negative") | |||
| 1130 | + | } | |||
| 1131 | + | ||||
| 1132 | + | return domain.RetentionPolicy{ | |||
| 1133 | + | Hourly: cfg.Backups.Retention.Hourly, | |||
| 1134 | + | Daily: cfg.Backups.Retention.Daily, | |||
| 1135 | + | Weekly: cfg.Backups.Retention.Weekly, | |||
| 1136 | + | Monthly: cfg.Backups.Retention.Monthly, | |||
| 1137 | + | }, nil | |||
| 1138 | + | } | |||
| 1139 | + | ||||
| 1118 | // registerEventHandlers registers all event handlers. | 1140 | // registerEventHandlers registers all event handlers. | ||
| 1119 | func registerEventHandlers(ctx context.Context, svc *services, cfg Config) error { | 1141 | func registerEventHandlers(ctx context.Context, svc *services, cfg Config) error { | ||
| 1120 | imagePushedHandler := container.NewImagePushedHandler(ctx, svc.containerSvc, svc.configSvc) | 1142 | imagePushedHandler := container.NewImagePushedHandler(ctx, svc.containerSvc, svc.configSvc) | ||
A internal/app/run_backup_test.go +71 -0
| @@ -0,0 +1,71 @@ | |||
| 1 | + | package app | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "testing" | |
| 5 | + | ||
| 6 | + | "github.com/stretchr/testify/assert" | |
| 7 | + | "github.com/stretchr/testify/require" | |
| 8 | + | ) | |
| 9 | + | ||
| 10 | + | func TestValidateBackupRetentionRejectsNegativeValues(t *testing.T) { | |
| 11 | + | tests := []struct { | |
| 12 | + | name string | |
| 13 | + | set func(*Config) | |
| 14 | + | err string | |
| 15 | + | }{ | |
| 16 | + | { | |
| 17 | + | name: "hourly", | |
| 18 | + | set: func(cfg *Config) { | |
| 19 | + | cfg.Backups.Retention.Hourly = -1 | |
| 20 | + | }, | |
| 21 | + | err: "backups.retention.hourly", | |
| 22 | + | }, | |
| 23 | + | { | |
| 24 | + | name: "daily", | |
| 25 | + | set: func(cfg *Config) { | |
| 26 | + | cfg.Backups.Retention.Daily = -1 | |
| 27 | + | }, | |
| 28 | + | err: "backups.retention.daily", | |
| 29 | + | }, | |
| 30 | + | { | |
| 31 | + | name: "weekly", | |
| 32 | + | set: func(cfg *Config) { | |
| 33 | + | cfg.Backups.Retention.Weekly = -1 | |
| 34 | + | }, | |
| 35 | + | err: "backups.retention.weekly", | |
| 36 | + | }, | |
| 37 | + | { | |
| 38 | + | name: "monthly", | |
| 39 | + | set: func(cfg *Config) { | |
| 40 | + | cfg.Backups.Retention.Monthly = -1 | |
| 41 | + | }, | |
| 42 | + | err: "backups.retention.monthly", | |
| 43 | + | }, | |
| 44 | + | } | |
| 45 | + | ||
| 46 | + | for _, tt := range tests { | |
| 47 | + | t.Run(tt.name, func(t *testing.T) { | |
| 48 | + | var cfg Config | |
| 49 | + | tt.set(&cfg) | |
| 50 | + | ||
| 51 | + | _, err := validateBackupRetention(cfg) | |
| 52 | + | require.Error(t, err) | |
| 53 | + | assert.Contains(t, err.Error(), tt.err) | |
| 54 | + | }) | |
| 55 | + | } | |
| 56 | + | } | |
| 57 | + | ||
| 58 | + | func TestValidateBackupRetentionAcceptsZeroAndPositiveValues(t *testing.T) { | |
| 59 | + | var cfg Config | |
| 60 | + | cfg.Backups.Retention.Hourly = 1 | |
| 61 | + | cfg.Backups.Retention.Daily = 7 | |
| 62 | + | cfg.Backups.Retention.Weekly = 4 | |
| 63 | + | cfg.Backups.Retention.Monthly = 12 | |
| 64 | + | ||
| 65 | + | retention, err := validateBackupRetention(cfg) | |
| 66 | + | require.NoError(t, err) | |
| 67 | + | assert.Equal(t, 1, retention.Hourly) | |
| 68 | + | assert.Equal(t, 7, retention.Daily) | |
| 69 | + | assert.Equal(t, 4, retention.Weekly) | |
| 70 | + | assert.Equal(t, 12, retention.Monthly) | |
| 71 | + | } | |
| @@ -0,0 +1,71 @@ | |||||
| 1 | + | package app | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "testing" | |||
| 5 | + | ||||
| 6 | + | "github.com/stretchr/testify/assert" | |||
| 7 | + | "github.com/stretchr/testify/require" | |||
| 8 | + | ) | |||
| 9 | + | ||||
| 10 | + | func TestValidateBackupRetentionRejectsNegativeValues(t *testing.T) { | |||
| 11 | + | tests := []struct { | |||
| 12 | + | name string | |||
| 13 | + | set func(*Config) | |||
| 14 | + | err string | |||
| 15 | + | }{ | |||
| 16 | + | { | |||
| 17 | + | name: "hourly", | |||
| 18 | + | set: func(cfg *Config) { | |||
| 19 | + | cfg.Backups.Retention.Hourly = -1 | |||
| 20 | + | }, | |||
| 21 | + | err: "backups.retention.hourly", | |||
| 22 | + | }, | |||
| 23 | + | { | |||
| 24 | + | name: "daily", | |||
| 25 | + | set: func(cfg *Config) { | |||
| 26 | + | cfg.Backups.Retention.Daily = -1 | |||
| 27 | + | }, | |||
| 28 | + | err: "backups.retention.daily", | |||
| 29 | + | }, | |||
| 30 | + | { | |||
| 31 | + | name: "weekly", | |||
| 32 | + | set: func(cfg *Config) { | |||
| 33 | + | cfg.Backups.Retention.Weekly = -1 | |||
| 34 | + | }, | |||
| 35 | + | err: "backups.retention.weekly", | |||
| 36 | + | }, | |||
| 37 | + | { | |||
| 38 | + | name: "monthly", | |||
| 39 | + | set: func(cfg *Config) { | |||
| 40 | + | cfg.Backups.Retention.Monthly = -1 | |||
| 41 | + | }, | |||
| 42 | + | err: "backups.retention.monthly", | |||
| 43 | + | }, | |||
| 44 | + | } | |||
| 45 | + | ||||
| 46 | + | for _, tt := range tests { | |||
| 47 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 48 | + | var cfg Config | |||
| 49 | + | tt.set(&cfg) | |||
| 50 | + | ||||
| 51 | + | _, err := validateBackupRetention(cfg) | |||
| 52 | + | require.Error(t, err) | |||
| 53 | + | assert.Contains(t, err.Error(), tt.err) | |||
| 54 | + | }) | |||
| 55 | + | } | |||
| 56 | + | } | |||
| 57 | + | ||||
| 58 | + | func TestValidateBackupRetentionAcceptsZeroAndPositiveValues(t *testing.T) { | |||
| 59 | + | var cfg Config | |||
| 60 | + | cfg.Backups.Retention.Hourly = 1 | |||
| 61 | + | cfg.Backups.Retention.Daily = 7 | |||
| 62 | + | cfg.Backups.Retention.Weekly = 4 | |||
| 63 | + | cfg.Backups.Retention.Monthly = 12 | |||
| 64 | + | ||||
| 65 | + | retention, err := validateBackupRetention(cfg) | |||
| 66 | + | require.NoError(t, err) | |||
| 67 | + | assert.Equal(t, 1, retention.Hourly) | |||
| 68 | + | assert.Equal(t, 7, retention.Daily) | |||
| 69 | + | assert.Equal(t, 4, retention.Weekly) | |||
| 70 | + | assert.Equal(t, 12, retention.Monthly) | |||
| 71 | + | } | |||
M internal/domain/backup.go +1 -1
| @@ -49,7 +49,7 @@ type DBInfo struct { | |||
| 49 | 49 | ImageName string | |
| 50 | 50 | // Credentials contains sensitive values (passwords/tokens). | |
| 51 | 51 | // Never log or expose this map in API responses. | |
| 52 | - | Credentials map[string]string | |
| 52 | + | Credentials map[string]string `json:"-"` | |
| 53 | 53 | } | |
| 54 | 54 | ||
| 55 | 55 | // ClearCredentials clears sensitive credential values from DBInfo. | |
| @@ -49,7 +49,7 @@ type DBInfo struct { | |||||
| 49 | ImageName string | 49 | ImageName string | ||
| 50 | // Credentials contains sensitive values (passwords/tokens). | 50 | // Credentials contains sensitive values (passwords/tokens). | ||
| 51 | // Never log or expose this map in API responses. | 51 | // Never log or expose this map in API responses. | ||
| 52 | - | Credentials map[string]string | 52 | + | Credentials map[string]string `json:"-"` |
| 53 | } | 53 | } | ||
| 54 | 54 | ||||
| 55 | // ClearCredentials clears sensitive credential values from DBInfo. | 55 | // ClearCredentials clears sensitive credential values from DBInfo. | ||
A internal/domain/backup_test.go +24 -0
| @@ -0,0 +1,24 @@ | |||
| 1 | + | package domain | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "encoding/json" | |
| 5 | + | "testing" | |
| 6 | + | ||
| 7 | + | "github.com/stretchr/testify/assert" | |
| 8 | + | "github.com/stretchr/testify/require" | |
| 9 | + | ) | |
| 10 | + | ||
| 11 | + | func TestDBInfoJSONOmitsCredentials(t *testing.T) { | |
| 12 | + | info := DBInfo{ | |
| 13 | + | Type: DBTypePostgreSQL, | |
| 14 | + | Name: "postgres", | |
| 15 | + | Credentials: map[string]string{ | |
| 16 | + | "password": "secret", | |
| 17 | + | }, | |
| 18 | + | } | |
| 19 | + | ||
| 20 | + | payload, err := json.Marshal(info) | |
| 21 | + | require.NoError(t, err) | |
| 22 | + | assert.NotContains(t, string(payload), "credentials") | |
| 23 | + | assert.NotContains(t, string(payload), "secret") | |
| 24 | + | } | |
| @@ -0,0 +1,24 @@ | |||||
| 1 | + | package domain | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "encoding/json" | |||
| 5 | + | "testing" | |||
| 6 | + | ||||
| 7 | + | "github.com/stretchr/testify/assert" | |||
| 8 | + | "github.com/stretchr/testify/require" | |||
| 9 | + | ) | |||
| 10 | + | ||||
| 11 | + | func TestDBInfoJSONOmitsCredentials(t *testing.T) { | |||
| 12 | + | info := DBInfo{ | |||
| 13 | + | Type: DBTypePostgreSQL, | |||
| 14 | + | Name: "postgres", | |||
| 15 | + | Credentials: map[string]string{ | |||
| 16 | + | "password": "secret", | |||
| 17 | + | }, | |||
| 18 | + | } | |||
| 19 | + | ||||
| 20 | + | payload, err := json.Marshal(info) | |||
| 21 | + | require.NoError(t, err) | |||
| 22 | + | assert.NotContains(t, string(payload), "credentials") | |||
| 23 | + | assert.NotContains(t, string(payload), "secret") | |||
| 24 | + | } | |||
M internal/usecase/backup/detector.go +14 -2
| @@ -14,8 +14,20 @@ func detectDatabaseFromAttachment(domainName string, a domain.Attachment) (domai | |||
| 14 | 14 | switch { | |
| 15 | 15 | case strings.Contains(image, "postgres"), strings.Contains(name, "postgres"): | |
| 16 | 16 | port := 5432 | |
| 17 | - | if len(a.Ports) > 0 && a.Ports[0] > 0 { | |
| 18 | - | port = a.Ports[0] | |
| 17 | + | hasPostgresPort := false | |
| 18 | + | firstPositive := 0 | |
| 19 | + | for _, p := range a.Ports { | |
| 20 | + | if p == 5432 { | |
| 21 | + | port = 5432 | |
| 22 | + | hasPostgresPort = true | |
| 23 | + | break | |
| 24 | + | } | |
| 25 | + | if p > 0 && firstPositive == 0 { | |
| 26 | + | firstPositive = p | |
| 27 | + | } | |
| 28 | + | } | |
| 29 | + | if !hasPostgresPort && firstPositive > 0 { | |
| 30 | + | port = firstPositive | |
| 19 | 31 | } | |
| 20 | 32 | return domain.DBInfo{ | |
| 21 | 33 | Type: domain.DBTypePostgreSQL, | |
| @@ -14,8 +14,20 @@ func detectDatabaseFromAttachment(domainName string, a domain.Attachment) (domai | |||||
| 14 | switch { | 14 | switch { | ||
| 15 | case strings.Contains(image, "postgres"), strings.Contains(name, "postgres"): | 15 | case strings.Contains(image, "postgres"), strings.Contains(name, "postgres"): | ||
| 16 | port := 5432 | 16 | port := 5432 | ||
| 17 | - | if len(a.Ports) > 0 && a.Ports[0] > 0 { | 17 | + | hasPostgresPort := false |
| 18 | - | port = a.Ports[0] | 18 | + | firstPositive := 0 |
| 19 | + | for _, p := range a.Ports { | |||
| 20 | + | if p == 5432 { | |||
| 21 | + | port = 5432 | |||
| 22 | + | hasPostgresPort = true | |||
| 23 | + | break | |||
| 24 | + | } | |||
| 25 | + | if p > 0 && firstPositive == 0 { | |||
| 26 | + | firstPositive = p | |||
| 27 | + | } | |||
| 28 | + | } | |||
| 29 | + | if !hasPostgresPort && firstPositive > 0 { | |||
| 30 | + | port = firstPositive | |||
| 19 | } | 31 | } | ||
| 20 | return domain.DBInfo{ | 32 | return domain.DBInfo{ | ||
| 21 | Type: domain.DBTypePostgreSQL, | 33 | Type: domain.DBTypePostgreSQL, | ||
M internal/usecase/backup/detector_test.go +24 -0
| @@ -35,6 +35,30 @@ func TestDetectDatabaseFromAttachment_DefaultPort(t *testing.T) { | |||
| 35 | 35 | assert.Equal(t, "16", db.Version) | |
| 36 | 36 | } | |
| 37 | 37 | ||
| 38 | + | func TestDetectDatabaseFromAttachment_PrefersPostgresPort(t *testing.T) { | |
| 39 | + | db, ok := detectDatabaseFromAttachment("app.example.com", domain.Attachment{ | |
| 40 | + | Name: "postgres", | |
| 41 | + | Image: "postgres:16", | |
| 42 | + | ContainerID: "xyz789", | |
| 43 | + | Ports: []int{15432, 5432, 25432}, | |
| 44 | + | }) | |
| 45 | + | ||
| 46 | + | require.True(t, ok) | |
| 47 | + | assert.Equal(t, 5432, db.Port) | |
| 48 | + | } | |
| 49 | + | ||
| 50 | + | func TestDetectDatabaseFromAttachment_UsesFirstPositivePortWhen5432Missing(t *testing.T) { | |
| 51 | + | db, ok := detectDatabaseFromAttachment("app.example.com", domain.Attachment{ | |
| 52 | + | Name: "postgres", | |
| 53 | + | Image: "postgres:16", | |
| 54 | + | ContainerID: "xyz789", | |
| 55 | + | Ports: []int{0, -1, 15432, 25432}, | |
| 56 | + | }) | |
| 57 | + | ||
| 58 | + | require.True(t, ok) | |
| 59 | + | assert.Equal(t, 15432, db.Port) | |
| 60 | + | } | |
| 61 | + | ||
| 38 | 62 | func TestPostgresVersionFromImage(t *testing.T) { | |
| 39 | 63 | assert.Equal(t, "18", postgresVersionFromImage("postgres:18")) | |
| 40 | 64 | assert.Equal(t, "17", postgresVersionFromImage("postgres:17.4-alpine")) | |
| @@ -35,6 +35,30 @@ func TestDetectDatabaseFromAttachment_DefaultPort(t *testing.T) { | |||||
| 35 | assert.Equal(t, "16", db.Version) | 35 | assert.Equal(t, "16", db.Version) | ||
| 36 | } | 36 | } | ||
| 37 | 37 | ||||
| 38 | + | func TestDetectDatabaseFromAttachment_PrefersPostgresPort(t *testing.T) { | |||
| 39 | + | db, ok := detectDatabaseFromAttachment("app.example.com", domain.Attachment{ | |||
| 40 | + | Name: "postgres", | |||
| 41 | + | Image: "postgres:16", | |||
| 42 | + | ContainerID: "xyz789", | |||
| 43 | + | Ports: []int{15432, 5432, 25432}, | |||
| 44 | + | }) | |||
| 45 | + | ||||
| 46 | + | require.True(t, ok) | |||
| 47 | + | assert.Equal(t, 5432, db.Port) | |||
| 48 | + | } | |||
| 49 | + | ||||
| 50 | + | func TestDetectDatabaseFromAttachment_UsesFirstPositivePortWhen5432Missing(t *testing.T) { | |||
| 51 | + | db, ok := detectDatabaseFromAttachment("app.example.com", domain.Attachment{ | |||
| 52 | + | Name: "postgres", | |||
| 53 | + | Image: "postgres:16", | |||
| 54 | + | ContainerID: "xyz789", | |||
| 55 | + | Ports: []int{0, -1, 15432, 25432}, | |||
| 56 | + | }) | |||
| 57 | + | ||||
| 58 | + | require.True(t, ok) | |||
| 59 | + | assert.Equal(t, 15432, db.Port) | |||
| 60 | + | } | |||
| 61 | + | ||||
| 38 | func TestPostgresVersionFromImage(t *testing.T) { | 62 | func TestPostgresVersionFromImage(t *testing.T) { | ||
| 39 | assert.Equal(t, "18", postgresVersionFromImage("postgres:18")) | 63 | assert.Equal(t, "18", postgresVersionFromImage("postgres:18")) | ||
| 40 | assert.Equal(t, "17", postgresVersionFromImage("postgres:17.4-alpine")) | 64 | assert.Equal(t, "17", postgresVersionFromImage("postgres:17.4-alpine")) | ||
M internal/usecase/backup/integration_test.go +7 -2
| @@ -141,7 +141,9 @@ func waitForPostgresReady(ctx context.Context, runtime *docker.Runtime, containe | |||
| 141 | 141 | default: | |
| 142 | 142 | } | |
| 143 | 143 | ||
| 144 | - | res, err := runtime.ExecInContainer(ctx, containerID, []string{"pg_isready", "-U", "postgres", "-d", "appdb"}) | |
| 144 | + | execCtx, cancelExec := context.WithTimeout(ctx, 10*time.Second) | |
| 145 | + | res, err := runtime.ExecInContainer(execCtx, containerID, []string{"pg_isready", "-U", "postgres", "-d", "appdb"}) | |
| 146 | + | cancelExec() | |
| 145 | 147 | if err == nil && res.ExitCode == 0 { | |
| 146 | 148 | return nil | |
| 147 | 149 | } | |
| @@ -152,7 +154,10 @@ func waitForPostgresReady(ctx context.Context, runtime *docker.Runtime, containe | |||
| 152 | 154 | ||
| 153 | 155 | func seedPostgresData(ctx context.Context, runtime *docker.Runtime, containerID string) error { | |
| 154 | 156 | cmd := `psql -v ON_ERROR_STOP=1 -U "$POSTGRES_USER" -d postgres -c "SELECT 1 FROM pg_database WHERE datname='${POSTGRES_DB}'" | grep -q 1 || createdb -U "$POSTGRES_USER" "$POSTGRES_DB"; psql -v ON_ERROR_STOP=1 -U "$POSTGRES_USER" -d "$POSTGRES_DB" -c "CREATE TABLE IF NOT EXISTS backup_items (id serial primary key, name text); INSERT INTO backup_items(name) VALUES ('one'), ('two');"` | |
| 155 | - | res, err := runtime.ExecInContainer(ctx, containerID, []string{"sh", "-c", cmd}) | |
| 157 | + | execCtx, cancelExec := context.WithTimeout(ctx, 30*time.Second) | |
| 158 | + | defer cancelExec() | |
| 159 | + | ||
| 160 | + | res, err := runtime.ExecInContainer(execCtx, containerID, []string{"sh", "-c", cmd}) | |
| 156 | 161 | if err != nil { | |
| 157 | 162 | return err | |
| 158 | 163 | } | |
| @@ -141,7 +141,9 @@ func waitForPostgresReady(ctx context.Context, runtime *docker.Runtime, containe | |||||
| 141 | default: | 141 | default: | ||
| 142 | } | 142 | } | ||
| 143 | 143 | ||||
| 144 | - | res, err := runtime.ExecInContainer(ctx, containerID, []string{"pg_isready", "-U", "postgres", "-d", "appdb"}) | 144 | + | execCtx, cancelExec := context.WithTimeout(ctx, 10*time.Second) |
| 145 | + | res, err := runtime.ExecInContainer(execCtx, containerID, []string{"pg_isready", "-U", "postgres", "-d", "appdb"}) | |||
| 146 | + | cancelExec() | |||
| 145 | if err == nil && res.ExitCode == 0 { | 147 | if err == nil && res.ExitCode == 0 { | ||
| 146 | return nil | 148 | return nil | ||
| 147 | } | 149 | } | ||
| @@ -152,7 +154,10 @@ func waitForPostgresReady(ctx context.Context, runtime *docker.Runtime, containe | |||||
| 152 | 154 | ||||
| 153 | func seedPostgresData(ctx context.Context, runtime *docker.Runtime, containerID string) error { | 155 | func seedPostgresData(ctx context.Context, runtime *docker.Runtime, containerID string) error { | ||
| 154 | cmd := `psql -v ON_ERROR_STOP=1 -U "$POSTGRES_USER" -d postgres -c "SELECT 1 FROM pg_database WHERE datname='${POSTGRES_DB}'" | grep -q 1 || createdb -U "$POSTGRES_USER" "$POSTGRES_DB"; psql -v ON_ERROR_STOP=1 -U "$POSTGRES_USER" -d "$POSTGRES_DB" -c "CREATE TABLE IF NOT EXISTS backup_items (id serial primary key, name text); INSERT INTO backup_items(name) VALUES ('one'), ('two');"` | 156 | cmd := `psql -v ON_ERROR_STOP=1 -U "$POSTGRES_USER" -d postgres -c "SELECT 1 FROM pg_database WHERE datname='${POSTGRES_DB}'" | grep -q 1 || createdb -U "$POSTGRES_USER" "$POSTGRES_DB"; psql -v ON_ERROR_STOP=1 -U "$POSTGRES_USER" -d "$POSTGRES_DB" -c "CREATE TABLE IF NOT EXISTS backup_items (id serial primary key, name text); INSERT INTO backup_items(name) VALUES ('one'), ('two');"` | ||
| 155 | - | res, err := runtime.ExecInContainer(ctx, containerID, []string{"sh", "-c", cmd}) | 157 | + | execCtx, cancelExec := context.WithTimeout(ctx, 30*time.Second) |
| 158 | + | defer cancelExec() | |||
| 159 | + | ||||
| 160 | + | res, err := runtime.ExecInContainer(execCtx, containerID, []string{"sh", "-c", cmd}) | |||
| 156 | if err != nil { | 161 | if err != nil { | ||
| 157 | return err | 162 | return err | ||
| 158 | } | 163 | } | ||
M internal/usecase/backup/service.go +37 -4
| @@ -1,12 +1,12 @@ | |||
| 1 | 1 | package backup | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | - | "bytes" | |
| 5 | 4 | "context" | |
| 6 | 5 | "crypto/rand" | |
| 7 | 6 | "encoding/hex" | |
| 8 | 7 | "errors" | |
| 9 | 8 | "fmt" | |
| 9 | + | "io" | |
| 10 | 10 | "sort" | |
| 11 | 11 | "strings" | |
| 12 | 12 | "sync" | |
| @@ -84,8 +84,9 @@ func (s *Service) RunBackup(ctx context.Context, domainName, dbName string) (*do | |||
| 84 | 84 | ||
| 85 | 85 | execCtx, cancelExec := context.WithTimeout(ctx, backupExecTimeout) | |
| 86 | 86 | defer cancelExec() | |
| 87 | + | dumpPath := fmt.Sprintf("/tmp/gordon-backup-%d.bak", started.UnixNano()) | |
| 87 | 88 | ||
| 88 | - | execResult, err := s.runtime.ExecInContainer(execCtx, db.ContainerID, []string{"sh", "-c", postgresDumpCommand()}) | |
| 89 | + | execResult, err := s.runtime.ExecInContainer(execCtx, db.ContainerID, []string{"sh", "-c", pgDumpToPathCommand(dumpPath)}) | |
| 89 | 90 | if err != nil { | |
| 90 | 91 | if errors.Is(execCtx.Err(), context.DeadlineExceeded) { | |
| 91 | 92 | return nil, fmt.Errorf("pg_dump timed out after %s", backupExecTimeout) | |
| @@ -96,7 +97,19 @@ func (s *Service) RunBackup(ctx context.Context, domainName, dbName string) (*do | |||
| 96 | 97 | return nil, fmt.Errorf("pg_dump failed with exit code %d: %s", execResult.ExitCode, string(execResult.Stderr)) | |
| 97 | 98 | } | |
| 98 | 99 | ||
| 99 | - | path, err := s.storage.Store(ctx, domainName, db.Name, domain.BackupSchedule(""), started, bytes.NewReader(execResult.Stdout)) | |
| 100 | + | defer s.cleanupDumpFile(db.ContainerID, dumpPath) | |
| 101 | + | ||
| 102 | + | dumpStream, err := s.runtime.CopyFromContainer(execCtx, db.ContainerID, dumpPath) | |
| 103 | + | if err != nil { | |
| 104 | + | if errors.Is(execCtx.Err(), context.DeadlineExceeded) { | |
| 105 | + | return nil, fmt.Errorf("backup copy timed out after %s", backupExecTimeout) | |
| 106 | + | } | |
| 107 | + | return nil, err | |
| 108 | + | } | |
| 109 | + | defer dumpStream.Close() | |
| 110 | + | ||
| 111 | + | counter := &byteCounter{} | |
| 112 | + | path, err := s.storage.Store(ctx, domainName, db.Name, domain.BackupSchedule(""), started, io.TeeReader(dumpStream, counter)) | |
| 100 | 113 | if err != nil { | |
| 101 | 114 | return nil, err | |
| 102 | 115 | } | |
| @@ -109,7 +122,7 @@ func (s *Service) RunBackup(ctx context.Context, domainName, dbName string) (*do | |||
| 109 | 122 | Status: domain.BackupStatusCompleted, | |
| 110 | 123 | StartedAt: started, | |
| 111 | 124 | CompletedAt: time.Now().UTC(), | |
| 112 | - | SizeBytes: int64(len(execResult.Stdout)), | |
| 125 | + | SizeBytes: counter.n, | |
| 113 | 126 | FilePath: path, | |
| 114 | 127 | } | |
| 115 | 128 | ||
| @@ -213,3 +226,23 @@ func selectDatabase(dbs []domain.DBInfo, requested string) (domain.DBInfo, error | |||
| 213 | 226 | func postgresDumpCommand() string { | |
| 214 | 227 | return "pg_dump -Fc --dbname=\"${POSTGRES_DB:-postgres}\" --username=\"${POSTGRES_USER:-postgres}\"" | |
| 215 | 228 | } | |
| 229 | + | ||
| 230 | + | func pgDumpToPathCommand(path string) string { | |
| 231 | + | return fmt.Sprintf("%s > %q", postgresDumpCommand(), path) | |
| 232 | + | } | |
| 233 | + | ||
| 234 | + | func (s *Service) cleanupDumpFile(containerID, dumpPath string) { | |
| 235 | + | cleanupCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second) | |
| 236 | + | defer cancel() | |
| 237 | + | ||
| 238 | + | _, _ = s.runtime.ExecInContainer(cleanupCtx, containerID, []string{"sh", "-c", fmt.Sprintf("rm -f %q", dumpPath)}) | |
| 239 | + | } | |
| 240 | + | ||
| 241 | + | type byteCounter struct { | |
| 242 | + | n int64 | |
| 243 | + | } | |
| 244 | + | ||
| 245 | + | func (c *byteCounter) Write(p []byte) (int, error) { | |
| 246 | + | c.n += int64(len(p)) | |
| 247 | + | return len(p), nil | |
| 248 | + | } | |
| @@ -1,12 +1,12 @@ | |||||
| 1 | package backup | 1 | package backup | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | - | "bytes" | |||
| 5 | "context" | 4 | "context" | ||
| 6 | "crypto/rand" | 5 | "crypto/rand" | ||
| 7 | "encoding/hex" | 6 | "encoding/hex" | ||
| 8 | "errors" | 7 | "errors" | ||
| 9 | "fmt" | 8 | "fmt" | ||
| 9 | + | "io" | |||
| 10 | "sort" | 10 | "sort" | ||
| 11 | "strings" | 11 | "strings" | ||
| 12 | "sync" | 12 | "sync" | ||
| @@ -84,8 +84,9 @@ func (s *Service) RunBackup(ctx context.Context, domainName, dbName string) (*do | |||||
| 84 | 84 | ||||
| 85 | execCtx, cancelExec := context.WithTimeout(ctx, backupExecTimeout) | 85 | execCtx, cancelExec := context.WithTimeout(ctx, backupExecTimeout) | ||
| 86 | defer cancelExec() | 86 | defer cancelExec() | ||
| 87 | + | dumpPath := fmt.Sprintf("/tmp/gordon-backup-%d.bak", started.UnixNano()) | |||
| 87 | 88 | ||||
| 88 | - | execResult, err := s.runtime.ExecInContainer(execCtx, db.ContainerID, []string{"sh", "-c", postgresDumpCommand()}) | 89 | + | execResult, err := s.runtime.ExecInContainer(execCtx, db.ContainerID, []string{"sh", "-c", pgDumpToPathCommand(dumpPath)}) |
| 89 | if err != nil { | 90 | if err != nil { | ||
| 90 | if errors.Is(execCtx.Err(), context.DeadlineExceeded) { | 91 | if errors.Is(execCtx.Err(), context.DeadlineExceeded) { | ||
| 91 | return nil, fmt.Errorf("pg_dump timed out after %s", backupExecTimeout) | 92 | return nil, fmt.Errorf("pg_dump timed out after %s", backupExecTimeout) | ||
| @@ -96,7 +97,19 @@ func (s *Service) RunBackup(ctx context.Context, domainName, dbName string) (*do | |||||
| 96 | return nil, fmt.Errorf("pg_dump failed with exit code %d: %s", execResult.ExitCode, string(execResult.Stderr)) | 97 | return nil, fmt.Errorf("pg_dump failed with exit code %d: %s", execResult.ExitCode, string(execResult.Stderr)) | ||
| 97 | } | 98 | } | ||
| 98 | 99 | ||||
| 99 | - | path, err := s.storage.Store(ctx, domainName, db.Name, domain.BackupSchedule(""), started, bytes.NewReader(execResult.Stdout)) | 100 | + | defer s.cleanupDumpFile(db.ContainerID, dumpPath) |
| 101 | + | ||||
| 102 | + | dumpStream, err := s.runtime.CopyFromContainer(execCtx, db.ContainerID, dumpPath) | |||
| 103 | + | if err != nil { | |||
| 104 | + | if errors.Is(execCtx.Err(), context.DeadlineExceeded) { | |||
| 105 | + | return nil, fmt.Errorf("backup copy timed out after %s", backupExecTimeout) | |||
| 106 | + | } | |||
| 107 | + | return nil, err | |||
| 108 | + | } | |||
| 109 | + | defer dumpStream.Close() | |||
| 110 | + | ||||
| 111 | + | counter := &byteCounter{} | |||
| 112 | + | path, err := s.storage.Store(ctx, domainName, db.Name, domain.BackupSchedule(""), started, io.TeeReader(dumpStream, counter)) | |||
| 100 | if err != nil { | 113 | if err != nil { | ||
| 101 | return nil, err | 114 | return nil, err | ||
| 102 | } | 115 | } | ||
| @@ -109,7 +122,7 @@ func (s *Service) RunBackup(ctx context.Context, domainName, dbName string) (*do | |||||
| 109 | Status: domain.BackupStatusCompleted, | 122 | Status: domain.BackupStatusCompleted, | ||
| 110 | StartedAt: started, | 123 | StartedAt: started, | ||
| 111 | CompletedAt: time.Now().UTC(), | 124 | CompletedAt: time.Now().UTC(), | ||
| 112 | - | SizeBytes: int64(len(execResult.Stdout)), | 125 | + | SizeBytes: counter.n, |
| 113 | FilePath: path, | 126 | FilePath: path, | ||
| 114 | } | 127 | } | ||
| 115 | 128 | ||||
| @@ -213,3 +226,23 @@ func selectDatabase(dbs []domain.DBInfo, requested string) (domain.DBInfo, error | |||||
| 213 | func postgresDumpCommand() string { | 226 | func postgresDumpCommand() string { | ||
| 214 | return "pg_dump -Fc --dbname=\"${POSTGRES_DB:-postgres}\" --username=\"${POSTGRES_USER:-postgres}\"" | 227 | return "pg_dump -Fc --dbname=\"${POSTGRES_DB:-postgres}\" --username=\"${POSTGRES_USER:-postgres}\"" | ||
| 215 | } | 228 | } | ||
| 229 | + | ||||
| 230 | + | func pgDumpToPathCommand(path string) string { | |||
| 231 | + | return fmt.Sprintf("%s > %q", postgresDumpCommand(), path) | |||
| 232 | + | } | |||
| 233 | + | ||||
| 234 | + | func (s *Service) cleanupDumpFile(containerID, dumpPath string) { | |||
| 235 | + | cleanupCtx, cancel := context.WithTimeout(context.Background(), 30*time.Second) | |||
| 236 | + | defer cancel() | |||
| 237 | + | ||||
| 238 | + | _, _ = s.runtime.ExecInContainer(cleanupCtx, containerID, []string{"sh", "-c", fmt.Sprintf("rm -f %q", dumpPath)}) | |||
| 239 | + | } | |||
| 240 | + | ||||
| 241 | + | type byteCounter struct { | |||
| 242 | + | n int64 | |||
| 243 | + | } | |||
| 244 | + | ||||
| 245 | + | func (c *byteCounter) Write(p []byte) (int, error) { | |||
| 246 | + | c.n += int64(len(p)) | |||
| 247 | + | return len(p), nil | |||
| 248 | + | } | |||
M internal/usecase/backup/service_test.go +20 -3
| @@ -1,7 +1,9 @@ | |||
| 1 | 1 | package backup | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "bytes" | |
| 4 | 5 | "context" | |
| 6 | + | "io" | |
| 5 | 7 | "testing" | |
| 6 | 8 | ||
| 7 | 9 | inmocks "github.com/bnema/gordon/internal/boundaries/in/mocks" | |
| @@ -42,8 +44,19 @@ func TestService_RunBackup_Postgres(t *testing.T) { | |||
| 42 | 44 | }) | |
| 43 | 45 | ||
| 44 | 46 | runtime.EXPECT().ExecInContainer(mock.Anything, "db123", mock.MatchedBy(func(cmd []string) bool { | |
| 45 | - | return len(cmd) == 3 && cmd[0] == "sh" && cmd[1] == "-c" | |
| 46 | - | })).Return(&outiface.ExecResult{ExitCode: 0, Stdout: []byte("backup-data")}, nil) | |
| 47 | + | if len(cmd) != 3 || cmd[0] != "sh" || cmd[1] != "-c" { | |
| 48 | + | return false | |
| 49 | + | } | |
| 50 | + | return bytes.Contains([]byte(cmd[2]), []byte("pg_dump -Fc")) && bytes.Contains([]byte(cmd[2]), []byte(" > ")) | |
| 51 | + | })).Return(&outiface.ExecResult{ExitCode: 0}, nil) | |
| 52 | + | ||
| 53 | + | runtime.EXPECT().CopyFromContainer(mock.Anything, "db123", mock.MatchedBy(func(path string) bool { | |
| 54 | + | return path != "" | |
| 55 | + | })).Return(io.NopCloser(bytes.NewReader([]byte("backup-data"))), nil) | |
| 56 | + | ||
| 57 | + | runtime.EXPECT().ExecInContainer(mock.Anything, "db123", mock.MatchedBy(func(cmd []string) bool { | |
| 58 | + | return len(cmd) == 3 && cmd[0] == "sh" && cmd[1] == "-c" && bytes.Contains([]byte(cmd[2]), []byte("rm -f")) | |
| 59 | + | })).Return(&outiface.ExecResult{ExitCode: 0}, nil) | |
| 47 | 60 | ||
| 48 | 61 | storage.EXPECT().Store( | |
| 49 | 62 | mock.Anything, | |
| @@ -51,7 +64,10 @@ func TestService_RunBackup_Postgres(t *testing.T) { | |||
| 51 | 64 | "postgres", | |
| 52 | 65 | domain.BackupSchedule(""), | |
| 53 | 66 | mock.Anything, | |
| 54 | - | mock.Anything, | |
| 67 | + | mock.MatchedBy(func(r io.Reader) bool { | |
| 68 | + | data, err := io.ReadAll(r) | |
| 69 | + | return err == nil && string(data) == "backup-data" | |
| 70 | + | }), | |
| 55 | 71 | ).Return("/tmp/backup.bak", nil) | |
| 56 | 72 | ||
| 57 | 73 | svc := NewService(runtime, storage, containerSvc, domain.BackupConfig{}, zerowrap.Default()) | |
| @@ -61,4 +77,5 @@ func TestService_RunBackup_Postgres(t *testing.T) { | |||
| 61 | 77 | require.NotNil(t, result) | |
| 62 | 78 | assert.Equal(t, domain.BackupStatusCompleted, result.Job.Status) | |
| 63 | 79 | assert.Equal(t, "/tmp/backup.bak", result.Job.FilePath) | |
| 80 | + | assert.Equal(t, int64(len("backup-data")), result.Job.SizeBytes) | |
| 64 | 81 | } | |
| @@ -1,7 +1,9 @@ | |||||
| 1 | package backup | 1 | package backup | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "bytes" | |||
| 4 | "context" | 5 | "context" | ||
| 6 | + | "io" | |||
| 5 | "testing" | 7 | "testing" | ||
| 6 | 8 | ||||
| 7 | inmocks "github.com/bnema/gordon/internal/boundaries/in/mocks" | 9 | inmocks "github.com/bnema/gordon/internal/boundaries/in/mocks" | ||
| @@ -42,8 +44,19 @@ func TestService_RunBackup_Postgres(t *testing.T) { | |||||
| 42 | }) | 44 | }) | ||
| 43 | 45 | ||||
| 44 | runtime.EXPECT().ExecInContainer(mock.Anything, "db123", mock.MatchedBy(func(cmd []string) bool { | 46 | runtime.EXPECT().ExecInContainer(mock.Anything, "db123", mock.MatchedBy(func(cmd []string) bool { | ||
| 45 | - | return len(cmd) == 3 && cmd[0] == "sh" && cmd[1] == "-c" | 47 | + | if len(cmd) != 3 || cmd[0] != "sh" || cmd[1] != "-c" { |
| 46 | - | })).Return(&outiface.ExecResult{ExitCode: 0, Stdout: []byte("backup-data")}, nil) | 48 | + | return false |
| 49 | + | } | |||
| 50 | + | return bytes.Contains([]byte(cmd[2]), []byte("pg_dump -Fc")) && bytes.Contains([]byte(cmd[2]), []byte(" > ")) | |||
| 51 | + | })).Return(&outiface.ExecResult{ExitCode: 0}, nil) | |||
| 52 | + | ||||
| 53 | + | runtime.EXPECT().CopyFromContainer(mock.Anything, "db123", mock.MatchedBy(func(path string) bool { | |||
| 54 | + | return path != "" | |||
| 55 | + | })).Return(io.NopCloser(bytes.NewReader([]byte("backup-data"))), nil) | |||
| 56 | + | ||||
| 57 | + | runtime.EXPECT().ExecInContainer(mock.Anything, "db123", mock.MatchedBy(func(cmd []string) bool { | |||
| 58 | + | return len(cmd) == 3 && cmd[0] == "sh" && cmd[1] == "-c" && bytes.Contains([]byte(cmd[2]), []byte("rm -f")) | |||
| 59 | + | })).Return(&outiface.ExecResult{ExitCode: 0}, nil) | |||
| 47 | 60 | ||||
| 48 | storage.EXPECT().Store( | 61 | storage.EXPECT().Store( | ||
| 49 | mock.Anything, | 62 | mock.Anything, | ||
| @@ -51,7 +64,10 @@ func TestService_RunBackup_Postgres(t *testing.T) { | |||||
| 51 | "postgres", | 64 | "postgres", | ||
| 52 | domain.BackupSchedule(""), | 65 | domain.BackupSchedule(""), | ||
| 53 | mock.Anything, | 66 | mock.Anything, | ||
| 54 | - | mock.Anything, | 67 | + | mock.MatchedBy(func(r io.Reader) bool { |
| 68 | + | data, err := io.ReadAll(r) | |||
| 69 | + | return err == nil && string(data) == "backup-data" | |||
| 70 | + | }), | |||
| 55 | ).Return("/tmp/backup.bak", nil) | 71 | ).Return("/tmp/backup.bak", nil) | ||
| 56 | 72 | ||||
| 57 | svc := NewService(runtime, storage, containerSvc, domain.BackupConfig{}, zerowrap.Default()) | 73 | svc := NewService(runtime, storage, containerSvc, domain.BackupConfig{}, zerowrap.Default()) | ||
| @@ -61,4 +77,5 @@ func TestService_RunBackup_Postgres(t *testing.T) { | |||||
| 61 | require.NotNil(t, result) | 77 | require.NotNil(t, result) | ||
| 62 | assert.Equal(t, domain.BackupStatusCompleted, result.Job.Status) | 78 | assert.Equal(t, domain.BackupStatusCompleted, result.Job.Status) | ||
| 63 | assert.Equal(t, "/tmp/backup.bak", result.Job.FilePath) | 79 | assert.Equal(t, "/tmp/backup.bak", result.Job.FilePath) | ||
| 80 | + | assert.Equal(t, int64(len("backup-data")), result.Job.SizeBytes) | |||
| 64 | } | 81 | } | ||
M internal/usecase/cron/scheduler.go +14 -6
| @@ -153,12 +153,9 @@ func (s *Scheduler) RunNow(ctx context.Context, id string) error { | |||
| 153 | 153 | ||
| 154 | 154 | func (s *Scheduler) runDue(ctx context.Context) { | |
| 155 | 155 | now := s.nowFn() | |
| 156 | - | entries := s.snapshotEntries() | |
| 156 | + | entries := s.dueEntries(now) | |
| 157 | 157 | sem := make(chan struct{}, s.maxJobs) | |
| 158 | 158 | for _, e := range entries { | |
| 159 | - | if now.Before(e.nextRun) { | |
| 160 | - | continue | |
| 161 | - | } | |
| 162 | 159 | sem <- struct{}{} | |
| 163 | 160 | ||
| 164 | 161 | e := e | |
| @@ -178,7 +175,15 @@ func (s *Scheduler) executeEntry(ctx context.Context, e *entry) error { | |||
| 178 | 175 | defer e.running.Store(false) | |
| 179 | 176 | ||
| 180 | 177 | now := s.nowFn() | |
| 181 | - | jobErr := e.job(ctx) | |
| 178 | + | var jobErr error | |
| 179 | + | func() { | |
| 180 | + | defer func() { | |
| 181 | + | if rec := recover(); rec != nil { | |
| 182 | + | jobErr = fmt.Errorf("job panic: %v", rec) | |
| 183 | + | } | |
| 184 | + | }() | |
| 185 | + | jobErr = e.job(ctx) | |
| 186 | + | }() | |
| 182 | 187 | ||
| 183 | 188 | nextRun, nextErr := calculateNextRun(now, e.schedule) | |
| 184 | 189 | if nextErr != nil { | |
| @@ -202,12 +207,15 @@ func (s *Scheduler) getEntry(id string) *entry { | |||
| 202 | 207 | return s.entries[id] | |
| 203 | 208 | } | |
| 204 | 209 | ||
| 205 | - | func (s *Scheduler) snapshotEntries() []*entry { | |
| 210 | + | func (s *Scheduler) dueEntries(now time.Time) []*entry { | |
| 206 | 211 | s.mu.RLock() | |
| 207 | 212 | defer s.mu.RUnlock() | |
| 208 | 213 | ||
| 209 | 214 | entries := make([]*entry, 0, len(s.entries)) | |
| 210 | 215 | for _, e := range s.entries { | |
| 216 | + | if now.Before(e.nextRun) { | |
| 217 | + | continue | |
| 218 | + | } | |
| 211 | 219 | entries = append(entries, e) | |
| 212 | 220 | } | |
| 213 | 221 | return entries | |
| @@ -153,12 +153,9 @@ func (s *Scheduler) RunNow(ctx context.Context, id string) error { | |||||
| 153 | 153 | ||||
| 154 | func (s *Scheduler) runDue(ctx context.Context) { | 154 | func (s *Scheduler) runDue(ctx context.Context) { | ||
| 155 | now := s.nowFn() | 155 | now := s.nowFn() | ||
| 156 | - | entries := s.snapshotEntries() | 156 | + | entries := s.dueEntries(now) |
| 157 | sem := make(chan struct{}, s.maxJobs) | 157 | sem := make(chan struct{}, s.maxJobs) | ||
| 158 | for _, e := range entries { | 158 | for _, e := range entries { | ||
| 159 | - | if now.Before(e.nextRun) { | |||
| 160 | - | continue | |||
| 161 | - | } | |||
| 162 | sem <- struct{}{} | 159 | sem <- struct{}{} | ||
| 163 | 160 | ||||
| 164 | e := e | 161 | e := e | ||
| @@ -178,7 +175,15 @@ func (s *Scheduler) executeEntry(ctx context.Context, e *entry) error { | |||||
| 178 | defer e.running.Store(false) | 175 | defer e.running.Store(false) | ||
| 179 | 176 | ||||
| 180 | now := s.nowFn() | 177 | now := s.nowFn() | ||
| 181 | - | jobErr := e.job(ctx) | 178 | + | var jobErr error |
| 179 | + | func() { | |||
| 180 | + | defer func() { | |||
| 181 | + | if rec := recover(); rec != nil { | |||
| 182 | + | jobErr = fmt.Errorf("job panic: %v", rec) | |||
| 183 | + | } | |||
| 184 | + | }() | |||
| 185 | + | jobErr = e.job(ctx) | |||
| 186 | + | }() | |||
| 182 | 187 | ||||
| 183 | nextRun, nextErr := calculateNextRun(now, e.schedule) | 188 | nextRun, nextErr := calculateNextRun(now, e.schedule) | ||
| 184 | if nextErr != nil { | 189 | if nextErr != nil { | ||
| @@ -202,12 +207,15 @@ func (s *Scheduler) getEntry(id string) *entry { | |||||
| 202 | return s.entries[id] | 207 | return s.entries[id] | ||
| 203 | } | 208 | } | ||
| 204 | 209 | ||||
| 205 | - | func (s *Scheduler) snapshotEntries() []*entry { | 210 | + | func (s *Scheduler) dueEntries(now time.Time) []*entry { |
| 206 | s.mu.RLock() | 211 | s.mu.RLock() | ||
| 207 | defer s.mu.RUnlock() | 212 | defer s.mu.RUnlock() | ||
| 208 | 213 | ||||
| 209 | entries := make([]*entry, 0, len(s.entries)) | 214 | entries := make([]*entry, 0, len(s.entries)) | ||
| 210 | for _, e := range s.entries { | 215 | for _, e := range s.entries { | ||
| 216 | + | if now.Before(e.nextRun) { | |||
| 217 | + | continue | |||
| 218 | + | } | |||
| 211 | entries = append(entries, e) | 219 | entries = append(entries, e) | ||
| 212 | } | 220 | } | ||
| 213 | return entries | 221 | return entries | ||
M internal/usecase/cron/scheduler_test.go +18 -0
| @@ -106,3 +106,21 @@ func TestSchedulerRunNowRejectsConcurrentRun(t *testing.T) { | |||
| 106 | 106 | err1 := <-firstErrCh | |
| 107 | 107 | require.NoError(t, err1) | |
| 108 | 108 | } | |
| 109 | + | ||
| 110 | + | func TestSchedulerRunNowRecoversFromPanics(t *testing.T) { | |
| 111 | + | s := NewScheduler(zerowrap.Default()) | |
| 112 | + | err := s.Add("panic-job", "panic job", domain.CronSchedule{Preset: domain.ScheduleHourly}, func(context.Context) error { | |
| 113 | + | panic("boom") | |
| 114 | + | }) | |
| 115 | + | require.NoError(t, err) | |
| 116 | + | ||
| 117 | + | err = s.RunNow(context.Background(), "panic-job") | |
| 118 | + | require.Error(t, err) | |
| 119 | + | assert.Contains(t, err.Error(), "panic") | |
| 120 | + | ||
| 121 | + | entries := s.List() | |
| 122 | + | require.Len(t, entries, 1) | |
| 123 | + | assert.False(t, entries[0].Running) | |
| 124 | + | assert.False(t, entries[0].LastRun.IsZero()) | |
| 125 | + | assert.False(t, entries[0].NextRun.IsZero()) | |
| 126 | + | } | |
| @@ -106,3 +106,21 @@ func TestSchedulerRunNowRejectsConcurrentRun(t *testing.T) { | |||||
| 106 | err1 := <-firstErrCh | 106 | err1 := <-firstErrCh | ||
| 107 | require.NoError(t, err1) | 107 | require.NoError(t, err1) | ||
| 108 | } | 108 | } | ||
| 109 | + | ||||
| 110 | + | func TestSchedulerRunNowRecoversFromPanics(t *testing.T) { | |||
| 111 | + | s := NewScheduler(zerowrap.Default()) | |||
| 112 | + | err := s.Add("panic-job", "panic job", domain.CronSchedule{Preset: domain.ScheduleHourly}, func(context.Context) error { | |||
| 113 | + | panic("boom") | |||
| 114 | + | }) | |||
| 115 | + | require.NoError(t, err) | |||
| 116 | + | ||||
| 117 | + | err = s.RunNow(context.Background(), "panic-job") | |||
| 118 | + | require.Error(t, err) | |||
| 119 | + | assert.Contains(t, err.Error(), "panic") | |||
| 120 | + | ||||
| 121 | + | entries := s.List() | |||
| 122 | + | require.Len(t, entries, 1) | |||
| 123 | + | assert.False(t, entries[0].Running) | |||
| 124 | + | assert.False(t, entries[0].LastRun.IsZero()) | |||
| 125 | + | assert.False(t, entries[0].NextRun.IsZero()) | |||
| 126 | + | } | |||