[Bug]: user-media served content is broken, locally
What happened?
A path such as http://olympia.test/user-media/version-previews/full/0/1.png should work, if "storage/shared_storage/uploads/version-previews/full/0/1.png" exists, but locally it's 404'ing. It's actually available at http://olympia.test/user-media/shared_storage/uploads/version-previews/full/0/1.png, which is wrong.
What did you expect to happen?
we serve content from the urls that django generates (which are the same path structure as on dev/stage/prod).
Is there an existing issue for this?
- [X] I have searched the existing issues
┆Issue is synchronized with this Jira Task
Probable regression candidate: https://github.com/mozilla/addons-server/pull/22409
Version previews (autogenerated theme previews), Add-on previews (screenshots uploaded by the developer) and custom icons are broken that way locally.
We'll have to play with aliases I suspect. Interestingly patch that regressed this removed some aliases for xpis, but on dev/stage/prod we do use them (leaving the previews/icons alone, they work sort of out of the box)
Related https://github.com/mozilla/addons/issues/14886
I think the first step to solve this will be adding the tests I reference in #14886 this is sufficiently complex and esoteric that we should cover the configuration with tests to ensure it works as we make changes.
I think the first step to solve this will be adding the tests I reference in #14886 this is sufficiently complex and esoteric that we should cover the configuration with tests to ensure it works as we make changes.
I agree it'd be good to add some tests at the same time we fix it, assuming it doesn't balloon the scope so much it takes too long to actually fix. (Any other tests we think are needed for general docker,etc config can be added in a follow-up)
@eviljeff can you provide STR mapping sample request paths to expected host file locations? example /user-media/test.txt ./storage/test.txt (that one should work now)
But it's unclear exactly how we are creating a reliance on these non standard paths so it's hard to test my patch fixing this..
LInk to the original sin https://github.com/mozilla/addons-server/commit/bc186195bc5da7fc0f7268a11f67a5470a64c454#diff-e45e45baeda1c1e73482975a664062aa56f20c03dd9d64a827aba57775bed0d3
How are guarded-addons and sitemaps even handled? I see we had mappings for them, but they are not "saved" in the .gitkeep files.
Also why are we mapping /user-media/addons to ./storage/files... why not sync the names? then we wouldn't need special routing for that path.
We also don't seem to need special routing for guarded addons at all.. or for sitemaps.. but idk how to test sitemaps as the view logic seems a bit complicated.
guarded-addons is no longer a thing and can go away, FYI.
Files that we are referencing directly:
- Theme previews (automatically generated, attached to a
VersionthroughVersionPreview). Typical URL:/user-media/version-previews/full/3970/3970835.svg?modified=1702917049 - Add-on Previews (uploaded by developers, goes through some processing, attached to an
AddonthroughPreview). Typical URL:/user-media/previews/thumbs/238/238546.jpg?modified=1622132421 - Add-on custom icons (uploaded by developers, goes through some processing, attached to an
Addonthoughicon_type). Typical URL:/user-media/addon_icons/607/607454-64.png?modified=mcrushed - User avatars (uploaded by developers, goes through some processing, attached to an
UserProfilethroughpicture_type). Typical URL:/user-media/userpics/06/0106/10160106/10160106.png?modified=1698231186
Files that we are serving through X-Accel-Redirect (need the dir to have internal in nginx):
- XPI files (uploaded by developers, goes through some processing, attached to
FileUploadorFile). Typical URL:/firefox/downloads/file/4328681/ublock_origin-1.59.0.xpi,/uploads/file/dummy/?access_token=blah - Source files (uploaded by developers). Typical URL:
/firefox/downloads/source/123456 - Activity attachments (uploaded by reviewers). Typical URL:
/activity/attachment/4568 - Sitemap files (generated by
manage.py cron write_sitemaps, triggered by cron in dev/stage/prod). Typical URL:/sitemap.xml?section=amo
@diox do you have a mapping of the urls to the location in the storage directory? From what I can see the specific problem is that these mappings are not straight forward and so we need not only the url but the path where we expect the file to exist.
For files we are referencing directly:
- Theme previews are under
{root_storage}/shared_storage/uploads/version-previews - Add-on previews are under
{root_storage}/shared_storage/uploads/previews - Icons are under
{root_storage}/shared_storage/uploads/addon_icons - User avatars are under
{root_storage}/shared_storage/uploads/userpics
As you can see, for those the URL all map directly /user-media/ to {root_storage}/shared_storage/uploads/, nothing fancy.
For files we're serving through X-Accel-Redirect the URL should not matter at all. We just need the various dirs to be in a location that has internal set (and possibly an alias to the same dir). See https://github.com/mozilla-it/webservices-infra/blob/11890a7248ecc8f600889624bac3b61f94bd6832/amo/k8s/amo-proxy/configs/addons.conf.tpl#L70-L95 (+ https://github.com/mozilla-it/webservices-infra/pull/2824/files for attachments)
Ha oops, I think something like this should fix the issue:
diff --git a/docker/nginx/addons.conf b/docker/nginx/addons.conf
index e53f6ac26a..1d87ec43ac 100644
--- a/docker/nginx/addons.conf
+++ b/docker/nginx/addons.conf
@@ -20,7 +20,7 @@ server {
}
location /user-media/ {
- alias /srv/user-media/;
+ alias /srv/user-media/shared_storage/uploads/;
}
location ~ ^/api/ {
Hello,
I see this has the local dev env label, but I did some checks on -DEV environment to make sure everything works as expected. If there is need for any additional verification, please reach out.
Thank you!