[Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

[Martin Basti]

On 08/28/2015 10:03 AM, Petr Spacek wrote:
> On 27.8.2015 14:22, David Kupka wrote:
>> @@ -2101,11 +2101,25 @@ class DNSZoneBase(LDAPObject):
>>   
>>   class DNSZoneBase_add(LDAPCreate):
>>   
>> +    takes_options = LDAPCreate.takes_options + (
>> +        Flag('force',
>> +             label=_('Force'),
>> +             doc=_('Force DNS zone creation.')
>> +        ),
>> +        Flag('skip_overlap_check',
>> +             doc=_('Force DNS zone creation even if it will overlap with '
>> +                   'existing zone.')
>> +        ),
>> +    )
>> +
>>       has_output_params = LDAPCreate.has_output_params + dnszone_output_params
>>   
>>       def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
>>           assert isinstance(dn, DN)
>>   
>> +        if options['force']:
>> +            options['skip_overlap_check'] = True
>> +
>>           try:
>>               entry = ldap.get_entry(dn)
>>           except errors.NotFound:
>> @@ -2120,6 +2134,12 @@ class DNSZoneBase_add(LDAPCreate):
>>   
>>           entry_attrs['idnszoneactive'] = 'TRUE'
>>   
>> +        if not options['skip_overlap_check']:
>> +            try:
>> +                check_zone_overlap(keys[-1])
>> +            except RuntimeError as e:
>> +                raise errors.InvocationError(e.message)
>> +
>>           return dn
>>   
>>   
>> @@ -2673,9 +2693,9 @@ class dnszone_add(DNSZoneBase_add):
>>       __doc__ = _('Create new DNS zone (SOA record).')
>>   
>>       takes_options = DNSZoneBase_add.takes_options + (
>> -        Flag('force',
>> -             label=_('Force'),
>> -             doc=_('Force DNS zone creation even if nameserver is not resolvable.'),
>> +        Flag('skip_nameserver_check',
>> +             doc=_('Force DNS zone creation even if nameserver is not '
>> +                   'resolvable.')
>>           ),
>>   
>>           # Deprecated
>> @@ -2699,6 +2719,9 @@ class dnszone_add(DNSZoneBase_add):
>>       def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
>>           assert isinstance(dn, DN)
>>   
>> +        if options['force']:
>> +            options['skip_nameserver_check'] = True
> Why is it in DNSZoneBase_add.pre_callback?
>
> Shouldn't the equation force = (skip_nameserver_check + skip_nameserver_check)
> be handled in parameter parsing & validation? (Again, I do not know the IPA
> framework :-))
>
>> +
>>           dn = super(dnszone_add, self).pre_callback(
>>               ldap, dn, entry_attrs, attrs_list, *keys, **options)
>>   
>> @@ -2713,7 +2736,7 @@ class dnszone_add(DNSZoneBase_add):
>>                       error=_("Nameserver for reverse zone cannot be a relative DNS name"))
>>   
>>               # verify if user specified server is resolvable
>> -            if not options['force']:
>> +            if not options['skip_nameserver_check']:
>>                   check_ns_rec_resolvable(keys[0], entry_attrs['idnssoamname'])
>>               # show warning about --name-server option
>>               context.show_warning_nameserver_option = True
>> diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
>> index d959bb369d946217acd080e78483cc9013dda4c7..18f477d4fb6620090b7073689c8df51b65a8307a 100644
>> --- a/ipapython/ipautil.py
>> +++ b/ipapython/ipautil.py
>> @@ -924,6 +924,20 @@ def host_exists(host):
>>       else:
>>           return True
>>   
>> +def check_zone_overlap(zone):
>> +    if resolver.zone_for_name(zone) == zone:
>> +        try:
>> +            ns = [ans.to_text() for ans in resolver.query(zone, 'NS')]
>> +        except DNSException as e:
>> +            root_logger.debug("Failed to resolve nameserver(s) for domain"
>> +                " {0}: {1}".format(zone, e))
>> +            ns = []
>> +
>> +        msg = u"DNS zone {0} already exists".format(zone)
> Nitpick: I would say "already exists in DNS" to make it absolutely clear. Just
> 'already exists' might be confusing because ipa dnszone-show will say that the
> zone does not exist ...
>
>> +        if ns:
>> +            msg += u" and is handled by server(s): {0}".format(', '.join(ns))
>> +        raise RuntimeError(msg)
>> +
>>   def get_ipa_basedn(conn):
>>       """
>>       Get base DN of IPA suffix in given LDAP server.
0064
NACK

ipa-replica-install should have the --skip-overlap-check too, because 
any replica can be the first DNS server.

0064+0058
Can be the options --allow-zone-overlap and --skip-overlap-check merged 
into an one name, to have just one name for same thing?