Skip to content

Commit ed732c2

Browse files
authored
Hosting: manual integrations via build contract (#10127)
* Hosting: manual integrations via build contract * Use a single script to load everything * Include Read the Docs analytics to integrations * Initial work for hosting features * External version banner and doc-diff integration * Old version warning * Do not inject doc-diff on search page * Inject old version warning only for non-external versions * Comments! * More comments * Build: pass `PATH` environment variable to Docker container Instead of prepending all the commands with the `PATH=` variable, we pass the environment variable directly to the Docker container. This allow us to run multi-line shell script without failing with weird syntax errors. Besides, the implementation is cleaner since all the environment variables are passed to the commands in the same way. I added some _default paths_ that I found by checking the local Docker container. I'm also passing the users' path, depending if we are working locally as root or in production. This is not 100% complete and there may be some other issues that I'm not seeing yet, but I think it's a first step to behave in a way our users are expecting. Closes #10103 * Lint: for some reason fails at CircleCI otherwise Locally it tries to reverted back 🤷 * Feature flag for new hosting integrations X-RTD-Hosting-Integration: true/false This can be used from CloudFlare to decide whether or not inject a `<script>` into the resulting HTML or not. * Load `readthedocs-build.yaml` and generate `readthedocs-data.html` * Load READTHEDOCS_DATA async * Absolute proxied API path * Remove duplicated code * New approach using `readthedocs-client.js` and `/_/readthedocs-config.json` See https://github.com/humitos/readthedocs-client * Do not require `readthedocs-build.YAML` for now * Expand the JSON response with more data * Remove non-required files and rely on `readthedocs-client.js` only * Improve helper text * Builds: save `readthedocs-build.yaml` into database I added a `Version.build_data` field that may be used from `/_/readthedocs-config.json` to extend with data generated by the doctool at build time if necessary. * Use `Version.build_data` from the endpoint * Flyout: return data required to generate flyout dynamically * Updates to the API * Minor updates * Update the javascript client compiled version * doc-diff object returned * Build: check if the YAML file exists before trying to open it * Proxito: don't inject the header if the feature is turned off * Test: add hosting integrations tests * Remove JS This file will be deployed directly into S3. * Load the javascript from a local server for now * Update URL to remove .json from it * Remove non-required f-string * Allow saving `build_data` via API endpoint * Lint * Migrate nodejs installation to asdf * Change port to match `npm run dev` from readthedocs-client
1 parent b50e04c commit ed732c2

File tree

13 files changed

+368
-9
lines changed

13 files changed

+368
-9
lines changed

dockerfiles/nginx/proxito.conf.template

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,17 @@ server {
8888
proxy_hide_header Content-Security-Policy;
8989
set $content_security_policy $upstream_http_content_security_policy;
9090
add_header Content-Security-Policy $content_security_policy always;
91+
92+
# This header allows us to decide whether or not inject the script at CloudFlare level
93+
# Now, I'm injecting it in all the NGINX responses because `sub_filter` is not allowed inside an `if` statement.
94+
set $rtd_hosting_integrations $upstream_http_x_rtd_hosting_integrations;
95+
add_header X-RTD-Hosting-Integrations $rtd_hosting_integrations always;
96+
97+
# Inject our own script dynamically
98+
# TODO: find a way to make this work _without_ running `npm run dev` from the `readthedocs-client` repository
99+
sub_filter '</head>' '<script language="javascript" src="http://localhost:8000/readthedocs-client.js"></script>\n</head>';
100+
sub_filter_last_modified on;
101+
sub_filter_once on;
91102
}
92103

93104
# Serve 404 pages here

readthedocs/api/v2/serializers.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ class VersionSerializer(serializers.ModelSerializer):
101101

102102
class Meta:
103103
model = Version
104-
fields = (
104+
fields = [
105105
'id',
106106
'project',
107107
'slug',
@@ -116,14 +116,20 @@ class Meta:
116116
'has_epub',
117117
'has_htmlzip',
118118
'documentation_type',
119-
)
119+
]
120120

121121

122122
class VersionAdminSerializer(VersionSerializer):
123123

124124
"""Version serializer that returns admin project data."""
125125

126126
project = ProjectAdminSerializer()
127+
build_data = serializers.JSONField(required=False, write_only=True)
128+
129+
class Meta(VersionSerializer.Meta):
130+
fields = VersionSerializer.Meta.fields + [
131+
"build_data",
132+
]
127133

128134

129135
class BuildCommandSerializer(serializers.ModelSerializer):
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
# Generated by Django 3.2.18 on 2023-03-13 15:15
2+
3+
from django.db import migrations, models
4+
5+
6+
class Migration(migrations.Migration):
7+
8+
dependencies = [
9+
("builds", "0047_build_default_triggered"),
10+
]
11+
12+
operations = [
13+
migrations.AddField(
14+
model_name="version",
15+
name="build_data",
16+
field=models.JSONField(
17+
default=None,
18+
null=True,
19+
verbose_name="Data generated at build time by the doctool (`readthedocs-build.yaml`).",
20+
),
21+
),
22+
]

readthedocs/builds/models.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,11 +65,9 @@
6565
BITBUCKET_COMMIT_URL,
6666
BITBUCKET_URL,
6767
DOCTYPE_CHOICES,
68-
GITHUB_BRAND,
6968
GITHUB_COMMIT_URL,
7069
GITHUB_PULL_REQUEST_COMMIT_URL,
7170
GITHUB_URL,
72-
GITLAB_BRAND,
7371
GITLAB_COMMIT_URL,
7472
GITLAB_MERGE_REQUEST_COMMIT_URL,
7573
GITLAB_URL,
@@ -186,6 +184,12 @@ class Version(TimeStampedModel):
186184
),
187185
)
188186

