Address ACME onboarding review feedback

commit: d200400726be9a444655f75d69cbcb28c6a328f4
author: brice · 5mo ago · 2026-05-01 10:03 UTC
committer: brice · 5mo ago · 2026-05-01 10:05 UTC
parent: b026f15
signature: GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email

6 files · +106 -137

M docs/config/server.md +2 -2


@@ -106,9 +106,9 @@ Challenge behavior:
106	106
107	107		HTTP-01 requires public access to port 80 for each hostname being validated. If you use Cloudflare in DNS-only/gray-cloud mode, your firewall/NAT must allow direct public traffic to Gordon's HTTP proxy port. A firewall rule that only allows Cloudflare source IPs on port 80 is compatible with orange-cloud proxying, but it blocks gray-cloud HTTP-01 validation; use DNS-01 or temporarily open port 80 for direct validation.
108	108
109		-	Gordon limits new ACME certificate orders to `obtain_batch_size` per reconcile run (default `1`) so enabling ACME on an existing multi-route server does not burst through every route and hit Let's Encrypt rate limits. Later reloads, restarts, or renewal-loop reconciles continue issuing remaining certificates.
	109	+	Gordon limits new ACME certificate orders to `obtain_batch_size` per reconcile run (default `1`) so enabling ACME on an existing multi-route server does not burst through every route and hit Let's Encrypt rate limits. Later reloads, restarts, or other explicit reconcile runs continue issuing remaining certificates.
110	110
111		-	If the initial ACME reconcile fails (e.g. due to a transient network error or misconfiguration), Gordon logs the failure and does not start the renewal loop. Operators should fix the underlying error and restart/reload Gordon to trigger a new reconcile attempt.
	111	+	If the initial ACME reconcile fails (e.g. due to a transient network error or misconfiguration), Gordon logs the failure but still starts the renewal loop for already cached certificates. Operators should fix the underlying error and restart/reload Gordon to trigger a new obtain attempt for missing certificates.
112	112
113	113		For Cloudflare Full/Strict, Cloudflare terminates browser TLS at the edge and connects to Gordon over HTTPS. A public ACME certificate served by Gordon is the preferred origin certificate because Cloudflare Strict can validate it without custom origin trust. Cloudflare Flexible mode (HTTPS at the edge, HTTP to Gordon) remains a legacy/compatibility option, but it is not end-to-end HTTPS.
114	114


@@ -106,9 +106,9 @@ Challenge behavior:
106			106
107		HTTP-01 requires public access to port 80 for each hostname being validated. If you use Cloudflare in DNS-only/gray-cloud mode, your firewall/NAT must allow direct public traffic to Gordon's HTTP proxy port. A firewall rule that only allows Cloudflare source IPs on port 80 is compatible with orange-cloud proxying, but it blocks gray-cloud HTTP-01 validation; use DNS-01 or temporarily open port 80 for direct validation.	107		HTTP-01 requires public access to port 80 for each hostname being validated. If you use Cloudflare in DNS-only/gray-cloud mode, your firewall/NAT must allow direct public traffic to Gordon's HTTP proxy port. A firewall rule that only allows Cloudflare source IPs on port 80 is compatible with orange-cloud proxying, but it blocks gray-cloud HTTP-01 validation; use DNS-01 or temporarily open port 80 for direct validation.
108			108
109	-	Gordon limits new ACME certificate orders to `obtain_batch_size` per reconcile run (default `1`) so enabling ACME on an existing multi-route server does not burst through every route and hit Let's Encrypt rate limits. Later reloads, restarts, or renewal-loop reconciles continue issuing remaining certificates.	109	+	Gordon limits new ACME certificate orders to `obtain_batch_size` per reconcile run (default `1`) so enabling ACME on an existing multi-route server does not burst through every route and hit Let's Encrypt rate limits. Later reloads, restarts, or other explicit reconcile runs continue issuing remaining certificates.
110			110
111	-	If the initial ACME reconcile fails (e.g. due to a transient network error or misconfiguration), Gordon logs the failure and does not start the renewal loop. Operators should fix the underlying error and restart/reload Gordon to trigger a new reconcile attempt.	111	+	If the initial ACME reconcile fails (e.g. due to a transient network error or misconfiguration), Gordon logs the failure but still starts the renewal loop for already cached certificates. Operators should fix the underlying error and restart/reload Gordon to trigger a new obtain attempt for missing certificates.
112			112
113		For Cloudflare Full/Strict, Cloudflare terminates browser TLS at the edge and connects to Gordon over HTTPS. A public ACME certificate served by Gordon is the preferred origin certificate because Cloudflare Strict can validate it without custom origin trust. Cloudflare Flexible mode (HTTPS at the edge, HTTP to Gordon) remains a legacy/compatibility option, but it is not end-to-end HTTPS.	113		For Cloudflare Full/Strict, Cloudflare terminates browser TLS at the edge and connects to Gordon over HTTPS. A public ACME certificate served by Gordon is the preferred origin certificate because Cloudflare Strict can validate it without custom origin trust. Cloudflare Flexible mode (HTTPS at the edge, HTTP to Gordon) remains a legacy/compatibility option, but it is not end-to-end HTTPS.
114			114

