Skip to content

fix(backups): validate S3 storage before scheduling - #10389

Merged
andrasbacsai merged 4 commits into
nextfrom
s3-backup-validation
Jun 1, 2026
Merged

fix(backups): validate S3 storage before scheduling#10389
andrasbacsai merged 4 commits into
nextfrom
s3-backup-validation

Conversation

@andrasbacsai

Copy link
Copy Markdown
Member

Summary

  • Validate selected S3 storage when creating scheduled database backups, rejecting missing or non-team-owned storage.
  • Force-disable S3 backup settings when editing an existing schedule with invalid or unavailable S3 storage, including dependent local-backup deletion settings.
  • Preserve the previous S3 storage ID in backup job error messages before clearing invalid configuration.
  • Add feature coverage for S3 validation during backup creation, editing, and job execution.

Prevent scheduled database backups from enabling S3 uploads without a valid team-owned storage configuration, and preserve the previous S3 storage ID in missing-storage error messages.

Add coverage for backup edit/create validation and S3 upload failure messaging.
@andrasbacsai

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented May 23, 2026

Copy link
Copy Markdown
Contributor
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented May 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

This PR adds S3 storage ownership and availability validation to database backup workflows. It captures the prior s3_storage_id in DatabaseBackupJob before clearing it, ensuring exception messages report the correct value when S3 storage is deleted. The BackupEdit component extends customValidate() to disable S3 if the selected storage is not in the team's available list. Similarly, CreateScheduledBackup adds a validation gate in submit() to reject S3-enabled backups without a valid team-owned S3 storage. Supporting model changes include adding team_id to S3Storage::$fillable and explicit integer casting for ScheduledDatabaseBackupExecution::size.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch s3-backup-validation

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@app/Livewire/Project/Database/CreateScheduledBackup.php`:
- Around line 52-58: In submit(), replace the stale mount-time snapshot check
that uses $this->definedS3s->contains('id', $this->s3StorageId) with a fresh DB
existence query scoped to the current team: when $this->saveToS3 and
$this->s3StorageId is set, perform an existence check (e.g., query the S3
storage model for team_id = currentTeam()->id and id = $this->s3StorageId using
exists()) and if it does not exist dispatch the same error and return; update
the logic around $this->saveToS3, $this->s3StorageId, and submit() accordingly
so runtime ownership/authorization is validated at submit time.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: cbeb2378-f9d0-4d52-9405-916c18ad84d1

📥 Commits

Reviewing files that changed from the base of the PR and between a49bc5d and a4d75ff.

📒 Files selected for processing (8)
  • app/Jobs/DatabaseBackupJob.php
  • app/Livewire/Project/Database/BackupEdit.php
  • app/Livewire/Project/Database/CreateScheduledBackup.php
  • app/Models/S3Storage.php
  • app/Models/ScheduledDatabaseBackupExecution.php
  • tests/Feature/BackupEditValidationTest.php
  • tests/Feature/CreateScheduledBackupValidationTest.php
  • tests/Feature/DatabaseBackupJobTest.php

Comment thread app/Livewire/Project/Database/CreateScheduledBackup.php
Check the selected S3 storage against the database at submit time so
stale Livewire state cannot schedule backups with storage that was
reassigned or marked unusable after the component mounted.
@andrasbacsai
andrasbacsai merged commit c2e9c51 into next Jun 1, 2026
4 checks passed
@andrasbacsai
andrasbacsai deleted the s3-backup-validation branch June 1, 2026 08:37
@andrasbacsai andrasbacsai mentioned this pull request Jun 2, 2026
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Aug 19, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant