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

Initial stub of proxito #6226

Merged
merged 34 commits into from
Oct 9, 2019
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
d43db8e
Add inital spike of Proxito
ericholscher Aug 15, 2019
b7202ca
More interim work.
ericholscher Aug 19, 2019
092cef9
Cleanup URLs to have less RTD logic.
ericholscher Sep 3, 2019
7aaef0d
Put old middleware back in for now
ericholscher Sep 4, 2019
532084e
Cleanup some logic and add logging
ericholscher Oct 1, 2019
624ac72
Minimum viable proxito settings
ericholscher Oct 1, 2019
0c1b846
Much simpler settings file
ericholscher Oct 1, 2019
dd7651c
Cleanup settings
ericholscher Oct 1, 2019
cb6c0b7
Cleanup settings changes
ericholscher Oct 1, 2019
5252400
Merge remote-tracking branch 'origin/master' into proxito-stub
ericholscher Oct 1, 2019
801a240
Remove other changed logic
ericholscher Oct 1, 2019
2384035
Remove dev setting chagnes as well
ericholscher Oct 1, 2019
79c9205
Add middleware tests
ericholscher Oct 1, 2019
2e8cf67
Update lots of tests for proxito
ericholscher Oct 2, 2019
8138da2
Cleanup more tests
ericholscher Oct 2, 2019
85b4462
Test invalid input
ericholscher Oct 2, 2019
f74173c
Clean up the tests more
ericholscher Oct 2, 2019
d78675f
Review feedback
ericholscher Oct 2, 2019
339f9a9
Last bit of review feedback
ericholscher Oct 2, 2019
03f49d7
Remove debug_toolbar from URLs.
ericholscher Oct 7, 2019
091de9c
Only depend on PYTHON_MEDIA for serving, not DEBUG.
ericholscher Oct 7, 2019
512dc15
Fix linting
ericholscher Oct 7, 2019
af471dc
Fix linting once again
ericholscher Oct 7, 2019
21d0e9e
Fix lint once more :/
ericholscher Oct 7, 2019
38bc63a
Fix lies in tests.
ericholscher Oct 7, 2019
3bdce44
Don’t set PYTHON_MEDIA in Proxito setting.
ericholscher Oct 7, 2019
4acc04f
Review feedback
ericholscher Oct 7, 2019
1fb4ec7
Address review feedback
ericholscher Oct 8, 2019
5ca6d15
Clarify use of settings
ericholscher Oct 8, 2019
596aa85
Clarify comment and cleanup more logging
ericholscher Oct 8, 2019
2db8a4c
Fix lint
ericholscher Oct 8, 2019
644c5ba
Validate subproject is actually a subproject for the project
ericholscher Oct 8, 2019
ed62904
Update readthedocs/proxito/views.py
ericholscher Oct 9, 2019
c6d6d83
Fix lint
ericholscher Oct 9, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Empty file added readthedocs/proxito/__init__.py
Empty file.
128 changes: 128 additions & 0 deletions readthedocs/proxito/middleware.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
"""
Middleware for Proxito.

This is used to take the request and map the host to the proper project slug.

Additional processing is done to get the project from the URL in the ``views.py`` as well.
"""
import logging

from django.conf import settings
from django.core.exceptions import MiddlewareNotUsed
from django.http import HttpResponseBadRequest
from django.shortcuts import render
from django.utils.deprecation import MiddlewareMixin
from django.utils.translation import ugettext_lazy as _

from readthedocs.projects.models import Domain


log = logging.getLogger(__name__) # noqa


def map_host_to_project_slug(request):
"""
Take the request and map the host to the proper project slug.

We check, in order:

* The ``HTTP_X_RTD_SLUG`` host header for explicit Project mapping
- This sets ``request.rtdheader`` True
* The ``PUBLIC_DOMAIN`` where we can use the subdomain as the project name
- This sets ``request.subdomain`` True
* The hostname without port information, which maps to ``Domain`` objects
ericholscher marked this conversation as resolved.
Show resolved Hide resolved
- This sets ``request.cname`` True
"""

host = request.get_host().lower().split(':')[0]
public_domain = settings.PUBLIC_DOMAIN.lower().split(':')[0]
host_parts = host.split('.')
public_domain_parts = public_domain.split('.')

# Explicit Project slug being passed in
if 'HTTP_X_RTD_SLUG' in request.META:
project_slug = request.META['HTTP_X_RTD_SLUG'].lower()
request.rtdheader = True

