Skip to content
21 changes: 18 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -1044,7 +1044,22 @@ to verify DNS actually routes here — unreachable domains are quarantined
(5m, then 15m → 1h → 4h → 24h backoff) without burning an order. Failing
domains quarantine alone; the rest of a batch is retried once. Renewals reuse
the exact same identifier set (exempt from most rate limits) and pass ARI
`replaces` where supported.
`replaces` where supported. Every renewal re-probes its dynamic members
first, so a tenant whose DNS moved away after issuance is quarantined out of
the order instead of failing it — and when an order fails without naming a
domain, the members are probed to find the culprit before anyone is blamed.

**A bad poll cannot destroy certificates.** Two guards protect the estate
from the source itself. A poll that removes more than 30% of the applied
domain set has its removals *held*: the previous set stays allowed (additions
still apply), and only three consecutive shrunken polls confirm and apply the
removal — a single (or transient) empty or truncated response from the app
evicts nothing. Held removals are visible in `kamal-proxy domains list`
(Removal held column) and logged at Warn on every held poll. Independently, a
certificate whose domains were all evicted is never deleted before its own
expiry: it stops renewing but keeps serving, and can be reused immediately if
the domains return before it expires — normal replacement and renewal rules
still apply.

`--tls-domains-batch-size` (max 25) opts into stable SAN batching for dynamic
domains: batches fill append-only, and membership only changes at renewal
Expand All @@ -1059,8 +1074,8 @@ the app is down.
**Inspecting:**

```bash
kamal-proxy domains list # every dynamic domain, cert + quarantine status
kamal-proxy domains stats # counters: domains, certified, queued, quarantined
kamal-proxy domains list # every dynamic domain, cert + quarantine + held status
kamal-proxy domains stats # counters: domains, certified, queued, quarantined, held
kamal-proxy domains refresh # trigger an immediate re-poll of all sources
```