D docs/superpowers/plans/2026-04-30-acme-http-onboarding-hardening.md +0 -110

@@ -1,110 +0,0 @@
1	-	# ACME HTTP Onboarding Hardening Implementation Plan
2	-
3	-	> For agentic workers: REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan phase-by-phase. Each phase contains sub-tasks with checkbox steps (`- [ ]`) for tracking. Review gates happen at the end of each phase, not after every sub-task.
4	-
5	-	Goal: Make Gordon's public HTTP/TLS behavior safe and practical for ACME HTTP-01, Cloudflare Full/Strict origin HTTPS, and internal CA onboarding without stealing application paths.
6	-
7	-	Architecture: Keep ACME HTTP-01 as the highest-priority public HTTP route. Move CA onboarding resources into a Gordon-owned `/.well-known/gordon/*` namespace, keep legacy direct-HTTP onboarding paths for compatibility, and gate HTTPS onboarding to `server.gordon_domain` so app hosts can own `/ca`. Delay runtime ACME reconciliation until listeners are bound. Treat Cloudflare Flexible HTTP-origin mode as legacy/compat; the target mode is Cloudflare Full/Strict with Cloudflare connecting to Gordon over HTTPS and validating a Gordon-served certificate. Limit ACME obtains per reconcile run so enabling ACME on an existing multi-route host does not burst through every route and risk Let's Encrypt rate limits.
8	-
9	-	Tech Stack: Go 1.25, `net/http`, Cobra CLI, Gordon hexagonal app/usecase/adapters, testify tests.
10	-
11	-	---
12	-
13	-	## Phase 1: Failing behavior tests
14	-
15	-	Goal: Codify desired HTTP/onboarding routing and ACME startup timing before production edits.
16	-
17	-	Files:
18	-	- Modify: `internal/app/run_http_handlers_test.go`
19	-	- Modify/Create: `internal/app/public_tls_startup_test.go` if helper isolation is needed
20	-
21	-	### Sub-task 1: Well-known HTTP onboarding paths
22	-
23	-	- [ ] Add tests showing direct HTTP serves `/.well-known/gordon/`, `/.well-known/gordon/ca.crt`, and `/.well-known/gordon/ca.mobileconfig`.
24	-	- [ ] Keep existing direct HTTP `/`, `/ca`, `/ca.crt`, `/ca.mobileconfig` compatibility tests green.
25	-	- [ ] Run `rtk go test ./internal/app/... -run 'TestCreateHTTPHandlers_DirectHTTP.Gordon\|TestCreateHTTPHandlers_DirectHTTP.CA'` and confirm new tests fail before implementation.
26	-
27	-	### Sub-task 2: HTTPS host gating
28	-
29	-	- [ ] Add tests showing `httpsHandler` serves `/ca` only when `req.Host == cfg.Server.GordonDomain`.
30	-	- [ ] Add tests showing `/ca` on an arbitrary app host reaches the proxy path instead of onboarding.
31	-	- [ ] Run `rtk go test ./internal/app/... -run 'TestCreateHTTPHandlers_HTTPSOnboarding'` and confirm the arbitrary-host test fails before implementation.
32	-
33	-	### Sub-task 3: Delayed ACME runtime startup
34	-
35	-	- [ ] Add a small app-level test helper/fake proving runtime public TLS `Reconcile` and `StartRenewalLoop` happen after proxy readiness.
36	-	- [ ] Run the targeted test and confirm it fails with current service-construction-time reconciliation.
37	-
38	-	### Sub-task 4: ACME obtain batch limiting
39	-
40	-	- [ ] Add usecase tests proving one reconcile run obtains only a bounded number of missing certificates.
41	-	- [ ] Add a test proving a later reconcile run can continue with the remaining missing certificates.
42	-	- [ ] Run `rtk go test ./internal/usecase/publictls/... -run 'Batch\|Limit\|Reconcile'` and confirm the new batch-limit tests fail before implementation.
43	-
44	-	Phase review checkpoint: Confirm tests express product behavior, not implementation details.
45	-
46	-	---
47	-
48	-	## Phase 2: Implementation
49	-
50	-	Goal: Pass tests with minimal production changes.
51	-
52	-	Files:
53	-	- Modify: `internal/app/run.go`
54	-	- Modify: `internal/app/run_http_handlers_test.go`
55	-
56	-	### Sub-task 1: Onboarding route changes
57	-
58	-	- [ ] Add `/.well-known/gordon/` direct HTTP onboarding routes.
59	-	- [ ] Keep legacy direct HTTP `/`, `/ca`, `/ca.crt`, `/ca.mobileconfig` routes.
60	-	- [ ] Replace unconditional HTTPS `/ca*` registrations with a host-gated handler that only serves onboarding on `server.gordon_domain`.
61	-
62	-	### Sub-task 2: Delayed runtime ACME startup
63	-
64	-	- [ ] Change runtime service creation to initialize public TLS without immediate reconcile/renewal loop side effects.
65	-	- [ ] After registry, HTTP proxy, and TLS listeners are ready, call public TLS `Reconcile` and `StartRenewalLoop`.
66	-	- [ ] Preserve local CLI read-only behavior: `NewKernelQuiet` must still avoid reconcile/renewal loop side effects.
67	-
68	-	### Sub-task 3: ACME obtain batch limiting
69	-
70	-	- [ ] Add a conservative default obtain batch size for runtime reconciliation.
71	-	- [ ] Make `Service.Reconcile` process at most the configured number of missing targets per run.
72	-	- [ ] Log when targets remain pending so operators understand that issuance is intentionally staged.
73	-
74	-	### Sub-task 4: Compatibility check
75	-
76	-	- [ ] Verify ACME challenge handler is still registered before onboarding/proxy handlers.
77	-	- [ ] Verify trusted proxy traffic keeps existing behavior.
78	-	- [ ] Verify TLS-disabled HTTP behavior is unchanged.
79	-
80	-	Phase review checkpoint: Review for startup ordering, goroutine lifecycle, and route conflicts.
81	-
82	-	---
83	-
84	-	## Phase 3: Docs and verification
85	-
86	-	Goal: Document the operational model and validate the branch.
87	-
88	-	Files:
89	-	- Modify: `docs/config/server.md`
90	-	- Modify: `docs/concepts.md` if needed
91	-	- Modify: `docs/cli/tls.md` if status docs need wording
92	-	- Modify: `docs/config/server.md` to document Cloudflare Full/Strict origin HTTPS expectations
93	-
94	-	### Sub-task 1: Documentation
95	-
96	-	- [ ] Document that gray-cloud HTTP-01 requires public/NAT/firewall access to port 80.
97	-	- [ ] Document that Cloudflare-only IP allowlisting on port 80 is incompatible with gray-cloud HTTP-01; use DNS-01 or open 80 for direct validation.
98	-	- [ ] Document `/.well-known/gordon/` CA onboarding paths and mark `/ca` direct HTTP paths as legacy compatibility.
99	-	- [ ] Explain Cloudflare flexible HTTP origin as legacy/explicit, not preferred for end-to-end HTTPS.
100	-	- [ ] Document Cloudflare Full/Strict expected flow: browser -> Cloudflare HTTPS edge cert, Cloudflare -> Gordon HTTPS origin cert. Public ACME certs served by Gordon are the preferred origin certificate because Cloudflare Strict can validate them without a custom origin trust setup.
101	-
102	-	### Sub-task 2: Verification
103	-
104	-	- [ ] Run `rtk gofmt -w` on changed Go files.
105	-	- [ ] Run targeted app tests.
106	-	- [ ] Run `rtk go test ./...`.
107	-	- [ ] Run `rtk golangci-lint run ./...`.
108	-	- [ ] Request Go reviewer feedback and fix blocking issues.
109	-
110	-	Phase review checkpoint: Ensure docs match code and Igor findings.

