Skip to content

ci: gate PR merges on >=90% patch coverage#23692

Open
Surabhi-1605 wants to merge 6 commits into
mainfrom
ci/enforce-patch-coverage
Open

ci: gate PR merges on >=90% patch coverage#23692
Surabhi-1605 wants to merge 6 commits into
mainfrom
ci/enforce-patch-coverage

Conversation

@Surabhi-1605

@Surabhi-1605 Surabhi-1605 commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Builds on the existing Go coverage reporting (#23613) to add a deterministic, self-contained patch-coverage merge gate. It enforces that the new/changed Go code in a PR is at least 90% covered by tests, without depending on Codecov or the repo's historical total.

This mirrors the gate landing in consul-k8s and consul-esm, adapted to consul's multi-module layout and existing coverage pipeline.

Changes (workflow YAML only)

  • reusable-coverage-report.yml
    • New "Compute patch coverage" step (PR-only), reusing the already-merged/filtered profile.
    • Exposes patch, patch_covered, patch_coverable (and total) as workflow_call outputs.
    • Adds a patch-coverage line to the existing PR coverage comment data.
  • coverage-comment.yml — includes the patch line in the fork-safe workflow_run comment.
  • go-tests.yml
    • New coverage-gate job that fails the run when patch coverage < 90%. Runs only for pull requests; pushes to main still produce the badge but are not gated. No coverable Go lines changed → N/A → passes.
    • coverage-gate is added to the go-tests-success fan-in, so it blocks merges through the existing required checkno branch-protection change required.

Scope / follow-up

The gate uses the coverage data the report already merges (today: the go-test-ce root-module profiles). Changed lines in modules that don't yet upload coverage (api, sdk, envoyextensions, troubleshoot, testing/deployer) are treated as non-coverable and reported as N/A. It is forward-compatible: when those module jobs start uploading *-coverage artifacts, both the total and the patch gate pick them up automatically. Wiring up module coverage uploads can be a follow-up.

Validation

  • All three workflow files pass YAML parsing; all GitHub output names use underscores.
  • The diff parser + profile-intersection awk scripts were extracted from this PR and unit-tested offline with synthetic multi-module data (root agent/, api/, sdk/): each module maps correctly, generated/excluded files are ignored, and the percentage matches by hand. Threshold boundaries verified (89.9% → FAIL, 90.0% → PASS) and the N/A path passes.

Note: this PR only touches workflow YAML, so the patch step finds no coverable Go lines → reports N/A → gate passes. That exercises the plumbing; real enforcement kicks in on PRs that touch covered Go code.

Builds on the existing Go coverage reporting (#23613) to add a deterministic,
self-contained patch-coverage merge gate for pull requests.

What it does
- reusable-coverage-report.yml computes "patch coverage": of the non-test,
  non-mock, non-generated Go lines a PR adds/modifies (within the modules that
  have coverage data), the share exercised by tests. Changed lines are read
  from the PR diff via the GitHub API and intersected with the already-merged,
  already-filtered coverage profile. All consul modules share the
  github.com/hashicorp/consul/<subdir> import prefix and diff paths are
  repo-relative, so a single prefix maps every module correctly.
- The result is exposed as workflow_call outputs (patch, patch_covered,
  patch_coverable) and surfaced in the existing PR coverage comment (both the
  direct and the fork-safe workflow_run paths).
- A new coverage-gate job in go-tests.yml fails when patch coverage is below
  PATCH_THRESHOLD (90%). It runs only for pull requests; pushes to main still
  produce the badge but are not gated. PRs that change no coverable Go lines
  report N/A and pass. The job is added to the go-tests-success fan-in, so it
  blocks merges through the existing required check (no branch-protection
  change required).

Why patch coverage
- Enforces test quality on new/changed code without being held hostage to the
  repo's historical total; the 90% bar is independent of overall coverage.

Codecov stays informational; this gate is the source of truth for merges.
@Surabhi-1605
Surabhi-1605 requested review from a team as code owners June 24, 2026 16:54
@github-actions github-actions Bot added type/ci Relating to continuous integration (CI) tooling for testing or releases theme/contributing Additions and enhancements to community contributing materials labels Jun 24, 2026
@github-actions

github-actions Bot commented Jun 24, 2026

Copy link
Copy Markdown

Go Test Coverage: 71.6%

See the workflow run for the full per-package breakdown and downloadable HTML report.

@codecov-commenter

codecov-commenter commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.62%. Comparing base (bf60308) to head (bd55028).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #23692      +/-   ##
==========================================
+ Coverage   62.15%   65.62%   +3.46%     
==========================================
  Files         755      951     +196     
  Lines       91730   114904   +23174     
==========================================
+ Hits        57014    75403   +18389     
- Misses      29095    32473    +3378     
- Partials     5621     7028    +1407     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Surabhi-1605 Surabhi-1605 self-assigned this Jun 25, 2026
@Surabhi-1605 Surabhi-1605 added backport/2.0 Changes are backported to 2.0 pr/no-changelog PR does not need a corresponding .changelog entry labels Jun 25, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a PR-only patch coverage computation to the existing Go coverage reporting pipeline and introduces a CI gate that blocks merges when new/changed Go code in a PR is < 90% covered, without relying on historical total coverage.

Changes:

  • Exposes patch-coverage metrics (patch, patch_covered, patch_coverable) as outputs from reusable-coverage-report.yml.
  • Adds a coverage-gate job to go-tests.yml that fails PR runs when patch coverage is below the threshold (passes on N/A).
  • Extends the PR coverage comment (both inline and fork-safe workflow_run) to include a patch coverage line.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
.github/workflows/reusable-coverage-report.yml Computes patch coverage from PR diffs, exports outputs, and includes patch coverage in the PR comment payload.
.github/workflows/go-tests.yml Adds a PR-only coverage-gate job and wires it into the existing required-check fan-in.
.github/workflows/coverage-comment.yml Updates the fork-safe comment workflow to include patch coverage information.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +159 to +166
gh api --paginate "repos/${REPO}/pulls/${PR}/files" \
--jq '.[]
| select(.filename | test("\\.go$"))
| select(.filename | test("_test\\.go$") | not)
| select(.filename | test("\\.pb\\.go$") | not)
| select(.filename | test("(^|/)mocks/|(^|/)mock_") | not)
| "FILE\t\(.filename)\n\(.patch // "")"' > /tmp/pr-patches.txt || true

Comment on lines +175 to +178
ch=substr($0,1,1)
if (ch=="+") { print module"/"file"\t"newline; newline++ }
else if (ch=="-") { } # removed line: no new-file number
else { newline++ } # context line advances pointer
Comment thread .github/workflows/reusable-coverage-report.yml Outdated
Comment thread .github/workflows/coverage-comment.yml Outdated
Address Copilot review on #23692:

- Fail closed in the patch-coverage computation: stop suppressing gh/API
  errors (drop '|| true') and flag changed .go files whose diff the API
  cannot return (.patch == null) as NULLPATCH, failing with a clear
  message instead of silently undercounting changed lines and letting the
  gate pass as N/A.
- Ignore unified-diff metadata lines ('\ No newline at end of file') in
  the line-number parser so they don't advance the new-file pointer and
  shift subsequent changed-line numbers.
- Drop the hard-coded '>= 90%' text from the PR comment in both the direct
  and workflow_run comment paths; the threshold lives in go-tests.yml
  (PATCH_THRESHOLD) and the gate job's failure message remains the
  authoritative source, avoiding drift.
Surabhi-1605 added a commit to hashicorp/consul-k8s that referenced this pull request Jun 25, 2026
- Fail closed: stop suppressing gh/API errors and flag changed .go files
  whose diff the API cannot return (.patch == null) as NULLPATCH, failing
  with a clear message instead of silently undercounting changed lines.
- Ignore unified-diff metadata lines ('\ No newline at end of file') so
  they don't advance the new-file line pointer and shift line numbers.
- Drop the hard-coded '>= 90%' text from the PR comment; the threshold
  lives in the gate job (PATCH_THRESHOLD), avoiding drift.

Mirrors hashicorp/consul#23692 review hardening.
Surabhi-1605 added a commit to hashicorp/consul-ecs that referenced this pull request Jun 25, 2026
- Fail closed: stop suppressing gh/API errors and flag changed .go files
  whose diff the API cannot return (.patch == null) as NULLPATCH, failing
  with a clear message instead of silently undercounting changed lines.
- Ignore unified-diff metadata lines ('\ No newline at end of file') so
  they don't advance the new-file line pointer and shift line numbers.
- Drop the hard-coded '>= 90%' text from the PR comment; the threshold
  lives in the gate job (PATCH_THRESHOLD), avoiding drift.

Mirrors hashicorp/consul#23692 review hardening.
Surabhi-1605 added a commit to hashicorp/consul-terraform-sync that referenced this pull request Jun 25, 2026
- Fail closed: stop suppressing gh/API errors and flag changed .go files
  whose diff the API cannot return (.patch == null) as NULLPATCH, failing
  with a clear message instead of silently undercounting changed lines.
- Ignore unified-diff metadata lines ('\ No newline at end of file') so
  they don't advance the new-file line pointer and shift line numbers.
- Drop the hard-coded '>= 90%' text from the PR comment in both the direct
  and workflow_run comment paths; the threshold lives in the gate job
  (PATCH_THRESHOLD), avoiding drift.

Mirrors hashicorp/consul#23692 review hardening.
Surabhi-1605 added a commit to hashicorp/consul-esm that referenced this pull request Jun 25, 2026
- Fail closed: stop suppressing gh/API errors and flag changed .go files
  whose diff the API cannot return (.patch == null) as NULLPATCH, failing
  with a clear message instead of silently undercounting changed lines.
- Ignore unified-diff metadata lines ('\ No newline at end of file') so
  they don't advance the new-file line pointer and shift line numbers.
- Drop the hard-coded '>= 90%' text from the PR comment; the threshold
  lives in the gate job (PATCH_THRESHOLD), avoiding drift.

Mirrors hashicorp/consul#23692 review hardening.
Surabhi-1605 added a commit to hashicorp/consul-k8s that referenced this pull request Jul 16, 2026
* ci: gate PR merges on >=90% patch coverage

Add a deterministic, self-contained patch-coverage merge gate that runs on
pull requests across both coverage-tested modules (control-plane and cli).

What it does
- reusable-coverage-report.yml computes "patch coverage": of the non-test,
  non-mock Go lines a PR adds/modifies, the share that is exercised by tests.
  Changed lines are read from the PR diff via the GitHub API and intersected
  with the already mock/acceptance-filtered merged coverage profile. Because
  repo-relative diff paths already include the module subdir (control-plane/...,
  cli/...), a single import-path prefix maps both modules correctly.
- The result is exposed as workflow_call outputs (patch, patch_covered,
  patch_coverable) and shown in the existing PR coverage comment.
- A new coverage-gate job in coverage.yml fails the run when patch coverage is
  below PATCH_THRESHOLD (90%). It runs only for pull requests (pr-number set);
  push/merge runs skip it. PRs that change no coverable Go lines (docs/tests
  only) report N/A and pass.

Why patch coverage
- Enforces test quality on new/changed code without being held hostage to the
  repo's historical total. The threshold is fixed and independent of overall
  coverage.

Codecov stays informational; this gate is the source of truth for merges.
To activate enforcement, add "coverage-gate" as a required status check in
branch protection for main.

* ci: harden patch-coverage gate per review feedback

- Fail closed: stop suppressing gh/API errors and flag changed .go files
  whose diff the API cannot return (.patch == null) as NULLPATCH, failing
  with a clear message instead of silently undercounting changed lines.
- Ignore unified-diff metadata lines ('\ No newline at end of file') so
  they don't advance the new-file line pointer and shift line numbers.
- Drop the hard-coded '>= 90%' text from the PR comment; the threshold
  lives in the gate job (PATCH_THRESHOLD), avoiding drift.

Mirrors hashicorp/consul#23692 review hardening.

* ci: derive coverage pr-number from event_name (avoid null on push)

* ci: fix NULLPATCH fail-closed grep (tab escape not honored by GNU grep)

* ci: fail coverage gate closed when patch coverage is not computed
pajay-rao added a commit to hashicorp/consul-k8s that referenced this pull request Jul 24, 2026
* update dependencies: bump hashicorp/consul/api to v1.34.4, hashicorp/serf to v0.10.4, and hashicorp/go-metrics to v0.6.0 (#5467)

* update dependencies: bump hashicorp/consul/api to v1.34.4, hashicorp/serf to v0.10.4, and hashicorp/go-metrics to v0.6.0

* Update module replacements in go.mod

Updated module replacements for control-plane types.

* update alpine to 3.24 (#5474)

* update alpine to 3.24

* add changelog for alpine 3.24 update

* ext_authz support for api-gateway  (#5444)

* initial commit

* mod update

* update unit tests

* changelog: add entry for api-gateway ext_authz support

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* test: add acceptance tests for API gateway ext_authz with OIDC

* chore: update dependencies in go.mod and go.sum

* test: add waitForConsulServiceRegistered helper to improve route status assertions

* chore: remove unused golang.org/x/net dependency from go.sum

* test(api-gateway): create ext_authz OIDC route after backend is ready

The ext_authz OIDC acceptance tests (TestAPIGateway_ExtAuthz_OIDC and its
within-mesh counterpart) applied the HTTPRoute together with static-server via
kustomize. When the route was reconciled before static-server had Ready
endpoints, consul-k8s wrote the Consul http-route with zero upstreams and Consul
reported NoUpstreamServicesTargeted; recovery could exceed the assertion window,
most notably in the heavier within-mesh scenario (TestAPIGateway_ExtAuthz_OIDC_Mesh).

Mirror the proven pattern from api_gateway_test.go: remove httproute.yaml from
both kustomizations and apply it from the test only after static-server is Ready
and registered in the Consul catalog, so the route's first (and only) Consul
write already targets a resolvable upstream and reaches Accepted directly.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Enabling routeextproc crd and addition of it to httproutes for Consul ENT(#5449)

* Enabling routeextproc crd and addition of it to httproutes

* Addition of RouteExtProc crd and its related changes

* Addition of ent check for consul/templates/crd-routeextprocs.yaml

* Updating go mod for testing

* Updating go mod for testing

* Updating go mod and sum changes for testing

* Addition of consul version for testing

* Implemeting missing constoller

* Testing multiport changes

* Testing multiport changes

* Testing multiport changes

* Potential fix for pull request finding

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* Addition of testcases

* Reverting changes to makefile

* Reverting changes to makefile

* Reverting changes to makefile

---------

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

* security: upgrade Python deps in gateway-api 0.7.1 module (4 advisories) (#5473)

* security: upgrade Markdown 3.3.7 -> 3.8.1 to fix GHSA-5wmx-573v-2qwq (CVE-2025-69534)

Markdown < 3.8.1 raises an uncaught AssertionError from html.parser on
malformed HTML-like input, enabling a remote DoS in anything rendering
untrusted Markdown. Bumps mkdocs 1.4.3 -> 1.6.1 as well because mkdocs
1.4.3 pins markdown<3.4, which excludes the fix. Applies to the vendored
gateway-api 0.7.1 custom docs toolchain.

* changelog: add entry for Markdown GHSA-5wmx-573v-2qwq (#5473)

* security: upgrade tornado 6.5.5 -> 6.5.7 to fix three tornado advisories

Folds the tornado bump into this PR so a single change covers all four
gateway-api 0.7.1 module advisories:
- GHSA-mgf9-4vpg-hj56 (CVE-2026-49855) AsyncHTTPClient gzip bomb
- GHSA-3x9g-8vmp-wqvf cross-origin Authorization/Cookie leak
- GHSA-pw6j-qg29-8w7f CurlAsyncHTTPClient credential leak on handle reuse
(Markdown 3.8.1 / mkdocs 1.6.1 for GHSA-5wmx-573v-2qwq already in this branch.)

* build: bump gateway-api-0.7.1-custom to v0.1.7

Pin control-plane and acceptance to the newly tagged v0.1.7 of the custom
gateway-api 0.7.1 module, which carries the tornado 6.5.7 and Markdown 3.8.1
security fixes (GHSA-mgf9-4vpg-hj56, GHSA-3x9g-8vmp-wqvf, GHSA-pw6j-qg29-8w7f,
GHSA-5wmx-573v-2qwq).

* argocd fix (#5491)

* argocd fix

* add changelog

* add comments for "generateManifestsStorageClassName"

* fix space issue

* auto take the storageClass

* ci: gate PR merges on >=90% patch coverage (#5424)

* ci: gate PR merges on >=90% patch coverage

Add a deterministic, self-contained patch-coverage merge gate that runs on
pull requests across both coverage-tested modules (control-plane and cli).

What it does
- reusable-coverage-report.yml computes "patch coverage": of the non-test,
  non-mock Go lines a PR adds/modifies, the share that is exercised by tests.
  Changed lines are read from the PR diff via the GitHub API and intersected
  with the already mock/acceptance-filtered merged coverage profile. Because
  repo-relative diff paths already include the module subdir (control-plane/...,
  cli/...), a single import-path prefix maps both modules correctly.
- The result is exposed as workflow_call outputs (patch, patch_covered,
  patch_coverable) and shown in the existing PR coverage comment.
- A new coverage-gate job in coverage.yml fails the run when patch coverage is
  below PATCH_THRESHOLD (90%). It runs only for pull requests (pr-number set);
  push/merge runs skip it. PRs that change no coverable Go lines (docs/tests
  only) report N/A and pass.

Why patch coverage
- Enforces test quality on new/changed code without being held hostage to the
  repo's historical total. The threshold is fixed and independent of overall
  coverage.

Codecov stays informational; this gate is the source of truth for merges.
To activate enforcement, add "coverage-gate" as a required status check in
branch protection for main.

* ci: harden patch-coverage gate per review feedback

- Fail closed: stop suppressing gh/API errors and flag changed .go files
  whose diff the API cannot return (.patch == null) as NULLPATCH, failing
  with a clear message instead of silently undercounting changed lines.
- Ignore unified-diff metadata lines ('\ No newline at end of file') so
  they don't advance the new-file line pointer and shift line numbers.
- Drop the hard-coded '>= 90%' text from the PR comment; the threshold
  lives in the gate job (PATCH_THRESHOLD), avoiding drift.

Mirrors hashicorp/consul#23692 review hardening.

* ci: derive coverage pr-number from event_name (avoid null on push)

* ci: fix NULLPATCH fail-closed grep (tab escape not honored by GNU grep)

* ci: fail coverage gate closed when patch coverage is not computed

* Removing for ENT check crd-routeextprocs (#5508)

* Removing for check crd-routeextprocs

* Addition of controller check for RouteExtProc

* Reverting controller changes

* Increase stale PR windows to 90/60 days (#5506)

* Increase stale PR windows to 90/60 days

* Add changelog entry for stale PR window update

* Upgrade actions/stale to v10.4.0

* Disabling ENT testcases for Consul CE acceptance test case runs (#5510)

* Disabling ent related tast cases for CE runs

* testing changes to acceptance workflow

* testing changes to acceptance workflow

* Skipping an enterprise testcase

* Skipping an enterprise testcase

* Addition of enterprise testcase

---------

Co-authored-by: Bharath <kallamadibharath.reddy@hashicorp.com>
Co-authored-by: Paras Gupta <paras.gupta@hashicorp.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Surabhi-1605 <surabhi.singh@hashicorp.com>
Co-authored-by: vdinesh4738 <dinesh.vadlamudi@hashicorp.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport/2.0 Changes are backported to 2.0 pr/no-changelog PR does not need a corresponding .changelog entry theme/contributing Additions and enhancements to community contributing materials type/ci Relating to continuous integration (CI) tooling for testing or releases

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants