refactor(s3): Don't hardcode endpoint and region
Don't hardcode endpoint and region for S3 buckets
There were many references to the ap-south-1 region and a global implicit reference to AWS. This PR contains refactors of the S3 bucket systems for "offsite backups", "dashboard uploads" and "Backup Bucket doctype" buckets, to avoid vendor lock-in.
I tried my best to make it backwards compatible with what I assume is the configuration on Frappe Cloud, but I can't check.
Details:
- Added region and endpoint fields in Press Settings for the "offsite backups" and "dashboard uploads" buckets
- Fields are:
offsite_backups_endpoint,remote_uploads_region,remote_uploads_endpoint
- Fields are:
- Dropped unused fields:
offsite_backups_provider(replaced by endpoint),data_40(typo probably) - There might be some agent stuff left that is not handled correctly (e.g. retrieve the ENDPOINT_URL param here)
- I did not add unit tests
~~cc: @gavindsouza :wave:~~ re: https://github.com/frappe/press/pull/19 https://github.com/frappe/press/pull/22
For the Frappe team
- There was a hardcoded
uploads.frappe.cloudURL somewhere, I just kept it unchanged, but the refactor won't be truly finished until a migration path can be found for you. - I'm not so sure about the other buckets (Backup Bucket doctype), they already had the Endpoint and Region fields, but these were not used everywhere (e.g. not in delete_s3_file)
Typical configuration values for the fields
AWS S3 (Fallback/Legacy)
- Region: leave empty for
ap-south-1 - Endpoint: leave empty for automatic endpoint generation by boto3 = AWS S3 (I believe
https://s3.amazonaws.com)
AWS S3
- Region:
ap-south-1 - Endpoint:
https://s3.ap-south-1.amazonaws.com(possibly a custom DNS name such asuploads.example.com)
OVH
- Region:
de - Endpoint:
https://storage.de.cloud.ovh.net
A patch could be written to set the value back in Press Settings if migration needs to be automatic.
- [ ] Need to migrate
boto3_offsite_backup_sessionfor audit
Codecov Report
Attention: Patch coverage is 16.66667% with 35 lines in your changes missing coverage. Please review.
Project coverage is 39.90%. Comparing base (
ee3fe50) to head (72b56a8).
| Files with missing lines | Patch % | Lines |
|---|---|---|
| press/press/doctype/remote_file/remote_file.py | 3.33% | 29 Missing :warning: |
| press/api/site.py | 0.00% | 6 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## master #2159 +/- ##
==========================================
- Coverage 39.93% 39.90% -0.03%
==========================================
Files 372 372
Lines 28210 28232 +22
==========================================
+ Hits 11266 11267 +1
- Misses 16944 16965 +21
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.