On 28/08/15 13:36, Martin Basti wrote:


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.

Thanks for the catch, added.


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?


Each option has bit different behavior:
The '--skip-overlap-check' option in API call prevent the check to be performed and thus no error or warning is raised. This is the way '--force' option was originally working.

The '--allow-zone-overlap' options in installers do not skip the check but change the error to warning instead and let the installation continue.

If you think that this can confuse users we need to change the names or even the logic.

Updated patches attached.

--
David Kupka
From 816ee3102ee0603450f897f8f6bfed4d5460636c 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 ++++++
 ipaserver/install/server/replicainstall.py |  6 ++++++
 4 files changed, 28 insertions(+)

diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install
index b6b922e881c21e00994a8fba696d6858f018ac0e..3fb2c8aeeaceb35a5e793184ab596d058ffe7de2 100755
--- a/install/tools/ipa-dns-install
+++ b/install/tools/ipa-dns-install
@@ -72,6 +72,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 4864720949ffee997247ca0762c434199cba2a13..4646c8603d95ba4629a014e36b4313df5d3f574d 100644
--- a/ipaserver/install/dns.py
+++ b/ipaserver/install/dns.py
@@ -10,11 +10,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
@@ -104,6 +106,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 f9baa0757df488e7b15e6e60a368d822ed3cd063..0336976ea6f7c4324a91cb4e8e18db9f9497a57b 100644
--- a/ipaserver/install/server/install.py
+++ b/ipaserver/install/server/install.py
@@ -1357,6 +1357,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(
@@ -1624,6 +1629,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
 
diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index c36548e7a0af9ca09c931b6e581e7029456fce9b..50b0fcb04f891bd716fdb71e934943b05a012943 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -714,6 +714,11 @@ class ReplicaDNS(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 Replica(common.Installable, common.Interactive, core.Composite):
     replica_file = Knob(
@@ -862,6 +867,7 @@ class Replica(common.Installable, common.Interactive, core.Composite):
         self.zonemgr = None
         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

From 2f4fc7860b0652fbfe82b23575b956ccc6405634 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 a4c947a2a844bdec797771c311b9032463fe6d0d..dd6e27b4ad22c07c451dc6335721c3651b688982 100644
--- a/API.txt
+++ b/API.txt
@@ -959,15 +959,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)
@@ -1366,7 +1368,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')
@@ -1393,6 +1395,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 be0639b6ada1b95364dc53a42f8a762e0671fe1a..0c53babf4672d8692877fb39aacb84ece0ecfca4 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -53,7 +53,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__ = _("""
@@ -2104,11 +2104,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:
@@ -2123,6 +2137,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
 
 
@@ -2660,9 +2680,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
@@ -2686,6 +2706,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)
 
@@ -2700,7 +2723,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 2402689ccc1980117166969944e09e1ab5063ec2..9b7274621e67fdbdfa408f0c9da6c4a40edb0b8a 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -938,6 +938,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 in DNS".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