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

Hosting: manual integrations via build contract #10127

Merged
merged 43 commits into from
Mar 20, 2023
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
5b28071
Hosting: manual integrations via build contract
humitos Mar 8, 2023
e18b40f
Use a single script to load everything
humitos Mar 8, 2023
7754d5f
Include Read the Docs analytics to integrations
humitos Mar 8, 2023
2925ed9
Initial work for hosting features
humitos Mar 8, 2023
3385649
External version banner and doc-diff integration
humitos Mar 8, 2023
1e391b8
Old version warning
humitos Mar 8, 2023
6afca0b
Do not inject doc-diff on search page
humitos Mar 8, 2023
7ce98a4
Inject old version warning only for non-external versions
humitos Mar 8, 2023
3ca96ec
Comments!
humitos Mar 8, 2023
f4f1268
More comments
humitos Mar 8, 2023
a596512
Build: pass `PATH` environment variable to Docker container
humitos Mar 9, 2023
33fdb2b
Lint: for some reason fails at CircleCI otherwise
humitos Mar 9, 2023
4ced5c3
Merge branch 'humitos/build-cmd-environment' of github.com:readthedoc…
humitos Mar 9, 2023
ea2af4c
Feature flag for new hosting integrations
humitos Mar 10, 2023
79b7393
Load `readthedocs-build.yaml` and generate `readthedocs-data.html`
humitos Mar 10, 2023
17effaf
Load READTHEDOCS_DATA async
humitos Mar 10, 2023
2b9cdbf
Absolute proxied API path
humitos Mar 10, 2023
0116f41
Remove duplicated code
humitos Mar 10, 2023
d5130cc
New approach using `readthedocs-client.js` and `/_/readthedocs-config…
humitos Mar 11, 2023
761e3b6
Do not require `readthedocs-build.YAML` for now
humitos Mar 11, 2023
bd9f70e
Expand the JSON response with more data
humitos Mar 13, 2023
842a228
Remove non-required files and rely on `readthedocs-client.js` only
humitos Mar 13, 2023
2ad30cc
Improve helper text
humitos Mar 13, 2023
89662fa
Builds: save `readthedocs-build.yaml` into database
humitos Mar 13, 2023
f83eee6
Use `Version.build_data` from the endpoint
humitos Mar 13, 2023
d14115a
Flyout: return data required to generate flyout dynamically
humitos Mar 13, 2023
067bb4c
Updates to the API
humitos Mar 14, 2023
17c1af3
Minor updates
humitos Mar 15, 2023
53366aa
Update the javascript client compiled version
humitos Mar 15, 2023
0f89186
doc-diff object returned
humitos Mar 16, 2023
48de597
Build: check if the YAML file exists before trying to open it
humitos Mar 16, 2023
4b05e77
Proxito: don't inject the header if the feature is turned off
humitos Mar 16, 2023
e90af75
Merge branch 'main' of github.com:readthedocs/readthedocs.org into hu…
humitos Mar 16, 2023
364de9c
Test: add hosting integrations tests
humitos Mar 16, 2023
c1cf8cb
Remove JS
humitos Mar 20, 2023
3930915
Load the javascript from a local server for now
humitos Mar 20, 2023
2d7b6fe
Update URL to remove .json from it
humitos Mar 20, 2023
ab43635
Remove non-required f-string
humitos Mar 20, 2023
85ab2e7
Allow saving `build_data` via API endpoint
humitos Mar 20, 2023
3439e24
Merge branch 'main' of github.com:readthedocs/readthedocs.org into hu…
humitos Mar 20, 2023
6154c7f
Lint
humitos Mar 20, 2023
0e8f245
Migrate nodejs installation to asdf
humitos Mar 20, 2023
a6c5977
Change port to match `npm run dev` from readthedocs-client
humitos Mar 20, 2023
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
12 changes: 12 additions & 0 deletions dockerfiles/nginx/proxito.conf.template
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,18 @@ 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;
humitos marked this conversation as resolved.
Show resolved Hide resolved

# Inject our own script dynamically
# TODO: decide where is the best place to do this
sub_filter '</head>' '<script language="javascript" src="http://devthedocs.org/static/core/js/integrations.js"></script>\n</head>';
# sub_filter_types text/html;
sub_filter_last_modified on;
sub_filter_once on;
}
humitos marked this conversation as resolved.
Show resolved Hide resolved

# Serve 404 pages here
Expand Down
44 changes: 44 additions & 0 deletions readthedocs/core/static/core/js/integrations.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// Unique entry point that our servers will inject

// Load Read the Docs data first
// This data is generated at build time by the doctool
fetch("/en/manual-integration-docsify/readthedocs-data.html", {method: 'GET'})
humitos marked this conversation as resolved.
Show resolved Hide resolved
.then(response => {
return response.text();
})
.then(text => {
document.head.insertAdjacentHTML("beforeend", text);
})
.then(() => {
// TODO: use `READTHEDOCS_DATA.features.hosting.version` to decide
// what version of these Javascript libraries we need to inject here.
// See https://github.com/readthedocs/readthedocs.org/issues/9063#issuecomment-1325483505
let link = document.createElement("link");
link.setAttribute("rel", "stylesheet");
link.setAttribute("type", "text/css");
link.setAttribute("href", "/_/static/css/readthedocs-doc-embed.css");
document.head.appendChild(link);

let embed = document.createElement("script");
embed.setAttribute("async", "async");
embed.setAttribute("src", "/_/static/javascript/readthedocs-doc-embed.js");
document.head.appendChild(embed);

let analytics = document.createElement("script");
analytics.setAttribute("async", "async");
analytics.setAttribute("src", "/_/static/javascript/readthedocs-analytics.js");
document.head.appendChild(analytics);

let hosting = document.createElement("script");
hosting.setAttribute("async", "async");
hosting.setAttribute("src", "/_/static/core/js/readthedocs-hosting.js");
document.head.appendChild(hosting);
humitos marked this conversation as resolved.
Show resolved Hide resolved

// TODO: insert search-as-you-type once it becomes a JS library
// decoupled from Sphinx.
// See https://github.com/readthedocs/readthedocs-sphinx-search/issues/67

// TODO: find a way to remove the Sphinx default search, since it duplicates the results sometimes.
// This removal is currently happening at our Sphinx extension:
// https://github.com/readthedocs/readthedocs-sphinx-ext/blob/7cc1e60f7dcdeb7af35e3479509a621d5bac0976/readthedocs_ext/readthedocs.py#L239
});
1 change: 1 addition & 0 deletions readthedocs/core/static/core/js/readthedocs-doc-diff.js

Large diffs are not rendered by default.

77 changes: 77 additions & 0 deletions readthedocs/core/static/core/js/readthedocs-hosting.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
READTHEDOCS_DATA = JSON.parse(document.getElementById('READTHEDOCS_DATA').textContent);

function injectExternalVersionWarning() {
// TODO: make all these banners (injected HTML) templates that users can override with their own.
// This way, we allow customization of the look&feel without compromising the logic.
let admonition = `
<div class="admonition warning">
<p class="admonition-title">Warning</p>
<p>
This page
<a class="reference external" href="${ window.location.protocol }//${ window.location.hostname }/projects/${ READTHEDOCS_DATA.project }/builds/${ READTHEDOCS_DATA.build.id }/">was created </a>
from a pull request
(<a class="reference external" href="${ READTHEDOCS_DATA.repository_url }/pull/${ READTHEDOCS_DATA.version }">#${ READTHEDOCS_DATA.version }</a>).
</p>
</div>
`;

let main = document.querySelector('[role=main]') || document.querySelector('#main');
let node = document.createElement("div");
node.innerHTML = admonition;
main.insertBefore(node, main.firstChild);
}

function injectDocDiff() {
let docdiff = document.createElement("script");
docdiff.setAttribute("async", "async");
docdiff.setAttribute("src", "/_/static/core/js/readthedocs-doc-diff.js");
document.head.appendChild(docdiff);
}

function injectOldVersionWarning() {
// TODO: compute if the user is reading the latest version or not before showing the warning.
let admonition = `
<div class="admonition warning">
<p class="admonition-title">Warning</p>
<p>
This page documents version
<a class="reference" href="${ window.location.protocol }//${ window.location.hostname }/${ READTHEDOCS_DATA.language }/${ READTHEDOCS_DATA.version }/">${ READTHEDOCS_DATA.version }</a>.
The latest version is
<a class="reference" href="${ window.location.protocol }//${ window.location.hostname }/${ READTHEDOCS_DATA.language }/${ READTHEDOCS_DATA.version }/">${ READTHEDOCS_DATA.version }</a>.
</p>
</div>
`;

// Borrowed and adapted from:
// https://github.com/readthedocs/readthedocs.org/blob/7ce98a4d4f34a4c1845dc6e3e0e5112af7c39b0c/readthedocs/core/static-src/core/js/doc-embed/version-compare.js#L1
let main = document.querySelector('[role=main]') || document.querySelector('#main');
let node = document.createElement("div");
node.innerHTML = admonition;
main.insertBefore(node, main.firstChild);
}


window.addEventListener("load", (event) => {
// TODO: use the proxied API here
// "repository_url" could be retrived using the API, but there are some CORS issues and design decisions
// that it's probably smart to avoid and just rely on a fixed value from the build output :)
if (READTHEDOCS_DATA.build.external_version === true) {
injectExternalVersionWarning();

if (READTHEDOCS_DATA.features.docdiff.enabled === true) {
if(!window.location.pathname.endsWith("search.html")) {
// Avoid injecting doc-diff on search page because it doesn't make sense
injectDocDiff();
}
}
}
else {
// TODO: there some data we need from `/api/v2/footer_html/`,
// like `is_highest` and `show_version_warning`.
// However, this call is happening inside the `readthedocs-doc-embed.js`.
// We could make the call again here for now as demo,
// but it would be good to refactor that code
// Inject old version warning only for non-external versions
injectOldVersionWarning();
}
});
91 changes: 91 additions & 0 deletions readthedocs/doc_builder/director.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@
import tarfile

import structlog
import yaml
from django.conf import settings
from django.template.loader import render_to_string
from django.utils import timezone
from django.utils.translation import gettext_lazy as _

from readthedocs.builds.constants import EXTERNAL
Expand Down Expand Up @@ -187,6 +190,7 @@ def build(self):
self.build_epub()

self.run_build_job("post_build")
self.generate_readthedocs_data_javascript()

after_build.send(
sender=self.data.version,
Expand Down Expand Up @@ -392,6 +396,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.generate_readthedocs_data_javascript()

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

def generate_readthedocs_data_javascript(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",
)

try:
log.warning(yaml_path)
data = yaml.safe_load(open(yaml_path, "r"))
except Exception:
# TODO: Improve this message
raise BuildUserError(
"The required 'readthedocs-build.yaml' file was not provided."
)

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

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

# Populate the data provided by the doctool with data we have from the build context
# TODO: this is the new structure that has to be defined
# context = {
# "project": {
# "slug": self.data.project.slug,
# },
# "version": {
# "slug": self.data.version.slug,
# },
# "build": {
# "id": self.data.build["pk"],
# },
# }

# NOTE: this is the OLD structure.
# This will be changed for a new structure that we have to decide yet.
# I'm using the old one for now just to keep compatibility.
context = {
"ad_free": False,
"api_host": "http://devthedocs.org",
"build_date": timezone.now(),
"builder": "$DOCTOOL_NAME",
"canonical_url": self.data.project.canonical_url,
"commit": self.data.build["commit"],
"docroot": "$DOCTOOL_DOCROOT",
"language": self.data.project.language,
"page": "$DOCTOOL_PAGE", # Can we define this dynamically via Javascript?
"programming_language": self.data.project.programming_language,
"project": self.data.project.slug,
"version": self.data.version.slug,
"source_suffix": "$DOCTOOL_SOURCE_SUFFIX",
"theme": "$DOCTOOL_THEME",
# These can be removed
"user_analytics_code": None,
"global_analytics_code": None,
"proxied_api_host": f"/{settings.DOC_PATH_PREFIX}",
"subprojects": None,
# TODO: remove the following ones, they are just my own tests
# NOTE: eventually, some of these settings should be enabled/disabled by the reader
"build": {
"id": self.data.build_pk,
"external_version": self.data.version.type == EXTERNAL,
},
"repository_url": self.data.project.repo,
}

# Update user's generated data with our own data.
data.update(context)

js_path = os.path.join(
self.data.project.artifact_path(
version=self.data.version.slug, type_="html"
),
"readthedocs-data.html",
)
content = render_to_string(
template_name="doc_builder/readthedocs-data.html",
context=dict(data=context),
)
with open(js_path, "w") as f:
f.write(content)
log.info("readthedocs-data.html written.", path=js_path)
67 changes: 39 additions & 28 deletions readthedocs/doc_builder/environments.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from docker.errors import APIError as DockerAPIError
from docker.errors import DockerException
from docker.errors import NotFound as DockerNotFoundError
from requests.exceptions import ConnectionError, ReadTimeout
from requests.exceptions import ConnectionError, ReadTimeout # noqa
from requests_toolbelt.multipart.encoder import MultipartEncoder

from readthedocs.api.v2.client import api as api_v2
Expand Down Expand Up @@ -73,7 +73,7 @@ def __init__(
bin_path=None,
record_as_success=False,
demux=False,
**kwargs,
**kwargs, # pylint: disable=unused-argument
):
self.command = command
self.shell = shell
Expand Down Expand Up @@ -252,8 +252,8 @@ def save(self):
{key: str(value) for key, value in data.items()}
)
resource = api_v2.command
resp = resource._store['session'].post(
resource._store['base_url'] + '/',
resp = resource._store["session"].post( # pylint: disable=protected-access
resource._store["base_url"] + "/", # pylint: disable=protected-access
data=encoder,
headers={
'Content-Type': encoder.content_type,
Expand Down Expand Up @@ -301,11 +301,35 @@ def run(self):

self.start_time = datetime.utcnow()
client = self.build_env.get_client()

# Create a copy of the environment to update PATH variable
environment = self._environment.copy()
# Default PATH variable
environment[
"PATH"
] = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
# Add asdf extra paths
environment["PATH"] += (
":/home/{settings.RTD_DOCKER_USER}/.asdf/shims"
":/home/{settings.RTD_DOCKER_USER}/.asdf/bin"
)

if settings.RTD_DOCKER_COMPOSE:
environment["PATH"] += ":/root/.asdf/shims:/root/.asdf/bin"

# Prepend the BIN_PATH if it's defined
if self.bin_path:
path = environment.get("PATH")
bin_path = self._escape_command(self.bin_path)
environment["PATH"] = bin_path
if path:
environment["PATH"] += f":{path}"

try:
exec_cmd = client.exec_create(
container=self.build_env.container_id,
cmd=self.get_wrapped_command(),
environment=self._environment,
environment=environment,
user=self.user,
workdir=self.cwd,
stdout=True,
Expand Down Expand Up @@ -357,31 +381,18 @@ def get_wrapped_command(self):
"""
Wrap command in a shell and optionally escape special bash characters.

In order to set the current working path inside a docker container, we
need to wrap the command in a shell call manually.

Some characters will be interpreted as shell characters without
escaping, such as: ``pip install requests<0.8``. When passing
``escape_command=True`` in the init method this escapes a good majority
of those characters.
"""
prefix = ''
if self.bin_path:
bin_path = self._escape_command(self.bin_path)
prefix += f'PATH={bin_path}:$PATH '

command = (
' '.join(
self._escape_command(part) if self.escape_command else part
for part in self.command
)
)
return (
"/bin/sh -c '{prefix}{cmd}'".format(
prefix=prefix,
cmd=command,
)
)
return f"/bin/bash -c '{command}'"

def _escape_command(self, cmd):
r"""Escape the command by prefixing suspicious chars with `\`."""
Expand Down Expand Up @@ -524,14 +535,14 @@ class BuildEnvironment(BaseEnvironment):
"""

def __init__(
self,
project=None,
version=None,
build=None,
config=None,
environment=None,
record=True,
**kwargs,
self,
project=None,
version=None,
build=None,
config=None,
environment=None,
record=True,
**kwargs, # pylint: disable=unused-argument
):
super().__init__(project, environment)
self.version = version
Expand All @@ -557,7 +568,7 @@ def run(self, *cmd, **kwargs):
})
return super().run(*cmd, **kwargs)

def run_command_class(self, *cmd, **kwargs): # pylint: disable=arguments-differ
def run_command_class(self, *cmd, **kwargs): # pylint: disable=signature-differs
kwargs.update({
'build_env': self,
})
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{{ data|json_script:"READTHEDOCS_DATA" }}
humitos marked this conversation as resolved.
Show resolved Hide resolved

{% comment %}
The resulting file, will be available to use as:
const value = JSON.parse(document.getElementById('readthedocs-data').textContent);
{% endcomment %}
Loading