@@ -1,110 +0,0 @@
1	-	# ACME HTTP Onboarding Hardening Implementation Plan
2	-
3	-	> For agentic workers: REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan phase-by-phase. Each phase contains sub-tasks with checkbox steps (`- [ ]`) for tracking. Review gates happen at the end of each phase, not after every sub-task.
4	-
5	-	Goal: Make Gordon's public HTTP/TLS behavior safe and practical for ACME HTTP-01, Cloudflare Full/Strict origin HTTPS, and internal CA onboarding without stealing application paths.
6	-
7	-	Architecture: Keep ACME HTTP-01 as the highest-priority public HTTP route. Move CA onboarding resources into a Gordon-owned `/.well-known/gordon/*` namespace, keep legacy direct-HTTP onboarding paths for compatibility, and gate HTTPS onboarding to `server.gordon_domain` so app hosts can own `/ca`. Delay runtime ACME reconciliation until listeners are bound. Treat Cloudflare Flexible HTTP-origin mode as legacy/compat; the target mode is Cloudflare Full/Strict with Cloudflare connecting to Gordon over HTTPS and validating a Gordon-served certificate. Limit ACME obtains per reconcile run so enabling ACME on an existing multi-route host does not burst through every route and risk Let's Encrypt rate limits.
8	-
9	-	Tech Stack: Go 1.25, `net/http`, Cobra CLI, Gordon hexagonal app/usecase/adapters, testify tests.
10	-
11	-	---
12	-
13	-	## Phase 1: Failing behavior tests
14	-
15	-	Goal: Codify desired HTTP/onboarding routing and ACME startup timing before production edits.
16	-
17	-	Files:
18	-	- Modify: `internal/app/run_http_handlers_test.go`
19	-	- Modify/Create: `internal/app/public_tls_startup_test.go` if helper isolation is needed
20	-
21	-	### Sub-task 1: Well-known HTTP onboarding paths
22	-
23	-	- [ ] Add tests showing direct HTTP serves `/.well-known/gordon/`, `/.well-known/gordon/ca.crt`, and `/.well-known/gordon/ca.mobileconfig`.
24	-	- [ ] Keep existing direct HTTP `/`, `/ca`, `/ca.crt`, `/ca.mobileconfig` compatibility tests green.
25	-	- [ ] Run `rtk go test ./internal/app/... -run 'TestCreateHTTPHandlers_DirectHTTP.Gordon\|TestCreateHTTPHandlers_DirectHTTP.CA'` and confirm new tests fail before implementation.
26	-
27	-	### Sub-task 2: HTTPS host gating
28	-
29	-	- [ ] Add tests showing `httpsHandler` serves `/ca` only when `req.Host == cfg.Server.GordonDomain`.
30	-	- [ ] Add tests showing `/ca` on an arbitrary app host reaches the proxy path instead of onboarding.
31	-	- [ ] Run `rtk go test ./internal/app/... -run 'TestCreateHTTPHandlers_HTTPSOnboarding'` and confirm the arbitrary-host test fails before implementation.
32	-
33	-	### Sub-task 3: Delayed ACME runtime startup
34	-
35	-	- [ ] Add a small app-level test helper/fake proving runtime public TLS `Reconcile` and `StartRenewalLoop` happen after proxy readiness.
36	-	- [ ] Run the targeted test and confirm it fails with current service-construction-time reconciliation.
37	-
38	-	### Sub-task 4: ACME obtain batch limiting
39	-
40	-	- [ ] Add usecase tests proving one reconcile run obtains only a bounded number of missing certificates.
41	-	- [ ] Add a test proving a later reconcile run can continue with the remaining missing certificates.
42	-	- [ ] Run `rtk go test ./internal/usecase/publictls/... -run 'Batch\|Limit\|Reconcile'` and confirm the new batch-limit tests fail before implementation.
43	-
44	-	Phase review checkpoint: Confirm tests express product behavior, not implementation details.
45	-
46	-	---
47	-
48	-	## Phase 2: Implementation
49	-
50	-	Goal: Pass tests with minimal production changes.
51	-
52	-	Files:
53	-	- Modify: `internal/app/run.go`
54	-	- Modify: `internal/app/run_http_handlers_test.go`
55	-
56	-	### Sub-task 1: Onboarding route changes
57	-
58	-	- [ ] Add `/.well-known/gordon/` direct HTTP onboarding routes.
59	-	- [ ] Keep legacy direct HTTP `/`, `/ca`, `/ca.crt`, `/ca.mobileconfig` routes.
60	-	- [ ] Replace unconditional HTTPS `/ca*` registrations with a host-gated handler that only serves onboarding on `server.gordon_domain`.
61	-
62	-	### Sub-task 2: Delayed runtime ACME startup
63	-
64	-	- [ ] Change runtime service creation to initialize public TLS without immediate reconcile/renewal loop side effects.
65	-	- [ ] After registry, HTTP proxy, and TLS listeners are ready, call public TLS `Reconcile` and `StartRenewalLoop`.
66	-	- [ ] Preserve local CLI read-only behavior: `NewKernelQuiet` must still avoid reconcile/renewal loop side effects.
67	-
68	-	### Sub-task 3: ACME obtain batch limiting
69	-
70	-	- [ ] Add a conservative default obtain batch size for runtime reconciliation.
71	-	- [ ] Make `Service.Reconcile` process at most the configured number of missing targets per run.
72	-	- [ ] Log when targets remain pending so operators understand that issuance is intentionally staged.
73	-
74	-	### Sub-task 4: Compatibility check
75	-
76	-	- [ ] Verify ACME challenge handler is still registered before onboarding/proxy handlers.
77	-	- [ ] Verify trusted proxy traffic keeps existing behavior.
78	-	- [ ] Verify TLS-disabled HTTP behavior is unchanged.
79	-
80	-	Phase review checkpoint: Review for startup ordering, goroutine lifecycle, and route conflicts.
81	-
82	-	---
83	-
84	-	## Phase 3: Docs and verification
85	-
86	-	Goal: Document the operational model and validate the branch.
87	-
88	-	Files:
89	-	- Modify: `docs/config/server.md`
90	-	- Modify: `docs/concepts.md` if needed
91	-	- Modify: `docs/cli/tls.md` if status docs need wording
92	-	- Modify: `docs/config/server.md` to document Cloudflare Full/Strict origin HTTPS expectations
93	-
94	-	### Sub-task 1: Documentation
95	-
96	-	- [ ] Document that gray-cloud HTTP-01 requires public/NAT/firewall access to port 80.
97	-	- [ ] Document that Cloudflare-only IP allowlisting on port 80 is incompatible with gray-cloud HTTP-01; use DNS-01 or open 80 for direct validation.
98	-	- [ ] Document `/.well-known/gordon/` CA onboarding paths and mark `/ca` direct HTTP paths as legacy compatibility.
99	-	- [ ] Explain Cloudflare flexible HTTP origin as legacy/explicit, not preferred for end-to-end HTTPS.
100	-	- [ ] Document Cloudflare Full/Strict expected flow: browser -> Cloudflare HTTPS edge cert, Cloudflare -> Gordon HTTPS origin cert. Public ACME certs served by Gordon are the preferred origin certificate because Cloudflare Strict can validate them without a custom origin trust setup.
101	-
102	-	### Sub-task 2: Verification
103	-
104	-	- [ ] Run `rtk gofmt -w` on changed Go files.
105	-	- [ ] Run targeted app tests.
106	-	- [ ] Run `rtk go test ./...`.
107	-	- [ ] Run `rtk golangci-lint run ./...`.
108	-	- [ ] Request Go reviewer feedback and fix blocking issues.
109	-
110	-	Phase review checkpoint: Ensure docs match code and Igor findings.