Expand Down
17 changes: 15 additions & 2 deletions internal/cmd/domains.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,19 @@ func newDomainsListCommand() *domainsListCommand {
func (c *domainsListCommand) run(cmd *cobra.Command, args []string) error {
return fetchDomainsStatus(func(response server.DomainsStatusResponse) {
table := NewTable()
table.AddRow([]string{"Service", "Domain", "Certified", "Quarantined until"})
table.AddRow([]string{"Service", "Domain", "Certified", "Quarantined until", "Removal held"})

for _, name := range slices.Sorted(maps.Keys(response.Services)) {
service := response.Services[name]
domains := slices.SortedFunc(slices.Values(service.Domains), func(a, b server.DomainStatus) int {
return strings.Compare(a.Domain, b.Domain)
})

heldRemovals := make(map[string]struct{}, len(service.HeldRemovals))
for _, domain := range service.HeldRemovals {
heldRemovals[domain] = struct{}{}
}

for _, domain := range domains {
certified := "no"
if domain.Certified {
Expand All @@ -83,7 +88,12 @@ func (c *domainsListCommand) run(cmd *cobra.Command, args []string) error {
quarantined = entry.Until.Format("2006-01-02 15:04:05")
}

table.AddRow([]string{name, domain.Domain, certified, quarantined})
held := ""
if _, ok := heldRemovals[domain.Domain]; ok {
held = "yes"
}

table.AddRow([]string{name, domain.Domain, certified, quarantined, held})
}
}

Expand Down Expand Up @@ -111,8 +121,10 @@ func (c *domainsStatsCommand) run(cmd *cobra.Command, args []string) error {
return fetchDomainsStatus(func(response server.DomainsStatusResponse) {
domains := 0
certified := 0
held := 0
for _, service := range response.Services {
domains += len(service.Domains)
held += len(service.HeldRemovals)
for _, domain := range service.Domains {
if domain.Certified {
certified++
Expand All @@ -125,6 +137,7 @@ func (c *domainsStatsCommand) run(cmd *cobra.Command, args []string) error {
fmt.Printf("Certified: %d\n", certified)
fmt.Printf("Queued for issuance: %d\n", response.QueueLength)
fmt.Printf("Quarantined: %d\n", len(response.Quarantine))
fmt.Printf("Held removals: %d\n", held)
fmt.Printf("Managed certificates: %d\n", response.Certificates)
})
}
Expand Down
4 changes: 4 additions & 0 deletions internal/server/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ type DomainsServiceStatus struct {
Source string `json:"source"`
Domains []DomainStatus `json:"domains"`
FetchedAt time.Time `json:"fetched_at"`

// HeldRemovals lists domains the source stopped reporting but whose
// removal is held by the shrink guard, pending confirmation.
HeldRemovals []string `json:"held_removals,omitempty"`
}

type QuarantineStatus struct {
Expand Down
87 changes: 87 additions & 0 deletions internal/server/domain_failure.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
package server

import (
"strings"
"sync"
)

// Attribution of failed ACME orders to the domains that caused them, shared by
// the dynamic issuer and the background renewer.

// maxConcurrentProbes bounds parallel pre-flight probes. Each probe can take
// up to preflightTimeout, so a serial sweep over a large batch would block an
// issuance slot (or a waiting handshake) for minutes; concurrency keeps the
// worst case to a few probe timeouts.
const maxConcurrentProbes = 16

// identifyFailedDomains names the domains responsible for a failed order:
// lego's per-domain error lines when present, a pre-flight probe of each
// member otherwise, and the whole set when neither can tell — an
// unattributable failure holds the entire batch on the quarantine ladder so
// retries back off instead of looping against ACME rate limits.
func identifyFailedDomains(err error, domains []string, preflight func(string) error) []string {
if failed := failedDomainsFromError(err, domains); len(failed) > 0 {
return failed
}

if failed, _ := probeDomains(domains, preflight); len(failed) > 0 {
return failed
}

return domains
}

// probeDomains runs the pre-flight probe over a set of domains with bounded
// concurrency and returns the ones that failed, in input order, with each
// failure's error. Wildcards are skipped — there is no name to answer on one.
// A nil probe reports nothing.
func probeDomains(domains []string, preflight func(string) error) ([]string, map[string]error) {
if preflight == nil {
return nil, nil
}

errs := make([]error, len(domains))
sem := make(chan struct{}, maxConcurrentProbes)
var wg sync.WaitGroup
for idx, domain := range domains {
if strings.HasPrefix(domain, "*.") {
continue
}
wg.Add(1)
sem <- struct{}{}
go func(idx int, domain string) {
defer wg.Done()
defer func() { <-sem }()
errs[idx] = preflight(domain)
}(idx, domain)
}
wg.Wait()

failed := []string{}
failures := map[string]error{}
for idx, domain := range domains {
if errs[idx] != nil {
failed = append(failed, domain)
failures[domain] = errs[idx]
}
}
return failed, failures
}

// failedDomainsFromError matches lego's per-domain error lines
// ("<domain>: <cause>") against the attempted domains.
func failedDomainsFromError(err error, domains []string) []string {
lines := strings.Split(err.Error(), "\n")

failed := []string{}
for _, domain := range domains {
prefix := domain + ": "
for _, line := range lines {
if strings.HasPrefix(strings.TrimSpace(line), prefix) {
failed = append(failed, domain)
break
}
}
}
return failed
}
70 changes: 70 additions & 0 deletions internal/server/domain_failure_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
package server

import (
"errors"
"fmt"
"testing"

"github.com/stretchr/testify/assert"
)

func TestIdentifyFailedDomains(t *testing.T) {
domains := []string{"a.example.com", "b.example.com"}
failB := func(domain string) error {
if domain == "b.example.com" {
return errors.New("does not route here")
}
return nil
}
failAll := func(domain string) error { return errors.New("does not route here") }
passAll := func(domain string) error { return nil }

tests := []struct {
name string
err error
domains []string
preflight func(string) error
expected []string
}{
{
name: "per-domain error lines take precedence over probing",
err: fmt.Errorf("error: one or more domains had a problem:\na.example.com: acme: dns problem"),
domains: domains,
preflight: failB,
expected: []string{"a.example.com"},
},
{
name: "probe names the culprit when the error does not",
err: errors.New("acme: internal error"),
domains: domains,
preflight: failB,
expected: []string{"b.example.com"},
},
{
name: "everything passes probing: the whole batch is held",
err: errors.New("acme: internal error"),
domains: domains,
preflight: passAll,
expected: domains,
},
{
name: "no probe available: the whole batch is held",
err: errors.New("acme: internal error"),
domains: domains,
expected: domains,
},
{
name: "wildcard members are never probed",
err: errors.New("acme: internal error"),
domains: []string{"*.example.com", "a.example.com"},
preflight: failAll,
expected: []string{"a.example.com"},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.expected, identifyFailedDomains(tt.err, tt.domains, tt.preflight))
})
}
}
24 changes: 1 addition & 23 deletions internal/server/domain_issuer.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"log/slog"
"slices"
"strings"
"sync"
"time"

Expand Down Expand Up @@ -376,10 +375,7 @@ func (i *domainIssuer) issue(batch []*issueRequest) {
// quarantined too, so a failing batch cannot loop against ACME rate limits;
// the poller re-requests them after the backoff expires.
func (i *domainIssuer) handleObtainFailure(batch []*issueRequest, domains []string, requests map[string]*issueRequest, err error) {
failed := failedDomainsFromError(err, domains)
if len(failed) == 0 {
failed = domains
}
failed := identifyFailedDomains(err, domains, i.config.Preflight)
Comment thread
mhenrixon marked this conversation as resolved.

slog.Warn("Certificate order failed", "domains", domains, "failed", failed, "error", err)

Expand Down Expand Up @@ -417,24 +413,6 @@ func (i *domainIssuer) handleObtainFailure(batch []*issueRequest, domains []stri
i.notify()
}

// failedDomainsFromError matches lego's per-domain error lines
// ("<domain>: <cause>") against the attempted domains.
func failedDomainsFromError(err error, domains []string) []string {
lines := strings.Split(err.Error(), "\n")

failed := []string{}
for _, domain := range domains {
prefix := domain + ": "
for _, line := range lines {
if strings.HasPrefix(strings.TrimSpace(line), prefix) {
failed = append(failed, domain)
break
}
}
}
return failed
}

func (i *domainIssuer) notify() {
select {
case i.wake <- struct{}{}:
Expand Down
36 changes: 36 additions & 0 deletions internal/server/domain_issuer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package server
import (
"errors"
"fmt"
"sync/atomic"
"testing"
"time"

Expand Down Expand Up @@ -158,6 +159,41 @@ func TestDomainIssuer_Issue_UnidentifiableFailureQuarantinesWholeBatch(t *testin
require.Len(t, obtainer.Calls(), 1)
}

func TestDomainIssuer_Issue_ProbesForCulpritsOnUnattributableFailure(t *testing.T) {
var ordered atomic.Bool
obtainer := &fakeObtainer{respond: func(request certificate.ObtainRequest) (*certificate.Resource, error) {
ordered.Store(true)
return nil, errors.New("acme: internal error")
}}

issuer, manager, quarantine := testIssuer(t, obtainer, domainIssuerConfig{
BatchSize: func(service string) int { return 2 },
// Both domains route here when the order is placed; gone.example.com
// stops routing by the time the failure is investigated.
Preflight: func(domain string) error {
if ordered.Load() && domain == "gone.example.com" {
return errors.New("no longer routes here")
}
return nil
},
})
manager.SetDynamicDomains("service1", []string{"good.example.com", "gone.example.com"})

issuer.Request("good.example.com", "service1")
issuer.Request("gone.example.com", "service1")
issuer.issue(issuer.nextBatch())

// The probe identified the culprit: quarantined, while the survivor is
// re-enqueued for its retry instead of being quarantined with it.
assert.True(t, quarantine.IsQuarantined("gone.example.com"))
assert.False(t, quarantine.IsQuarantined("good.example.com"))

batch := issuer.nextBatch()
require.Len(t, batch, 1)
assert.Equal(t, "good.example.com", batch[0].domain)
assert.True(t, batch[0].retried)
}

func TestDomainIssuer_Issue_RetriedSurvivorsAreNotReenqueuedAgain(t *testing.T) {
obtainer := &fakeObtainer{respond: func(request certificate.ObtainRequest) (*certificate.Resource, error) {
return nil, fmt.Errorf("error: one or more domains had a problem:\n%s: acme: failed", request.Domains[0])
Expand Down
Loading
Loading