187+
build_data = models.JSONField(
188+
_("Data generated at build time by the doctool (`readthedocs-build.yaml`)."),
189+
default=None,
190+
null=True,
191+
)
192+
189193
objects = VersionManager.from_queryset(VersionQuerySet)()
190194
# Only include BRANCH, TAG, UNKNOWN type Versions.
191195
internal = InternalVersionManager.from_queryset(partial(VersionQuerySet, internal_only=True))()

readthedocs/doc_builder/director.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import tarfile
33

44
import structlog
5+
import yaml
56
from django.conf import settings
67
from django.utils.translation import gettext_lazy as _
78

@@ -187,6 +188,7 @@ def build(self):
187188
self.build_epub()
188189

189190
self.run_build_job("post_build")
191+
self.store_readthedocs_build_yaml()
190192

191193
after_build.send(
192194
sender=self.data.version,
@@ -392,6 +394,7 @@ def run_build_commands(self):
392394
# Update the `Version.documentation_type` to match the doctype defined
393395
# by the config file. When using `build.commands` it will be `GENERIC`
394396
self.data.version.documentation_type = self.data.config.doctype
397+
self.store_readthedocs_build_yaml()
395398

396399
def install_build_tools(self):
397400
"""
@@ -625,3 +628,37 @@ def get_build_env_vars(self):
625628
def is_type_sphinx(self):
626629
"""Is documentation type Sphinx."""
627630
return "sphinx" in self.data.config.doctype
631+
632+
def store_readthedocs_build_yaml(self):
633+
# load YAML from user
634+
yaml_path = os.path.join(
635+
self.data.project.artifact_path(
636+
version=self.data.version.slug, type_="html"
637+
),
638+
"readthedocs-build.yaml",
639+
)
640+
641+
if not os.path.exists(yaml_path):
642+
log.debug("Build output YAML file (readtehdocs-build.yaml) does not exist.")
643+
return
644+
645+
try:
646+
with open(yaml_path, "r") as f:
647+
data = yaml.safe_load(f)
648+
except Exception:
649+
# NOTE: skip this work for now until we decide whether or not this
650+
# YAML file is required.
651+
#
652+
# NOTE: decide whether or not we want this
653+
# file to be mandatory and raise an exception here.
654+
return
655+
656+
log.info("readthedocs-build.yaml loaded.", path=yaml_path)
657+
658+
# TODO: validate the YAML generated by the user
659+
# self._validate_readthedocs_build_yaml(data)
660+
661+
# Copy the YAML data into `Version.build_data`.
662+
# It will be saved when the API is hit.
663+
# This data will be used by the `/_/readthedocs-config.json` API endpoint.
664+
self.data.version.build_data = data

readthedocs/projects/models.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1884,6 +1884,7 @@ def add_features(sender, **kwargs):
18841884
CANCEL_OLD_BUILDS = "cancel_old_builds"
18851885
DONT_CREATE_INDEX = "dont_create_index"
18861886
USE_RCLONE = "use_rclone"
1887+
HOSTING_INTEGRATIONS = "hosting_integrations"
18871888

18881889
FEATURES = (
18891890
(ALLOW_DEPRECATED_WEBHOOKS, _('Allow deprecated webhook views')),
@@ -2058,6 +2059,10 @@ def add_features(sender, **kwargs):
20582059
USE_RCLONE,
20592060
_("Use rclone for syncing files to the media storage."),
20602061
),
2062+
(
2063+
HOSTING_INTEGRATIONS,
2064+
_("Inject 'readthedocs-client.js' as <script> HTML tag in responses."),
2065+
),
20612066
)
20622067

20632068
projects = models.ManyToManyField(

readthedocs/projects/tasks/builds.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,10 @@ class TaskData:
103103
build_commit: str = None
104104

105105
start_time: timezone.datetime = None
106+
107+
# pylint: disable=unsubscriptable-object
106108
environment_class: type[DockerBuildEnvironment] | type[LocalBuildEnvironment] = None
109+
107110
build_director: BuildDirector = None
108111
config: BuildConfigV1 | BuildConfigV2 = None
109112
project: APIProject = None
@@ -572,7 +575,7 @@ def get_valid_artifact_types(self):
572575
artifact_type=artifact_type
573576
)
574577
)
575-
elif artifact_format_files == 0:
578+
if artifact_format_files == 0:
576579
raise BuildUserError(
577580
BuildUserError.BUILD_OUTPUT_HAS_0_FILES.format(
578581
artifact_type=artifact_type
@@ -598,6 +601,7 @@ def on_success(self, retval, task_id, args, kwargs):
598601
"has_pdf": "pdf" in valid_artifacts,
599602
"has_epub": "epub" in valid_artifacts,
600603
"has_htmlzip": "htmlzip" in valid_artifacts,
604+
"build_data": self.data.version.build_data,
601605
}
602606
)
603607
except HttpClientError:

readthedocs/projects/tests/test_build_tasks.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -216,6 +216,7 @@ def test_build_updates_documentation_type(self, load_yaml_config):
216216
assert self.requests_mock.request_history[7]._request.method == "PATCH"
217217
assert self.requests_mock.request_history[7].path == "/api/v2/version/1/"
218218
assert self.requests_mock.request_history[7].json() == {
219+
"build_data": None,
219220
"built": True,
220221
"documentation_type": "mkdocs",
221222
"has_pdf": True,
@@ -457,6 +458,7 @@ def test_successful_build(
457458
assert self.requests_mock.request_history[7]._request.method == "PATCH"
458459
assert self.requests_mock.request_history[7].path == "/api/v2/version/1/"
459460
assert self.requests_mock.request_history[7].json() == {
461+
"build_data": None,
460462
"built": True,
461463
"documentation_type": "sphinx",
462464
"has_pdf": True,

readthedocs/proxito/middleware.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
unresolver,
2727
)
2828
from readthedocs.core.utils import get_cache_tag
29+
from readthedocs.projects.models import Feature, Project
2930

3031
log = structlog.get_logger(__name__)
3132

@@ -278,9 +279,17 @@ def process_request(self, request): # noqa
278279

279280
return None
280281

282+
def add_hosting_integrations_headers(self, request, response):
283+
project_slug = getattr(request, "path_project_slug", "")
284+
if project_slug:
285+
project = Project.objects.get(slug=project_slug)
286+
if project.has_feature(Feature.HOSTING_INTEGRATIONS):
287+
response["X-RTD-Hosting-Integrations"] = "true"
288+
281289
def process_response(self, request, response): # noqa
282290
self.add_proxito_headers(request, response)
283291
self.add_cache_headers(request, response)
284292
self.add_hsts_headers(request, response)
285293
self.add_user_headers(request, response)
294+
self.add_hosting_integrations_headers(request, response)
286295
return response
Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
"""Test hosting views."""
2+
3+
import django_dynamic_fixture as fixture
4+
import pytest
5+
from django.conf import settings
6+
from django.contrib.auth.models import User
7+
from django.test import TestCase, override_settings
8+
from django.urls import reverse
9+
10+
from readthedocs.builds.constants import EXTERNAL, INTERNAL, LATEST
11+
from readthedocs.builds.models import Build
12+
from readthedocs.projects.constants import PUBLIC
13+
from readthedocs.projects.models import Project
14+
15+
16+
@override_settings(
17+
PUBLIC_DOMAIN="dev.readthedocs.io",
18+
PUBLIC_DOMAIN_USES_HTTPS=True,
19+
)
20+
@pytest.mark.proxito
21+
class TestReadTheDocsConfigJson(TestCase):
22+
def setUp(self):
23+
self.user = fixture.get(User, username="user")
24+
self.user.set_password("user")
25+
self.user.save()
26+
27+
self.project = fixture.get(
28+
Project,
29+
slug="project",
30+
language="en",
31+
privacy_level=PUBLIC,
32+
external_builds_privacy_level=PUBLIC,
33+
repo="git://10.10.0.1/project",
34+
programming_language="words",
35+
single_version=False,
36+
users=[self.user],
37+
main_language_project=None,
38+
)
39+
self.project.versions.update(
40+
privacy_level=PUBLIC,
41+
built=True,
42+
active=True,
43+
type=INTERNAL,
44+
identifier="1a2b3c",
45+
)
46+
self.version = self.project.versions.get(slug=LATEST)
47+
self.build = fixture.get(
48+
Build,
49+
version=self.version,
50+
)
51+
52+
def test_get_config(self):
53+
r = self.client.get(
54+
reverse("proxito_readthedocs_config_json"),
55+
{"url": "https://project.dev.readthedocs.io/en/latest/"},
56+
secure=True,
57+
HTTP_HOST="project.dev.readthedocs.io",
58+
)
59+
assert r.status_code == 200
60+
61+
expected = {
62+
"comment": "THIS RESPONSE IS IN ALPHA FOR TEST PURPOSES ONLY AND IT'S GOING TO CHANGE COMPLETELY -- DO NOT USE IT!",
63+
"project": {
64+
"slug": self.project.slug,
65+
"language": self.project.language,
66+
"repository_url": self.project.repo,
67+
"programming_language": self.project.programming_language,
68+
},
69+
"version": {
70+
"slug": self.version.slug,
71+
"external": self.version.type == EXTERNAL,
72+
},
73+
"build": {
74+
"id": self.build.pk,
75+
},
76+
"domains": {
77+
"dashboard": settings.PRODUCTION_DOMAIN,
78+
},
79+
"readthedocs": {
80+
"analytics": {
81+
"code": None,
82+
}
83+
},
84+
"features": {
85+
"analytics": {
86+
"code": None,
87+
},
88+
"external_version_warning": {
89+
"enabled": True,
90+
"query_selector": "[role=main]",
91+
},
92+
"non_latest_version_warning": {
93+
"enabled": True,
94+
"query_selector": "[role=main]",
95+
"versions": [
96+
"latest",
97+
],
98+
},
99+
"doc_diff": {
100+
"enabled": True,
101+
"base_url": "https://project.dev.readthedocs.io/en/latest/index.html",
102+
"root_selector": "[role=main]",
103+
"inject_styles": True,
104+
"base_host": "",
105+
"base_page": "",
106+
},
107+
"flyout": {
108+
"translations": [],
109+
"versions": [
110+
{"slug": "latest", "url": "/en/latest/"},
111+
],
112+
"downloads": [],
113+
"vcs": {
114+
"url": "https://github.com",
115+
"username": "readthedocs",
116+
"repository": "test-builds",
117+
"branch": self.version.identifier,
118+
"filepath": "/docs/index.rst",
119+
},
120+
},
121+
},
122+
}
123+
assert r.json() == expected

readthedocs/proxito/urls.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
from readthedocs.constants import pattern_opts
4141
from readthedocs.core.views import HealthCheckView
4242
from readthedocs.projects.views.public import ProjectDownloadMedia
43+
from readthedocs.proxito.views.hosting import ReadTheDocsConfigJson
4344
from readthedocs.proxito.views.serve import (
4445
ServeDocs,
4546
ServeError404,
@@ -114,6 +115,12 @@
114115
ServeStaticFiles.as_view(),
115116
name="proxito_static_files",
116117
),
118+
# readthedocs-config.js
119+
path(
120+
f"{DOC_PATH_PREFIX}readthedocs-config/",
121+
ReadTheDocsConfigJson.as_view(),
122+
name="proxito_readthedocs_config_json",
123+
),
117124
]
118125

119126
core_urls = [

0 commit comments

Comments
 (0)