[Freeipa-devel] [PATCH 0123-132] DNS upgrade: change forwarding policy to "only" if private IPs are used
Martin Basti
mbasti at redhat.com
Mon Jun 6 12:30:17 UTC 2016
On 06.06.2016 14:28, Petr Spacek wrote:
> On 6.6.2016 11:55, Martin Basti wrote:
>>
>> On 30.05.2016 12:49, Petr Spacek wrote:
>>> On 29.5.2016 14:45, Martin Basti wrote:
>>>> On 27.05.2016 14:12, Petr Spacek wrote:
>>>>> On 25.5.2016 12:50, Martin Basti wrote:
>>>>>> On 20.05.2016 12:19, Petr Spacek wrote:
>>>>>>> On 11.5.2016 12:08, Martin Basti wrote:
>>>>>>>> On 03.05.2016 14:59, Petr Spacek wrote:
>>>>>>>>> Hello,
>>>>>>>>>
>>>>>>>>> DNS upgrade: change forwarding policy to "only" if private IPs are used.
>>>>>>>>>
>>>>>>>>> https://fedorahosted.org/freeipa/ticket/5710
>>>>>>>>>
>>>>>>>>> This is the upgrade part. I will add one more patch to print a warning in
>>>>>>>>> dnsforwardzone* commands to avoid surprises. Please do not close the
>>>>>>>>> ticket
>>>>>>>>> yet.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>> 1)
>>>>>>>> Upgrade failed with 'BindInstance' object has no attribute
>>>>>>>> 'named_conf_get_directive'
>>>>>>>> IPA server upgrade failed: Inspect /var/log/ipaupgrade.log and run command
>>>>>>>> ipa-server-upgrade manually.
>>>>>>>> ('IPA upgrade failed.', 1)
>>>>>>>> The ipa-server-upgrade command failed. See /var/log/ipaupgrade.log for
>>>>>>>> more
>>>>>>>> information
>>>>>>>>
>>>>>>>> 2016-05-11T08:26:20Z ERROR Upgrade failed with 'BindInstance' object
>>>>>>>> has no
>>>>>>>> attribute 'named_conf_get_directive'
>>>>>>>> 2016-05-11T08:26:20Z DEBUG Traceback (most recent call last):
>>>>>>>> File
>>>>>>>> "/usr/lib/python2.7/site-packages/ipaserver/install/upgradeinstance.py",
>>>>>>>> line
>>>>>>>> 213, in __upgrade
>>>>>>>> self.modified = (ld.update(self.files) or self.modified)
>>>>>>>> File
>>>>>>>> "/usr/lib/python2.7/site-packages/ipaserver/install/ldapupdate.py",
>>>>>>>> line 917, in update
>>>>>>>> self._run_updates(all_updates)
>>>>>>>> File
>>>>>>>> "/usr/lib/python2.7/site-packages/ipaserver/install/ldapupdate.py",
>>>>>>>> line 889, in _run_updates
>>>>>>>> self._run_update_plugin(update['plugin'])
>>>>>>>> File
>>>>>>>> "/usr/lib/python2.7/site-packages/ipaserver/install/ldapupdate.py",
>>>>>>>> line 862, in _run_update_plugin
>>>>>>>> restart_ds, updates = self.api.Updater[plugin_name]()
>>>>>>>> File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line
>>>>>>>> 1418, in
>>>>>>>> __call__
>>>>>>>> return self.execute(**options)
>>>>>>>> File
>>>>>>>> "/usr/lib/python2.7/site-packages/ipaserver/install/plugins/dns.py",
>>>>>>>> line 547, in execute
>>>>>>>> self.update_global_named_conf_forwarder(bind)
>>>>>>>> File
>>>>>>>> "/usr/lib/python2.7/site-packages/ipaserver/install/plugins/dns.py",
>>>>>>>> line 508, in update_global_named_conf_forwarder
>>>>>>>> if bind.named_conf_get_directive(
>>>>>>>> AttributeError: 'BindInstance' object has no attribute
>>>>>>>> 'named_conf_get_directive'
>>>>>>>>
>>>>>>>> 2016-05-11T08:26:20Z DEBUG Traceback (most recent call last):
>>>>>>>> File "/usr/lib/python2.7/site-packages/ipaserver/install/service.py",
>>>>>>>> line
>>>>>>>> 447, in start_creation
>>>>>>>> run_step(full_msg, method)
>>>>>>>> File "/usr/lib/python2.7/site-packages/ipaserver/install/service.py",
>>>>>>>> line
>>>>>>>> 437, in run_step
>>>>>>>> method()
>>>>>>>> File
>>>>>>>> "/usr/lib/python2.7/site-packages/ipaserver/install/upgradeinstance.py",
>>>>>>>> line
>>>>>>>> 221, in __upgrade
>>>>>>>> raise RuntimeError(e)
>>>>>>>> RuntimeError: 'BindInstance' object has no attribute
>>>>>>>> 'named_conf_get_directive'
>>>>>>>>
>>>>>>>> PATCH * Add ipaDNSVersion option to dnsconfig* commands and use new
>>>>>>>> attribute *
>>>>>>>> 2)
>>>>>>>> + Int('ipadnsversion?',
>>>>>>>> + label=_('IPA DNS version'),
>>>>>>>> + ),
>>>>>>>>
>>>>>>>> Shouldn't be this part of System: Read DNS Configuration permission?
>>>>>>>>
>>>>>>>> 3)
>>>>>>>> - def postprocess_result(self, result):
>>>>>>>> + def postprocess_result(self, result, show_version):
>>>>>>>> if not any(param in result['result'] for param in
>>>>>>>> self.params):
>>>>>>>> result['summary'] = unicode(_('Global DNS configuration is
>>>>>>>> empty'))
>>>>>>>>
>>>>>>>> show_version param was added but I don't see it used in this patch.
>>>>>>>>
>>>>>>>> 4)
>>>>>>>> + Int('ipadnsversion?',
>>>>>>>> + label=_('IPA DNS version'),
>>>>>>>> + ),
>>>>>>>>
>>>>>>>> Could we add comment here that this option is accessible only from
>>>>>>>> installers
>>>>>>>> and upgrade?
>>>>>>>>
>>>>>>>> 5)
>>>>>>>> + for config_option in container_entry.get("ipaConfigString", []):
>>>>>>>> + matched = re.match("^DNSVersion\s+(?P<version>\d+)$",
>>>>>>>> + config_option, flags=re.I)
>>>>>>>> + if matched:
>>>>>>>> + version = int(matched.group("version"))
>>>>>>>>
>>>>>>>> Shouldn't we print error if version cannot be parsed?
>>>>>>>>
>>>>>>>> PATCH * DNS upgrade: separate backup logic to make it reusable *
>>>>>>>>
>>>>>>>> LGTM
>>>>>>>>
>>>>>>>> PATCH * Add function ipapython.dnsutil.related_to_auto_empty_zone() *
>>>>>>>>
>>>>>>>> 7)
>>>>>>>> I'm curious why do you need to check superdomains?
>>>>>>>>
>>>>>>>> PATCH * DNS upgrade: change forwarding policy to = only for conflicting
>>>>>>>> forward zones*
>>>>>>>>
>>>>>>>> 8)
>>>>>>>> + self.log.debug('Zone %s was sucessfully modified to use '
>>>>>>>> + 'forward policy "only"', zone['idnsname'][0])
>>>>>>>> <---missing empty line---->
>>>>>>>> + def execute(self, **options):
>>>>>>>>
>>>>>>>> PATCH * DNS upgrade: change global forwarding policy in LDAP to "only" if
>>>>>>>> private IPs are used *
>>>>>>>> 9)
>>>>>>>> - dnsutil.related_to_auto_empty_zone(zone.get('idnsname')[0])
>>>>>>>> + dnsutil.related_to_auto_empty_zone(
>>>>>>>> + dnsutil.DNSName(zone.get('idnsname')[0]))
>>>>>>>>
>>>>>>>> Should be in previous commit
>>>>>>>>
>>>>>>>> 10)
>>>>>>>> - return
>>>>>>>> + return False, []
>>>>>>>> This should be fixed in the previous commit
>>>>>>>>
>>>>>>>> PATCH * DNS upgrade: change global forwarding policy in named.conf to
>>>>>>>> "only"
>>>>>>>> if private IPs are used *
>>>>>>>> 11)
>>>>>>>> IMO this is an upgrade of configuration and this should be in
>>>>>>>> ipaserver/install/server/upgrade.py, upgrade plugins are used only for
>>>>>>>> updating of LDAP values
>>>>>>>>
>>>>>>>> Unless you really want to use this as precedence, but then it requires
>>>>>>>> broader
>>>>>>>> discussion.
>>>>>>>>
>>>>>>>> 12)
>>>>>>>>
>>>>>>>> bind.named_conf_get_directive
>>>>>>>> should be
>>>>>>>> bindinstance.named_conf_get_directive
>>>>>>>>
>>>>>>>> see 1)
>>>>>>> This new patchset completely obsoletes the old one. I had to reshuffle few
>>>>>>> things to to make the split between server config & LDAP upgrade possible.
>>>>>>>
>>>>>>> Hopefully I addressed all your comment.
>>>>>>>
>>>>>> commits
>>>>>> * Move IP address resolution from ipaserver.install.installutils to
>>>>>> ipapython.dnsutil * and * Turn verify_host_resolvable() into a wrapper
>>>>>> around
>>>>>> ipapython.dnsutil *
>>>>>>
>>>>>> cause regression in case that dns.python resolver returns NoNameservers
>>>>>> exception, it is handled as 'Internal server error'
>>>>>>
>>>>>> In original code every exception was caught and transformed to
>>>>>> DNSNotARecordError.
>>>>>>
>>>>>> So we have following options:
>>>>>> * keep the old behavior in 'resolve_rrsets' and catch all exceptions there
>>>>>> * or catch all DNS errors in 'verify_host_resolvable' and raise it as new
>>>>>> PublicError (DNSGenericError (doesn't exist) for example)
>>>>>>
>>>>>>
>>>>>> E InternalError: an internal error has occurred
>>>>>>
>>>>>> ../ipalib/rpc.py:1100: InternalError
>>>>>> test_forwardzone_delegation_warnings.test_command[0017: dnsrecord_mod:
>>>>>> Delete
>>>>>> (using dnsrecord-mod) NS record which delegates zone
>>>>>> u'fw.sub2.sub.dnszone.test.' from zone u'dnszone.test' (expected warning for
>>>>>> u'fw.sub2.sub.dnszone.test.')]
>>>>>>
>>>>>> [Wed May 25 12:17:00.172143 2016] [wsgi:error] [pid 62789] Traceback (most
>>>>>> recent call last):
>>>>>> [Wed May 25 12:17:00.172152 2016] [wsgi:error] [pid 62789] File
>>>>>> "/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py", line 350, in
>>>>>> wsgi_execute
>>>>>> [Wed May 25 12:17:00.172158 2016] [wsgi:error] [pid 62789] result =
>>>>>> self.Command[name](*args, **options)
>>>>>> [Wed May 25 12:17:00.172164 2016] [wsgi:error] [pid 62789] File
>>>>>> "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 434, in __call__
>>>>>> [Wed May 25 12:17:00.172168 2016] [wsgi:error] [pid 62789] return
>>>>>> self.__do_call(*args, **options)
>>>>>> [Wed May 25 12:17:00.172173 2016] [wsgi:error] [pid 62789] File
>>>>>> "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 460, in
>>>>>> __do_call
>>>>>> [Wed May 25 12:17:00.172178 2016] [wsgi:error] [pid 62789] ret =
>>>>>> self.run(*args, **options)
>>>>>> [Wed May 25 12:17:00.172183 2016] [wsgi:error] [pid 62789] File
>>>>>> "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 777, in run
>>>>>> [Wed May 25 12:17:00.172189 2016] [wsgi:error] [pid 62789] return
>>>>>> self.execute(*args, **options)
>>>>>> [Wed May 25 12:17:00.172194 2016] [wsgi:error] [pid 62789] File
>>>>>> "/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line 3774, in
>>>>>> execute
>>>>>> [Wed May 25 12:17:00.172199 2016] [wsgi:error] [pid 62789] result =
>>>>>> super(dnsrecord_add, self).execute(*keys, **options)
>>>>>> [Wed May 25 12:17:00.172204 2016] [wsgi:error] [pid 62789] File
>>>>>> "/usr/lib/python2.7/site-packages/ipalib/plugins/baseldap.py", line 1230, in
>>>>>> execute
>>>>>> [Wed May 25 12:17:00.172209 2016] [wsgi:error] [pid 62789] *keys, **options)
>>>>>> [Wed May 25 12:17:00.172213 2016] [wsgi:error] [pid 62789] File
>>>>>> "/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line 3719, in
>>>>>> pre_callback
>>>>>> [Wed May 25 12:17:00.172229 2016] [wsgi:error] [pid 62789]
>>>>>> self.obj.run_precallback_validators(dn, entry_attrs, *keys, **options)
>>>>>> [Wed May 25 12:17:00.172237 2016] [wsgi:error] [pid 62789] File
>>>>>> "/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line 3135, in
>>>>>> run_precallback_validators
>>>>>> [Wed May 25 12:17:00.172242 2016] [wsgi:error] [pid 62789] rtype_cb(ldap,
>>>>>> dn,
>>>>>> entry_attrs, *keys, **options)
>>>>>> [Wed May 25 12:17:00.172247 2016] [wsgi:error] [pid 62789] File
>>>>>> "/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line 3057, in
>>>>>> _nsrecord_pre_callback
>>>>>> [Wed May 25 12:17:00.172252 2016] [wsgi:error] [pid 62789]
>>>>>> check_ns_rec_resolvable(keys[0], DNSName(nsrecord), self.log)
>>>>>> [Wed May 25 12:17:00.172256 2016] [wsgi:error] [pid 62789] File
>>>>>> "/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line 1577, in
>>>>>> check_ns_rec_resolvable
>>>>>> [Wed May 25 12:17:00.172261 2016] [wsgi:error] [pid 62789]
>>>>>> verify_host_resolvable(name)
>>>>>> [Wed May 25 12:17:00.172265 2016] [wsgi:error] [pid 62789] File
>>>>>> "/usr/lib/python2.7/site-packages/ipalib/util.py", line 70, in
>>>>>> verify_host_resolvable
>>>>>> [Wed May 25 12:17:00.172270 2016] [wsgi:error] [pid 62789] if not
>>>>>> resolve_ip_addresses(fqdn):
>>>>>> [Wed May 25 12:17:00.172274 2016] [wsgi:error] [pid 62789] File
>>>>>> "/usr/lib/python2.7/site-packages/ipapython/dnsutil.py", line 328, in
>>>>>> resolve_ip_addresses
>>>>>> [Wed May 25 12:17:00.172278 2016] [wsgi:error] [pid 62789] rrsets =
>>>>>> resolve_rrsets(fqdn, ['A', 'AAAA'])
>>>>>> [Wed May 25 12:17:00.172282 2016] [wsgi:error] [pid 62789] File
>>>>>> "/usr/lib/python2.7/site-packages/ipapython/dnsutil.py", line 305, in
>>>>>> resolve_rrsets
>>>>>> [Wed May 25 12:17:00.172287 2016] [wsgi:error] [pid 62789] answer =
>>>>>> dns.resolver.query(fqdn, rdtype)
>>>>>> [Wed May 25 12:17:00.172292 2016] [wsgi:error] [pid 62789] File
>>>>>> "/usr/lib/python2.7/site-packages/dns/resolver.py", line 1029, in query
>>>>>> [Wed May 25 12:17:00.172296 2016] [wsgi:error] [pid 62789]
>>>>>> raise_on_no_answer,
>>>>>> source_port)
>>>>>> [Wed May 25 12:17:00.172301 2016] [wsgi:error] [pid 62789] File
>>>>>> "/usr/lib/python2.7/site-packages/dns/resolver.py", line 856, in query
>>>>>> [Wed May 25 12:17:00.172328 2016] [wsgi:error] [pid 62789] raise
>>>>>> NoNameservers(request=request, errors=errors)
>>>>> Fixed patches are attached.
>>>>>
>>>>>
>>>>> Please note that I've renumbered the patches because the naming does not
>>>>> match
>>>>> the original set anymore.
>>>>>
>>>> NACK
>>>>
>>>> # ipa-run-tests test_xmlrpc/test_host_plugin.py
>>>> ===============================================================================================
>>>>
>>>> test session starts
>>>> ===============================================================================================
>>>>
>>>>
>>>> platform linux2 -- Python 2.7.11, pytest-2.9.1, py-1.4.31, pluggy-0.3.1
>>>> rootdir: /usr/lib/python2.7/site-packages/ipatests, inifile: pytest.ini
>>>> plugins: sourceorder-0.5, multihost-1.0
>>>> collected 41 items
>>>>
>>>> test_xmlrpc/test_host_plugin.py ...................F...........s.........
>>>>
>>>> ====================================================================================================
>>>>
>>>> FAILURES
>>>> =====================================================================================================
>>>>
>>>>
>>>> ________________________________________________________________________________________
>>>>
>>>> TestCRUD.test_try_add_not_in_dns
>>>> _________________________________________________________________________________________
>>>>
>>>>
>>>>
>>>> self = <ipatests.test_xmlrpc.test_host_plugin.TestCRUD object at
>>>> 0x7efc061e4110>, host = <ipatests.test_xmlrpc.tracker.host_plugin.HostTracker
>>>> object at 0x7efc0623bb90>
>>>>
>>>> def test_try_add_not_in_dns(self, host):
>>>> host.ensure_missing()
>>>> command = host.make_create_command(force=False)
>>>> with raises_exact(errors.DNSNotARecordError(
>>>>> reason=u'Host does not have corresponding DNS A/AAAA
>>>>> record')):
>>>> test_xmlrpc/test_host_plugin.py:315:
>>>> _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
>>>> _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
>>>> _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
>>>> ../ipalib/errors.py:264: in __init__
>>>> messages.process_message_arguments(self, format, message, **kw)
>>>> _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
>>>> _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
>>>> _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
>>>>
>>>> obj = DNSNotARecordError(), format = None, message = None, kw = {'reason':
>>>> 'Host does not have corresponding DNS A/AAAA record'}, key = 'reason', value =
>>>> 'Host does not have corresponding DNS A/AAAA record'
>>>> name = 'DNSNotARecordError'
>>>>
>>>> def process_message_arguments(obj, format=None, message=None, **kw):
>>>> for key, value in kw.items():
>>>> if not isinstance(value, six.integer_types):
>>>> try:
>>>> kw[key] = unicode(value)
>>>> except UnicodeError:
>>>> pass
>>>> obj.kw = kw
>>>> name = obj.__class__.__name__
>>>> if obj.format is not None and format is not None:
>>>> raise ValueError(
>>>> 'non-generic %r needs format=None; got format=%r' % (
>>>> name, format)
>>>> )
>>>> if message is None:
>>>> if obj.format is None:
>>>> if format is None:
>>>> raise ValueError(
>>>> '%s.format is None yet format=None, message=None'
>>>> % name
>>>> )
>>>> obj.format = format
>>>> obj.forwarded = False
>>>>> obj.msg = obj.format % kw
>>>> E KeyError: 'hostname'
>>> Sorry, this got lost in all the 'normal' failures :-)
>>>
>>> I do not 100% understand this test logic so I hope that attached patch fixes
>>> it correctly.
>>>
>> We broke IPA 4.3 tests, patch that fixes them is attached.
> Of yes, we forgot to backport it. ACK.
>
Pushed to ipa-4-3: 8f6db8ffe69c0e14c97f5bdc2c0635c95f1fa225
More information about the Freeipa-devel
mailing list