From 0266233254058fe2069c8f135040275d0b39c16d Mon Sep 17 00:00:00 2001 From: Eric Holscher Date: Sat, 14 Mar 2020 11:27:12 -0700 Subject: [PATCH 1/6] Properly proxy the Proxito headers via nginx/sendfile This is a hack, but it works. --- dockerfiles/nginx/proxito.conf | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/dockerfiles/nginx/proxito.conf b/dockerfiles/nginx/proxito.conf index 1bd2bcb9282..87750c10ad1 100644 --- a/dockerfiles/nginx/proxito.conf +++ b/dockerfiles/nginx/proxito.conf @@ -3,6 +3,7 @@ server { listen 80 default_server; server_name *.community.dev.readthedocs.io *.org.dev.readthedocs.build; + # TODO: serve the favicon.ico from the project/version itself instead location /favicon.ico { proxy_pass http://storage:10000/devstoreaccount1/static/images/favicon.ico; @@ -20,10 +21,10 @@ server { location / { proxy_pass http://proxito:8000; proxy_set_header Host $host; - proxy_intercept_errors on; - error_page 404 = @notfoundfallback; + proxy_set_header X-Real-IP $remote_addr; + proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; + proxy_set_header X-Forwarded-Host $host; - add_header X-Served Django-Proxito always; } # Sendfile support to serve the actual files that Proxito has specified @@ -31,17 +32,27 @@ server { internal; # Nginx will strip the `/proxito/` and pass just the `$storage/$type/$proj/$ver/$filename` proxy_pass http://storage:10000/; + proxy_set_header Host storage:10000; proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Host $host; - proxy_intercept_errors on; error_page 404 = @notfoundfallback; - add_header X-Served Nginx-Proxito-Sendfile always; - add_header X-RTD-Project $upstream_http_x_rtd_project always; + # Nginx is silly, so we have to *set* these then use them. + # Using them directly gets the values from the storage upstream. + set $rtd_project $upstream_http_x_rtd_project; + add_header X-RTD-Project $rtd_project always; + set $rtd_version $upstream_http_x_rtd_version; + add_header X-RTD-Version $rtd_version always; + set $rtd_path $upstream_http_x_rtd_path; + add_header X-RTD-Path $rtd_path always; + set $rtd_domain $upstream_http_x_rtd_domain; + add_header X-RTD-Domain $rtd_domain always; + set $rtd_method $upstream_http_x_rtd_version_method; + add_header X-RTD-Version-Method $rtd_method always; } # Serve 404 pages here From 3b5465f5a7126d40996338a8f4d8d78e91f638c0 Mon Sep 17 00:00:00 2001 From: Eric Holscher Date: Sat, 14 Mar 2020 11:29:24 -0700 Subject: [PATCH 2/6] Cleanup --- dockerfiles/nginx/proxito.conf | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dockerfiles/nginx/proxito.conf b/dockerfiles/nginx/proxito.conf index 87750c10ad1..890d034a8f4 100644 --- a/dockerfiles/nginx/proxito.conf +++ b/dockerfiles/nginx/proxito.conf @@ -3,7 +3,6 @@ server { listen 80 default_server; server_name *.community.dev.readthedocs.io *.org.dev.readthedocs.build; - # TODO: serve the favicon.ico from the project/version itself instead location /favicon.ico { proxy_pass http://storage:10000/devstoreaccount1/static/images/favicon.ico; @@ -24,7 +23,7 @@ server { proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Host $host; - + add_header X-Served Django-Proxito always; } # Sendfile support to serve the actual files that Proxito has specified From a23109b569dd8533367ce94689059a379b12aeef Mon Sep 17 00:00:00 2001 From: Eric Holscher Date: Sat, 14 Mar 2020 11:32:51 -0700 Subject: [PATCH 3/6] Better comment --- dockerfiles/nginx/proxito.conf | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dockerfiles/nginx/proxito.conf b/dockerfiles/nginx/proxito.conf index 890d034a8f4..85cd6606e9b 100644 --- a/dockerfiles/nginx/proxito.conf +++ b/dockerfiles/nginx/proxito.conf @@ -42,6 +42,8 @@ server { # Nginx is silly, so we have to *set* these then use them. # Using them directly gets the values from the storage upstream. + # I think the `set` must get read before doing the proxy_pass + # but the `add_header` is read after set $rtd_project $upstream_http_x_rtd_project; add_header X-RTD-Project $rtd_project always; set $rtd_version $upstream_http_x_rtd_version; From 41c96d68ada82e4c2b78f87cfc4db60705f98416 Mon Sep 17 00:00:00 2001 From: Eric Holscher Date: Sat, 14 Mar 2020 11:34:52 -0700 Subject: [PATCH 4/6] backport 404 comment --- dockerfiles/nginx/proxito.conf | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dockerfiles/nginx/proxito.conf b/dockerfiles/nginx/proxito.conf index 85cd6606e9b..7b9703a6134 100644 --- a/dockerfiles/nginx/proxito.conf +++ b/dockerfiles/nginx/proxito.conf @@ -23,6 +23,10 @@ server { proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Host $host; + + # Don't handle 404's here because they are from the application + # We only want to catch 404's from the storage backend. + add_header X-Served Django-Proxito always; } From d017db6400539ff3e1275bd3a1f85b0c977146b0 Mon Sep 17 00:00:00 2001 From: Eric Holscher Date: Sat, 14 Mar 2020 11:35:30 -0700 Subject: [PATCH 5/6] Pass version_slug properly to `_serve_docs`. This isn't used, but useful for logging --- readthedocs/proxito/views/serve.py | 1 + 1 file changed, 1 insertion(+) diff --git a/readthedocs/proxito/views/serve.py b/readthedocs/proxito/views/serve.py index ac3aaa11324..672030a1254 100644 --- a/readthedocs/proxito/views/serve.py +++ b/readthedocs/proxito/views/serve.py @@ -143,6 +143,7 @@ def get(self, return self._serve_docs( request, final_project=final_project, + version_slug=version_slug, path=final_url, ) From dc4c8f7398421c47fc8e4b3640811fa080e2ced6 Mon Sep 17 00:00:00 2001 From: Eric Holscher Date: Sat, 14 Mar 2020 11:56:14 -0700 Subject: [PATCH 6/6] Handle 404's still --- dockerfiles/nginx/proxito.conf | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dockerfiles/nginx/proxito.conf b/dockerfiles/nginx/proxito.conf index 7b9703a6134..9320066cbd8 100644 --- a/dockerfiles/nginx/proxito.conf +++ b/dockerfiles/nginx/proxito.conf @@ -24,8 +24,8 @@ server { proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; proxy_set_header X-Forwarded-Host $host; - # Don't handle 404's here because they are from the application - # We only want to catch 404's from the storage backend. + proxy_intercept_errors on; + error_page 404 = @proxito404; add_header X-Served Django-Proxito always; }