Merge pull request #6629 from readthedocs/external-builds-new-domain

Initial attempt to serve PR builds at `readthedocs.build`

dockerfiles/nginx/proxito.conf

@@ -1,14 +1,21 @@
 # Proxito
 server {
-    listen 80;
-    server_name *.community.dev.readthedocs.io;
+    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;
         break;
     }
 
+    # Redirect old URLs to our new domain
+    # external-builds.readthedocs.io/<type>/<slug>/<pr-number>/
+    location  ~* ^/html/(?P<project>.+)/(?P<version>.+)/(?P<path>.*) {
+        add_header X-Served Nginx-External-Redirect always;
+        rewrite ^ http://$project--$version.org.dev.readthedocs.build/page/$path;
+    }
+
     # Proxito doc serving
     location / {
         proxy_pass http://proxito:8000;
@@ -29,10 +36,12 @@ server {
         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;
     }
 
     # Serve 404 pages here

readthedocs/builds/models.py

@@ -302,27 +302,6 @@ def commit_name(self):
 
     def get_absolute_url(self):
         """Get absolute url to the docs of the version."""
-        # Hack external versions for now.
-        # TODO: We can integrate them into the resolver
-        # but this is much simpler to handle since we only link them a couple places for now
-        if self.type == EXTERNAL:
-            # Django's static file serving doesn't automatically append index.html
-            scheme = 'https' if settings.PUBLIC_DOMAIN_USES_HTTPS else 'http'
-            path = self.project.get_storage_path(
-                type_='html',
-                version_slug=self.slug,
-                version_type=self.type,
-                include_file=False,
-            )
-
-            # We don't want the `external/` part in the user-facing URL
-            if path.startswith(EXTERNAL):
-                path = path.replace(f'{EXTERNAL}/', '', 1)
-
-            domain = settings.RTD_EXTERNAL_VERSION_DOMAIN
-            url = f'{scheme}://{domain}/{path}/index.html'
-            return url
-
         if not self.built and not self.uploaded:
             return reverse(
                 'project_version_detail',
@@ -332,9 +311,11 @@ def get_absolute_url(self):
                 },
             )
         private = self.privacy_level == PRIVATE
+        external = self.type == EXTERNAL
         return self.project.get_docs_url(
             version_slug=self.slug,
             private=private,
+            external=external,
         )
 
     def save(self, *args, **kwargs):  # pylint: disable=arguments-differ
@@ -387,10 +368,12 @@ def supports_wipe(self):
 
     def get_subdomain_url(self):
         private = self.privacy_level == PRIVATE
+        external = self.type == EXTERNAL
         return self.project.get_docs_url(
             version_slug=self.slug,
             lang_slug=self.project.language,
             private=private,
+            external=external,
         )
 
     def get_downloads(self, pretty=False):

readthedocs/core/resolver.py

@@ -6,7 +6,8 @@
 from django.conf import settings
 
 from readthedocs.core.utils.extend import SettingsOverrideObject
-from readthedocs.projects.constants import PRIVATE, PUBLIC
+from readthedocs.projects.constants import PRIVATE
+from readthedocs.builds.constants import EXTERNAL
 
 log = logging.getLogger(__name__)
 
