Poll the ACME order URL after finalize instead of the Location header

[wallrj-cyberark]

Pull Request Motivation

ACME e2e tests fail intermittently on master with:

Failed to finalize Order: 403 urn:ietf:params:acme:error:orderNotReady: Order's status ("processing") was not ready

For example, ci-cert-manager-master-e2e-v1-35 on 2026-10-05.

The cause is in our forked x/crypto/acme. After finalizing, CreateOrderCert polls the order at the URL in the finalize response's Location header. RFC 8555 does not require that header, and Pebble v2.9.0+ omits it. So every finalize in e2e fails with Post "": unsupported protocol scheme "", and the Order is requeued. Even a passing run logs this: ci-cert-manager-master-e2e-v1-37 today has 206 of these lines in the controller log.

Usually the retry sees the order as valid and recovers. Sometimes the controller finalizes the same order a second time while Pebble is still issuing. Pebble answers 403 because the order is processing, and finalizeOrder only recovers from a 403 when the order is valid, so it marks the Order as errored.

golang/crypto fixed this in b4dcfb54b (golang/go#77704) by adding CreateCertFromOrder, which polls the order's own URI. This PR:

• re-vendors the forked acme package from acme: support standards-compliant order finalization (cherry-pick b4dcfb54b) crypto#9, which cherry-picks b4dcfb54b. That PR has merged with a merge commit, so the pinned hash is reachable from its master.
• switches the Order controller to CreateCertFromOrder, passing the URL already stored in the Order status.

#8968 handles the processing 403 in finalizeOrder for a different trigger, where Let's Encrypt shares one order between two Order resources (#8960). It is still needed after this PR: if the controller restarts while it waits for a finalized order, the next sync finalizes again and gets the same 403. This PR makes the double finalize rarer, but does not remove it.

I would like to cherry-pick this to release-1.21 once merged, because the 1.21 e2e runs the same Pebble.

How this was tested

• make verify-golangci-lint: 0 issues.
• go test ./pkg/controller/acmeorders/... ./pkg/acme/... ./third_party/forked/acme/... passes. The forked package's TestWithPebble now runs against Pebble v2.10.1, which omits the header.
• The happy-path finalize case in sync_test.go now checks that the controller passes status.url and status.finalizeURL. With the URI dropped from the call, it fails with Invalid order: expected URI "http://testurl.com/abcde" ... got "".
• In the cert-manager/crypto fork, TestWithPebble against Pebble v2.10.1 fails on master with Post "": unsupported protocol scheme "" and passes with the cherry-pick.
• After this merges, the e2e controller logs should contain no unsupported protocol scheme lines. I will check this PR's e2e artifacts for that.

Why the vendored files change

The files under third_party/forked/acme were produced by make update-third-party after bumping repo_hash in third_party/klone.yaml. They match golang/crypto b4dcfb54b except for autocert, which we do not vendor. The metrics label stays create_order_cert, so existing dashboards keep working.

Kind

/kind bug
/kind flake

Release Note

Fixed ACME order finalization against servers that omit the `Location` header from the finalize response, such as Pebble v2.9.0 and later. Previously each finalization failed once and was retried, and the retry could mark the Order as failed.
The `CreateOrderCert` method of the ACME client interface in `pkg/acme/client` is replaced by `CreateCertFromOrder`. Out-of-tree implementations of that interface must be updated.

[with Claude]

[cert-manager-prow]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[cert-manager-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign hjoshi123 for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[wallrj-cyberark]

Self-review. Everything under third_party/forked/acme is generated by make update-third-party, so the only hand-written changes are in pkg/, test/integration/ and klone.yaml.

[wallrj-cyberark]

CreateCertFromOrder only reads URI and FinalizeURL from the order, so a partial acme.Order built from the Order status is enough. It avoids an extra GetOrder round trip before finalizing. Both fields are always set by this point: the Sync switch returns early while status.url or status.finalizeURL is empty.

[wallrj-cyberark]

The follow-up that moves the forked acme package onto upstream's new ACME profiles support is #9431. It is master-only and stacked on this PR, so this one stays small enough to cherry-pick to release-1.21.

[Copilot]

🟢 Approval recommended

The implementation consistently removes reliance on the optional header and includes targeted unit and Pebble coverage.

0 open findings

What changed in this PR

Updates ACME order finalization to poll the persisted order URL instead of relying on an optional Location header.

Changes:

• Vendors CreateCertFromOrder from the crypto fork.
• Updates controller and client interfaces to use the order and finalize URLs.
• Adds unit and Pebble coverage for missing finalize Location headers.

The pinned crypto fork PR remains an acknowledged merge prerequisite.

File	Description
third_party/​klone.yaml	Updates the crypto fork revision.
third_party/​forked/​acme/​types.go	Updates finalization documentation.
third_party/​forked/​acme/​rfc8555.go	Adds order-aware certificate creation and polling.
third_party/​forked/​acme/​rfc8555_test.go	Tests header-independent finalization.
third_party/​forked/​acme/​pebble_test.go	Updates Pebble and exercises the new API.
third_party/​forked/​acme/​internal/​acmeprobe/​prober.go	Migrates probe finalization calls.
third_party/​forked/​acme/​acme.go	Updates deprecated API guidance.
test/​integration/​acme/​orders_controller_test.go	Updates the integration fake.
pkg/​controller/​acmeorders/​sync.go	Passes persisted order and finalize URLs.
pkg/​controller/​acmeorders/​sync_test.go	Verifies both URLs are forwarded.
pkg/​acme/​client/​middleware/​logger.go	Wraps and labels the new operation.
pkg/​acme/​client/​interfaces.go	Updates the ACME client contract.
pkg/​acme/​client/​fake.go	Updates the test client implementation.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[wallrj]

I checked the vendored hunks against golang/crypto b4dcfb54b. They are byte-identical, with autocert dropped as intended. The fake, the logger middleware and the real client are the only implementations of acmecl.Interface in this repo, and all three are updated. The e2e Pebble fork is ahead of v2.9.0, so it does omit the Location header as described.

Two things outside the diff:

1. The workaround comment and its TODO(wallrj) in updateOrderStatusFromACMEOrder describe the bug this PR vendors the fix for:

   cert-manager/pkg/controller/acmeorders/sync.go

   Lines 359 to 366 in 63225e7

   	// Workaround bug in golang.org/x/crypto/acme implementation whereby the
   	// order's URI field will be empty when calling GetOrder due to the
   	// 'Location' header not being set on the response from the ACME server.
   	//
   	// TODO(wallrj): We have vendored golang.org/x/crypto/acme so there's
   	// nothing stopping us fixing this bug.
   	if acmeOrder.URI != "" {
   	o.Status.URL = acmeOrder.URI

   . The vendored GetOrder and WaitOrder now fill o.URI from the request URL, so the if acmeOrder.URI != "" guard is dead for the real client. Please delete the comment and the guard, or reword it to say why the guard stays.
2. The commit trailer and the last line of the description should name the model: Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> and [Claude Fable 5.1].

The inline comments below are follow-ups rather than blockers.

[Claude Fable 5.1]

[wallrj-cyberark]

Replies to the two points in the review body:

1. The vendored GetOrder now falls back to the request URL, so the old workaround comment and its TODO were wrong. In d1a17a4 I replaced them with a comment saying why the guard stays: URI can still be empty from other Interface implementations, such as test fakes. I kept the guard rather than deleting it, so the change in behaviour stays as small as possible for the release-1.21 cherry-pick.
2. I left the attribution as it is. The trailer and description footer follow the format set by the tool that wrote this commit, and Claude Fable 5.1 would name a different model from the one that wrote it.

[with Claude]