v2: add pgbackrest support #7556
Conversation
algora-pbc
bot
added
the
🙋 Bounty claim
SkyfallWasTaken
force-pushed
the
pgbackrest
branch
from
8efe8db to
10f55a7
Compare
pgBackRest Further code quality changes Clean up code Small fix Hopefully the last bit of cleanup? Remove old comment! A couple more improvements Squash migrations
SkyfallWasTaken
force-pushed
the
pgbackrest
branch
from
a331a6b to
1e529c5
Compare
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds full pgBackRest support: new models, service, jobs, action, controller endpoints, Livewire UI, notifications, helpers, migration, and StartPostgresql integration to perform, monitor, and restore pgBackRest backups for standalone PostgreSQL databases. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant API as DatabasesController
participant DB as StandalonePostgresql
participant Action as PgBackrestRestore
participant RestoreRec as DatabaseRestore
participant Job as PgBackrestRestoreJob
participant Service as PgBackrestService
participant Sidecar as SidecarContainer
Client->>API: POST /databases/{uuid}/restore (execution_uuid?, target_time?)
API->>DB: authorize & load database
API->>Action: PgBackrestRestore::handle(database, execution?, targetTime?)
Action->>RestoreRec: create pending DatabaseRestore
Action->>Job: dispatch PgBackrestRestoreJob(database, restore, execution?, targetTime?)
Job->>Service: build restore/info commands (stanza/label/targetTime, env)
Job->>DB: stop PostgreSQL container
Job->>Sidecar: run sidecar info/stanza-create/restore commands (with mounts + env)
Sidecar-->>Job: return logs/results
Job->>DB: start PostgreSQL container
Job->>RestoreRec: updateStatus(success/failed), appendLog
Job->>API: send notifications (DatabaseRestoreSuccess/Failed)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Areas to focus during review:
Possibly related issues
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 23
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (26)
app/Actions/Database/PgBackrestRestore.php(1 hunks)app/Actions/Database/StartPostgresql.php(8 hunks)app/Http/Controllers/Api/DatabasesController.php(11 hunks)app/Jobs/DatabaseBackupJob.php(3 hunks)app/Jobs/PgBackrestRestoreJob.php(1 hunks)app/Livewire/Project/Database/BackupEdit.php(5 hunks)app/Livewire/Project/Database/BackupExecutions.php(3 hunks)app/Models/DatabaseRestore.php(1 hunks)app/Models/PgbackrestRepo.php(1 hunks)app/Models/ScheduledDatabaseBackup.php(1 hunks)app/Models/ScheduledDatabaseBackupExecution.php(2 hunks)app/Models/StandalonePostgresql.php(2 hunks)app/Notifications/Database/BackupFailed.php(2 hunks)app/Notifications/Database/BackupSuccess.php(2 hunks)app/Notifications/Database/DatabaseRestoreFailed.php(1 hunks)app/Notifications/Database/DatabaseRestoreSuccess.php(1 hunks)app/Services/Backup/PgBackrestService.php(1 hunks)bootstrap/helpers/databases.php(1 hunks)bootstrap/helpers/shared.php(1 hunks)database/migrations/2025_12_09_231049_add_pgbackrest_fields_to_scheduled_database_backup_executions_table.php(1 hunks)database/migrations/2025_12_09_231050_create_database_restores_table.php(1 hunks)resources/views/emails/database-restore-failed.blade.php(1 hunks)resources/views/emails/database-restore-success.blade.php(1 hunks)resources/views/livewire/project/database/backup-edit.blade.php(1 hunks)resources/views/livewire/project/database/backup-executions.blade.php(5 hunks)routes/api.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
**/*.php
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
Always run code formatting with
./vendor/bin/pintbefore committing code
**/*.php: Follow PSR-12 coding standards. Use Laravel Pint for automatic formatting. Write descriptive variable and method names. Keep methods small and focused. Document complex logic with clear comments
Use PHP 8.4 constructor property promotion and typed properties
Never useenv()outside config files in Laravel
Files:
app/Models/DatabaseRestore.phpapp/Notifications/Database/BackupSuccess.phpresources/views/livewire/project/database/backup-edit.blade.phpbootstrap/helpers/databases.phpapp/Actions/Database/PgBackrestRestore.phpapp/Notifications/Database/BackupFailed.phpresources/views/emails/database-restore-success.blade.phpdatabase/migrations/2025_12_09_231049_add_pgbackrest_fields_to_scheduled_database_backup_executions_table.phpresources/views/livewire/project/database/backup-executions.blade.phpapp/Jobs/PgBackrestRestoreJob.phpapp/Models/StandalonePostgresql.phproutes/api.phpapp/Actions/Database/StartPostgresql.phpdatabase/migrations/2025_12_09_231050_create_database_restores_table.phpapp/Models/ScheduledDatabaseBackup.phpapp/Http/Controllers/Api/DatabasesController.phpapp/Models/PgbackrestRepo.phpresources/views/emails/database-restore-failed.blade.phpapp/Notifications/Database/DatabaseRestoreSuccess.phpapp/Livewire/Project/Database/BackupExecutions.phpapp/Livewire/Project/Database/BackupEdit.phpbootstrap/helpers/shared.phpapp/Notifications/Database/DatabaseRestoreFailed.phpapp/Jobs/DatabaseBackupJob.phpapp/Models/ScheduledDatabaseBackupExecution.phpapp/Services/Backup/PgBackrestService.php
{**/*Policy.php,**/*Gate.php,app/Models/**/*.php,routes/**/*.php}
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
Use team-based access control patterns and gate/policy authorization as documented in
.ai/patterns/security-patterns.md
Files:
app/Models/DatabaseRestore.phpapp/Models/StandalonePostgresql.phproutes/api.phpapp/Models/ScheduledDatabaseBackup.phpapp/Models/PgbackrestRepo.phpapp/Models/ScheduledDatabaseBackupExecution.php
app/Models/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
app/Models/**/*.php: When adding new database columns, ALWAYS update the model's$fillablearray to allow mass assignment
Use Eloquent ORM for database interactions, implement relationships properly (HasMany, BelongsTo, etc.), use database transactions for critical operations, leverage query scopes for reusable queries, and apply indexes for performance-critical queries
Always use team() method to return relationship instance, not direct property access. App\Models\Application::team must return a relationship instance
Files:
app/Models/DatabaseRestore.phpapp/Models/StandalonePostgresql.phpapp/Models/ScheduledDatabaseBackup.phpapp/Models/PgbackrestRepo.phpapp/Models/ScheduledDatabaseBackupExecution.php
app/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
app/**/*.php: Use eager loading to prevent N+1 queries, implement caching for frequently accessed data, queue heavy operations, optimize database queries with proper indexes, use chunking for large data operations
UseownedByCurrentTeamCached()instead ofownedByCurrentTeam()->get()for team-scoped queries to avoid duplicate database queries
Queue heavy operations with Laravel Horizon
Files:
app/Models/DatabaseRestore.phpapp/Notifications/Database/BackupSuccess.phpapp/Actions/Database/PgBackrestRestore.phpapp/Notifications/Database/BackupFailed.phpapp/Jobs/PgBackrestRestoreJob.phpapp/Models/StandalonePostgresql.phpapp/Actions/Database/StartPostgresql.phpapp/Models/ScheduledDatabaseBackup.phpapp/Http/Controllers/Api/DatabasesController.phpapp/Models/PgbackrestRepo.phpapp/Notifications/Database/DatabaseRestoreSuccess.phpapp/Livewire/Project/Database/BackupExecutions.phpapp/Livewire/Project/Database/BackupEdit.phpapp/Notifications/Database/DatabaseRestoreFailed.phpapp/Jobs/DatabaseBackupJob.phpapp/Models/ScheduledDatabaseBackupExecution.phpapp/Services/Backup/PgBackrestService.php
**/*.{php,blade.php}
📄 CodeRabbit inference engine (CLAUDE.md)
Use named routes with
route()function instead of hardcoded URLs
Files:
app/Models/DatabaseRestore.phpapp/Notifications/Database/BackupSuccess.phpresources/views/livewire/project/database/backup-edit.blade.phpbootstrap/helpers/databases.phpapp/Actions/Database/PgBackrestRestore.phpapp/Notifications/Database/BackupFailed.phpresources/views/emails/database-restore-success.blade.phpdatabase/migrations/2025_12_09_231049_add_pgbackrest_fields_to_scheduled_database_backup_executions_table.phpresources/views/livewire/project/database/backup-executions.blade.phpapp/Jobs/PgBackrestRestoreJob.phpapp/Models/StandalonePostgresql.phproutes/api.phpapp/Actions/Database/StartPostgresql.phpdatabase/migrations/2025_12_09_231050_create_database_restores_table.phpapp/Models/ScheduledDatabaseBackup.phpapp/Http/Controllers/Api/DatabasesController.phpapp/Models/PgbackrestRepo.phpresources/views/emails/database-restore-failed.blade.phpapp/Notifications/Database/DatabaseRestoreSuccess.phpapp/Livewire/Project/Database/BackupExecutions.phpapp/Livewire/Project/Database/BackupEdit.phpbootstrap/helpers/shared.phpapp/Notifications/Database/DatabaseRestoreFailed.phpapp/Jobs/DatabaseBackupJob.phpapp/Models/ScheduledDatabaseBackupExecution.phpapp/Services/Backup/PgBackrestService.php
**/**/livewire/**/*.blade.php
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
Livewire components MUST have exactly ONE root element with no exceptions
Files:
resources/views/livewire/project/database/backup-edit.blade.phpresources/views/livewire/project/database/backup-executions.blade.php
**/*.blade.php
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
**/*.blade.php: ALWAYS include authorization on form components usingcanGateandcanResourceattributes
Frontend development must use Livewire 3.5.20 for server-side state, Alpine.js for client interactions, and Tailwind CSS 4.1.4 for styling
Files:
resources/views/livewire/project/database/backup-edit.blade.phpresources/views/emails/database-restore-success.blade.phpresources/views/livewire/project/database/backup-executions.blade.phpresources/views/emails/database-restore-failed.blade.php
resources/views/livewire/**/*.blade.php
📄 CodeRabbit inference engine (CLAUDE.md)
resources/views/livewire/**/*.blade.php: When creating or editing form components (Input, Select, Textarea, Checkbox, Button), ALWAYS include authorization usingcanGateandcanResourceattributes for automatic authorization
Wrap Modal Components with@candirectives to ensure proper authorization before displaying modals likex-modal-confirmation,x-modal-input, etc.
Livewire component views MUST have exactly ONE root element. ALL content must be contained within this single root element. Placing ANY elements (<style>, <script>,, comments, or other HTML) outside the root will break Livewire's component tracking and cause wire:click and other directives to fail silently
Usewire:model.livefor real-time updates in Livewire components
Files:
resources/views/livewire/project/database/backup-edit.blade.phpresources/views/livewire/project/database/backup-executions.blade.php
resources/views/**/*.blade.php
📄 CodeRabbit inference engine (CLAUDE.md)
Tailwind CSS: Use new utilities (version 4.1.4), not deprecated ones. Use
gaputilities for spacing, not margins
Files:
resources/views/livewire/project/database/backup-edit.blade.phpresources/views/emails/database-restore-success.blade.phpresources/views/livewire/project/database/backup-executions.blade.phpresources/views/emails/database-restore-failed.blade.php
{**/*Model.php,database/migrations/**/*.php}
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
Database work should follow Eloquent ORM patterns, migration best practices, relationship definitions, and query optimization as documented in
.ai/patterns/database-patterns.md
Files:
database/migrations/2025_12_09_231049_add_pgbackrest_fields_to_scheduled_database_backup_executions_table.phpdatabase/migrations/2025_12_09_231050_create_database_restores_table.php
routes/api.php
📄 CodeRabbit inference engine (CLAUDE.md)
RESTful endpoints in
routes/api.phpshould use API Resources for response formatting, implement rate limiting for public endpoints, version APIs when making breaking changes, and document endpoints with clear examples
Files:
routes/api.php
routes/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
Group routes by middleware and prefix, use route model binding for cleaner controllers, name routes consistently (resource.action), implement proper HTTP verbs (GET, POST, PUT, DELETE)
Files:
routes/api.php
app/Livewire/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
In Livewire Components, always add the
AuthorizesRequeststrait and check permissions with$this->authorize()calls in mount() and action methods
Files:
app/Livewire/Project/Database/BackupExecutions.phpapp/Livewire/Project/Database/BackupEdit.php
🧠 Learnings (5)
📚 Learning: 2025-11-25T09:32:48.519Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/coolify-ai-docs.mdc:0-0
Timestamp: 2025-11-25T09:32:48.519Z
Learning: Applies to {**/*Model.php,database/migrations/**/*.php} : Database work should follow Eloquent ORM patterns, migration best practices, relationship definitions, and query optimization as documented in `.ai/patterns/database-patterns.md`
Applied to files:
app/Models/DatabaseRestore.phpdatabase/migrations/2025_12_09_231050_create_database_restores_table.php
📚 Learning: 2025-12-08T20:23:33.775Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T20:23:33.775Z
Learning: Applies to app/Models/**/*.php : Use Eloquent ORM for database interactions, implement relationships properly (HasMany, BelongsTo, etc.), use database transactions for critical operations, leverage query scopes for reusable queries, and apply indexes for performance-critical queries
Applied to files:
app/Models/DatabaseRestore.phpapp/Models/ScheduledDatabaseBackup.phpapp/Http/Controllers/Api/DatabasesController.phpapp/Models/PgbackrestRepo.phpapp/Models/ScheduledDatabaseBackupExecution.php
📚 Learning: 2025-12-08T20:23:33.775Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T20:23:33.775Z
Learning: Applies to routes/api.php : RESTful endpoints in `routes/api.php` should use API Resources for response formatting, implement rate limiting for public endpoints, version APIs when making breaking changes, and document endpoints with clear examples
Applied to files:
routes/api.php
📚 Learning: 2025-12-08T20:23:33.775Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T20:23:33.775Z
Learning: Applies to routes/**/*.php : Group routes by middleware and prefix, use route model binding for cleaner controllers, name routes consistently (resource.action), implement proper HTTP verbs (GET, POST, PUT, DELETE)
Applied to files:
routes/api.php
📚 Learning: 2025-12-08T20:23:33.775Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T20:23:33.775Z
Learning: Applies to app/**/*.php : Use eager loading to prevent N+1 queries, implement caching for frequently accessed data, queue heavy operations, optimize database queries with proper indexes, use chunking for large data operations
Applied to files:
app/Models/ScheduledDatabaseBackupExecution.php
🧬 Code graph analysis (13)
app/Models/DatabaseRestore.php (1)
app/Models/ScheduledDatabaseBackupExecution.php (2)
casts(12-20)ScheduledDatabaseBackupExecution(8-48)
app/Notifications/Database/BackupSuccess.php (2)
app/Notifications/Database/BackupFailed.php (1)
__construct(22-29)app/Models/ScheduledDatabaseBackup.php (3)
ScheduledDatabaseBackup(9-144)database(89-92)isPgBackrest(24-27)
bootstrap/helpers/databases.php (2)
app/Models/ScheduledDatabaseBackup.php (1)
isPgBackrest(24-27)app/Models/ScheduledDatabaseBackupExecution.php (1)
isPgBackrest(32-35)
app/Notifications/Database/BackupFailed.php (2)
app/Notifications/Database/BackupSuccess.php (1)
__construct(22-30)app/Models/ScheduledDatabaseBackup.php (3)
ScheduledDatabaseBackup(9-144)database(89-92)isPgBackrest(24-27)
database/migrations/2025_12_09_231049_add_pgbackrest_fields_to_scheduled_database_backup_executions_table.php (1)
database/migrations/2025_12_09_231050_create_database_restores_table.php (2)
up(9-37)down(39-42)
app/Models/StandalonePostgresql.php (6)
app/Models/LocalPersistentVolume.php (2)
LocalPersistentVolume(9-117)database(23-26)app/Models/DatabaseRestore.php (2)
database(20-23)DatabaseRestore(8-77)app/Models/ScheduledDatabaseBackup.php (1)
database(89-92)app/Models/ServiceDatabase.php (2)
scheduledBackups(155-158)persistentStorages(140-143)app/Models/ServiceApplication.php (1)
persistentStorages(116-119)app/Models/Application.php (1)
persistentStorages(492-495)
app/Actions/Database/StartPostgresql.php (4)
app/Services/Backup/PgBackrestService.php (3)
PgBackrestService(9-353)getStanzaName(23-26)generateConfig(28-75)app/Jobs/PgBackrestRestoreJob.php (1)
getHostPath(413-416)app/Models/StandalonePostgresql.php (1)
pgbackrestBackups(354-357)bootstrap/helpers/shared.php (1)
convertPathToDockerHost(3319-3326)
database/migrations/2025_12_09_231050_create_database_restores_table.php (1)
database/migrations/2025_12_09_231049_add_pgbackrest_fields_to_scheduled_database_backup_executions_table.php (1)
up(9-55)
app/Models/ScheduledDatabaseBackup.php (3)
app/Models/DatabaseRestore.php (2)
casts(12-18)DatabaseRestore(8-77)app/Models/PgbackrestRepo.php (2)
casts(12-20)PgbackrestRepo(8-72)app/Models/ScheduledDatabaseBackupExecution.php (5)
casts(12-20)isPgBackrest(32-35)isNative(37-40)restores(27-30)ScheduledDatabaseBackupExecution(8-48)
app/Models/PgbackrestRepo.php (1)
app/Models/ScheduledDatabaseBackup.php (2)
ScheduledDatabaseBackup(9-144)database(89-92)
app/Livewire/Project/Database/BackupEdit.php (4)
app/Models/PgbackrestRepo.php (1)
PgbackrestRepo(8-72)bootstrap/helpers/shared.php (1)
validateShellSafePath(119-147)app/Models/ScheduledDatabaseBackup.php (2)
localRepo(44-47)s3Repo(49-52)app/Models/StandalonePostgresql.php (1)
type(242-245)
app/Notifications/Database/DatabaseRestoreFailed.php (3)
app/Models/StandalonePostgresql.php (1)
StandalonePostgresql(11-424)app/Notifications/Database/DatabaseRestoreSuccess.php (3)
__construct(20-30)via(32-35)toMail(37-48)app/Traits/HasNotificationSettings.php (1)
getEnabledChannels(73-97)
app/Models/ScheduledDatabaseBackupExecution.php (2)
app/Models/ScheduledDatabaseBackup.php (4)
ScheduledDatabaseBackup(9-144)restores(64-72)isPgBackrest(24-27)isNative(29-32)app/Models/DatabaseRestore.php (1)
DatabaseRestore(8-77)
🪛 PHPMD (2.15.0)
app/Actions/Database/PgBackrestRestore.php
27-27: Missing class import via use statement (line '27', column '27'). (undefined)
(MissingImport)
31-40: Avoid using static access to class '\App\Models\DatabaseRestore' in method 'handle'. (undefined)
(StaticAccess)
42-42: Avoid using static access to class '\App\Jobs\PgBackrestRestoreJob' in method 'handle'. (undefined)
(StaticAccess)
app/Jobs/PgBackrestRestoreJob.php
22-439: The class PgBackrestRestoreJob has an overall complexity of 58 which is very high. The configured complexity threshold is 50. (undefined)
(ExcessiveClassComplexity)
41-41: Avoid unused local variables such as '$server'. (undefined)
(UnusedLocalVariable)
42-42: Avoid unused local variables such as '$containerName'. (undefined)
(UnusedLocalVariable)
43-43: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'handle'. (undefined)
(StaticAccess)
52-52: Avoid using static access to class '\App\Actions\Database\StopDatabase' in method 'handle'. (undefined)
(StaticAccess)
68-68: Avoid using static access to class '\App\Actions\Database\StartPostgresql' in method 'handle'. (undefined)
(StaticAccess)
91-96: Avoid using static access to class '\Illuminate\Support\Facades\Log' in method 'handle'. (undefined)
(StaticAccess)
111-111: Avoid using static access to class '\App\Actions\Database\StartPostgresql' in method 'handle'. (undefined)
(StaticAccess)
120-183: The method preflight() has a Cyclomatic Complexity of 15. The configured cyclomatic complexity threshold is 10. (undefined)
(CyclomaticComplexity)
120-183: The method preflight() has an NPath complexity of 2400. The configured NPath complexity threshold is 200. (undefined)
(NPathComplexity)
125-125: Missing class import via use statement (line '125', column '23'). (undefined)
(MissingImport)
130-130: Missing class import via use statement (line '130', column '23'). (undefined)
(MissingImport)
135-135: Avoid variables with short names like $s3. Configured minimum length is 3. (undefined)
(ShortVariable)
137-137: Missing class import via use statement (line '137', column '27'). (undefined)
(MissingImport)
143-143: Missing class import via use statement (line '143', column '27'). (undefined)
(MissingImport)
149-149: Missing class import via use statement (line '149', column '23'). (undefined)
(MissingImport)
155-155: Missing class import via use statement (line '155', column '23'). (undefined)
(MissingImport)
161-161: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'preflight'. (undefined)
(StaticAccess)
162-162: Missing class import via use statement (line '162', column '23'). (undefined)
(MissingImport)
165-165: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'preflight'. (undefined)
(StaticAccess)
166-166: Missing class import via use statement (line '166', column '23'). (undefined)
(MissingImport)
170-170: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'preflight'. (undefined)
(StaticAccess)
172-172: Missing class import via use statement (line '172', column '27'). (undefined)
(MissingImport)
175-180: The method preflight uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
176-176: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'preflight'. (undefined)
(StaticAccess)
202-202: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runInfoSidecar'. (undefined)
(StaticAccess)
208-208: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runInfoSidecar'. (undefined)
(StaticAccess)
224-224: Missing class import via use statement (line '224', column '23'). (undefined)
(MissingImport)
232-232: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runInfoSidecar'. (undefined)
(StaticAccess)
234-234: Missing class import via use statement (line '234', column '23'). (undefined)
(MissingImport)
246-246: Missing class import via use statement (line '246', column '23'). (undefined)
(MissingImport)
304-304: Missing class import via use statement (line '304', column '23'). (undefined)
(MissingImport)
320-320: Missing class import via use statement (line '320', column '27'). (undefined)
(MissingImport)
331-331: Missing class import via use statement (line '331', column '27'). (undefined)
(MissingImport)
336-336: Missing class import via use statement (line '336', column '23'). (undefined)
(MissingImport)
348-348: Missing class import via use statement (line '348', column '23'). (undefined)
(MissingImport)
368-368: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runRestoreSidecar'. (undefined)
(StaticAccess)
374-379: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runRestoreSidecar'. (undefined)
(StaticAccess)
407-407: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'getInstallAndRunCommand'. (undefined)
(StaticAccess)
app/Models/StandalonePostgresql.php
35-41: Avoid using static access to class 'App\Models\LocalPersistentVolume' in method 'booted'. (undefined)
(StaticAccess)
app/Actions/Database/StartPostgresql.php
345-345: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'add_custom_conf'. (undefined)
(StaticAccess)
355-357: The method add_custom_conf uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
380-380: The variable $content_base64 is not named in camelCase. (undefined)
(CamelCaseVariableName)
384-404: The method setup_pgbackrest_config is not named in camelCase. (undefined)
(CamelCaseMethodName)
386-386: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'setup_pgbackrest_config'. (undefined)
(StaticAccess)
406-457: The method create_pgbackrest_entrypoint is not named in camelCase. (undefined)
(CamelCaseMethodName)
408-408: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'create_pgbackrest_entrypoint'. (undefined)
(StaticAccess)
409-409: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'create_pgbackrest_entrypoint'. (undefined)
(StaticAccess)
414-414: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'create_pgbackrest_entrypoint'. (undefined)
(StaticAccess)
app/Http/Controllers/Api/DatabasesController.php
2874-2925: The method restore_database is not named in camelCase. (undefined)
(CamelCaseMethodName)
2919-2919: Avoid using static access to class '\App\Actions\Database\PgBackrestRestore' in method 'restore_database'. (undefined)
(StaticAccess)
2964-2989: The method list_restores is not named in camelCase. (undefined)
(CamelCaseMethodName)
3037-3068: The method restore_status is not named in camelCase. (undefined)
(CamelCaseMethodName)
app/Notifications/Database/DatabaseRestoreSuccess.php
60-60: Avoid using static access to class '\App\Notifications\Dto\DiscordMessage' in method 'toDiscord'. (undefined)
(StaticAccess)
111-111: Avoid using static access to class '\App\Notifications\Dto\SlackMessage' in method 'toSlack'. (undefined)
(StaticAccess)
app/Livewire/Project/Database/BackupExecutions.php
50-50: Avoid excessively long variable names like $showRestoreProgressModal. Keep variable name length under 20. (undefined)
(LongVariable)
129-129: Avoid using static access to class '\App\Models\ScheduledDatabaseBackupExecution' in method 'confirmRestore'. (undefined)
(StaticAccess)
146-146: Avoid unused parameters such as '$selectedActions'. (undefined)
(UnusedFormalParameter)
148-148: Avoid using static access to class '\App\Models\InstanceSettings' in method 'startRestore'. (undefined)
(StaticAccess)
149-149: Avoid using static access to class '\Illuminate\Support\Facades\Hash' in method 'startRestore'. (undefined)
(StaticAccess)
156-156: Avoid using static access to class '\App\Models\ScheduledDatabaseBackupExecution' in method 'startRestore'. (undefined)
(StaticAccess)
173-173: Avoid using static access to class '\App\Actions\Database\PgBackrestRestore' in method 'startRestore'. (undefined)
(StaticAccess)
app/Livewire/Project/Database/BackupEdit.php
93-93: Avoid excessively long variable names like $pgbackrestCompressType. Keep variable name length under 20. (undefined)
(LongVariable)
96-96: Avoid excessively long variable names like $pgbackrestCompressLevel. Keep variable name length under 20. (undefined)
(LongVariable)
102-102: Avoid excessively long variable names like $pgbackrestArchiveMode. Keep variable name length under 20. (undefined)
(LongVariable)
105-105: Avoid excessively long variable names like $localRepoRetentionFullType. Keep variable name length under 20. (undefined)
(LongVariable)
108-108: Avoid excessively long variable names like $localRepoRetentionFull. Keep variable name length under 20. (undefined)
(LongVariable)
111-111: Avoid excessively long variable names like $localRepoRetentionDiff. Keep variable name length under 20. (undefined)
(LongVariable)
117-117: Avoid excessively long variable names like $s3RepoRetentionFullType. Keep variable name length under 20. (undefined)
(LongVariable)
155-185: The method syncData uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
174-174: Missing class import via use statement (line '174', column '39'). (undefined)
(MissingImport)
190-220: The method syncData uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
211-214: The method syncData uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
245-302: The method syncPgbackrestRepos() has a Cyclomatic Complexity of 11. The configured cyclomatic complexity threshold is 10. (undefined)
(CyclomaticComplexity)
251-251: Missing class import via use statement (line '251', column '23'). (undefined)
(MissingImport)
258-258: Missing class import via use statement (line '258', column '23'). (undefined)
(MissingImport)
277-279: The method syncPgbackrestRepos uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
283-283: Missing class import via use statement (line '283', column '27'). (undefined)
(MissingImport)
299-301: The method syncPgbackrestRepos uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
app/Notifications/Database/DatabaseRestoreFailed.php
60-60: Avoid using static access to class '\App\Notifications\Dto\DiscordMessage' in method 'toDiscord'. (undefined)
(StaticAccess)
109-109: Avoid using static access to class '\App\Notifications\Dto\SlackMessage' in method 'toSlack'. (undefined)
(StaticAccess)
app/Jobs/DatabaseBackupJob.php
693-826: The method run_pgbackrest_backup() has a Cyclomatic Complexity of 22. The configured cyclomatic complexity threshold is 10. (undefined)
(CyclomaticComplexity)
693-826: The method run_pgbackrest_backup() has an NPath complexity of 27680. The configured NPath complexity threshold is 200. (undefined)
(NPathComplexity)
693-826: The method run_pgbackrest_backup() has 134 lines of code. Current threshold is set to 100. Avoid really long methods. (undefined)
(ExcessiveMethodLength)
693-826: The method run_pgbackrest_backup is not named in camelCase. (undefined)
(CamelCaseMethodName)
695-695: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
705-705: Missing class import via use statement (line '705', column '27'). (undefined)
(MissingImport)
709-717: Avoid using static access to class '\App\Models\ScheduledDatabaseBackupExecution' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
722-722: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
724-724: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
732-734: The method run_pgbackrest_backup uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
743-745: The method run_pgbackrest_backup uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
749-749: Missing class import via use statement (line '749', column '27'). (undefined)
(MissingImport)
752-752: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
768-777: The method run_pgbackrest_backup uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
779-779: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
780-780: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
783-783: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
784-784: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
798-798: Avoid using static access to class '\Carbon\Carbon' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
812-812: Avoid using static access to class '\Carbon\Carbon' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
824-824: Avoid using static access to class '\App\Events\BackupCreated' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
828-868: The method run_pgbackrest_expire is not named in camelCase. (undefined)
(CamelCaseMethodName)
834-834: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_expire'. (undefined)
(StaticAccess)
836-836: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_expire'. (undefined)
(StaticAccess)
851-860: The method run_pgbackrest_expire uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
863-866: Avoid using static access to class '\Illuminate\Support\Facades\Log' in method 'run_pgbackrest_expire'. (undefined)
(StaticAccess)
870-884: The method update_pgbackrest_config is not named in camelCase. (undefined)
(CamelCaseMethodName)
872-872: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'update_pgbackrest_config'. (undefined)
(StaticAccess)
875-875: Missing class import via use statement (line '875', column '23'). (undefined)
(MissingImport)
app/Services/Backup/PgBackrestService.php
9-353: The class PgBackrestService has 14 public methods. Consider refactoring PgBackrestService to keep number of public methods under 10. (undefined)
(TooManyPublicMethods)
9-353: The class PgBackrestService has an overall complexity of 59 which is very high. The configured complexity threshold is 50. (undefined)
(ExcessiveClassComplexity)
83-83: Avoid variables with short names like $s3. Configured minimum length is 3. (undefined)
(ShortVariable)
92-92: Missing class import via use statement (line '92', column '27'). (undefined)
(MissingImport)
103-109: The method generateRepoConfig uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
119-119: Avoid variables with short names like $k. Configured minimum length is 3. (undefined)
(ShortVariable)
119-119: Avoid variables with short names like $v. Configured minimum length is 3. (undefined)
(ShortVariable)
170-170: Avoid variables with short names like $s3. Configured minimum length is 3. (undefined)
(ShortVariable)
186-186: Avoid variables with short names like $s3. Configured minimum length is 3. (undefined)
(ShortVariable)
246-246: The method buildInfoCommand has a boolean flag argument $json, which is a certain sign of a Single Responsibility Principle violation. (undefined)
(BooleanArgumentFlag)
🔇 Additional comments (40)
bootstrap/helpers/shared.php (1)
3311-3326: LGTM! I'll be back... to use this function.This path conversion helper is clean and solves the dev environment volume mounting discrepancy elegantly. The PHPDoc clearly explains the "why" behind this SSH-target-to-Docker-host translation. Self-hosting infrastructure requires these kinds of pragmatic solutions - no serverless magic here, just good old-fashioned path manipulation.
app/Actions/Database/StartPostgresql.php (4)
8-8: LGTM! PgBackrest integration initialized.The new
PgBackrestServiceimport andhasPgBackrestflag are clean additions. Good encapsulation of the pgBackrest state within this action class.Also applies to: 26-26
459-465: Come with me if you want to live... in a properly configured dev environment.The
getHostPath()wrapper is a clean abstraction overconvertPathToDockerHost(). This keeps the path translation logic DRY and makes the class easier to test if needed.
214-228: Add error handling to pgBackrest installation in container entrypoint.The entrypoint override mounts pgBackrest config and runs the install script before PostgreSQL starts. The chaining with
exec docker-entrypoint.sh "$@"preserves the original entrypoint behavior.However, if
/etc/pgbackrest/install-pgbackrest.shfails, the container startup will fail hard with no fallback. Consider adding error handling to either make failures explicit for debugging or implement a recovery strategy.
384-404: Fix PSR-12 method naming violations and verify bash script compatibility across PostgreSQL container images.The new pgBackrest setup methods violate PSR-12 naming conventions. Private methods must use camelCase, not snake_case:
setup_pgbackrest_config→ should besetupPgbackrestConfigcreate_pgbackrest_entrypoint→ should becreatePgbackrestEntrypointThe bash script in
create_pgbackrest_entrypoint()(lines 421-452) has potential runtime issues:
Line 438:
chown -R postgres:postgres /etc/pgbackrestmay fail on containers where the script user lacks privileges. While|| truesuppresses the error, the operation silently fails.Lines 446-450:
su postgres -c "pgbackrest..."assumessuis available and properly configured in the container. Minimal PostgreSQL images may not havesuinstalled or might require alternative user-switching approaches.Rename the methods to comply with PSR-12 and test the bash script against common minimal PostgreSQL base images to ensure portability.
bootstrap/helpers/databases.php (1)
245-247: I'll be back... but pgBackrest backups won't be deleted by this function.Perfect guard clause. PgBackrest manages its own backup lifecycle, so bypassing the native deletion logic is exactly right. This prevents the two backup systems from stepping on each other's toes.
The early return is clean and the
isPgBackrest()method name is self-documenting.resources/views/emails/database-restore-success.blade.php (1)
1-9: I'm a cybernetic organism. Living tissue over a metal endoskeleton. This email template is living markup over a data structure.The restore success email template is straightforward and does exactly what it needs to do. The conditional rendering of
$labeland$target_timehandles optional data gracefully.Minor style note: Blade's
@issetdirective could make the intent even clearer, but@ifworks fine here.resources/views/emails/database-restore-failed.blade.php (1)
1-8: Your database restore was terminated. By an error.The failure email template correctly includes the error message and optional backup label. The structure mirrors the success template, which is good for consistency.
One suggestion: Consider truncating very long error messages for email readability, though that might be better handled in the notification class itself.
app/Notifications/Database/BackupSuccess.php (1)
18-20: Your backup succeeded. I'll send notifications to all channels. It's in my nature to protect you.The additions cleanly integrate pgBackrest engine metadata into the notification:
- Lines 18-20: New properties
engineandbackupTypeare properly typed (string and nullable string)- Lines 28-29: Initialization uses sensible defaults ('native' for engine, conditional 'full' for pgBackrest)
- Lines 99-115: Webhook payload construction conditionally includes
backup_typeonly when setThis mirrors the pattern in
BackupFailed, which is excellent for consistency. The webhook consumers will now know whether they're dealing with native or pgBackrest backups, and get backup type details when relevant.Also applies to: 28-29, 99-115
app/Notifications/Database/BackupFailed.php (1)
18-20: Come with me if you want to receive properly structured failure notifications.The failure notification follows the exact same pattern as
BackupSuccessfor engine and backup type handling. This consistency is crucial for webhook consumers and multi-channel notification systems.The conditional inclusion of
backup_typein the webhook payload (lines 115-117) ensures backward compatibility while enriching the data when pgBackrest is in use. No serverless webhooks needed - these notifications work perfectly with self-hosted infrastructure!Also applies to: 27-28, 102-119
database/migrations/2025_12_09_231050_create_database_restores_table.php (1)
11-36: I need your database schema. Your indexes. And your restore tracking table.The
database_restorestable schema is well-designed:
- Lines 13-16: UUID and polymorphic
databaserelation properly set up for supporting multiple database types- Lines 19-22: Foreign key to
scheduled_database_backup_executionswithnullOnDeletecorrectly handles backup cleanup without orphaning restores- Line 25: Default engine
'pgbackrest'is appropriate since this table is specifically for the new pgBackrest restore workflow- Lines 26-27: Nullable
target_labelandtarget_timeallow point-in-time restore flexibility- Lines 30-35: Status tracking with
message,log, andfinished_atprovides full restore lifecycle observabilityThe migration follows Laravel best practices and aligns with the learnings about Eloquent ORM patterns. The polymorphic relationship will work well with the
DatabaseRestoremodel introduced in this PR.Based on learnings, database schema follows documented patterns for relationships and indexing.
routes/api.php (1)
139-142: Hasta la vista, poorly designed APIs! These routes look solid.The new restore endpoints follow existing patterns, use appropriate HTTP verbs (POST for initiating action, GET for reads), and are properly secured with
api.abilitymiddleware. Consistent with the rest of the database routes — like a well-oiled T-800.One minor suggestion for the future: consider adding route names (e.g.,
->name('databases.restore')) for easier reference elsewhere in the codebase. But not a blocker — your servers will be just fine.database/migrations/2025_12_09_231049_add_pgbackrest_fields_to_scheduled_database_backup_executions_table.php (2)
57-81: Down migration is properly ordered — termination sequence approved.Dropping
pgbackrest_repostable first before removing columns from other tables is the correct order to avoid foreign key constraint violations. Clean rollback like a clean kill shot. 🌮
9-55: I cannot complete the verification of this review comment due to persistent repository access failures. The review requests verification that the corresponding models (ScheduledDatabaseBackup,ScheduledDatabaseBackupExecution, andPgbackrestRepo) have their$fillablearrays updated with the new columns introduced in the migration. Without access to the codebase, I cannot:
- Locate or inspect the model files
- Verify that
$fillablearrays include all new columns (engine, pgbackrest_backup_type, pgbackrest_label, pgbackrest_stanza, pgbackrest_repo_size, etc.)- Confirm mass assignment protection is properly configured
- Validate the models follow Eloquent ORM patterns as per coding guidelines
app/Actions/Database/PgBackrestRestore.php (2)
31-44: Database restoration creation — self-hosted recovery FTW!The restore record creation and job dispatch pattern is solid. Using the morphClass for polymorphic relationships is correct. Job dispatch follows Laravel conventions.
Static access to Eloquent models and job dispatching is idiomatic Laravel — ignore PHPMD's concerns here. That tool is more confused than a serverless function trying to maintain state.
47-52: Validate at the action boundary to ensure rules are enforced for all callers.The
rules()method won't automatically validate unless this action is invoked as a controller action or FormRequest. Per Laravel best practices, validation should occur at the application boundary. Ensure validation is explicitly enforced beforehandle()executes, or move the validation logic insidehandle()to guarantee it runs regardless of how the action is called.app/Models/ScheduledDatabaseBackupExecution.php (3)
27-30: Relationship definition is correct — like a proper neural network connection.The
restores()HasMany relationship correctly links toDatabaseRestorevia the appropriate foreign key. This matches the inverseBelongsTorelationship inDatabaseRestore::execution()shown in the relevant snippets. Proper bidirectional relationships make querying efficient — unlike serverless cold starts.
32-47: Predicate methods are solid and consistent.The
isPgBackrest()andisNative()methods mirror the same logic inScheduledDatabaseBackupmodel (per relevant snippets). ThecanRestore()method is well-designed with clear conditions.One observation:
canRestore()currently only returnstruefor pgBackRest executions. If native backups should also support restoration in the future, this method would need updating. But for now, targeting only pgBackRest restores makes sense given the PR scope.
12-20: Cast addition approved — integers should be integers.Adding
'pgbackrest_repo_size' => 'integer'ensures proper type handling for the repository size field. No more string-to-int shenanigans. Clean data types are like a clean conscience — essential for a well-functioning machine.resources/views/livewire/project/database/backup-edit.blade.php (1)
57-185: Verify whether custom authorization attributes exist before requiring them.The review comment references
canGateandcanResourceattributes as required for form components. However, these are not standard Laravel helpers, and it cannot be confirmed whether the customx-forms.*components in this project actually support these attributes without examining the component implementations and similar forms in the codebase. If these attributes don't exist, the suggestion is invalid.app/Models/DatabaseRestore.php (1)
8-18: LGTM - Model structure is solid like a T-800 endoskeleton.The model setup with
$guarded = []and datetime casts is appropriate for this use case. The relationships are correctly defined using MorphTo and BelongsTo. Self-hosted databases deserve reliable restore tracking. No serverless nonsense here - just good old-fashioned Eloquent models running on real servers. 🌮app/Models/StandalonePostgresql.php (2)
346-376: LGTM - These helper methods are efficient like a well-oiled killing machine.The pgBackRest helper methods (
hasPgBackrestBackups,pgbackrestBackups,restores,pgdataVolume,pgbackrestRepoVolume) are clean and follow existing model patterns. The use ofexists()inhasPgBackrestBackups()is efficient - no unnecessary data loading. The polymorphicrestores()relationship correctly mirrors the DatabaseRestore model'sdatabase()MorphTo.Self-hosted PostgreSQL with proper backup support? That's the way to run databases. No serverless database-as-a-service here - just real servers with real backups. 🌮
34-41: Clarify the design decision for pgBackRest volume creation.The code creates a pgBackRest repository volume unconditionally for every PostgreSQL database. Confirm whether this proactive approach is intentional for all users or if volumes should be created on-demand when pgBackRest is first enabled. If volumes are created upfront, document the approach and verify there's a cleanup strategy for users who don't utilize pgBackRest backups.
app/Models/PgbackrestRepo.php (1)
8-72: LGTM - Repository configuration model is terminated... I mean, well-structured.The model correctly implements pgBackRest repository configuration with proper relationships and helper methods. The use of Cuid2 for UUID generation, safe navigation operators in
getDefaultPath(), and the clear type predicates (isLocal(),isS3()) all demonstrate solid design.The
getRepoKey()method correctly follows pgBackRest's naming convention (repo1, repo2, etc.). ThegetEffectivePath()pattern with fallback to default is a clean approach.Self-hosting with pgBackRest for incremental backups? That's how you protect your data on real servers. Much better than trusting some VC-backed serverless provider with your precious databases. Now I want a taco. 🌮
app/Notifications/Database/DatabaseRestoreFailed.php (2)
113-128: LGTM - Webhook payload is comprehensive, like a detailed mission report.The webhook payload includes all necessary fields for external integrations: success flag, event type, database identifiers, engine type, backup label, error details, and a URL. The URL construction using
data_get()with safe navigation through the environment and project relationships is defensive and correct.Hardcoding
'engine' => 'pgbackrest'is appropriate since only pgBackRest supports restore operations. External systems will appreciate the structured data. 🌮
32-35: Verify channel name for restore notification.The
via()method uses'backup_failed'channel for a restore failure notification. Confirm whether this is intentional (grouping restore and backup notification preferences) or if a separate'restore_failed'channel should be used. If intentional, add a comment explaining this design choice.resources/views/livewire/project/database/backup-executions.blade.php (2)
69-77: LGTM - pgBackRest UI indicators are clear and informative.The pgBackRest badge with backup type display (Full/Incr/Diff) and the "pgBackRest Repository" storage indicator provide clear visual feedback for users. The conditional rendering based on
engine === 'pgbackrest'cleanly separates the pgBackRest flow from native backups.Self-hosted backups with a proper UI to monitor them? This is the way. No obscure serverless console here - just transparent backup management on your own servers. Hasta la vista, data loss! 🌮
Also applies to: 109-121
322-429: LGTM - Restore progress modal with automatic polling.The restore progress modal is well-designed with:
- Conditional polling (
wire:poll.2000ms) only when restore is running/pending- Clear status icons and colors for different states
- Scrollable log output for debugging
- Appropriate action buttons based on state
The polling stops automatically when restore finishes via the
@if (!$currentRestore->isFinished())condition. Efficient like a T-1000 adapting to the situation. 🌮app/Livewire/Project/Database/BackupExecutions.php (1)
182-200: Restore polling/progress flow is straightforward and safe
pollRestoreStatus()andcloseRestoreProgress()do the right small thing:
- Refresh the
DatabaseRestoremodel.- When finished, reload executions and clear UI state.
No racey stuff here; this is a clean, minimal Livewire loop. Ship it.
app/Models/ScheduledDatabaseBackup.php (1)
13-22: New casts for pgBackRest flags look correctThe
casts()override adding:'enabled' => 'boolean', 'save_s3' => 'boolean', 'dump_all' => 'boolean', 'disable_local_backup' => 'boolean', 'pgbackrest_compress_level' => 'integer',lines up with how these flags are used in the jobs and UI. No behavioral traps here; the metal brain approves.
app/Jobs/PgBackrestRestoreJob.php (2)
120-183: Preflight checks are thorough and aligned with pgBackRest expectations
preflight()does a nice job of failing early:
- Verifies the DB actually has pgBackRest backups configured and enabled.
- Confirms S3 repos are wired correctly and testable.
- Ensures PGDATA and (if required) repo volumes exist.
- Uses
runInfoSidecar()+PgBackrestServicehelpers to validate stanza and backup presence.- Validates a specific
pgbackrest_labelwhen provided.From a correctness and safety standpoint this is the right amount of paranoia before you start nuking PGDATA. No changes required here; just keep this function as the grumpy bouncer in front of the restore club.
340-398: Sidecar restore flow looks correct and matches pgBackRest usageThe
runRestoreSidecar()method wires things up as expected:
- Mounts PGDATA and, if present, the pgBackRest repo volume.
- Mounts the generated config directory read‑only.
- Sets
PGBACKREST_PG1_PATHand S3 env vars.- Builds the restore command via
PgBackrestService::buildRestoreCommand(...).- Uses the same sidecar image as the DB (
getSidecarImage()), with an install‑and‑run wrapper.chown -R 999:999on PGDATA afterwards to hand ownership back to postgres.- Runs with
throwOnError = true, so a non‑zero exit aborts the restore.This is a solid, self‑hosted restore dance: one small container, lots of volume mounts, zero serverless hype.
app/Notifications/Database/DatabaseRestoreSuccess.php (3)
115-130: URL building that would make a T-800 proud.The webhook payload URL construction is solid. Self-hosted servers handling their own backups - much better than serverless functions disappearing like tacos at a party.
One small observation: the hardcoded
'engine' => 'pgbackrest'is appropriate since this notification is specifically for pgBackRest restores.
37-48: Ensure the email viewemails.database-restore-successexists inresources/views/emails/.The
toMail()method references viewemails.database-restore-success. This view file must exist at the expected location for the notification to render properly.
32-35: Clarify the notification channel naming: does 'backup_success' intentionally consolidate restore and backup notifications, or should this use a dedicated 'restore_success' channel?The
via()method uses'backup_success'channel for a restore notification. Per Laravel best practices, channel names should be explicit and match their purpose for consistency and clarity. If restore notifications should use a separate channel, consider renaming to'restore_success'or documenting why the'backup_success'channel is shared between backup and restore operations.app/Livewire/Project/Database/BackupEdit.php (1)
403-406: isPostgresql() looking good - like a perfectly grilled taco (gluten-free tortilla, obviously).Clean helper method. The full class name comparison is the correct approach here since
database_typestores the morph class name.app/Services/Backup/PgBackrestService.php (4)
23-26: Stanza naming using UUID - solid choice, like choosing servers over serverless.Using the database UUID as the stanza name is a clean, unique identifier approach. No collision issues, no special character concerns.
77-120: Config generation with S3 validation - I approve this message.Good defensive coding with
validateShellSafePath()for S3 bucket and endpoint validation. The repo configuration generation handles both S3 and local (posix) storage types correctly.One note: using the imported
Exceptionclass would be more consistent (line 92).
130-151: Multi-package-manager install command - supporting all the self-hosted servers out there!This is beautiful. Supporting apk, apt-get, and yum means users can run pgBackRest on Alpine, Debian/Ubuntu, and RHEL-based containers. Much better than being locked into one cloud provider's serverless offering. Like a taco that comes with multiple sauce options - everyone wins (except people with gluten intolerance eating regular tortillas).
283-292: JSON parsing - clean and simple, like a good taco recipe.Straightforward JSON parsing with proper error handling. Returns null on parse failure rather than throwing - a reasonable choice for this utility method.
resources/views/livewire/project/database/backup-edit.blade.php
Outdated
Show resolved
Hide resolved
resources/views/livewire/project/database/backup-executions.blade.php
Outdated
Show resolved
Hide resolved
SkyfallWasTaken
force-pushed
the
pgbackrest
branch
from
3d64437 to
7ba5142
Compare
CodeRabbit fixes CodeRabbit fixes Handle case where PGDATA was already empty S3 fix Squash migrations
SkyfallWasTaken
force-pushed
the
pgbackrest
branch
from
7ba5142 to
dd73b3c
Compare
|
@CodeRabbit review Alright, let's see if we're good to go now? |
|
@SkyfallWasTaken, I'll be back... with a comprehensive review! 🤖💪 Like a good self-hosted backup system running on real servers (not that serverless marketing fluff), I'm going to thoroughly analyze this pgBackRest implementation. This looks like a substantial PR with lots of incremental improvements - much like pgBackRest's incremental backups themselves! Let me review all these changes to make sure your backup restoration is as reliable as a T-800's metal endoskeleton. I'll check the action flows, job orchestration, API endpoints, and make sure everything is properly wired up. Hasta la vista, bugs! 🌮 ✅ Actions performedReview triggered.
|
|
One question that I’m wondering about. From my understanding, all the ‘default’ Postgres images work fine with PgBackRest as is. However, if you:
Backups will fail because the packaged version of PgBackRest in the Alpine repositories doesn’t support Postgres 18. I think a GH Actions-based auto-builder for PgBackRest would work great here - is making a separate repo to build PGBR for both musl & glibc something that you guys would be interested in? (I can also make this on a personal repo if you want and are fine with Coolify downloading from my repo’s releases) Update: I asked this on the Discord server, apparently this shouldn't be a blocker since Coolify doesn't support PG18 anyway + the responsibility lies on the Alpine maintainers |
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (1)
app/Http/Controllers/Api/DatabasesController.php (1)
3008-3013: Consider using a resource/transformer for public API consistency.Previous review noted that returning raw
DatabaseRestoremodels may expose internal fields like IDs and verbose logs. While acceptable for internal use, if this becomes a public API, consider mapping to a stable response shape.$restores = DatabaseRestore::where('database_id', $database->id) ->where('database_type', $database->getMorphClass()) ->orderBy('created_at', 'desc') ->get() ->map(fn ($r) => [ 'uuid' => $r->uuid, 'engine' => $r->engine, 'status' => $r->status, 'target_label' => $r->target_label, 'message' => $r->message, 'created_at' => $r->created_at?->toIso8601String(), 'finished_at' => $r->finished_at?->toIso8601String(), ]);Not blocking - your servers don't need to be perfect on day one. Unlike serverless, you can actually iterate on your own infrastructure.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
app/Actions/Database/PgBackrestRestore.php(1 hunks)app/Actions/Database/StartPostgresql.php(8 hunks)app/Http/Controllers/Api/DatabasesController.php(14 hunks)app/Jobs/PgBackrestRestoreJob.php(1 hunks)app/Livewire/Project/Database/BackupEdit.php(7 hunks)app/Livewire/Project/Database/BackupExecutions.php(4 hunks)app/Models/DatabaseRestore.php(1 hunks)app/Models/ScheduledDatabaseBackup.php(1 hunks)app/Services/Backup/PgBackrestService.php(1 hunks)database/migrations/2025_12_09_231049_add_pgbackrest_support.php(1 hunks)resources/views/livewire/project/database/backup-edit.blade.php(1 hunks)resources/views/livewire/project/database/backup-executions.blade.php(5 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
**/*.php
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
Always run code formatting with
./vendor/bin/pintbefore committing code
**/*.php: Follow PSR-12 coding standards. Use Laravel Pint for automatic formatting. Write descriptive variable and method names. Keep methods small and focused. Document complex logic with clear comments
Use PHP 8.4 constructor property promotion and typed properties
Never useenv()outside config files in Laravel
Files:
resources/views/livewire/project/database/backup-edit.blade.phpresources/views/livewire/project/database/backup-executions.blade.phpapp/Jobs/PgBackrestRestoreJob.phpapp/Actions/Database/PgBackrestRestore.phpapp/Actions/Database/StartPostgresql.phpdatabase/migrations/2025_12_09_231049_add_pgbackrest_support.phpapp/Livewire/Project/Database/BackupEdit.phpapp/Livewire/Project/Database/BackupExecutions.phpapp/Http/Controllers/Api/DatabasesController.phpapp/Models/DatabaseRestore.phpapp/Services/Backup/PgBackrestService.phpapp/Models/ScheduledDatabaseBackup.php
**/**/livewire/**/*.blade.php
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
Livewire components MUST have exactly ONE root element with no exceptions
Files:
resources/views/livewire/project/database/backup-edit.blade.phpresources/views/livewire/project/database/backup-executions.blade.php
**/*.blade.php
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
**/*.blade.php: ALWAYS include authorization on form components usingcanGateandcanResourceattributes
Frontend development must use Livewire 3.5.20 for server-side state, Alpine.js for client interactions, and Tailwind CSS 4.1.4 for styling
Files:
resources/views/livewire/project/database/backup-edit.blade.phpresources/views/livewire/project/database/backup-executions.blade.php
resources/views/livewire/**/*.blade.php
📄 CodeRabbit inference engine (CLAUDE.md)
resources/views/livewire/**/*.blade.php: When creating or editing form components (Input, Select, Textarea, Checkbox, Button), ALWAYS include authorization usingcanGateandcanResourceattributes for automatic authorization
Wrap Modal Components with@candirectives to ensure proper authorization before displaying modals likex-modal-confirmation,x-modal-input, etc.
Livewire component views MUST have exactly ONE root element. ALL content must be contained within this single root element. Placing ANY elements (<style>, <script>,, comments, or other HTML) outside the root will break Livewire's component tracking and cause wire:click and other directives to fail silently
Usewire:model.livefor real-time updates in Livewire components
Files:
resources/views/livewire/project/database/backup-edit.blade.phpresources/views/livewire/project/database/backup-executions.blade.php
**/*.{php,blade.php}
📄 CodeRabbit inference engine (CLAUDE.md)
Use named routes with
route()function instead of hardcoded URLs
Files:
resources/views/livewire/project/database/backup-edit.blade.phpresources/views/livewire/project/database/backup-executions.blade.phpapp/Jobs/PgBackrestRestoreJob.phpapp/Actions/Database/PgBackrestRestore.phpapp/Actions/Database/StartPostgresql.phpdatabase/migrations/2025_12_09_231049_add_pgbackrest_support.phpapp/Livewire/Project/Database/BackupEdit.phpapp/Livewire/Project/Database/BackupExecutions.phpapp/Http/Controllers/Api/DatabasesController.phpapp/Models/DatabaseRestore.phpapp/Services/Backup/PgBackrestService.phpapp/Models/ScheduledDatabaseBackup.php
resources/views/**/*.blade.php
📄 CodeRabbit inference engine (CLAUDE.md)
Tailwind CSS: Use new utilities (version 4.1.4), not deprecated ones. Use
gaputilities for spacing, not margins
Files:
resources/views/livewire/project/database/backup-edit.blade.phpresources/views/livewire/project/database/backup-executions.blade.php
app/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
app/**/*.php: Use eager loading to prevent N+1 queries, implement caching for frequently accessed data, queue heavy operations, optimize database queries with proper indexes, use chunking for large data operations
UseownedByCurrentTeamCached()instead ofownedByCurrentTeam()->get()for team-scoped queries to avoid duplicate database queries
Queue heavy operations with Laravel Horizon
Files:
app/Jobs/PgBackrestRestoreJob.phpapp/Actions/Database/PgBackrestRestore.phpapp/Actions/Database/StartPostgresql.phpapp/Livewire/Project/Database/BackupEdit.phpapp/Livewire/Project/Database/BackupExecutions.phpapp/Http/Controllers/Api/DatabasesController.phpapp/Models/DatabaseRestore.phpapp/Services/Backup/PgBackrestService.phpapp/Models/ScheduledDatabaseBackup.php
{**/*Model.php,database/migrations/**/*.php}
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
Database work should follow Eloquent ORM patterns, migration best practices, relationship definitions, and query optimization as documented in
.ai/patterns/database-patterns.md
Files:
database/migrations/2025_12_09_231049_add_pgbackrest_support.php
app/Livewire/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
In Livewire Components, always add the
AuthorizesRequeststrait and check permissions with$this->authorize()calls in mount() and action methods
Files:
app/Livewire/Project/Database/BackupEdit.phpapp/Livewire/Project/Database/BackupExecutions.php
{**/*Policy.php,**/*Gate.php,app/Models/**/*.php,routes/**/*.php}
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
Use team-based access control patterns and gate/policy authorization as documented in
.ai/patterns/security-patterns.md
Files:
app/Models/DatabaseRestore.phpapp/Models/ScheduledDatabaseBackup.php
app/Models/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
app/Models/**/*.php: When adding new database columns, ALWAYS update the model's$fillablearray to allow mass assignment
Use Eloquent ORM for database interactions, implement relationships properly (HasMany, BelongsTo, etc.), use database transactions for critical operations, leverage query scopes for reusable queries, and apply indexes for performance-critical queries
Always use team() method to return relationship instance, not direct property access. App\Models\Application::team must return a relationship instance
Files:
app/Models/DatabaseRestore.phpapp/Models/ScheduledDatabaseBackup.php
🧠 Learnings (11)
📚 Learning: 2025-12-08T20:23:33.775Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T20:23:33.775Z
Learning: Applies to resources/views/livewire/**/*.blade.php : Use `wire:model.live` for real-time updates in Livewire components
Applied to files:
resources/views/livewire/project/database/backup-edit.blade.php
📚 Learning: 2025-12-08T20:23:33.775Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T20:23:33.775Z
Learning: Applies to resources/views/livewire/**/*.blade.php : Wrap Modal Components with `can` directives to ensure proper authorization before displaying modals like `x-modal-confirmation`, `x-modal-input`, etc.
Applied to files:
resources/views/livewire/project/database/backup-executions.blade.php
📚 Learning: 2025-12-08T20:23:33.775Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T20:23:33.775Z
Learning: Applies to resources/views/livewire/**/*.blade.php : When creating or editing form components (Input, Select, Textarea, Checkbox, Button), ALWAYS include authorization using `canGate` and `canResource` attributes for automatic authorization
Applied to files:
resources/views/livewire/project/database/backup-executions.blade.php
📚 Learning: 2025-11-25T09:32:48.519Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/coolify-ai-docs.mdc:0-0
Timestamp: 2025-11-25T09:32:48.519Z
Learning: Applies to **/*.blade.php : ALWAYS include authorization on form components using `canGate` and `canResource` attributes
Applied to files:
resources/views/livewire/project/database/backup-executions.blade.php
📚 Learning: 2025-12-10T01:53:52.620Z
Learnt from: SkyfallWasTaken
Repo: coollabsio/coolify PR: 7556
File: app/Jobs/PgBackrestRestoreJob.php:39-118
Timestamp: 2025-12-10T01:53:52.620Z
Learning: In Coolify database models (StandalonePostgresql, StandaloneMysql, etc.), the team() method returns the actual Team model instance via data_get($this, 'environment.project.team'), NOT a BelongsTo relation. Therefore, $database->team() can be used directly for operations like ->notify() without needing to access a property.
Applied to files:
app/Jobs/PgBackrestRestoreJob.php
📚 Learning: 2025-12-08T20:23:33.775Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T20:23:33.775Z
Learning: Applies to app/Models/**/*.php : Always use team() method to return relationship instance, not direct property access. App\Models\Application::team must return a relationship instance
Applied to files:
app/Jobs/PgBackrestRestoreJob.phpapp/Models/ScheduledDatabaseBackup.php
📚 Learning: 2025-12-08T20:23:33.775Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T20:23:33.775Z
Learning: Applies to app/**/*.php : Use `ownedByCurrentTeamCached()` instead of `ownedByCurrentTeam()->get()` for team-scoped queries to avoid duplicate database queries
Applied to files:
app/Jobs/PgBackrestRestoreJob.phpapp/Models/ScheduledDatabaseBackup.php
📚 Learning: 2025-11-25T09:32:48.519Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/coolify-ai-docs.mdc:0-0
Timestamp: 2025-11-25T09:32:48.519Z
Learning: Applies to {**/*Model.php,database/migrations/**/*.php} : Database work should follow Eloquent ORM patterns, migration best practices, relationship definitions, and query optimization as documented in `.ai/patterns/database-patterns.md`
Applied to files:
app/Jobs/PgBackrestRestoreJob.phpapp/Models/DatabaseRestore.php
📚 Learning: 2025-12-08T20:23:33.775Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T20:23:33.775Z
Learning: Applies to app/Models/**/*.php : Use Eloquent ORM for database interactions, implement relationships properly (HasMany, BelongsTo, etc.), use database transactions for critical operations, leverage query scopes for reusable queries, and apply indexes for performance-critical queries
Applied to files:
app/Jobs/PgBackrestRestoreJob.phpapp/Http/Controllers/Api/DatabasesController.phpapp/Models/DatabaseRestore.phpapp/Models/ScheduledDatabaseBackup.php
📚 Learning: 2025-11-25T09:32:48.519Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/coolify-ai-docs.mdc:0-0
Timestamp: 2025-11-25T09:32:48.519Z
Learning: Applies to {**/*Policy.php,**/*Gate.php,app/Models/**/*.php,routes/**/*.php} : Use team-based access control patterns and gate/policy authorization as documented in `.ai/patterns/security-patterns.md`
Applied to files:
app/Jobs/PgBackrestRestoreJob.php
📚 Learning: 2025-12-10T01:53:52.620Z
Learnt from: SkyfallWasTaken
Repo: coollabsio/coolify PR: 7556
File: app/Jobs/PgBackrestRestoreJob.php:39-118
Timestamp: 2025-12-10T01:53:52.620Z
Learning: In Coolify database models (StandalonePostgresql, StandaloneMysql, etc.), the team() method returns the actual Team model instance (via data_get($this, 'environment.project.team')), not a BelongsTo relation. Therefore, treat $database->team() as the model you can operate on (e.g., $database->team()->notify(...)) directly, without accessing a property. Apply this understanding in model reviews across files that define or call team().
Applied to files:
app/Models/DatabaseRestore.phpapp/Models/ScheduledDatabaseBackup.php
🧬 Code graph analysis (5)
app/Jobs/PgBackrestRestoreJob.php (5)
app/Actions/Database/StartPostgresql.php (3)
StartPostgresql(12-468)handle(28-255)getHostPath(464-467)app/Actions/Database/StopDatabase.php (1)
StopDatabase(17-56)app/Models/DatabaseRestore.php (4)
DatabaseRestore(8-80)database(20-23)updateStatus(65-79)appendLog(55-63)app/Services/Backup/PgBackrestService.php (11)
PgBackrestService(9-364)getStanzaName(23-26)stanzaExists(345-354)hasBackups(356-363)findBackupByLabel(320-333)getLatestBackup(305-318)buildS3EnvVars(163-178)buildInfoCommand(253-269)parseInfoJson(294-303)buildRestoreCommand(219-251)buildInstallAndSetupCommand(153-161)bootstrap/helpers/shared.php (1)
convertPathToDockerHost(3319-3326)
app/Actions/Database/PgBackrestRestore.php (3)
app/Jobs/PgBackrestRestoreJob.php (2)
PgBackrestRestoreJob(22-458)handle(39-120)app/Models/DatabaseRestore.php (3)
DatabaseRestore(8-80)database(20-23)execution(25-28)app/Models/ScheduledDatabaseBackupExecution.php (1)
ScheduledDatabaseBackupExecution(8-48)
app/Livewire/Project/Database/BackupExecutions.php (4)
app/Actions/Database/PgBackrestRestore.php (1)
PgBackrestRestore(12-43)app/Models/DatabaseRestore.php (4)
DatabaseRestore(8-80)execution(25-28)database(20-23)isFinished(50-53)app/Models/ScheduledDatabaseBackupExecution.php (2)
ScheduledDatabaseBackupExecution(8-48)canRestore(42-47)app/Models/StandalonePostgresql.php (1)
StandalonePostgresql(11-424)
app/Models/DatabaseRestore.php (1)
app/Models/ScheduledDatabaseBackupExecution.php (2)
casts(12-20)ScheduledDatabaseBackupExecution(8-48)
app/Models/ScheduledDatabaseBackup.php (4)
app/Models/DatabaseRestore.php (2)
casts(12-18)DatabaseRestore(8-80)app/Models/ScheduledDatabaseBackupExecution.php (5)
casts(12-20)isPgBackrest(32-35)isNative(37-40)restores(27-30)ScheduledDatabaseBackupExecution(8-48)app/Models/PgbackrestRepo.php (2)
casts(12-20)PgbackrestRepo(8-72)app/Models/StandalonePostgresql.php (1)
restores(359-362)
🪛 PHPMD (2.15.0)
app/Jobs/PgBackrestRestoreJob.php
22-458: The class PgBackrestRestoreJob has an overall complexity of 62 which is very high. The configured complexity threshold is 50. (undefined)
(ExcessiveClassComplexity)
41-41: Avoid unused local variables such as '$server'. (undefined)
(UnusedLocalVariable)
42-42: Avoid unused local variables such as '$containerName'. (undefined)
(UnusedLocalVariable)
43-43: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'handle'. (undefined)
(StaticAccess)
52-52: Avoid using static access to class '\App\Actions\Database\StopDatabase' in method 'handle'. (undefined)
(StaticAccess)
68-68: Avoid using static access to class '\App\Actions\Database\StartPostgresql' in method 'handle'. (undefined)
(StaticAccess)
93-98: Avoid using static access to class '\Illuminate\Support\Facades\Log' in method 'handle'. (undefined)
(StaticAccess)
113-113: Avoid using static access to class '\App\Actions\Database\StartPostgresql' in method 'handle'. (undefined)
(StaticAccess)
122-185: The method preflight() has a Cyclomatic Complexity of 15. The configured cyclomatic complexity threshold is 10. (undefined)
(CyclomaticComplexity)
122-185: The method preflight() has an NPath complexity of 2400. The configured NPath complexity threshold is 200. (undefined)
(NPathComplexity)
127-127: Missing class import via use statement (line '127', column '23'). (undefined)
(MissingImport)
132-132: Missing class import via use statement (line '132', column '23'). (undefined)
(MissingImport)
137-137: Avoid variables with short names like $s3. Configured minimum length is 3. (undefined)
(ShortVariable)
139-139: Missing class import via use statement (line '139', column '27'). (undefined)
(MissingImport)
145-145: Missing class import via use statement (line '145', column '27'). (undefined)
(MissingImport)
151-151: Missing class import via use statement (line '151', column '23'). (undefined)
(MissingImport)
157-157: Missing class import via use statement (line '157', column '23'). (undefined)
(MissingImport)
163-163: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'preflight'. (undefined)
(StaticAccess)
164-164: Missing class import via use statement (line '164', column '23'). (undefined)
(MissingImport)
167-167: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'preflight'. (undefined)
(StaticAccess)
168-168: Missing class import via use statement (line '168', column '23'). (undefined)
(MissingImport)
172-172: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'preflight'. (undefined)
(StaticAccess)
174-174: Missing class import via use statement (line '174', column '27'). (undefined)
(MissingImport)
177-182: The method preflight uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
178-178: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'preflight'. (undefined)
(StaticAccess)
204-204: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runInfoSidecar'. (undefined)
(StaticAccess)
210-210: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runInfoSidecar'. (undefined)
(StaticAccess)
226-226: Missing class import via use statement (line '226', column '23'). (undefined)
(MissingImport)
234-234: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runInfoSidecar'. (undefined)
(StaticAccess)
236-236: Missing class import via use statement (line '236', column '23'). (undefined)
(MissingImport)
248-248: Missing class import via use statement (line '248', column '23'). (undefined)
(MissingImport)
271-271: Missing class import via use statement (line '271', column '23'). (undefined)
(MissingImport)
323-323: Missing class import via use statement (line '323', column '23'). (undefined)
(MissingImport)
339-339: Missing class import via use statement (line '339', column '27'). (undefined)
(MissingImport)
350-350: Missing class import via use statement (line '350', column '27'). (undefined)
(MissingImport)
355-355: Missing class import via use statement (line '355', column '23'). (undefined)
(MissingImport)
367-367: Missing class import via use statement (line '367', column '23'). (undefined)
(MissingImport)
387-387: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runRestoreSidecar'. (undefined)
(StaticAccess)
393-398: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runRestoreSidecar'. (undefined)
(StaticAccess)
426-426: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'getInstallAndRunCommand'. (undefined)
(StaticAccess)
app/Actions/Database/PgBackrestRestore.php
21-30: Avoid using static access to class '\App\Models\DatabaseRestore' in method 'handle'. (undefined)
(StaticAccess)
32-32: Avoid using static access to class '\App\Jobs\PgBackrestRestoreJob' in method 'handle'. (undefined)
(StaticAccess)
app/Actions/Database/StartPostgresql.php
343-345: The method add_custom_conf uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
349-349: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'add_custom_conf'. (undefined)
(StaticAccess)
382-382: The variable $content_base64 is not named in camelCase. (undefined)
(CamelCaseVariableName)
386-406: The method setup_pgbackrest_config is not named in camelCase. (undefined)
(CamelCaseMethodName)
388-388: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'setup_pgbackrest_config'. (undefined)
(StaticAccess)
408-459: The method create_pgbackrest_entrypoint is not named in camelCase. (undefined)
(CamelCaseMethodName)
410-410: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'create_pgbackrest_entrypoint'. (undefined)
(StaticAccess)
411-411: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'create_pgbackrest_entrypoint'. (undefined)
(StaticAccess)
416-416: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'create_pgbackrest_entrypoint'. (undefined)
(StaticAccess)
app/Livewire/Project/Database/BackupEdit.php
93-93: Avoid excessively long variable names like $pgbackrestCompressType. Keep variable name length under 20. (undefined)
(LongVariable)
96-96: Avoid excessively long variable names like $pgbackrestCompressLevel. Keep variable name length under 20. (undefined)
(LongVariable)
102-102: Avoid excessively long variable names like $pgbackrestArchiveMode. Keep variable name length under 20. (undefined)
(LongVariable)
105-105: Avoid excessively long variable names like $localRepoRetentionFullType. Keep variable name length under 20. (undefined)
(LongVariable)
108-108: Avoid excessively long variable names like $localRepoRetentionFull. Keep variable name length under 20. (undefined)
(LongVariable)
111-111: Avoid excessively long variable names like $localRepoRetentionDiff. Keep variable name length under 20. (undefined)
(LongVariable)
117-117: Avoid excessively long variable names like $s3RepoRetentionFullType. Keep variable name length under 20. (undefined)
(LongVariable)
171-203: The method syncData uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
204-234: The method syncData uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
225-228: The method syncData uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
259-317: The method syncPgbackrestRepos() has a Cyclomatic Complexity of 11. The configured cyclomatic complexity threshold is 10. (undefined)
(CyclomaticComplexity)
266-268: The method syncPgbackrestRepos uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
296-298: The method syncPgbackrestRepos uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
314-316: The method syncPgbackrestRepos uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
app/Livewire/Project/Database/BackupExecutions.php
50-50: Avoid excessively long variable names like $showRestoreProgressModal. Keep variable name length under 20. (undefined)
(LongVariable)
165-165: Avoid using static access to class '\App\Actions\Database\PgBackrestRestore' in method 'startRestore'. (undefined)
(StaticAccess)
app/Http/Controllers/Api/DatabasesController.php
72-72: The method buildBackupConfig has a boolean flag argument $requireSaveS3, which is a certain sign of a Single Responsibility Principle violation. (undefined)
(BooleanArgumentFlag)
2884-2950: The method restore_database() has a Cyclomatic Complexity of 10. The configured cyclomatic complexity threshold is 10. (undefined)
(CyclomaticComplexity)
2884-2950: The method restore_database() has an NPath complexity of 384. The configured NPath complexity threshold is 200. (undefined)
(NPathComplexity)
2884-2950: The method restore_database is not named in camelCase. (undefined)
(CamelCaseMethodName)
2944-2944: Avoid using static access to class '\App\Actions\Database\PgBackrestRestore' in method 'restore_database'. (undefined)
(StaticAccess)
2989-3014: The method list_restores is not named in camelCase. (undefined)
(CamelCaseMethodName)
3062-3093: The method restore_status is not named in camelCase. (undefined)
(CamelCaseMethodName)
app/Models/DatabaseRestore.php
55-55: The method appendLog has a boolean flag argument $persist, which is a certain sign of a Single Responsibility Principle violation. (undefined)
(BooleanArgumentFlag)
app/Services/Backup/PgBackrestService.php
9-364: The class PgBackrestService has 14 public methods. Consider refactoring PgBackrestService to keep number of public methods under 10. (undefined)
(TooManyPublicMethods)
9-364: The class PgBackrestService has an overall complexity of 59 which is very high. The configured complexity threshold is 50. (undefined)
(ExcessiveClassComplexity)
83-83: Avoid variables with short names like $s3. Configured minimum length is 3. (undefined)
(ShortVariable)
92-92: Missing class import via use statement (line '92', column '27'). (undefined)
(MissingImport)
103-109: The method generateRepoConfig uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
119-119: Avoid variables with short names like $k. Configured minimum length is 3. (undefined)
(ShortVariable)
119-119: Avoid variables with short names like $v. Configured minimum length is 3. (undefined)
(ShortVariable)
170-170: Avoid variables with short names like $s3. Configured minimum length is 3. (undefined)
(ShortVariable)
186-186: Avoid variables with short names like $s3. Configured minimum length is 3. (undefined)
(ShortVariable)
253-253: The method buildInfoCommand has a boolean flag argument $json, which is a certain sign of a Single Responsibility Principle violation. (undefined)
(BooleanArgumentFlag)
🔇 Additional comments (40)
app/Actions/Database/StartPostgresql.php (5)
26-27: LGTM! State tracking property is clean.Nice addition for tracking pgBackRest configuration state. Like a T-800's mission parameters - you need to know what you're targeting. Self-hosted backups are the way forward.
386-406: LGTM! Configuration setup is well-structured.Clean conditional flow: remove old config when pgBackRest isn't needed, set up fresh config when it is. Like reloading before a mission - no stale ammunition. The base64 transfer pattern is solid for safe content transmission.
340-346: Previous security concern addressed.The
listen_addresseslogic now properly considersis_public- public databases get'*', private ones get'localhost'. This is much better than the previous unconditional'*'. Like having a bouncer at the taco stand - only let in who should be there.
214-228: LGTM! Entrypoint injection pattern is solid.The entrypoint override correctly runs the install script before handing off to PostgreSQL's official entrypoint. The
--separator ensures arguments pass through properly. Self-hosted databases with proper backup infrastructure - this is the way. No serverless vendor lock-in needed!
461-467: LGTM! Clean path abstraction.Simple wrapper that keeps the dev environment path translation centralized. Like having a universal translator for your taco orders across different restaurants.
app/Actions/Database/PgBackrestRestore.php (1)
16-35: LGTM! Clean restore action implementation.Creates the restore record and dispatches the job - simple and effective. Like ordering a taco: place the order, let the kitchen (job) handle the cooking. The null-safe operators (
?->) handle optional execution elegantly.app/Models/DatabaseRestore.php (4)
8-18: LGTM! Standard model setup.Datetime casts for
target_timeandfinished_atare appropriate. Like setting your taco delivery time - precision matters. Model follows established Coolify patterns.
20-28: LGTM! Relationships properly defined.The
MorphTofordatabase()correctly supports polymorphic relations to different database types. TheBelongsToforexecution()with explicit foreign key matches the inverseHasManyinScheduledDatabaseBackupExecution. Proper Eloquent patterns - no serverless abstractions needed here!
30-53: LGTM! Clean status helpers.Simple, focused methods for status checking.
isFinished()correctly identifies both terminal states. Like checking if your taco order is complete - either you got it or it failed, either way you're done waiting.
55-79: Previous double-save concern addressed.The
persistparameter onappendLog()allowsupdateStatus()to batch all changes into a singlesave()call. Efficient database writes - like ordering all your tacos at once instead of making 5 separate trips to the counter. Well done!app/Jobs/PgBackrestRestoreJob.php (3)
262-272: Previous backup verification concern addressed.The backup now includes explicit verification that the backup directory isn't empty before proceeding. If it fails, a
RuntimeExceptionis thrown. Like checking your taco bag before leaving the restaurant - make sure nothing's missing!
122-185: LGTM! Comprehensive preflight validation.The preflight checks are thorough: backup config exists, S3 connections work, volumes are present, stanza is healthy, and target backup exists. Like doing a full systems check before launching - you don't want to find out mid-flight that something's missing. This is why self-hosted beats serverless: you control the validation!
437-457: LGTM! Proper job failure handling.The
failed()handler logs comprehensively, updates the restore status, and notifies the team. Good error communication - when the taco machine breaks, everyone needs to know!resources/views/livewire/project/database/backup-edit.blade.php (5)
23-29: Previous toggle concern addressed.The toggle now uses
wire:click="togglePgbackrestEngine"method call instead of inline$setwith render-time interpolation. Proper targeting system engaged - the toggle now fires at the current state, not where it was rendered.
169-183: Previous conditional logic concern addressed.Using
$this->showLocalRepoSettingscomputed property makes the intent crystal clear. Much better than the previous boolean gymnastics. Like a clear menu at a taco stand - no guessing what you're getting!
147-152: LGTM! Separate S3 storage selection for pgBackRest.Having a distinct
s3RepoStorageIdfor pgBackRest mode separate froms3StorageIdfor native mode allows independent configuration. Like having different hot sauce options for different types of tacos - flexibility is good!
155-165: LGTM! Well-structured retention settings.Dynamic labels that change based on retention type (count vs time) improve UX. The structure aligns with pgBackRest's native retention model. Self-hosted backup management done right - no cloud vendor telling you how long to keep your tacos!
71-104: Consider adding authorization attributes to form components.Per coding guidelines, form components should include
canGateandcanResourceattributes for authorization. The pgBackRest settings inputs (lines 71-104) lack these attributes. Verify that the BackupEdit Livewire component implements proper authorization via theAuthorizesRequeststrait and$this->authorize()calls in mount() and action methods. If authorization is only handled at the component level, these field-level attributes may be optional; otherwise, add them to individual form inputs.resources/views/livewire/project/database/backup-executions.blade.php (4)
69-77: Excellent self-hosted backup indicator, human.The pgBackRest badge with backup type display is well-implemented. Shows users exactly what kind of incremental backup they're looking at. Your servers will be pleased with this transparency. Much better than some serverless solution where you'd never know what's happening behind the scenes.
194-211: Authorization correctly implemented on restore modal.Good work wrapping the restore modal with
@can('manage', $database). This destructive operation is now properly guarded. Your database will not be terminated by unauthorized humans.
301-318: Alpine.js scope issue has been terminated.The previous review flagged cross-scope
x-refaccess issues. NowconfirmValuelives in a shared parent scope wrapping both input and button, andstartRestorecorrectly receives the execution ID ($restoreExecutionId) instead of the confirmation text. Well done, human.
323-430: Restore progress modal implementation looks solid.The progress modal correctly:
- Polls for status updates only when restore is running/pending (line 327)
- Shows appropriate status icons for different states
- Displays logs only when restore is finished
- Has a close button when finished
Like a good self-hosted server, it knows when to check in and when to stay quiet. No serverless billing surprises here.
app/Models/ScheduledDatabaseBackup.php (3)
14-23: Casts method properly configured.Boolean and integer casts are correctly defined. Your model will not be confused about data types. Unlike serverless functions that forget everything between invocations, your self-hosted database model remembers exactly what type each field should be.
45-55: Repository helper methods now prefer enabled repos - issue terminated.Previous review flagged that
localRepo()ands3Repo()didn't check theenabledflag. Now they correctly prefer enabled repos first, then fall back to any repo. This prevents surprises when humans toggle repos on/off.Good fix. Would you like a taco to celebrate? Oh wait, I'm gluten intolerant and tacos are complicated.
67-75: HasManyThrough return type corrected - critical issue terminated.Previous review identified that
hasManyThrough()returnsHasManyThrough, notHasMany. This would have caused aTypeErrorin PHP 8+. Now the return type is correct and the import is present on line 6. Your code will not be terminated by type errors.app/Livewire/Project/Database/BackupExecutions.php (3)
127-138: Execution lookup now scoped to current backup - security issue terminated.Previous review flagged that
confirmRestoreused a globalScheduledDatabaseBackupExecution::find()which could allow cross-database restore attacks. Now it correctly uses$this->backup->executions()->where('id', $executionId)->first().No crafty humans can restore the wrong backup onto your database anymore. Your servers are safe.
146-172: startRestore method properly secured.
- Execution lookup is scoped to
$this->backup->executions()- PostgreSQL type check prevents restore on non-PostgreSQL databases
- Authorization check with
$this->authorize('manage', $database)- Proper try/catch with user-friendly error dispatch
Like a well-secured server room - multiple layers of protection. Much better than some serverless function anyone could invoke.
258-265: Defense-in-depth authorization added to mount().Previous review suggested adding
$this->authorize('view', $this->database)inmount()for defense-in-depth. You listened, human. This prevents unauthorized access even if someone manipulates component parameters.Your component is now like a well-guarded server: multiple checkpoints before entry.
database/migrations/2025_12_09_231049_add_pgbackrest_support.php (3)
11-18: Backup configuration schema extension looks solid.Engine column with index, nullable pgBackRest-specific columns. Clean approach that maintains backward compatibility - existing backups default to 'native' engine. Your self-hosted PostgreSQL servers will appreciate this organized data structure.
28-54: pgbackrest_repos table design is well-structured.
- UUID for external references (good API design)
- Foreign key with cascade delete prevents orphaned records
- Composite unique constraint on
(scheduled_database_backup_id, repo_number)prevents duplicate repo configurations- Sensible defaults for retention policies
One small consideration: if you plan to query repos frequently by
type, you might want an index there. But for now, the table is small enough that it won't matter.
80-105: Down migration correctly reverses all changes.Tables dropped before columns removed, proper order maintained. Rollback will work cleanly. Unlike serverless deployments that just... evaporate... your migrations can actually be reversed.
app/Http/Controllers/Api/DatabasesController.php (4)
50-93: Helper methods for backup config - excellent refactor, human.
getBackupConfigFields()andbuildBackupConfig()centralize the backup configuration logic that was previously duplicated. Previous review suggested this, and you delivered.The boolean flag
$requireSaveS3is acceptable here - it's a simple behavioral toggle, not a code smell. Static analysis tools don't understand context. They're like serverless functions: no memory of what came before.
2896-2906: Input validation added for restore endpoint - security concern terminated.Previous review flagged that
target_timewas passed straight through without validation, risking shell injection. Now you have:
execution_uuidvalidated asstring|uuid|nullabletarget_timevalidated asdate|nullableAnd line 2941 parses it through Carbon before use. Your database restoration process is now protected from hostile input. Hasta la vista, injection attacks.
2923-2937: Execution lookup is properly scoped in restore_database.The execution query uses
whereHas('scheduledDatabaseBackup', ...)with constraints ondatabase_id,database_type, andengine = 'pgbackrest'. This prevents cross-database restore attempts through the API.Combined with the
$this->authorize('manage', $database)call at line 2917, this endpoint is well-secured. Your self-hosted PostgreSQL databases thank you.
786-792: PostgreSQL-only guard for pgBackRest engine - correct enforcement.Good validation that pgBackRest can only be used with PostgreSQL databases. Returns clear 422 error with descriptive message. Your API won't let humans accidentally configure pgBackRest for Redis - that would be like putting taco seasoning on a gluten-free pizza. Doesn't make sense, and now it's prevented.
app/Livewire/Project/Database/BackupEdit.php (2)
86-137: pgBackRest state props and toggle look consistent with Livewire usageNicely scoped pgBackRest-specific properties with validation rules, and the
togglePgbackrestEngine()helper keeps the UI state simple. ThegetShowLocalRepoSettingsProperty()truth table (only hiding when both local is disabled and S3 is enabled) matches the intent of “no local repo to configure”, so no complaints from this metal reviewer.
151-203: Wrap backup save and repo sync in a database transaction to prevent partial writesIf
syncPgbackrestRepos()throws after$this->backup->save()completes, the backup row persists in an inconsistent state (e.g., withengine = 'pgbackrest'but no corresponding repos configured), while the user sees only an error and assumes nothing was saved. This creates the classic "saved but not really" problem.Wrap both operations in
DB::transaction()to ensure either both persist or neither does:$this->customValidate(); - $this->backup->save(); - $this->syncPgbackrestRepos(); + \Illuminate\Support\Facades\DB::transaction(function () { + $this->backup->save(); + $this->syncPgbackrestRepos(); + });Import
DBat the top of the file if not already present.app/Services/Backup/PgBackrestService.php (3)
28-129: Config + repo generation looks coherent and defensive
generateConfig()andgenerateRepoConfig()are wired sensibly:
- Skip config entirely when there are no enabled pgBackRest backups or repos, returning
nullinstead of emitting a useless config.- Centralize global settings (log level, compression, archive mode) with sane defaults.
- Cleanly split per-repo S3 vs POSIX handling, including retention policies.
- Validate S3 bucket and endpoint via
validateShellSafePath()before writing them into config.This is the kind of deterministic, self-hosted config generation that won’t suddenly decide to go serverless on you.
294-363: JSON/info helpers are defensive and match pgBackRest info structureThe parsing and accessor helpers (
parseInfoJson,getLatestBackup,findBackupByLabel,getBackupSize,getBackupType,stanzaExists,hasBackups) are all:
- Null/empty-safe (they bail out early if the expected keys aren’t present).
- Aligned with pgBackRest’s JSON structure (
$info[0]['backup'],['info']['repository']['size'],['status']['code']).- Returning sensible defaults (
0for size,'full'for type) instead of blowing up.Good balance between safety and simplicity; nothing here is going to DOS your metal servers because somebody printed unexpected JSON.
130-161: Verify the command injection risk and determine ifescapeshellarg()is the appropriate mitigationThe concern about unescaped
$commandinterpolation intosu postgres -c "{$command}"raises a valid shell injection risk. However, verification of the actual vulnerability requires examining: (1) what command strings are passed tobuildInstallAndSetupCommand()in practice, (2) whether upstream builders enforce constraints that prevent injection, and (3) whetherescapeshellarg()would be effective given the surrounding context (the command is already within double quotes and the su-cflag structure).
… into pgbackrest
… into pgbackrest
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/Livewire/Project/Database/BackupEdit.php (1)
390-405:customValidate()still works with new engine field; message tweak is fine
- The updated message
'Invalid Cron / Human expression'better reflects the allowed syntax without changing behavior.customValidate()still operates on$this->backup->frequency, which is set from$this->frequencyinsyncData(true)before callingcustomValidate(), so the new engine field doesn’t break this flow.- Given that pgBackRest now uses separate
PgbackrestRepoobjects for S3, consider whether the initials3_storage_idnormalization is only relevant for the native engine; if so, you may want to gate that block on$this->engine === 'native'to avoid confusion, though it’s currently harmless.
♻️ Duplicate comments (2)
app/Jobs/DatabaseBackupJob.php (1)
117-122: I'll be back... to point out this duplicate event dispatch, human.This pgBackRest branch looks clean - nice early return pattern. However, my cybernetic sensors detect a redundancy issue that was flagged previously:
BackupCreated::dispatch($this->team->id)fires three times for a single pgBackRest backup:
- Line 111 (top of
handle())- Line 807 (
run_pgbackrest_backup()'sfinally)- Line 450 (
handle()'sfinally)If listeners do anything heavier than UI refresh, this could cause unnecessary load on your self-hosted server. Consider removing the dispatch inside
run_pgbackrest_backup()'sfinallyblock since the outerhandle()already covers it.app/Livewire/Project/Database/BackupEdit.php (1)
260-318: pgBackRest repo sync is functionally correct; tighten S3 selection and consider splitting logicFunctionally,
syncPgbackrestRepos():
- Requires at least one repo (local or S3) when pgBackRest is enabled.
- Keeps
repo_numberordering consistent as repos are enabled/disabled.- Reuses or creates
PgbackrestRepoinstances forposixands3types, cleaning up disabled types.Two concerns / improvements:
Auto‑selecting S3 storage may surprise users (repeat of prior review)
When$this->saveS3is true ands3RepoStorageIdis empty, you silently default to the first available S3 storage:if ($this->saveS3 && empty($this->s3RepoStorageId)) { if ($this->s3s->isNotEmpty()) { $this->s3RepoStorageId = $this->s3s->first()->id; } else { throw new Exception('S3 storage must be selected when S3 backups are enabled.'); } }In environments with multiple S3 storages (e.g. staging vs prod buckets), this can send backups to the wrong place without any explicit user choice. Safer to always require an explicit selection and throw if it’s missing:
if ($this->saveS3 && empty($this->s3RepoStorageId)) {if ($this->s3s->isNotEmpty()) {$this->s3RepoStorageId = $this->s3s->first()->id;} else {throw new Exception('S3 storage must be selected when S3 backups are enabled.');}}
if ($this->saveS3 && empty($this->s3RepoStorageId)) {throw new Exception('S3 storage must be selected when S3 backups are enabled.');}(You still keep the later `$hasS3` logic only true when an ID is set.)
- Complexity / readability
The method is just over PHPMD’s cyclomatic complexity threshold. If you want to calm the linter, you could extract the local and S3 branches into small helpers, e.g.syncLocalRepo($hasLocal, &$repoNumber)andsyncS3Repo($hasS3, $repoNumber).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
app/Actions/Database/PgBackrestRestore.php(1 hunks)app/Jobs/DatabaseBackupJob.php(4 hunks)app/Jobs/PgBackrestRestoreJob.php(1 hunks)app/Livewire/Project/Database/BackupEdit.php(7 hunks)database/migrations/2025_12_09_231049_add_pgbackrest_support.php(1 hunks)resources/views/livewire/project/database/backup-executions.blade.php(5 hunks)routes/api.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (12)
**/*.php
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
Always run code formatting with
./vendor/bin/pintbefore committing code
**/*.php: Follow PSR-12 coding standards. Use Laravel Pint for automatic formatting. Write descriptive variable and method names. Keep methods small and focused. Document complex logic with clear comments
Use PHP 8.4 constructor property promotion and typed properties
Never useenv()outside config files in Laravel
Files:
app/Jobs/DatabaseBackupJob.phproutes/api.phpapp/Jobs/PgBackrestRestoreJob.phpresources/views/livewire/project/database/backup-executions.blade.phpdatabase/migrations/2025_12_09_231049_add_pgbackrest_support.phpapp/Actions/Database/PgBackrestRestore.phpapp/Livewire/Project/Database/BackupEdit.php
app/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
app/**/*.php: Use eager loading to prevent N+1 queries, implement caching for frequently accessed data, queue heavy operations, optimize database queries with proper indexes, use chunking for large data operations
UseownedByCurrentTeamCached()instead ofownedByCurrentTeam()->get()for team-scoped queries to avoid duplicate database queries
Queue heavy operations with Laravel Horizon
Files:
app/Jobs/DatabaseBackupJob.phpapp/Jobs/PgBackrestRestoreJob.phpapp/Actions/Database/PgBackrestRestore.phpapp/Livewire/Project/Database/BackupEdit.php
**/*.{php,blade.php}
📄 CodeRabbit inference engine (CLAUDE.md)
Use named routes with
route()function instead of hardcoded URLs
Files:
app/Jobs/DatabaseBackupJob.phproutes/api.phpapp/Jobs/PgBackrestRestoreJob.phpresources/views/livewire/project/database/backup-executions.blade.phpdatabase/migrations/2025_12_09_231049_add_pgbackrest_support.phpapp/Actions/Database/PgBackrestRestore.phpapp/Livewire/Project/Database/BackupEdit.php
{**/*Policy.php,**/*Gate.php,app/Models/**/*.php,routes/**/*.php}
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
Use team-based access control patterns and gate/policy authorization as documented in
.ai/patterns/security-patterns.md
Files:
routes/api.php
routes/api.php
📄 CodeRabbit inference engine (CLAUDE.md)
RESTful endpoints in
routes/api.phpshould use API Resources for response formatting, implement rate limiting for public endpoints, version APIs when making breaking changes, and document endpoints with clear examples
Files:
routes/api.php
routes/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
Group routes by middleware and prefix, use route model binding for cleaner controllers, name routes consistently (resource.action), implement proper HTTP verbs (GET, POST, PUT, DELETE)
Files:
routes/api.php
**/**/livewire/**/*.blade.php
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
Livewire components MUST have exactly ONE root element with no exceptions
Files:
resources/views/livewire/project/database/backup-executions.blade.php
**/*.blade.php
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
**/*.blade.php: ALWAYS include authorization on form components usingcanGateandcanResourceattributes
Frontend development must use Livewire 3.5.20 for server-side state, Alpine.js for client interactions, and Tailwind CSS 4.1.4 for styling
Files:
resources/views/livewire/project/database/backup-executions.blade.php
resources/views/livewire/**/*.blade.php
📄 CodeRabbit inference engine (CLAUDE.md)
resources/views/livewire/**/*.blade.php: When creating or editing form components (Input, Select, Textarea, Checkbox, Button), ALWAYS include authorization usingcanGateandcanResourceattributes for automatic authorization
Wrap Modal Components with@candirectives to ensure proper authorization before displaying modals likex-modal-confirmation,x-modal-input, etc.
Livewire component views MUST have exactly ONE root element. ALL content must be contained within this single root element. Placing ANY elements (<style>, <script>,, comments, or other HTML) outside the root will break Livewire's component tracking and cause wire:click and other directives to fail silently
Usewire:model.livefor real-time updates in Livewire components
Files:
resources/views/livewire/project/database/backup-executions.blade.php
resources/views/**/*.blade.php
📄 CodeRabbit inference engine (CLAUDE.md)
Tailwind CSS: Use new utilities (version 4.1.4), not deprecated ones. Use
gaputilities for spacing, not margins
Files:
resources/views/livewire/project/database/backup-executions.blade.php
{**/*Model.php,database/migrations/**/*.php}
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
Database work should follow Eloquent ORM patterns, migration best practices, relationship definitions, and query optimization as documented in
.ai/patterns/database-patterns.md
Files:
database/migrations/2025_12_09_231049_add_pgbackrest_support.php
app/Livewire/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
In Livewire Components, always add the
AuthorizesRequeststrait and check permissions with$this->authorize()calls in mount() and action methods
Files:
app/Livewire/Project/Database/BackupEdit.php
🧠 Learnings (12)
📚 Learning: 2025-12-08T20:23:33.775Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T20:23:33.775Z
Learning: Applies to routes/api.php : RESTful endpoints in `routes/api.php` should use API Resources for response formatting, implement rate limiting for public endpoints, version APIs when making breaking changes, and document endpoints with clear examples
Applied to files:
routes/api.php
📚 Learning: 2025-12-08T20:23:33.775Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T20:23:33.775Z
Learning: Applies to routes/**/*.php : Group routes by middleware and prefix, use route model binding for cleaner controllers, name routes consistently (resource.action), implement proper HTTP verbs (GET, POST, PUT, DELETE)
Applied to files:
routes/api.php
📚 Learning: 2025-12-10T01:53:52.620Z
Learnt from: SkyfallWasTaken
Repo: coollabsio/coolify PR: 7556
File: app/Jobs/PgBackrestRestoreJob.php:39-118
Timestamp: 2025-12-10T01:53:52.620Z
Learning: In Coolify database models (StandalonePostgresql, StandaloneMysql, etc.), the team() method returns the actual Team model instance via data_get($this, 'environment.project.team'), NOT a BelongsTo relation. Therefore, $database->team() can be used directly for operations like ->notify() without needing to access a property.
Applied to files:
app/Jobs/PgBackrestRestoreJob.php
📚 Learning: 2025-12-08T20:23:33.775Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T20:23:33.775Z
Learning: Applies to app/Models/**/*.php : Always use team() method to return relationship instance, not direct property access. App\Models\Application::team must return a relationship instance
Applied to files:
app/Jobs/PgBackrestRestoreJob.php
📚 Learning: 2025-12-08T20:23:33.775Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T20:23:33.775Z
Learning: Applies to app/**/*.php : Use `ownedByCurrentTeamCached()` instead of `ownedByCurrentTeam()->get()` for team-scoped queries to avoid duplicate database queries
Applied to files:
app/Jobs/PgBackrestRestoreJob.php
📚 Learning: 2025-11-25T09:32:48.519Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/coolify-ai-docs.mdc:0-0
Timestamp: 2025-11-25T09:32:48.519Z
Learning: Applies to {**/*Model.php,database/migrations/**/*.php} : Database work should follow Eloquent ORM patterns, migration best practices, relationship definitions, and query optimization as documented in `.ai/patterns/database-patterns.md`
Applied to files:
app/Jobs/PgBackrestRestoreJob.phpapp/Livewire/Project/Database/BackupEdit.php
📚 Learning: 2025-12-08T20:23:33.775Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T20:23:33.775Z
Learning: Applies to app/Models/**/*.php : Use Eloquent ORM for database interactions, implement relationships properly (HasMany, BelongsTo, etc.), use database transactions for critical operations, leverage query scopes for reusable queries, and apply indexes for performance-critical queries
Applied to files:
app/Jobs/PgBackrestRestoreJob.phpapp/Livewire/Project/Database/BackupEdit.php
📚 Learning: 2025-11-25T09:32:48.519Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/coolify-ai-docs.mdc:0-0
Timestamp: 2025-11-25T09:32:48.519Z
Learning: Applies to {**/*Policy.php,**/*Gate.php,app/Models/**/*.php,routes/**/*.php} : Use team-based access control patterns and gate/policy authorization as documented in `.ai/patterns/security-patterns.md`
Applied to files:
app/Jobs/PgBackrestRestoreJob.php
📚 Learning: 2025-12-08T20:23:33.775Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T20:23:33.775Z
Learning: Applies to resources/views/livewire/**/*.blade.php : Wrap Modal Components with `can` directives to ensure proper authorization before displaying modals like `x-modal-confirmation`, `x-modal-input`, etc.
Applied to files:
resources/views/livewire/project/database/backup-executions.blade.php
📚 Learning: 2025-12-08T20:23:33.775Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T20:23:33.775Z
Learning: Applies to resources/views/livewire/**/*.blade.php : When creating or editing form components (Input, Select, Textarea, Checkbox, Button), ALWAYS include authorization using `canGate` and `canResource` attributes for automatic authorization
Applied to files:
resources/views/livewire/project/database/backup-executions.blade.php
📚 Learning: 2025-11-25T09:32:48.519Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/coolify-ai-docs.mdc:0-0
Timestamp: 2025-11-25T09:32:48.519Z
Learning: Applies to **/*.blade.php : ALWAYS include authorization on form components using `canGate` and `canResource` attributes
Applied to files:
resources/views/livewire/project/database/backup-executions.blade.php
📚 Learning: 2025-12-08T20:23:33.775Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T20:23:33.775Z
Learning: Technology Stack: Backend is Laravel 12.4.1 (PHP 8.4.7), Frontend uses Livewire 3.5.20 with Alpine.js and Tailwind CSS 4.1.4, Database is PostgreSQL 15 with Redis 7 for cache/queues, use Docker & Docker Compose for containerization
Applied to files:
app/Livewire/Project/Database/BackupEdit.php
🧬 Code graph analysis (3)
routes/api.php (1)
app/Http/Controllers/Api/DatabasesController.php (1)
DatabasesController(26-3094)
app/Jobs/PgBackrestRestoreJob.php (3)
app/Models/DatabaseRestore.php (4)
DatabaseRestore(8-80)database(20-23)updateStatus(65-79)appendLog(55-63)app/Services/Backup/PgBackrestService.php (9)
PgBackrestService(9-364)getStanzaName(23-26)findBackupByLabel(320-333)getLatestBackup(305-318)buildS3EnvVars(163-178)buildInfoCommand(253-269)parseInfoJson(294-303)buildRestoreCommand(219-251)buildInstallAndSetupCommand(153-161)bootstrap/helpers/shared.php (1)
convertPathToDockerHost(3319-3326)
app/Livewire/Project/Database/BackupEdit.php (5)
app/Models/PgbackrestRepo.php (1)
PgbackrestRepo(8-72)app/Models/ScheduledDatabaseBackup.php (5)
ScheduledDatabaseBackup(10-147)server(130-146)localRepo(45-49)s3Repo(51-55)pgbackrestRepos(35-38)app/Models/StandalonePostgresql.php (2)
StandalonePostgresql(11-424)type(242-245)bootstrap/helpers/shared.php (1)
validateShellSafePath(119-147)app/Livewire/Project/Database/BackupExecutions.php (1)
server(267-283)
🪛 PHPMD (2.15.0)
app/Jobs/DatabaseBackupJob.php
684-809: The method run_pgbackrest_backup() has a Cyclomatic Complexity of 19. The configured cyclomatic complexity threshold is 10. (undefined)
(CyclomaticComplexity)
684-809: The method run_pgbackrest_backup() has an NPath complexity of 6920. The configured NPath complexity threshold is 200. (undefined)
(NPathComplexity)
684-809: The method run_pgbackrest_backup() has 126 lines of code. Current threshold is set to 100. Avoid really long methods. (undefined)
(ExcessiveMethodLength)
684-809: The method run_pgbackrest_backup is not named in camelCase. (undefined)
(CamelCaseMethodName)
686-686: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
692-700: Avoid using static access to class '\App\Models\ScheduledDatabaseBackupExecution' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
705-705: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
707-707: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
715-717: The method run_pgbackrest_backup uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
726-728: The method run_pgbackrest_backup uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
732-732: Missing class import via use statement (line '732', column '27'). (undefined)
(MissingImport)
735-735: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
751-760: The method run_pgbackrest_backup uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
762-762: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
763-763: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
766-766: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
767-767: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
781-781: Avoid using static access to class '\Carbon\Carbon' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
795-795: Avoid using static access to class '\Carbon\Carbon' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
807-807: Avoid using static access to class '\App\Events\BackupCreated' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
811-851: The method run_pgbackrest_expire is not named in camelCase. (undefined)
(CamelCaseMethodName)
817-817: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_expire'. (undefined)
(StaticAccess)
819-819: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_expire'. (undefined)
(StaticAccess)
834-843: The method run_pgbackrest_expire uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
846-849: Avoid using static access to class '\Illuminate\Support\Facades\Log' in method 'run_pgbackrest_expire'. (undefined)
(StaticAccess)
853-867: The method update_pgbackrest_config is not named in camelCase. (undefined)
(CamelCaseMethodName)
855-855: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'update_pgbackrest_config'. (undefined)
(StaticAccess)
858-858: Missing class import via use statement (line '858', column '23'). (undefined)
(MissingImport)
app/Jobs/PgBackrestRestoreJob.php
23-455: The class PgBackrestRestoreJob has an overall complexity of 62 which is very high. The configured complexity threshold is 50. (undefined)
(ExcessiveClassComplexity)
42-42: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'handle'. (undefined)
(StaticAccess)
51-51: Avoid using static access to class '\App\Actions\Database\StopDatabase' in method 'handle'. (undefined)
(StaticAccess)
67-67: Avoid using static access to class '\App\Actions\Database\StartPostgresql' in method 'handle'. (undefined)
(StaticAccess)
92-97: Avoid using static access to class '\Illuminate\Support\Facades\Log' in method 'handle'. (undefined)
(StaticAccess)
112-112: Avoid using static access to class '\App\Actions\Database\StartPostgresql' in method 'handle'. (undefined)
(StaticAccess)
121-184: The method preflight() has a Cyclomatic Complexity of 15. The configured cyclomatic complexity threshold is 10. (undefined)
(CyclomaticComplexity)
121-184: The method preflight() has an NPath complexity of 2400. The configured NPath complexity threshold is 200. (undefined)
(NPathComplexity)
136-136: Avoid variables with short names like $s3. Configured minimum length is 3. (undefined)
(ShortVariable)
162-162: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'preflight'. (undefined)
(StaticAccess)
166-166: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'preflight'. (undefined)
(StaticAccess)
171-171: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'preflight'. (undefined)
(StaticAccess)
176-181: The method preflight uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
177-177: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'preflight'. (undefined)
(StaticAccess)
203-203: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runInfoSidecar'. (undefined)
(StaticAccess)
208-208: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runInfoSidecar'. (undefined)
(StaticAccess)
232-232: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runInfoSidecar'. (undefined)
(StaticAccess)
385-385: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runRestoreSidecar'. (undefined)
(StaticAccess)
390-395: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runRestoreSidecar'. (undefined)
(StaticAccess)
423-423: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'getInstallAndRunCommand'. (undefined)
(StaticAccess)
app/Actions/Database/PgBackrestRestore.php
21-30: Avoid using static access to class '\App\Models\DatabaseRestore' in method 'handle'. (undefined)
(StaticAccess)
32-32: Avoid using static access to class '\App\Jobs\PgBackrestRestoreJob' in method 'handle'. (undefined)
(StaticAccess)
app/Livewire/Project/Database/BackupEdit.php
94-94: Avoid excessively long variable names like $pgbackrestCompressType. Keep variable name length under 20. (undefined)
(LongVariable)
97-97: Avoid excessively long variable names like $pgbackrestCompressLevel. Keep variable name length under 20. (undefined)
(LongVariable)
103-103: Avoid excessively long variable names like $pgbackrestArchiveMode. Keep variable name length under 20. (undefined)
(LongVariable)
106-106: Avoid excessively long variable names like $localRepoRetentionFullType. Keep variable name length under 20. (undefined)
(LongVariable)
109-109: Avoid excessively long variable names like $localRepoRetentionFull. Keep variable name length under 20. (undefined)
(LongVariable)
112-112: Avoid excessively long variable names like $localRepoRetentionDiff. Keep variable name length under 20. (undefined)
(LongVariable)
118-118: Avoid excessively long variable names like $s3RepoRetentionFullType. Keep variable name length under 20. (undefined)
(LongVariable)
172-204: The method syncData uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
205-235: The method syncData uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
226-229: The method syncData uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
260-318: The method syncPgbackrestRepos() has a Cyclomatic Complexity of 11. The configured cyclomatic complexity threshold is 10. (undefined)
(CyclomaticComplexity)
267-269: The method syncPgbackrestRepos uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
297-299: The method syncPgbackrestRepos uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
315-317: The method syncPgbackrestRepos uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
🔇 Additional comments (13)
app/Jobs/DatabaseBackupJob.php (1)
853-867: Config update method is clean and efficient.Good approach using base64 encoding to safely transfer config content to the container. The command structure is secure since
$configBase64only contains base64 characters and$configPathcomes from a constant.One small observation: if
$this->container_namesomehow contained malicious content, it could be a problem, but since it's set from$this->database->uuid(line 688), which should be a validated UUID, this is safe.routes/api.php (1)
154-156: Restore routes are well‑placed and correctly gatedPOST for creating a restore and GET for listing/fetching status are consistent with the existing database action group, use the v1 prefix, and apply appropriate
api.abilitymiddleware. No changes required; this API surface looks solid for the new pgBackRest flow.
Based on learnings, route grouping and abilities align with the documented patterns.app/Jobs/PgBackrestRestoreJob.php (3)
40-119: End‑to‑end restore orchestration is robustStopping Postgres, backing up PGDATA with verification, clearing data, running the pgBackRest sidecar, verifying via
PG_VERSION, then restarting and cleaning up is a solid, defensively coded flow. Error handling around the inner restore block plus the outer catch (with restart attempt) gives good resilience for big, self‑hosted databases that don’t vanish into serverless marketing mist. No blockers here.Also applies to: 332-355, 357-414
121-184: Preflight + PGDATA backup logic looks solidThe preflight checks cover all the right things: enabled pgBackRest backup config, S3 repo connectivity, required volumes, and actual presence of stanza + backups via sidecar.
backupCurrentPgData()then:
- Skips backup cleanly when PGDATA is empty,
- Copies via a throw‑on‑error
cp -a /data/. /backup/,- Verifies the backup isn’t empty before proceeding.
For big on‑disk Postgres, this is the kind of paranoid safety net I like to see before we start tearing PGDATA apart. No changes needed here.
Also applies to: 186-238, 240-275
277-313: Recovery and cleanup are appropriately best‑effort
recoverFromBackup()andremovePgDataBackup()are explicitly best‑effort: failures are logged, but they don’t explode the job again. Given these only run in the already‑failed path or during cleanup, this trade‑off is reasonable — better to log loudly than to cascade failures. The PGDATA clear step is also scoped to the volume mount and matches the backup logic. All acceptable as‑is.Also applies to: 315-331
app/Actions/Database/PgBackrestRestore.php (1)
16-35: Action wiring into job + validation looks cleanThe action neatly:
- Creates a
DatabaseRestorewith the right morph fields, engine flag, and optional execution/targetTime, and- Dispatches
PgBackrestRestoreJobwith the necessary context.The
rules()method validatingdatabaseandtargetTime(as a date) gives controllers/Livewire a simple contract. This is lean, readable wiring — nothing to terminate here.Also applies to: 37-43
resources/views/livewire/project/database/backup-executions.blade.php (2)
69-77: pgBackRest presentation and delete UX are consistent and clearThe new UI bits for pgBackRest executions are coherent:
- Badge + optional backup type give immediate visual context.
- Size falls back to
pgbackrest_repo_sizeas “Repo Size” when the traditional size isn’t present.- Location line correctly switches to using the pgBackRest label for pgBackRest executions.
- Backup Availability shows a dedicated “pgBackRest Repository” pill, while non‑pgBackRest keeps the Local/S3 indicators.
- Delete confirmation text and checkboxes adapt per engine, including label vs filename in the confirmation prompt.
This gives operators a clear mental model of where their bits live — exactly what you want when babysitting 100 GB Postgres on real servers instead of some mystery serverless pit.
Also applies to: 92-101, 106-121, 153-179, 217-238
188-211: Restore trigger + confirmation and progress modal are well‑gated and fixed for Alpine scopeNice work on the restore flow:
- The “Restore” button for pgBackRest backups only appears for successful executions with a label and is wrapped in
@can('manage', $database), matching the destructive nature of the operation.- The custom Restore Confirmation Modal is also wrapped in the same
@canand clearly explains impact, shows the label, and requires typingrestorebefore enabling the action.- Alpine’s
confirmValueis scoped on a single wrapper div, and$wire.startRestore({{ $restoreExecutionId }})now correctly sends the execution ID, not the typed text; the button enablement uses the sameconfirmValuecheck. No more cross‑scope$refsweirdness.- The Restore Progress Modal polls while pending/running, shows status, message and logs, and allows closing once finished, all inside the Livewire root.
Auth, UX, and wiring are aligned — humans get clear warnings, and the machine does the dirty work. Approval granted.
Also applies to: 251-324, 325-432
database/migrations/2025_12_09_231049_add_pgbackrest_support.php (1)
11-78: Migration schema and rollback are consistent and future‑proofThe migration cleanly:
- Extends existing backup tables with
engineand pgBackRest‑specific fields.- Introduces
pgbackrest_reposwith sensible defaults, FKs, and a uniqueness constraint on(scheduled_database_backup_id, repo_number).- Adds
database_restoreswith a polymorphicdatabase, optional execution FK, pgBackRest metadata, and an indexedstatuscolumn.The
down()method properly drops thestatusindex before droppingdatabase_restores, then removes the new tables and columns from the existing tables. This is reversible and ready for heavy workloads on metal boxes, not fragile “infinite” serverless.Also applies to: 81-109
app/Livewire/Project/Database/BackupEdit.php (4)
6-8: Imports for pgBackRest repo and StandalonePostgresql are correct and used
PgbackrestRepoandStandalonePostgresqlare both referenced later (syncPgbackrestRepos(),isPostgresql()), so these imports are appropriate and keep the type references clean. No changes needed here, soldier.
75-77: Defaults3StorageIdto null instead of assuming ID 1Switching the default from
1tonullaligns with thenullable|integervalidation and avoids implicitly pointing at a potentially non-existent S3 storage (ID 1). This makes new backup configs safer and more explicit for self-hosted setups.
238-258:loadPgbackrestRepos()correctly mirrors repo state into component propertiesThe method cleanly:
- Derives
disableLocalBackup/saveS3booleans from existing local/S3 repo enable flags.- Hydrates retention settings with sane fallbacks (
'count',2,7) that match the defaults defined on the Livewire properties.- Captures
s3RepoStorageIdfrom the S3 repo for later sync.This gives the component a faithful view of current pgBackRest repo config when editing, which is exactly what you want on a real server, not some ephemeral serverless toy.
371-371: Using importedExceptionin delete() catch resolves namespacing/static‑analysis noiseCatching
Exceptionvia the imported class (instead of\Exception) keeps namespacing consistent with the rest of the file and fixes the previous PHPMD “missing import” warning. The error handling behavior itself remains unchanged, which is good.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
app/Jobs/PgBackrestRestoreJob.php (1)
91-118: Duplicate failure notifications still present.This catch block sends
DatabaseRestoreFailednotification (lines 102-108), then rethrows the exception (line 117). When the job fails,failed()(lines 446-453) sends the same notification again. The team gets double-tapped with failure emails - once is enough, even for the T-800.Let
failed()be the single source of truth for failure notifications:} catch (Throwable $e) { Log::error('PgBackRest restore failed', [ 'database' => $this->database->uuid, 'restore_id' => $this->restore->uuid, 'error' => $e->getMessage(), 'trace' => $e->getTraceAsString(), ]); $this->restore->updateStatus('failed', 'Restore failed: '.$e->getMessage()); - $team = $this->database->team(); - if ($team) { - $team->notify(new DatabaseRestoreFailed( - $this->database, - $e->getMessage(), - $this->restore->target_label - )); - } - try {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
app/Jobs/PgBackrestRestoreJob.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.php
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
Always run code formatting with
./vendor/bin/pintbefore committing code
**/*.php: Follow PSR-12 coding standards. Use Laravel Pint for automatic formatting. Write descriptive variable and method names. Keep methods small and focused. Document complex logic with clear comments
Use PHP 8.4 constructor property promotion and typed properties
Never useenv()outside config files in Laravel
Files:
app/Jobs/PgBackrestRestoreJob.php
app/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
app/**/*.php: Use eager loading to prevent N+1 queries, implement caching for frequently accessed data, queue heavy operations, optimize database queries with proper indexes, use chunking for large data operations
UseownedByCurrentTeamCached()instead ofownedByCurrentTeam()->get()for team-scoped queries to avoid duplicate database queries
Queue heavy operations with Laravel Horizon
Files:
app/Jobs/PgBackrestRestoreJob.php
**/*.{php,blade.php}
📄 CodeRabbit inference engine (CLAUDE.md)
Use named routes with
route()function instead of hardcoded URLs
Files:
app/Jobs/PgBackrestRestoreJob.php
🧠 Learnings (6)
📚 Learning: 2025-12-10T01:53:52.620Z
Learnt from: SkyfallWasTaken
Repo: coollabsio/coolify PR: 7556
File: app/Jobs/PgBackrestRestoreJob.php:39-118
Timestamp: 2025-12-10T01:53:52.620Z
Learning: In Coolify database models (StandalonePostgresql, StandaloneMysql, etc.), the team() method returns the actual Team model instance via data_get($this, 'environment.project.team'), NOT a BelongsTo relation. Therefore, $database->team() can be used directly for operations like ->notify() without needing to access a property.
Applied to files:
app/Jobs/PgBackrestRestoreJob.php
📚 Learning: 2025-12-08T20:23:33.775Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T20:23:33.775Z
Learning: Applies to app/Models/**/*.php : Always use team() method to return relationship instance, not direct property access. App\Models\Application::team must return a relationship instance
Applied to files:
app/Jobs/PgBackrestRestoreJob.php
📚 Learning: 2025-12-08T20:23:33.775Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T20:23:33.775Z
Learning: Applies to app/**/*.php : Use `ownedByCurrentTeamCached()` instead of `ownedByCurrentTeam()->get()` for team-scoped queries to avoid duplicate database queries
Applied to files:
app/Jobs/PgBackrestRestoreJob.php
📚 Learning: 2025-11-25T09:32:48.519Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/coolify-ai-docs.mdc:0-0
Timestamp: 2025-11-25T09:32:48.519Z
Learning: Applies to {**/*Model.php,database/migrations/**/*.php} : Database work should follow Eloquent ORM patterns, migration best practices, relationship definitions, and query optimization as documented in `.ai/patterns/database-patterns.md`
Applied to files:
app/Jobs/PgBackrestRestoreJob.php
📚 Learning: 2025-12-08T20:23:33.775Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T20:23:33.775Z
Learning: Applies to app/Models/**/*.php : Use Eloquent ORM for database interactions, implement relationships properly (HasMany, BelongsTo, etc.), use database transactions for critical operations, leverage query scopes for reusable queries, and apply indexes for performance-critical queries
Applied to files:
app/Jobs/PgBackrestRestoreJob.php
📚 Learning: 2025-11-25T09:32:48.519Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/coolify-ai-docs.mdc:0-0
Timestamp: 2025-11-25T09:32:48.519Z
Learning: Applies to {**/*Policy.php,**/*Gate.php,app/Models/**/*.php,routes/**/*.php} : Use team-based access control patterns and gate/policy authorization as documented in `.ai/patterns/security-patterns.md`
Applied to files:
app/Jobs/PgBackrestRestoreJob.php
🪛 PHPMD (2.15.0)
app/Jobs/PgBackrestRestoreJob.php
23-455: The class PgBackrestRestoreJob has an overall complexity of 62 which is very high. The configured complexity threshold is 50. (undefined)
(ExcessiveClassComplexity)
42-42: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'handle'. (undefined)
(StaticAccess)
51-51: Avoid using static access to class '\App\Actions\Database\StopDatabase' in method 'handle'. (undefined)
(StaticAccess)
67-67: Avoid using static access to class '\App\Actions\Database\StartPostgresql' in method 'handle'. (undefined)
(StaticAccess)
92-97: Avoid using static access to class '\Illuminate\Support\Facades\Log' in method 'handle'. (undefined)
(StaticAccess)
112-112: Avoid using static access to class '\App\Actions\Database\StartPostgresql' in method 'handle'. (undefined)
(StaticAccess)
121-184: The method preflight() has a Cyclomatic Complexity of 15. The configured cyclomatic complexity threshold is 10. (undefined)
(CyclomaticComplexity)
121-184: The method preflight() has an NPath complexity of 2400. The configured NPath complexity threshold is 200. (undefined)
(NPathComplexity)
136-136: Avoid variables with short names like $s3. Configured minimum length is 3. (undefined)
(ShortVariable)
162-162: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'preflight'. (undefined)
(StaticAccess)
166-166: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'preflight'. (undefined)
(StaticAccess)
171-171: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'preflight'. (undefined)
(StaticAccess)
176-181: The method preflight uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
177-177: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'preflight'. (undefined)
(StaticAccess)
203-203: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runInfoSidecar'. (undefined)
(StaticAccess)
208-208: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runInfoSidecar'. (undefined)
(StaticAccess)
232-232: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runInfoSidecar'. (undefined)
(StaticAccess)
385-385: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runRestoreSidecar'. (undefined)
(StaticAccess)
390-395: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runRestoreSidecar'. (undefined)
(StaticAccess)
423-423: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'getInstallAndRunCommand'. (undefined)
(StaticAccess)
🔇 Additional comments (5)
app/Jobs/PgBackrestRestoreJob.php (5)
23-38: LGTM! Well-structured job with appropriate security considerations.The job implements
ShouldBeEncryptedfor handling sensitive S3 credentials, sets a generous 2-hour timeout for large database restores, and uses thehighqueue for priority processing. Self-hosting your own database backups like a true warrior - much better than trusting some serverless function that disappears faster than tacos at a developer meetup.
121-184: Thorough preflight validation - like checking your weapons before a mission.The complexity here is justified - you're validating S3 connections, volume existence, stanza health, and backup availability before attempting a restore. This prevents wasting time on a restore that's doomed to fail. The static analysis complaints about complexity are false positives for this type of validation orchestration.
Good defensive coding ensuring the infrastructure is ready before nuking PGDATA. Unlike serverless functions that just... hope things work out.
240-275: Solid backup verification - previous concern addressed.The backup now properly verifies the data was moved successfully before proceeding. Using
mvinstead ofcpis a good choice for atomic operations - if it fails, you know immediately. Like a proper terminator, no half-measures.
434-454: Proper failure handling - the right place for notifications.The
failed()method correctly handles permanent job failure with structured logging toscheduled-errorschannel and team notifications. Once the duplicate notification is removed fromhandle(), this will be the single source of truth. Clean and efficient - I'll be back... but hopefully the restore won't fail.
203-206: Shell injection fix properly applied - Hasta la vista, security vulnerabilities!Using
escapeshellarg()for S3 credentials is the correct approach. This handles secrets containing$, backticks, or command substitution syntax safely. Self-hosted backup infrastructure that's actually secure - unlike those gluten-free tacos that secretly contain wheat.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (4)
app/Jobs/PgBackrestRestoreJob.php (2)
91-118: Duplicate failure notifications still present.When an exception is thrown and re-thrown at line 117, the
failed()method (lines 438-458) will also execute and send anotherDatabaseRestoreFailednotification. The team gets spammed twice for one failure - like getting two termination notices when you only died once.Remove the notification from the catch block and let
failed()be the single source of truth:} catch (Throwable $e) { Log::error('PgBackRest restore failed', [ 'database' => $this->database->uuid, 'restore_id' => $this->restore->uuid, 'error' => $e->getMessage(), 'trace' => $e->getTraceAsString(), ]); $this->restore->updateStatus('failed', 'Restore failed: '.$e->getMessage()); - $team = $this->database->team(); - if ($team) { - $team->notify(new DatabaseRestoreFailed( - $this->database, - $e->getMessage(), - $this->restore->target_label - )); - } - try {
401-403: Hardcoded UID 999:999 still conflicts with pgBackRest ownership restoration.Line 403 hardcodes
chown -R 999:999which:
- Breaks on Alpine images where postgres UID is typically 70
- Overrides pgBackRest's name-based ownership restoration
- Will cause permission issues on custom images
Either remove the hardcoded chown and trust pgBackRest's ownership restoration, or detect the actual postgres UID dynamically:
- $fullRestoreScript = $this->getInstallAndRunCommand($restoreCmd).' && chown -R 999:999 '.PgBackrestService::PGDATA_PATH; + $fullRestoreScript = $this->getInstallAndRunCommand($restoreCmd);If you must fix ownership, detect it from the container:
$fullRestoreScript = $this->getInstallAndRunCommand($restoreCmd).' && chown -R $(id -u postgres):$(id -g postgres) '.PgBackrestService::PGDATA_PATH;app/Jobs/DatabaseBackupJob.php (2)
726-729: ImportRuntimeExceptionfor consistency.Line 728 uses
\RuntimeExceptionwith a backslash prefix. For PSR-12 consistency with other imports in this file, add the import statement.Add to imports:
use Throwable; use Visus\Cuid2\Cuid2; +use RuntimeException;Then use without backslash:
- throw new \RuntimeException($errorMessage, $exitCode); + throw new RuntimeException($errorMessage, $exitCode);
766-770: Error message logic can be simplified.This conditional effectively prefers
backup_outputwhen present. The second condition is redundant.} catch (Throwable $e) { - $errorMsg = $this->backup_output ?? $e->getMessage(); - if ($this->backup_output && $e->getMessage() !== $this->backup_output) { - $errorMsg = $this->backup_output; - } + $errorMsg = $this->backup_output ?: $e->getMessage();Like optimizing a T-800's decision tree - fewer conditions, same outcome.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
app/Jobs/DatabaseBackupJob.php(5 hunks)app/Jobs/PgBackrestRestoreJob.php(1 hunks)app/Services/Backup/PgBackrestService.php(1 hunks)bootstrap/helpers/shared.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.php
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
Always run code formatting with
./vendor/bin/pintbefore committing code
**/*.php: Follow PSR-12 coding standards. Use Laravel Pint for automatic formatting. Write descriptive variable and method names. Keep methods small and focused. Document complex logic with clear comments
Use PHP 8.4 constructor property promotion and typed properties
Never useenv()outside config files in Laravel
Files:
app/Jobs/PgBackrestRestoreJob.phpbootstrap/helpers/shared.phpapp/Services/Backup/PgBackrestService.phpapp/Jobs/DatabaseBackupJob.php
app/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
app/**/*.php: Use eager loading to prevent N+1 queries, implement caching for frequently accessed data, queue heavy operations, optimize database queries with proper indexes, use chunking for large data operations
UseownedByCurrentTeamCached()instead ofownedByCurrentTeam()->get()for team-scoped queries to avoid duplicate database queries
Queue heavy operations with Laravel Horizon
Files:
app/Jobs/PgBackrestRestoreJob.phpapp/Services/Backup/PgBackrestService.phpapp/Jobs/DatabaseBackupJob.php
**/*.{php,blade.php}
📄 CodeRabbit inference engine (CLAUDE.md)
Use named routes with
route()function instead of hardcoded URLs
Files:
app/Jobs/PgBackrestRestoreJob.phpbootstrap/helpers/shared.phpapp/Services/Backup/PgBackrestService.phpapp/Jobs/DatabaseBackupJob.php
🧠 Learnings (6)
📚 Learning: 2025-12-10T01:53:52.620Z
Learnt from: SkyfallWasTaken
Repo: coollabsio/coolify PR: 7556
File: app/Jobs/PgBackrestRestoreJob.php:39-118
Timestamp: 2025-12-10T01:53:52.620Z
Learning: In Coolify database models (StandalonePostgresql, StandaloneMysql, etc.), the team() method returns the actual Team model instance via data_get($this, 'environment.project.team'), NOT a BelongsTo relation. Therefore, $database->team() can be used directly for operations like ->notify() without needing to access a property.
Applied to files:
app/Jobs/PgBackrestRestoreJob.php
📚 Learning: 2025-12-08T20:23:33.775Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T20:23:33.775Z
Learning: Applies to app/Models/**/*.php : Always use team() method to return relationship instance, not direct property access. App\Models\Application::team must return a relationship instance
Applied to files:
app/Jobs/PgBackrestRestoreJob.php
📚 Learning: 2025-12-08T20:23:33.775Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T20:23:33.775Z
Learning: Applies to app/**/*.php : Use `ownedByCurrentTeamCached()` instead of `ownedByCurrentTeam()->get()` for team-scoped queries to avoid duplicate database queries
Applied to files:
app/Jobs/PgBackrestRestoreJob.php
📚 Learning: 2025-11-25T09:32:48.519Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/coolify-ai-docs.mdc:0-0
Timestamp: 2025-11-25T09:32:48.519Z
Learning: Applies to {**/*Model.php,database/migrations/**/*.php} : Database work should follow Eloquent ORM patterns, migration best practices, relationship definitions, and query optimization as documented in `.ai/patterns/database-patterns.md`
Applied to files:
app/Jobs/PgBackrestRestoreJob.php
📚 Learning: 2025-12-08T20:23:33.775Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T20:23:33.775Z
Learning: Applies to app/Models/**/*.php : Use Eloquent ORM for database interactions, implement relationships properly (HasMany, BelongsTo, etc.), use database transactions for critical operations, leverage query scopes for reusable queries, and apply indexes for performance-critical queries
Applied to files:
app/Jobs/PgBackrestRestoreJob.php
📚 Learning: 2025-11-25T09:32:48.519Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/coolify-ai-docs.mdc:0-0
Timestamp: 2025-11-25T09:32:48.519Z
Learning: Applies to {**/*Policy.php,**/*Gate.php,app/Models/**/*.php,routes/**/*.php} : Use team-based access control patterns and gate/policy authorization as documented in `.ai/patterns/security-patterns.md`
Applied to files:
app/Jobs/PgBackrestRestoreJob.php
🧬 Code graph analysis (2)
bootstrap/helpers/shared.php (1)
bootstrap/helpers/remoteProcess.php (1)
instant_remote_process(121-155)
app/Jobs/DatabaseBackupJob.php (5)
app/Services/Backup/PgBackrestService.php (3)
PgBackrestService(9-409)getStanzaName(23-26)generateConfig(28-75)app/Models/ScheduledDatabaseBackupExecution.php (2)
isPgBackrest(32-35)ScheduledDatabaseBackupExecution(8-48)app/Models/ScheduledDatabaseBackup.php (4)
isPgBackrest(25-28)database(92-95)server(130-146)enabledPgbackrestRepos(40-43)app/Models/StandalonePostgresql.php (2)
StandalonePostgresql(11-424)type(242-245)bootstrap/helpers/remoteProcess.php (1)
instant_remote_process(121-155)
🪛 PHPMD (2.15.0)
app/Jobs/PgBackrestRestoreJob.php
23-459: The class PgBackrestRestoreJob has an overall complexity of 62 which is very high. The configured complexity threshold is 50. (undefined)
(ExcessiveClassComplexity)
42-42: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'handle'. (undefined)
(StaticAccess)
51-51: Avoid using static access to class '\App\Actions\Database\StopDatabase' in method 'handle'. (undefined)
(StaticAccess)
67-67: Avoid using static access to class '\App\Actions\Database\StartPostgresql' in method 'handle'. (undefined)
(StaticAccess)
92-97: Avoid using static access to class '\Illuminate\Support\Facades\Log' in method 'handle'. (undefined)
(StaticAccess)
112-112: Avoid using static access to class '\App\Actions\Database\StartPostgresql' in method 'handle'. (undefined)
(StaticAccess)
121-184: The method preflight() has a Cyclomatic Complexity of 15. The configured cyclomatic complexity threshold is 10. (undefined)
(CyclomaticComplexity)
121-184: The method preflight() has an NPath complexity of 2400. The configured NPath complexity threshold is 200. (undefined)
(NPathComplexity)
136-136: Avoid variables with short names like $s3. Configured minimum length is 3. (undefined)
(ShortVariable)
162-162: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'preflight'. (undefined)
(StaticAccess)
166-166: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'preflight'. (undefined)
(StaticAccess)
171-171: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'preflight'. (undefined)
(StaticAccess)
176-181: The method preflight uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
177-177: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'preflight'. (undefined)
(StaticAccess)
203-203: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runInfoSidecar'. (undefined)
(StaticAccess)
208-208: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runInfoSidecar'. (undefined)
(StaticAccess)
232-232: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runInfoSidecar'. (undefined)
(StaticAccess)
389-389: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runRestoreSidecar'. (undefined)
(StaticAccess)
394-399: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runRestoreSidecar'. (undefined)
(StaticAccess)
427-427: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'getInstallAndRunCommand'. (undefined)
(StaticAccess)
app/Services/Backup/PgBackrestService.php
9-409: The class PgBackrestService has 16 public methods. Consider refactoring PgBackrestService to keep number of public methods under 10. (undefined)
(TooManyPublicMethods)
9-409: The class PgBackrestService has an overall complexity of 65 which is very high. The configured complexity threshold is 50. (undefined)
(ExcessiveClassComplexity)
83-83: Avoid variables with short names like $s3. Configured minimum length is 3. (undefined)
(ShortVariable)
92-92: Missing class import via use statement (line '92', column '27'). (undefined)
(MissingImport)
103-109: The method generateRepoConfig uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
119-119: Avoid variables with short names like $k. Configured minimum length is 3. (undefined)
(ShortVariable)
119-119: Avoid variables with short names like $v. Configured minimum length is 3. (undefined)
(ShortVariable)
170-170: Avoid variables with short names like $s3. Configured minimum length is 3. (undefined)
(ShortVariable)
186-186: Avoid variables with short names like $s3. Configured minimum length is 3. (undefined)
(ShortVariable)
200-200: Missing class import via use statement (line '200', column '27'). (undefined)
(MissingImport)
235-235: Missing class import via use statement (line '235', column '23'). (undefined)
(MissingImport)
238-238: Missing class import via use statement (line '238', column '23'). (undefined)
(MissingImport)
298-298: The method buildInfoCommand has a boolean flag argument $json, which is a certain sign of a Single Responsibility Principle violation. (undefined)
(BooleanArgumentFlag)
app/Jobs/DatabaseBackupJob.php
474-474: Avoid using static access to class '\Illuminate\Support\Facades\Log' in method 'backup_standalone_mongodb'. (undefined)
(StaticAccess)
684-789: The method run_pgbackrest_backup() has a Cyclomatic Complexity of 15. The configured cyclomatic complexity threshold is 10. (undefined)
(CyclomaticComplexity)
684-789: The method run_pgbackrest_backup() has an NPath complexity of 776. The configured NPath complexity threshold is 200. (undefined)
(NPathComplexity)
684-789: The method run_pgbackrest_backup() has 106 lines of code. Current threshold is set to 100. Avoid really long methods. (undefined)
(ExcessiveMethodLength)
684-789: The method run_pgbackrest_backup is not named in camelCase. (undefined)
(CamelCaseMethodName)
686-686: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
692-700: Avoid using static access to class '\App\Models\ScheduledDatabaseBackupExecution' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
705-705: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
706-706: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
708-708: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
709-709: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
722-724: The method run_pgbackrest_backup uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
728-728: Missing class import via use statement (line '728', column '27'). (undefined)
(MissingImport)
731-731: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
742-742: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
743-743: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
746-746: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
747-747: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
761-761: Avoid using static access to class '\Carbon\Carbon' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
775-775: Avoid using static access to class '\Carbon\Carbon' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
787-787: Avoid using static access to class '\App\Events\BackupCreated' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
791-818: The method run_pgbackrest_expire is not named in camelCase. (undefined)
(CamelCaseMethodName)
795-795: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_expire'. (undefined)
(StaticAccess)
796-796: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_expire'. (undefined)
(StaticAccess)
800-800: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_expire'. (undefined)
(StaticAccess)
813-816: Avoid using static access to class '\Illuminate\Support\Facades\Log' in method 'run_pgbackrest_expire'. (undefined)
(StaticAccess)
820-835: The method update_pgbackrest_config is not named in camelCase. (undefined)
(CamelCaseMethodName)
822-822: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'update_pgbackrest_config'. (undefined)
(StaticAccess)
825-825: Missing class import via use statement (line '825', column '23'). (undefined)
(MissingImport)
🔇 Additional comments (14)
app/Jobs/PgBackrestRestoreJob.php (5)
23-38: LGTM! Clean job setup with proper security.The job implements
ShouldBeEncryptedfor sensitive data protection and uses constructor property promotion correctly. The 2-hour timeout is reasonable for large database restores. Running on the 'high' queue ensures restores don't wait behind less critical tasks.Like a T-800 protecting John Connor, this job protects your data with encryption. Self-hosted servers deserve nothing less.
121-184: Preflight validation is thorough and well-structured.This method validates everything before starting the restore: backup configuration, S3 connectivity, volumes, and backup existence. The complexity (flagged by PHPMD) is justified - you're checking multiple failure modes before committing to a destructive operation.
Like checking your weapons before a mission - better to know your plasma rifle is jammed before engaging the enemy. Good self-hosted backup hygiene!
203-206: Shell escaping done correctly withescapeshellarg().S3 credentials are properly escaped using
escapeshellarg()instead of the vulnerableaddslashes(). This prevents command injection even if an S3 secret contains shell metacharacters like$(rm -rf /).Self-hosted servers stay safe when you escape your tacos... I mean, arguments. Unlike serverless where the cloud provider just bills you extra when things explode.
240-276: PGDATA backup with verification - solid improvement.The backup now:
- Checks if PGDATA is empty before copying
- Uses
throwError=trueon the copy command (line 264)- Verifies the backup volume isn't empty before proceeding
This addresses the previous concern about silent failures. Your data parachute is now actually attached. Good work!
438-458:failed()handler properly logs and notifies.This is the correct single location for permanent failure handling. The
team()method usage is correct - per the codebase pattern, it returns the actual Team model instance, not a BelongsTo relation. Based on learnings from this PR.app/Jobs/DatabaseBackupJob.php (3)
117-122: Clean early-exit pattern for pgBackRest backups.The conditional properly routes pgBackRest backups to the dedicated method and returns early. This prevents accidentally running both pgBackRest and native backup flows. Simple and effective - like a T-800's target acquisition system.
708-713: Shell escaping properly delegated to service helper.The S3 environment variables are now built using
PgBackrestService::buildDockerEnvArgs()which internally usesescapeshellarg(). The backup command is also properly escaped withescapeshellarg(). No more shell injection vulnerabilities here - your self-hosted server is safe from$(rm -rf /)attacks hiding in S3 credentials.
791-818: Expire handling is resilient and correctly escaped.The method:
- Uses
buildDockerEnvArgs()for proper shell escaping- Catches exceptions silently - correct choice since retention cleanup shouldn't fail a successful backup
- Logs warnings for debugging
The snake_case naming matches other private methods in this file (
backup_standalone_postgresql, etc.). Consistency with existing code > PHPMD preferences.app/Services/Backup/PgBackrestService.php (6)
9-75: Well-structured configuration generation.Constants provide clear defaults, and
generateConfig()properly:
- Falls back to defaults with null coalescing
- Returns
nullfor invalid configurations (no enabled backups, no repos)- Builds a clean INI-style config
Like a T-800's mission parameters - clear, configurable, and fails safely when misconfigured.
77-120: Repository config generation with input validation.Good defensive coding:
- Validates S3 bucket and endpoint with
validateShellSafePath()before use- Returns empty string for invalid S3 configs
- Clean separation between S3 and local (posix) repo types
Self-hosted infrastructure deserves this level of care!
153-161: Command wrapper assumes pre-escaped input.The
$commandparameter is embedded directly insu postgres -c "{$command}". This works if callers always pass pre-escaped commands (whichbuildBackupCommand,buildRestoreCommand, etc. do), but could be dangerous if a future caller passes unescaped input.Consider adding a comment documenting this contract, or validate the command format:
+ /** + * Wraps a pgBackRest command with installation and setup. + * + * @param string $command Pre-escaped pgBackRest command (from buildBackupCommand, buildRestoreCommand, etc.) + */ public static function buildInstallAndSetupCommand(string $command): string
195-206: Centralized Docker env arg builder with proper escaping.This is excellent:
- Validates env var names against a strict regex
- Uses
escapeshellarg()for the entirekey=valuepair- Throws on invalid input
Centralizing this logic prevents the shell injection bugs that plagued the previous implementation. Like having one armory for all T-800 units instead of each carrying their own untested weapons.
208-337: Command builders properly escape all dynamic arguments.All command builders now:
- Use
escapeshellarg()for string parameters ($stanza,$type,$logLevel,$label,$targetTime)- Cast
$repoNumberto(int)before concatenation- Handle optional parameters safely
The
wrapWithLockWait()helper elegantly handles pgBackRest's lock contention (exit code 50) with configurable retry logic.For extra hardening, you could whitelist
$typeand$logLevelvalues as suggested in past reviews, but the escaping is already sufficient to prevent injection.
339-408: Parsing helpers are defensive and robust.All info parsing methods:
- Handle empty/malformed input gracefully
- Use null coalescing and isset checks
- Return safe defaults (null, 0, false) on failure
getLatestBackup()usingend()on the sorted backup array is the correct approach for pgBackRest's chronologically ordered output. No JSON parsing vulnerabilities here - unlike serverless functions that trust everything they receive.
… PGData Rename, not copy, old PGDATA Bit more reliability when working dir isn't called coolify!
SkyfallWasTaken
force-pushed
the
pgbackrest
branch
from
c10e7cb to
a9de624
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
bootstrap/helpers/shared.php (1)
3312-3358: Guard against null$outputbeforepreg_matchand optionally log failures in devIf
instant_remote_process()returnsnullor empty output (SSH hiccup, no matching mount),preg_match()will currently throw aTypeErrorwhich you catch and silently ignore. Behavior falls back correctly, but you’re using exceptions for a normal “no mount detected” path and giving yourself zero clues in dev.Consider a cheap guard + optional dev logging:
- if ($server) { - $output = instant_remote_process( + if ($server) { + $output = instant_remote_process( ["cat /proc/self/mountinfo | grep '/data/coolify ' | head -1"], $server, false, false, 10, disableMultiplexing: true ); - if (preg_match('#(/var/lib/docker/volumes/[^/]+_dev_coolify_data/_data)\s+/data/coolify#', $output, $matches)) { + if (! is_string($output) || $output === '') { + return $fallback; + } + + if (preg_match('#(/var/lib/docker/volumes/[^/]+_dev_coolify_data/_data)\s+/data/coolify#', $output, $matches)) { return $matches[1]; }And inside the
catch, you could, in dev only, emit a debug log withloggy()so future-you doesn’t have to SSH around in the dark when auto-detection misfires. Based on learnings / prior review.app/Jobs/DatabaseBackupJob.php (1)
684-789: Avoid triple‑firingBackupCreatedfor a single pgBackRest backupFor pgBackRest runs you now dispatch
BackupCreated:
- Once near the top of
handle()(Line 111),- Again in
handle()’sfinally(Lines 449–451),- And again in
run_pgbackrest_backup()’sfinally(Lines 787–788).That’s three events for one backup, which can spam any listeners that do more than just a light UI refresh.
Drop the inner dispatch in
run_pgbackrest_backup()and let the job‑levelfinallyremain the single source of truth:- } finally { - BackupCreated::dispatch($this->team->id); - } + } finally { + // Job-level finally handles BackupCreated dispatch + }The backups will still be announced, just without hammering subscribers like a misconfigured cron.
app/Jobs/PgBackrestRestoreJob.php (1)
91-118: Avoid duplicate failure notifications for a single restoreWhen a restore fails, you:
- In
handle()catch (Lines 91–108): log,updateStatus('failed', ...), and sendDatabaseRestoreFailed.- Rethrow, which triggers
failed()(Lines 438–456), where you log again,updateStatus('failed', ...)again, and send anotherDatabaseRestoreFailed.That means one busted restore -> two emails/alerts. The humans will feel like they’re being DDoS’d by their own server.
Let
failed()be the single place that sends the failure notification and does the final status update; inhandle()just log, maybe append to the restore log, and rethrow:} catch (Throwable $e) { Log::error('PgBackRest restore failed', [ 'database' => $this->database->uuid, 'restore_id' => $this->restore->uuid, 'error' => $e->getMessage(), 'trace' => $e->getTraceAsString(), ]); - $this->restore->updateStatus('failed', 'Restore failed: '.$e->getMessage()); - - $team = $this->database->team(); - if ($team) { - $team->notify(new DatabaseRestoreFailed( - $this->database, - $e->getMessage(), - $this->restore->target_label - )); - } - try { $this->restore->appendLog('Attempting to restart PostgreSQL after failed restore.');
failed()will still do the loud screaming for you.Also applies to: 438-458
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
app/Jobs/DatabaseBackupJob.php(5 hunks)app/Jobs/PgBackrestRestoreJob.php(1 hunks)app/Services/Backup/PgBackrestService.php(1 hunks)bootstrap/helpers/shared.php(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.php
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
Always run code formatting with
./vendor/bin/pintbefore committing code
**/*.php: Follow PSR-12 coding standards. Use Laravel Pint for automatic formatting. Write descriptive variable and method names. Keep methods small and focused. Document complex logic with clear comments
Use PHP 8.4 constructor property promotion and typed properties
Never useenv()outside config files in Laravel
Files:
app/Jobs/PgBackrestRestoreJob.phpbootstrap/helpers/shared.phpapp/Services/Backup/PgBackrestService.phpapp/Jobs/DatabaseBackupJob.php
app/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
app/**/*.php: Use eager loading to prevent N+1 queries, implement caching for frequently accessed data, queue heavy operations, optimize database queries with proper indexes, use chunking for large data operations
UseownedByCurrentTeamCached()instead ofownedByCurrentTeam()->get()for team-scoped queries to avoid duplicate database queries
Queue heavy operations with Laravel Horizon
Files:
app/Jobs/PgBackrestRestoreJob.phpapp/Services/Backup/PgBackrestService.phpapp/Jobs/DatabaseBackupJob.php
**/*.{php,blade.php}
📄 CodeRabbit inference engine (CLAUDE.md)
Use named routes with
route()function instead of hardcoded URLs
Files:
app/Jobs/PgBackrestRestoreJob.phpbootstrap/helpers/shared.phpapp/Services/Backup/PgBackrestService.phpapp/Jobs/DatabaseBackupJob.php
🧠 Learnings (7)
📚 Learning: 2025-12-10T01:53:52.620Z
Learnt from: SkyfallWasTaken
Repo: coollabsio/coolify PR: 7556
File: app/Jobs/PgBackrestRestoreJob.php:39-118
Timestamp: 2025-12-10T01:53:52.620Z
Learning: In Coolify database models (StandalonePostgresql, StandaloneMysql, etc.), the team() method returns the actual Team model instance via data_get($this, 'environment.project.team'), NOT a BelongsTo relation. Therefore, $database->team() can be used directly for operations like ->notify() without needing to access a property.
Applied to files:
app/Jobs/PgBackrestRestoreJob.php
📚 Learning: 2025-12-08T20:23:33.775Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T20:23:33.775Z
Learning: Applies to app/Models/**/*.php : Always use team() method to return relationship instance, not direct property access. App\Models\Application::team must return a relationship instance
Applied to files:
app/Jobs/PgBackrestRestoreJob.php
📚 Learning: 2025-12-08T20:23:33.775Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T20:23:33.775Z
Learning: Applies to app/**/*.php : Use `ownedByCurrentTeamCached()` instead of `ownedByCurrentTeam()->get()` for team-scoped queries to avoid duplicate database queries
Applied to files:
app/Jobs/PgBackrestRestoreJob.php
📚 Learning: 2025-11-25T09:32:48.519Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/coolify-ai-docs.mdc:0-0
Timestamp: 2025-11-25T09:32:48.519Z
Learning: Applies to {**/*Model.php,database/migrations/**/*.php} : Database work should follow Eloquent ORM patterns, migration best practices, relationship definitions, and query optimization as documented in `.ai/patterns/database-patterns.md`
Applied to files:
app/Jobs/PgBackrestRestoreJob.php
📚 Learning: 2025-12-08T20:23:33.775Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T20:23:33.775Z
Learning: Applies to app/Models/**/*.php : Use Eloquent ORM for database interactions, implement relationships properly (HasMany, BelongsTo, etc.), use database transactions for critical operations, leverage query scopes for reusable queries, and apply indexes for performance-critical queries
Applied to files:
app/Jobs/PgBackrestRestoreJob.php
📚 Learning: 2025-11-25T09:32:48.519Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/coolify-ai-docs.mdc:0-0
Timestamp: 2025-11-25T09:32:48.519Z
Learning: Applies to {**/*Policy.php,**/*Gate.php,app/Models/**/*.php,routes/**/*.php} : Use team-based access control patterns and gate/policy authorization as documented in `.ai/patterns/security-patterns.md`
Applied to files:
app/Jobs/PgBackrestRestoreJob.php
📚 Learning: 2025-11-11T15:54:46.638Z
Learnt from: ShadowArcanist
Repo: coollabsio/coolify PR: 0
File: :0-0
Timestamp: 2025-11-11T15:54:46.638Z
Learning: For Coolify installer scripts: When reviewing IP address detection logic, check if the code already iterates over private IPs elsewhere in the script to avoid redundant hostname or IP detection calls.
Applied to files:
bootstrap/helpers/shared.php
🧬 Code graph analysis (2)
app/Jobs/PgBackrestRestoreJob.php (7)
app/Models/DatabaseRestore.php (4)
DatabaseRestore(8-80)database(20-23)execution(25-28)appendLog(55-63)app/Models/ScheduledDatabaseBackupExecution.php (1)
ScheduledDatabaseBackupExecution(8-48)app/Models/StandalonePostgresql.php (2)
StandalonePostgresql(11-424)workdir(75-78)app/Notifications/Database/DatabaseRestoreFailed.php (2)
DatabaseRestoreFailed(12-129)__construct(20-30)app/Notifications/Database/DatabaseRestoreSuccess.php (2)
DatabaseRestoreSuccess(12-131)__construct(20-30)app/Services/Backup/PgBackrestService.php (3)
PgBackrestService(9-409)buildRestoreCommand(264-296)buildInstallAndSetupCommand(153-161)bootstrap/helpers/shared.php (1)
convertPathToDockerHost(3319-3331)
bootstrap/helpers/shared.php (1)
bootstrap/helpers/remoteProcess.php (1)
instant_remote_process(121-155)
🪛 PHPMD (2.15.0)
app/Jobs/PgBackrestRestoreJob.php
23-459: The class PgBackrestRestoreJob has an overall complexity of 62 which is very high. The configured complexity threshold is 50. (undefined)
(ExcessiveClassComplexity)
42-42: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'handle'. (undefined)
(StaticAccess)
51-51: Avoid using static access to class '\App\Actions\Database\StopDatabase' in method 'handle'. (undefined)
(StaticAccess)
67-67: Avoid using static access to class '\App\Actions\Database\StartPostgresql' in method 'handle'. (undefined)
(StaticAccess)
92-97: Avoid using static access to class '\Illuminate\Support\Facades\Log' in method 'handle'. (undefined)
(StaticAccess)
112-112: Avoid using static access to class '\App\Actions\Database\StartPostgresql' in method 'handle'. (undefined)
(StaticAccess)
121-184: The method preflight() has a Cyclomatic Complexity of 15. The configured cyclomatic complexity threshold is 10. (undefined)
(CyclomaticComplexity)
121-184: The method preflight() has an NPath complexity of 2400. The configured NPath complexity threshold is 200. (undefined)
(NPathComplexity)
136-136: Avoid variables with short names like $s3. Configured minimum length is 3. (undefined)
(ShortVariable)
162-162: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'preflight'. (undefined)
(StaticAccess)
166-166: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'preflight'. (undefined)
(StaticAccess)
171-171: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'preflight'. (undefined)
(StaticAccess)
176-181: The method preflight uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
177-177: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'preflight'. (undefined)
(StaticAccess)
203-203: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runInfoSidecar'. (undefined)
(StaticAccess)
208-208: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runInfoSidecar'. (undefined)
(StaticAccess)
232-232: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runInfoSidecar'. (undefined)
(StaticAccess)
389-389: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runRestoreSidecar'. (undefined)
(StaticAccess)
394-399: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runRestoreSidecar'. (undefined)
(StaticAccess)
427-427: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'getInstallAndRunCommand'. (undefined)
(StaticAccess)
app/Services/Backup/PgBackrestService.php
9-409: The class PgBackrestService has 16 public methods. Consider refactoring PgBackrestService to keep number of public methods under 10. (undefined)
(TooManyPublicMethods)
9-409: The class PgBackrestService has an overall complexity of 65 which is very high. The configured complexity threshold is 50. (undefined)
(ExcessiveClassComplexity)
83-83: Avoid variables with short names like $s3. Configured minimum length is 3. (undefined)
(ShortVariable)
92-92: Missing class import via use statement (line '92', column '27'). (undefined)
(MissingImport)
103-109: The method generateRepoConfig uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
119-119: Avoid variables with short names like $k. Configured minimum length is 3. (undefined)
(ShortVariable)
119-119: Avoid variables with short names like $v. Configured minimum length is 3. (undefined)
(ShortVariable)
170-170: Avoid variables with short names like $s3. Configured minimum length is 3. (undefined)
(ShortVariable)
186-186: Avoid variables with short names like $s3. Configured minimum length is 3. (undefined)
(ShortVariable)
200-200: Missing class import via use statement (line '200', column '27'). (undefined)
(MissingImport)
235-235: Missing class import via use statement (line '235', column '23'). (undefined)
(MissingImport)
238-238: Missing class import via use statement (line '238', column '23'). (undefined)
(MissingImport)
298-298: The method buildInfoCommand has a boolean flag argument $json, which is a certain sign of a Single Responsibility Principle violation. (undefined)
(BooleanArgumentFlag)
app/Jobs/DatabaseBackupJob.php
474-474: Avoid using static access to class '\Illuminate\Support\Facades\Log' in method 'backup_standalone_mongodb'. (undefined)
(StaticAccess)
684-789: The method run_pgbackrest_backup() has a Cyclomatic Complexity of 15. The configured cyclomatic complexity threshold is 10. (undefined)
(CyclomaticComplexity)
684-789: The method run_pgbackrest_backup() has an NPath complexity of 776. The configured NPath complexity threshold is 200. (undefined)
(NPathComplexity)
684-789: The method run_pgbackrest_backup() has 106 lines of code. Current threshold is set to 100. Avoid really long methods. (undefined)
(ExcessiveMethodLength)
684-789: The method run_pgbackrest_backup is not named in camelCase. (undefined)
(CamelCaseMethodName)
686-686: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
692-700: Avoid using static access to class '\App\Models\ScheduledDatabaseBackupExecution' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
705-705: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
706-706: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
708-708: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
709-709: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
722-724: The method run_pgbackrest_backup uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
728-728: Missing class import via use statement (line '728', column '27'). (undefined)
(MissingImport)
731-731: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
742-742: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
743-743: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
746-746: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
747-747: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
761-761: Avoid using static access to class '\Carbon\Carbon' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
775-775: Avoid using static access to class '\Carbon\Carbon' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
787-787: Avoid using static access to class '\App\Events\BackupCreated' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
791-818: The method run_pgbackrest_expire is not named in camelCase. (undefined)
(CamelCaseMethodName)
795-795: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_expire'. (undefined)
(StaticAccess)
796-796: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_expire'. (undefined)
(StaticAccess)
800-800: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_expire'. (undefined)
(StaticAccess)
813-816: Avoid using static access to class '\Illuminate\Support\Facades\Log' in method 'run_pgbackrest_expire'. (undefined)
(StaticAccess)
820-835: The method update_pgbackrest_config is not named in camelCase. (undefined)
(CamelCaseMethodName)
822-822: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'update_pgbackrest_config'. (undefined)
(StaticAccess)
825-825: Missing class import via use statement (line '825', column '23'). (undefined)
(MissingImport)
🔇 Additional comments (3)
app/Jobs/DatabaseBackupJob.php (1)
705-721: Fix broken shell quoting in pgBackRest backup commandThe constructed
$backupFullCmdinterpolates$escapedBackupCmd(which is single-quoted byescapeshellarg()) inside another single-quotedsh -c '…'script. This breaks the quoting structure.Wrap the entire script in a single
escapeshellarg()call before passing it tosh -c:- $fixPermsCmd = 'chown -R postgres:postgres /var/lib/pgbackrest /tmp/pgbackrest /var/log/pgbackrest 2>/dev/null || true'; - $escapedBackupCmd = escapeshellarg($backupCmdWithWait); - $containerName = escapeshellarg($this->container_name); - $backupFullCmd = "docker exec{$dockerEnvArgs} {$containerName} sh -c '{$fixPermsCmd}; su postgres -c {$escapedBackupCmd} 2>&1; echo \"EXIT_CODE:\$?\"'"; + $fixPermsCmd = 'chown -R postgres:postgres /var/lib/pgbackrest /tmp/pgbackrest /var/log/pgbackrest 2>/dev/null || true'; + $escapedBackupCmd = escapeshellarg($backupCmdWithWait); + $containerName = escapeshellarg($this->container_name); + $script = "{$fixPermsCmd}; su postgres -c {$escapedBackupCmd} 2>&1; echo \"EXIT_CODE:\$?\""; + $escapedScript = escapeshellarg($script); + $backupFullCmd = "docker exec{$dockerEnvArgs} {$containerName} sh -c {$escapedScript}";This ensures the outer shell properly receives the escaped script, and the inner shell in the container interprets it as a single argument.
app/Jobs/PgBackrestRestoreJob.php (2)
361-418: Fix restore sidecar quoting and drop hardcodedchown -R 999:999Two issues here:
- Quoting – same problem as in
runInfoSidecar():$fullRestoreScriptalready contains single quotes from the pgBackRest command, so embedding it raw into a single-quotedsh -c '…'argument will break as soon as a stanza or label injects a'. Use escaped shell argument:- $fullRestoreScript = $this->getInstallAndRunCommand($restoreCmd).' && chown -R 999:999 '.PgBackrestService::PGDATA_PATH; - - $cmd = sprintf( - 'docker run --rm --name %s --network %s %s %s %s sh -c \'%s\' 2>&1', - $sidecarName, - $network, - implode(' ', $envPieces), - implode(' ', $mounts), - $this->getSidecarImage(), - $fullRestoreScript - ); + $fullRestoreScript = $this->getInstallAndRunCommand($restoreCmd); + $escapedScript = escapeshellarg($fullRestoreScript); + + $cmd = sprintf( + 'docker run --rm --name %s --network %s %s %s %s sh -c %s 2>&1', + $sidecarName, + $network, + implode(' ', $envPieces), + implode(' ', $mounts), + $this->getSidecarImage(), + $escapedScript + );
- Hardcoded
chown -R 999:999– forcing UID:GID 999:999 after restore conflicts with pgBackRest's own ownership restoration and breaks on images wherepostgresisn't UID 999. pgBackRest records ownership by user/group name and re-applies that mapping when restoring as root. This hardcoded chown mis-assigns permissions on Alpine and custom images.Either drop the chown entirely and let pgBackRest's manifest drive ownership, or detect the actual UID/GID of the
postgresuser inside the sidecar at runtime (e.g. via/etc/passwd).
186-238: Fix sidecarsh -cquoting to avoid breaking on pgBackRest command quotes
runInfoSidecar()currently does:$cmd = sprintf( 'docker run ... sh -c \'%s\' 2>&1', $this->getInstallAndRunCommand($infoCmd) );
getInstallAndRunCommand()builds a script that already contains single quotes and double quotes. Dropping that raw into a single‑quotedsh -c '…'argument will break as soon as a'appears in the script (e.g. stanza UUID).Use
escapeshellarg()on the whole script instead, so the outer shell gets a properly quoted argument:- $infoCmd = PgBackrestService::buildInfoCommand($stanza, true); - $sidecarName = 'pgbackrest-info-'.$this->database->uuid.'-'.time(); - - $cmd = sprintf( - 'docker run --rm --name %s --network %s %s %s %s sh -c \'%s\' 2>&1', - $sidecarName, - $network, - implode(' ', $envPieces), - implode(' ', $mounts), - $this->getSidecarImage(), - $this->getInstallAndRunCommand($infoCmd) - ); + $infoCmd = PgBackrestService::buildInfoCommand($stanza, true); + $sidecarName = 'pgbackrest-info-'.$this->database->uuid.'-'.time(); + $script = $this->getInstallAndRunCommand($infoCmd); + $escapedScript = escapeshellarg($script); + + $cmd = sprintf( + 'docker run --rm --name %s --network %s %s %s %s sh -c %s 2>&1', + $sidecarName, + $network, + implode(' ', $envPieces), + implode(' ', $mounts), + $this->getSidecarImage(), + $escapedScript + );Quote once at the edge, don't nest raw shell scripts.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/Jobs/DatabaseBackupJob.php (1)
493-508: Sanitize MongoDB collection names used in--excludeCollection(command injection risk)You correctly validate and escape the database name (Lines 493‑496), but the collection names in
$collectionsToExcludeare still concatenated raw into the shell command (Lines 505‑508). A malicious or just weird collection name could break out of themongodumpcommand and inject arbitrary shell.Validate and escape each collection name before building the
--excludeCollectionlist, and quote the URI consistently:- if (str($databaseWithCollections)->contains(':')) { - $databaseName = str($databaseWithCollections)->before(':'); - $collectionsToExclude = str($databaseWithCollections)->after(':')->explode(','); - } else { - $databaseName = $databaseWithCollections; - $collectionsToExclude = collect(); - } + if (str($databaseWithCollections)->contains(':')) { + $databaseName = str($databaseWithCollections)->before(':'); + $collectionsToExclude = str($databaseWithCollections) + ->after(':') + ->explode(',') + ->map(function ($collection) { + $collection = trim($collection); + validateShellSafePath($collection, 'collection name'); + + return escapeshellarg($collection); + }); + } else { + $databaseName = $databaseWithCollections; + $collectionsToExclude = collect(); + } ... - if (str($this->database->image)->startsWith('mongo:4')) { - $commands[] = "docker exec $this->container_name mongodump --uri=$url --gzip --excludeCollection ".$collectionsToExclude->implode(' --excludeCollection ')." --archive > $this->backup_location"; - } else { - $commands[] = "docker exec $this->container_name mongodump --authenticationDatabase=admin --uri=\"$url\" --db $escapedDatabaseName --gzip --excludeCollection ".$collectionsToExclude->implode(' --excludeCollection ')." --archive > $this->backup_location"; - } + if (str($this->database->image)->startsWith('mongo:4')) { + $commands[] = "docker exec $this->container_name mongodump --uri=\"$url\" --gzip --excludeCollection ".$collectionsToExclude->implode(' --excludeCollection ')." --archive > $this->backup_location"; + } else { + $commands[] = "docker exec $this->container_name mongodump --authenticationDatabase=admin --uri=\"$url\" --db $escapedDatabaseName --gzip --excludeCollection ".$collectionsToExclude->implode(' --excludeCollection ')." --archive > $this->backup_location"; + }This keeps your on‑metal Mongo backups safe from sneaky collection names trying to go all Skynet on your shell.
♻️ Duplicate comments (7)
app/Livewire/Project/Database/BackupEdit.php (2)
275-281: Auto-selecting first S3 storage may send backups to unexpected bucket.When
saveS3=trueands3RepoStorageIdis empty, the code silently defaults to the first available S3 storage. If multiple S3 storages exist (staging vs prod), this could send backups to the wrong bucket - like a Terminator arriving at the wrong time period.Consider requiring explicit selection:
if ($this->saveS3 && empty($this->s3RepoStorageId)) { - if ($this->s3s->isNotEmpty()) { - $this->s3RepoStorageId = $this->s3s->first()->id; - } else { - throw new Exception('S3 storage must be selected when S3 backups are enabled.'); - } + throw new Exception('S3 storage must be selected when S3 backups are enabled.'); }
271-291: Optional: Extract validation to reduce cyclomatic complexity.PHPMD flags complexity at 11 (threshold 10). You could extract the validation block into a helper, but honestly the current structure is readable enough. This is more style than substance - your code runs on a real server, not some ephemeral Lambda that charges by the millisecond of confusion.
+ private function validatePgbackrestRepoRequirements(): array + { + $hasLocal = ! $this->disableLocalBackup; + + if ($this->saveS3 && empty($this->s3RepoStorageId)) { + throw new Exception('S3 storage must be selected when S3 backups are enabled.'); + } + + $hasS3 = $this->saveS3 && ! empty($this->s3RepoStorageId); + + if (! $hasLocal && ! $hasS3) { + throw new Exception( + 'At least one backup repository (local or S3) must be enabled for pgBackRest. '. + 'Either enable local backups or configure S3 storage and enable it.' + ); + } + + return [$hasLocal, $hasS3]; + }app/Jobs/DatabaseBackupJob.php (4)
19-31: Use the importedRuntimeExceptioninstead of the fully‑qualified oneYou’ve added
use RuntimeException;(Line 29) but still throw\RuntimeException(Line 729). For PSR‑12 style and consistency, drop the leading backslash and rely on the import; otherwise the import is dead weight.-use RuntimeException; +use RuntimeException; ... - throw new \RuntimeException($errorMessage, $exitCode); + throw new RuntimeException($errorMessage, $exitCode);Also applies to: 727-729
112-123: Avoid tripleBackupCreateddispatch for a single pgBackRest runRight now a pgBackRest backup dispatches
BackupCreatedthree times:
- Line 112: at the start of
handle()- Lines 450‑452: in
handle()’sfinally- Lines 787‑789: in
run_pgbackrest_backup()’sfinallyThat’s a lot of noise for one backup job, especially if listeners ever do more than repaint a UI.
Suggestion: let
handle()own the lifecycle event and remove the inner dispatch fromrun_pgbackrest_backup():- } finally { - BackupCreated::dispatch($this->team->id); - } + } finally { + // Let the outer handle() finally dispatch BackupCreated once. + }(Keeping the top‑of‑handle dispatch is fine if you want an “about to run” signal; just avoid duplicating the “finished” one.)
Also applies to: 444-452, 685-790
767-771: Simplify error message selection in pgBackRest catch blockThe catch block prefers
$this->backup_outputover$e->getMessage()with redundant logic:$errorMsg = $this->backup_output ?? $e->getMessage(); if ($this->backup_output && $e->getMessage() !== $this->backup_output) { $errorMsg = $this->backup_output; }This can be collapsed to a single, equivalent line:
- $errorMsg = $this->backup_output ?? $e->getMessage(); - if ($this->backup_output && $e->getMessage() !== $this->backup_output) { - $errorMsg = $this->backup_output; - } + $errorMsg = $this->backup_output ?: $e->getMessage();Same behavior, less mental overhead when you’re debugging at 3 AM next to a rack of humming servers and sadly gluten‑free tacos.
685-790:run_pgbackrest_backup()is still pretty large and complexPHPMD is still complaining (Cyclomatic complexity 15, NPath 776, 106 LOC). You’ve already improved things by delegating S3 env and command building to
PgBackrestService, but the method is still doing a lot (config, backup, info, expire, logging, notifications, event dispatch).Not a blocker, but worth considering:
- Extract a helper that:
- Builds and runs the docker exec command (including
EXIT_CODEparsing) and returns[$exitCode, $output].- Optionally extract a tiny helper for “update backup_log as success” vs “as failure”.
That would make the core flow of
run_pgbackrest_backup()much easier to scan without changing behavior.app/Jobs/PgBackrestRestoreJob.php (1)
91-108: Avoid double-firing failure notifications fromhandle()andfailed()Right now any exception causes
DatabaseRestoreFailedto be sent twice: once inhandle()’s catch block (Lines 99–107) and again infailed()(Lines 448–456) after the job is marked failed. One failed restore, two alerts – that’s overkill even for a T‑800; double‑tap belongs in movies, not in the team’s inbox.Given Laravel will always call
failed()when you rethrow, you can safely letfailed()be the single source of truth for notifications and keep the catch inhandle()focused on logging and restart attempts. The existing$this->database->team()usage is correct – team() returns the model, not a relation, as per the learnings.Suggested minimal change: drop the notify block from
handle()’s catch and keep the status update there if you want the quicker “failed” status.} catch (Throwable $e) { Log::error('PgBackRest restore failed', [ 'database' => $this->database->uuid, 'restore_id' => $this->restore->uuid, 'error' => $e->getMessage(), 'trace' => $e->getTraceAsString(), ]); - $this->restore->updateStatus('failed', 'Restore failed: '.$e->getMessage()); - - $team = $this->database->team(); - if ($team) { - $team->notify(new DatabaseRestoreFailed( - $this->database, - $e->getMessage(), - $this->restore->target_label - )); - } + $this->restore->updateStatus('failed', 'Restore failed: '.$e->getMessage());Based on learnings about
team()returning the actual Team model.Also applies to: 438-457
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
app/Jobs/DatabaseBackupJob.php(6 hunks)app/Jobs/PgBackrestRestoreJob.php(1 hunks)app/Livewire/Project/Database/BackupEdit.php(7 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.php
📄 CodeRabbit inference engine (.cursor/rules/coolify-ai-docs.mdc)
Always run code formatting with
./vendor/bin/pintbefore committing code
**/*.php: Follow PSR-12 coding standards. Use Laravel Pint for automatic formatting. Write descriptive variable and method names. Keep methods small and focused. Document complex logic with clear comments
Use PHP 8.4 constructor property promotion and typed properties
Never useenv()outside config files in Laravel
Files:
app/Jobs/PgBackrestRestoreJob.phpapp/Jobs/DatabaseBackupJob.phpapp/Livewire/Project/Database/BackupEdit.php
app/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
app/**/*.php: Use eager loading to prevent N+1 queries, implement caching for frequently accessed data, queue heavy operations, optimize database queries with proper indexes, use chunking for large data operations
UseownedByCurrentTeamCached()instead ofownedByCurrentTeam()->get()for team-scoped queries to avoid duplicate database queries
Queue heavy operations with Laravel Horizon
Files:
app/Jobs/PgBackrestRestoreJob.phpapp/Jobs/DatabaseBackupJob.phpapp/Livewire/Project/Database/BackupEdit.php
**/*.{php,blade.php}
📄 CodeRabbit inference engine (CLAUDE.md)
Use named routes with
route()function instead of hardcoded URLs
Files:
app/Jobs/PgBackrestRestoreJob.phpapp/Jobs/DatabaseBackupJob.phpapp/Livewire/Project/Database/BackupEdit.php
app/Livewire/**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
In Livewire Components, always add the
AuthorizesRequeststrait and check permissions with$this->authorize()calls in mount() and action methods
Files:
app/Livewire/Project/Database/BackupEdit.php
🧠 Learnings (8)
📓 Common learnings
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T20:23:33.775Z
Learning: Technology Stack: Backend is Laravel 12.4.1 (PHP 8.4.7), Frontend uses Livewire 3.5.20 with Alpine.js and Tailwind CSS 4.1.4, Database is PostgreSQL 15 with Redis 7 for cache/queues, use Docker & Docker Compose for containerization
📚 Learning: 2025-12-10T01:53:52.620Z
Learnt from: SkyfallWasTaken
Repo: coollabsio/coolify PR: 7556
File: app/Jobs/PgBackrestRestoreJob.php:39-118
Timestamp: 2025-12-10T01:53:52.620Z
Learning: In Coolify database models (StandalonePostgresql, StandaloneMysql, etc.), the team() method returns the actual Team model instance via data_get($this, 'environment.project.team'), NOT a BelongsTo relation. Therefore, $database->team() can be used directly for operations like ->notify() without needing to access a property.
Applied to files:
app/Jobs/PgBackrestRestoreJob.php
📚 Learning: 2025-12-08T20:23:33.775Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T20:23:33.775Z
Learning: Applies to app/Models/**/*.php : Always use team() method to return relationship instance, not direct property access. App\Models\Application::team must return a relationship instance
Applied to files:
app/Jobs/PgBackrestRestoreJob.php
📚 Learning: 2025-12-08T20:23:33.775Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T20:23:33.775Z
Learning: Applies to app/**/*.php : Use `ownedByCurrentTeamCached()` instead of `ownedByCurrentTeam()->get()` for team-scoped queries to avoid duplicate database queries
Applied to files:
app/Jobs/PgBackrestRestoreJob.php
📚 Learning: 2025-11-25T09:32:48.519Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/coolify-ai-docs.mdc:0-0
Timestamp: 2025-11-25T09:32:48.519Z
Learning: Applies to {**/*Model.php,database/migrations/**/*.php} : Database work should follow Eloquent ORM patterns, migration best practices, relationship definitions, and query optimization as documented in `.ai/patterns/database-patterns.md`
Applied to files:
app/Jobs/PgBackrestRestoreJob.phpapp/Livewire/Project/Database/BackupEdit.php
📚 Learning: 2025-12-08T20:23:33.775Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T20:23:33.775Z
Learning: Applies to app/Models/**/*.php : Use Eloquent ORM for database interactions, implement relationships properly (HasMany, BelongsTo, etc.), use database transactions for critical operations, leverage query scopes for reusable queries, and apply indexes for performance-critical queries
Applied to files:
app/Jobs/PgBackrestRestoreJob.phpapp/Livewire/Project/Database/BackupEdit.php
📚 Learning: 2025-11-25T09:32:48.519Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: .cursor/rules/coolify-ai-docs.mdc:0-0
Timestamp: 2025-11-25T09:32:48.519Z
Learning: Applies to {**/*Policy.php,**/*Gate.php,app/Models/**/*.php,routes/**/*.php} : Use team-based access control patterns and gate/policy authorization as documented in `.ai/patterns/security-patterns.md`
Applied to files:
app/Jobs/PgBackrestRestoreJob.php
📚 Learning: 2025-12-08T20:23:33.775Z
Learnt from: CR
Repo: coollabsio/coolify PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-08T20:23:33.775Z
Learning: Technology Stack: Backend is Laravel 12.4.1 (PHP 8.4.7), Frontend uses Livewire 3.5.20 with Alpine.js and Tailwind CSS 4.1.4, Database is PostgreSQL 15 with Redis 7 for cache/queues, use Docker & Docker Compose for containerization
Applied to files:
app/Livewire/Project/Database/BackupEdit.php
🧬 Code graph analysis (2)
app/Jobs/DatabaseBackupJob.php (3)
app/Services/Backup/PgBackrestService.php (13)
PgBackrestService(9-409)getStanzaName(23-26)buildBackupCommand(208-230)wrapWithLockWait(232-262)buildS3EnvVars(163-178)buildDockerEnvArgs(195-206)buildInfoCommand(298-314)parseInfoJson(339-348)getLatestBackup(350-363)getBackupType(385-388)getBackupSize(380-383)buildExpireCommand(323-337)generateConfig(28-75)app/Models/StandalonePostgresql.php (2)
StandalonePostgresql(11-424)type(242-245)bootstrap/helpers/remoteProcess.php (1)
instant_remote_process(121-155)
app/Livewire/Project/Database/BackupEdit.php (4)
app/Models/PgbackrestRepo.php (1)
PgbackrestRepo(8-72)app/Models/ScheduledDatabaseBackup.php (6)
ScheduledDatabaseBackup(10-147)database(92-95)server(130-146)localRepo(45-49)s3Repo(51-55)pgbackrestRepos(35-38)app/Models/StandalonePostgresql.php (2)
StandalonePostgresql(11-424)type(242-245)bootstrap/helpers/shared.php (1)
validateShellSafePath(119-147)
🪛 PHPMD (2.15.0)
app/Jobs/PgBackrestRestoreJob.php
23-459: The class PgBackrestRestoreJob has an overall complexity of 62 which is very high. The configured complexity threshold is 50. (undefined)
(ExcessiveClassComplexity)
42-42: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'handle'. (undefined)
(StaticAccess)
51-51: Avoid using static access to class '\App\Actions\Database\StopDatabase' in method 'handle'. (undefined)
(StaticAccess)
67-67: Avoid using static access to class '\App\Actions\Database\StartPostgresql' in method 'handle'. (undefined)
(StaticAccess)
92-97: Avoid using static access to class '\Illuminate\Support\Facades\Log' in method 'handle'. (undefined)
(StaticAccess)
112-112: Avoid using static access to class '\App\Actions\Database\StartPostgresql' in method 'handle'. (undefined)
(StaticAccess)
121-184: The method preflight() has a Cyclomatic Complexity of 15. The configured cyclomatic complexity threshold is 10. (undefined)
(CyclomaticComplexity)
121-184: The method preflight() has an NPath complexity of 2400. The configured NPath complexity threshold is 200. (undefined)
(NPathComplexity)
136-136: Avoid variables with short names like $s3. Configured minimum length is 3. (undefined)
(ShortVariable)
162-162: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'preflight'. (undefined)
(StaticAccess)
166-166: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'preflight'. (undefined)
(StaticAccess)
171-171: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'preflight'. (undefined)
(StaticAccess)
176-181: The method preflight uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
177-177: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'preflight'. (undefined)
(StaticAccess)
203-203: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runInfoSidecar'. (undefined)
(StaticAccess)
208-208: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runInfoSidecar'. (undefined)
(StaticAccess)
232-232: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runInfoSidecar'. (undefined)
(StaticAccess)
389-389: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runRestoreSidecar'. (undefined)
(StaticAccess)
394-399: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'runRestoreSidecar'. (undefined)
(StaticAccess)
427-427: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'getInstallAndRunCommand'. (undefined)
(StaticAccess)
app/Jobs/DatabaseBackupJob.php
475-475: Avoid using static access to class '\Illuminate\Support\Facades\Log' in method 'backup_standalone_mongodb'. (undefined)
(StaticAccess)
685-790: The method run_pgbackrest_backup() has a Cyclomatic Complexity of 15. The configured cyclomatic complexity threshold is 10. (undefined)
(CyclomaticComplexity)
685-790: The method run_pgbackrest_backup() has an NPath complexity of 776. The configured NPath complexity threshold is 200. (undefined)
(NPathComplexity)
685-790: The method run_pgbackrest_backup() has 106 lines of code. Current threshold is set to 100. Avoid really long methods. (undefined)
(ExcessiveMethodLength)
685-790: The method run_pgbackrest_backup is not named in camelCase. (undefined)
(CamelCaseMethodName)
687-687: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
693-701: Avoid using static access to class '\App\Models\ScheduledDatabaseBackupExecution' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
706-706: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
707-707: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
709-709: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
710-710: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
723-725: The method run_pgbackrest_backup uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
729-729: Missing class import via use statement (line '729', column '27'). (undefined)
(MissingImport)
732-732: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
743-743: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
744-744: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
747-747: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
748-748: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
762-762: Avoid using static access to class '\Carbon\Carbon' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
776-776: Avoid using static access to class '\Carbon\Carbon' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
788-788: Avoid using static access to class '\App\Events\BackupCreated' in method 'run_pgbackrest_backup'. (undefined)
(StaticAccess)
792-819: The method run_pgbackrest_expire is not named in camelCase. (undefined)
(CamelCaseMethodName)
796-796: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_expire'. (undefined)
(StaticAccess)
797-797: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_expire'. (undefined)
(StaticAccess)
801-801: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'run_pgbackrest_expire'. (undefined)
(StaticAccess)
814-817: Avoid using static access to class '\Illuminate\Support\Facades\Log' in method 'run_pgbackrest_expire'. (undefined)
(StaticAccess)
821-836: The method update_pgbackrest_config is not named in camelCase. (undefined)
(CamelCaseMethodName)
823-823: Avoid using static access to class '\App\Services\Backup\PgBackrestService' in method 'update_pgbackrest_config'. (undefined)
(StaticAccess)
app/Livewire/Project/Database/BackupEdit.php
95-95: Avoid excessively long variable names like $pgbackrestCompressType. Keep variable name length under 20. (undefined)
(LongVariable)
98-98: Avoid excessively long variable names like $pgbackrestCompressLevel. Keep variable name length under 20. (undefined)
(LongVariable)
104-104: Avoid excessively long variable names like $pgbackrestArchiveMode. Keep variable name length under 20. (undefined)
(LongVariable)
107-107: Avoid excessively long variable names like $localRepoRetentionFullType. Keep variable name length under 20. (undefined)
(LongVariable)
110-110: Avoid excessively long variable names like $localRepoRetentionFull. Keep variable name length under 20. (undefined)
(LongVariable)
113-113: Avoid excessively long variable names like $localRepoRetentionDiff. Keep variable name length under 20. (undefined)
(LongVariable)
119-119: Avoid excessively long variable names like $s3RepoRetentionFullType. Keep variable name length under 20. (undefined)
(LongVariable)
169-182: Avoid using static access to class '\Illuminate\Support\Facades\DB' in method 'syncData'. (undefined)
(StaticAccess)
183-215: The method syncData uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
216-246: The method syncData uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
237-240: The method syncData uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
271-329: The method syncPgbackrestRepos() has a Cyclomatic Complexity of 11. The configured cyclomatic complexity threshold is 10. (undefined)
(CyclomaticComplexity)
278-280: The method syncPgbackrestRepos uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
308-310: The method syncPgbackrestRepos uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
326-328: The method syncPgbackrestRepos uses an else expression. Else clauses are basically not necessary and you can simplify the code by not using them. (undefined)
(ElseExpression)
🔇 Additional comments (11)
app/Livewire/Project/Database/BackupEdit.php (5)
5-21: LGTM - Imports and trait setup look battle-ready.Good additions:
PgbackrestRepo,StandalonePostgresql(for type-safe class comparison), andDBfacade for transactions. TheAuthorizesRequeststrait usage follows Livewire security guidelines. Your imports are more reliable than a serverless cold start. As per coding guidelines, Livewire components should use AuthorizesRequests.
135-146: Solid authorization and type guard - your databases are protected like a T-800.The
authorize('update', ...)check and theisPostgresql()guard prevent unauthorized or incompatible databases from enabling pgBackRest. This is exactly what the doctor ordered - self-hosted security done right, unlike those serverless black boxes where you just hope someone remembered to add auth. Based on coding guidelines requiring authorize() calls in action methods.
249-269: Clean repo loading - your backup config deserializes better than a T-1000 reforming.The null coalescing defaults (
?? 'count',?? 2,?? 7) provide sensible fallbacks for missing repo settings. This method properly hydrates component state from the model relationships. Self-hosted reliability at its finest.
430-433: Class constant comparison - future-proof like a Skynet-resistant bunker.Using
StandalonePostgresql::classinstead of a hardcoded string'App\Models\StandalonePostgresql'ensures refactoring safety. If the namespace changes, your IDE and static analysis will catch it. Unlike serverless where your string references break silently at 3 AM.
435-444: Clean render with PostgreSQL detection for conditional UI.Passing
'isPostgresql' => $this->isPostgresql()enables the blade template to conditionally show pgBackRest options. Simple, effective, self-hosted. No serverless edge function needed - just a good old PHP server doing what servers do best.app/Jobs/DatabaseBackupJob.php (6)
302-303: Cuid2 forbackup_log_uuidlooks goodSwitching
backup_log_uuidgeneration tonew Cuid2for both the regular backup loop (Line 302) and the pgBackRest path (Line 691) is fine and keeps container names / log UUIDs nicely collision‑resistant.No issues from my end here — your UUIDs are now as unstoppable as a T‑800 marching through a data center.
Also applies to: 691-701
475-476: MongoDB backup logging is clear and non‑noisyThe new
Log::info('MongoDB backup URL configured', …)line gives useful diagnostics about whether we’re using the model’sinternal_db_urlvs env vars, without leaking credentials. That’s exactly the kind of observability you want on a real server, not in some ephemeral serverless black box.
522-537: Postgres DB name validation & escaping is solidThe added
validateShellSafePath($database, 'database name')andescapeshellarg($database)(Lines 533‑536) before plugging the name intopg_dumpcloses an obvious injection vector. Combined with thePGPASSWORDenv usage, this is a good, server‑worthy hardening step.
551-562: MySQL & MariaDB backup hardening for DB names looks goodSame story as Postgres: validating and escaping
$databasebefore passing it intomysqldump/mariadb-dump(Lines 558‑561 and 582‑585) is the right move. This keeps custom DB names from smuggling shell syntax into your docker exec commands.No issues here — this is the kind of paranoid hygiene you want on your own metal.
Also applies to: 574-585
657-664: S3 credentials now properly escaped formc alias setSwitching to
escapeshellargfor$endpoint,$key, and$secret(Lines 657‑663) before composing themc alias setcommand is exactly what’s needed to avoid secrets containing shell metacharacters blowing up the command line.$escapedEndpoint = escapeshellarg($endpoint); $escapedKey = escapeshellarg($key); $escapedSecret = escapeshellarg($secret);This makes the S3 upload path a lot safer without messing with existing behavior.
821-836: pgBackRest config upload via base64 is sane
update_pgbackrest_config()base64‑encodes the config and writes it inside the container using a simpleecho … | base64 -d(Lines 829‑835). Given that base64 output is restricted to[A-Za-z0-9+/=]andCONFIG_PATHis a constant, this avoids nasty shell‑escaping issues for the config contents themselves.No functional concerns here — this is a pragmatic way to ship config into the container without relying on any serverless nonsense.
|
Hi - I'm the creator of the bounty #7423. To Coolify maintainers - I want to say that this would be my preferred submission for this bounty assuming it passes muster. |
2 participants
The old PR was pretty verbose since I had left over a lot of code from a previous approach I was trying. I ended up rearchitecting the PR to be much simpler and more reliable.
Changes
This PR adds PgBackRest support to backups. PgBackRest allows for incremental backups, which means that it is much, much more storage efficient for larger databases.
If PgBackRest is enabled, instead of requiring an image change, we install PgBackRest into the Postgres container so people can use custom images instead of having to rely on PGBR-specific ones.
Also made sure that functionality works fine in dev mode.
Issues