[Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
Petr Spacek
pspacek at redhat.com
Wed Sep 9 11:39:12 UTC 2015
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.
"""
--
Petr^2 Spacek
More information about the Freeipa-devel
mailing list