add custom DNS Resolver configuration

[vyzo]

No description provided.

[Stebalien]

Do we need a default resolver? I'd assume that we could have a single "." entry in a Resolvers map, right?

Also, let's briefly (very briefly) document the format (i.e., URL? Multiaddr?).

[aschmahmann]

note: some documentation in code can happen here, but the user facing docs live in go-ipfs https://github.com/ipfs/go-ipfs/blob/master/docs/config.md

[lidel]

+1 on removing DefaultResolver and renaming CustomResolvers to simple Resolvers

Don't skip in JSON

We should avoid json:",omitempty" here.
It is better for discoverability to have empty DNS.Resolvers (ok to have {}) – that is what we already do in Pinning.RemoteServices.

Values

We could support multiaddrs and URLs, but for DoH URL will be what people want to copy&paste while setting it up, so ok with saying this needs to be https:// URL with DoH endpoint.

Keys

I was thinking about interop with wider DNS ecosystem here and suggest we don't invent any new notation with * as I initially syggested, but follow the existing DNS convention of using the explicit FQDN syntax in our config files.

To illustrate, a custom DoH-only config for multiple resolvers could look like this::

  "DNS": {
    "Resolvers": {
      "eth.": "https://different-ens.example.net/dns-query",
      "crypto.": "https://unstoppablesomething.example.com/dns-query",
      "libre.": "https://ns1.iriseden.fr/dns-query", 
      ".": "https://doh-ch.blahdns.com:4443/dns-query"
    }
  }

Idea here is to closely follow the format of zone files, which should make things easier to understand. (The DNS root is unnamed, expressed as the empty label terminated by the dot.)

We should enforce keys are explicit FQDNs by validating them with dns.IsFqdn – removes surface for DNS ambiguity.

[vyzo]

ok fair enough; we can call the field just Resolvers.

[vyzo]

I will also make some minor changes in the madns Resolver to normalize to fqdns.

[vyzo]

In principle we could, but it seems more user friendly to have an explicit default.

…

On Mon, Apr 12, 2021, 23:20 Steven Allen ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In dns.go <#126 (comment)>: > + DefaultResolver string `json:",omitempty"` + // CustomResolvers is a map of domains to URLs for custom DoH resolution. + CustomResolvers map[string]string `json:",omitempty"` Do we *need* a default resolver? I'd assume that we could have a single "." entry in a Resolvers map, right? Also, let's briefly (very briefly) document the format (i.e., URL? Multiaddr?). — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#126 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAI4SXYPWOCWSG4M6QDR33TINIZFANCNFSM42ZBMCIA> .

[vyzo]

and sure, i'll add some documentation.

…

On Mon, Apr 12, 2021, 23:20 Steven Allen ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In dns.go <#126 (comment)>: > + DefaultResolver string `json:",omitempty"` + // CustomResolvers is a map of domains to URLs for custom DoH resolution. + CustomResolvers map[string]string `json:",omitempty"` Do we *need* a default resolver? I'd assume that we could have a single "." entry in a Resolvers map, right? Also, let's briefly (very briefly) document the format (i.e., URL? Multiaddr?). — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#126 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAI4SXYPWOCWSG4M6QDR33TINIZFANCNFSM42ZBMCIA> .

[Stebalien]

Both SGTM.

[lidel]

Some asks in comment inline.

Rationale: I'd like to avoid inventing custom syntax in keys and concepts, if we can.
DNS ecosystem and tooling already has pre-existing concepts like FQDN and configuration in form of Zone files, we should follow what exists where possible to improve onboarding.

[lidel]

+1 on removing DefaultResolver and renaming CustomResolvers to simple Resolvers

Don't skip in JSON

We should avoid json:",omitempty" here.
It is better for discoverability to have empty DNS.Resolvers (ok to have {}) – that is what we already do in Pinning.RemoteServices.

Values

We could support multiaddrs and URLs, but for DoH URL will be what people want to copy&paste while setting it up, so ok with saying this needs to be https:// URL with DoH endpoint.

Keys

I was thinking about interop with wider DNS ecosystem here and suggest we don't invent any new notation with * as I initially syggested, but follow the existing DNS convention of using the explicit FQDN syntax in our config files.

To illustrate, a custom DoH-only config for multiple resolvers could look like this::

  "DNS": {
    "Resolvers": {
      "eth.": "https://different-ens.example.net/dns-query",
      "crypto.": "https://unstoppablesomething.example.com/dns-query",
      "libre.": "https://ns1.iriseden.fr/dns-query", 
      ".": "https://doh-ch.blahdns.com:4443/dns-query"
    }
  }

Idea here is to closely follow the format of zone files, which should make things easier to understand. (The DNS root is unnamed, expressed as the empty label terminated by the dot.)

We should enforce keys are explicit FQDNs by validating them with dns.IsFqdn – removes surface for DNS ambiguity.

[lidel]

LGTM, but please update the comment.
It is a good opportunity to explain the purpose of this field to a drive-by contributor
(devs don't read docs, but they do read comments :))