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.

--
David Kupka
From ab7aa126a68bcea95f707a78ca1f776270d3b5ec 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  | 17 +++++++++++++++++
 3 files changed, 51 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..bcd3793bd94f3836f5127fce153ae29b2cbfd151 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.DuplicateEntry(message=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 88e89706b8e2aa6dea80809510d88bceaa836e85..fded22e5f24e75529a854bfc3f6af0dab4f1508f 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -923,6 +923,23 @@ 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 check_zone_overlap(zone):
+    if is_zone_resolvable(zone):
+        ns = zone_nameservers(zone)
+        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

From fe8cbf8eab8fb153dbc01e9a5d6a30e9de593faa Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Fri, 21 Aug 2015 13:25:34 +0200
Subject: [PATCH 2/2] dns: Check if domain already exists.

Raise an error when the domain already exists. This can be overriden using
--force or --allow-zone-overlap options.

https://fedorahosted.org/freeipa/ticket/3681
---
 install/tools/ipa-dns-install       |  3 +++
 ipaserver/install/dns.py            | 13 +++++++++++++
 ipaserver/install/server/install.py |  6 ++++++
 3 files changed, 22 insertions(+)

diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install
index 34b952859e56c6aa5ae861a4d1fb615f0a2d8f55..11767f106dfc47721aa038977fcc4357c83c94c3 100755
--- a/install/tools/ipa-dns-install
+++ b/install/tools/ipa-dns-install
@@ -70,6 +70,9 @@ def parse_options():
                       "kasp.db file)")
     parser.add_option("--force", dest="force", action="store_true",
                       help="Force install")
+    parser.add_option("--allow-zone-overlap", dest="allow_zone_overlap",
+                      action="store_true", default=False, help="Create DNS "
+                      "zone even if it already exists")
 
     options, args = parser.parse_args()
     safe_options = parser.get_safe_opts(options)
diff --git a/ipaserver/install/dns.py b/ipaserver/install/dns.py
index 9430d189978b0984b0b71d7d754516a4135053fb..146774f74000495aad5580d15c6bd993e692a192 100644
--- a/ipaserver/install/dns.py
+++ b/ipaserver/install/dns.py
@@ -8,11 +8,13 @@ from subprocess import CalledProcessError
 
 from ipalib import api
 from ipalib import errors
+from ipalib import util
 from ipaplatform.paths import paths
 from ipaplatform.constants import constants
 from ipaplatform import services
 from ipapython import ipautil
 from ipapython import sysrestore
+from ipapython import dnsutil
 from ipapython.dn import DN
 from ipapython.ipa_log_manager import root_logger
 from ipapython.ipaldap import AUTOBIND_ENABLED
@@ -101,6 +103,17 @@ def install_check(standalone, replica, options, hostname):
         raise RuntimeError("Integrated DNS requires '%s' package" %
                            constants.IPA_DNS_PACKAGE_NAME)
 
+    domain = dnsutil.DNSName(util.normalize_zone(api.env.domain))
+    try:
+        ipautil.check_zone_overlap(domain)
+    except RuntimeError as e:
+        msg = e.message + u" Either use different domain or delete its" \
+            " record(s) on name server(s)."
+        if options.force or options.allow_zone_overlap:
+            root_logger.warning(msg)
+        else:
+            raise RuntimeError(msg)
+
     if standalone:
         print "=============================================================================="
         print "This program will setup DNS for the FreeIPA Server."
diff --git a/ipaserver/install/server/install.py b/ipaserver/install/server/install.py
index b9bf3f34bdb7c32115e5c6a7038f11f901ab06b8..b076bdd53347d65372353db2bc7afed5452334e7 100644
--- a/ipaserver/install/server/install.py
+++ b/ipaserver/install/server/install.py
@@ -1381,6 +1381,11 @@ class ServerDNS(common.Installable, core.Group, core.Composite):
         description="Do not automatically create DNS SSHFP records",
     )
 
+    allow_zone_overlap = Knob(
+        bool, False,
+        description="Create DNS zone even if it already exists",
+    )
+
 
 class Server(common.Installable, common.Interactive, core.Composite):
     realm_name = Knob(
@@ -1646,6 +1651,7 @@ class Server(common.Installable, common.Interactive, core.Composite):
         self.zonemgr = self.dns.zonemgr
         self.no_host_dns = self.dns.no_host_dns
         self.no_dns_sshfp = self.dns.no_dns_sshfp
+        self.allow_zone_overlap = self.dns.allow_zone_overlap
 
         self.unattended = not self.interactive
 
-- 
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