Skip to content

fix: decouple challenge cleanup from canceled context - #404

Merged
mholt merged 1 commit into
caddyserver:masterfrom
ousamabenyounes:fix/issue-313
Sep 4, 2026
Merged

fix: decouple challenge cleanup from canceled context#404
mholt merged 1 commit into
caddyserver:masterfrom
ousamabenyounes:fix/issue-313

Conversation

@ousamabenyounes

@ousamabenyounes ousamabenyounes commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Run wrapped ACME solver cleanup with a context that preserves values but ignores issuance cancellation. Add a regression test covering cleanup after the obtaining context is canceled.

Test verification (RED → GREEN)

With the fix reverted, the new test fails (RED):

--- FAIL: TestSolverWrapperCleanUpUsesUncanceledContext (0.00s)
    solvers_test.go:42: cleanup context is canceled: context canceled
FAIL
FAIL	github.com/caddyserver/certmagic	0.005s
FAIL

With the fix applied, the test passes (GREEN):

ok  	github.com/caddyserver/certmagic	0.006s

Full local suite

Command: go test -v -short -race ./...

=== RUN   Test_challengeKey/ok/http-ipv4
=== RUN   Test_challengeKey/ok/http-ipv6
=== RUN   Test_challengeKey/ok/tls-ipv4
=== RUN   Test_challengeKey/ok/tls-ipv6
=== RUN   Test_challengeKey/fail/tls-ipv4
=== RUN   Test_challengeKey/fail/tls-ipv6
--- PASS: Test_challengeKey (0.00s)
    --- PASS: Test_challengeKey/ok/dns-dns (0.00s)
    --- PASS: Test_challengeKey/ok/http-dns (0.00s)
    --- PASS: Test_challengeKey/ok/tls-dns (0.00s)
    --- PASS: Test_challengeKey/ok/http-ipv4 (0.00s)
    --- PASS: Test_challengeKey/ok/http-ipv6 (0.00s)
    --- PASS: Test_challengeKey/ok/tls-ipv4 (0.00s)
    --- PASS: Test_challengeKey/ok/tls-ipv6 (0.00s)
    --- PASS: Test_challengeKey/fail/tls-ipv4 (0.00s)
    --- PASS: Test_challengeKey/fail/tls-ipv6 (0.00s)
=== RUN   TestDNSManagerCleanUpRecordPreservesProviderData
--- PASS: TestDNSManagerCleanUpRecordPreservesProviderData (0.00s)
=== RUN   TestGetACMEChallenge_IPv6Brackets
--- PASS: TestGetACMEChallenge_IPv6Brackets (0.00s)
=== RUN   TestTryListen
--- PASS: TestTryListen (0.10s)
=== RUN   TestPrefixAndKeyBuilders
--- PASS: TestPrefixAndKeyBuilders (0.01s)
=== RUN   TestFileStorageStoreLoad
--- PASS: TestFileStorageStoreLoad (0.00s)
=== RUN   TestFileStorageStoreLoadRace
--- PASS: TestFileStorageStoreLoadRace (0.04s)
=== RUN   TestFileStorageWriteLock
--- PASS: TestFileStorageWriteLock (0.00s)
PASS
ok  	github.com/caddyserver/certmagic	1.528s
=== RUN   TestFile
--- PASS: TestFile (0.00s)
=== RUN   TestConcurrentWrites
--- PASS: TestConcurrentWrites (0.00s)
PASS
ok  	github.com/caddyserver/certmagic/internal/atomicfile	(cached)
?   	github.com/caddyserver/certmagic/internal/filedescriptor	[no test files]
?   	github.com/caddyserver/certmagic/internal/testutil	[no test files]

Fix #313

@mholt mholt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This is probably a good change.

@mholt
mholt merged commit 31be911 into caddyserver:master Sep 4, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Decouple cleanup context from obtaining context

2 participants