Skip to content

feat(client): check broker health before issuance - #91

Merged
lidel merged 2 commits into
mainfrom
feat/broker-health-gate
Jul 17, 2026
Merged

feat(client): check broker health before issuance#91
lidel merged 2 commits into
mainfrom
feat/broker-health-gate

Conversation

@lidel

@lidel lidel commented Jul 17, 2026

Copy link
Copy Markdown
Member

Problem

A node without a certificate starts ACME issuance even when the registration broker is unreachable (offline network, firewall, outage). Every attempt is doomed: certmagic retries the full flow with backoff for up to 30 days, filling logs with ERRORs that no amount of retrying can resolve.

Fix

  • before first-time issuance, startCertManagement waits until the broker answers GET /v1/health with HTTP 204 (15s probe timeout)
  • while the check fails, the client logs one ERROR and re-checks after max(1h, min(Retry-After, 24h)); issuance starts automatically once the broker recovers
  • probe exported as client.CheckBrokerHealth with the client.HealthCheckPath constant

One cheap GET per interval replaces full ACME attempts while the broker is down. Nodes with a certificate in storage are unaffected; renewal behavior is out of scope.

Merging cuts release v0.10.0 (version.json and changelog are included). Downstream wiring is validated in ipfs/kubo#11397, whose CI is green against this PR's feature commit.

First-time certificate setup now confirms the registration broker
responds with HTTP 204 on /v1/health before starting ACME issuance.
While the broker keeps failing the check, the client logs one ERROR
and re-checks after max(1h, min(Retry-After, 24h)) instead of running
doomed ACME flows that certmagic would retry with backoff for weeks.
Issuance starts automatically once the broker recovers. Nodes with a
certificate already in storage are unaffected.

The probe is exported as client.CheckBrokerHealth together with the
client.HealthCheckPath constant.
@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.43860% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.17%. Comparing base (d629320) to head (254b386).

Files with missing lines Patch % Lines
client/health.go 74.46% 11 Missing and 1 partial ⚠️
client/acme.go 80.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #91      +/-   ##
==========================================
+ Coverage   68.70%   69.17%   +0.46%     
==========================================
  Files          21       22       +1     
  Lines        1713     1765      +52     
==========================================
+ Hits         1177     1221      +44     
- Misses        416      425       +9     
+ Partials      120      119       -1     

☔ 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.

@github-actions

Copy link
Copy Markdown

Suggested tag: v0.10.0

Comparing to: v0.9.1 (diff)

gorelease says:

# summary
v0.10.0 is a valid semantic version for this release.

gocompat says:

HEAD is now at d629320 fix(client): keep registration on one LB backend (#90)
Switched to branch 'main'
Your branch is up to date with 'origin/main'.

Cutting a Release (and modifying code files)

This PR is modifying both version.json and code files (not markdown, YAML, TOML or lock files).
The Release Checker is not able to analyse files that are not checked in to main. This might cause the above analysis to be inaccurate.
Please consider performing all the code changes in a separate PR before cutting the release.

Automatically created GitHub Release

A draft GitHub Release has been created.
It is going to be published when this PR is merged.
You can modify its' body to include any release notes you wish to include with the release.

@lidel
lidel marked this pull request as ready for review July 17, 2026 13:17
@lidel
lidel merged commit a804ecb into main Jul 17, 2026
10 checks passed
@lidel
lidel deleted the feat/broker-health-gate branch July 17, 2026 13:35
lidel added a commit to ipfs/kubo that referenced this pull request Jul 17, 2026
* feat(autotls): skip issuance when broker is down

Bump github.com/ipshipyard/p2p-forge to the head of
ipshipyard/p2p-forge#91: before first-time ACME issuance the client
now confirms the broker responds with HTTP 204 on /v1/health, after
the registration delay and once the node is publicly reachable.
While the broker keeps failing the check, certificate setup is
postponed with one ERROR and an hourly re-check (respecting
Retry-After, capped at 24h) instead of doomed ACME retries for weeks.

Ephemeral nodes (CI runners) still produce no broker traffic at all,
and nodes with a certificate in storage are unaffected.

* chore: update p2p-forge to v0.10.0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant