From 9724a2ce50afc932079a5997db402e41210391f5 Mon Sep 17 00:00:00 2001 From: Bryan Frimin Date: Wed, 22 Jul 2026 09:59:00 +0200 Subject: [PATCH] Harden cert provisioning failure and write-back Rate limits no longer inflate ssl_retry_count into an instant FAILED path. Centralize outcomes in decideProvisioningOutcome, keep ACME order state on transient and rate-limit errors, bound each Process tick with a timeout, and block on FOR UPDATE when persisting a freshly issued certificate. Signed-off-by: Ludovic Vielle Signed-off-by: Bryan Frimin --- .../_components/CompliancePageDomainCard.tsx | 10 +- .../CompliancePageDomainDialog.tsx | 10 +- packages/helpers/src/customDomain.ts | 31 + packages/helpers/src/index.ts | 1 + pkg/certmanager/acme.go | 230 ++++-- pkg/certmanager/acme_errors.go | 128 +++ pkg/certmanager/acme_test.go | 76 ++ pkg/certmanager/metrics.go | 123 +++ pkg/certmanager/provision_worker.go | 774 ++++++++++++------ pkg/certmanager/provision_worker_test.go | 165 ++++ pkg/certmanager/provisioning_error.go | 69 ++ pkg/coredata/certificate.go | 85 +- pkg/probod/probod.go | 1 + 13 files changed, 1375 insertions(+), 328 deletions(-) create mode 100644 pkg/certmanager/acme_errors.go create mode 100644 pkg/certmanager/acme_test.go create mode 100644 pkg/certmanager/metrics.go create mode 100644 pkg/certmanager/provision_worker_test.go create mode 100644 pkg/certmanager/provisioning_error.go diff --git a/apps/console/src/pages/organizations/compliance-page/domain/_components/CompliancePageDomainCard.tsx b/apps/console/src/pages/organizations/compliance-page/domain/_components/CompliancePageDomainCard.tsx index 95f76df5b..966b36903 100644 --- a/apps/console/src/pages/organizations/compliance-page/domain/_components/CompliancePageDomainCard.tsx +++ b/apps/console/src/pages/organizations/compliance-page/domain/_components/CompliancePageDomainCard.tsx @@ -19,6 +19,7 @@ // SOFTWARE. import { + getCertificateProvisioningErrorMessage, getCustomDomainStatusBadgeLabel, getCustomDomainStatusBadgeVariant, } from "@probo/helpers"; @@ -55,7 +56,10 @@ export function CompliancePageDomainCard(props: { const domain = useFragment(fragment, fKey); const sslStatus = domain.certificate?.status ?? "PENDING"; - const provisioningError = domain.certificate?.provisioningError; + const provisioningErrorMessage = getCertificateProvisioningErrorMessage( + domain.certificate?.provisioningError, + __, + ); return ( @@ -73,8 +77,8 @@ export function CompliancePageDomainCard(props: {

{sslStatus === "ACTIVE" ? __("Verified and serving traffic") - : provisioningError - ? provisioningError + : provisioningErrorMessage + ? provisioningErrorMessage : __("Pending DNS verification")}

diff --git a/apps/console/src/pages/organizations/compliance-page/domain/_components/CompliancePageDomainDialog.tsx b/apps/console/src/pages/organizations/compliance-page/domain/_components/CompliancePageDomainDialog.tsx index ab262ac00..659b9f91f 100644 --- a/apps/console/src/pages/organizations/compliance-page/domain/_components/CompliancePageDomainDialog.tsx +++ b/apps/console/src/pages/organizations/compliance-page/domain/_components/CompliancePageDomainDialog.tsx @@ -19,6 +19,7 @@ // SOFTWARE. import { + getCertificateProvisioningErrorMessage, getCustomDomainStatusBadgeLabel, getCustomDomainStatusBadgeVariant, } from "@probo/helpers"; @@ -76,7 +77,10 @@ export function CompliancePageDomainDialog(props: CompliancePageDomainDialogProp const domain = useFragment(fragment, fKey); const sslStatus = domain.certificate?.status ?? "PENDING"; const expiresAt = domain.certificate?.expiresAt; - const provisioningError = domain.certificate?.provisioningError; + const provisioningErrorMessage = getCertificateProvisioningErrorMessage( + domain.certificate?.provisioningError, + __, + ); return ( - {provisioningError && ( + {provisioningErrorMessage && (

{__("Provisioning error")}

-

{provisioningError}

+

