Skip to content

Commit

Permalink
Hosting: manual integrations via build contract (#10127)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
humitos authored Mar 20, 2023
1 parent b50e04c commit ed732c2
Show file tree
Hide file tree
Showing 13 changed files with 368 additions and 9 deletions.
11 changes: 11 additions & 0 deletions dockerfiles/nginx/proxito.conf.template
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,17 @@ server {
proxy_hide_header Content-Security-Policy;
set $content_security_policy $upstream_http_content_security_policy;
add_header Content-Security-Policy $content_security_policy always;

# This header allows us to decide whether or not inject the script at CloudFlare level
# Now, I'm injecting it in all the NGINX responses because `sub_filter` is not allowed inside an `if` statement.
set $rtd_hosting_integrations $upstream_http_x_rtd_hosting_integrations;
add_header X-RTD-Hosting-Integrations $rtd_hosting_integrations always;

# Inject our own script dynamically
# TODO: find a way to make this work _without_ running `npm run dev` from the `readthedocs-client` repository
sub_filter '</head>' '<script language="javascript" src="http://localhost:8000/readthedocs-client.js"></script>\n</head>';
sub_filter_last_modified on;
sub_filter_once on;
}

# Serve 404 pages here
Expand Down
10 changes: 8 additions & 2 deletions readthedocs/api/v2/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ class VersionSerializer(serializers.ModelSerializer):

class Meta:
model = Version
fields = (
fields = [
'id',
'project',
'slug',
Expand All @@ -116,14 +116,20 @@ class Meta:
'has_epub',
'has_htmlzip',
'documentation_type',
)
]


class VersionAdminSerializer(VersionSerializer):

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

project = ProjectAdminSerializer()
build_data = serializers.JSONField(required=False, write_only=True)

class Meta(VersionSerializer.Meta):
fields = VersionSerializer.Meta.fields + [
"build_data",
]


class BuildCommandSerializer(serializers.ModelSerializer):
Expand Down
22 changes: 22 additions & 0 deletions readthedocs/builds/migrations/0048_add_build_data.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# Generated by Django 3.2.18 on 2023-03-13 15:15

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
("builds", "0047_build_default_triggered"),
]

operations = [
migrations.AddField(
model_name="version",
name="build_data",
field=models.JSONField(
default=None,
null=True,
verbose_name="Data generated at build time by the doctool (`readthedocs-build.yaml`).",
),
),
]
8 changes: 6 additions & 2 deletions readthedocs/builds/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,9 @@
BITBUCKET_COMMIT_URL,
BITBUCKET_URL,
DOCTYPE_CHOICES,
GITHUB_BRAND,
GITHUB_COMMIT_URL,
GITHUB_PULL_REQUEST_COMMIT_URL,
GITHUB_URL,
GITLAB_BRAND,
GITLAB_COMMIT_URL,
GITLAB_MERGE_REQUEST_COMMIT_URL,
GITLAB_URL,
Expand Down Expand Up @@ -186,6 +184,12 @@ class Version(TimeStampedModel):
),
)

build_data = models.JSONField(
_("Data generated at build time by the doctool (`readthedocs-build.yaml`)."),
default=None,
null=True,
)

objects = VersionManager.from_queryset(VersionQuerySet)()
# Only include BRANCH, TAG, UNKNOWN type Versions.
internal = InternalVersionManager.from_queryset(partial(VersionQuerySet, internal_only=True))()
Expand Down
37 changes: 37 additions & 0 deletions readthedocs/doc_builder/director.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import tarfile

import structlog
import yaml
from django.conf import settings
from django.utils.translation import gettext_lazy as _

Expand Down Expand Up @@ -187,6 +188,7 @@ def build(self):
self.build_epub()

self.run_build_job("post_build")
self.store_readthedocs_build_yaml()

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

