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

Proxito: always use subdomain for serving docs #10799

Merged
merged 5 commits into from
Oct 10, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
2 changes: 1 addition & 1 deletion docs/dev/install.rst
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ Docker for builds

Serve documentation under a subdomain
There are a number of resolution bugs and cross-domain behavior that can
only be caught by using `USE_SUBDOMAIN` setting.
only be caught by using a ``PUBLIC_DOMAIN`` setting different from the ``PRODUCTION_DOMAIN`` setting.

PostgreSQL as a database
It is recommended that Postgres be used as the default database whenever
Expand Down
10 changes: 1 addition & 9 deletions docs/dev/settings.rst
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,10 @@ memory
Examples: '200m' for 200MB of total memory, or '2g' for 2GB of
total memory.

USE_SUBDOMAIN
---------------

Whether to use subdomains in URLs on the site, or the Django-served content.
When used in production, this should be ``True``, as Nginx will serve this content.
During development and other possible deployments, this might be ``False``.

PRODUCTION_DOMAIN
------------------

This is the domain that gets linked to throughout the site when used in production.
It depends on `USE_SUBDOMAIN`, otherwise it isn't used.
This is the domain that is used by the main application (not documentation pages).
stsewd marked this conversation as resolved.
Show resolved Hide resolved

RTD_INTERSPHINX_URL
-------------------
Expand Down
1 change: 0 additions & 1 deletion readthedocs/api/v3/tests/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
@override_settings(
PUBLIC_DOMAIN="readthedocs.io",
PRODUCTION_DOMAIN="readthedocs.org",
USE_SUBDOMAIN=True,
RTD_BUILD_MEDIA_STORAGE="readthedocs.rtd_tests.storage.BuildMediaFileSystemStorageTest",
RTD_ALLOW_ORGANIZATIONS=False,
)
Expand Down
1 change: 0 additions & 1 deletion readthedocs/core/context_processors.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ def readthedocs_processor(request):
exports = {
"PUBLIC_DOMAIN": settings.PUBLIC_DOMAIN,
"PRODUCTION_DOMAIN": settings.PRODUCTION_DOMAIN,
"USE_SUBDOMAIN": settings.USE_SUBDOMAIN,
"GLOBAL_ANALYTICS_CODE": settings.GLOBAL_ANALYTICS_CODE,
"DASHBOARD_ANALYTICS_CODE": settings.DASHBOARD_ANALYTICS_CODE,
"SITE_ROOT": settings.SITE_ROOT + "/",
Expand Down
20 changes: 2 additions & 18 deletions readthedocs/core/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,6 @@ def base_resolve_path(
language=None,
single_version=None,
project_relationship=None,
subdomain=None,
cname=None,
custom_prefix=None,
):
"""
Expand All @@ -77,12 +75,7 @@ def base_resolve_path(
the path will be prefixed with the subproject prefix
(defaults to ``/projects/<subproject-slug>/``).
"""
# Only support `/docs/project' URLs outside our normal environment. Normally
# the path should always have a subdomain or CNAME domain
if subdomain or cname or self._use_subdomain():
path = "/"
else:
path = "/docs/{project}/"
path = "/"

if project_relationship:
path = unsafe_join_url_path(path, project_relationship.subproject_prefix)
Expand Down Expand Up @@ -112,8 +105,6 @@ def resolve_path(
version_slug=None,
language=None,
single_version=None,
subdomain=None,
cname=None,
):
"""Resolve a URL with a subset of fields defined."""
version_slug = version_slug or project.get_default_version()
Expand All @@ -122,11 +113,6 @@ def resolve_path(
filename = self._fix_filename(filename)

parent_project, project_relationship = self._get_canonical_project_data(project)
cname = (
Copy link
Member

Choose a reason for hiding this comment

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

Do we not still need this logic to determine cname usage?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not when resolving the path, only when resolving the domain

cname
or self._use_subdomain()
or parent_project.get_canonical_custom_domain()
)
single_version = bool(project.single_version or single_version)

# If the project is a subproject, we use the custom prefix
Expand All @@ -145,8 +131,6 @@ def resolve_path(
language=language,
single_version=single_version,
project_relationship=project_relationship,
cname=cname,
subdomain=subdomain,
custom_prefix=custom_prefix,
)

Expand Down Expand Up @@ -385,7 +369,7 @@ def _use_custom_domain(self, custom_domain):

def _use_subdomain(self):
"""Make decision about whether to use a subdomain to serve docs."""
return settings.USE_SUBDOMAIN and settings.PUBLIC_DOMAIN is not None
return settings.PUBLIC_DOMAIN is not None

def _use_cname(self, project):
"""Test if to allow direct serving for project on CNAME."""
Expand Down
1 change: 0 additions & 1 deletion readthedocs/embed/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ def setup_method(self, settings):
self.version.privacy_level = PUBLIC
self.version.save()

settings.USE_SUBDOMAIN = True
settings.PUBLIC_DOMAIN = "readthedocs.io"
settings.RTD_DEFAULT_FEATURES = dict(
[RTDProductFeature(TYPE_EMBED_API).to_item()]
Expand Down
1 change: 0 additions & 1 deletion readthedocs/embed/v3/tests/test_access.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@


@override_settings(
USE_SUBDOMAIN=True,
PUBLIC_DOMAIN="readthedocs.io",
RTD_ALLOW_ORGAZATIONS=False,
RTD_DEFAULT_FEATURES=dict([RTDProductFeature(TYPE_EMBED_API).to_item()]),
Expand Down
1 change: 0 additions & 1 deletion readthedocs/embed/v3/tests/test_basics.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
class TestEmbedAPIv3Basics:
@pytest.fixture(autouse=True)
def setup_method(self, settings):
settings.USE_SUBDOMAIN = True
settings.PUBLIC_DOMAIN = "readthedocs.io"
settings.RTD_EMBED_API_EXTERNAL_DOMAINS = ["docs.project.com"]

Expand Down
1 change: 0 additions & 1 deletion readthedocs/embed/v3/tests/test_external_pages.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
class TestEmbedAPIv3ExternalPages:
@pytest.fixture(autouse=True)
def setup_method(self, settings):
settings.USE_SUBDOMAIN = True
settings.PUBLIC_DOMAIN = "readthedocs.io"
settings.RTD_EMBED_API_EXTERNAL_DOMAINS = ["docs.project.com"]

Expand Down
1 change: 0 additions & 1 deletion readthedocs/embed/v3/tests/test_internal_pages.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
class TestEmbedAPIv3InternalPages:
@pytest.fixture(autouse=True)
def setup_method(self, settings):
settings.USE_SUBDOMAIN = True
settings.PUBLIC_DOMAIN = "readthedocs.io"
settings.RTD_EMBED_API_EXTERNAL_DOMAINS = []
settings.RTD_DEFAULT_FEATURES = dict(
Expand Down
6 changes: 1 addition & 5 deletions readthedocs/projects/views/private.py
Original file line number Diff line number Diff line change
Expand Up @@ -755,11 +755,7 @@ def get_context_data(self, **kwargs):
ctx = super().get_context_data(**kwargs)

# Get the default docs domain
ctx['default_domain'] = (
settings.PUBLIC_DOMAIN
if settings.USE_SUBDOMAIN
else settings.PRODUCTION_DOMAIN
)
ctx["default_domain"] = settings.PUBLIC_DOMAIN

return ctx

Expand Down
7 changes: 1 addition & 6 deletions readthedocs/proxito/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,12 +200,7 @@ def process_request(self, request): # noqa
request.unresolved_domain = None

skip = any(request.path.startswith(reverse(view)) for view in self.skip_views)
if (
skip
or not settings.USE_SUBDOMAIN
or "localhost" in request.get_host()
or "testserver" in request.get_host()
Comment on lines -206 to -207
Copy link
Member Author

@stsewd stsewd Oct 5, 2023

Choose a reason for hiding this comment

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

These two checks are not really that useful, and they cause problems if your project has localhost in its name.

Copy link
Member

Choose a reason for hiding this comment

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

Hrm, I feel like it might cause some issues in tests, but if everythings looks OK, I agree this is a better approach.

):
if skip:
log.debug("Not processing Proxito middleware")
return None

Expand Down
1 change: 0 additions & 1 deletion readthedocs/proxito/tests/test_old_redirects.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
and adapted to use:

* El Proxito
* USE_SUBDOMAIN=True always
"""

import django_dynamic_fixture as fixture
Expand Down
27 changes: 13 additions & 14 deletions readthedocs/rtd_tests/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
from readthedocs.subscriptions.products import RTDProductFeature


@override_settings(PUBLIC_DOMAIN="readthedocs.io")
class APIBuildTests(TestCase):
fixtures = ['eric.json', 'test_data.json']

Expand Down Expand Up @@ -253,15 +254,12 @@ def test_response_finished_and_success(self):
resp = client.get('/api/v2/build/{build}/'.format(build=build.pk))
self.assertEqual(resp.status_code, 200)
build = resp.data
docs_url = 'http://readthedocs.org/docs/{project}/en/{version}/'.format(
project=project.slug,
version=version.slug,
)
self.assertEqual(build['state'], 'finished')
self.assertEqual(build['error'], '')
self.assertEqual(build['exit_code'], 0)
self.assertEqual(build['success'], True)
self.assertEqual(build['docs_url'], docs_url)
docs_url = f"http://{project.slug}.readthedocs.io/en/{version.slug}/"
self.assertEqual(build["state"], "finished")
self.assertEqual(build["error"], "")
self.assertEqual(build["exit_code"], 0)
self.assertEqual(build["success"], True)
self.assertEqual(build["docs_url"], docs_url)
# Verify the path is trimmed
self.assertEqual(
build["commands"][0]["command"],
Expand Down Expand Up @@ -3108,6 +3106,7 @@ def test_webhook_build_another_branch(self, trigger_build):
self.assertEqual(resp.data['versions'], ['v1.0'])


@override_settings(PUBLIC_DOMAIN="readthedocs.io")
class APIVersionTests(TestCase):
fixtures = ['eric', 'test_data']
maxDiff = None # So we get an actual diff when it fails
Expand Down Expand Up @@ -3139,11 +3138,11 @@ def test_get_version_by_id(self):
"built": False,
"id": 18,
"active": True,
"canonical_url": "http://readthedocs.org/docs/pip/en/0.8/",
"canonical_url": "http://pip.readthedocs.io/en/0.8/",
"project": {
"analytics_code": None,
"analytics_disabled": False,
"canonical_url": "http://readthedocs.org/docs/pip/en/latest/",
"canonical_url": "http://pip.readthedocs.io/en/latest/",
"cdn_enabled": False,
"conf_py_file": "",
"container_image": None,
Expand Down Expand Up @@ -3203,7 +3202,7 @@ def test_get_active_versions(self):
'active': 'true',
}
url = reverse("version-list")
with self.assertNumQueries(6):
with self.assertNumQueries(5):
resp = self.client.get(url, data)

self.assertEqual(resp.status_code, 200)
Expand All @@ -3213,15 +3212,15 @@ def test_get_active_versions(self):
data.update({
'active': 'false',
})
with self.assertNumQueries(6):
with self.assertNumQueries(5):
resp = self.client.get(url, data)
self.assertEqual(resp.status_code, 200)
self.assertEqual(resp.data['count'], pip.versions.filter(active=False).count())

def test_project_get_active_versions(self):
pip = Project.objects.get(slug="pip")
url = reverse("project-active-versions", args=[pip.pk])
with self.assertNumQueries(6):
with self.assertNumQueries(5):
resp = self.client.get(url)
self.assertEqual(
len(resp.data["versions"]), pip.versions.filter(active=True).count()
Expand Down
1 change: 0 additions & 1 deletion readthedocs/rtd_tests/tests/test_build_notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
@override_settings(
PRODUCTION_DOMAIN="readthedocs.org",
PUBLIC_DOMAIN="readthedocs.io",
USE_SUBDOMAIN=True,
)
class BuildNotificationsTests(TestCase):
def setUp(self):
Expand Down
4 changes: 2 additions & 2 deletions readthedocs/rtd_tests/tests/test_core_tags.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@
from readthedocs.projects.models import Project


@override_settings(USE_SUBDOMAIN=False, PRODUCTION_DOMAIN="readthedocs.org")
@override_settings(PRODUCTION_DOMAIN="readthedocs.org", PUBLIC_DOMAIN="readthedocs.org")
class CoreTagsTests(TestCase):
fixtures = ["eric", "test_data"]

def setUp(self):
url_base = "http://{domain}/docs/pip{{version}}".format(
url_base = "http://pip.{domain}{{version}}".format(
domain=settings.PRODUCTION_DOMAIN,
)

Expand Down
1 change: 0 additions & 1 deletion readthedocs/rtd_tests/tests/test_footer.py
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,6 @@ class TestFooterHTML(BaseTestFooterHTML, TestCase):


@override_settings(
USE_SUBDOMAIN=True,
PUBLIC_DOMAIN="readthedocs.io",
PUBLIC_DOMAIN_USES_HTTPS=True,
)
Expand Down
2 changes: 0 additions & 2 deletions readthedocs/rtd_tests/tests/test_notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ class TestNotification(Notification):
"SUPPORT_EMAIL": "[email protected]",
"TEMPLATE_ROOT": mock.ANY,
"USE_PROMOS": mock.ANY,
"USE_SUBDOMAIN": mock.ANY,
"USE_ORGANIZATIONS": mock.ANY,
},
)
Expand Down Expand Up @@ -236,7 +235,6 @@ def test_context_data(self):
"SUPPORT_EMAIL": "[email protected]",
"TEMPLATE_ROOT": mock.ANY,
"USE_PROMOS": mock.ANY,
"USE_SUBDOMAIN": mock.ANY,
"USE_ORGANIZATIONS": mock.ANY,
}
self.assertEqual(self.n.get_context_data(), context)
Expand Down
4 changes: 0 additions & 4 deletions readthedocs/rtd_tests/tests/test_privacy.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,6 @@ def test_private_public_repo_downloading(self):
@override_settings(
DEFAULT_PRIVACY_LEVEL="private",
PUBLIC_DOMAIN="readthedocs.io",
USE_SUBDOMAIN=True,
)
def test_private_download_filename(self):
self._create_kong("private", "private")
Expand Down Expand Up @@ -270,7 +269,6 @@ def test_private_download_filename(self):
@override_settings(
DEFAULT_PRIVACY_LEVEL="public",
PUBLIC_DOMAIN="readthedocs.io",
USE_SUBDOMAIN=True,
)
def test_public_repo_downloading(self):
self._create_kong("public", "public")
Expand Down Expand Up @@ -308,7 +306,6 @@ def test_public_repo_downloading(self):
@override_settings(
DEFAULT_PRIVACY_LEVEL="public",
PUBLIC_DOMAIN="readthedocs.io",
USE_SUBDOMAIN=True,
)
def test_public_private_repo_downloading(self):
self._create_kong("private", "private")
Expand Down Expand Up @@ -338,7 +335,6 @@ def test_public_private_repo_downloading(self):
@override_settings(
DEFAULT_PRIVACY_LEVEL="public",
PUBLIC_DOMAIN="readthedocs.io",
USE_SUBDOMAIN=True,
)
def test_public_download_filename(self):
self._create_kong("public", "public")
Expand Down
Loading