[libvirt] [PATCH] virNetworkDefUpdateDNSHost: Require both IP and a hostname to match

[W. Trevor King]

On Wed, Nov 07, 2018 at 11:17:51AM -0500, Laine Stump wrote:
> On 11/6/18 12:10 PM, W. Trevor King wrote:
> > But HOST records can have the same hostname for multiple records
> > (similar to TXT records with the same value).  The value that
> > needs to be distinct for HOST records is the IP address.
> 
> You're going to force me to go dig out the decades-old DNS RFCs,
> aren't you?

The relevant RFCs are probably [1,2].  But as far as they're concened
IP <-> domain-name mappings are many to many.  That doesn't really
impact libvirt's <dns><host/><dns> entries.  For example:

  IP           hostname
  192.168.1.1  alice
  192.168.1.1  bob
  192.168.1.2  alice

could be represented with unique IPs:

  <dns>
    <host ip="192.168.1.1">
      <hostname>alice</hostname>
      <hostname>bob</hostname>
    </host>
    <host ip="192.168.1.2">
      <hostname>alice</hostname>
    </host>
  </dns>

or with repeated IPs:

  <dns>
    <host ip="192.168.1.1">
      <hostname>alice</hostname>
    </host>
    <host ip="192.168.1.1">
      <hostname>bob</hostname>
    </host>
    <host ip="192.168.1.2">
      <hostname>alice</hostname>
    </host>
  </dns>

Depending on how generous you were feeling, you could also support
overlaps:

  <dns>
    <host ip="192.168.1.1">
      <hostname>alice</hostname>
      <hostname>bob</hostname>
    </host>
    <host ip="192.168.1.1">
      <hostname>bob</hostname>
    </host>
    <host ip="192.168.1.2">
      <hostname>alice</hostname>
    </host>
  </dns>

I know libvirt supports the unique-IP form.  I haven't looked to see
whether it supports the repeated IP form (with or without overlaps).

> (but in the meantime, if something was working before (e.g. removing
> an entry by hostname alone) then that needs to continue working,
> otherwise some application depending on that behavior will now be
> broken, and we've made promises about that not happening with
> libvirt.

I think continuing to support the IP/hostname-union logic makes things
too complicated.  Would you have to detect the "we match too many
things and would ordinarily error out" case so you could switch on the
"they must expect this to be an IP/hostname-intersection match" logic?
How would you feel about a v2 function (a la dup2), which callers
switch on with VIR_NETWORK_SECTION_DNS_HOST2 or something to opt in to
the new logic?  This would also give you a way to notice the old
section variable and log deprecation notices (does the libary have
hooks for logging?) to help get the word out, and help set the stage
for possibly removing the old logic after a few years.

> > 1. You can now delete entries from an existing network like:
> >
> >      <dns>
> >        <host ip="192.168.1.1">
> >          <hostname>example</hostname>
> >        </host>
> >        <host ip="192.168.1.2">
> >          <hostname>example</hostname>
> >        </host>
> >      </dns>
> >
> >   with input like:
> >
> >        <host ip="192.168.1.1">
> >        </host>
> >
> >   or:
> >
> >        <host ip="192.168.1.1">
> >          <hostname>example</hostname>
> >        </host>
> >
> >   Previously, only the former would work (the latter used to raise
> >   "multiple matching DNS HOST records were found in network").
> 
> 
> Without thinking too much about it, that (the "multiple matching
> ...."  error) sounds like a bug that can/should be fixed - since
> both hostname and ip find only a single record, there shouldn't be
> an error.

Both <hostname> elements in the original network are "example", so
they are not unique.  The current logic's IP/hostname-union logic
matches both, and errors out.  I don't think this is a bug we can fix
without something ugly like "if IP/hostname-union gives multiple
matches, implicitly switch to IP/hostname-intersection" or a way to
explicitly select the more-specific logic.

> > 3. You can now add multiple entries with a common hostname (as long as
> >    they have distinct IP addresses).  Previously, adding:
> >
> >      <host ip="192.168.1.1">
> >        <hostname>example</hostname>
> >      </host>
> >
> >    to an existing:
> >
> >      <host ip="192.168.1.2">
> >        <hostname>example</hostname>
> >      </host>
> >
> >    would have raised "there is already at least one DNS HOST record
> >    with a matching field in network".
> > ---
> >
> > I'm actually not clear on whether the 'ip' attribute is required
> > to be unique or not.
> 
> Well, *something* needs to be unique. Either one of the fields, or
> the combination of some fields. If possible, decisions about that
> should be based on the RFCs, and then if the backend implementation
> (dnsmasq in this case) is any different, that should be treated as a
> different kind of error.

We've had no trouble creating networks where multiple IP share a
common hostname in libvirt.  The only problem I'm aware of (and the
reason for this patch) is that libvirt doesn't allow you to *update*
those networks.  You currently have to delete them and then re-create
them, after which you need to re-attach your domains to the new
network.

Cheers,
Trevor

[1]: https://tools.ietf.org/html/rfc1035
[2]: https://tools.ietf.org/html/rfc1794

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy