libre-server / rolekit

'rolekit' is a daemon for Linux systems providing a stable D-BUS interface to manage the deployment of ​Server Roles.
19 stars 7 forks source link

domain controller role: cannot specify only ipv4 or ipv6 forwarders #64

Open AdamWill opened 8 years ago

AdamWill commented 8 years ago

I have a situation where I want to specify the DNS forwarders for the domain controller role, but only IPv4 forwarders (there is no IPv6 connectivity).

rolekit will refuse point blank to accept a dns_forwarders dict with no ipv6 key.

rolekit will accept a dns_forwarders dict with an ipv6 list containing valid IPv6 servers, but the deployment will fail because none of them can be contacted.

rolekit will accept a dns_forwarders dict with an empty ipv6 list, and sometimes this even works, but quite often it fails with an error message "Unable to guess signature from an empty list", which appears to be a ValueError that can be raised by python-dbus in some circumstances.

Fortunately I found a loophole: rolekit does not actually check that the values in the ipv4 list are IPv4 addresses and the values in the ipv6 list are IPv6 addresses. In fact it doesn't check anything about them other than that they're strings. It just turns the items in both lists into --forwarder args for ipa-server-install (which leads me to wonder why there are two separate lists in the first place). So I can work around this by just sticking IPv4 servers in the ipv6 list. But it certainly seems to be a bug, there is no reason to require both lists to be present and non-empty.

sgallagher commented 8 years ago

The fact that it's a dict is a historical curiosity and one I will scrap if-and-when we get to 1.0. The reason that IPv4 and IPv6 is specified independently is so that they can have independent input validation (which is something not yet implemented).

That said, the fact that they cannot be left empty is a clear bug and should be fixed. Thanks for spotting this.

sgallagher commented 8 years ago

@AdamWill Would you mind doing a quick review of https://reviewboard-fedoraserver.rhcloud.com/r/245/ please?

AdamWill commented 8 years ago

seems reasonable. you can do this a bit more compactly though:

if ('ipv4' in values['dns_forwarders'] and len(values['dns_forwarders']['ipv4']) > 0):

like this:

if len(values['dns_forwarders'].get('ipv4', [])) > 0:

sgallagher commented 8 years ago

Ah, that is a useful trick.

sgallagher commented 8 years ago

I updated the review request. You can see the interdiff here: https://reviewboard-fedoraserver.rhcloud.com/r/245/diff/1-2/

AdamWill commented 8 years ago

While I appreciate the tricksiness of sticking the ipa_install_args.append calls in a list like that, er, how about this? :)

forwarders = values['dns_forwarders'].get('ipv4', []) + values['dns_forwarders'].get('ipv6', [])
if forwarders:
    ipa_install_args.extend(["--forwarder=%s" % x for x in forwarders])
else:
    # Someone specified the forwarders explicitly but set no
    # values. This is an error.
    raise RolekitError(
        INVALID_VALUE,
        "dns_forwarders passed but contains no entries.")
AdamWill commented 8 years ago

BTW I don't think this will fix the dbus problem that sometimes happens when one of the values is an empty list, but of course it removes the reason I had to do that.

sgallagher commented 8 years ago

Yeah, the D-BUS thing is actually a limitation of python-dbus and a real pain in the neck. I'm not sure where exactly it needs to be fixed (in rolectl or in python-dbus). But for now, I'm going to ignore it.