From 4721a9ddf6e2703f8b1b79c9e087775a4a072ba3 Mon Sep 17 00:00:00 2001 From: szibis Date: Thu, 22 Oct 2026 14:51:01 +0200 Subject: [PATCH] ci: infra-only pull requests need no changelog entry and cut no release The changelog gate and the release bot each kept their own list of paths that do not ship. Both now ask the change classifier, so the lists cannot drift apart. Changelog gate: a pull request whose files are all infra needs no entry, whatever its commit subjects say, so an infra-only fix(ci) no longer forces one. A fix that only edits unit tests under internal still needs an entry. A change to the classifier or its workflow counts as infra here, although it runs every check. A dependency bump is accepted when every file is non-release or a dependency manifest by file name, which covers the bench manifests that are product for CI but not a release by themselves. The released-history guard and the metadata-sync exemption are unchanged. Release bot: the regex loop is replaced by the classifier, on the files changed since the last tag. The release is skipped when none of them ships. The chart version edit made by the metadata sync after each release is ignored while the chart still carries the released version, because it would otherwise make every range look like a chart change. --- .github/workflows/auto-release.yaml | 48 ++++-- scripts/ci/check_changelog_pr.py | 73 ++++++---- scripts/ci/tests/test_check_changelog_pr.py | 153 +++++++++++++++++++- 3 files changed, 227 insertions(+), 47 deletions(-) diff --git a/.github/workflows/auto-release.yaml b/.github/workflows/auto-release.yaml index 188210b6e..bb636bfb4 100644 --- a/.github/workflows/auto-release.yaml +++ b/.github/workflows/auto-release.yaml @@ -91,20 +91,42 @@ jobs: exit 0 fi - DOCS_OR_CI_ONLY="yes" - while IFS= read -r file; do - [ -z "$file" ] && continue - # The daily performance report's data counts as docs: merging that - # pull request never cuts a release. - if ! echo "$file" | grep -Eq '^(README\.md|CHANGELOG\.md|docs/|\.github/|bench/ab/(history|results)/|conformance/(registry/generated/perf-evidence\.json|reports/(performance|gaps)\.md)$)'; then - DOCS_OR_CI_ONLY="no" - break - fi - done <<< "$FILES_CHANGED" - - if [ "$DOCS_OR_CI_ONLY" = "yes" ]; then + # Release only when something users receive changed since the last + # tag. The classifier decides; an empty list or any error counts as + # product, so the check below stays the deciding gate in that case. + # A change to the classifier or its workflow (rule P4) runs every + # check but never ships on its own. + # + # The metadata sync that follows every release rewrites the chart + # version only. While the chart still carries the released version, + # that edit says nothing about what changed since the tag. A chart + # ahead of the tag is a deliberate release request, handled below. + SCOPE_FILES="${FILES_CHANGED}" + CHART_EDITS=$(git diff -U0 "${LATEST_TAG}..HEAD" -- charts/loki-vl-proxy/Chart.yaml 2>/dev/null \ + | grep -E '^[+-]' | grep -vE '^(\+\+\+|---) ' | grep -vE '^[+-](version|appVersion):' || true) + if [ "${CHART_VERSION}" = "${LATEST_VERSION}" ] && [ -z "${CHART_EDITS}" ]; then + SCOPE_FILES=$(printf '%s\n' "${FILES_CHANGED}" | grep -vx 'charts/loki-vl-proxy/Chart.yaml' || true) + fi + if [ -n "${FILES_CHANGED}" ] && [ -z "${SCOPE_FILES}" ]; then + PRODUCT_CHANGED="false" + else + PRODUCT_CHANGED=$(printf '%s\n' "${SCOPE_FILES}" | python3 -c ' + import sys + sys.path.insert(0, "scripts/ci") + try: + import change_scope + paths = [line.strip() for line in sys.stdin if line.strip()] + shipped = [p for p in paths + if change_scope.classify(p)[0] and change_scope.classify(p)[1] != "P4"] + print("true" if not paths or shipped else "false") + except Exception: + print("true") + ' || echo true) + fi + + if [ "${PRODUCT_CHANGED}" = "false" ]; then echo "should_release=false" >> "$GITHUB_OUTPUT" - echo "Only docs/ci/metadata files changed; skipping release" + echo "Only infra/docs files changed since ${LATEST_TAG}; skipping release" exit 0 fi diff --git a/scripts/ci/check_changelog_pr.py b/scripts/ci/check_changelog_pr.py index d9b1bbef9..07efc062a 100644 --- a/scripts/ci/check_changelog_pr.py +++ b/scripts/ci/check_changelog_pr.py @@ -3,12 +3,19 @@ from __future__ import annotations import argparse +import fnmatch import pathlib +import posixpath import re import subprocess import sys from typing import Iterable +# The classifier lives next to this script. Inserting the directory keeps the +# import working when this file runs as a script and when the tests import it +# as scripts.ci.check_changelog_pr. +sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent)) +import change_scope # noqa: E402 ROOT = pathlib.Path(__file__).resolve().parents[2] CHANGELOG = ROOT / "CHANGELOG.md" @@ -53,28 +60,23 @@ "go.sum", } -NON_RELEASE_PATH_PREFIXES = ( - "docs/", - "scripts/ci/tests/", - # The documentation website is published independently of the proxy - # binary/chart; its source and lockfile changes are not release-impacting. - "website/", - # The daily performance report's measurements and their history. The - # harness itself (bench/ab/*.py) still counts. - "bench/ab/history/", - "bench/ab/results/", +# Manifests a dependency bump rewrites, matched by file name in any directory. +# Some of them (bench/visual/requirements.txt) are product for CI but a bump of +# them is still not a release. +DEPENDENCY_MANIFEST_PATTERNS = ( + "go.mod", + "go.sum", + "Dockerfile", + "requirements*.txt", + "requirements*.in", + "package.json", + "package-lock.json", + "docker-compose*.yml", ) -NON_RELEASE_FILES = { - "README.md", - "CHANGELOG.md", - "LICENSE", - "scripts/ci/check_changelog_pr.py", - # The reports the daily performance run regenerates. - "conformance/registry/generated/perf-evidence.json", - "conformance/reports/performance.md", - "conformance/reports/gaps.md", -} +# Classifier rule that marks the gating machinery itself. Such a change runs +# every check, but it is not something users receive. +GATING_RULE = "P4" RELEASE_METADATA_FILES = { "CHANGELOG.md", @@ -172,12 +174,19 @@ def is_release_path(path: str) -> bool: return any(path.startswith(prefix) for prefix in IMPACTFUL_PATHS) +def is_product_path(path: str) -> bool: + """True when the classifier says the path can change what users receive.""" + is_product, rule_id = change_scope.classify(path) + return is_product and rule_id != GATING_RULE + + def is_non_release_path(path: str) -> bool: - if is_unit_test_only_path(path): - return True - if path in NON_RELEASE_FILES: - return True - return any(path.startswith(prefix) for prefix in NON_RELEASE_PATH_PREFIXES) + return is_unit_test_only_path(path) or not is_product_path(path) + + +def is_dependency_manifest(path: str) -> bool: + name = posixpath.basename(path) + return any(fnmatch.fnmatchcase(name, pattern) for pattern in DEPENDENCY_MANIFEST_PATTERNS) def is_unit_test_only_path(path: str) -> bool: @@ -190,6 +199,13 @@ def should_require_changelog(commits: Iterable[str], files: Iterable[str]) -> bo commit_list = [c for c in commits if c.strip()] file_list = [f for f in files if f.strip()] + # Nothing users receive changes, whatever the subject says: an infra-only + # "fix(ci): ..." needs no entry. This deliberately uses the classifier and + # not is_non_release_path, so a "fix:" that only edits unit tests under + # internal/ keeps requiring one. + if file_list and not any(is_product_path(f) for f in file_list): + return False + if any(is_release_commit(subject) for subject in commit_list): return True @@ -221,12 +237,7 @@ def is_dependency_only_pr(commits: Iterable[str], files: Iterable[str]) -> bool: ) if not all_dep_commits: return False - return all( - f in IMPACTFUL_FILES - or is_non_release_path(f) - or f.startswith(".github/") - for f in file_list - ) + return all(is_non_release_path(f) or is_dependency_manifest(f) for f in file_list) def extract_released_sections(text: str) -> str: diff --git a/scripts/ci/tests/test_check_changelog_pr.py b/scripts/ci/tests/test_check_changelog_pr.py index 132c0ce0c..e8df7adb5 100644 --- a/scripts/ci/tests/test_check_changelog_pr.py +++ b/scripts/ci/tests/test_check_changelog_pr.py @@ -8,6 +8,7 @@ extract_released_sections, is_changelog_history_fix, is_dependency_only_pr, + is_non_release_path, released_history_changed, is_release_metadata_sync, should_require_changelog, @@ -37,8 +38,11 @@ def test_has_meaningful_changelog_content(self): self.assertTrue(has_meaningful_changelog_content("### Features\n\n- add thing")) def test_should_require_changelog_for_release_commits(self): - self.assertTrue(should_require_changelog(["feat: add tail mode"], ["docs/readme.md"])) - self.assertTrue(should_require_changelog(["fix(proxy): handle tail fallback"], ["README.md"])) + self.assertTrue(should_require_changelog(["feat: add tail mode"], ["docs/readme.md", "charts/loki-vl-proxy/values.yaml"])) + self.assertTrue(should_require_changelog(["fix(proxy): handle tail fallback"], ["README.md", "bench/ab/daily.py"])) + # Nothing shipped changes, so the subject does not matter. + self.assertFalse(should_require_changelog(["feat: add tail mode"], ["docs/readme.md"])) + self.assertFalse(should_require_changelog(["fix(proxy): handle tail fallback"], ["README.md"])) def test_should_require_changelog_for_impactful_paths(self): self.assertTrue(should_require_changelog(["test: add coverage"], ["internal/proxy/proxy.go"])) @@ -56,7 +60,15 @@ def test_should_skip_for_docs_only(self): self.assertFalse(should_require_changelog(["docs: update guide"], ["docs/getting-started.md"])) def test_should_require_for_ci_and_tests_changes(self): - self.assertTrue(should_require_changelog(["ci: tune workflow"], [".github/workflows/ci.yaml"])) + # A workflow edit alone ships nothing, so it needs no entry. A change + # that also touches product code still does. + self.assertFalse(should_require_changelog(["ci: tune workflow"], [".github/workflows/ci.yaml"])) + self.assertTrue( + should_require_changelog( + ["ci: tune workflow"], + [".github/workflows/ci.yaml", "internal/proxy/proxy.go"], + ) + ) self.assertTrue(should_require_changelog(["test: add coverage"], ["test/e2e-compat/features_test.go"])) def test_should_skip_for_changelog_gate_policy_only_changes(self): @@ -306,5 +318,140 @@ def test_has_new_version_sections_ignores_unreleased(self): self.assertFalse(has_new_version_sections(head, base)) +class InfraScopeTests(unittest.TestCase): + # File lists are the real ones of the Dependabot pull requests named. + DEPENDABOT = { + "#814 semgrep": ( + "build(deps): bump semgrep from 1.161.0 to 1.181.0 in /security in the semgrep-deps group across 1 directory", + ["security/requirements-semgrep.in", "security/requirements-semgrep.txt"], + ), + "#816 certifi": ( + "build(deps): bump certifi from 2026.7.22 to 2026.10.19 in /security", + ["security/requirements-semgrep.txt"], + ), + "#813 bench/visual": ( + "build(deps-dev): bump @playwright/test from 1.59.1 to 1.64.1 in /bench/visual in the visual-deps group", + ["bench/visual/package-lock.json", "bench/visual/package.json"], + ), + "#811 test/e2e-ui": ( + "build(deps-dev): bump @playwright/test from 1.59.1 to 1.64.1 in /test/e2e-ui in the e2e-ui-deps group", + ["test/e2e-ui/package-lock.json", "test/e2e-ui/package.json"], + ), + "#810 test compose": ( + "build(deps): bump nginx from 1.27-alpine to 1.29-alpine in /test/e2e-compat in the test-stack-images group across 1 directory", + ["test/e2e-compat/docker-compose.yml"], + ), + "#815 website": ( + "build(deps): bump the website-deps group in /website with 2 updates", + ["website/package-lock.json", "website/package.json"], + ), + } + + def test_dependabot_pull_requests_pass_without_an_entry(self): + for name, (subject, files) in self.DEPENDABOT.items(): + with self.subTest(name): + self.assertTrue(is_dependency_only_pr([subject], files)) + + def test_bench_manifests_are_dependency_only(self): + # Product for CI, but a bump of them is not a release. + self.assertTrue( + is_dependency_only_pr( + ["build(deps): bump playwright from 1.59.1 to 1.64.1 in /bench/visual"], + ["bench/visual/requirements.txt", "bench/visual/requirements.in"], + ) + ) + + def test_dependency_manifests_match_by_name(self): + self.assertTrue( + is_dependency_only_pr( + ["build(deps): bump the image"], + ["Dockerfile", "test/e2e-compat/Dockerfile", "docker-compose.yml", "test/x/docker-compose.override.yml"], + ) + ) + self.assertFalse( + is_dependency_only_pr( + ["build(deps): bump X"], + ["bench/visual/package.json", "bench/visual/run.py"], + ) + ) + + def test_dependency_only_pr_still_rejects_a_release_commit(self): + self.assertFalse( + is_dependency_only_pr( + ["build(deps): bump X", "fix: handle the new API"], + ["bench/visual/package.json"], + ) + ) + + def test_infra_only_fix_needs_no_entry(self): + self.assertFalse( + should_require_changelog( + ["fix(ci): pin the runner image", "feat(ci): add a lint step"], + [".github/workflows/ci.yaml", "scripts/ci/run_lint.sh", "security/semgrep.yml"], + ) + ) + + def test_product_fix_still_needs_an_entry(self): + self.assertTrue(should_require_changelog(["fix: handle empty range"], ["internal/proxy/range.go"])) + self.assertTrue( + should_require_changelog( + ["fix(ci): pin the runner image"], + [".github/workflows/ci.yaml", "charts/loki-vl-proxy/values.yaml"], + ) + ) + + def test_unit_test_only_fix_still_needs_an_entry(self): + self.assertTrue(should_require_changelog(["fix: cover the empty range"], ["internal/x/y_test.go"])) + + def test_classifier_only_change_needs_no_entry(self): + for files in ( + ["scripts/ci/change_scope.py"], + [".github/workflows/change-scope.yaml"], + ["scripts/ci/change_scope.py", "scripts/ci/tests/test_change_scope.py", "docs/ci.md"], + ): + with self.subTest(files): + self.assertFalse(should_require_changelog(["fix(ci): refine the classifier"], files)) + self.assertTrue(all(is_non_release_path(f) for f in files)) + + def test_every_legacy_non_release_path_is_still_non_release(self): + legacy = [ + "docs/getting-started.md", + "docs/observability.md", + "scripts/ci/tests/test_check_changelog_pr.py", + "website/package.json", + "website/src/pages/index.tsx", + "bench/ab/history/control.jsonl", + "bench/ab/results/daily-control.json", + "README.md", + "CHANGELOG.md", + "LICENSE", + "scripts/ci/check_changelog_pr.py", + "conformance/registry/generated/perf-evidence.json", + "conformance/reports/performance.md", + "conformance/reports/gaps.md", + ] + for path in legacy: + with self.subTest(path): + self.assertTrue(is_non_release_path(path)) + + def test_product_paths_are_not_non_release(self): + for path in ( + "internal/proxy/proxy.go", + "cmd/proxy/main.go", + "charts/loki-vl-proxy/values.yaml", + "go.mod", + "Dockerfile", + "bench/ab/daily.py", + "bench/visual/package.json", + "conformance/registry/baseline.json", + ): + with self.subTest(path): + self.assertFalse(is_non_release_path(path)) + + def test_unit_test_files_stay_non_release_paths(self): + self.assertTrue(is_non_release_path("internal/x/y_test.go")) + self.assertTrue(is_non_release_path("pkg/cache/cache_test.go")) + + if __name__ == "__main__": unittest.main()