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

[Petr Spacek]

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.

-- 
Petr^2 Spacek