Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feature] S3 support #674

Merged
merged 9 commits into from
Jul 3, 2022
Merged

Conversation

theSuess
Copy link
Contributor

Resolves #308

This adds S3 support by abstracting the storage driver in a new package internal/storage. When accessing an asset via S3, a signed resource URL is generated and returned as an HTTP Redirect. This helps to prevent hotlinking while still reducing load on the server.

I've tested it against MinIO and will continue tests on other providers tomorrow (Backblaze and Scaleway as they offer free tiers).

I also might want to refactor the Content type into an interface to have more type safety but wanted to create this PR to get early feedback if this approach is usable

@tsmethurst
Copy link
Contributor

Gonna review this in the coming days, thank you for all your hard work!

@theSuess
Copy link
Contributor Author

FYI: I've now also verified functionality with scaleway and backblaze, so I feel confident that this supports the majority of S3 providers

Copy link
Contributor

@tsmethurst tsmethurst left a comment

Choose a reason for hiding this comment

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

Looks really good so far! Thank you for working on this, and for testing it :) It was a very pleasant surprise! Still have a few issues we need to discuss/resolve though, particularly around the interfacing and the contexts being passable-innable.

internal/api/client/fileserver/servefile.go Outdated Show resolved Hide resolved
internal/api/model/content.go Outdated Show resolved Hide resolved
internal/storage/s3.go Show resolved Hide resolved
internal/storage/local.go Show resolved Hide resolved
internal/storage/s3.go Outdated Show resolved Hide resolved
internal/storage/storage.go Show resolved Hide resolved
example/config.yaml Show resolved Hide resolved
docs/configuration/storage.md Show resolved Hide resolved
theSuess added 3 commits June 28, 2022 17:25
also adds license header to the storage package
HTTP 302 Found is the best fit, as it signifies that the resource
requested was found but not under its presumed URL

307/TemporaryRedirect would mean that this resource is usually located
here, not in this case

303/SeeOther indicates that the redirection does not link to the
requested resource but to another page
@tsmethurst
Copy link
Contributor

Alright, i discussed it with the other devs, and this looks good! So I'm gonna go ahead and merge it. Thank you so much for all your hard work! :)

@tsmethurst tsmethurst merged commit 9d0df42 into superseriousbusiness:main Jul 3, 2022
@theSuess theSuess deleted the s3-support branch July 3, 2022 10:20
@LittleFox94
Copy link
Contributor

Wow, this works great (running it on Ceph) and just the way I wanted to build it :D
Good job and many thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[feature] S3 storage for media
4 participants