feat(client): add WithHTTPClient option#87
Merged
Merged
Conversation
Test harnesses that run an in-process forge on a loopback address need to override resolution or trust a different CA for the registration endpoint, without losing the PeerID-auth signature scope. The new WithHTTPClient option lets callers thread a configured *http.Client through the DNS-01 challenge POST. SendChallenge keeps its existing signature and forwards to a new exported SendChallengeWithClient that takes the *http.Client; downstream callers are unaffected.
lidel
force-pushed
the
feat/with-http-client
branch
from
0faa47c to
430ae72
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #87 +/- ##
==========================================
+ Coverage 68.31% 69.05% +0.73%
==========================================
Files 21 21
Lines 2102 1703 -399
==========================================
- Hits 1436 1176 -260
+ Misses 548 409 -139
Partials 118 118 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Replace the SendChallengeWithClient variant with a trailing variadic opts ...SendChallengeOption on SendChallenge. Existing positional-only callers compile unchanged. The first option, WithChallengeHTTPClient, threads a caller-supplied *http.Client through the DNS-01 registration POST. Add tests covering option validation, the transport wiring through SendChallenge, and the positional-only backward-compat path.
|
Suggested tag:
Cutting a Release (and modifying code files)This PR is modifying both Automatically created GitHub ReleaseA draft GitHub Release has been created. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
This makes e2e test harness for autotls possible/simpler in
What was broken
Embedding the forge in a test harness needs a way to override DNS resolution or trust a different CA for the registration endpoint, without losing the PeerID-auth signature scope (which must stay tied to the production registration hostname).
client.SendChallengeandP2PForgeCertMgralways usedhttp.DefaultClient, so there was no seam to inject a customTransport, resolver, or root CA pool.How this PR fixes it
Adds two options that accept a
*http.Client:client.WithHTTPClient(*http.Client)onP2PForgeCertMgr.client.WithChallengeHTTPClient(*http.Client)forclient.SendChallenge, which gains a trailing variadicopts ...SendChallengeOption. Existing positional-only callers compile unchanged.Both options reject
nil. The supplied client is threaded throughhttppeeridauth.ClientPeerIDAuth, so PeerID auth signing wraps the caller'sTransport,CheckRedirect,Jar, andTimeout.