[Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
David Kupka
dkupka at redhat.com
Mon Dec 7 13:06:04 UTC 2015
On 09/09/15 13:39, Petr Spacek wrote:
> On 8.9.2015 16:30, David Kupka wrote:
>> 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.
>
> Hello,
>
> thank you very much for updating the patch.
>
> Unfortunately it is not yet ready, but we are getting there.
>
>
> * Serious problems:
>
> a) ipa-server/replica/dns-install by default creates reverse zones even if
> these zones exist. The check which is done for main IPA domain should be done
> for all reverse zones, too. If a zone exists user should use --no-reverse or
> --allow-zone-overlap if necessary.
>
> I believe that this is in scope of https://fedorahosted.org/freeipa/ticket/3681 .
>
>
> b) ipa-server/replica/dns-install by default always add DNS zone which
> contains hostname of the IPA server, even if the IPA domain is different and
> even if the DNS zone already exists.
>
> If the hostname of the server does not belong to IPA domain we should not add
> anything. Just check that the hostname is resolvable. Theoretically we could
> add an option which explicitly asks for creating zone which encloses the
> hostnmame.
>
> I believe that this is in scope of https://fedorahosted.org/freeipa/ticket/5087 .
>
>
> c) I did not realize that checks in ipa dns*zone-add will break zone addition
> for automatic empty zones.
>
> There is list of special-case DNS zones which need special handling:
> https://source.isc.org/cgi-bin/gitweb.cgi?p=bind9.git;a=blob;f=bin/named/server.c;h=1a25531df354f6f0593bfb86e5aa3e3c9b9c80e5;hb=HEAD#l272
>
> Feel free to copy&paste list of the domains to IPA code - list comes from
> RFCs, so it should be pretty stable. It would be good
>
> If one of these zones is detected as "already existing" we need to further
> check values in SOA record and automatically override the check if:
> (SOA mname = zone name) & (SOA rname = .)
>
> In that case we should print informational message "automatic empty zone will
> be overridden by the zone you defined now".
>
>
> d) ipa-server/replica/dns-install fails when attempt to resolve main IPA
> domain name fails with DNS timeout. This might easily happen if the domain
> name is already delegated to IPA server which is being installed. In that case
> we should print a warning and allow to continue.
>
> """
> Warning: DNS check for domain <IPA domain> failed with error <exception>.
> Please make sure that the domain is properly delegated to this IPA server.
> """
>
>
> * Nitpicks - can be handled in separate patches:
> 1) Interactive ipa-server-install tells the user that the zone name he entered
> exists too late in the process.
>
> I.e. user has to enter DM and admin password (twice!) and only after that he
> will receive an error, which may be frustrating :-)
>
> Please move the check right after DNS zone name input so user does not waste
> his time and nerves :-) Even better, in an interactive mode it could make
> sense to ask the user again if the previous input was not acceptable.
>
>
> 2) I found out that ipa-server-install prints message "Warning: skipping DNS
> resolution of host vm-206.abc.idm.lab.eng.brq.redhat.com" even if the host
> name does not belong to IPA domain. That is a mistake - the check should be
> skipped only in case when IPA server is part of the IPA domain.
>
>
> 3) When adding an forward as an override for automatic empty zones we should
> throw a warning if forward policy is not 'only'. It does not make sense to use
> policy 'forward' in these cases.
> """
> Warning: It is strongly recommended to use forward policy 'only' for the
> forward zones defined for private use.
> """
>
Hi!
I was finally able to get back to this patches. I've (hopefully)
addressed all the items from "Serious problems" section plus the first
"nitpick" as I found it really annoying.
Updated patches attached. Patch freeipa-dkupka-0070 is needed for this
to work.
--
David Kupka
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-dkupka-0058.6-dns-do-not-add-forward-zone-if-it-is-already-resolva.patch
Type: text/x-patch
Size: 11794 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20151207/640731ef/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-dkupka-0064.1-dns-Check-if-domain-already-exists.patch
Type: text/x-patch
Size: 3834 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20151207/640731ef/attachment-0001.bin>
More information about the Freeipa-devel
mailing list