From 2e928c715ba1f59ded0a81dbdc2305fa891578d0 Mon Sep 17 00:00:00 2001 From: bot-backend Date: Sat, 12 Sep 2026 15:57:11 +0500 Subject: [PATCH] =?UTF-8?q?=D0=93=D0=B5=D0=B9=D1=82=20#3448:=20=D0=B7?= =?UTF-8?q?=D0=B0=D0=BA=D1=80=D1=8B=D1=82=D1=8C=20=D0=B7=D0=B5=D0=BB=D1=91?= =?UTF-8?q?=D0=BD=D1=8B=D0=B5=20=D0=BC=D1=83=D1=82=D0=B0=D1=86=D0=B8=D0=B8?= =?UTF-8?q?,=20=D0=B4=D0=BE=D0=B1=D0=B0=D0=B2=D0=B8=D1=82=D1=8C=20=D0=BF?= =?UTF-8?q?=D1=80=D0=B8=D0=B7=D0=BD=D0=B0=D0=BA=20=D0=BD=D0=B5=D0=BF=D1=83?= =?UTF-8?q?=D1=81=D1=82=D0=BE=D1=82=D1=8B,=20=D0=B7=D0=B0=D0=BF=D1=83?= =?UTF-8?q?=D1=81=D0=BA=D0=B0=D1=82=D1=8C=20=D0=BD=D0=B0=20ci-tradein.yml?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Мутационный прогон нашёл пять зелёных мутаций — то есть мест, где логику можно сломать, а гейт этого не заметит. Закрыты фикстурами, каждая краснеет ровно на своей мутации: * CADDY_RE → `^(Caddyfile|caddy)`: тогда `caddy-extra/**` и `Caddyfile.bak` дают caddy_only=true — тихий пропуск полного деплоя, против которого весь PR; * снятие проверки «файлов больше нуля»: пустой дифф формально удовлетворяет «ни один файл не лежит вне caddy» и отключает сборку; * выпадение `data/sql/**` из backend: миграции едут в backend-образе; * подмена базы на `HEAD^..HEAD`: на ОДНОМ мерж-коммите даёт верный ответ и выглядит рабочей, а на push'е из нескольких коммитов теряет первый — фикстура «бэкенд-коммит + caddy-коммит» это ловит; * потеря `core.quotePath=false`: кириллический путь под backend/ выпадает из классификации. Плюс прод-сторож из deploy-caddy: его кусок (от PROD_HEAD до `git reset --hard`) извлекается из ssh-скрипта и ИСПОЛНЯЕТСЯ на временном репозитории, где прод-дерево отстаёт от origin/main — отдельно законный случай (отстал только конфиг прокси) и отказной (отстал бэкенд). Проверяется и порядок: сторож обязан стоять ДО `git reset`. Команда ищется регуляркой по началу строки, а не подстрокой: `git reset --hard` упоминается выше в комментариях, и поиск по тексту находил объяснение вместо кода. Признак непустоты у проверки исключающих `!`-шаблонов: раньше она бы прошла при нулевом охвате (переименуют действие, заведут .yaml) — теперь отдельно утверждается, что хотя бы один шаг paths-filter найден, как это сделано в ci.yml для shell-гейта. Маска расширена до *.y*ml, параметризация — по найденным шагам. ci.yml: в фильтр `backend` добавлен `.forgejo/workflows/ci-tradein.yml` — там тоже живёт paths-filter, и без этой строки правка с `!`-шаблоном не запустила бы backend-tests, то есть гейт не побежал бы ровно на той правке, от которой стережёт. Докстринг фикстуры с мержем переписан: он утверждал, что «дифф последнего коммита» на мерж-коммите даёт пустой список (это верно для `git show`, а не для `git diff HEAD^ HEAD`) — то есть обещал защиту, которой у этой фикстуры нет. Теперь там сказано, что подмену базы стережёт отдельная проверка. Co-Authored-By: Claude Opus 5 --- .forgejo/workflows/ci.yml | 6 + .../ops/test_3448_caddy_only_detection.py | 292 +++++++++++++++--- 2 files changed, 251 insertions(+), 47 deletions(-) diff --git a/.forgejo/workflows/ci.yml b/.forgejo/workflows/ci.yml index b38276cc..b0706e31 100644 --- a/.forgejo/workflows/ci.yml +++ b/.forgejo/workflows/ci.yml @@ -219,6 +219,12 @@ jobs: - '.forgejo/workflows/deploy.yml' - '.forgejo/workflows/deploy-tradein.yml' - '.forgejo/workflows/ci.yml' + # #3448: тот же класс, ещё раз. Гейт про исключающие `!`-шаблоны + # в paths-filter проверяет ВСЕ воркфлоу, а paths-filter живёт и + # здесь — без этой строки правка ci-tradein.yml с таким шаблоном + # не запустила бы backend-tests, то есть гейт не побежал бы ровно + # на той правке, от которой стережёт. + - '.forgejo/workflows/ci-tradein.yml' frontend: - 'frontend/**' - '.forgejo/workflows/ci.yml' diff --git a/backend/tests/ops/test_3448_caddy_only_detection.py b/backend/tests/ops/test_3448_caddy_only_detection.py index 9402210b..3a953a35 100644 --- a/backend/tests/ops/test_3448_caddy_only_detection.py +++ b/backend/tests/ops/test_3448_caddy_only_detection.py @@ -28,12 +28,16 @@ Forgejo рисует зелёной, поэтому «зелёный deploy-cadd когда быстрый путь сработал, и когда его вообще не было. Проверки ниже ИСПОЛНЯЮТ шаг определения файлов из deploy.yml на настоящем временном репозитории (включая мерж-коммит — ровно случай #3448) и смотрят на значения -флагов, а не на текст воркфлоу. Регресс к исключающим шаблонам paths-filter -ловит отдельная проверка в конце. +флагов, а не на текст воркфлоу. Так же исполняется и прод-сторож из джобы +`deploy-caddy`: быстрый путь пропускает джобу `deploy` целиком, а вместе с ней +и гард свежести :latest (#2950), поэтому перезагружать прокси можно, только +если прод отстаёт РОВНО на конфиг прокси. Регресс к исключающим шаблонам +paths-filter ловит отдельная проверка в конце. """ from __future__ import annotations +import re import subprocess from pathlib import Path @@ -95,13 +99,27 @@ def _git(repo: Path, *args: str) -> None: ) -def _make_repo(tmp_path: Path, changed: tuple[str, ...]) -> tuple[Path, str]: - """Репозиторий с базовым коммитом и МЕРЖ-коммитом поверх него. +def _sha(repo: Path) -> str: + return subprocess.run( + ["git", "-C", str(repo), "rev-parse", "HEAD"], + check=True, + capture_output=True, + text=True, + env=dict(GIT_ENV), + ).stdout.strip() - Мерж, а не обычный коммит, — намеренно: #3448 наблюдался именно на мерже - PR'а, и любой фолбэк вида «дифф последнего коммита» на мерж-коммите даёт - пустой список (git show у мержа без -m не печатает ничего). - """ + +def _commit(repo: Path, files: tuple[str, ...], msg: str = "c") -> None: + for name in files: + path = repo / name + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text("changed\n", encoding="utf-8") + _git(repo, "add", "-A") + _git(repo, "commit", "-qm", msg, *([] if files else ["--allow-empty"])) + + +def _base_repo(tmp_path: Path) -> tuple[Path, str]: + """Репозиторий с одним базовым коммитом; возвращает его sha — это `before`.""" repo = tmp_path / "repo" repo.mkdir(parents=True) _git(repo, "init", "-q", "-b", "main") @@ -111,35 +129,30 @@ def _make_repo(tmp_path: Path, changed: tuple[str, ...]) -> tuple[Path, str]: path.write_text("base\n", encoding="utf-8") _git(repo, "add", "-A") _git(repo, "commit", "-qm", "base") - base_sha = subprocess.run( - ["git", "-C", str(repo), "rev-parse", "HEAD"], - check=True, - capture_output=True, - text=True, - env=dict(GIT_ENV), - ).stdout.strip() + return repo, _sha(repo) + +def _merge_commit(repo: Path, changed: tuple[str, ...]) -> None: + """Ветка с правкой и мерж `--no-ff` обратно в main. + + Мерж, а не обычный коммит, — потому что #3448 наблюдался именно на мерже + PR'а: у мерж-коммита две родительские линии, и любой разбор диффа обязан + работать на этой форме. Что `before` нельзя заменить на `HEAD^`, стережёт + отдельная проверка — test_multi_commit_push_is_not_truncated: на ОДНОМ + мерж-коммите `HEAD^..HEAD` даёт верный ответ и такую подмену не ловит. + """ _git(repo, "checkout", "-q", "-b", "feature") - for name in changed: - path = repo / name - path.parent.mkdir(parents=True, exist_ok=True) - path.write_text("changed\n", encoding="utf-8") - _git(repo, "add", "-A") - _git(repo, "commit", "-qm", "feature") + _commit(repo, changed, "feature") _git(repo, "checkout", "-q", "main") _git(repo, "merge", "-q", "--no-ff", "-m", "merge feature", "feature") - return repo, base_sha -def _run( - tmp_path: Path, changed: tuple[str, ...], *, before: str | None = None, event: str = "push" -) -> tuple[dict[str, str], str]: - repo, base_sha = _make_repo(tmp_path, changed) - out_file = tmp_path / "outputs" +def _exec(repo: Path, before: str, event: str = "push") -> tuple[dict[str, str], str]: + out_file = repo.parent / "outputs" out_file.touch() env = { "PATH": "/usr/bin:/bin:/usr/local/bin", - "BEFORE": base_sha if before is None else before, + "BEFORE": before, "EVENT": event, "GITHUB_OUTPUT": str(out_file), **GIT_ENV, @@ -160,6 +173,14 @@ def _run( return outputs, proc.stdout +def _run( + tmp_path: Path, changed: tuple[str, ...], *, before: str | None = None, event: str = "push" +) -> tuple[dict[str, str], str]: + repo, base_sha = _base_repo(tmp_path) + _merge_commit(repo, changed) + return _exec(repo, base_sha if before is None else before, event) + + def test_merge_with_only_caddy_file_takes_the_fast_path(tmp_path: Path) -> None: """Случай #3448 дословно: мерж, в диффе один файл под caddy/.""" outputs, _ = _run(tmp_path, ("caddy/sites/apps.caddy",)) @@ -215,26 +236,203 @@ def test_decision_is_visible_in_the_log(tmp_path: Path) -> None: ) -@pytest.mark.parametrize("path", sorted(WORKFLOWS.glob("*.yml")), ids=lambda p: p.name) -def test_no_paths_filter_relies_on_exclusion_patterns(path: Path) -> None: - """Ни один paths-filter в репозитории не пытается вычитать пути через `!`. +# ── Быстрый путь на самом проде: джоба deploy-caddy ────────────────────────── +# +# Пока caddy_only был мёртв, каждый push шёл полным деплоем, и гард свежести +# :latest (#2950, job `deploy`) прикрывал прод по умолчанию. Оживший быстрый +# путь его обходит: при caddy_only=true джоба `deploy` пропускается целиком. +# Дифф between-push (before→HEAD) не знает, что реально доехало до прода: +# отменённая очередью `deploy` предыдущего прогона оставляет прод на старом +# образе, а следующий caddy-only push честно видит «изменился один caddy-файл». + + +def _caddy_deploy_script() -> str: + spec = yaml.safe_load(DEPLOY.read_text(encoding="utf-8")) + steps = [s for s in spec["jobs"]["deploy-caddy"]["steps"] if "ssh-action" in str(s.get("uses"))] + assert len(steps) == 1, "в deploy-caddy нет ровно одного ssh-шага — гейт #3448 ослеп" + return steps[0]["with"]["script"] + + +def _prod_lag_guard() -> str: + """Кусок ssh-скрипта от вычисления PROD_HEAD до `git reset --hard`.""" + script = _caddy_deploy_script() + assert "PROD_HEAD=" in script, ( + "джоба deploy-caddy не сверяет отставание прода: быстрый путь перезагрузит " + "прокси и уйдёт зелёным, оставив прод на старом образе (#3448)" + ) + # Ищем КОМАНДУ, а не подстроку: `git reset --hard` упоминается выше в + # комментариях, и поиск по тексту нашёл бы объяснение вместо кода. + reset_cmd = re.search(r"(?m)^\s*git reset --hard", script) + assert reset_cmd, "в deploy-caddy пропал `git reset --hard` — гейт опирается на него" + start, reset = script.index("PROD_HEAD="), reset_cmd.start() + assert start < reset, ( + "проверка отставания прода стоит ПОСЛЕ `git reset --hard` — при отказе " + "прод-HEAD уже переписан, и следующий прогон снова уйдёт быстрым путём" + ) + return "set -euo pipefail\n" + script[start:reset] + + +def _prod_repo(tmp_path: Path, ahead: tuple[str, ...]) -> Path: + """Прод-дерево на базовом коммите, origin/main — на `ahead` впереди.""" + repo, base_sha = _base_repo(tmp_path) + _commit(repo, ahead, "ahead") + _git(repo, "update-ref", "refs/remotes/origin/main", "HEAD") + _git(repo, "reset", "--hard", "-q", base_sha) + return repo + + +def _run_guard(repo: Path) -> subprocess.CompletedProcess: + return subprocess.run( + ["bash", "-c", _prod_lag_guard()], + cwd=repo, + capture_output=True, + text=True, + env={"PATH": "/usr/bin:/bin:/usr/local/bin", **GIT_ENV}, + ) + + +def test_fast_path_allowed_when_prod_lags_only_by_proxy_config(tmp_path: Path) -> None: + proc = _run_guard(_prod_repo(tmp_path, ("caddy/sites/apps.caddy",))) + assert proc.returncode == 0, f"законный быстрый путь заблокирован:\n{proc.stdout}{proc.stderr}" + + +def test_fast_path_refuses_when_prod_lags_by_code(tmp_path: Path) -> None: + """Прод отстаёт не только по конфигу прокси → перезагрузка прокси запрещена.""" + proc = _run_guard(_prod_repo(tmp_path, ("backend/app/main.py", "caddy/sites/apps.caddy"))) + assert proc.returncode != 0, ( + "быстрый путь разрешён, хотя прод отстаёт по коду бэкенда: перезагрузка " + f"прокси подменила бы выкатку, деплой ушёл бы зелёным.\n{proc.stdout}" + ) + assert "backend/app/main.py" in proc.stdout, ( + f"отказ не называет файлы, из-за которых он произошёл:\n{proc.stdout}" + ) + + +def test_fast_path_takes_the_same_host_lock() -> None: + """deploy-caddy правит прод-дерево — значит берёт тот же лок, что `deploy`. + + Проверка текстовая, как в test_2950: исполнить flock-секцию в тесте нельзя, + а её пропажа не даёт ни одного сигнала до совпадения окон двух деплоев. + """ + script = _caddy_deploy_script() + assert "exec 9>/var/lock/gendesign-docker-deploy.lock" in script, ( + "deploy-caddy делает `git reset --hard` в /opt/gendesign в обход лока, " + "которым полный деплой сериализует работу с прод-деревом (#2950)" + ) + assert "flock -w 900 9" in script, "лок открывается, но не захватывается" + + +@pytest.mark.parametrize( + "changed", [("caddy-extra/x.txt",), ("Caddyfile.bak",), ("docs/caddy.md",)] +) +def test_paths_that_merely_start_with_caddy_are_not_the_fast_path( + tmp_path: Path, changed: tuple[str, ...] +) -> None: + """`caddy-extra/…` и `Caddyfile.bak` — НЕ конфиг прокси. + + Граница шаблона — единственное, что отделяет быстрый путь от тихого + пропуска полного деплоя: `^(Caddyfile|caddy)` вместо `^(Caddyfile$|caddy/)` + отправил бы эти правки перезагружать прокси вместо выкатки. + """ + outputs, _ = _run(tmp_path, changed) + assert outputs["caddy_only"] == "false", f"{changed}: {outputs}" + + +def test_empty_diff_is_not_the_fast_path(tmp_path: Path) -> None: + """Пустой дифф (`before` == HEAD, пустой мерж) — не «всё под caddy». + + Без проверки «файлов больше нуля» пустой список формально удовлетворяет + «ни один файл не лежит вне caddy»: сборка отключается, деплой подменяется + перезагрузкой прокси — отказ, выглядящий как успешный быстрый путь. + """ + outputs, log = _run(tmp_path, ()) + assert outputs["caddy_only"] == "false", f"пустой дифф ушёл в быстрый путь: {outputs}" + assert "изменённых файлов: 0" in log + + +def test_data_sql_counts_as_backend(tmp_path: Path) -> None: + """`data/sql/**` собирает backend-образ: миграции едут в нём.""" + outputs, _ = _run(tmp_path, ("data/sql/002.sql",)) + assert outputs["backend"] == "true", outputs + assert outputs["caddy_only"] == "false", outputs + + +def test_non_ascii_path_is_classified(tmp_path: Path) -> None: + """Кириллица в пути не должна прятать файл от классификации. + + `git diff --name-only` при `core.quotePath=true` (умолчание) отдаёт + не-ASCII пути закавыченными и с \\NNN-экранированием — `^backend/` + такую строку не матчит. Старый paths-filter брал `--name-status -z`, где + квотирования нет; при переходе на свой diff это единственное место, где + поведение могло разойтись. В дереве такие пути уже живут (docs/). + """ + outputs, log = _run(tmp_path, ("backend/модуль.py",)) + assert outputs["backend"] == "true", f"кириллический путь потерян: {outputs}\n{log}" + + +def test_multi_commit_push_is_not_truncated(tmp_path: Path) -> None: + """Push из нескольких коммитов разбирается целиком, а не по последнему. + + Ровно та подмена, которую соблазнительно сделать «чтобы не зависеть от + before»: `HEAD^..HEAD`. На одном мерж-коммите она даёт верный ответ и + выглядит рабочей, а здесь — молча теряет бэкенд из первого коммита и + включает быстрый путь, то есть пропускает выкатку кода. + """ + repo, base_sha = _base_repo(tmp_path) + _commit(repo, ("backend/app/main.py",), "backend") + _commit(repo, ("caddy/sites/apps.caddy",), "caddy") + outputs, log = _exec(repo, base_sha) + assert outputs["backend"] == "true", f"первый коммит push'а потерян: {outputs}\n{log}" + assert outputs["caddy_only"] == "false", outputs + + +def _paths_filter_steps() -> list[tuple[Path, str, dict]]: + """Все шаги dorny/paths-filter во всех воркфлоу (включая .yaml).""" + found = [] + for path in sorted(WORKFLOWS.glob("*.y*ml")): + spec = yaml.safe_load(path.read_text(encoding="utf-8")) or {} + for job_name, job in (spec.get("jobs") or {}).items(): + for step in job.get("steps") or []: + if str(step.get("uses", "")).startswith("dorny/paths-filter"): + found.append((path, job_name, step)) + return found + + +def test_exclusion_gate_has_something_to_check() -> None: + """Признак непустоты: проверка ниже обязана что-то находить. + + Переименуют действие, разнесут воркфлоу по .yaml, уедут шаги — и гейт + пройдёт при нулевом охвате, молча (ровно то, от чего страхуется ci.yml:190). + """ + steps = _paths_filter_steps() + assert steps, ( + "не найдено ни одного шага dorny/paths-filter — проверка исключающих " + "шаблонов прошла бы впустую, перепроверь маску поиска" + ) + + +@pytest.mark.parametrize( + "path,job_name,step", + _paths_filter_steps(), + ids=[f"{p.name}:{j}" for p, j, _ in _paths_filter_steps()], +) +def test_no_paths_filter_relies_on_exclusion_patterns( + path: Path, job_name: str, step: dict +) -> None: + """Ни один paths-filter в репозитории не вычитает пути через `!`. Класс бага, а не единственный его случай: при `predicate-quantifier: some` (умолчание) шаблоны фильтра склеиваются через ИЛИ, и `!` ничего не вычитает. """ - spec = yaml.safe_load(path.read_text(encoding="utf-8")) or {} - for job_name, job in (spec.get("jobs") or {}).items(): - for step in job.get("steps") or []: - if not str(step.get("uses", "")).startswith("dorny/paths-filter"): - continue - with_ = step.get("with") or {} - if with_.get("predicate-quantifier") == "every": - continue - filters = yaml.safe_load(with_.get("filters") or "") or {} - for filter_name, patterns in filters.items(): - bad = [p for p in (patterns or []) if isinstance(p, str) and p.startswith("!")] - assert not bad, ( - f"{path.name}: job {job_name}, фильтр {filter_name!r} вычитает пути " - f"шаблонами {bad} — при `some` (умолчание) они склеиваются через ИЛИ " - "и фильтр становится true ВСЕГДА. Так #2916 не сработал ни разу (#3448)." - ) + with_ = step.get("with") or {} + if with_.get("predicate-quantifier") == "every": + pytest.skip("predicate-quantifier: every — шаблоны склеиваются через И") + filters = yaml.safe_load(with_.get("filters") or "") or {} + assert filters, f"{path.name}: job {job_name} — у paths-filter пустой блок filters" + for filter_name, patterns in filters.items(): + bad = [p for p in (patterns or []) if isinstance(p, str) and p.startswith("!")] + assert not bad, ( + f"{path.name}: job {job_name}, фильтр {filter_name!r} вычитает пути " + f"шаблонами {bad} — при `some` (умолчание) они склеиваются через ИЛИ " + "и фильтр становится true ВСЕГДА. Так #2916 не сработал ни разу (#3448)." + )