{provisioningErrorMessage}

)} diff --git a/packages/helpers/src/customDomain.ts b/packages/helpers/src/customDomain.ts index 5641c39fe..6f8b01639 100644 --- a/packages/helpers/src/customDomain.ts +++ b/packages/helpers/src/customDomain.ts @@ -63,3 +63,34 @@ export const getCustomDomainStatusBadgeLabel = ( } return __("Unknown"); }; + +const provisioningErrorMessages: Record = { + DNS_CNAME: "DNS is not configured correctly yet. Check the CNAME record.", + DNS_CAA: "DNS CAA records do not allow our certificate authority.", + ACME_RATE_LIMITED: + "Certificate authority is temporarily rate-limiting. We will retry automatically.", + ACME_INVALID_ORDER: + "Domain ownership could not be verified. We will retry automatically.", + ACME_TEMPORARY: + "Certificate provisioning hit a temporary error. We will retry automatically.", + ACME_FAILED: + "Certificate provisioning failed. Contact support if this persists.", +}; + +export const getCertificateProvisioningErrorMessage = ( + provisioningError: string | null | undefined, + __: (key: string) => string, +) => { + if (!provisioningError) { + return null; + } + + const knownMessage = provisioningErrorMessages[provisioningError]; + if (knownMessage) { + return __(knownMessage); + } + + return __( + "Certificate provisioning failed. Contact support if this persists.", + ); +}; diff --git a/packages/helpers/src/index.ts b/packages/helpers/src/index.ts index fbee85da2..8b63499af 100644 --- a/packages/helpers/src/index.ts +++ b/packages/helpers/src/index.ts @@ -21,6 +21,7 @@ export { objectKeys, objectEntries, cleanFormData } from "./object"; export { sprintf, faviconUrl, slugify } from "./string"; export { + getCertificateProvisioningErrorMessage, getCustomDomainStatusBadgeLabel, getCustomDomainStatusBadgeVariant, } from "./customDomain"; diff --git a/pkg/certmanager/acme.go b/pkg/certmanager/acme.go index 4e4b4193b..1b9fa6c43 100644 --- a/pkg/certmanager/acme.go +++ b/pkg/certmanager/acme.go @@ -30,8 +30,10 @@ import ( "errors" "fmt" "strings" + "sync" "time" + "github.com/prometheus/client_golang/prometheus" "go.gearno.de/kit/httpclient" "go.gearno.de/kit/log" "go.probo.inc/probo/pkg/crypto/keys" @@ -49,10 +51,13 @@ type ( } ACMEService struct { - client *acme.Client - email string - keyType keys.Type - logger *log.Logger + client *acme.Client + email string + keyType keys.Type + logger *log.Logger + metrics *metrics + cooldownMu sync.RWMutex + cooldownUntil time.Time } HTTPChallenge struct { @@ -62,8 +67,22 @@ type ( URL string OrderURL string } + + OrderPollStatus string ) +const ( + OrderPollStatusNotReady OrderPollStatus = "not_ready" + OrderPollStatusReady OrderPollStatus = "ready" + OrderPollStatusValid OrderPollStatus = "valid" + OrderPollStatusInvalid OrderPollStatus = "invalid" +) + +type OrderPollResult struct { + Status OrderPollStatus + Order *acme.Order +} + func NewACMEService( email string, keyType keys.Type, @@ -71,6 +90,7 @@ func NewACMEService( accountKey crypto.Signer, rootCAs *x509.CertPool, logger *log.Logger, + registerer prometheus.Registerer, ) (*ACMEService, error) { if accountKey == nil { var err error @@ -110,6 +130,7 @@ func NewACMEService( email: email, keyType: keyType, logger: logger.Named("acme"), + metrics: newMetrics(registerer), } ctx := context.Background() @@ -120,6 +141,35 @@ func NewACMEService( return service, nil } +func (s *ACMEService) InCooldown() bool { + s.cooldownMu.RLock() + defer s.cooldownMu.RUnlock() + + active := time.Now().Before(s.cooldownUntil) + + s.metrics.setCooldown(active) + + return active +} + +func (s *ACMEService) CooldownUntil() time.Time { + s.cooldownMu.RLock() + defer s.cooldownMu.RUnlock() + + return s.cooldownUntil +} + +func (s *ACMEService) enterCooldown(until time.Time) { + s.cooldownMu.Lock() + defer s.cooldownMu.Unlock() + + if until.After(s.cooldownUntil) { + s.cooldownUntil = until + } + + s.metrics.setCooldown(time.Now().Before(s.cooldownUntil)) +} + func (s *ACMEService) registerAccount(ctx context.Context) error { account := &acme.Account{Contact: []string{"mailto:" + s.email}} @@ -132,10 +182,14 @@ func (s *ACMEService) registerAccount(ctx context.Context) error { return nil } -func (s *ACMEService) GetHTTPChallenge(ctx context.Context, domain string) (*HTTPChallenge, error) { +// StartHTTPChallenge creates an ACME order, accepts the HTTP-01 challenge, and +// returns the persisted challenge metadata. It never waits for order completion. +func (s *ACMEService) StartHTTPChallenge(ctx context.Context, domain string) (*HTTPChallenge, error) { + started := time.Now() + order, err := s.client.AuthorizeOrder(ctx, acme.DomainIDs(domain)) if err != nil { - return nil, fmt.Errorf("cannot create order: %w", err) + return nil, s.handleError(provisionPhaseCreateOrder, started, "cannot create order", err) } var challenge *acme.Challenge @@ -143,7 +197,7 @@ func (s *ACMEService) GetHTTPChallenge(ctx context.Context, domain string) (*HTT for _, auth := range order.AuthzURLs { authz, err := s.client.GetAuthorization(ctx, auth) if err != nil { - return nil, fmt.Errorf("cannot get authorization: %w", err) + return nil, s.handleError(provisionPhaseCreateOrder, started, "cannot get authorization", err) } for _, ch := range authz.Challenges { @@ -159,14 +213,27 @@ func (s *ACMEService) GetHTTPChallenge(ctx context.Context, domain string) (*HTT } if challenge == nil { + s.metrics.observeStep(provisionPhaseCreateOrder, provisionResultError, started) return nil, fmt.Errorf("no HTTP-01 challenge found") } keyAuth, err := s.client.HTTP01ChallengeResponse(challenge.Token) if err != nil { + s.metrics.observeStep(provisionPhaseCreateOrder, provisionResultError, started) return nil, fmt.Errorf("cannot get challenge response: %w", err) } + challenge1 := &acme.Challenge{ + URI: challenge.URI, + Token: challenge.Token, + } + + if _, err := s.client.Accept(ctx, challenge1); err != nil && !isChallengeAlreadyValid(err) { + return nil, s.handleError(provisionPhaseCreateOrder, started, "cannot accept challenge", err) + } + + s.metrics.observeStep(provisionPhaseCreateOrder, provisionResultOK, started) + return &HTTPChallenge{ Domain: domain, Token: challenge.Token, @@ -176,41 +243,85 @@ func (s *ACMEService) GetHTTPChallenge(ctx context.Context, domain string) (*HTT }, nil } -func (s *ACMEService) CompleteHTTPChallenge( - ctx context.Context, - challenge0 *HTTPChallenge, -) (*Certificate, error) { - challenge1 := &acme.Challenge{ - URI: challenge0.URL, - Token: challenge0.Token, - } +// PollOrder performs a single GetOrder call and classifies the result. +func (s *ACMEService) PollOrder(ctx context.Context, orderURL string) (*OrderPollResult, error) { + started := time.Now() - if _, err := s.client.Accept(ctx, challenge1); err != nil && !isChallengeAlreadyValid(err) { - return nil, fmt.Errorf("cannot accept challenge: %w", err) - } - - order, err := s.client.WaitOrder(ctx, challenge0.OrderURL) + order, err := s.client.GetOrder(ctx, orderURL) if err != nil { - return nil, fmt.Errorf("cannot wait for order: %w", err) + return nil, s.handleError(provisionPhasePollOrder, started, "cannot get order", err) + } + + result := &OrderPollResult{Order: order} + + switch order.Status { + case acme.StatusPending, acme.StatusProcessing: + result.Status = OrderPollStatusNotReady + + s.metrics.observeStep(provisionPhasePollOrder, provisionResultNotReady, started) + case acme.StatusReady: + result.Status = OrderPollStatusReady + + s.metrics.observeStep(provisionPhasePollOrder, provisionResultOK, started) + case acme.StatusValid: + result.Status = OrderPollStatusValid + + s.metrics.observeStep(provisionPhasePollOrder, provisionResultOK, started) + case acme.StatusInvalid: + result.Status = OrderPollStatusInvalid + + s.metrics.observeStep(provisionPhasePollOrder, provisionResultError, started) + default: + s.metrics.observeStep(provisionPhasePollOrder, provisionResultError, started) + return nil, fmt.Errorf("order is in unexpected status %q", order.Status) + } + + return result, nil +} + +// IssueCertificate finalizes a ready order or fetches a valid certificate. +func (s *ACMEService) IssueCertificate( + ctx context.Context, + challenge *HTTPChallenge, + poll *OrderPollResult, +) (*Certificate, error) { + started := time.Now() + + if poll == nil || poll.Order == nil { + s.metrics.observeStep(provisionPhaseIssueCert, provisionResultError, started) + return nil, fmt.Errorf("missing order to issue certificate") + } + + if poll.Status == OrderPollStatusInvalid { + s.metrics.observeStep(provisionPhaseIssueCert, provisionResultError, started) + return nil, ErrOrderInvalid + } + + if poll.Status != OrderPollStatusReady && poll.Status != OrderPollStatusValid { + s.metrics.observeStep(provisionPhaseIssueCert, provisionResultNotReady, started) + return nil, ErrOrderNotReady } certKey, err := keys.Generate(s.keyType) if err != nil { + s.metrics.observeStep(provisionPhaseIssueCert, provisionResultError, started) return nil, fmt.Errorf("cannot generate certificate key: %w", err) } - csr, err := createCSR(challenge0.Domain, certKey) + csr, err := createCSR(challenge.Domain, certKey) if err != nil { + s.metrics.observeStep(provisionPhaseIssueCert, provisionResultError, started) return nil, fmt.Errorf("cannot create CSR: %w", err) } - der, err := s.issueOrderCertificate(ctx, order, challenge0.OrderURL, csr) + der, err := s.issueOrderCertificate(ctx, poll.Order, challenge.OrderURL, csr) if err != nil { - return nil, fmt.Errorf("cannot create certificate: %w", err) + return nil, s.handleError(provisionPhaseIssueCert, started, "cannot create certificate", err) } cert, err := x509.ParseCertificate(der[0]) if err != nil { + s.metrics.observeStep(provisionPhaseIssueCert, provisionResultError, started) return nil, fmt.Errorf("cannot parse certificate: %w", err) } @@ -218,6 +329,7 @@ func (s *ACMEService) CompleteHTTPChallenge( keyPEM, err := pem.EncodePrivateKey(certKey) if err != nil { + s.metrics.observeStep(provisionPhaseIssueCert, provisionResultError, started) return nil, fmt.Errorf("cannot encode key: %w", err) } @@ -228,6 +340,8 @@ func (s *ACMEService) CompleteHTTPChallenge( chainPEM := pem.EncodeCertificateChain(chainDER) + s.metrics.observeStep(provisionPhaseIssueCert, provisionResultOK, started) + return &Certificate{ CertPEM: certPEM, KeyPEM: keyPEM, @@ -240,6 +354,27 @@ func (s *ACMEService) CheckRenewalNeeded(expiresAt time.Time, threshold time.Dur return time.Until(expiresAt) <= threshold } +func (s *ACMEService) handleError( + phase provisionPhase, + started time.Time, + op string, + err error, +) error { + acmeErr := newACMEError(op, err) + s.metrics.recordACMEError(acmeErr.problemType) + + result := provisionResultError + if acmeErr.rateLimited { + result = provisionResultRateLimited + + s.enterCooldown(time.Now().Add(acmeErr.RetryAfter())) + } + + s.metrics.observeStep(phase, result, started) + + return acmeErr +} + func isChallengeAlreadyValid(err error) bool { acmeErr, ok := errors.AsType[*acme.Error](err) if !ok { @@ -271,10 +406,6 @@ func (s *ACMEService) issueOrderCertificate( return der, nil } - // CreateOrderCert finalizes the order but may fail to download the - // certificate when the CA marks the order valid before the certificate - // URL is populated. Poll the order using the known order URL because - // some CAs omit the Location header on poll responses, leaving order.URI empty. return s.fetchOrderCertificateAfterFinalize(ctx, pollURL, err) default: return nil, fmt.Errorf("order is in unexpected status %q", order.Status) @@ -300,10 +431,7 @@ func (s *ACMEService) fetchOrderCertificateAfterFinalize( } if refreshed.Status != acme.StatusValid { - refreshed, err = s.client.WaitOrder(ctx, orderURL) - if err != nil { - return nil, fmt.Errorf("cannot wait for order after finalize: %w", err) - } + return nil, fmt.Errorf("%w: order status %q after finalize", ErrOrderNotReady, refreshed.Status) } der, err := s.fetchOrderCertificate(ctx, orderURL, refreshed) @@ -319,7 +447,7 @@ func (s *ACMEService) fetchOrderCertificate( orderURL string, order *acme.Order, ) ([][]byte, error) { - orderWithCertURL, err := s.waitForCertificateURL(ctx, orderURL, order) + orderWithCertURL, err := s.refreshOrderCertificateURL(ctx, orderURL, order) if err != nil { return nil, err } @@ -332,7 +460,7 @@ func (s *ACMEService) fetchOrderCertificate( return der, nil } -func (s *ACMEService) waitForCertificateURL( +func (s *ACMEService) refreshOrderCertificateURL( ctx context.Context, orderURL string, order *acme.Order, @@ -341,30 +469,20 @@ func (s *ACMEService) waitForCertificateURL( return order, nil } - deadline := time.Now().Add(30 * time.Second) - - for time.Now().Before(deadline) { - refreshed, err := s.client.GetOrder(ctx, orderURL) - if err != nil { - return nil, fmt.Errorf("cannot refresh order: %w", err) - } - - if refreshed.CertURL != "" { - return refreshed, nil - } - - if refreshed.Status != acme.StatusValid { - return nil, fmt.Errorf("order left valid state while waiting for certificate URL") - } - - select { - case <-ctx.Done(): - return nil, ctx.Err() - case <-time.After(500 * time.Millisecond): - } + refreshed, err := s.client.GetOrder(ctx, orderURL) + if err != nil { + return nil, fmt.Errorf("cannot refresh order: %w", err) } - return nil, fmt.Errorf("timed out waiting for certificate URL") + if refreshed.CertURL != "" { + return refreshed, nil + } + + if refreshed.Status != acme.StatusValid { + return nil, fmt.Errorf("%w: order left valid state while waiting for certificate URL", ErrOrderNotReady) + } + + return nil, fmt.Errorf("%w: certificate URL not yet available", ErrOrderNotReady) } func createCSR(domain string, key crypto.Signer) ([]byte, error) { diff --git a/pkg/certmanager/acme_errors.go b/pkg/certmanager/acme_errors.go new file mode 100644 index 000000000..fb935821a --- /dev/null +++ b/pkg/certmanager/acme_errors.go @@ -0,0 +1,128 @@ +// Copyright (c) 2025-2026 Probo Inc . +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +package certmanager + +import ( + "errors" + "fmt" + "time" + + "golang.org/x/crypto/acme" +) + +const defaultCooldown = time.Hour + +var ( + ErrACMERateLimited = errors.New("acme rate limited") + ErrOrderNotReady = errors.New("acme order not ready") + ErrOrderInvalid = errors.New("acme order invalid") +) + +type ACMEError struct { + op string + err error + problemType string + detail string + rateLimited bool + retryAfter time.Duration +} + +func (e *ACMEError) Error() string { + if e == nil { + return "" + } + + if e.err != nil { + return fmt.Sprintf("%s: %v", e.op, e.err) + } + + return e.op +} + +func (e *ACMEError) Unwrap() error { + if e == nil { + return nil + } + + return e.err +} + +func (e *ACMEError) Is(target error) bool { + return e != nil && e.rateLimited && target == ErrACMERateLimited +} + +// RetryAfter returns how long callers should wait before retrying. +// For rate-limited errors it prefers the ACME Retry-After value and falls back +// to defaultCooldown when the header is absent. Non-rate-limited errors return 0. +func (e *ACMEError) RetryAfter() time.Duration { + if e == nil || !e.rateLimited { + return 0 + } + + if e.retryAfter > 0 { + return e.retryAfter + } + + return defaultCooldown +} + +func (e *ACMEError) ProblemType() string { + if e == nil { + return "" + } + + return e.problemType +} + +func (e *ACMEError) Detail() string { + if e == nil { + return "" + } + + return e.detail +} + +func newACMEError(op string, err error) *ACMEError { + if err == nil { + return nil + } + + out := &ACMEError{ + op: op, + err: err, + } + + acmeErr, ok := errors.AsType[*acme.Error](err) + if !ok { + out.detail = err.Error() + return out + } + + out.problemType = acmeErr.ProblemType + out.detail = acmeErr.Detail + + if retryAfter, ok := acme.RateLimit(acmeErr); ok { + out.rateLimited = true + out.retryAfter = retryAfter + } + + return out +} diff --git a/pkg/certmanager/acme_test.go b/pkg/certmanager/acme_test.go new file mode 100644 index 000000000..5483a3e2c --- /dev/null +++ b/pkg/certmanager/acme_test.go @@ -0,0 +1,76 @@ +// Copyright (c) 2026 Probo Inc . +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +package certmanager + +import ( + "errors" + "net/http" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/crypto/acme" +) + +func TestNewACMEError_RateLimited(t *testing.T) { + t.Parallel() + + err := newACMEError( + "cannot create order", + &acme.Error{ + ProblemType: "urn:ietf:params:acme:error:rateLimited", + Detail: "too many requests", + Header: http.Header{"Retry-After": []string{"120"}}, + }, + ) + + require.NotNil(t, err) + assert.ErrorIs(t, err, ErrACMERateLimited) + assert.Equal(t, 2*time.Minute, err.RetryAfter()) + assert.Equal(t, "urn:ietf:params:acme:error:rateLimited", err.problemType) + assert.Equal(t, "too many requests", err.detail) +} + +func TestNewACMEError_RateLimitedDefaultCooldown(t *testing.T) { + t.Parallel() + + err := newACMEError( + "cannot create order", + &acme.Error{ProblemType: "URN:IETF:PARAMS:ACME:ERROR:RATELIMITED"}, + ) + + require.NotNil(t, err) + assert.ErrorIs(t, err, ErrACMERateLimited) + assert.Equal(t, defaultCooldown, err.RetryAfter()) +} + +func TestNewACMEError_NonRateLimited(t *testing.T) { + t.Parallel() + + cause := errors.New("network timeout") + err := newACMEError("cannot get order", cause) + + require.NotNil(t, err) + assert.False(t, errors.Is(err, ErrACMERateLimited)) + assert.ErrorIs(t, err, cause) + assert.Equal(t, time.Duration(0), err.RetryAfter()) +} diff --git a/pkg/certmanager/metrics.go b/pkg/certmanager/metrics.go new file mode 100644 index 000000000..46b8e8418 --- /dev/null +++ b/pkg/certmanager/metrics.go @@ -0,0 +1,123 @@ +// Copyright (c) 2025-2026 Probo Inc . +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +package certmanager + +import ( + "time" + + "github.com/prometheus/client_golang/prometheus" +) + +type ( + provisionPhase string + provisionResult string +) + +const ( + provisionPhaseCreateOrder provisionPhase = "create_order" + provisionPhasePollOrder provisionPhase = "poll_order" + provisionPhaseIssueCert provisionPhase = "issue_cert" + provisionPhaseDNSCheck provisionPhase = "dns_check" + + provisionResultOK provisionResult = "ok" + provisionResultNotReady provisionResult = "not_ready" + provisionResultError provisionResult = "error" + provisionResultRateLimited provisionResult = "rate_limited" + provisionResultDNSError provisionResult = "dns_error" +) + +type metrics struct { + provisionSteps *prometheus.CounterVec + acmeErrors *prometheus.CounterVec + acmeCooldown prometheus.Gauge + stepDuration *prometheus.HistogramVec +} + +func newMetrics(registerer prometheus.Registerer) *metrics { + if registerer == nil { + registerer = prometheus.DefaultRegisterer + } + + m := &metrics{ + provisionSteps: prometheus.NewCounterVec( + prometheus.CounterOpts{ + Subsystem: "certmanager", + Name: "certificate_provision_steps_total", + Help: "Certificate provisioning steps by phase and result.", + }, + []string{"phase", "result"}, + ), + acmeErrors: prometheus.NewCounterVec( + prometheus.CounterOpts{ + Subsystem: "certmanager", + Name: "certificate_acme_errors_total", + Help: "ACME errors by problem type.", + }, + []string{"problem_type"}, + ), + acmeCooldown: prometheus.NewGauge( + prometheus.GaugeOpts{ + Subsystem: "certmanager", + Name: "certificate_acme_cooldown", + Help: "1 while the ACME client is in a global rate-limit cooldown.", + }, + ), + stepDuration: prometheus.NewHistogramVec( + prometheus.HistogramOpts{ + Subsystem: "certmanager", + Name: "certificate_provision_step_duration_seconds", + Help: "Duration of certificate provisioning steps in seconds.", + }, + []string{"phase"}, + ), + } + + registerer.MustRegister( + m.provisionSteps, + m.acmeErrors, + m.acmeCooldown, + m.stepDuration, + ) + + return m +} + +func (m *metrics) observeStep(phase provisionPhase, result provisionResult, started time.Time) { + m.provisionSteps.WithLabelValues(string(phase), string(result)).Inc() + m.stepDuration.WithLabelValues(string(phase)).Observe(time.Since(started).Seconds()) +} + +func (m *metrics) recordACMEError(problemType string) { + if problemType == "" { + problemType = "unknown" + } + + m.acmeErrors.WithLabelValues(problemType).Inc() +} + +func (m *metrics) setCooldown(active bool) { + if active { + m.acmeCooldown.Set(1) + return + } + + m.acmeCooldown.Set(0) +} diff --git a/pkg/certmanager/provision_worker.go b/pkg/certmanager/provision_worker.go index 2c298a465..5a40f61b3 100644 --- a/pkg/certmanager/provision_worker.go +++ b/pkg/certmanager/provision_worker.go @@ -25,23 +25,48 @@ import ( "go.gearno.de/kit/log" "go.gearno.de/kit/pg" "go.gearno.de/kit/worker" + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/attribute" + "go.opentelemetry.io/otel/codes" + "go.opentelemetry.io/otel/trace" "go.probo.inc/probo/pkg/coredata" "go.probo.inc/probo/pkg/crypto/cipher" "go.probo.inc/probo/pkg/gid" ) -const maxProvisioningRetries = 3 +const ( + // maxProvisioningRetries is the failure budget for ordinary transient ACME + // errors. Rate limits do not consume this budget; they use the in-process + // ACME cooldown instead. Claim SQL still caps the exponential backoff + // exponent at 5 (LEAST(ssl_retry_count, 5)), so normal retries only reach + // exponents 0–2 before FAILED. + maxProvisioningRetries = 3 + dnsExchangeTimeout = 10 * time.Second + processTickTimeout = 90 * time.Second -type provisionHandler struct { - pg *pg.Client - acmeService *ACMEService - encryptionKey cipher.EncryptionKey - cnameTarget string - caaIssuerDomain string - resolverAddr string - managedBaseDomain string - logger *log.Logger -} + tracerName = "go.probo.inc/probo/pkg/certmanager" +) + +type ( + provisionHandler struct { + pg *pg.Client + acmeService *ACMEService + encryptionKey cipher.EncryptionKey + cnameTarget string + caaIssuerDomain string + resolverAddr string + managedBaseDomain string + logger *log.Logger + tracer trace.Tracer + } + + provisioningOutcome struct { + status coredata.CertificateStatus + retryCount int + clearACMEState bool + errorCode string + } +) var ( _ worker.Handler[coredata.Certificate] = (*provisionHandler)(nil) @@ -68,8 +93,11 @@ func NewProvisionWorker( resolverAddr: resolverAddr, managedBaseDomain: managedBaseDomain, logger: logger, + tracer: otel.Tracer(tracerName), } + opts = append(opts, worker.WithMaxConcurrency(1)) + return worker.New( "certificate-provision-worker", h, @@ -79,6 +107,10 @@ func NewProvisionWorker( } func (h *provisionHandler) Claim(ctx context.Context) (coredata.Certificate, error) { + if h.acmeService.InCooldown() { + return coredata.Certificate{}, worker.ErrNoTask + } + var certificate coredata.Certificate if err := h.pg.WithTx( @@ -88,6 +120,13 @@ func (h *provisionHandler) Claim(ctx context.Context) (coredata.Certificate, err return err } + now := time.Now() + certificate.SSLLastAttemptAt = &now + + if err := certificate.Update(ctx, tx, coredata.NewNoScope()); err != nil { + return fmt.Errorf("cannot stamp certificate claim: %w", err) + } + return nil }, ); err != nil { @@ -102,57 +141,372 @@ func (h *provisionHandler) Claim(ctx context.Context) (coredata.Certificate, err } func (h *provisionHandler) Process(ctx context.Context, certificate coredata.Certificate) error { - challengeInitiated, err := h.runProvisionCertificate(ctx, certificate.ID) - if err != nil { - h.logger.ErrorCtx( - ctx, - "cannot provision certificate", - log.String("hostname", certificate.Hostname), - log.Error(err), - ) + ctx, cancel := context.WithTimeout(ctx, processTickTimeout) + defer cancel() - return err - } - - if !challengeInitiated { + switch certificate.Status { + case coredata.CertificateStatusPending, coredata.CertificateStatusRenewing: + return h.processBeginChallenge(ctx, certificate) + case coredata.CertificateStatusProvisioning: + return h.processPollOrder(ctx, certificate) + default: return nil } +} - if _, err := h.runProvisionCertificate(ctx, certificate.ID); err != nil { - h.logger.ErrorCtx( - ctx, - "cannot complete certificate challenge", - log.String("hostname", certificate.Hostname), - log.Error(err), - ) +func (h *provisionHandler) processBeginChallenge( + ctx context.Context, + certificate coredata.Certificate, +) error { + ctx, span := h.tracer.Start(ctx, "certmanager.create_order") + defer span.End() + h.setCertificateSpanAttributes(span, certificate) + + skipDNSChecks, err := h.loadSkipDNSChecks(ctx, certificate.Hostname) + if err != nil { + h.recordSpanError(span, err, "") return err } - return nil -} + if !skipDNSChecks { + dnsCtx, dnsSpan := h.tracer.Start(ctx, "certmanager.dns_check") + dnsStarted := time.Now() -func (h *provisionHandler) runProvisionCertificate( - ctx context.Context, - certificateID gid.GID, -) (bool, error) { - var challengeInitiated bool + if err := h.checkDNSConfiguration(dnsCtx, certificate.Hostname); err != nil { + h.acmeService.metrics.observeStep(provisionPhaseDNSCheck, provisionResultDNSError, dnsStarted) + h.recordSpanError(dnsSpan, err, classifyProvisioningError(err)) + dnsSpan.End() + h.recordSpanError(span, err, classifyProvisioningError(err)) + // DNS/CAA misconfig is intentionally non-terminal: retry forever so a + // customer DNS fix auto-recovers without marking the domain FAILED. + return h.persistFailure(ctx, certificate.ID, err) + } - err := h.pg.WithTx( - ctx, - func(ctx context.Context, tx pg.Tx) error { - var err error + if err := h.checkCAARecords(dnsCtx, certificate.Hostname); err != nil { + h.acmeService.metrics.observeStep(provisionPhaseDNSCheck, provisionResultDNSError, dnsStarted) + h.recordSpanError(dnsSpan, err, classifyProvisioningError(err)) + dnsSpan.End() + h.recordSpanError(span, err, classifyProvisioningError(err)) + // DNS/CAA misconfig is intentionally non-terminal (see above). + return h.persistFailure(ctx, certificate.ID, err) + } - challengeInitiated, err = h.provisionCertificate(ctx, tx, certificateID) - - return err - }, - ) - if err != nil { - return false, err + h.acmeService.metrics.observeStep(provisionPhaseDNSCheck, provisionResultOK, dnsStarted) + dnsSpan.End() } - return challengeInitiated, nil + challenge, err := h.acmeService.StartHTTPChallenge(ctx, certificate.Hostname) + if err != nil { + errorCode := classifyProvisioningError(err) + h.logACMEOutcome(ctx, certificate, provisionPhaseCreateOrder, err, errorCode) + h.recordSpanError(span, err, errorCode) + + return h.persistFailure(ctx, certificate.ID, err) + } + + return h.pg.WithTx( + ctx, + func(ctx context.Context, tx pg.Tx) error { + row := &coredata.Certificate{} + if err := row.LoadByIDForUpdateSkipLocked(ctx, tx, coredata.NewNoScope(), certificate.ID); err != nil { + if errors.Is(err, coredata.ErrResourceNotFound) { + return nil + } + + return fmt.Errorf("cannot load certificate %q: %w", certificate.ID, err) + } + + if row.Status != coredata.CertificateStatusPending && + row.Status != coredata.CertificateStatusRenewing { + return nil + } + + row.HTTPChallengeToken = &challenge.Token + row.HTTPChallengeKeyAuth = &challenge.KeyAuth + row.HTTPChallengeURL = &challenge.URL + row.HTTPOrderURL = &challenge.OrderURL + row.Status = coredata.CertificateStatusProvisioning + row.ProvisioningError = nil + + if err := row.Update(ctx, tx, coredata.NewNoScope()); err != nil { + return fmt.Errorf("cannot update certificate with challenge: %w", err) + } + + h.logger.InfoCtx( + ctx, + "HTTP challenge accepted, waiting for order validation", + log.String("hostname", row.Hostname), + log.String("certificate_id", row.ID.String()), + ) + + return nil + }, + ) +} + +func (h *provisionHandler) processPollOrder( + ctx context.Context, + certificate coredata.Certificate, +) error { + ctx, span := h.tracer.Start(ctx, "certmanager.poll_order") + defer span.End() + + h.setCertificateSpanAttributes(span, certificate) + + if certificate.HTTPOrderURL == nil { + return h.persistFailure( + ctx, + certificate.ID, + fmt.Errorf("provisioning certificate missing order URL"), + ) + } + + challenge := &HTTPChallenge{ + Domain: certificate.Hostname, + Token: stringValue(certificate.HTTPChallengeToken), + KeyAuth: stringValue(certificate.HTTPChallengeKeyAuth), + URL: stringValue(certificate.HTTPChallengeURL), + OrderURL: *certificate.HTTPOrderURL, + } + + poll, err := h.acmeService.PollOrder(ctx, *certificate.HTTPOrderURL) + if err != nil { + errorCode := classifyProvisioningError(err) + h.logACMEOutcome(ctx, certificate, provisionPhasePollOrder, err, errorCode) + h.recordSpanError(span, err, errorCode) + + return h.persistFailure(ctx, certificate.ID, err) + } + + switch poll.Status { + case OrderPollStatusNotReady: + h.logger.InfoCtx( + ctx, + "ACME order not ready yet, will poll again", + log.String("hostname", certificate.Hostname), + log.String("certificate_id", certificate.ID.String()), + log.String("order_status", poll.Order.Status), + ) + + return nil + case OrderPollStatusInvalid: + err := ErrOrderInvalid + errorCode := classifyProvisioningError(err) + h.logACMEOutcome(ctx, certificate, provisionPhasePollOrder, err, errorCode) + h.recordSpanError(span, err, errorCode) + + return h.persistFailure(ctx, certificate.ID, err) + case OrderPollStatusReady, OrderPollStatusValid: + return h.issueCertificate(ctx, certificate, challenge, poll) + default: + return nil + } +} + +func (h *provisionHandler) issueCertificate( + ctx context.Context, + certificate coredata.Certificate, + challenge *HTTPChallenge, + poll *OrderPollResult, +) error { + ctx, span := h.tracer.Start(ctx, "certmanager.issue_cert") + defer span.End() + + h.setCertificateSpanAttributes(span, certificate) + + cert, err := h.acmeService.IssueCertificate(ctx, challenge, poll) + if err != nil { + if errors.Is(err, ErrOrderNotReady) { + h.logger.InfoCtx( + ctx, + "ACME order not ready to issue yet, will poll again", + log.String("hostname", certificate.Hostname), + log.String("certificate_id", certificate.ID.String()), + ) + + return nil + } + + errorCode := classifyProvisioningError(err) + h.logACMEOutcome(ctx, certificate, provisionPhaseIssueCert, err, errorCode) + h.recordSpanError(span, err, errorCode) + + return h.persistFailure(ctx, certificate.ID, err) + } + + return h.pg.WithTx( + ctx, + func(ctx context.Context, tx pg.Tx) error { + row := &coredata.Certificate{} + if err := row.LoadByIDForUpdate(ctx, tx, coredata.NewNoScope(), certificate.ID); err != nil { + return fmt.Errorf("cannot load certificate %q: %w", certificate.ID, err) + } + + h.logger.InfoCtx( + ctx, + "certificate obtained successfully", + log.String("hostname", row.Hostname), + log.String("certificate_id", row.ID.String()), + log.Time("expires_at", cert.ExpiresAt), + ) + + row.ProvisioningError = nil + row.SSLCertificatePEM = cert.CertPEM + + if err := row.EncryptPrivateKey(cert.KeyPEM, h.encryptionKey); err != nil { + return fmt.Errorf("cannot encrypt private key: %w", err) + } + + chainStr := string(cert.ChainPEM) + row.SSLCertificateChain = &chainStr + row.SSLExpiresAt = &cert.ExpiresAt + row.Status = coredata.CertificateStatusActive + row.SSLRetryCount = 0 + row.SSLLastAttemptAt = nil + row.HTTPChallengeToken = nil + row.HTTPChallengeKeyAuth = nil + row.HTTPChallengeURL = nil + row.HTTPOrderURL = nil + + if err := row.Update(ctx, tx, coredata.NewNoScope()); err != nil { + return fmt.Errorf("cannot update certificate: %w", err) + } + + cache := &coredata.CachedCertificate{ + Domain: row.Hostname, + CertificatePEM: string(cert.CertPEM), + PrivateKeyPEM: string(cert.KeyPEM), + CertificateChain: &chainStr, + ExpiresAt: cert.ExpiresAt, + CachedAt: time.Now(), + CertificateID: row.ID, + } + + if err := cache.Upsert(ctx, tx); err != nil { + h.logger.ErrorCtx( + ctx, + "cannot update certificate cache", + log.String("hostname", row.Hostname), + log.Error(err), + ) + } + + return nil + }, + ) +} + +func (h *provisionHandler) persistFailure( + ctx context.Context, + certificateID gid.GID, + provisionErr error, +) error { + errorCode := classifyProvisioningError(provisionErr) + + return h.pg.WithTx( + ctx, + func(ctx context.Context, tx pg.Tx) error { + row := &coredata.Certificate{} + if err := row.LoadByIDForUpdateSkipLocked(ctx, tx, coredata.NewNoScope(), certificateID); err != nil { + if errors.Is(err, coredata.ErrResourceNotFound) { + return nil + } + + return fmt.Errorf("cannot load certificate %q: %w", certificateID, err) + } + + outcome := decideProvisioningOutcome(row, errorCode) + + row.ProvisioningError = provisioningErrorCodePtr(outcome.errorCode) + now := time.Now() + row.SSLLastAttemptAt = &now + row.Status = outcome.status + row.SSLRetryCount = outcome.retryCount + + if outcome.clearACMEState { + row.HTTPChallengeToken = nil + row.HTTPChallengeKeyAuth = nil + row.HTTPChallengeURL = nil + row.HTTPOrderURL = nil + } + + if err := row.Update(ctx, tx, coredata.NewNoScope()); err != nil { + return fmt.Errorf("cannot update certificate with provisioning error: %w", err) + } + + return nil + }, + ) +} + +// decideProvisioningOutcome computes status/retry/clear policy for a failed step. +// +// Two backoff regimes: +// - Normal transient errors: ssl_retry_count increments; at maxProvisioningRetries +// the domain becomes FAILED. +// - Rate limits: never increment ssl_retry_count and never mark FAILED; the +// in-process ACME cooldown gates Claim. When an order URL is present +// it is preserved so the next tick resumes polling instead of minting a new +// order. +// +// DNS/CAA misconfig is intentionally non-terminal so customer DNS fixes +// auto-recover on the claim backoff schedule. +func decideProvisioningOutcome( + certificate *coredata.Certificate, + errorCode string, +) provisioningOutcome { + retryCount := certificate.SSLRetryCount + hasResumableOrder := certificate.HTTPOrderURL != nil && *certificate.HTTPOrderURL != "" + + switch errorCode { + case ProvisioningErrorACMERateLimited: + status := coredata.CertificateStatusPending + if hasResumableOrder { + status = coredata.CertificateStatusProvisioning + } + + return provisioningOutcome{ + status: status, + retryCount: retryCount, + clearACMEState: false, + errorCode: errorCode, + } + + case ProvisioningErrorDNSCNAME, ProvisioningErrorDNSCAA: + return provisioningOutcome{ + status: coredata.CertificateStatusPending, + retryCount: retryCount, + clearACMEState: true, + errorCode: errorCode, + } + } + + retryCount++ + if retryCount >= maxProvisioningRetries { + return provisioningOutcome{ + status: coredata.CertificateStatusFailed, + retryCount: retryCount, + clearACMEState: true, + errorCode: ProvisioningErrorACMEFailed, + } + } + + if errorCode == ProvisioningErrorACMEInvalidOrder || !hasResumableOrder { + return provisioningOutcome{ + status: coredata.CertificateStatusPending, + retryCount: retryCount, + clearACMEState: true, + errorCode: errorCode, + } + } + + return provisioningOutcome{ + status: coredata.CertificateStatusProvisioning, + retryCount: retryCount, + clearACMEState: false, + errorCode: errorCode, + } } func (h *provisionHandler) RecoverStale(ctx context.Context) error { @@ -186,7 +540,103 @@ func (h *provisionHandler) RecoverStale(ctx context.Context) error { ) } -func (h *provisionHandler) checkDNSConfiguration(hostname string) error { +func (h *provisionHandler) logACMEOutcome( + ctx context.Context, + certificate coredata.Certificate, + phase provisionPhase, + err error, + errorCode string, +) { + var ( + problemType string + detail string + ) + if acmeErr, ok := errors.AsType[*ACMEError](err); ok { + problemType = acmeErr.ProblemType() + detail = acmeErr.Detail() + } + + level := h.logger.WarnCtx + + if errorCode == ProvisioningErrorACMETemporary { + level = h.logger.ErrorCtx + } + + level( + ctx, + "certificate provisioning step failed", + log.String("hostname", certificate.Hostname), + log.String("certificate_id", certificate.ID.String()), + log.String("phase", string(phase)), + log.String("error_code", errorCode), + log.String("acme_problem_type", problemType), + log.String("acme_detail", detail), + log.Int("retry_count", certificate.SSLRetryCount), + log.Time("cool_down_until", h.acmeService.CooldownUntil()), + log.Error(err), + ) +} + +func (h *provisionHandler) setCertificateSpanAttributes(span trace.Span, certificate coredata.Certificate) { + span.SetAttributes( + attribute.String("certificate.id", certificate.ID.String()), + attribute.String("certificate.hostname", certificate.Hostname), + attribute.String("certificate.status", string(certificate.Status)), + ) +} + +func (h *provisionHandler) recordSpanError(span trace.Span, err error, errorCode string) { + if err == nil { + return + } + + var ( + problemType string + detail string + ) + if acmeErr, ok := errors.AsType[*ACMEError](err); ok { + problemType = acmeErr.ProblemType() + detail = acmeErr.Detail() + } + + span.RecordError(err) + span.SetStatus(codes.Error, errorCode) + span.SetAttributes( + attribute.String("provisioning.error_code", errorCode), + attribute.String("acme.problem_type", problemType), + attribute.String("acme.detail", detail), + ) +} + +func (h *provisionHandler) loadSkipDNSChecks(ctx context.Context, hostname string) (bool, error) { + var skip bool + + err := h.pg.WithConn( + ctx, + func(ctx context.Context, conn pg.Querier) error { + var err error + + skip, err = h.skipsDNSChecks(ctx, conn, hostname) + + return err + }, + ) + if err != nil { + return false, err + } + + return skip, nil +} + +func stringValue(value *string) string { + if value == nil { + return "" + } + + return *value +} + +func (h *provisionHandler) checkDNSConfiguration(ctx context.Context, hostname string) error { customerFQDN := hostname if !strings.HasSuffix(customerFQDN, ".") { customerFQDN = customerFQDN + "." @@ -200,9 +650,12 @@ func (h *provisionHandler) checkDNSConfiguration(hostname string) error { msg := &dns.Msg{MsgHeader: dns.MsgHeader{ID: dns.ID(), RecursionDesired: true}} msg.Question = []dns.RR{&dns.CNAME{Hdr: dns.Header{Name: customerFQDN, Class: dns.ClassINET}}} + dnsCtx, cancel := context.WithTimeout(ctx, dnsExchangeTimeout) + defer cancel() + client := dns.NewClient() - resp, _, err := client.Exchange(context.Background(), msg, "udp", h.resolverAddr) + resp, _, err := client.Exchange(dnsCtx, msg, "udp", h.resolverAddr) if err != nil { return fmt.Errorf("cannot exchange dns message: %w", err) } @@ -232,7 +685,7 @@ func (h *provisionHandler) checkDNSConfiguration(hostname string) error { return nil } -func (h *provisionHandler) checkCAARecords(hostname string) error { +func (h *provisionHandler) checkCAARecords(ctx context.Context, hostname string) error { fqdn := hostname if !strings.HasSuffix(fqdn, ".") { fqdn = fqdn + "." @@ -241,10 +694,13 @@ func (h *provisionHandler) checkCAARecords(hostname string) error { msg := &dns.Msg{MsgHeader: dns.MsgHeader{ID: dns.ID(), RecursionDesired: true}} msg.Question = []dns.RR{&dns.CAA{Hdr: dns.Header{Name: fqdn, Class: dns.ClassINET}}} + dnsCtx, cancel := context.WithTimeout(ctx, dnsExchangeTimeout) + defer cancel() + client := dns.NewClient() resp, _, err := client.Exchange( - context.Background(), + dnsCtx, msg, "udp", h.resolverAddr, @@ -358,215 +814,3 @@ func (h *provisionHandler) resetStaleCertificate( return nil } - -func (h *provisionHandler) provisionCertificate( - ctx context.Context, - tx pg.Tx, - certificateID gid.GID, -) (bool, error) { - certificate := &coredata.Certificate{} - if err := certificate.LoadByIDForUpdateSkipLocked(ctx, tx, coredata.NewNoScope(), certificateID); err != nil { - if errors.Is(err, coredata.ErrResourceNotFound) { - return false, nil - } - - return false, fmt.Errorf("cannot load by id for update %q certificate: %w", certificateID, err) - } - - if certificate.Status == coredata.CertificateStatusPending || certificate.Status == coredata.CertificateStatusRenewing { - skipDNSChecks, err := h.skipsDNSChecks(ctx, tx, certificate.Hostname) - if err != nil { - return false, fmt.Errorf("cannot check managed domain: %w", err) - } - - if !skipDNSChecks { - if err := h.checkDNSConfiguration(certificate.Hostname); err != nil { - h.logger.WarnCtx( - ctx, - "dns configuration check failed", - log.String("hostname", certificate.Hostname), - log.Error(err), - ) - - errMsg := err.Error() - - certificate.ProvisioningError = &errMsg - if err := certificate.Update(ctx, tx, coredata.NewNoScope()); err != nil { - return false, fmt.Errorf("cannot update certificate with provisioning error: %w", err) - } - - return false, nil - } - - if err := h.checkCAARecords(certificate.Hostname); err != nil { - h.logger.WarnCtx( - ctx, - "caa record check failed", - log.String("hostname", certificate.Hostname), - log.Error(err), - ) - - errMsg := err.Error() - - certificate.ProvisioningError = &errMsg - if err := certificate.Update(ctx, tx, coredata.NewNoScope()); err != nil { - return false, fmt.Errorf("cannot update certificate with provisioning error: %w", err) - } - - return false, nil - } - } - - certificate.ProvisioningError = nil - if err := certificate.Update(ctx, tx, coredata.NewNoScope()); err != nil { - return false, fmt.Errorf("cannot clear provisioning error: %w", err) - } - - h.logger.InfoCtx(ctx, "DNS configuration verified, initiating HTTP challenge for hostname", log.String("hostname", certificate.Hostname)) - - challenge, err := h.acmeService.GetHTTPChallenge(ctx, certificate.Hostname) - if err != nil { - h.logger.ErrorCtx( - ctx, - "cannot get HTTP challenge", - log.String("hostname", certificate.Hostname), - log.Error(err), - ) - - return false, err - } - - certificate.HTTPChallengeToken = &challenge.Token - certificate.HTTPChallengeKeyAuth = &challenge.KeyAuth - certificate.HTTPChallengeURL = &challenge.URL - certificate.HTTPOrderURL = &challenge.OrderURL - certificate.Status = coredata.CertificateStatusProvisioning - - if err := certificate.Update(ctx, tx, coredata.NewNoScope()); err != nil { - return false, fmt.Errorf("cannot update certificate with challenge: %w", err) - } - - h.logger.InfoCtx( - ctx, - "HTTP challenge initiated, completing in same cycle", - log.String("hostname", certificate.Hostname), - log.String("token", challenge.Token), - ) - - return true, nil - } - - if certificate.HTTPChallengeToken == nil || - certificate.HTTPChallengeKeyAuth == nil || - certificate.HTTPChallengeURL == nil || - certificate.HTTPOrderURL == nil { - certificate.Status = coredata.CertificateStatusPending - - if err := certificate.Update(ctx, tx, coredata.NewNoScope()); err != nil { - return false, fmt.Errorf("cannot reset certificate without challenge data: %w", err) - } - - return false, nil - } - - challenge := &HTTPChallenge{ - Domain: certificate.Hostname, - Token: *certificate.HTTPChallengeToken, - KeyAuth: *certificate.HTTPChallengeKeyAuth, - URL: *certificate.HTTPChallengeURL, - OrderURL: *certificate.HTTPOrderURL, - } - - cert, err := h.acmeService.CompleteHTTPChallenge(ctx, challenge) - if err != nil { - h.logger.WarnCtx( - ctx, - "cannot complete HTTP challenge", - log.String("hostname", certificate.Hostname), - log.Int("retry_count", certificate.SSLRetryCount), - log.Error(err), - ) - - errMsg := err.Error() - certificate.ProvisioningError = &errMsg - certificate.SSLRetryCount = certificate.SSLRetryCount + 1 - now := time.Now() - certificate.SSLLastAttemptAt = &now - - certificate.HTTPChallengeToken = nil - certificate.HTTPChallengeKeyAuth = nil - certificate.HTTPChallengeURL = nil - certificate.HTTPOrderURL = nil - - if certificate.SSLRetryCount >= maxProvisioningRetries { - h.logger.ErrorCtx( - ctx, - "certificate has exceeded max retry attempts, marking as failed", - log.String("hostname", certificate.Hostname), - log.Int("retry_count", certificate.SSLRetryCount), - ) - - certificate.Status = coredata.CertificateStatusFailed - } else { - certificate.Status = coredata.CertificateStatusPending - } - - if err := certificate.Update(ctx, tx, coredata.NewNoScope()); err != nil { - return false, fmt.Errorf("cannot update certificate: %w", err) - } - - return false, nil - } - - h.logger.InfoCtx( - ctx, - "certificate obtained successfully", - log.String("hostname", certificate.Hostname), - log.Time("expires_at", cert.ExpiresAt), - ) - - certificate.ProvisioningError = nil - - certificate.SSLCertificatePEM = cert.CertPEM - if err := certificate.EncryptPrivateKey(cert.KeyPEM, h.encryptionKey); err != nil { - return false, fmt.Errorf("cannot encrypt private key: %w", err) - } - - chainStr := string(cert.ChainPEM) - certificate.SSLCertificateChain = &chainStr - certificate.SSLExpiresAt = &cert.ExpiresAt - certificate.Status = coredata.CertificateStatusActive - - certificate.SSLRetryCount = 0 - certificate.SSLLastAttemptAt = nil - - certificate.HTTPChallengeToken = nil - certificate.HTTPChallengeKeyAuth = nil - certificate.HTTPChallengeURL = nil - certificate.HTTPOrderURL = nil - - if err := certificate.Update(ctx, tx, coredata.NewNoScope()); err != nil { - return false, fmt.Errorf("cannot update certificate: %w", err) - } - - cache := &coredata.CachedCertificate{ - Domain: certificate.Hostname, - CertificatePEM: string(cert.CertPEM), - PrivateKeyPEM: string(cert.KeyPEM), - CertificateChain: &chainStr, - ExpiresAt: cert.ExpiresAt, - CachedAt: time.Now(), - CertificateID: certificate.ID, - } - - if err := cache.Upsert(ctx, tx); err != nil { - h.logger.ErrorCtx( - ctx, - "cannot update certificate cache", - log.String("hostname", certificate.Hostname), - log.Error(err), - ) - } - - return false, nil -} diff --git a/pkg/certmanager/provision_worker_test.go b/pkg/certmanager/provision_worker_test.go new file mode 100644 index 000000000..acb95eee3 --- /dev/null +++ b/pkg/certmanager/provision_worker_test.go @@ -0,0 +1,165 @@ +// Copyright (c) 2026 Probo Inc . +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +package certmanager + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.probo.inc/probo/pkg/coredata" +) + +func TestClassifyProvisioningError(t *testing.T) { + t.Parallel() + + assert.Equal(t, ProvisioningErrorACMERateLimited, classifyProvisioningError(ErrACMERateLimited)) + assert.Equal(t, ProvisioningErrorACMEInvalidOrder, classifyProvisioningError(ErrOrderInvalid)) + assert.Equal(t, ProvisioningErrorDNSCNAME, classifyProvisioningError(errors.New("cname target mismatch"))) + assert.Equal(t, ProvisioningErrorDNSCAA, classifyProvisioningError(errors.New("caa records for domain"))) + assert.Equal(t, ProvisioningErrorACMETemporary, classifyProvisioningError(errors.New("network timeout"))) +} + +func TestDecideProvisioningOutcome_RateLimitKeepsRetryCountAndOrder(t *testing.T) { + t.Parallel() + + orderURL := "https://acme.example/order/1" + certificate := &coredata.Certificate{ + Status: coredata.CertificateStatusProvisioning, + SSLRetryCount: 2, + HTTPOrderURL: &orderURL, + } + + outcome := decideProvisioningOutcome(certificate, ProvisioningErrorACMERateLimited) + + assert.Equal(t, coredata.CertificateStatusProvisioning, outcome.status) + assert.Equal(t, 2, outcome.retryCount) + assert.False(t, outcome.clearACMEState) + assert.Equal(t, ProvisioningErrorACMERateLimited, outcome.errorCode) +} + +func TestDecideProvisioningOutcome_RateLimitThenTransientDoesNotFail(t *testing.T) { + t.Parallel() + + orderURL := "https://acme.example/order/1" + certificate := &coredata.Certificate{ + Status: coredata.CertificateStatusProvisioning, + SSLRetryCount: 0, + HTTPOrderURL: &orderURL, + } + + rateLimited := decideProvisioningOutcome(certificate, ProvisioningErrorACMERateLimited) + certificate.SSLRetryCount = rateLimited.retryCount + certificate.Status = rateLimited.status + + // Previously rate-limit floored ssl_retry_count to 5, so the next transient + // immediately crossed maxProvisioningRetries and marked FAILED. + transient := decideProvisioningOutcome(certificate, ProvisioningErrorACMETemporary) + + assert.Equal(t, coredata.CertificateStatusProvisioning, transient.status) + assert.Equal(t, 1, transient.retryCount) + assert.False(t, transient.clearACMEState) + assert.NotEqual(t, coredata.CertificateStatusFailed, transient.status) +} + +func TestDecideProvisioningOutcome_MarksFailedAfterMaxRetries(t *testing.T) { + t.Parallel() + + certificate := &coredata.Certificate{ + Status: coredata.CertificateStatusProvisioning, + SSLRetryCount: maxProvisioningRetries - 1, + } + + outcome := decideProvisioningOutcome(certificate, ProvisioningErrorACMEInvalidOrder) + + assert.Equal(t, coredata.CertificateStatusFailed, outcome.status) + assert.Equal(t, maxProvisioningRetries, outcome.retryCount) + assert.True(t, outcome.clearACMEState) + assert.Equal(t, ProvisioningErrorACMEFailed, outcome.errorCode) +} + +func TestDecideProvisioningOutcome_TransientPreservesOrder(t *testing.T) { + t.Parallel() + + orderURL := "https://acme.example/order/1" + certificate := &coredata.Certificate{ + Status: coredata.CertificateStatusProvisioning, + SSLRetryCount: 0, + HTTPOrderURL: &orderURL, + } + + outcome := decideProvisioningOutcome(certificate, ProvisioningErrorACMETemporary) + + assert.Equal(t, coredata.CertificateStatusProvisioning, outcome.status) + assert.Equal(t, 1, outcome.retryCount) + assert.False(t, outcome.clearACMEState) + assert.Equal(t, ProvisioningErrorACMETemporary, outcome.errorCode) +} + +func TestDecideProvisioningOutcome_InvalidOrderClearsState(t *testing.T) { + t.Parallel() + + orderURL := "https://acme.example/order/1" + certificate := &coredata.Certificate{ + Status: coredata.CertificateStatusProvisioning, + SSLRetryCount: 0, + HTTPOrderURL: &orderURL, + } + + outcome := decideProvisioningOutcome(certificate, ProvisioningErrorACMEInvalidOrder) + + assert.Equal(t, coredata.CertificateStatusPending, outcome.status) + assert.Equal(t, 1, outcome.retryCount) + assert.True(t, outcome.clearACMEState) + assert.Equal(t, ProvisioningErrorACMEInvalidOrder, outcome.errorCode) +} + +func TestDecideProvisioningOutcome_DNSIsNonTerminal(t *testing.T) { + t.Parallel() + + certificate := &coredata.Certificate{ + Status: coredata.CertificateStatusPending, + SSLRetryCount: 0, + } + + outcome := decideProvisioningOutcome(certificate, ProvisioningErrorDNSCNAME) + + assert.Equal(t, coredata.CertificateStatusPending, outcome.status) + assert.Equal(t, 0, outcome.retryCount) + assert.True(t, outcome.clearACMEState) + assert.Equal(t, ProvisioningErrorDNSCNAME, outcome.errorCode) +} + +func TestDecideProvisioningOutcome_RateLimitWithoutOrderStaysPending(t *testing.T) { + t.Parallel() + + certificate := &coredata.Certificate{ + Status: coredata.CertificateStatusPending, + SSLRetryCount: 1, + } + + outcome := decideProvisioningOutcome(certificate, ProvisioningErrorACMERateLimited) + + require.Equal(t, coredata.CertificateStatusPending, outcome.status) + assert.Equal(t, 1, outcome.retryCount) + assert.False(t, outcome.clearACMEState) +} diff --git a/pkg/certmanager/provisioning_error.go b/pkg/certmanager/provisioning_error.go new file mode 100644 index 000000000..dda6ae6cb --- /dev/null +++ b/pkg/certmanager/provisioning_error.go @@ -0,0 +1,69 @@ +// Copyright (c) 2025-2026 Probo Inc . +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +// SOFTWARE. + +package certmanager + +import ( + "errors" + "strings" +) + +const ( + ProvisioningErrorDNSCNAME = "DNS_CNAME" + ProvisioningErrorDNSCAA = "DNS_CAA" + ProvisioningErrorACMERateLimited = "ACME_RATE_LIMITED" + ProvisioningErrorACMEInvalidOrder = "ACME_INVALID_ORDER" + ProvisioningErrorACMETemporary = "ACME_TEMPORARY" + ProvisioningErrorACMEFailed = "ACME_FAILED" +) + +func classifyProvisioningError(err error) string { + if err == nil { + return "" + } + + if errors.Is(err, ErrACMERateLimited) { + return ProvisioningErrorACMERateLimited + } + + if errors.Is(err, ErrOrderInvalid) { + return ProvisioningErrorACMEInvalidOrder + } + + msg := strings.ToLower(err.Error()) + switch { + case strings.Contains(msg, "cname"): + return ProvisioningErrorDNSCNAME + case strings.Contains(msg, "caa record"): + return ProvisioningErrorDNSCAA + case strings.Contains(msg, "status: invalid"), strings.Contains(msg, "order is in unexpected status \"invalid\""): + return ProvisioningErrorACMEInvalidOrder + default: + return ProvisioningErrorACMETemporary + } +} + +func provisioningErrorCodePtr(code string) *string { + if code == "" { + return nil + } + + return &code +} diff --git a/pkg/coredata/certificate.go b/pkg/coredata/certificate.go index fadb130a4..07aca1e8c 100644 --- a/pkg/coredata/certificate.go +++ b/pkg/coredata/certificate.go @@ -242,6 +242,66 @@ FOR UPDATE SKIP LOCKED return nil } +// LoadByIDForUpdate locks the row with a blocking FOR UPDATE. Prefer this for +// write-backs that must not be dropped (e.g. persisting a freshly issued cert). +// Use LoadByIDForUpdateSkipLocked when skipping a locked row is acceptable. +func (c *Certificate) LoadByIDForUpdate( + ctx context.Context, + conn pg.Tx, + scope Scoper, + certificateID gid.GID, +) error { + q := ` +SELECT + id, + hostname, + http_challenge_token, + http_challenge_key_auth, + http_challenge_url, + http_order_url, + ssl_certificate, + encrypted_ssl_private_key, + ssl_certificate_chain, + status, + ssl_expires_at, + ssl_retry_count, + ssl_last_attempt_at, + provisioning_error, + created_at, + updated_at +FROM + certificates +WHERE + %s + AND id = @id +LIMIT 1 +FOR UPDATE +` + + q = fmt.Sprintf(q, scope.SQLFragment()) + + args := pgx.NamedArgs{"id": certificateID} + maps.Copy(args, scope.SQLArguments()) + + rows, err := conn.Query(ctx, q, args) + if err != nil { + return fmt.Errorf("cannot query certificate for update: %w", err) + } + + certificate, err := pgx.CollectExactlyOneRow(rows, pgx.RowToStructByName[Certificate]) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return ErrResourceNotFound + } + + return fmt.Errorf("cannot collect certificate: %w", err) + } + + *c = certificate + + return nil +} + func (c *Certificate) LoadByHostname( ctx context.Context, conn pg.Querier, @@ -772,7 +832,7 @@ FROM WHERE %s AND ( - (status IN (@provisioning_status, @renewing_status) AND updated_at < CURRENT_TIMESTAMP - INTERVAL '4 hours') + (status IN (@provisioning_status, @renewing_status) AND updated_at < CURRENT_TIMESTAMP - INTERVAL '10 minutes') OR (ssl_retry_count > 0 AND ssl_last_attempt_at < CURRENT_TIMESTAMP - INTERVAL '24 hours') ) @@ -809,6 +869,11 @@ func (c *Certificate) LoadNextForProvisioningForUpdateSkipLocked( ctx context.Context, tx pg.Tx, ) error { + // PROVISIONING rows with an open order poll every ~30s. Pending/Renewing + // rows (and provisioning rows without an order) use exponential backoff + // from ssl_last_attempt_at: 15m * 2^min(retry,5). Ordinary failures only + // reach retry counts 0–2 before FAILED; higher exponents are unused by the + // current failure budget but keep the SQL ceiling defensive. q := ` SELECT id, @@ -831,6 +896,23 @@ FROM certificates WHERE status = ANY(@statuses) + AND ( + ssl_last_attempt_at IS NULL + OR ( + status = @provisioning_status + AND http_order_url IS NOT NULL + AND ssl_last_attempt_at < CURRENT_TIMESTAMP - INTERVAL '30 seconds' + ) + OR ( + NOT ( + status = @provisioning_status + AND http_order_url IS NOT NULL + ) + AND ssl_last_attempt_at < CURRENT_TIMESTAMP - ( + INTERVAL '15 minutes' * (POWER(2, LEAST(ssl_retry_count, 5))::int) + ) + ) + ) ORDER BY updated_at ASC LIMIT 1 @@ -846,6 +928,7 @@ FOR UPDATE SKIP LOCKED string(CertificateStatusProvisioning), string(CertificateStatusRenewing), }, + "provisioning_status": string(CertificateStatusProvisioning), }, ) if err != nil { diff --git a/pkg/probod/probod.go b/pkg/probod/probod.go index 98f7e75b9..35dc861bd 100644 --- a/pkg/probod/probod.go +++ b/pkg/probod/probod.go @@ -524,6 +524,7 @@ func (impl *Implm) Run( accountKey, rootCAs, l, + r, ) if err != nil { return fmt.Errorf("cannot initialize ACME service: %w", err)