feat(backup): sync SFTP + scheduling config from sycorax#194
Conversation
- Add spatie/laravel-backup and league/flysystem-sftp-v3 - Add sftp filesystem disk driven by BACKUP_VPS_* env vars - Make backup destinations configurable via BACKUP_DISKS - Enable gzip DB dump compression and colon-safe YmdHis timestamps - Derive backup identifier from slugged APP_NAME - Raise retry_delay to 5s (backup + cleanup) - Switch to custom SimpleStrategy with env-driven keep_old_backups_count - Reschedule: backup:run every 15min, backup:clean every odd hour, backup:monitor daily at 09:00, cloudflare:reload twice monthly - Guard telescope:prune and cloudflare:reload with when() checks
…oser.json - Bump axios from 1.13.6 to 1.15.0 - Update rollup and related packages to version 4.60.1 - Upgrade filament and related plugins to version 5.5.0 - Update laravel/framework to version 12.56.0 - Upgrade laravel/telescope to version 5.20.0 - Update other dependencies to their latest versions
📝 WalkthroughWalkthroughThis pull request introduces backup functionality to the application. Environment variables for backup configuration are added to example files, including settings for enabling backups, selecting target disks, retention policies, and optional Discord notifications. A new 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
routes/console.php (1)
23-26: Minor: UsedailyAt()for conciseness.♻️ Optional simplification
Schedule::command('backup:monitor') ->when(fn () => config('backup.enabled')) - ->daily() - ->at('09:00'); + ->dailyAt('09:00');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@routes/console.php` around lines 23 - 26, Replace the Schedule job chaining that uses ->daily()->at('09:00') with the concise ->dailyAt('09:00') form: update the Schedule::command('backup:monitor')->when(fn () => config('backup.enabled'))->daily()->at('09:00') chain to use ->dailyAt('09:00') while keeping the command name and the when(...) callback unchanged.config/filesystems.php (1)
77-85: Consider adding timeout and error handling options for SFTP resilience.The SFTP disk lacks
timeout,throw, andreportoptions that are present on other disks. For backup operations over potentially slow or unreliable connections, a timeout prevents indefinite hangs.♻️ Proposed enhancement
'sftp' => [ 'driver' => 'sftp', 'host' => env('BACKUP_VPS_HOST'), 'username' => env('BACKUP_VPS_USER'), 'password' => env('BACKUP_VPS_PASSWORD'), 'privateKey' => env('BACKUP_VPS_KEY_PATH'), 'port' => (int) env('BACKUP_VPS_PORT', 22), 'root' => env('BACKUP_VPS_ROOT', '/backups'), + 'timeout' => 30, + 'throw' => false, + 'report' => false, ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@config/filesystems.php` around lines 77 - 85, The SFTP disk configuration ('sftp' array) currently lacks connection resilience settings; add a 'timeout' option (use (int) env('BACKUP_VPS_TIMEOUT', 30) or similar), and include 'throw' and 'report' boolean options (driven by env vars like BACKUP_VPS_THROW/BACKUP_VPS_REPORT or true/false defaults) so operations time out and errors are surfaced/handled consistently; update the 'sftp' entry in config/filesystems.php to read these env-backed settings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.example:
- Around line 90-94: Add the two missing environment variables referenced in
config/backup.php to the .env.example: document BACKUP_ARCHIVE_PASSWORD (used
for archive encryption; leave blank by default or show placeholder) and
BACKUP_NOTIFIABLE_CHANNEL (used for notification routing/driver/channel; provide
a brief description and default/empty value). Ensure the variable names exactly
match BACKUP_ARCHIVE_PASSWORD and BACKUP_NOTIFIABLE_CHANNEL and include concise
comments/descriptions so developers know their purpose and expected format.
In `@app/Tasks/Cleanup/Strategies/SimpleStrategy.php`:
- Around line 21-31: After calling $backups->shift() and deleting all backups
when $keepOldBackupsCount === 0, add an early return to stop further processing
so the subsequent check ($backups->count() > $keepOldBackupsCount) does not run
and attempt to delete the same backups again; update the method in
SimpleStrategy (the block using $backups->shift(), the if ($keepOldBackupsCount
=== 0) { ... } branch, and the later if ($backups->count() >
$keepOldBackupsCount) { ... } block) to return immediately after deleting when
keepOldBackupsCount is zero.
- Around line 12-15: The config value for
backup.cleanup.simple_strategy.keep_old_backups_count is being forced to an int
which turns null into 0 and prevents
SimpleStrategy::deleteOldBackups(BackupCollection $backups) from seeing null;
remove the (int) cast in config/backup.php so the null setting can propagate
(or, if you prefer a non-null sentinel, change the logic and config to use a
sentinel like -1 consistently and update SimpleStrategy to check for that
instead).
In `@config/backup.php`:
- Around line 231-232: Replace the hardcoded placeholder in the backup config's
mail recipient by reading from an environment variable: change the 'mail'
array's 'to' value (the 'mail' => ['to' => 'your@example.com'] entry) to use
env('BACKUP_MAIL_TO', 'your@example.com') (or env('MAIL_TO', ...)) so the
address is configurable; update your .env with BACKUP_MAIL_TO and re-run
config:cache when deploying.
- Around line 296-313: The (int) cast on the simple_strategy config key
keep_old_backups_count forces env('BACKUP_KEEP_OLD_BACKUPS_COUNT', 50) to an
integer and prevents null from being preserved to disable cleanup; change the
assignment to preserve null/empty by removing the (int) cast and parsing the env
value so that empty/unset returns null (or explicitly map '' to null and
otherwise cast to int), e.g., read env('BACKUP_KEEP_OLD_BACKUPS_COUNT') into a
variable and return null if it's null/empty else (int) the value; also update
.env.example to document that leaving BACKUP_KEEP_OLD_BACKUPS_COUNT empty
disables cleanup and that numeric values enable retention.
---
Nitpick comments:
In `@config/filesystems.php`:
- Around line 77-85: The SFTP disk configuration ('sftp' array) currently lacks
connection resilience settings; add a 'timeout' option (use (int)
env('BACKUP_VPS_TIMEOUT', 30) or similar), and include 'throw' and 'report'
boolean options (driven by env vars like BACKUP_VPS_THROW/BACKUP_VPS_REPORT or
true/false defaults) so operations time out and errors are surfaced/handled
consistently; update the 'sftp' entry in config/filesystems.php to read these
env-backed settings.
In `@routes/console.php`:
- Around line 23-26: Replace the Schedule job chaining that uses
->daily()->at('09:00') with the concise ->dailyAt('09:00') form: update the
Schedule::command('backup:monitor')->when(fn () =>
config('backup.enabled'))->daily()->at('09:00') chain to use ->dailyAt('09:00')
while keeping the command name and the when(...) callback unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c6506f3d-a4e8-426e-9982-e5cfc972893f
⛔ Files ignored due to path filters (2)
composer.lockis excluded by!**/*.lockpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
.env.example.env.testing.exampleapp/Tasks/Cleanup/Strategies/SimpleStrategy.phpcomposer.jsonconfig/backup.phpconfig/filesystems.phppackage.jsonroutes/console.php
Summary by CodeRabbit
New Features
Chores