feat(client): check broker health before issuance

CHANGELOG.md

@@ -18,6 +18,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+## [v0.10.0] - 2026-07-17
+
+### Added
+- ✨ First-time certificate setup now confirms the registration broker is healthy (HTTP 204 from `/v1/health`) before starting ACME issuance. While the broker keeps failing the check, the client logs a single ERROR and re-checks hourly 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 re-check interval respects a `Retry-After` header sent by the broker when it is longer than the hourly default, capped at 24h. The probe is exposed as `client.CheckBrokerHealth` together with the `client.HealthCheckPath` constant; see [ipfs/kubo#11397](https://github.com/ipfs/kubo/pull/11397) for an example of wiring this in a downstream node. ([#91](https://github.com/ipshipyard/p2p-forge/pull/91))
+
 ## [v0.9.1] - 2026-06-22
 
 ### Fixed

client/acme.go

@@ -42,6 +42,8 @@ type P2PForgeCertMgr struct {
 	log                        *zap.SugaredLogger
 	allowPrivateForgeAddresses bool
 	produceShortAddrs          bool
+	userAgent                  string
+	httpClient                 *http.Client
 
 	hasCert     bool // tracking if we've received a certificate
 	certCheckMx sync.RWMutex
@@ -344,6 +346,8 @@ func NewP2PForgeCertMgr(opts ...P2PForgeCertMgrOptions) (*P2PForgeCertMgr, error
 		allowPrivateForgeAddresses: mgrCfg.allowPrivateForgeAddresses,
 		produceShortAddrs:          mgrCfg.produceShortAddrs,
 		registrationDelay:          mgrCfg.registrationDelay,
+		userAgent:                  mgrCfg.userAgent,
+		httpClient:                 mgrCfg.httpClient,
 	}
 
 	// NOTE: callback getter is necessary to avoid circular dependency
@@ -450,12 +454,21 @@ func (m *P2PForgeCertMgr) Start() error {
 		name := certName(h.ID(), m.forgeDomain)
 		certExists := localCertExists(m.ctx, m.certmagic, name)
 		startCertManagement := func() {
-			// respect WithRegistrationDelay if no cert exists
-			if !certExists && m.registrationDelay != 0 {
-				remainingDelay := m.registrationDelay - time.Since(start)
-				if remainingDelay > 0 {
-					log.Infof("registration delay set to %s, sleeping for remaining %s", m.registrationDelay, remainingDelay)
-					time.Sleep(remainingDelay)
+			if !certExists {
+				// respect WithRegistrationDelay
+				if m.registrationDelay != 0 {
+					remainingDelay := m.registrationDelay - time.Since(start)
+					if remainingDelay > 0 {
+						log.Infof("registration delay set to %s, sleeping for remaining %s", m.registrationDelay, remainingDelay)
+						time.Sleep(remainingDelay)
+					}
+				}
+				// confirm the broker is up before first-time issuance:
+				// without it issuance cannot succeed and certmagic would keep
+				// retrying full ACME flows with backoff for weeks, spamming
+				// ERRORs in logs
+				if !m.waitForHealthyBroker(m.ctx, log) {
+					return
 				}
 			}
 			// start internal certmagic instance

client/health.go

@@ -0,0 +1,123 @@
+package client
+
+import (
+	"context"
+	"fmt"
+	"net/http"
+	"strconv"
+	"strings"
+	"time"
+
+	"go.uber.org/zap"
+)
+
+const (
+	// HealthCheckPath is the well-known liveness endpoint exposed by p2p-forge
+	// registration brokers. A healthy broker responds with HTTP 204.
+	HealthCheckPath = "/v1/health"
+
+	// healthCheckTimeout bounds a single broker health probe. Generous on
+	// purpose: the check runs in a background goroutine, so a slow node or
+	// link delays nothing user-visible, while a timeout that is too tight
+	// would misreport a healthy broker as down.
+	healthCheckTimeout = 15 * time.Second
+
+	// healthCheckRetryInterval is the minimum wait between health checks
+	// while the broker keeps failing them.
+	healthCheckRetryInterval = 1 * time.Hour
+
+	// healthCheckRetryIntervalMax caps how far a broker-supplied Retry-After
+	// header can push out the next health check, so a misconfigured broker
+	// cannot silence a client indefinitely.
+	healthCheckRetryIntervalMax = 24 * time.Hour
+)
+
+// CheckBrokerHealth probes HealthCheckPath of the registration broker at
+// registrationEndpoint and returns nil only when the broker confirms it is
+// healthy with HTTP 204. An empty userAgent defaults to this module's version
+// string, a nil httpClient to http.DefaultClient. The probe is bounded by an
+// internal timeout on top of ctx.
+func CheckBrokerHealth(ctx context.Context, registrationEndpoint string, userAgent string, httpClient *http.Client) error {
+	_, err := checkBrokerHealth(ctx, registrationEndpoint, userAgent, httpClient)
+	return err
+}
+
+// checkBrokerHealth implements CheckBrokerHealth and additionally returns the
+// broker-requested wait from a Retry-After header on a failing response
+// (zero when absent, invalid, or in the past).
+func checkBrokerHealth(ctx context.Context, registrationEndpoint string, userAgent string, httpClient *http.Client) (retryAfter time.Duration, err error) {
+	if userAgent == "" {
+		userAgent = defaultUserAgent
+	}
+	if httpClient == nil {
+		httpClient = http.DefaultClient
+	}
+	ctx, cancel := context.WithTimeout(ctx, healthCheckTimeout)
+	defer cancel()
+
+	healthURL := strings.TrimSuffix(registrationEndpoint, "/") + HealthCheckPath
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, healthURL, nil)
+	if err != nil {
+		return 0, err
+	}
+	req.Header.Set("User-Agent", userAgent)
+
+	resp, err := httpClient.Do(req)
+	if err != nil {
+		return 0, err
+	}
+	defer resp.Body.Close()
+
+	if resp.StatusCode != http.StatusNoContent {
+		return parseRetryAfter(resp.Header.Get("Retry-After")), fmt.Errorf("GET %s: expected HTTP %d, got %s", healthURL, http.StatusNoContent, resp.Status)
+	}
+	return 0, nil
+}
+
+// parseRetryAfter reads a Retry-After value in either RFC 9110 form,
+// delay-seconds or HTTP-date, returning zero when it is absent, invalid, or
+// in the past.
+func parseRetryAfter(v string) time.Duration {
+	if v == "" {
+		return 0
+	}
+	if secs, err := strconv.Atoi(v); err == nil {
+		return max(0, time.Duration(secs)*time.Second)
+	}
+	if t, err := http.ParseTime(v); err == nil {
+		return max(0, time.Until(t))
+	}
+	return 0
+}
+
+// nextHealthCheckDelay returns how long to wait before the next health check
+// after a failing one: at least healthCheckRetryInterval, stretched by a
+// broker-supplied Retry-After up to healthCheckRetryIntervalMax.
+func nextHealthCheckDelay(retryAfter time.Duration) time.Duration {
+	return max(healthCheckRetryInterval, min(retryAfter, healthCheckRetryIntervalMax))
+}
+
+// waitForHealthyBroker blocks until the registration broker confirms it is
+// healthy, and returns false when ctx is canceled first. One cheap GET per
+// interval replaces doomed ACME attempts while the broker is down, and lets
+// certificate setup start automatically once the broker recovers.
+func (m *P2PForgeCertMgr) waitForHealthyBroker(ctx context.Context, log *zap.SugaredLogger) bool {
+	for {
+		retryAfter, err := checkBrokerHealth(ctx, m.forgeRegistrationEndpoint, m.userAgent, m.httpClient)
+		if err == nil {
+			return true
+		}
+		if ctx.Err() != nil {
+			return false
+		}
+		wait := nextHealthCheckDelay(retryAfter)
+		log.Errorf("registration broker at %s did not confirm it is healthy (%s); certificate setup postponed, next health check in %s", m.forgeRegistrationEndpoint, err, wait)
+		timer := time.NewTimer(wait)
+		select {
+		case <-ctx.Done():
+			timer.Stop()
+			return false
+		case <-timer.C:
+		}
+	}
+}

client/health_test.go

@@ -0,0 +1,167 @@
+package client
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+	"time"
+)
+
+func TestParseRetryAfter(t *testing.T) {
+	t.Parallel()
+
+	for _, tc := range []struct {
+		name string
+		in   string
+		want time.Duration
+	}{
+		{"absent", "", 0},
+		{"delay-seconds", "7200", 2 * time.Hour},
+		{"negative seconds", "-5", 0},
+		{"garbage", "not-a-date", 0},
+		{"http-date in the past", "Mon, 02 Jan 2006 15:04:05 GMT", 0},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			if got := parseRetryAfter(tc.in); got != tc.want {
+				t.Fatalf("parseRetryAfter(%q) = %s, want %s", tc.in, got, tc.want)
+			}
+		})
+	}
+
+	t.Run("http-date in the future", func(t *testing.T) {
+		t.Parallel()
+		in := time.Now().Add(90 * time.Minute).UTC().Format(http.TimeFormat)
+		got := parseRetryAfter(in)
+		if got < 89*time.Minute || got > 90*time.Minute {
+			t.Fatalf("parseRetryAfter(%q) = %s, want ~90m", in, got)
+		}
+	})
+}
+
+func TestNextHealthCheckDelay(t *testing.T) {
+	t.Parallel()
+
+	for _, tc := range []struct {
+		name       string
+		retryAfter time.Duration
+		want       time.Duration
+	}{
+		{"no Retry-After keeps the default interval", 0, healthCheckRetryInterval},
+		{"shorter Retry-After does not shrink the interval", 30 * time.Minute, healthCheckRetryInterval},
+		{"longer Retry-After stretches the interval", 2 * time.Hour, 2 * time.Hour},
+		{"excessive Retry-After is capped", 100 * time.Hour, healthCheckRetryIntervalMax},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			if got := nextHealthCheckDelay(tc.retryAfter); got != tc.want {
+				t.Fatalf("nextHealthCheckDelay(%s) = %s, want %s", tc.retryAfter, got, tc.want)
+			}
+		})
+	}
+}
+
+func TestCheckBrokerHealth(t *testing.T) {
+	t.Parallel()
+
+	for _, tc := range []struct {
+		name    string
+		status  int
+		healthy bool
+	}{
+		{"204 is healthy", http.StatusNoContent, true},
+		{"200 is not healthy", http.StatusOK, false},
+		{"404 is not healthy", http.StatusNotFound, false},
+		{"503 is not healthy", http.StatusServiceUnavailable, false},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			gotPath := make(chan string, 1)
+			srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				select {
+				case gotPath <- r.URL.Path:
+				default:
+				}
+				w.WriteHeader(tc.status)
+			}))
+			defer srv.Close()
+
+			err := CheckBrokerHealth(context.Background(), srv.URL, "", nil)
+			if tc.healthy && err != nil {
+				t.Fatalf("expected healthy broker, got error: %v", err)
+			}
+			if !tc.healthy && err == nil {
+				t.Fatal("expected error for unhealthy broker")
+			}
+			if path := <-gotPath; path != HealthCheckPath {
+				t.Fatalf("expected probe of %q, got %q", HealthCheckPath, path)
+			}
+		})
+	}
+
+	t.Run("trailing slash in endpoint", func(t *testing.T) {
+		t.Parallel()
+		gotPath := make(chan string, 1)
+		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			select {
+			case gotPath <- r.URL.Path:
+			default:
+			}
+			w.WriteHeader(http.StatusNoContent)
+		}))
+		defer srv.Close()
+
+		if err := CheckBrokerHealth(context.Background(), srv.URL+"/", "", nil); err != nil {
+			t.Fatalf("expected healthy broker, got error: %v", err)
+		}
+		if path := <-gotPath; path != HealthCheckPath {
+			t.Fatalf("expected probe of %q, got %q", HealthCheckPath, path)
+		}
+	})
+
+	t.Run("unreachable broker", func(t *testing.T) {
+		t.Parallel()
+		// port 0 is never connectable, no dependency on real port state
+		if err := CheckBrokerHealth(context.Background(), "http://127.0.0.1:0", "", nil); err == nil {
+			t.Fatal("expected error for unreachable broker")
+		}
+	})
+
+	t.Run("Retry-After header is surfaced on failure", func(t *testing.T) {
+		t.Parallel()
+		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			w.Header().Set("Retry-After", "7200")
+			w.WriteHeader(http.StatusServiceUnavailable)
+		}))
+		defer srv.Close()
+
+		retryAfter, err := checkBrokerHealth(context.Background(), srv.URL, "", nil)
+		if err == nil {
+			t.Fatal("expected error for unhealthy broker")
+		}
+		if retryAfter != 2*time.Hour {
+			t.Fatalf("expected Retry-After of 2h, got %s", retryAfter)
+		}
+	})
+
+	t.Run("custom user agent is sent", func(t *testing.T) {
+		t.Parallel()
+		gotUA := make(chan string, 1)
+		srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			select {
+			case gotUA <- r.UserAgent():
+			default:
+			}
+			w.WriteHeader(http.StatusNoContent)
+		}))
+		defer srv.Close()
+
+		if err := CheckBrokerHealth(context.Background(), srv.URL, "test-agent/1.0", nil); err != nil {
+			t.Fatalf("expected healthy broker, got error: %v", err)
+		}
+		if ua := <-gotUA; ua != "test-agent/1.0" {
+			t.Fatalf("expected User-Agent %q, got %q", "test-agent/1.0", ua)
+		}
+	})
+}

version.json

@@ -1,3 +1,3 @@
 {
-  "version": "v0.9.1"
+  "version": "v0.10.0"
 }