feat: create DNS library #141
Merged
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
jyyi1
reviewed
Dec 18, 2023
There was a problem hiding this comment.
Exchange sounds good. Or we can use a name similar to net.Resolver, like Lookup()?
fortuna
commented
Dec 18, 2023
dns/roundtrip.go
Outdated
| // RoundTripper is an interface representing the ability to execute a | ||
| // single DNS transaction, obtaining the Response for a given Request. | ||
| // This abstraction helps hide the underlying transport protocol. | ||
| type RoundTripper interface { |
There was a problem hiding this comment.
I ended up settling for Resolver and Query, since that's the nomenclature that is predominantly used in the standards (example: https://datatracker.ietf.org/doc/html/rfc1034)
fortuna
commented
Dec 19, 2023
fortuna
force-pushed
the
fortuna-dns-clean
branch
from
be23071 to
51e55dd
Compare
fortuna
force-pushed
the
fortuna-dns-clean
branch
from
6ac3c5f to
df90c43
Compare
jyyi1
reviewed
Jan 3, 2024
dns/resolver_test.go
Outdated
| Header: dnsmessage.ResourceHeader{ | ||
| Name: dnsmessage.MustNewName("."), | ||
| Type: dnsmessage.TypeOPT, | ||
| Class: maxDNSPacketSize, |
There was a problem hiding this comment.
Suggested change
| Class: maxDNSPacketSize, | |
| Class: dnsmessage.ClassINET, |
There was a problem hiding this comment.
Yes. The OPT RR is a fake record that overloads the unused fields for common attributes. See https://datatracker.ietf.org/doc/html/rfc6891#section-6.1.2.
I added a comment.
jyyi1
reviewed
Jan 4, 2024
jyyi1
approved these changes
Jan 9, 2024
Comment on lines
+27
to
+37
| The Outline SDK is built upon a simple basic concepts, defined as interoperable interfaces that allow for composition and easy reuse. | ||
|
|
||
| **Connections** enable communication between two endpoints over an abstract transport. There are two types of connections: | ||
| - `transport.StreamConn`: stream-based connection, like TCP and the `SOCK_STREAM` Posix socket type. | ||
| - `transport.PacketConn`: datagram-based connection, like UDP and the `SOCK_DGRAM` Posix socket type. We use "Packet" instead of "Datagram" because that is the convention in the Go standard library. | ||
|
|
||
| Connections can be wrapped to create nested connections over a new transport. For example, a `StreamConn` could be over TCP, over TLS over TCP, over HTTP over TLS over TCP, over QUIC, among oter options. | ||
|
|
||
| **Dialers** enable the creation of connections given a host:port address while encapsulating the underlying transport or proxy protocol. The `StreamDialer` and `PacketDialer` types create `StreamConn` and `PacketConn` connections, respectively, given an address. Dialers can also be nested. For example, a TLS Stream Dialer can use a TCP dialer to create a `StreamConn` backed by a TCP connection, then create a TLS `StreamConn` backed by the TCP `StreamConn`. A SOCKS5-over-TLS Dialer could use the TLS Dialer to create the TLS `StreamConn` to the proxy before doing the SOCKS5 connection to the target address. | ||
|
|
||
| **Resolvers** (`dns.Resolver`) enable the answering of DNS questions while encapsulating the underlying algorithm or protocol. Resolvers are primarily used to map domain names to IP addresses. |
There was a problem hiding this comment.
These lines can also be the package documentation of transport.
There was a problem hiding this comment.
I agree. I think we need it in both. I'll leave the revamp of the transport Go doc to another PR.
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.
This PR introduces a library for DNS, and the concept of a DNS RoundTripper (similar to http.RoundTripper), to facilitate the exchange of DNS messages of various protocols.
It establishes the base for other functionality, such as the use in Dialers, caching and strategy selection.
I still need to add tests, but I believe it's in good shape to start a review.
I'm still not convinced about the name. I borrowed
RoundTripperfrom the http library, but it's kind of an unusual name. It's particularly bad as a verb. Some other options that may be better for the function:We could also add a DNS prefix like ExchangeDNS, AskDNS, ...
The DoH RFC uses both Exchange and Request. I'm inclined to use Exchange, since it clearly conveys there's a message back from the server, and it's a phrase that is in common use. Answer or Ask seems appropriate too, given we pass a question.
Any thoughts?