[Freeipa-devel] [PATCH 0123-132] DNS upgrade: change forwarding policy to "only" if private IPs are used
Petr Spacek
pspacek at redhat.com
Mon May 30 10:49:51 UTC 2016
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.
--
Petr^2 Spacek
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pspacek-0123-2-Move-check_zone_overlap-from-ipapython.ipautil-to-ip.patch
Type: text/x-patch
Size: 8779 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160530/8eba4e47/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pspacek-0124-2-Use-root_logger-for-verify_host_resolvable.patch
Type: text/x-patch
Size: 7354 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160530/8eba4e47/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pspacek-0125-2-Move-IP-address-resolution-from-ipaserver.install.in.patch
Type: text/x-patch
Size: 7559 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160530/8eba4e47/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pspacek-0126-2-Turn-verify_host_resolvable-into-a-wrapper-around-ip.patch
Type: text/x-patch
Size: 7203 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160530/8eba4e47/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pspacek-0127-2-Add-ipaDNSVersion-option-to-dnsconfig-commands-and-u.patch
Type: text/x-patch
Size: 16167 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160530/8eba4e47/attachment-0004.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pspacek-0128-2-DNS-upgrade-separate-backup-logic-to-make-it-reusabl.patch
Type: text/x-patch
Size: 9297 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160530/8eba4e47/attachment-0005.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pspacek-0129-2-Add-function-ipapython.dnsutil.related_to_auto_empty.patch
Type: text/x-patch
Size: 1859 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160530/8eba4e47/attachment-0006.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pspacek-0130-2-DNS-upgrade-change-forwarding-policy-to-only-for-con.patch
Type: text/x-patch
Size: 6178 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160530/8eba4e47/attachment-0007.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pspacek-0131-2-DNS-upgrade-change-global-forwarding-policy-in-LDAP-.patch
Type: text/x-patch
Size: 3275 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160530/8eba4e47/attachment-0008.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-pspacek-0132-2-DNS-upgrade-change-global-forwarding-policy-in-named.patch
Type: text/x-patch
Size: 5852 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160530/8eba4e47/attachment-0009.bin>
More information about the Freeipa-devel
mailing list