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()