On 25/08/15 14:39, David Kupka wrote:
On 25/08/15 10:37, David Kupka wrote:
On 24/08/15 16:51, Martin Basti wrote:


On 08/20/2015 10:28 AM, David Kupka wrote:
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).

Note: Petr2 agreed with David's solution

LGTM, works as expected, but this patch prevents users to add
conflicting zones via webUI (there is no --force field).
We should improve webUI together with this patch.

Martin^2


Martin^2




The '--force' option was not in WebUI before even though it was in API.
IMO we should not expose '--force' options in WebUI at all.


Added similar options to ipa-{server,dns}-install and reworked the patch
to not duplicate the code.
Updated patch and one new attached.



Updated patch attached.

--
David Kupka
From 660e4dcc7bb1b828b24515d89b4f6dad4f86b7bf Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Thu, 2 Jul 2015 15:10:40 +0200
Subject: [PATCH 1/2] 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 | 33 ++++++++++++++++++++++++++++-----
 ipapython/ipautil.py  | 14 ++++++++++++++
 3 files changed, 48 insertions(+), 7 deletions(-)

diff --git a/API.txt b/API.txt
index afd5017bee2bc1eed54497ccd504b92619ff7a58..6fc1c3e5b2b9af48ff12803654340f91e826a507 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 23871a58f441cdc598d49c76035da243031b49d7..8bdea7ab5a46a366021c258788057e13c74be41a 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, check_zone_overlap
 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,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
+
         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)
+        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.
-- 
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