elif public_domain in host:
# Serve from the PUBLIC_DOMAIN, ensuring it looks like `foo.PUBLIC_DOMAIN`
if public_domain_parts == host_parts[1:]:
project_slug = host_parts[0]
request.subdomain = True
log.debug('Proxito Public Domain: host=%s', host)
else:
# TODO: This can catch some possibly valid domains (docs.readthedocs.io.com) for example
# But these feel like they might be phishing, etc. so let's block them for now.
project_slug = None
ericholscher marked this conversation as resolved.
Show resolved Hide resolved
log.warning('Weird variation on our hostname: %s', host)
return HttpResponseBadRequest(_('Invalid hostname'))

# Serve CNAMEs
else:
domain_qs = Domain.objects.filter(domain=host
).prefetch_related('project')
if domain_qs.exists():
project_slug = domain_qs.first().project.slug
request.cname = True
log.debug('Proxito CNAME: %s', host)
else:
# Some person is CNAMEing to us without configuring a domain - 404.
project_slug = None
ericholscher marked this conversation as resolved.
Show resolved Hide resolved
log.debug('CNAME 404: %s', host)
return render(
request, 'core/dns-404.html', context={'host': host}, status=404
)
log.debug('Proxito Project: %s', project_slug)
return project_slug
ericholscher marked this conversation as resolved.
Show resolved Hide resolved


class ProxitoMiddleware(MiddlewareMixin):

"""The actual middleware we'll be using in prod."""

def process_request(self, request): # noqa
if any([not settings.USE_SUBDOMAIN, 'localhost' in request.get_host(),
'testserver' in request.get_host()]):
log.debug('Not processing Proxito middleware')
return None

ret = map_host_to_project_slug(request)

# Handle returning a response
if hasattr(ret, 'status_code'):
return ret

# Otherwise set the slug on the request
request.host_project_slug = request.slug = ret

return None


class NewStyleProxitoMiddleware:

"""The new style middleware, I can't figure out how to test it."""
Copy link
Member

Choose a reason for hiding this comment

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

I think you can test the request at least by creating a method process_request(self, request), that way you don't depend on self.get_response or mock it to test with localhost and testserver as hostname.

Copy link
Member

Choose a reason for hiding this comment

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

And I think you only want to test is the request

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea, the get_response thing just feels hard to test. I think having it old-style for now is fine, and we can figure out the migration in the future.

Copy link
Member Author

@ericholscher ericholscher Oct 8, 2019

Choose a reason for hiding this comment

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

Just going to delete the new-style middleware for now.


def __init__(self, get_response):
self.get_response = get_response
if not settings.USE_SUBDOMAIN:
raise MiddlewareNotUsed('USE_SUBDOMAIN setting is not on')

def __call__(self, request):
# For local dev to hit the main site
if 'localhost' in request.get_host() or 'testserver' in request.get_host():
return self.get_response(request)

# Code to be executed for each request before
# the view (and later middleware) are called.

host_project = map_host_to_project_slug(request)
Copy link
Member

Choose a reason for hiding this comment

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

That function doesn't always return a slug, sometimes it returns a response.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe refactor the function to return a tuple slug, response or try..cath exceptions and move the responses to the other part of the code?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yea, I wasn't sure the best way to refactor this. It does validation, and we have multiple different responses we want (invalid and also CNAME 404).

request.host_project_slug = host_project
request.slug = host_project
# request.urlconf = 'readthedocs.proxito.urls'

response = self.get_response(request)

# Code to be executed for each request/response after
# the view is called.

return response
Empty file.
49 changes: 49 additions & 0 deletions readthedocs/proxito/tests/base.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
# Copied from .org


import django_dynamic_fixture as fixture
from django.contrib.auth.models import User
from django.test import TestCase
from django.test.utils import override_settings

from readthedocs.projects.models import Project


@override_settings(
PUBLIC_DOMAIN='dev.readthedocs.io',
ROOT_URLCONF='readthedocs.proxito.urls',
MIDDLEWARE=['readthedocs.proxito.middleware.ProxitoMiddleware'],
USE_SUBDOMAIN=True,
)
class BaseDocServing(TestCase):

def setUp(self):
self.eric = fixture.get(User, username='eric')
self.eric.set_password('eric')
self.eric.save()
self.private = fixture.get(
Project, slug='private', privacy_level='private',
version_privacy_level='private', users=[self.eric],
main_language_project=None,
)
self.subproject = fixture.get(
Project,
slug='subproject',
users=[self.eric],
main_language_project=None,
)
self.private.add_subproject(self.subproject)
self.translation = fixture.get(
Project,
language='es',
slug='translation',
users=[self.eric],
main_language_project=self.private,
)
self.subproject_translation = fixture.get(
Project,
language='es',
slug='subproject-translation',
users=[self.eric],
main_language_project=self.subproject,
)
131 changes: 131 additions & 0 deletions readthedocs/proxito/tests/test_full.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
# Copied from .org

import os

