digicert: persist private key across pending orders

[jonathanvdwatt]

Summary

• Replace the hardcoded @retry(stop_max_attempt_number=10, wait_fixed=1000) on get_certificate_id with a configurable polling loop (default 300s timeout, 5s interval)
• When DigiCert doesn't issue within the polling window, save the cert as a PendingCertificate instead of crashing - the private key, CSR, and order ID are persisted to the database
• A background Celery task (fetch_digicert_cert) retries with exponential backoff (30s -> 600s cap) until the order completes or hits a terminal state
• Fail fast on terminal DigiCert states (rejected/revoked/canceled)
• Log each poll attempt so operators can see progress
• Error messages include the order ID and current state

Context

The old retry gave DigiCert ~10 seconds to issue a cert. Multi-SAN certs (50+ SANs) routinely take longer due to OV validation, DCV re-checks, or CAA issues on individual domains. When the retry exhausted, the worker crashed and the private key - generated in-memory - was lost. The DigiCert order would complete later, but with no matching key it was useless and could not be imported manually.

This follows the same pattern Lemur already uses for ACME certs: PendingCertificate + background resolution task.

Config

Optional keys in lemur.conf.py (defaults are fine for most cases):

• DIGICERT_ORDER_TIMEOUT: seconds for initial poll before creating a pending cert (default 300)
• DIGICERT_ORDER_POLL_INTERVAL: seconds between polls during initial window (default 5)
• DIGICERT_PENDING_MAX_ATTEMPTS: max background retries before giving up (default 100)

[gijzelaerr]

The PendingCertificate approach is useful, but I would address the findings below before merging: the new synchronous polling window can outlive the web worker before the key is saved, transient resolver errors are treated as terminal, and interrupted finalization has no recovery path.

Validation: reviewed commit 53a20c1 and the current deployment configuration. Nine focused diagnostics ran the unchanged function bodies with mocked external dependencies and reproduced the reported failure cases and happy paths. The normal DigiCert pytest module could not collect locally because Flask is missing; these diagnostics do not replace integration tests.

[gijzelaerr]

Re-reviewed 0e13612. Both original P1 findings are addressed: order creation returns immediately after acceptance, and transient status/download errors now retry rather than marking the pending record resolved. The original immediate-download key-loss gap is also removed from creation.

Recovery still needs the fixes below: the new sweep is not scheduled, finalization is not idempotent across retries/concurrent tasks, and finalization errors bypass the configured attempt limit.

Validation: 12 focused diagnostics executed the unchanged function bodies with mocked dependencies, covering acceptance, pending/issued/terminal states, transient failures, recovery dispatch, overlapping resolvers, failure after successful import, and attempt-limit handling. Changed Python files parse successfully. These diagnostics do not replace the normal Flask/PostgreSQL integration suite, which remains unavailable locally. No CI results are visible for this commit.

[gijzelaerr]

Re-reviewed 63d2d79. Finalization failures now consume the attempt budget and use backoff. The sweep's required beat entry is documented; adding that entry to lemur-deploy remains a deployment prerequisite. Its ten-minute stale threshold also avoids the earlier six-minute overlap case.

One blocking P2 finding remains: the new read-only finalization guard does not prevent concurrent imports or recover safely after import succeeds but pending-state persistence fails. Details and reproductions are in the inline comment.

Validation: 14 focused diagnostics ran the unchanged function bodies with mocked external dependencies. They confirm the bounded retries and earlier fixes, while still reproducing concurrent imports, duplicate import after a linkage-write failure, and an unfinished resolved flag after partial success. The changed Python file parses. The normal Flask/PostgreSQL suite remains unverified locally; the four CI build jobs are still in progress.

[gijzelaerr]

Rechecked the latest revision. The previous linkage-failure and partial-resolution cases now recover correctly, and the finalization attempt budget remains bounded. Eight focused diagnostics using the repository's pinned SQLAlchemy 1.3.24 and SQLite confirm those paths and reproduce the remaining unsafe-reuse cases below. These are isolated task-body checks, not the full Flask/PostgreSQL suite; they do not validate PostgreSQL concurrency. The recovery sweep still needs the documented deployment schedule. One P2 remains in deciding which existing certificate is safe to reuse.

CI has now completed: all four Python/Node matrix jobs fail test_issuer_plugin_create_certificate because it still asserts an immediate certificate body while create_certificate now correctly returns a pending order (None, None, order_id). Each job reports 1 failed, 914 passed, 18 skipped. Please update that stale contract test to assert deferral (including no polling/download during creation) and cover the issued result through resolve_pending_order. CI run: https://github.com/spotify/lemur/actions/runs/36112152105

[gijzelaerr]

Re-reviewed 04062eb. The authority filter prevents cross-authority reuse, the private-key guard avoids directly resolving to a keyless source import, and the updated tests pass in CI. One finalization issue remains at lemur/common/celery.py:470-475:

[P2] Find and verify the completed import before reusing it. The query takes .first() and only then checks existing.private_key. If a keyless source-import row is first for the same authority and external ID, a resolver can import the issued certificate, fail while writing resolved_cert_id, then find that same keyless row on retry and import a second copy. A focused ORM reproduction against this head produced 2 certificate rows after the first attempt and 3 after one retry. A separate reproduction showed that a keyed row with the same authority and external ID but a different body/key is reused and marks the pending order resolved, leaving its saved key unused. Please select a completed candidate and validate its certificate body and key against this pending order before reuse, then add regression coverage for these cases.

Validation: focused checks ran the current task body with the repository-pinned SQLAlchemy 1.3.24 and SQLite. They confirm the cross-authority and keyless direct-resolution fixes, and reproduce the remaining retry/identity issue. SQLite does not validate PostgreSQL row-lock concurrency. All four CI build jobs are green. The documented Celery beat schedule for the new recovery sweep remains a deployment prerequisite.

[gijzelaerr]

Re-reviewed 04062eb after the earlier requested changes. The delayed DigiCert order now persists the private key and resolves in the background; the retry and finalization changes address the core failure. All four CI builds are green.

I agree the remaining reuse case is narrow enough to handle as a follow-up, rather than hold up this fix. Please track a follow-up to select an existing import by authority and verified certificate identity, reconcile the saved private key and pending metadata before marking the order resolved, and cover retry after a partial import or a keyless source import with a regression test.

The new DigiCert recovery sweep still needs the documented Celery beat schedule when this is deployed. Approving this PR.