refactor(client): clean-up dns01 request code #23
Conversation
This file contains 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
lidel
changed the title
removed duplicated/unused code, added logs
lidel
force-pushed
the
refactor-client-request
branch
from
7e83663 to
af85ef7
Compare
aschmahmann
reviewed
Dec 19, 2024
| // Construction of the request requires a list of multiaddresses that the peerID is listening on using | ||
| // publicly reachable IP addresses. | ||
| // | ||
| // Sending the request to the DNS server requires performing HTTP PeerID Authentication for the corresponding peerID | ||
| func ChallengeRequest(ctx context.Context, baseURL string, challenge string, addrs []multiaddr.Multiaddr) (*http.Request, error) { | ||
| func ChallengeRequest(ctx context.Context, registrationURL string, challenge string, addrs []multiaddr.Multiaddr) (*http.Request, error) { |
There was a problem hiding this comment.
@lidel the reason why the test is now failing is that you changed the semantics to this function which is the one called by the test (i.e. there is now a 404 here because adding "/v1/_acme-challenge" was removed. So the test needs modification also (e.g. to the changed ChallengeRequest semantics or perhaps to just use SendChallenge now).
It might also be worth considering if both of these functions need exporting or not.
There was a problem hiding this comment.
Thanks!
Let's keep them, I feel they may be useful in testing, solving the problem of the same code in 2-3 places, like in was in our case before this PR.
|
CI green, merging to include in Kubo 0.33.0-rc1 |
lidel
added a commit
to ipfs/kubo
that referenced
this pull request
Dec 19, 2024
main commit with ipshipyard/p2p-forge#23
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Client code was really bad when things went wrong. p2p-forge looked like error returned by Let's Encrypt, and we don't want users bothering them with problems caused by our DNS TXT record broker.
e2e_test.go:250: failed to run handshake: signature not set) must be something obvious, but I don't see it yet. Created chore: go-libp2p@37fbd7 #24 to confirm it is not a problem with my local setup. Ideas welcome.