M internal/app/run.go +3 -5


@@ -622,12 +622,10 @@ func startPublicTLSRuntime(ctx context.Context, svc publicTLSRuntime, log zerowr
622	622		if svc == nil {
623	623		return nil
624	624		}
625		-	if err := svc.Reconcile(ctx); err != nil {
626		-	return err
627		-	}
	625	+	reconcileErr := svc.Reconcile(ctx)
628	626		svc.StartRenewalLoop(ctx, time.Hour)
629	627		log.Info().Msg("public ACME TLS runtime started")
630		-	return nil
	628	+	return reconcileErr
631	629		}
632	630
633	631		// initSecrets creates the domain secret store, env loader, and secret service.

@@ -2552,7 +2550,7 @@ func runServers(ctx context.Context, v viper.Viper, cfg Config, svc services,
2552	2550
2553	2551		func startPublicTLSRuntimeWithWarning(ctx context.Context, svc publicTLSRuntime, log zerowrap.Logger) {
2554	2552		if err := startPublicTLSRuntime(ctx, svc, log); err != nil {
2555		-	log.Warn().Err(err).Msg("failed to start public ACME TLS runtime, continuing")
	2553	+	log.Warn().Err(err).Msg("initial public ACME reconcile failed, continuing with renewal loop")
2556	2554		}
2557	2555		}
2558	2556

 	if svc == nil {
 		return nil
 	}
-	if err := svc.Reconcile(ctx); err != nil {
+	reconcileErr := svc.Reconcile(ctx)
-		return err
-	}
 	svc.StartRenewalLoop(ctx, time.Hour)
 	log.Info().Msg("public ACME TLS runtime started")
-	return nil
+	return reconcileErr
 }
 // initSecrets creates the domain secret store, env loader, and secret service.
 func startPublicTLSRuntimeWithWarning(ctx context.Context, svc publicTLSRuntime, log zerowrap.Logger) {
 	if err := startPublicTLSRuntime(ctx, svc, log); err != nil {
-		log.Warn().Err(err).Msg("failed to start public ACME TLS runtime, continuing")
+		log.Warn().Err(err).Msg("initial public ACME reconcile failed, continuing with renewal loop")
 	}
 }