@@ -107,7 +108,7 @@ def resolve_path(
         language = language or project.language
 
         if private is None:
-            private = self._get_private(project, version_slug)
+            private, _ = self._get_private_and_external(project, version_slug)
 
         filename = self._fix_filename(project, filename)
 
@@ -161,19 +162,22 @@ def resolve_domain(self, project, private=None):
 
     def resolve(
             self, project, require_https=False, filename='', query_params='',
-            private=None, **kwargs
+            private=None, external=None, **kwargs
     ):
-        if private is None:
-            version_slug = kwargs.get('version_slug')
+        version_slug = kwargs.get('version_slug')
+
+        if private is None or external is None:
             if version_slug is None:
                 version_slug = project.get_default_version()
-            private = self._get_private(project, version_slug)
+            private, external = self._get_private_and_external(project, version_slug)
 
         canonical_project = self._get_canonical_project(project)
         custom_domain = canonical_project.get_canonical_custom_domain()
         use_custom_domain = self._use_custom_domain(custom_domain)
 
-        if use_custom_domain:
+        if external:
+            domain = self._get_external_subdomain(canonical_project, version_slug)
+        elif use_custom_domain:
             domain = custom_domain.domain
         elif self._use_subdomain():
             domain = self._get_project_subdomain(canonical_project)
@@ -226,21 +230,28 @@ def _get_canonical_project(self, project, projects=None):
             return self._get_canonical_project(next_project, projects)
         return project
 
+    def _get_external_subdomain(self, project, version_slug):
+        """Determine domain for an external version."""
+        subdomain_slug = project.slug.replace('_', '-')
+        # Version slug is in the domain so we can properly serve single-version projects
+        # and have them resolve the proper version from the PR.
+        return f'{subdomain_slug}--{version_slug}.{settings.RTD_EXTERNAL_VERSION_DOMAIN}'
+
     def _get_project_subdomain(self, project):
         """Determine canonical project domain as subdomain."""
-        if self._use_subdomain():
-            project = self._get_canonical_project(project)
-            subdomain_slug = project.slug.replace('_', '-')
-            return '{}.{}'.format(subdomain_slug, settings.PUBLIC_DOMAIN)
+        subdomain_slug = project.slug.replace('_', '-')
+        return '{}.{}'.format(subdomain_slug, settings.PUBLIC_DOMAIN)
 
-    def _get_private(self, project, version_slug):
+    def _get_private_and_external(self, project, version_slug):
         from readthedocs.builds.models import Version
         try:
             version = project.versions.get(slug=version_slug)
             private = version.privacy_level == PRIVATE
+            external = version.type == EXTERNAL
         except Version.DoesNotExist:
             private = settings.DEFAULT_PRIVACY_LEVEL == PRIVATE
-        return private
+            external = False
+        return private, external
 
     def _fix_filename(self, project, filename):
         """

readthedocs/projects/models.py

@@ -489,17 +489,18 @@ def delete(self, *args, **kwargs):  # pylint: disable=arguments-differ
     def get_absolute_url(self):
         return reverse('projects_detail', args=[self.slug])
 
-    def get_docs_url(self, version_slug=None, lang_slug=None, private=None):
+    def get_docs_url(self, version_slug=None, lang_slug=None, private=None, external=False):
         """
         Return a URL for the docs.
 
-        Always use http for now, to avoid content warnings.
+        ``external`` defaults False because we only link external versions in very specific places
         """
         return resolve(
             project=self,
             version_slug=version_slug,
             language=lang_slug,
             private=private,
+            external=external,
         )
 
     def get_builds_url(self):
@@ -1304,6 +1305,8 @@ def get_absolute_url(self):
             project=self.project,
             version_slug=self.version.slug,
             filename=self.path,
+            # this should always be False because we don't have ImportedFile's for external versions
+            external=False,
         )
 
     def __str__(self):

readthedocs/projects/tasks.py

@@ -810,6 +810,23 @@ def store_build_artifacts(
         :param pdf: whether to save PDF output
         :param epub: whether to save ePub output
         """
+        # TODO: Remove this logic from `update_app_instances`
+        # It's in both places to make sure it runs.
+        try:
+            if html:
+                version = api_v2.version(self.version.pk)
+                version.patch({
+                    'built': True,
+                    'documentation_type': self.config.doctype,
+                    'has_pdf': pdf,
+                    'has_epub': epub,
+                    'has_htmlzip': localmedia,
+                })
+        except HttpClientError:
+            log.exception(
+                'Updating version failed, skipping file sync: version=%s',
+                self.version,
+            )
         if not settings.RTD_BUILD_MEDIA_STORAGE:
             # Note: this check can be removed once corporate build servers use storage
             log.warning(

readthedocs/proxito/middleware.py

@@ -16,7 +16,7 @@
 log = logging.getLogger(__name__)  # noqa
 
 
-def map_host_to_project_slug(request):
+def map_host_to_project_slug(request):  # pylint: disable=too-many-return-statements
     """
     Take the request and map the host to the proper project slug.
 
@@ -32,8 +32,11 @@ def map_host_to_project_slug(request):
 
     host = request.get_host().lower().split(':')[0]
     public_domain = settings.PUBLIC_DOMAIN.lower().split(':')[0]
+    external_domain = settings.RTD_EXTERNAL_VERSION_DOMAIN.lower().split(':')[0]
+
     host_parts = host.split('.')
     public_domain_parts = public_domain.split('.')
+    external_domain_parts = external_domain.split('.')
 
     project_slug = None
 
@@ -59,6 +62,21 @@ def map_host_to_project_slug(request):
             request, 'core/dns-404.html', context={'host': host}, status=400
         )
 
+    if external_domain in host:
+        # Serve custom versions on external-host-domain
+        if external_domain_parts == host_parts[1:]:
+            try:
+                project_slug, version_slug = host_parts[0].split('--', 1)
+                request.external_domain = True
+                request.host_version_slug = version_slug
+                log.debug('Proxito External Version Domain: host=%s', host)
+                return project_slug
+            except ValueError:
+                log.warning('Weird variation on our hostname: host=%s', host)
+                return render(
+                    request, 'core/dns-404.html', context={'host': host}, status=400
+                )
+
     # Serve CNAMEs
     domain = Domain.objects.filter(domain=host).first()
     if domain:

readthedocs/proxito/tests/test_full.py

@@ -92,7 +92,7 @@ def test_single_version_serving_looks_like_normal(self):
         )
 
     @override_settings(
-        RTD_EXTERNAL_VERSION_DOMAIN='external-builds.dev.readthedocs.io',
+        RTD_EXTERNAL_VERSION_DOMAIN='dev.readthedocs.build',
     )
     def test_single_version_external_serving(self):
         self.project.single_version = True
@@ -105,15 +105,15 @@ def test_single_version_external_serving(self):
             active=True,
             project=self.project,
         )
-        url = '/html/project/10/awesome.html'
-        host = 'external-builds.dev.readthedocs.io'
+        url = '/awesome.html'
+        host = 'project--10.dev.readthedocs.build'
         resp = self.client.get(url, HTTP_HOST=host)
         self.assertEqual(
             resp['x-accel-redirect'], '/proxito/media/external/html/project/10/awesome.html',
         )
 
     @override_settings(
-        RTD_EXTERNAL_VERSION_DOMAIN='external-builds.dev.readthedocs.io',
+        RTD_EXTERNAL_VERSION_DOMAIN='dev.readthedocs.build',
     )
     def test_external_version_serving(self):
         fixture.get(
@@ -124,8 +124,8 @@ def test_external_version_serving(self):
             active=True,
             project=self.project,
         )
-        url = '/html/project/10/awesome.html'
-        host = 'external-builds.dev.readthedocs.io'
+        url = '/awesome.html'
+        host = 'project--10.dev.readthedocs.build'
         resp = self.client.get(url, HTTP_HOST=host)
         self.assertEqual(
             resp['x-accel-redirect'], '/proxito/media/external/html/project/10/awesome.html',
@@ -134,7 +134,7 @@ def test_external_version_serving(self):
         # Invalid tests
 
     @override_settings(
-        RTD_EXTERNAL_VERSION_DOMAIN='external-builds.dev.readthedocs.io',
+        RTD_EXTERNAL_VERSION_DOMAIN='dev.readthedocs.build',
     )
     def test_invalid_domain_for_external_version_serving(self):
         fixture.get(

readthedocs/proxito/tests/test_urls.py

@@ -105,15 +105,3 @@ def test_single_version(self):
                 'filename': 'some/path/index.html',
             },
         )
-
-    def test_external_version(self):
-        match = resolve('/html/project/version/path/index.html')
-        self.assertEqual(match.url_name, 'docs_detail_external_version')
-        self.assertEqual(match.args, ())
-        self.assertEqual(
-            match.kwargs, {
-                'project_slug': 'project',
-                'version_slug': 'version',
-                'filename': 'path/index.html',
-            },
-        )

readthedocs/proxito/urls.py

@@ -34,15 +34,14 @@
 from django.views import defaults
 
 from readthedocs.constants import pattern_opts
-from readthedocs.builds.constants import EXTERNAL
 from readthedocs.projects.views.public import ProjectDownloadMedia
 from readthedocs.proxito.views.serve import (
+    ServePageRedirect,
     ServeDocs,
     ServeError404,
     ServeRobotsTXT,
     ServeSitemapXML,
 )
-from readthedocs.proxito.views.redirects import redirect_page_with_filename
 from readthedocs.proxito.views.utils import fast_404
 
 DOC_PATH_PREFIX = getattr(settings, 'DOC_PATH_PREFIX', '')
@@ -92,7 +91,7 @@
     url(
         r'^(?:projects/(?P<subproject_slug>{project_slug})/)?'
         r'page/(?P<filename>.*)$'.format(**pattern_opts),
-        redirect_page_with_filename,
+        ServePageRedirect.as_view(),
         name='redirect_page_with_filename',
     ),
 
@@ -133,21 +132,6 @@
     #     name='docs_detail',
     # ),
 
-    # External versions
-    # (RTD_EXTERNAL_VERSION_DOMAIN/html/<project-slug>/<version-slug>/<filename>)
-    # NOTE: requires to be before single version
-    url(
-        (
-            r'^html/(?P<project_slug>{project_slug})/'
-            r'(?P<version_slug>{version_slug})/'
-            r'(?P<filename>{filename_slug})'.format(
-                **pattern_opts,
-            )
-        ),
-        ServeDocs.as_view(version_type=EXTERNAL),
-        name='docs_detail_external_version',
-    ),
-
     # (Sub)project single version
     url(
         (