On 7.5.2015 18:12, Martin Basti wrote: > On 07/05/15 12:19, Petr Spacek wrote: >> On 7.5.2015 08:59, David Kupka wrote: >>> On 05/06/2015 03:20 PM, Martin Basti wrote: >>>> On 05/05/15 15:00, Martin Basti wrote: >>>>> On 30/04/15 15:37, David Kupka wrote: >>>>>> On 04/24/2015 02:56 PM, Martin Basti wrote: >>>>>>> Patches attached. >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>> Hi, >>>>>> thanks for patches. >>>>>> >>>>>> 1. You changed message in DNSServerNotRespondingWarning class but not >>>>>> the test in ipatest/test_xmlrpc/test_dns_plugin.py >>>>>> >>>>>> nitpick. Please spell 'edns' correctly. I've seen several instances >>>>>> of 'ends'. >>>>>> >>>>> Thank you, >>>>> >>>>> updated patches attached: >>>>> * new error messages >>>>> * logging to debug log server output if exception was raised >>>>> * fixed test >>>>> * fixed spelling >>>>> >>>>> >>>>> >>>> Fixed tests (again) >>>> >>>> Updated patches attached >>>> >>> The code looks good to me and tests are no longer broken. (I would prefer >>> better fix of the tests but given that the priorities are different now it >>> can >>> wait.) >>> >>> Petr, can you please confirm that the patch set works for you? >> Sorry, NACK: >> >> $ ipa dnsforwardzone-add ptr.test. --forwarder=10.34.47.236 >> Server will check DNS forwarder(s). >> This may take some time, please wait ... >> ipa: ERROR: an internal error has occurred >> >> # /var/log/httpd/error_log >> ipa: ERROR: non-public: AssertionError: >> Traceback (most recent call last): >> File "/usr/lib/python2.7/site-packages/ipaserver/rpcserver.py", line 350, >> in >> wsgi_execute >> result = self.Command[name](*args, **options) >> File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 443, in >> __call__ >> ret = self.run(*args, **options) >> File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 760, in >> run >> return self.execute(*args, **options) >> File "/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line 4444, >> in >> execute >> **options) >> File "/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line 4405, >> in >> _warning_if_forwarders_do_not_work >> log=self.log) >> File "/usr/lib/python2.7/site-packages/ipalib/util.py", line 715, in >> validate_dnssec_zone_forwarder_step2 >> timeout=timeout) >> File "/usr/lib/python2.7/site-packages/ipalib/util.py", line 610, in >> _resolve_record >> assert isinstance(nameserver_ip, basestring) >> AssertionError >> ipa: INFO: [jsonserver_session] admin@IPA.EXAMPLE: dnsforwardzone_add(<DNS >> name ptr.test.>, idnsforwarders=(u'10.34.47.236',), all=False, raw=False, >> version=u'2.116'): AssertionError >> >> This is constantly reproducible in my vm-090.abc. Let me know if you want to >> take a look. >> >> >> I'm attaching little response.patch which improves compatibility with older >> python-dns packages. This patch allows IPA to work while error messages are >> simply not as nice as they could be with latest python-dns :-) >> >> check_fwd_msg.patch is a little nitpick, just to make sure everyone >> understands the message. >> >> BTW why some messages in check_forwarders() are printed using 'print' and >> others using logger? I would prefer to use logger for everything to make sure >> that logs contain all the information, including warnings. >> >> Thank you for your time! >> > Thank you, fixed. > > I added missing except block after forwarders validation step2.
I confirm that this works but I just discovered another deficiency. Setup: - DNSSEC validation is enabled on IPA server - forwarders uses fake TLD, e.g. 'test.' - remote DNS server is responding, supports EDNS0 and so on $ ipa dnsforwardzone-add ptr.test. --forwarder=10.34.47.236 Server will check DNS forwarder(s). This may take some time, please wait ... ipa: WARNING: DNS server 10.34.78.90: query 'ptr.test. SOA': The DNS query name does not exist: ptr.test.. Huh? Let's check named log: forward zone 'ptr.test': loaded validating ./SOA: got insecure response; parent indicates it should be secure Sometimes I get SERVFAIL from IPA server, too. Unfortunately this check was the main reason for writing this patchset so we need to improve it. Maybe validate_dnssec_zone_forwarder_step2() could special-case NXDOMAIN and print the DNSSEC-validation-failed error, too? The problem is that it could trigger some false positives because NXDOMAIN may simply be caused by a delay somewhere. Any ideas? By the way, this is also weird: $ ipa dnsforwardzone-add ptr.test. --forwarder=10.34.47.236 Server will check DNS forwarder(s). This may take some time, please wait ... ipa: ERROR: DNS forward zone with name "ptr.test." already exists Is it actually doing the check even if the forward zone exists already? (This is just nitpick, not a blocker!) -- Petr^2 Spacek -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
