From 4c869b856c049938b5c79baef9e966a64112cc96 Mon Sep 17 00:00:00 2001 From: Aleksandr Meshchryakov Date: Wed, 16 Sep 2026 09:39:56 +0200 Subject: [PATCH] fix: exclude infrastructure probes from API throttling --- .gitea/workflows/ci-cd.yml | 61 ------------------ docker/Dockerfile.health-hotfix | 5 ++ docs/health-probes.md | 36 +++++++++++ src/apps/core/views.py | 5 +- src/core/openapi.json | 32 ---------- tests/apps/core/test_views.py | 96 ++++++++++++++++++++++++++--- tests/test_deployment_boundaries.py | 26 ++++++++ 7 files changed, 157 insertions(+), 104 deletions(-) create mode 100644 docker/Dockerfile.health-hotfix create mode 100644 docs/health-probes.md create mode 100644 tests/test_deployment_boundaries.py diff --git a/.gitea/workflows/ci-cd.yml b/.gitea/workflows/ci-cd.yml index 7276820..08220fb 100644 --- a/.gitea/workflows/ci-cd.yml +++ b/.gitea/workflows/ci-cd.yml @@ -20,9 +20,6 @@ env: REGISTRY_NAMESPACE: ${{ github.repository_owner }} WEB_IMAGE: mostovik-backend-web CELERY_IMAGE: mostovik-backend-celery - CUSTOMER_DEPLOY_HOST: 10.0.10.174 - CUSTOMER_DEPLOY_USER: ecos - CUSTOMER_SSH_PROXY_HOST: ecos-proxy@10.10.0.121 jobs: quality: @@ -303,61 +300,3 @@ jobs: done echo "Internal-main backend readiness failed" >&2 exit 1 - - deploy_customer_main: - name: Deploy customer main - runs-on: ubuntu-latest - timeout-minutes: 30 - needs: [build, deploy_internal_main] - if: ${{ github.ref == 'refs/heads/main' && needs.build.result == 'success' && needs.deploy_internal_main.result == 'success' }} - - steps: - - name: Invoke customer release wrapper with the verified digests - env: - CUSTOMER_DEPLOY_SSH_KEY_B64: ${{ secrets.CUSTOMER_DEPLOY_SSH_KEY_B64 }} - CUSTOMER_KNOWN_HOSTS_B64: ${{ secrets.CUSTOMER_KNOWN_HOSTS_B64 }} - WEB_REF: ${{ needs.build.outputs.web_ref }} - CELERY_REF: ${{ needs.build.outputs.celery_ref }} - run: | - set -euo pipefail - for name in CUSTOMER_DEPLOY_HOST CUSTOMER_DEPLOY_USER CUSTOMER_SSH_PROXY_HOST CUSTOMER_DEPLOY_SSH_KEY_B64 CUSTOMER_KNOWN_HOSTS_B64; do - if [ -z "${!name:-}" ]; then - echo "Missing required customer SSH secret: ${name}" >&2 - exit 1 - fi - done - case "${CUSTOMER_DEPLOY_USER}" in *[!A-Za-z0-9._-]*) echo "Customer SSH user has unsupported characters" >&2; exit 1;; esac - case "${CUSTOMER_DEPLOY_HOST}" in *[!A-Za-z0-9.:-]*) echo "Customer SSH host has unsupported characters" >&2; exit 1;; esac - case "${CUSTOMER_SSH_PROXY_HOST}" in *[!A-Za-z0-9._@:-]*) echo "Customer SSH proxy has unsupported characters" >&2; exit 1;; esac - printf '%s\n' "${WEB_REF}" | grep -Eq '@sha256:[0-9a-f]{64}$' - printf '%s\n' "${CELERY_REF}" | grep -Eq '@sha256:[0-9a-f]{64}$' - - ssh_dir="$(mktemp -d)" - trap 'rm -rf "${ssh_dir}"' EXIT - key_path="${ssh_dir}/deploy_key" - known_hosts_path="${ssh_dir}/known_hosts" - printf '%s' "${CUSTOMER_DEPLOY_SSH_KEY_B64}" | base64 -d > "${key_path}" - printf '%s' "${CUSTOMER_KNOWN_HOSTS_B64}" | base64 -d > "${known_hosts_path}" - chmod 600 "${key_path}" "${known_hosts_path}" - ssh-keygen -y -f "${key_path}" >/dev/null - - ssh_common=( - -i "${key_path}" - -o BatchMode=yes - -o IdentitiesOnly=yes - -o StrictHostKeyChecking=yes - -o "UserKnownHostsFile=${known_hosts_path}" - -o GlobalKnownHostsFile=/dev/null - -o ConnectTimeout=15 - ) - proxy_command="ssh -i ${key_path} -o BatchMode=yes -o IdentitiesOnly=yes -o StrictHostKeyChecking=yes -o UserKnownHostsFile=${known_hosts_path} -o GlobalKnownHostsFile=/dev/null -o ConnectTimeout=15 -W %h:%p ${CUSTOMER_SSH_PROXY_HOST}" - - ssh \ - "${ssh_common[@]}" \ - -o "ProxyCommand=${proxy_command}" \ - "${CUSTOMER_DEPLOY_USER}@${CUSTOMER_DEPLOY_HOST}" \ - /ecos/release-service \ - mostovik-backend \ - "${GITHUB_RUN_ID:-${GITHUB_SHA}}-${GITHUB_RUN_ATTEMPT:-1}" \ - "${WEB_REF}" \ - "${CELERY_REF}" diff --git a/docker/Dockerfile.health-hotfix b/docker/Dockerfile.health-hotfix new file mode 100644 index 0000000..e11e44d --- /dev/null +++ b/docker/Dockerfile.health-hotfix @@ -0,0 +1,5 @@ +# Use only with the verified deployed image and a views-only application diff. +ARG HEALTH_BASE_IMAGE +FROM ${HEALTH_BASE_IMAGE} + +COPY --chown=0:0 --chmod=0644 src/apps/core/views.py /app/src/apps/core/views.py diff --git a/docs/health-probes.md b/docs/health-probes.md new file mode 100644 index 0000000..48d5b5f --- /dev/null +++ b/docs/health-probes.md @@ -0,0 +1,36 @@ +# Health probes + +`GET /health/live/` and `GET /health/ready/` are unauthenticated infrastructure +probes and do not consume API rate limits. Liveness reports the running process; +readiness checks the database and still returns HTTP 503 when it is unavailable. +The comprehensive `/health/` endpoint and ordinary API endpoints retain their +configured throttling. + +## Internal-main health hotfix + +The internal-main Docker probe runs every 10 seconds. Applying the shared +anonymous limit of 100 requests/hour to it causes false HTTP 429 failures. + +Customer deployment workflows have been removed. Do not rerun historical +customer workflows: they execute the configuration from their original commit. + +For this views-only fix, `docker/Dockerfile.health-hotfix` can build a web image +from the exact deployed `repository@sha256:digest`, preserving its dependencies, +startup command and all other application files. Before using this recipe, +verify that the base image matches the source baseline and that the application +diff contains only the probe changes in `src/apps/core/views.py`. Label the new +image with the committed source revision and publish an immutable digest. + +The normal internal-main backend release requires `--refresh-data` and replaces +the database/media clone. It must not be used for this health-only fix. Instead, +under the existing release lock, verify that the candidate has no pending +migrations, keep a private copy of the current manifest, and change only +`MOSTOVIK_BACKEND_WEB_IMAGE`. Recreate only `mostovik-web` with `--no-deps` and the +already-pulled image. Accept the manifest atomically after HTTP probes and Docker +health succeed; restore the previous web image and manifest if they fail. +Preserve database/media selections, Redis state and every other service image. + +Acceptance includes repeated probes beyond the anonymous request allowance, +database-failure regression coverage, and a control proving that ordinary +endpoints are still throttled. Do not disable throttling globally or clear Redis +to make the healthcheck pass. diff --git a/src/apps/core/views.py b/src/apps/core/views.py index 2ebd2ce..386ac86 100644 --- a/src/apps/core/views.py +++ b/src/apps/core/views.py @@ -167,6 +167,7 @@ class LivenessView(APIView): permission_classes = [AllowAny] authentication_classes = [] + throttle_classes = [] @swagger_auto_schema( tags=[HEALTH_TAG], @@ -174,7 +175,7 @@ class LivenessView(APIView): operation_description="Возвращает 200 если приложение запущено.", responses={ 200: "Приложение запущено", - **ErrorResponses.PUBLIC, + 500: CommonResponses.SERVER_ERROR, }, ) def get(self, request: Request) -> Response: @@ -202,7 +203,7 @@ class ReadinessView(APIView): responses={ 200: "Приложение готово обрабатывать запросы", 503: CommonResponses.SERVICE_UNAVAILABLE, - **ErrorResponses.PUBLIC, + 500: CommonResponses.SERVER_ERROR, }, ) def get(self, request: Request) -> Response: diff --git a/src/core/openapi.json b/src/core/openapi.json index 699cd84..b8a78c7 100644 --- a/src/core/openapi.json +++ b/src/core/openapi.json @@ -10818,22 +10818,6 @@ "200": { "description": "Приложение запущено" }, - "429": { - "description": "Превышен лимит запросов", - "content": { - "application/json": { - "schema": { - "type": "object", - "properties": { - "detail": { - "type": "string", - "default": "Превышен лимит запросов. Повторите позже." - } - } - } - } - } - }, "500": { "description": "Внутренняя ошибка сервера", "content": { @@ -10882,22 +10866,6 @@ "200": { "description": "Приложение готово обрабатывать запросы" }, - "429": { - "description": "Превышен лимит запросов", - "content": { - "application/json": { - "schema": { - "type": "object", - "properties": { - "detail": { - "type": "string", - "default": "Превышен лимит запросов. Повторите позже." - } - } - } - } - } - }, "500": { "description": "Внутренняя ошибка сервера", "content": { diff --git a/tests/apps/core/test_views.py b/tests/apps/core/test_views.py index e4202bf..3b9c0c4 100644 --- a/tests/apps/core/test_views.py +++ b/tests/apps/core/test_views.py @@ -7,16 +7,28 @@ from unittest.mock import patch from apps.core import views as core_views from apps.core.views import HealthCheckView -from django.urls import reverse +from django.core.cache.backends.locmem import LocMemCache +from django.urls import path, reverse from django.utils import timezone +from drf_yasg import openapi +from drf_yasg.generators import OpenAPISchemaGenerator from rest_framework import status from rest_framework.test import APIRequestFactory, APITestCase +from rest_framework.throttling import AnonRateThrottle from rest_framework.views import APIView from tests.apps.user.factories import UserFactory from tests.utils.fixtures import fake +class _RejectAllThrottle: + def allow_request(self, request, view): + return False + + def wait(self): + return None + + class HealthCheckViewTest(APITestCase): """Tests for HealthCheckView""" @@ -25,6 +37,12 @@ class HealthCheckViewTest(APITestCase): url = reverse("core:health") self.assertEqual(url, "/health/") + def test_detailed_health_check_keeps_global_throttling(self): + with patch.object(APIView, "throttle_classes", [_RejectAllThrottle]): + response = self.client.get(reverse("core:health")) + + self.assertEqual(response.status_code, status.HTTP_429_TOO_MANY_REQUESTS) + def test_health_check_success(self): """Test health check returns healthy status""" url = reverse("core:health") @@ -230,6 +248,13 @@ class LivenessViewTest(APITestCase): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.data["status"], "alive") + def test_liveness_bypasses_global_throttling(self): + with patch.object(APIView, "throttle_classes", [_RejectAllThrottle]): + response = self.client.get(reverse("core:liveness")) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["status"], "alive") + class ReadinessViewTest(APITestCase): """Tests for ReadinessView""" @@ -248,13 +273,6 @@ class ReadinessViewTest(APITestCase): self.assertEqual(response.data["status"], "ready") def test_readiness_bypasses_global_throttling(self): - class _RejectAllThrottle: - def allow_request(self, request, view): - return False - - def wait(self): - return None - with patch.object(APIView, "throttle_classes", [_RejectAllThrottle]): response = self.client.get(reverse("core:readiness")) @@ -278,7 +296,8 @@ class ReadinessViewTest(APITestCase): try: core_views.connection = _BrokenConnection() url = reverse("core:readiness") - response = self.client.get(url) + with patch.object(APIView, "throttle_classes", [_RejectAllThrottle]): + response = self.client.get(url) finally: core_views.connection = original_connection @@ -286,6 +305,65 @@ class ReadinessViewTest(APITestCase): self.assertEqual(response.data["status"], "not_ready") +class HealthProbeThrottlingTest(APITestCase): + def test_openapi_keeps_rate_limit_only_on_detailed_health(self): + generator = OpenAPISchemaGenerator( + info=openapi.Info(title="Health probes", default_version="v1"), + patterns=[ + path("health/", HealthCheckView.as_view()), + path("health/live/", core_views.LivenessView.as_view()), + path("health/ready/", core_views.ReadinessView.as_view()), + ], + ) + schema = generator.get_schema(request=None, public=True) + responses = { + f"{schema.base_path.rstrip('/')}{url}": item["get"].responses + for url, item in schema.paths.items() + } + + self.assertNotIn("429", responses["/health/live/"]) + self.assertNotIn("429", responses["/health/ready/"]) + self.assertIn("503", responses["/health/ready/"]) + self.assertIn("429", responses["/health/"]) + + def test_probes_do_not_consume_or_obey_exhausted_anonymous_quota(self): + probe_cache = LocMemCache(self.id(), {}) + probe_cache.clear() + self.addCleanup(probe_cache.clear) + + class OnePerMinuteAnonThrottle(AnonRateThrottle): + rate = "1/min" + cache = probe_cache + + with ( + patch.object(APIView, "throttle_classes", [OnePerMinuteAnonThrottle]), + patch.object( + HealthCheckView, "_check_database", return_value={"status": "up"} + ), + patch.object( + HealthCheckView, "_check_redis", return_value={"status": "up"} + ), + ): + for quota_exhausted in (False, True): + for _ in range(3): + for name, expected_status in ( + ("liveness", "alive"), + ("readiness", "ready"), + ): + with self.subTest(probe=name, quota_exhausted=quota_exhausted): + response = self.client.get(reverse(f"core:{name}")) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["status"], expected_status) + + if not quota_exhausted: + response = self.client.get(reverse("core:health")) + self.assertEqual(response.status_code, status.HTTP_200_OK) + response = self.client.get(reverse("core:health")) + self.assertEqual( + response.status_code, status.HTTP_429_TOO_MANY_REQUESTS + ) + + class APIVersioningURLTest(APITestCase): """Tests for API versioning URL structure""" diff --git a/tests/test_deployment_boundaries.py b/tests/test_deployment_boundaries.py new file mode 100644 index 0000000..a02d9da --- /dev/null +++ b/tests/test_deployment_boundaries.py @@ -0,0 +1,26 @@ +from pathlib import Path + +import yaml + +WORKFLOWS = Path(__file__).resolve().parents[1] / ".gitea" / "workflows" + + +def test_workflows_do_not_deploy_to_center(): + assert not (WORKFLOWS / "deploy-customer-main.yml").exists() + for path in WORKFLOWS.glob("*.y*ml"): + workflow = yaml.safe_load(path.read_text()) + assert "deploy_customer_main" not in workflow["jobs"] + content = path.read_text() + for forbidden in ("CUSTOMER_DEPLOY_", "10.0.10.174", "/ecos/release-service"): + assert forbidden not in content, (path.name, forbidden) + + +def test_ci_does_not_prune_shared_runner_resources(): + for path in WORKFLOWS.glob("*.y*ml"): + content = path.read_text() + for command in ( + "docker system prune", + "docker builder prune", + "docker buildx prune", + ): + assert command not in content, (path.name, command)