diff --git a/pkg/certmanager/acme.go b/pkg/certmanager/acme.go index fe2e828fb..72753ecc6 100644 --- a/pkg/certmanager/acme.go +++ b/pkg/certmanager/acme.go @@ -78,6 +78,8 @@ const ( OrderPollStatusInvalid OrderPollStatus = "invalid" ) +const certificateURLPollInterval = 2 * time.Second + type OrderPollResult struct { Status OrderPollStatus Order *acme.Order @@ -435,7 +437,14 @@ func (s *ACMEService) fetchOrderCertificateAfterFinalize( } if refreshed.Status != acme.StatusValid { - return nil, fmt.Errorf("%w: order status %q after finalize", ErrOrderNotReady, refreshed.Status) + // Keep finalizeErr in the chain so a rate-limited finalize still reaches + // handleError and triggers the ACME cooldown. + return nil, fmt.Errorf( + "%w: order status %q after finalize: %w", + ErrOrderNotReady, + refreshed.Status, + finalizeErr, + ) } der, err := s.fetchOrderCertificate(ctx, orderURL, refreshed) @@ -473,20 +482,29 @@ func (s *ACMEService) refreshOrderCertificateURL( return order, nil } - refreshed, err := s.client.GetOrder(ctx, orderURL) - if err != nil { - return nil, fmt.Errorf("cannot refresh order: %w", err) - } + for { + 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.CertURL != "" { + return refreshed, nil + } - if refreshed.Status != acme.StatusValid { - return nil, fmt.Errorf("%w: order left valid state while waiting for certificate URL", ErrOrderNotReady) - } + 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) + // Order is VALID but CertURL not published yet; keep polling within this + // tick so we fetch the certificate while we still hold the key that + // finalized it, rather than abandoning the order next tick. + select { + case <-ctx.Done(): + return nil, fmt.Errorf("%w: certificate URL not yet available: %w", ErrOrderNotReady, ctx.Err()) + case <-time.After(certificateURLPollInterval): + } + } } func createCSR(domain string, key crypto.Signer) ([]byte, error) { diff --git a/pkg/certmanager/provision_worker.go b/pkg/certmanager/provision_worker.go index cb2aefe92..a0600f5eb 100644 --- a/pkg/certmanager/provision_worker.go +++ b/pkg/certmanager/provision_worker.go @@ -335,6 +335,20 @@ func (h *provisionHandler) processPollOrder( switch poll.Status { case OrderPollStatusNotReady: + // Re-accept best-effort: if a prior tick committed the challenge but its + // Accept never reached the CA, the order would otherwise stay pending + // forever since this path only polls. Re-accepting a pending challenge + // starts validation; one already processing is a no-op at the CA. + if err := h.acmeService.AcceptHTTPChallenge(ctx, challenge); err != nil { + h.logger.WarnCtx( + ctx, + "re-accepting HTTP challenge for not-ready order failed, will poll again", + log.String("hostname", certificate.Hostname), + log.String("certificate_id", certificate.ID.String()), + log.Error(err), + ) + } + h.logger.InfoCtx( ctx, "ACME order not ready yet, will poll again", @@ -531,11 +545,19 @@ func (h *provisionHandler) persistFailure( outcome := decideProvisioningOutcome(row, errorCode) row.ProvisioningError = provisioningErrorCodePtr(outcome.errorCode) - now := time.Now() - row.SSLLastAttemptAt = &now row.Status = outcome.status row.SSLRetryCount = outcome.retryCount + if isImmediateRetryRateLimit(provisionErr) { + // Retry-After: 0 permits an immediate retry; clearing the + // timestamp exempts the row from the claim query's backoff gates + // that would otherwise hold it despite the zero cooldown. + row.SSLLastAttemptAt = nil + } else { + now := time.Now() + row.SSLLastAttemptAt = &now + } + if outcome.clearACMEState { row.HTTPChallengeToken = nil row.HTTPChallengeKeyAuth = nil @@ -552,6 +574,17 @@ func (h *provisionHandler) persistFailure( ) } +// isImmediateRetryRateLimit reports whether err is an ACME rate limit with an +// explicit Retry-After of zero, i.e. the CA permits an immediate retry. +func isImmediateRetryRateLimit(err error) bool { + acmeErr, ok := errors.AsType[*ACMEError](err) + if !ok { + return false + } + + return errors.Is(acmeErr, ErrACMERateLimited) && acmeErr.RetryAfter() == 0 +} + // decideProvisioningOutcome computes status/retry/clear policy for a failed step. // // Two backoff regimes: diff --git a/pkg/certmanager/service.go b/pkg/certmanager/service.go index 15a2eeeef..cc7ca56d1 100644 --- a/pkg/certmanager/service.go +++ b/pkg/certmanager/service.go @@ -141,42 +141,6 @@ func (s *Service) EnsureCertificate( return certificate, nil } -// UpdateHostname renames a certificate and resets its lifecycle so a fresh -// certificate is provisioned for the new hostname. It returns the certificate -// unchanged when the hostname already matches. -func (s *Service) UpdateHostname( - ctx context.Context, - tx pg.Tx, - scope coredata.Scoper, - certificateID gid.GID, - newHostname string, -) (*coredata.Certificate, error) { - certificate := &coredata.Certificate{} - if err := certificate.LoadByID(ctx, tx, scope, certificateID); err != nil { - return nil, fmt.Errorf("cannot load certificate: %w", err) - } - - if certificate.Hostname == newHostname { - return certificate, nil - } - - certificate.Hostname = newHostname - certificate.Status = coredata.CertificateStatusPending - certificate.SSLRetryCount = 0 - certificate.SSLLastAttemptAt = nil - certificate.ProvisioningError = nil - certificate.HTTPChallengeToken = nil - certificate.HTTPChallengeKeyAuth = nil - certificate.HTTPChallengeURL = nil - certificate.HTTPOrderURL = nil - - if err := certificate.Update(ctx, tx, scope); err != nil { - return nil, fmt.Errorf("cannot update certificate: %w", err) - } - - return certificate, nil -} - // Get returns a certificate by ID. func (s *Service) Get( ctx context.Context,