LGTM

The Type field should be of type Type (say that quickly) rather than an untyped uint16. ResourceHeader already has:

type ResourceHeader struct {
...
    // Type is the type of DNS resource record.
    //
    // This field will be set automatically during packing.
    Type Type
...
}

It should also be possible to add an UnknownResource to a Builder, otherwise you can't parse and then reassemble a message containing an unknown RR using that API (only with Message?).

This is definitely a good addition though, miekg/dns (of which I contribute) has similar support for unknown RRs with a type RFC3597 struct that can be both packed and unpacked.

Thanks, Tom!

I've switched the uint16 to Type and added UnknownResource functions to Parser and Builder.

Here's the diff for those changes: https://go-review.googlesource.com/c/net/+/262037/6..8

It's been brought to my attention that this design might be a compatibility issue when adding support for new RR types.

Rather than adding an UnknownResource, we could add a Raw []byte field to ResourceHeader. When an RR is unparseable, the ResourceBody would just be nil.

Suppose a user of this API wants to support a new RR type, such as draft-ietf-dnsop-svcb-https-01. If the caller relies on UnknownResource to catch the new record, adding explicit support for the new record to dnsmessage would break the user's code.

If their code instead branched on Type == newRRType and read the ResourceHeader.Raw bytes, it would not be broken when dnsmessage adds support for the new RR type.

In my opinion, code that relies on UnknownResource like that should use the Parser. There is no compatibility guarantee for this package and the ResourceHeader.Raw solution will still likely break applications which don't follow best practices when adding RR types. I think the solution is to detail contracts and best practices in comments

Adding to minutes but it seems headed for likely accept.

Based on the discussion, this seems like a likely accept.

No change in consensus, so accepted.

I'm surprised this issue didn't get auto-closed yesterday when the change landed: golang/net@cd0ac97.

Whoops, the line in the CL should have been Fixes golang/go#42128.