def install_build_tools(self):
"""
Expand Down Expand Up @@ -625,3 +628,37 @@ def get_build_env_vars(self):
def is_type_sphinx(self):
"""Is documentation type Sphinx."""
return "sphinx" in self.data.config.doctype

def store_readthedocs_build_yaml(self):
# load YAML from user
yaml_path = os.path.join(
self.data.project.artifact_path(
version=self.data.version.slug, type_="html"
),
"readthedocs-build.yaml",
)

if not os.path.exists(yaml_path):
log.debug("Build output YAML file (readtehdocs-build.yaml) does not exist.")
return

try:
with open(yaml_path, "r") as f:
data = yaml.safe_load(f)
except Exception:
# NOTE: skip this work for now until we decide whether or not this
# YAML file is required.
#
# NOTE: decide whether or not we want this
# file to be mandatory and raise an exception here.
return

log.info("readthedocs-build.yaml loaded.", path=yaml_path)

# TODO: validate the YAML generated by the user
# self._validate_readthedocs_build_yaml(data)

# Copy the YAML data into `Version.build_data`.
# It will be saved when the API is hit.
# This data will be used by the `/_/readthedocs-config.json` API endpoint.
self.data.version.build_data = data
5 changes: 5 additions & 0 deletions readthedocs/projects/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1884,6 +1884,7 @@ def add_features(sender, **kwargs):
CANCEL_OLD_BUILDS = "cancel_old_builds"
DONT_CREATE_INDEX = "dont_create_index"
USE_RCLONE = "use_rclone"
HOSTING_INTEGRATIONS = "hosting_integrations"

FEATURES = (
(ALLOW_DEPRECATED_WEBHOOKS, _('Allow deprecated webhook views')),
Expand Down Expand Up @@ -2058,6 +2059,10 @@ def add_features(sender, **kwargs):
USE_RCLONE,
_("Use rclone for syncing files to the media storage."),
),
(
HOSTING_INTEGRATIONS,
_("Inject 'readthedocs-client.js' as <script> HTML tag in responses."),
),
)

projects = models.ManyToManyField(
Expand Down
6 changes: 5 additions & 1 deletion readthedocs/projects/tasks/builds.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,10 @@ class TaskData:
build_commit: str = None

start_time: timezone.datetime = None

# pylint: disable=unsubscriptable-object
environment_class: type[DockerBuildEnvironment] | type[LocalBuildEnvironment] = None

build_director: BuildDirector = None
config: BuildConfigV1 | BuildConfigV2 = None
project: APIProject = None
Expand Down Expand Up @@ -572,7 +575,7 @@ def get_valid_artifact_types(self):
artifact_type=artifact_type
)
)
elif artifact_format_files == 0:
if artifact_format_files == 0:
raise BuildUserError(
BuildUserError.BUILD_OUTPUT_HAS_0_FILES.format(
artifact_type=artifact_type
Expand All @@ -598,6 +601,7 @@ def on_success(self, retval, task_id, args, kwargs):
"has_pdf": "pdf" in valid_artifacts,
"has_epub": "epub" in valid_artifacts,
"has_htmlzip": "htmlzip" in valid_artifacts,
"build_data": self.data.version.build_data,
}
)
except HttpClientError:
Expand Down
2 changes: 2 additions & 0 deletions readthedocs/projects/tests/test_build_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@ def test_build_updates_documentation_type(self, load_yaml_config):
assert self.requests_mock.request_history[7]._request.method == "PATCH"
assert self.requests_mock.request_history[7].path == "/api/v2/version/1/"
assert self.requests_mock.request_history[7].json() == {
"build_data": None,
"built": True,
"documentation_type": "mkdocs",
"has_pdf": True,
Expand Down Expand Up @@ -457,6 +458,7 @@ def test_successful_build(
assert self.requests_mock.request_history[7]._request.method == "PATCH"
assert self.requests_mock.request_history[7].path == "/api/v2/version/1/"
assert self.requests_mock.request_history[7].json() == {
"build_data": None,
"built": True,
"documentation_type": "sphinx",
"has_pdf": True,
Expand Down
9 changes: 9 additions & 0 deletions readthedocs/proxito/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
unresolver,
)
from readthedocs.core.utils import get_cache_tag
from readthedocs.projects.models import Feature, Project

log = structlog.get_logger(__name__)

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

return None

def add_hosting_integrations_headers(self, request, response):
project_slug = getattr(request, "path_project_slug", "")
if project_slug:
project = Project.objects.get(slug=project_slug)
if project.has_feature(Feature.HOSTING_INTEGRATIONS):
response["X-RTD-Hosting-Integrations"] = "true"

def process_response(self, request, response): # noqa
self.add_proxito_headers(request, response)
self.add_cache_headers(request, response)
self.add_hsts_headers(request, response)
self.add_user_headers(request, response)
self.add_hosting_integrations_headers(request, response)
return response
123 changes: 123 additions & 0 deletions readthedocs/proxito/tests/test_hosting.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
"""Test hosting views."""

import django_dynamic_fixture as fixture
import pytest
from django.conf import settings
from django.contrib.auth.models import User
from django.test import TestCase, override_settings
from django.urls import reverse

from readthedocs.builds.constants import EXTERNAL, INTERNAL, LATEST
from readthedocs.builds.models import Build
from readthedocs.projects.constants import PUBLIC
from readthedocs.projects.models import Project


@override_settings(
PUBLIC_DOMAIN="dev.readthedocs.io",
PUBLIC_DOMAIN_USES_HTTPS=True,
)
@pytest.mark.proxito
class TestReadTheDocsConfigJson(TestCase):
def setUp(self):
self.user = fixture.get(User, username="user")
self.user.set_password("user")
self.user.save()

self.project = fixture.get(
Project,
slug="project",
language="en",
privacy_level=PUBLIC,
external_builds_privacy_level=PUBLIC,
repo="git://10.10.0.1/project",
programming_language="words",
single_version=False,
users=[self.user],
main_language_project=None,
)
self.project.versions.update(
privacy_level=PUBLIC,
built=True,
active=True,
type=INTERNAL,
identifier="1a2b3c",
)
self.version = self.project.versions.get(slug=LATEST)
self.build = fixture.get(
Build,
version=self.version,
)

def test_get_config(self):
r = self.client.get(
reverse("proxito_readthedocs_config_json"),
{"url": "https://project.dev.readthedocs.io/en/latest/"},
secure=True,
HTTP_HOST="project.dev.readthedocs.io",
)
assert r.status_code == 200

expected = {
"comment": "THIS RESPONSE IS IN ALPHA FOR TEST PURPOSES ONLY AND IT'S GOING TO CHANGE COMPLETELY -- DO NOT USE IT!",
"project": {
"slug": self.project.slug,
"language": self.project.language,
"repository_url": self.project.repo,
"programming_language": self.project.programming_language,
},
"version": {
"slug": self.version.slug,
"external": self.version.type == EXTERNAL,
},
"build": {
"id": self.build.pk,
},
"domains": {
"dashboard": settings.PRODUCTION_DOMAIN,
},
"readthedocs": {
"analytics": {
"code": None,
}
},
"features": {
"analytics": {
"code": None,
},
"external_version_warning": {
"enabled": True,
"query_selector": "[role=main]",
},
"non_latest_version_warning": {
"enabled": True,
"query_selector": "[role=main]",
"versions": [
"latest",
],
},
"doc_diff": {
"enabled": True,
"base_url": "https://project.dev.readthedocs.io/en/latest/index.html",
"root_selector": "[role=main]",
"inject_styles": True,
"base_host": "",
"base_page": "",
},
"flyout": {
"translations": [],
"versions": [
{"slug": "latest", "url": "/en/latest/"},
],
"downloads": [],
"vcs": {
"url": "https://github.com",
"username": "readthedocs",
"repository": "test-builds",
"branch": self.version.identifier,
"filepath": "/docs/index.rst",
},
},
},
}
assert r.json() == expected
7 changes: 7 additions & 0 deletions readthedocs/proxito/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
from readthedocs.constants import pattern_opts
from readthedocs.core.views import HealthCheckView
from readthedocs.projects.views.public import ProjectDownloadMedia
from readthedocs.proxito.views.hosting import ReadTheDocsConfigJson
from readthedocs.proxito.views.serve import (
ServeDocs,
ServeError404,
Expand Down Expand Up @@ -114,6 +115,12 @@
ServeStaticFiles.as_view(),
name="proxito_static_files",
),
# readthedocs-config.js
path(
f"{DOC_PATH_PREFIX}readthedocs-config/",
ReadTheDocsConfigJson.as_view(),
name="proxito_readthedocs_config_json",
),
]

core_urls = [
Expand Down
Loading

0 comments on commit ed732c2

Please sign in to comment.