M internal/app/run_http_handlers_test.go +2 -2


@@ -441,7 +441,7 @@ func TestStartPublicTLSRuntime_NilSvc(t *testing.T) {
441	441		require.NoError(t, err)
442	442		}
443	443
444		-	func TestStartPublicTLSRuntime_DoesNotStartRenewalLoopWhenReconcileFails(t *testing.T) {
	444	+	func TestStartPublicTLSRuntime_StartsRenewalLoopWhenReconcileFails(t *testing.T) {
445	445		t.Parallel()
446	446
447	447		runtime := &recordingPublicTLSRuntime{reconcileErr: errors.New("boom")}

@@ -449,7 +449,7 @@ func TestStartPublicTLSRuntime_DoesNotStartRenewalLoopWhenReconcileFails(t *test
449	449
450	450		require.Error(t, err)
451	451		assert.Equal(t, 1, runtime.reconcileCalls)
452		-	assert.Equal(t, 0, runtime.loopCalls)
	452	+	assert.Equal(t, 1, runtime.loopCalls)
453	453		}
454	454
455	455		func TestCreateHTTPHandlers_TLSConfiguredWithoutCA_FailsStartup(t *testing.T) {


@@ -441,7 +441,7 @@ func TestStartPublicTLSRuntime_NilSvc(t *testing.T) {
441		require.NoError(t, err)	441		require.NoError(t, err)
442		}	442		}
443			443
444	-	func TestStartPublicTLSRuntime_DoesNotStartRenewalLoopWhenReconcileFails(t *testing.T) {	444	+	func TestStartPublicTLSRuntime_StartsRenewalLoopWhenReconcileFails(t *testing.T) {
445		t.Parallel()	445		t.Parallel()
446			446
447		runtime := &recordingPublicTLSRuntime{reconcileErr: errors.New("boom")}	447		runtime := &recordingPublicTLSRuntime{reconcileErr: errors.New("boom")}

@@ -449,7 +449,7 @@ func TestStartPublicTLSRuntime_DoesNotStartRenewalLoopWhenReconcileFails(t *test
449			449
450		require.Error(t, err)	450		require.Error(t, err)
451		assert.Equal(t, 1, runtime.reconcileCalls)	451		assert.Equal(t, 1, runtime.reconcileCalls)
452	-	assert.Equal(t, 0, runtime.loopCalls)	452	+	assert.Equal(t, 1, runtime.loopCalls)
453		}	453		}
454			454
455		func TestCreateHTTPHandlers_TLSConfiguredWithoutCA_FailsStartup(t *testing.T) {	455		func TestCreateHTTPHandlers_TLSConfiguredWithoutCA_FailsStartup(t *testing.T) {

M internal/usecase/publictls/service.go +45 -18


@@ -37,13 +37,14 @@ const defaultObtainBatchSize = 1
37	37
38	38		// Service manages public TLS certificates via ACME.
39	39		type Service struct {
40		-	mu sync.RWMutex
41		-	cfg Config
42		-	deps ServiceDeps
43		-	log zerowrap.Logger
44		-	certs map[string]*out.StoredCertificate // indexed by cert ID
45		-	lastErr map[string]string // indexed by cert ID
46		-	routeErr map[string]string // indexed by host
	40	+	mu sync.RWMutex
	41	+	cfg Config
	42	+	deps ServiceDeps
	43	+	log zerowrap.Logger
	44	+	certs map[string]*out.StoredCertificate // indexed by cert ID
	45	+	lastErr map[string]string // indexed by cert ID
	46	+	routeErr map[string]string // indexed by host
	47	+	obtainCursor int // next missing target index for batched obtains
47	48
48	49		// requiredHosts is the set of hosts that must be covered by ACME certs.
49	50		requiredHosts map[string]struct{}

@@ -91,6 +92,7 @@ func (s *Service) Load(ctx context.Context) error {
91	92		s.certs = make(map[string]*out.StoredCertificate, len(stored))
92	93		s.lastErr = make(map[string]string)
93	94		s.requiredHosts = make(map[string]struct{})
	95	+	s.obtainCursor = 0
94	96
95	97		for i := range stored {
96	98		cert := &stored[i]

@@ -189,17 +191,7 @@ func (s *Service) Reconcile(ctx context.Context) error {
189	191		batchSize = defaultObtainBatchSize
190	192		}
191	193
192		-	var missing []CertificateTarget
193		-	pendingCount := 0
194		-	for _, target := range targets {
195		-	if s.certificateExistsLocked(target) {
196		-	continue
197		-	}
198		-	pendingCount++
199		-	if len(missing) < batchSize {
200		-	missing = append(missing, target)
201		-	}
202		-	}
	194	+	missing, pendingCount := s.selectMissingTargetsLocked(targets, batchSize)
203	195		s.mu.Unlock()
204	196
205	197		// Process missing targets without holding s.mu or the store lock so

@@ -216,6 +208,41 @@ func (s *Service) Reconcile(ctx context.Context) error {
216	208		return nil
217	209		}
218	210
	211	+	// selectMissingTargetsLocked returns up to batchSize missing certificate targets.
	212	+	// It rotates the starting point across reconcile runs so one repeatedly failing
	213	+	// target cannot indefinitely block later targets when batching is enabled.
	214	+	// Must be called with s.mu held.
	215	+	func (s *Service) selectMissingTargetsLocked(targets []CertificateTarget, batchSize int) ([]CertificateTarget, int) {
	216	+	if batchSize <= 0 {
	217	+	batchSize = defaultObtainBatchSize
	218	+	}
	219	+
	220	+	allMissing := make([]CertificateTarget, 0, len(targets))
	221	+	for _, target := range targets {
	222	+	if !s.certificateExistsLocked(target) {
	223	+	allMissing = append(allMissing, target)
	224	+	}
	225	+	}
	226	+
	227	+	pendingCount := len(allMissing)
	228	+	if pendingCount == 0 {
	229	+	s.obtainCursor = 0
	230	+	return nil, 0
	231	+	}
	232	+	if batchSize >= pendingCount {
	233	+	s.obtainCursor = 0
	234	+	return allMissing, pendingCount
	235	+	}
	236	+
	237	+	start := s.obtainCursor % pendingCount
	238	+	missing := make([]CertificateTarget, 0, batchSize)
	239	+	for i := range batchSize {
	240	+	missing = append(missing, allMissing[(start+i)%pendingCount])
	241	+	}
	242	+	s.obtainCursor = (start + batchSize) % pendingCount
	243	+	return missing, pendingCount
	244	+	}
	245	+
219	246		func populateStoredCertificate(cert *out.StoredCertificate) error {
220	247		if cert == nil {
221	248		return fmt.Errorf("stored certificate is nil")

 // Service manages public TLS certificates via ACME.
 type Service struct {
-	mu       sync.RWMutex
+	mu           sync.RWMutex
-	cfg      Config
+	cfg          Config
-	deps     ServiceDeps
+	deps         ServiceDeps
-	log      zerowrap.Logger
+	log          zerowrap.Logger
-	certs    map[string]*out.StoredCertificate // indexed by cert ID
+	certs        map[string]*out.StoredCertificate // indexed by cert ID
-	lastErr  map[string]string                 // indexed by cert ID
+	lastErr      map[string]string                 // indexed by cert ID
-	routeErr map[string]string                 // indexed by host
+	routeErr     map[string]string                 // indexed by host
+	obtainCursor int                               // next missing target index for batched obtains
 	// requiredHosts is the set of hosts that must be covered by ACME certs.
 	requiredHosts map[string]struct{}
 	s.certs = make(map[string]*out.StoredCertificate, len(stored))
 	s.lastErr = make(map[string]string)
 	s.requiredHosts = make(map[string]struct{})
+	s.obtainCursor = 0
 	for i := range stored {
 		cert := &stored[i]
 		batchSize = defaultObtainBatchSize
 	}
-	var missing []CertificateTarget
+	missing, pendingCount := s.selectMissingTargetsLocked(targets, batchSize)
-	pendingCount := 0
-	for _, target := range targets {
-		if s.certificateExistsLocked(target) {
-			continue
-		}
-		pendingCount++
-		if len(missing) < batchSize {
-			missing = append(missing, target)
-		}
-	}
 	s.mu.Unlock()
 	// Process missing targets without holding s.mu or the store lock so
 	return nil
 }
+// selectMissingTargetsLocked returns up to batchSize missing certificate targets.
+// It rotates the starting point across reconcile runs so one repeatedly failing
+// target cannot indefinitely block later targets when batching is enabled.
+// Must be called with s.mu held.
+func (s *Service) selectMissingTargetsLocked(targets []CertificateTarget, batchSize int) ([]CertificateTarget, int) {
+	if batchSize <= 0 {
+		batchSize = defaultObtainBatchSize
+	}
+	allMissing := make([]CertificateTarget, 0, len(targets))
+	for _, target := range targets {
+		if !s.certificateExistsLocked(target) {
+			allMissing = append(allMissing, target)
+		}
+	}
+	pendingCount := len(allMissing)
+	if pendingCount == 0 {
+		s.obtainCursor = 0
+		return nil, 0
+	}
+	if batchSize >= pendingCount {
+		s.obtainCursor = 0
+		return allMissing, pendingCount
+	}
+	start := s.obtainCursor % pendingCount
+	missing := make([]CertificateTarget, 0, batchSize)
+	for i := range batchSize {
+		missing = append(missing, allMissing[(start+i)%pendingCount])
+	}
+	s.obtainCursor = (start + batchSize) % pendingCount
+	return missing, pendingCount
+}
 func populateStoredCertificate(cert *out.StoredCertificate) error {
 	if cert == nil {
 		return fmt.Errorf("stored certificate is nil")

M internal/usecase/publictls/service_test.go +54 -0


@@ -300,6 +300,60 @@ func TestServiceReconcileLimitsMissingObtainsPerRun(t *testing.T) {
300	300		assert.Len(t, storeState.All(), 3)
301	301		}
302	302
	303	+	func TestServiceReconcileRotatesBatchAfterFailedObtain(t *testing.T) {
	304	+	ctx := context.Background()
	305	+
	306	+	routes := &fakeRoutes{
	307	+	routes: []domain.Route{
	308	+	{Domain: "b.example.com"},
	309	+	{Domain: "a.example.com"},
	310	+	},
	311	+	}
	312	+	issuer, recorder := newMockPublicCertificateIssuer(t, func(_ context.Context, order out.CertificateOrder) (*out.StoredCertificate, error) {
	313	+	if order.ID == "http01-a.example.com" {
	314	+	return nil, fmt.Errorf("acme validation failed")
	315	+	}
	316	+	return defaultStoredCert(order)
	317	+	}, nil)
	318	+	store, storeState := newMockCertificateStore(t)
	319	+
	320	+	cfg := Config{
	321	+	Enabled: true,
	322	+	Email: "admin@example.com",
	323	+	Challenge: "http-01",
	324	+	HTTPPort: 8088,
	325	+	TLSPort: 8443,
	326	+	ObtainBatchSize: 1,
	327	+	}
	328	+
	329	+	svc := NewService(cfg, ServiceDeps{
	330	+	Config: cfg,
	331	+	Routes: routes,
	332	+	Issuer: issuer,
	333	+	Store: store,
	334	+	Challenges: NewHTTP01Challenges(),
	335	+	Effective: EffectiveChallenge{
	336	+	ConfiguredMode: domain.ACMEChallengeHTTP01,
	337	+	Mode: domain.ACMEChallengeHTTP01,
	338	+	TokenSource: domain.ACMETokenSourceNone,
	339	+	Reason: "http-01 challenge selected",
	340	+	},
	341	+	})
	342	+
	343	+	require.NoError(t, svc.Load(ctx))
	344	+	require.NoError(t, svc.Reconcile(ctx))
	345	+	require.NoError(t, svc.Reconcile(ctx))
	346	+
	347	+	orders := recorder.Orders()
	348	+	require.Len(t, orders, 2)
	349	+	assert.Equal(t, "http01-a.example.com", orders[0].ID)
	350	+	assert.Equal(t, "http01-b.example.com", orders[1].ID, "second reconcile should rotate past the failed first target")
	351	+
	352	+	stored := storeState.All()
	353	+	require.Len(t, stored, 1)
	354	+	assert.Equal(t, "http01-b.example.com", stored[0].ID)
	355	+	}
	356	+
303	357		func TestServiceReconcileObtainsMissingHTTP01Cert(t *testing.T) {
304	358		ctx := context.Background()
305	359


@@ -300,6 +300,60 @@ func TestServiceReconcileLimitsMissingObtainsPerRun(t *testing.T) {
300	assert.Len(t, storeState.All(), 3)	300		assert.Len(t, storeState.All(), 3)
301	}	301		}
302		302
		303	+	func TestServiceReconcileRotatesBatchAfterFailedObtain(t *testing.T) {
		304	+	ctx := context.Background()
		305	+
		306	+	routes := &fakeRoutes{
		307	+	routes: []domain.Route{
		308	+	{Domain: "b.example.com"},
		309	+	{Domain: "a.example.com"},
		310	+	},
		311	+	}
		312	+	issuer, recorder := newMockPublicCertificateIssuer(t, func(_ context.Context, order out.CertificateOrder) (*out.StoredCertificate, error) {
		313	+	if order.ID == "http01-a.example.com" {
		314	+	return nil, fmt.Errorf("acme validation failed")
		315	+	}
		316	+	return defaultStoredCert(order)
		317	+	}, nil)
		318	+	store, storeState := newMockCertificateStore(t)
		319	+
		320	+	cfg := Config{
		321	+	Enabled: true,
		322	+	Email: "admin@example.com",
		323	+	Challenge: "http-01",
		324	+	HTTPPort: 8088,
		325	+	TLSPort: 8443,
		326	+	ObtainBatchSize: 1,
		327	+	}
		328	+
		329	+	svc := NewService(cfg, ServiceDeps{
		330	+	Config: cfg,
		331	+	Routes: routes,
		332	+	Issuer: issuer,
		333	+	Store: store,
		334	+	Challenges: NewHTTP01Challenges(),
		335	+	Effective: EffectiveChallenge{
		336	+	ConfiguredMode: domain.ACMEChallengeHTTP01,
		337	+	Mode: domain.ACMEChallengeHTTP01,
		338	+	TokenSource: domain.ACMETokenSourceNone,
		339	+	Reason: "http-01 challenge selected",
		340	+	},
		341	+	})
		342	+
		343	+	require.NoError(t, svc.Load(ctx))
		344	+	require.NoError(t, svc.Reconcile(ctx))
		345	+	require.NoError(t, svc.Reconcile(ctx))
		346	+
		347	+	orders := recorder.Orders()
		348	+	require.Len(t, orders, 2)
		349	+	assert.Equal(t, "http01-a.example.com", orders[0].ID)
		350	+	assert.Equal(t, "http01-b.example.com", orders[1].ID, "second reconcile should rotate past the failed first target")
		351	+
		352	+	stored := storeState.All()
		353	+	require.Len(t, stored, 1)
		354	+	assert.Equal(t, "http01-b.example.com", stored[0].ID)
		355	+	}
		356	+
303	func TestServiceReconcileObtainsMissingHTTP01Cert(t *testing.T) {	357		func TestServiceReconcileObtainsMissingHTTP01Cert(t *testing.T) {
304	ctx := context.Background()	358		ctx := context.Background()
305		359