On 31/07/15 13:32, Martin Basti wrote:
On 30/07/15 14:38, Martin Basti wrote:
On 29/07/15 16:12, David Kupka wrote:
https://fedorahosted.org/freeipa/ticket/5087
NACK

You forgot to update API.txt file

Thanks for catching that. Updated patch attached.


I'm just curious, what is the reason to check if forward zone exists?

IMO forwardzone must exists somewhere as the master zone. I don't think
we should check forwardzones, this may give too many false positive errors.

AIUI if the zone exist somewhere and is resolvable there is no need to add it as a forward zone. If user for some reason want to do it he's hiding the original zone and we should not allow this (without --force).


Martin^2


--
David Kupka
From a628d51785b4a8f20f09d49158b0eeb489937ec6 Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Thu, 2 Jul 2015 15:10:40 +0200
Subject: [PATCH] dns: do not add (forward)zone if it is already resolvable.

Check if the zone user wants to add is already resolvable and refuse to
create it if yes. --skip-overlap-check and --force options suppress this check.

https://fedorahosted.org/freeipa/ticket/5087
---
 API.txt               |  8 ++++++--
 ipalib/plugins/dns.py | 38 +++++++++++++++++++++++++++++++++-----
 ipapython/ipautil.py  |  9 +++++++++
 3 files changed, 48 insertions(+), 7 deletions(-)

diff --git a/API.txt b/API.txt
index 6ab30ddab41715fdbccb4f37aa1852621bca62b4..f6db1e99fca2acf410308c5fdaa13b40c43df933 100644
--- a/API.txt
+++ b/API.txt
@@ -960,15 +960,17 @@ output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDA
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
 output: PrimaryKey('value', None, None)
 command: dnsforwardzone_add
-args: 1,8,3
+args: 1,10,3
 arg: DNSNameParam('idnsname', attribute=True, cli_name='name', multivalue=False, only_absolute=True, primary_key=True, required=True)
 option: Str('addattr*', cli_name='addattr', exclude='webui')
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
+option: Flag('force', autofill=True, default=False)
 option: Str('idnsforwarders', attribute=True, cli_name='forwarder', csv=True, multivalue=True, required=False)
 option: StrEnum('idnsforwardpolicy', attribute=True, cli_name='forward_policy', multivalue=False, required=False, values=(u'only', u'first', u'none'))
 option: Str('name_from_ip', attribute=False, cli_name='name_from_ip', multivalue=False, required=False)
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
 option: Str('setattr*', cli_name='setattr', exclude='webui')
+option: Flag('skip_overlap_check', autofill=True, default=False)
 option: Str('version?', exclude='webui')
 output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
@@ -1367,7 +1369,7 @@ output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDA
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
 output: PrimaryKey('value', None, None)
 command: dnszone_add
-args: 1,26,3
+args: 1,28,3
 arg: DNSNameParam('idnsname', attribute=True, cli_name='name', multivalue=False, only_absolute=True, primary_key=True, required=True)
 option: Str('addattr*', cli_name='addattr', exclude='webui')
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
@@ -1394,6 +1396,8 @@ option: Str('name_from_ip', attribute=False, cli_name='name_from_ip', multivalue
 option: Str('nsec3paramrecord', attribute=True, cli_name='nsec3param_rec', multivalue=False, pattern='^\\d+ \\d+ \\d+ (([0-9a-fA-F]{2})+|-)$', required=False)
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
 option: Str('setattr*', cli_name='setattr', exclude='webui')
+option: Flag('skip_nameserver_check', autofill=True, default=False)
+option: Flag('skip_overlap_check', autofill=True, default=False)
 option: Str('version?', exclude='webui')
 output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index 512a653c3cc8ee641debec0d20f58e17eff08266..4e7b72628930c5be340ac37c08f1e45654e80c9e 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -50,7 +50,7 @@ from ipalib.util import (normalize_zonemgr,
                          validate_dnssec_zone_forwarder_step1,
                          validate_dnssec_zone_forwarder_step2)
 
-from ipapython.ipautil import CheckedIPAddress, is_host_resolvable
+from ipapython.ipautil import CheckedIPAddress, is_host_resolvable, is_zone_resolvable, zone_nameservers
 from ipapython.dnsutil import DNSName
 
 __doc__ = _("""
@@ -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,17 @@ class DNSZoneBase_add(LDAPCreate):
 
         entry_attrs['idnszoneactive'] = 'TRUE'
 
+        zone = keys[-1]
+        if not options['skip_overlap_check']:
+            if is_zone_resolvable(zone):
+                ns = zone_nameservers(zone)
+                if ns:
+                    msg = _(u"DNS zone {0} already exists and is handled "
+                            "by server(s): {1}".format(zone, ', '.join(ns)))
+                else:
+                    msg = _(u"DNS zone {0} already exists".format(zone))
+                raise errors.DuplicateEntry(message=msg)
+
         return dn
 
 
@@ -2673,9 +2698,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 +2724,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
+
         dn = super(dnszone_add, self).pre_callback(
             ldap, dn, entry_attrs, attrs_list, *keys, **options)
 
@@ -2713,7 +2741,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 88e89706b8e2aa6dea80809510d88bceaa836e85..6bfba377c54a96ea197be7f7c8f633e433429db3 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -923,6 +923,15 @@ def host_exists(host):
     else:
         return True
 
+def is_zone_resolvable(zone):
+    return resolver.zone_for_name(zone) == zone
+
+def zone_nameservers(zone):
+        try:
+            return [ns.to_text() for ns in resolver.query(zone, 'NS')]
+        except DNSException:
+            return []
+
 def get_ipa_basedn(conn):
     """
     Get base DN of IPA suffix in given LDAP server.
-- 
2.4.3

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to