search

spatie / laravel-backup

A package to backup your Laravel app
https://spatie.be/docs/laravel-backup
MIT License
5.64k stars 763 forks source link

Optimize storage and speed when making an encrypted backup #1825

Open pellaras opened 3 months ago

pellaras commented 3 months ago

This PR addresses two issues:

In more detail:

Using a Listener to wait for a successful creation of backup had the side effect of first creating a zip archive, unencrypted, then create the encrypted zip archive using the unencrypted one, which demanded double the available disk space, as well as more time.

Further to the above, and most concerning, is when the Listener fails to encrypt the zip file, it shows a warning about a notification failed to be sent, but it continues to store the archive on the destination disks, allowing for the unencrypted version to make it to a remote destination with possible privacy implications. A very likely scenario for this to happen, is when there is not enough disk space available for the encrypted backup to be generated, but the unencrypted one was successfully created.

This PR tackles the issue by setting the zip archive to use encryption as soon as the files are added, without the need of first making an unencrypted version on the temp folder. This also has a nice side-effect of needing the same available disk space as if no encryption was configured.

Happy to create a PR also for v9

Note: Tide-up the tests related to the encryption, as the Listener is no longer used.

flexchar commented 2 months ago

LGTM! Would you be as kind to spare a few minutes on this one @freekmurze?:)

freekmurze commented 2 months ago

Will do to, could you rebase against main to resolve the conflicts?

pellaras commented 1 month ago

@freekmurze are you asking me? If yes, I can do a PR against main, after getting this merged under v8.

Nielsvanpach commented 1 month ago

There are merge conflicts preventing this from being merged. You can rebase against the v8 branch.

pellaras commented 2 weeks ago

@freekmurze rebased

pellaras commented 5 days ago

Is there anything else that can be done to help merge this PR?

I am emphasizing the issue of having unencrypted archives make it to a remote storage (i.e. s3) under specific circumstances.