import mock
from django.conf import settings
from django.http import HttpResponse
from django.test.utils import override_settings

from .base import BaseDocServing


@override_settings(PYTHON_MEDIA=False)
class TestFullDocServing(BaseDocServing):
# Test the full range of possible doc URL's

def test_subproject_serving(self):
url = '/projects/subproject/en/latest/awesome.html'
host = 'private.dev.readthedocs.io'
resp = self.client.get(url, HTTP_HOST=host)
self.assertEqual(
resp['x-accel-redirect'], '/proxito/html/subproject/latest/awesome.html',
ericholscher marked this conversation as resolved.
Show resolved Hide resolved
)

def test_subproject_single_version(self):
self.subproject.single_version = True
self.subproject.save()
url = '/projects/subproject/awesome.html'
host = 'private.dev.readthedocs.io'
resp = self.client.get(url, HTTP_HOST=host)
self.assertEqual(
resp['x-accel-redirect'], '/proxito/html/subproject/latest/awesome.html',
)

def test_subproject_translation_serving(self):
url = '/projects/subproject/es/latest/awesome.html'
host = 'private.dev.readthedocs.io'
resp = self.client.get(url, HTTP_HOST=host)
self.assertEqual(
resp['x-accel-redirect'], '/proxito/html/subproject-translation/latest/awesome.html',
)

def test_translation_serving(self):
url = '/es/latest/awesome.html'
host = 'private.dev.readthedocs.io'
resp = self.client.get(url, HTTP_HOST=host)
self.assertEqual(
resp['x-accel-redirect'], '/proxito/html/translation/latest/awesome.html',
)

def test_normal_serving(self):
url = '/en/latest/awesome.html'
host = 'private.dev.readthedocs.io'
resp = self.client.get(url, HTTP_HOST=host)
self.assertEqual(
resp['x-accel-redirect'], '/proxito/html/private/latest/awesome.html',
)

def test_single_version_serving(self):
self.private.single_version = True
self.private.save()
url = '/awesome.html'
host = 'private.dev.readthedocs.io'
resp = self.client.get(url, HTTP_HOST=host)
self.assertEqual(
resp['x-accel-redirect'], '/proxito/html/private/latest/awesome.html',
)

def test_single_version_serving_looks_like_normal(self):
self.private.single_version = True
self.private.save()
url = '/en/stable/awesome.html'
host = 'private.dev.readthedocs.io'
resp = self.client.get(url, HTTP_HOST=host)
self.assertEqual(
resp['x-accel-redirect'], '/proxito/html/private/latest/en/stable/awesome.html',
)

# Invalid tests

def test_invalid_url_for_project_with_versions(self):
url = '/cn/latest/awesome.html'
host = 'private.dev.readthedocs.io'
resp = self.client.get(url, HTTP_HOST=host)
self.assertEqual(resp.status_code, 404)

def test_invalid_translation(self):
url = '/cs/latest/awesome.html'
host = 'private.dev.readthedocs.io'
resp = self.client.get(url, HTTP_HOST=host)
self.assertEqual(resp.status_code, 404)

def test_invalid_subproject(self):
url = '/projects/doesnt-exist/foo.html'
host = 'private.dev.readthedocs.io'
resp = self.client.get(url, HTTP_HOST=host)
self.assertEqual(resp.status_code, 404)


class TestDocServingBackends(BaseDocServing):
# Test that nginx and python backends both work

@override_settings(PYTHON_MEDIA=True)
def test_python_media_serving(self):
with mock.patch(
'readthedocs.proxito.views.serve', return_value=HttpResponse()) as serve_mock:
url = '/en/latest/awesome.html'
host = 'private.dev.readthedocs.io'
self.client.get(url, HTTP_HOST=host)
serve_mock.assert_called_with(
mock.ANY,
'html/private/latest/awesome.html',
os.path.join(settings.SITE_ROOT, 'media'),
)

@override_settings(PYTHON_MEDIA=False)
def test_nginx_media_serving(self):
resp = self.client.get('/en/latest/awesome.html', HTTP_HOST='private.dev.readthedocs.io')
self.assertEqual(resp.status_code, 200)
self.assertEqual(
resp['x-accel-redirect'], '/proxito/html/private/latest/awesome.html',
)

@override_settings(PYTHON_MEDIA=False)
def test_private_nginx_serving_unicode_filename(self):
resp = self.client.get('/en/latest/úñíčódé.html', HTTP_HOST='private.dev.readthedocs.io')
self.assertEqual(resp.status_code, 200)
self.assertEqual(
resp['x-accel-redirect'],
'/proxito/html/private/latest/%C3%BA%C3%B1%C3%AD%C4%8D%C3%B3d%C3%A9.html',
)
Loading