[Freeipa-devel] [PATCH 0064-0065] ipa-dns-install offers IP addresses from resolv.conf as default forwarder

[Jan Cholasta]

On 11.11.2015 09:27, Martin Babinsky wrote:
> On 11/11/2015 08:12 AM, Jan Cholasta wrote:
>> On 10.11.2015 16:58, Petr Spacek wrote:
>>> Hello,
>>>
>>> Patch 64:
>>> ipa-dns-install offer IP addresses from resolv.conf as default
>>> forwarders
>>>
>>> In non-interactive more option --auto-forwarders can be used to do the
>>> same. --forward option can be used to supply additional IP addresses.
>>>
>>> https://fedorahosted.org/freeipa/ticket/5438
>>
>> IMO it's perverse to add option which effectively means "use default
>> value" instead of actually using the value as default. This is
>> inconsistent with every other option and I don't see what makes
>> forwarders so special to require this.
>>
>> NACK unless you have a strong justification for this.
>>
> Is it possible to use default_getter decorator to fetch defaults for the
> 'forwarders' knob from the resolver if it is avaliable like so (warning:
> untested and possibly wrong)?

Yes, this is exactly how it should be used (although the exception 
handling could be better).

>
> """
> @@ -160,20 +162,27 @@ class BaseServerCA(common.Installable, core.Group,
> core.Composite):
>   class BaseServerDNS(common.Installable, core.Group, core.Composite):
>       description = "DNS"
>
>       forwarders = Knob(
>           (list, 'ip'), None,
>           description=("Add a DNS forwarder. This option can be used
> multiple "
>                        "times"),
>           cli_name='forwarder',
>       )
>
> +    @forwarders.default_getter
> +    def forwarders(self):
> +        try:
> +            return resolver.get_default_resolver().nameservers
> +        except Exception:
> +            return None
> +
>       no_forwarders = Knob(
>           bool, False,
>           description="Do not add any DNS forwarders, use root servers
> instead",
>       )
>
>       reverse_zones = Knob(
>           (list, str), [],
>           description=("The reverse DNS zone to use. This option can be
> used "
>
> """
>
>>>
>>>
>>> Patch 65:
>>> Remove global variable dns_forwarders from ipaserver.install.dns
>>> It seems to me that the global thingy is not necessary, so I've ripped
>>> it out.
>>
>> ACK.
>>
>
>


-- 
Jan Cholasta