fix: exclude infrastructure probes from API throttling
All checks were successful
Mostovik Backend CI/CD / Tests and lint (push) Successful in 3m28s
Mostovik Backend CI/CD / Build linux/amd64 release images (push) Successful in 3m23s
Mostovik Backend CI/CD / Deploy and verify internal main (push) Has been skipped
Mostovik Backend CI/CD / Deploy dev (push) Successful in 1m49s
All checks were successful
Mostovik Backend CI/CD / Tests and lint (push) Successful in 3m28s
Mostovik Backend CI/CD / Build linux/amd64 release images (push) Successful in 3m23s
Mostovik Backend CI/CD / Deploy and verify internal main (push) Has been skipped
Mostovik Backend CI/CD / Deploy dev (push) Successful in 1m49s
This commit is contained in:
@@ -20,9 +20,6 @@ env:
|
|||||||
REGISTRY_NAMESPACE: ${{ github.repository_owner }}
|
REGISTRY_NAMESPACE: ${{ github.repository_owner }}
|
||||||
WEB_IMAGE: mostovik-backend-web
|
WEB_IMAGE: mostovik-backend-web
|
||||||
CELERY_IMAGE: mostovik-backend-celery
|
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:
|
jobs:
|
||||||
quality:
|
quality:
|
||||||
@@ -303,61 +300,3 @@ jobs:
|
|||||||
done
|
done
|
||||||
echo "Internal-main backend readiness failed" >&2
|
echo "Internal-main backend readiness failed" >&2
|
||||||
exit 1
|
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}"
|
|
||||||
|
|||||||
5
docker/Dockerfile.health-hotfix
Normal file
5
docker/Dockerfile.health-hotfix
Normal file
@@ -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
|
||||||
36
docs/health-probes.md
Normal file
36
docs/health-probes.md
Normal file
@@ -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.
|
||||||
@@ -167,6 +167,7 @@ class LivenessView(APIView):
|
|||||||
|
|
||||||
permission_classes = [AllowAny]
|
permission_classes = [AllowAny]
|
||||||
authentication_classes = []
|
authentication_classes = []
|
||||||
|
throttle_classes = []
|
||||||
|
|
||||||
@swagger_auto_schema(
|
@swagger_auto_schema(
|
||||||
tags=[HEALTH_TAG],
|
tags=[HEALTH_TAG],
|
||||||
@@ -174,7 +175,7 @@ class LivenessView(APIView):
|
|||||||
operation_description="Возвращает 200 если приложение запущено.",
|
operation_description="Возвращает 200 если приложение запущено.",
|
||||||
responses={
|
responses={
|
||||||
200: "Приложение запущено",
|
200: "Приложение запущено",
|
||||||
**ErrorResponses.PUBLIC,
|
500: CommonResponses.SERVER_ERROR,
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
def get(self, request: Request) -> Response:
|
def get(self, request: Request) -> Response:
|
||||||
@@ -202,7 +203,7 @@ class ReadinessView(APIView):
|
|||||||
responses={
|
responses={
|
||||||
200: "Приложение готово обрабатывать запросы",
|
200: "Приложение готово обрабатывать запросы",
|
||||||
503: CommonResponses.SERVICE_UNAVAILABLE,
|
503: CommonResponses.SERVICE_UNAVAILABLE,
|
||||||
**ErrorResponses.PUBLIC,
|
500: CommonResponses.SERVER_ERROR,
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
def get(self, request: Request) -> Response:
|
def get(self, request: Request) -> Response:
|
||||||
|
|||||||
@@ -10818,22 +10818,6 @@
|
|||||||
"200": {
|
"200": {
|
||||||
"description": "Приложение запущено"
|
"description": "Приложение запущено"
|
||||||
},
|
},
|
||||||
"429": {
|
|
||||||
"description": "Превышен лимит запросов",
|
|
||||||
"content": {
|
|
||||||
"application/json": {
|
|
||||||
"schema": {
|
|
||||||
"type": "object",
|
|
||||||
"properties": {
|
|
||||||
"detail": {
|
|
||||||
"type": "string",
|
|
||||||
"default": "Превышен лимит запросов. Повторите позже."
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
},
|
|
||||||
"500": {
|
"500": {
|
||||||
"description": "Внутренняя ошибка сервера",
|
"description": "Внутренняя ошибка сервера",
|
||||||
"content": {
|
"content": {
|
||||||
@@ -10882,22 +10866,6 @@
|
|||||||
"200": {
|
"200": {
|
||||||
"description": "Приложение готово обрабатывать запросы"
|
"description": "Приложение готово обрабатывать запросы"
|
||||||
},
|
},
|
||||||
"429": {
|
|
||||||
"description": "Превышен лимит запросов",
|
|
||||||
"content": {
|
|
||||||
"application/json": {
|
|
||||||
"schema": {
|
|
||||||
"type": "object",
|
|
||||||
"properties": {
|
|
||||||
"detail": {
|
|
||||||
"type": "string",
|
|
||||||
"default": "Превышен лимит запросов. Повторите позже."
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
},
|
|
||||||
"500": {
|
"500": {
|
||||||
"description": "Внутренняя ошибка сервера",
|
"description": "Внутренняя ошибка сервера",
|
||||||
"content": {
|
"content": {
|
||||||
|
|||||||
@@ -7,16 +7,28 @@ from unittest.mock import patch
|
|||||||
|
|
||||||
from apps.core import views as core_views
|
from apps.core import views as core_views
|
||||||
from apps.core.views import HealthCheckView
|
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 django.utils import timezone
|
||||||
|
from drf_yasg import openapi
|
||||||
|
from drf_yasg.generators import OpenAPISchemaGenerator
|
||||||
from rest_framework import status
|
from rest_framework import status
|
||||||
from rest_framework.test import APIRequestFactory, APITestCase
|
from rest_framework.test import APIRequestFactory, APITestCase
|
||||||
|
from rest_framework.throttling import AnonRateThrottle
|
||||||
from rest_framework.views import APIView
|
from rest_framework.views import APIView
|
||||||
|
|
||||||
from tests.apps.user.factories import UserFactory
|
from tests.apps.user.factories import UserFactory
|
||||||
from tests.utils.fixtures import fake
|
from tests.utils.fixtures import fake
|
||||||
|
|
||||||
|
|
||||||
|
class _RejectAllThrottle:
|
||||||
|
def allow_request(self, request, view):
|
||||||
|
return False
|
||||||
|
|
||||||
|
def wait(self):
|
||||||
|
return None
|
||||||
|
|
||||||
|
|
||||||
class HealthCheckViewTest(APITestCase):
|
class HealthCheckViewTest(APITestCase):
|
||||||
"""Tests for HealthCheckView"""
|
"""Tests for HealthCheckView"""
|
||||||
|
|
||||||
@@ -25,6 +37,12 @@ class HealthCheckViewTest(APITestCase):
|
|||||||
url = reverse("core:health")
|
url = reverse("core:health")
|
||||||
self.assertEqual(url, "/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):
|
def test_health_check_success(self):
|
||||||
"""Test health check returns healthy status"""
|
"""Test health check returns healthy status"""
|
||||||
url = reverse("core:health")
|
url = reverse("core:health")
|
||||||
@@ -230,6 +248,13 @@ class LivenessViewTest(APITestCase):
|
|||||||
self.assertEqual(response.status_code, status.HTTP_200_OK)
|
self.assertEqual(response.status_code, status.HTTP_200_OK)
|
||||||
self.assertEqual(response.data["status"], "alive")
|
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):
|
class ReadinessViewTest(APITestCase):
|
||||||
"""Tests for ReadinessView"""
|
"""Tests for ReadinessView"""
|
||||||
@@ -248,13 +273,6 @@ class ReadinessViewTest(APITestCase):
|
|||||||
self.assertEqual(response.data["status"], "ready")
|
self.assertEqual(response.data["status"], "ready")
|
||||||
|
|
||||||
def test_readiness_bypasses_global_throttling(self):
|
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]):
|
with patch.object(APIView, "throttle_classes", [_RejectAllThrottle]):
|
||||||
response = self.client.get(reverse("core:readiness"))
|
response = self.client.get(reverse("core:readiness"))
|
||||||
|
|
||||||
@@ -278,6 +296,7 @@ class ReadinessViewTest(APITestCase):
|
|||||||
try:
|
try:
|
||||||
core_views.connection = _BrokenConnection()
|
core_views.connection = _BrokenConnection()
|
||||||
url = reverse("core:readiness")
|
url = reverse("core:readiness")
|
||||||
|
with patch.object(APIView, "throttle_classes", [_RejectAllThrottle]):
|
||||||
response = self.client.get(url)
|
response = self.client.get(url)
|
||||||
finally:
|
finally:
|
||||||
core_views.connection = original_connection
|
core_views.connection = original_connection
|
||||||
@@ -286,6 +305,65 @@ class ReadinessViewTest(APITestCase):
|
|||||||
self.assertEqual(response.data["status"], "not_ready")
|
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):
|
class APIVersioningURLTest(APITestCase):
|
||||||
"""Tests for API versioning URL structure"""
|
"""Tests for API versioning URL structure"""
|
||||||
|
|
||||||
|
|||||||
26
tests/test_deployment_boundaries.py
Normal file
26
tests/test_deployment_boundaries.py
Normal file
@@ -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)
|
||||||
Reference in New Issue
Block a user