Skip to content

Commit e8ce592

Browse files
authored
Builds: don't use an build API token for healthcheck (#12350)
Make that endpoint public. The endpoint checks the build is not in final states and the builder hostname matches with the builder saved in the database for that build.
1 parent 5cdca50 commit e8ce592

File tree

4 files changed

+17
-15
lines changed

4 files changed

+17
-15
lines changed

readthedocs/api/v2/views/model_views.py

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
from readthedocs.aws.security_token_service import AWSTemporaryCredentialsError
3333
from readthedocs.aws.security_token_service import get_s3_build_media_scoped_credentials
3434
from readthedocs.aws.security_token_service import get_s3_build_tools_scoped_credentials
35+
from readthedocs.builds.constants import BUILD_FINAL_STATES
3536
from readthedocs.builds.constants import INTERNAL
3637
from readthedocs.builds.models import Build
3738
from readthedocs.builds.models import BuildCommandResult
@@ -300,22 +301,24 @@ def concurrent(self, request, **kwargs):
300301

301302
@decorators.action(
302303
detail=True,
303-
permission_classes=[HasBuildAPIKey],
304+
# We make this endpoint public because we don't want to expose the build API key inside the user's container.
305+
# To emulate "auth" we check for the builder hostname to match the `Build.builder` defined in the database.
306+
permission_classes=[],
304307
methods=["post"],
305308
)
306309
def healthcheck(self, request, **kwargs):
307310
build = self.get_object()
308-
log.debug(
309-
"Healthcheck received.",
311+
builder_hostname = request.GET.get("builder")
312+
structlog.contextvars.bind_contextvars(
310313
build_id=build.pk,
311314
project_slug=build.version.project.slug,
315+
builder_hostname=builder_hostname,
312316
)
313-
build_api_key = request.build_api_key
314-
if build.version.project.slug != build_api_key.project.slug:
317+
318+
log.info("Healthcheck received.")
319+
if build.state in BUILD_FINAL_STATES or build.builder != builder_hostname:
315320
log.warning(
316-
"Project slug doesn't match the one attached to the API key.",
317-
api_key_id=build_api_key.id,
318-
project_slug=build.version.project.slug,
321+
"Build is not running anymore or builder hostname doesn't match.",
319322
)
320323
raise Http404()
321324

readthedocs/doc_builder/director.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,6 @@ def create_vcs_environment(self):
120120
environment=self.get_vcs_env_vars(),
121121
container_image=settings.RTD_DOCKER_CLONE_IMAGE,
122122
api_client=self.data.api_client,
123-
build_api_key=self.data.build_api_key,
124123
)
125124

126125
def create_build_environment(self):
@@ -131,7 +130,6 @@ def create_build_environment(self):
131130
build=self.data.build,
132131
environment=self.get_build_env_vars(),
133132
api_client=self.data.api_client,
134-
build_api_key=self.data.build_api_key,
135133
)
136134

137135
def setup_environment(self):

readthedocs/doc_builder/environments.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -585,7 +585,6 @@ class DockerBuildEnvironment(BaseBuildEnvironment):
585585
container_time_limit = DOCKER_LIMITS.get("time")
586586

587587
def __init__(self, *args, **kwargs):
588-
self.build_api_key = kwargs.pop("build_api_key", None)
589588
container_image = kwargs.pop("container_image", None)
590589
super().__init__(*args, **kwargs)
591590
self.client = None
@@ -862,6 +861,7 @@ def _run_background_healthcheck(self):
862861
log.debug("Running build with healthcheck.")
863862

864863
build_id = self.build.get("id")
864+
build_builder = self.build.get("builder")
865865
healthcheck_url = reverse("build-healthcheck", kwargs={"pk": build_id})
866866
if settings.RTD_DOCKER_COMPOSE and "ngrok" in settings.PRODUCTION_DOMAIN:
867867
# NOTE: we do require using NGROK here to go over internet because I
@@ -874,7 +874,10 @@ def _run_background_healthcheck(self):
874874
else:
875875
url = f"{settings.SLUMBER_API_HOST}{healthcheck_url}"
876876

877-
cmd = f"/bin/bash -c 'while true; do curl --max-time 2 -H \"Authorization: Token {self.build_api_key}\" -X POST {url}; sleep {settings.RTD_BUILD_HEALTHCHECK_DELAY}; done;'"
877+
# Add the builder hostname to the URL
878+
url += f"?builder={build_builder}"
879+
880+
cmd = f"/bin/bash -c 'while true; do curl --max-time 2 -X POST {url}; sleep {settings.RTD_BUILD_HEALTHCHECK_DELAY}; done;'"
878881
log.debug("Healthcheck command to run.", command=cmd)
879882

880883
client = self.get_client()

readthedocs/projects/tasks/builds.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,6 @@ class TaskData:
108108

109109
# Slumber client to interact with the API v2.
110110
api_client: API = None
111-
build_api_key: str = None
112111

113112
start_time: timezone.datetime = None
114113
environment_class: type[DockerBuildEnvironment] | type[LocalBuildEnvironment] = None
@@ -382,8 +381,7 @@ def before_start(self, task_id, args, kwargs):
382381
# anymore and we are not using it
383382
self.data.environment_class = LocalBuildEnvironment
384383

385-
self.data.build_api_key = kwargs["build_api_key"]
386-
self.data.api_client = setup_api(self.data.build_api_key)
384+
self.data.api_client = setup_api(kwargs["build_api_key"])
387385

388386
self.data.build = self.get_build(self.data.build_pk)
389387
self.data.version = self.get_version(self.data.version_pk)

0 commit comments

Comments
 (0)