[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 09:55:48 UTC 2016



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.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-4.3-mbasti-0496-Fix-exceptions-in-DNS-tests-should-not-have-data-att.patch
Type: text/x-patch
Size: 1616 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160606/46cc328a/attachment.bin>


More information about the Freeipa-devel mailing list