Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
On 14.12.2015 16:31, David Kupka wrote: > On 14/12/15 15:25, David Kupka wrote: >> On 14/12/15 14:52, David Kupka wrote: >>> On 11/12/15 15:00, Petr Spacek wrote: On 11.12.2015 12:35, David Kupka wrote: > On 10/12/15 18:10, Petr Spacek wrote: >> On 10.12.2015 17:31, David Kupka wrote: >>> On 09/12/15 18:55, Petr Spacek wrote: On 9.12.2015 13:37, David Kupka wrote: > On 08/12/15 15:24, Petr Spacek wrote: >> On 8.12.2015 12:19, David Kupka wrote: >>> On 08/12/15 08:56, Petr Spacek wrote: On 7.12.2015 14:41, David Kupka wrote: > +def is_host_resolvable(fqdn): > +if not isinstance(fqdn, DNSName): > +fqdn = DNSName(fqdn) > +for rdtype in (rdatatype.A, rdatatype.): > +try: > +resolver.query(fqdn.make_absolute(), rdtype) > +except DNSException: > +continue > +else: > +return True > + > +return False > NACK, you are re-introducing duplicate function which was removed in 498471e4aed1367b72cd74d15811d0584a6ee268. Please amend the patch ASAP to use new verify_host_resolvable() function so I can test it and get it into 4.3. >>> Updated patches attached. >> >> Hmm, my review script screams: >> >> Detected base branch: origin/master >> Checks will be made against following base commit: 848912a add >> missing >> /ipaplatform/constants.py to .gitignore >> Writing API to API.txt >> ./ipalib/plugins/dns.py:2127:9: E124 closing bracket does not >> match visual >> indentation >> ./ipalib/plugins/dns.py:2711:13: E128 continuation line >> under-indented for >> visual indent >> ./ipalib/plugins/dns.py:2713:9: E124 closing bracket does not >> match visual >> indentation >> ./ipalib/plugins/dns.py:2716:13: E128 continuation line >> under-indented for >> visual indent >> ./ipapython/ipautil.py:928:1: E302 expected 2 blank lines, found 1 >> ./ipapython/ipautil.py:948:17: E128 continuation line >> under-indented for >> visual indent >> ./ipapython/ipautil.py:956:1: E302 expected 2 blank lines, found 1 >> ./ipapython/ipautil.py:997:80: E501 line too long (83 > 79 >> characters) >> ./ipapython/ipautil.py:998:80: E501 line too long (83 > 79 >> characters) >> ./ipaserver/install/bindinstance.py:49:9: E128 continuation line >> under-indented for visual indent >> ./ipaserver/install/bindinstance.py:292:80: E501 line too long >> (80 > 79 >> characters) >> ./ipaserver/install/bindinstance.py:438:19: E128 continuation line >> under-indented for visual indent >> ./ipaserver/install/server/common.py:271:63: E261 at least two >> spaces >> before >> inline comment >> ./ipaserver/install/server/common.py:271:80: E501 line too long >> (90 > 79 >> characters) >> ERROR: PEP8 --diff failed, continuing ... >> ERROR: API.txt was changed without a change in VERSION, >> continuing ... >> Summary of detected errors: >> ERROR: PEP8 --diff failed >> ERROR: API.txt was changed without a change in VERSION >> >> Please fix it :-) >> >> Make make this more pleasant for you, you can use (and review) >> my attached >> patch. It adds 'review' target to Makefile, it will do the same >> checks as >> I do. >> > > Unfortunately your tool does not work for me (output w/ -o xtrace > attached). > Anyway I have incremented API version and fixed most of the pep8 > errors > except > for E124 twice in ipalib/plugins/dns.py as it seems to be > convention in the > file and E501 twice in ipapython/ipautil.py because IMO breaking > string > constant into multiple lines does not help readability. > > Updated patches also attached. We are almost there, but NACK for now. 1) Following attempt to install IPA explodes. Please note that dom-245.idm.lab.eng.brq.redhat.com DNS domain is delegated to the IPA server before installation is started, so it gives 'timeout' or possibly SERVFAIL. # ipa-server-install --ds-password=root4lab --admin-password=root4lab --setup-dns --forwarder=10.38.5.26 --no-reverse --allow-zone-overlap --domain=dom-245.idm.lab.eng.brq.redhat.com
Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
On 14/12/15 15:25, David Kupka wrote: On 14/12/15 14:52, David Kupka wrote: On 11/12/15 15:00, Petr Spacek wrote: On 11.12.2015 12:35, David Kupka wrote: On 10/12/15 18:10, Petr Spacek wrote: On 10.12.2015 17:31, David Kupka wrote: On 09/12/15 18:55, Petr Spacek wrote: On 9.12.2015 13:37, David Kupka wrote: On 08/12/15 15:24, Petr Spacek wrote: On 8.12.2015 12:19, David Kupka wrote: On 08/12/15 08:56, Petr Spacek wrote: On 7.12.2015 14:41, David Kupka wrote: +def is_host_resolvable(fqdn): +if not isinstance(fqdn, DNSName): +fqdn = DNSName(fqdn) +for rdtype in (rdatatype.A, rdatatype.): +try: +resolver.query(fqdn.make_absolute(), rdtype) +except DNSException: +continue +else: +return True + +return False NACK, you are re-introducing duplicate function which was removed in 498471e4aed1367b72cd74d15811d0584a6ee268. Please amend the patch ASAP to use new verify_host_resolvable() function so I can test it and get it into 4.3. Updated patches attached. Hmm, my review script screams: Detected base branch: origin/master Checks will be made against following base commit: 848912a add missing /ipaplatform/constants.py to .gitignore Writing API to API.txt ./ipalib/plugins/dns.py:2127:9: E124 closing bracket does not match visual indentation ./ipalib/plugins/dns.py:2711:13: E128 continuation line under-indented for visual indent ./ipalib/plugins/dns.py:2713:9: E124 closing bracket does not match visual indentation ./ipalib/plugins/dns.py:2716:13: E128 continuation line under-indented for visual indent ./ipapython/ipautil.py:928:1: E302 expected 2 blank lines, found 1 ./ipapython/ipautil.py:948:17: E128 continuation line under-indented for visual indent ./ipapython/ipautil.py:956:1: E302 expected 2 blank lines, found 1 ./ipapython/ipautil.py:997:80: E501 line too long (83 > 79 characters) ./ipapython/ipautil.py:998:80: E501 line too long (83 > 79 characters) ./ipaserver/install/bindinstance.py:49:9: E128 continuation line under-indented for visual indent ./ipaserver/install/bindinstance.py:292:80: E501 line too long (80 > 79 characters) ./ipaserver/install/bindinstance.py:438:19: E128 continuation line under-indented for visual indent ./ipaserver/install/server/common.py:271:63: E261 at least two spaces before inline comment ./ipaserver/install/server/common.py:271:80: E501 line too long (90 > 79 characters) ERROR: PEP8 --diff failed, continuing ... ERROR: API.txt was changed without a change in VERSION, continuing ... Summary of detected errors: ERROR: PEP8 --diff failed ERROR: API.txt was changed without a change in VERSION Please fix it :-) Make make this more pleasant for you, you can use (and review) my attached patch. It adds 'review' target to Makefile, it will do the same checks as I do. Unfortunately your tool does not work for me (output w/ -o xtrace attached). Anyway I have incremented API version and fixed most of the pep8 errors except for E124 twice in ipalib/plugins/dns.py as it seems to be convention in the file and E501 twice in ipapython/ipautil.py because IMO breaking string constant into multiple lines does not help readability. Updated patches also attached. We are almost there, but NACK for now. 1) Following attempt to install IPA explodes. Please note that dom-245.idm.lab.eng.brq.redhat.com DNS domain is delegated to the IPA server before installation is started, so it gives 'timeout' or possibly SERVFAIL. # ipa-server-install --ds-password=root4lab --admin-password=root4lab --setup-dns --forwarder=10.38.5.26 --no-reverse --allow-zone-overlap --domain=dom-245.idm.lab.eng.brq.redhat.com --realm=DOM-245.IDM.LAB.ENG.BRQ.REDHAT.COM [...] Configuring DNS (named) [1/11]: generating rndc key file [2/11]: adding DNS container [3/11]: setting up our zone [error] InvocationError: DNS check for domain dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after 30.000453949 seconds. Please make sure that the domain is properly delegated to this IPA server. ipa.ipapython.install.cli.install_tool(Server): ERRORDNS check for domain dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after 30.000453949 seconds. Please make sure that the domain is properly delegated to this IPA server. ipa.ipapython.install.cli.install_tool(Server): ERRORThe ipa-server-install command failed. See /var/log/ipaserver-install.log for more information 2015-12-09T17:15:37Z DEBUG File "/usr/lib/python2.7/site-packages/ipapython/admintool.py", line 171, in execute return_value = self.run() File "/usr/lib/python2.7/site-packages/ipapython/install/cli.py", line 318, in run cfgr.run() File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 310, in run self.execute() File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 332, in execute for nothing in self._executor():
Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
On 14/12/15 14:52, David Kupka wrote: On 11/12/15 15:00, Petr Spacek wrote: On 11.12.2015 12:35, David Kupka wrote: On 10/12/15 18:10, Petr Spacek wrote: On 10.12.2015 17:31, David Kupka wrote: On 09/12/15 18:55, Petr Spacek wrote: On 9.12.2015 13:37, David Kupka wrote: On 08/12/15 15:24, Petr Spacek wrote: On 8.12.2015 12:19, David Kupka wrote: On 08/12/15 08:56, Petr Spacek wrote: On 7.12.2015 14:41, David Kupka wrote: +def is_host_resolvable(fqdn): +if not isinstance(fqdn, DNSName): +fqdn = DNSName(fqdn) +for rdtype in (rdatatype.A, rdatatype.): +try: +resolver.query(fqdn.make_absolute(), rdtype) +except DNSException: +continue +else: +return True + +return False NACK, you are re-introducing duplicate function which was removed in 498471e4aed1367b72cd74d15811d0584a6ee268. Please amend the patch ASAP to use new verify_host_resolvable() function so I can test it and get it into 4.3. Updated patches attached. Hmm, my review script screams: Detected base branch: origin/master Checks will be made against following base commit: 848912a add missing /ipaplatform/constants.py to .gitignore Writing API to API.txt ./ipalib/plugins/dns.py:2127:9: E124 closing bracket does not match visual indentation ./ipalib/plugins/dns.py:2711:13: E128 continuation line under-indented for visual indent ./ipalib/plugins/dns.py:2713:9: E124 closing bracket does not match visual indentation ./ipalib/plugins/dns.py:2716:13: E128 continuation line under-indented for visual indent ./ipapython/ipautil.py:928:1: E302 expected 2 blank lines, found 1 ./ipapython/ipautil.py:948:17: E128 continuation line under-indented for visual indent ./ipapython/ipautil.py:956:1: E302 expected 2 blank lines, found 1 ./ipapython/ipautil.py:997:80: E501 line too long (83 > 79 characters) ./ipapython/ipautil.py:998:80: E501 line too long (83 > 79 characters) ./ipaserver/install/bindinstance.py:49:9: E128 continuation line under-indented for visual indent ./ipaserver/install/bindinstance.py:292:80: E501 line too long (80 > 79 characters) ./ipaserver/install/bindinstance.py:438:19: E128 continuation line under-indented for visual indent ./ipaserver/install/server/common.py:271:63: E261 at least two spaces before inline comment ./ipaserver/install/server/common.py:271:80: E501 line too long (90 > 79 characters) ERROR: PEP8 --diff failed, continuing ... ERROR: API.txt was changed without a change in VERSION, continuing ... Summary of detected errors: ERROR: PEP8 --diff failed ERROR: API.txt was changed without a change in VERSION Please fix it :-) Make make this more pleasant for you, you can use (and review) my attached patch. It adds 'review' target to Makefile, it will do the same checks as I do. Unfortunately your tool does not work for me (output w/ -o xtrace attached). Anyway I have incremented API version and fixed most of the pep8 errors except for E124 twice in ipalib/plugins/dns.py as it seems to be convention in the file and E501 twice in ipapython/ipautil.py because IMO breaking string constant into multiple lines does not help readability. Updated patches also attached. We are almost there, but NACK for now. 1) Following attempt to install IPA explodes. Please note that dom-245.idm.lab.eng.brq.redhat.com DNS domain is delegated to the IPA server before installation is started, so it gives 'timeout' or possibly SERVFAIL. # ipa-server-install --ds-password=root4lab --admin-password=root4lab --setup-dns --forwarder=10.38.5.26 --no-reverse --allow-zone-overlap --domain=dom-245.idm.lab.eng.brq.redhat.com --realm=DOM-245.IDM.LAB.ENG.BRQ.REDHAT.COM [...] Configuring DNS (named) [1/11]: generating rndc key file [2/11]: adding DNS container [3/11]: setting up our zone [error] InvocationError: DNS check for domain dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after 30.000453949 seconds. Please make sure that the domain is properly delegated to this IPA server. ipa.ipapython.install.cli.install_tool(Server): ERRORDNS check for domain dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after 30.000453949 seconds. Please make sure that the domain is properly delegated to this IPA server. ipa.ipapython.install.cli.install_tool(Server): ERRORThe ipa-server-install command failed. See /var/log/ipaserver-install.log for more information 2015-12-09T17:15:37Z DEBUG File "/usr/lib/python2.7/site-packages/ipapython/admintool.py", line 171, in execute return_value = self.run() File "/usr/lib/python2.7/site-packages/ipapython/install/cli.py", line 318, in run cfgr.run() File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 310, in run self.execute() File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 332, in execute for nothing in self._executor(): File "/usr/lib/python2.7/site-package
Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
On 11/12/15 15:00, Petr Spacek wrote: On 11.12.2015 12:35, David Kupka wrote: On 10/12/15 18:10, Petr Spacek wrote: On 10.12.2015 17:31, David Kupka wrote: On 09/12/15 18:55, Petr Spacek wrote: On 9.12.2015 13:37, David Kupka wrote: On 08/12/15 15:24, Petr Spacek wrote: On 8.12.2015 12:19, David Kupka wrote: On 08/12/15 08:56, Petr Spacek wrote: On 7.12.2015 14:41, David Kupka wrote: +def is_host_resolvable(fqdn): +if not isinstance(fqdn, DNSName): +fqdn = DNSName(fqdn) +for rdtype in (rdatatype.A, rdatatype.): +try: +resolver.query(fqdn.make_absolute(), rdtype) +except DNSException: +continue +else: +return True + +return False NACK, you are re-introducing duplicate function which was removed in 498471e4aed1367b72cd74d15811d0584a6ee268. Please amend the patch ASAP to use new verify_host_resolvable() function so I can test it and get it into 4.3. Updated patches attached. Hmm, my review script screams: Detected base branch: origin/master Checks will be made against following base commit: 848912a add missing /ipaplatform/constants.py to .gitignore Writing API to API.txt ./ipalib/plugins/dns.py:2127:9: E124 closing bracket does not match visual indentation ./ipalib/plugins/dns.py:2711:13: E128 continuation line under-indented for visual indent ./ipalib/plugins/dns.py:2713:9: E124 closing bracket does not match visual indentation ./ipalib/plugins/dns.py:2716:13: E128 continuation line under-indented for visual indent ./ipapython/ipautil.py:928:1: E302 expected 2 blank lines, found 1 ./ipapython/ipautil.py:948:17: E128 continuation line under-indented for visual indent ./ipapython/ipautil.py:956:1: E302 expected 2 blank lines, found 1 ./ipapython/ipautil.py:997:80: E501 line too long (83 > 79 characters) ./ipapython/ipautil.py:998:80: E501 line too long (83 > 79 characters) ./ipaserver/install/bindinstance.py:49:9: E128 continuation line under-indented for visual indent ./ipaserver/install/bindinstance.py:292:80: E501 line too long (80 > 79 characters) ./ipaserver/install/bindinstance.py:438:19: E128 continuation line under-indented for visual indent ./ipaserver/install/server/common.py:271:63: E261 at least two spaces before inline comment ./ipaserver/install/server/common.py:271:80: E501 line too long (90 > 79 characters) ERROR: PEP8 --diff failed, continuing ... ERROR: API.txt was changed without a change in VERSION, continuing ... Summary of detected errors: ERROR: PEP8 --diff failed ERROR: API.txt was changed without a change in VERSION Please fix it :-) Make make this more pleasant for you, you can use (and review) my attached patch. It adds 'review' target to Makefile, it will do the same checks as I do. Unfortunately your tool does not work for me (output w/ -o xtrace attached). Anyway I have incremented API version and fixed most of the pep8 errors except for E124 twice in ipalib/plugins/dns.py as it seems to be convention in the file and E501 twice in ipapython/ipautil.py because IMO breaking string constant into multiple lines does not help readability. Updated patches also attached. We are almost there, but NACK for now. 1) Following attempt to install IPA explodes. Please note that dom-245.idm.lab.eng.brq.redhat.com DNS domain is delegated to the IPA server before installation is started, so it gives 'timeout' or possibly SERVFAIL. # ipa-server-install --ds-password=root4lab --admin-password=root4lab --setup-dns --forwarder=10.38.5.26 --no-reverse --allow-zone-overlap --domain=dom-245.idm.lab.eng.brq.redhat.com --realm=DOM-245.IDM.LAB.ENG.BRQ.REDHAT.COM [...] Configuring DNS (named) [1/11]: generating rndc key file [2/11]: adding DNS container [3/11]: setting up our zone [error] InvocationError: DNS check for domain dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after 30.000453949 seconds. Please make sure that the domain is properly delegated to this IPA server. ipa.ipapython.install.cli.install_tool(Server): ERRORDNS check for domain dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after 30.000453949 seconds. Please make sure that the domain is properly delegated to this IPA server. ipa.ipapython.install.cli.install_tool(Server): ERRORThe ipa-server-install command failed. See /var/log/ipaserver-install.log for more information 2015-12-09T17:15:37Z DEBUG File "/usr/lib/python2.7/site-packages/ipapython/admintool.py", line 171, in execute return_value = self.run() File "/usr/lib/python2.7/site-packages/ipapython/install/cli.py", line 318, in run cfgr.run() File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 310, in run self.execute() File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 332, in execute for nothing in self._executor(): File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 372,
Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
On 9.12.2015 13:37, David Kupka wrote: > On 08/12/15 15:24, Petr Spacek wrote: >> On 8.12.2015 12:19, David Kupka wrote: >>> On 08/12/15 08:56, Petr Spacek wrote: On 7.12.2015 14:41, David Kupka wrote: > +def is_host_resolvable(fqdn): > +if not isinstance(fqdn, DNSName): > +fqdn = DNSName(fqdn) > +for rdtype in (rdatatype.A, rdatatype.): > +try: > +resolver.query(fqdn.make_absolute(), rdtype) > +except DNSException: > +continue > +else: > +return True > + > +return False > NACK, you are re-introducing duplicate function which was removed in 498471e4aed1367b72cd74d15811d0584a6ee268. Please amend the patch ASAP to use new verify_host_resolvable() function so I can test it and get it into 4.3. >>> Updated patches attached. >> >> Hmm, my review script screams: >> >> Detected base branch: origin/master >> Checks will be made against following base commit: 848912a add missing >> /ipaplatform/constants.py to .gitignore >> Writing API to API.txt >> ./ipalib/plugins/dns.py:2127:9: E124 closing bracket does not match visual >> indentation >> ./ipalib/plugins/dns.py:2711:13: E128 continuation line under-indented for >> visual indent >> ./ipalib/plugins/dns.py:2713:9: E124 closing bracket does not match visual >> indentation >> ./ipalib/plugins/dns.py:2716:13: E128 continuation line under-indented for >> visual indent >> ./ipapython/ipautil.py:928:1: E302 expected 2 blank lines, found 1 >> ./ipapython/ipautil.py:948:17: E128 continuation line under-indented for >> visual indent >> ./ipapython/ipautil.py:956:1: E302 expected 2 blank lines, found 1 >> ./ipapython/ipautil.py:997:80: E501 line too long (83 > 79 characters) >> ./ipapython/ipautil.py:998:80: E501 line too long (83 > 79 characters) >> ./ipaserver/install/bindinstance.py:49:9: E128 continuation line >> under-indented for visual indent >> ./ipaserver/install/bindinstance.py:292:80: E501 line too long (80 > 79 >> characters) >> ./ipaserver/install/bindinstance.py:438:19: E128 continuation line >> under-indented for visual indent >> ./ipaserver/install/server/common.py:271:63: E261 at least two spaces before >> inline comment >> ./ipaserver/install/server/common.py:271:80: E501 line too long (90 > 79 >> characters) >> ERROR: PEP8 --diff failed, continuing ... >> ERROR: API.txt was changed without a change in VERSION, continuing ... >> Summary of detected errors: >> ERROR: PEP8 --diff failed >> ERROR: API.txt was changed without a change in VERSION >> >> Please fix it :-) >> >> Make make this more pleasant for you, you can use (and review) my attached >> patch. It adds 'review' target to Makefile, it will do the same checks as I >> do. >> > > Unfortunately your tool does not work for me (output w/ -o xtrace attached). > Anyway I have incremented API version and fixed most of the pep8 errors except > for E124 twice in ipalib/plugins/dns.py as it seems to be convention in the > file and E501 twice in ipapython/ipautil.py because IMO breaking string > constant into multiple lines does not help readability. > > Updated patches also attached. We are almost there, but NACK for now. 1) Following attempt to install IPA explodes. Please note that dom-245.idm.lab.eng.brq.redhat.com DNS domain is delegated to the IPA server before installation is started, so it gives 'timeout' or possibly SERVFAIL. # ipa-server-install --ds-password=root4lab --admin-password=root4lab --setup-dns --forwarder=10.38.5.26 --no-reverse --allow-zone-overlap --domain=dom-245.idm.lab.eng.brq.redhat.com --realm=DOM-245.IDM.LAB.ENG.BRQ.REDHAT.COM [...] Configuring DNS (named) [1/11]: generating rndc key file [2/11]: adding DNS container [3/11]: setting up our zone [error] InvocationError: DNS check for domain dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after 30.000453949 seconds. Please make sure that the domain is properly delegated to this IPA server. ipa.ipapython.install.cli.install_tool(Server): ERRORDNS check for domain dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after 30.000453949 seconds. Please make sure that the domain is properly delegated to this IPA server. ipa.ipapython.install.cli.install_tool(Server): ERRORThe ipa-server-install command failed. See /var/log/ipaserver-install.log for more information 2015-12-09T17:15:37Z DEBUG File "/usr/lib/python2.7/site-packages/ipapython/admintool.py", line 171, in execute return_value = self.run() File "/usr/lib/python2.7/site-packages/ipapython/install/cli.py", line 318, in run cfgr.run() File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 310, in run self.execute() File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 332, in execute for nothing in self._executor(): File "/usr/lib/python2.7/site-packages/ipapython/insta
Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
On 08/12/15 15:24, Petr Spacek wrote:
On 8.12.2015 12:19, David Kupka wrote:
On 08/12/15 08:56, Petr Spacek wrote:
On 7.12.2015 14:41, David Kupka wrote:
+def is_host_resolvable(fqdn):
+if not isinstance(fqdn, DNSName):
+fqdn = DNSName(fqdn)
+for rdtype in (rdatatype.A, rdatatype.):
+try:
+resolver.query(fqdn.make_absolute(), rdtype)
+except DNSException:
+continue
+else:
+return True
+
+return False
NACK, you are re-introducing duplicate function which was removed in
498471e4aed1367b72cd74d15811d0584a6ee268.
Please amend the patch ASAP to use new verify_host_resolvable() function so I
can test it and get it into 4.3.
Updated patches attached.
Hmm, my review script screams:
Detected base branch: origin/master
Checks will be made against following base commit: 848912a add missing
/ipaplatform/constants.py to .gitignore
Writing API to API.txt
./ipalib/plugins/dns.py:2127:9: E124 closing bracket does not match visual
indentation
./ipalib/plugins/dns.py:2711:13: E128 continuation line under-indented for
visual indent
./ipalib/plugins/dns.py:2713:9: E124 closing bracket does not match visual
indentation
./ipalib/plugins/dns.py:2716:13: E128 continuation line under-indented for
visual indent
./ipapython/ipautil.py:928:1: E302 expected 2 blank lines, found 1
./ipapython/ipautil.py:948:17: E128 continuation line under-indented for
visual indent
./ipapython/ipautil.py:956:1: E302 expected 2 blank lines, found 1
./ipapython/ipautil.py:997:80: E501 line too long (83 > 79 characters)
./ipapython/ipautil.py:998:80: E501 line too long (83 > 79 characters)
./ipaserver/install/bindinstance.py:49:9: E128 continuation line
under-indented for visual indent
./ipaserver/install/bindinstance.py:292:80: E501 line too long (80 > 79
characters)
./ipaserver/install/bindinstance.py:438:19: E128 continuation line
under-indented for visual indent
./ipaserver/install/server/common.py:271:63: E261 at least two spaces before
inline comment
./ipaserver/install/server/common.py:271:80: E501 line too long (90 > 79
characters)
ERROR: PEP8 --diff failed, continuing ...
ERROR: API.txt was changed without a change in VERSION, continuing ...
Summary of detected errors:
ERROR: PEP8 --diff failed
ERROR: API.txt was changed without a change in VERSION
Please fix it :-)
Make make this more pleasant for you, you can use (and review) my attached
patch. It adds 'review' target to Makefile, it will do the same checks as I do.
Unfortunately your tool does not work for me (output w/ -o xtrace attached).
Anyway I have incremented API version and fixed most of the pep8 errors
except for E124 twice in ipalib/plugins/dns.py as it seems to be
convention in the file and E501 twice in ipapython/ipautil.py because
IMO breaking string constant into multiple lines does not help readability.
Updated patches also attached.
--
David Kupka
From 4fb72e2b197d365b99bb426fa4b4919c60efe44b Mon Sep 17 00:00:00 2001
From: David Kupka
Date: Wed, 2 Dec 2015 13:17:13 +
Subject: [PATCH] dns: do not add (forward)zone if it is already resolvable.
Check if the zone user wants to add is already resolvable and refuse to
create it if yes. --skip-overlap-check and --force options suppress this check.
https://fedorahosted.org/freeipa/ticket/5087
---
API.txt | 7 ++--
VERSION | 2 +-
ipalib/plugins/dns.py | 32 +++---
ipapython/ipautil.py | 89 +--
4 files changed, 120 insertions(+), 10 deletions(-)
diff --git a/API.txt b/API.txt
index 60c98c31aa85d6c8879cd145f3d84188d4fea5b7..3a9fb65a386a2a6529b8cd241642446c135471f2 100644
--- a/API.txt
+++ b/API.txt
@@ -959,7 +959,7 @@ output: Entry('result', , Gettext('A dictionary representing an LDA
output: Output('summary', (, ), None)
output: PrimaryKey('value', None, None)
command: dnsforwardzone_add
-args: 1,8,3
+args: 1,9,3
arg: DNSNameParam('idnsname', attribute=True, cli_name='name', multivalue=False, only_absolute=True, primary_key=True, required=True)
option: Str('addattr*', cli_name='addattr', exclude='webui')
option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
@@ -968,6 +968,7 @@ option: StrEnum('idnsforwardpolicy', attribute=True, cli_name='forward_policy',
option: Str('name_from_ip', attribute=False, cli_name='name_from_ip', multivalue=False, required=False)
option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
option: Str('setattr*', cli_name='setattr', exclude='webui')
+option: Flag('skip_overlap_check', autofill=True, default=False)
option: Str('version?', exclude='webui')
output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
output: Output('summary', (, ), None)
@@ -1366,7 +1367,7 @@ output: Entry('result', , Gettext('A dictionary representing an LDA
output: Output('summary', (, ), None)
output: Pr
Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
On 11.12.2015 12:35, David Kupka wrote: > On 10/12/15 18:10, Petr Spacek wrote: >> On 10.12.2015 17:31, David Kupka wrote: >>> On 09/12/15 18:55, Petr Spacek wrote: On 9.12.2015 13:37, David Kupka wrote: > On 08/12/15 15:24, Petr Spacek wrote: >> On 8.12.2015 12:19, David Kupka wrote: >>> On 08/12/15 08:56, Petr Spacek wrote: On 7.12.2015 14:41, David Kupka wrote: > +def is_host_resolvable(fqdn): > +if not isinstance(fqdn, DNSName): > +fqdn = DNSName(fqdn) > +for rdtype in (rdatatype.A, rdatatype.): > +try: > +resolver.query(fqdn.make_absolute(), rdtype) > +except DNSException: > +continue > +else: > +return True > + > +return False > NACK, you are re-introducing duplicate function which was removed in 498471e4aed1367b72cd74d15811d0584a6ee268. Please amend the patch ASAP to use new verify_host_resolvable() function so I can test it and get it into 4.3. >>> Updated patches attached. >> >> Hmm, my review script screams: >> >> Detected base branch: origin/master >> Checks will be made against following base commit: 848912a add missing >> /ipaplatform/constants.py to .gitignore >> Writing API to API.txt >> ./ipalib/plugins/dns.py:2127:9: E124 closing bracket does not match >> visual >> indentation >> ./ipalib/plugins/dns.py:2711:13: E128 continuation line under-indented >> for >> visual indent >> ./ipalib/plugins/dns.py:2713:9: E124 closing bracket does not match >> visual >> indentation >> ./ipalib/plugins/dns.py:2716:13: E128 continuation line under-indented >> for >> visual indent >> ./ipapython/ipautil.py:928:1: E302 expected 2 blank lines, found 1 >> ./ipapython/ipautil.py:948:17: E128 continuation line under-indented for >> visual indent >> ./ipapython/ipautil.py:956:1: E302 expected 2 blank lines, found 1 >> ./ipapython/ipautil.py:997:80: E501 line too long (83 > 79 characters) >> ./ipapython/ipautil.py:998:80: E501 line too long (83 > 79 characters) >> ./ipaserver/install/bindinstance.py:49:9: E128 continuation line >> under-indented for visual indent >> ./ipaserver/install/bindinstance.py:292:80: E501 line too long (80 > 79 >> characters) >> ./ipaserver/install/bindinstance.py:438:19: E128 continuation line >> under-indented for visual indent >> ./ipaserver/install/server/common.py:271:63: E261 at least two spaces >> before >> inline comment >> ./ipaserver/install/server/common.py:271:80: E501 line too long (90 > 79 >> characters) >> ERROR: PEP8 --diff failed, continuing ... >> ERROR: API.txt was changed without a change in VERSION, continuing ... >> Summary of detected errors: >> ERROR: PEP8 --diff failed >> ERROR: API.txt was changed without a change in VERSION >> >> Please fix it :-) >> >> Make make this more pleasant for you, you can use (and review) my >> attached >> patch. It adds 'review' target to Makefile, it will do the same checks as >> I do. >> > > Unfortunately your tool does not work for me (output w/ -o xtrace > attached). > Anyway I have incremented API version and fixed most of the pep8 errors > except > for E124 twice in ipalib/plugins/dns.py as it seems to be convention in > the > file and E501 twice in ipapython/ipautil.py because IMO breaking string > constant into multiple lines does not help readability. > > Updated patches also attached. We are almost there, but NACK for now. 1) Following attempt to install IPA explodes. Please note that dom-245.idm.lab.eng.brq.redhat.com DNS domain is delegated to the IPA server before installation is started, so it gives 'timeout' or possibly SERVFAIL. # ipa-server-install --ds-password=root4lab --admin-password=root4lab --setup-dns --forwarder=10.38.5.26 --no-reverse --allow-zone-overlap --domain=dom-245.idm.lab.eng.brq.redhat.com --realm=DOM-245.IDM.LAB.ENG.BRQ.REDHAT.COM [...] Configuring DNS (named) [1/11]: generating rndc key file [2/11]: adding DNS container [3/11]: setting up our zone [error] InvocationError: DNS check for domain dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after 30.000453949 seconds. Please make sure that the domain is properly delegated to this IPA server. ipa.ipapython.install.cli.install_tool(Server): ERRORDNS check for domain dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after 30.000453949 seconds. Please make sure that the domain is properly
Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
On 10/12/15 18:10, Petr Spacek wrote: On 10.12.2015 17:31, David Kupka wrote: On 09/12/15 18:55, Petr Spacek wrote: On 9.12.2015 13:37, David Kupka wrote: On 08/12/15 15:24, Petr Spacek wrote: On 8.12.2015 12:19, David Kupka wrote: On 08/12/15 08:56, Petr Spacek wrote: On 7.12.2015 14:41, David Kupka wrote: +def is_host_resolvable(fqdn): +if not isinstance(fqdn, DNSName): +fqdn = DNSName(fqdn) +for rdtype in (rdatatype.A, rdatatype.): +try: +resolver.query(fqdn.make_absolute(), rdtype) +except DNSException: +continue +else: +return True + +return False NACK, you are re-introducing duplicate function which was removed in 498471e4aed1367b72cd74d15811d0584a6ee268. Please amend the patch ASAP to use new verify_host_resolvable() function so I can test it and get it into 4.3. Updated patches attached. Hmm, my review script screams: Detected base branch: origin/master Checks will be made against following base commit: 848912a add missing /ipaplatform/constants.py to .gitignore Writing API to API.txt ./ipalib/plugins/dns.py:2127:9: E124 closing bracket does not match visual indentation ./ipalib/plugins/dns.py:2711:13: E128 continuation line under-indented for visual indent ./ipalib/plugins/dns.py:2713:9: E124 closing bracket does not match visual indentation ./ipalib/plugins/dns.py:2716:13: E128 continuation line under-indented for visual indent ./ipapython/ipautil.py:928:1: E302 expected 2 blank lines, found 1 ./ipapython/ipautil.py:948:17: E128 continuation line under-indented for visual indent ./ipapython/ipautil.py:956:1: E302 expected 2 blank lines, found 1 ./ipapython/ipautil.py:997:80: E501 line too long (83 > 79 characters) ./ipapython/ipautil.py:998:80: E501 line too long (83 > 79 characters) ./ipaserver/install/bindinstance.py:49:9: E128 continuation line under-indented for visual indent ./ipaserver/install/bindinstance.py:292:80: E501 line too long (80 > 79 characters) ./ipaserver/install/bindinstance.py:438:19: E128 continuation line under-indented for visual indent ./ipaserver/install/server/common.py:271:63: E261 at least two spaces before inline comment ./ipaserver/install/server/common.py:271:80: E501 line too long (90 > 79 characters) ERROR: PEP8 --diff failed, continuing ... ERROR: API.txt was changed without a change in VERSION, continuing ... Summary of detected errors: ERROR: PEP8 --diff failed ERROR: API.txt was changed without a change in VERSION Please fix it :-) Make make this more pleasant for you, you can use (and review) my attached patch. It adds 'review' target to Makefile, it will do the same checks as I do. Unfortunately your tool does not work for me (output w/ -o xtrace attached). Anyway I have incremented API version and fixed most of the pep8 errors except for E124 twice in ipalib/plugins/dns.py as it seems to be convention in the file and E501 twice in ipapython/ipautil.py because IMO breaking string constant into multiple lines does not help readability. Updated patches also attached. We are almost there, but NACK for now. 1) Following attempt to install IPA explodes. Please note that dom-245.idm.lab.eng.brq.redhat.com DNS domain is delegated to the IPA server before installation is started, so it gives 'timeout' or possibly SERVFAIL. # ipa-server-install --ds-password=root4lab --admin-password=root4lab --setup-dns --forwarder=10.38.5.26 --no-reverse --allow-zone-overlap --domain=dom-245.idm.lab.eng.brq.redhat.com --realm=DOM-245.IDM.LAB.ENG.BRQ.REDHAT.COM [...] Configuring DNS (named) [1/11]: generating rndc key file [2/11]: adding DNS container [3/11]: setting up our zone [error] InvocationError: DNS check for domain dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after 30.000453949 seconds. Please make sure that the domain is properly delegated to this IPA server. ipa.ipapython.install.cli.install_tool(Server): ERRORDNS check for domain dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after 30.000453949 seconds. Please make sure that the domain is properly delegated to this IPA server. ipa.ipapython.install.cli.install_tool(Server): ERRORThe ipa-server-install command failed. See /var/log/ipaserver-install.log for more information 2015-12-09T17:15:37Z DEBUG File "/usr/lib/python2.7/site-packages/ipapython/admintool.py", line 171, in execute return_value = self.run() File "/usr/lib/python2.7/site-packages/ipapython/install/cli.py", line 318, in run cfgr.run() File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 310, in run self.execute() File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 332, in execute for nothing in self._executor(): File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 372, in __runner self._handle_exception(exc_info) File "/usr/lib/python2.7/site-packages
Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
On 10.12.2015 17:31, David Kupka wrote: > On 09/12/15 18:55, Petr Spacek wrote: >> On 9.12.2015 13:37, David Kupka wrote: >>> On 08/12/15 15:24, Petr Spacek wrote: On 8.12.2015 12:19, David Kupka wrote: > On 08/12/15 08:56, Petr Spacek wrote: >> On 7.12.2015 14:41, David Kupka wrote: >>> +def is_host_resolvable(fqdn): >>> +if not isinstance(fqdn, DNSName): >>> +fqdn = DNSName(fqdn) >>> +for rdtype in (rdatatype.A, rdatatype.): >>> +try: >>> +resolver.query(fqdn.make_absolute(), rdtype) >>> +except DNSException: >>> +continue >>> +else: >>> +return True >>> + >>> +return False >>> >> >> NACK, you are re-introducing duplicate function which was removed in >> 498471e4aed1367b72cd74d15811d0584a6ee268. >> >> Please amend the patch ASAP to use new verify_host_resolvable() function >> so I >> can test it and get it into 4.3. >> > Updated patches attached. Hmm, my review script screams: Detected base branch: origin/master Checks will be made against following base commit: 848912a add missing /ipaplatform/constants.py to .gitignore Writing API to API.txt ./ipalib/plugins/dns.py:2127:9: E124 closing bracket does not match visual indentation ./ipalib/plugins/dns.py:2711:13: E128 continuation line under-indented for visual indent ./ipalib/plugins/dns.py:2713:9: E124 closing bracket does not match visual indentation ./ipalib/plugins/dns.py:2716:13: E128 continuation line under-indented for visual indent ./ipapython/ipautil.py:928:1: E302 expected 2 blank lines, found 1 ./ipapython/ipautil.py:948:17: E128 continuation line under-indented for visual indent ./ipapython/ipautil.py:956:1: E302 expected 2 blank lines, found 1 ./ipapython/ipautil.py:997:80: E501 line too long (83 > 79 characters) ./ipapython/ipautil.py:998:80: E501 line too long (83 > 79 characters) ./ipaserver/install/bindinstance.py:49:9: E128 continuation line under-indented for visual indent ./ipaserver/install/bindinstance.py:292:80: E501 line too long (80 > 79 characters) ./ipaserver/install/bindinstance.py:438:19: E128 continuation line under-indented for visual indent ./ipaserver/install/server/common.py:271:63: E261 at least two spaces before inline comment ./ipaserver/install/server/common.py:271:80: E501 line too long (90 > 79 characters) ERROR: PEP8 --diff failed, continuing ... ERROR: API.txt was changed without a change in VERSION, continuing ... Summary of detected errors: ERROR: PEP8 --diff failed ERROR: API.txt was changed without a change in VERSION Please fix it :-) Make make this more pleasant for you, you can use (and review) my attached patch. It adds 'review' target to Makefile, it will do the same checks as I do. >>> >>> Unfortunately your tool does not work for me (output w/ -o xtrace attached). >>> Anyway I have incremented API version and fixed most of the pep8 errors >>> except >>> for E124 twice in ipalib/plugins/dns.py as it seems to be convention in the >>> file and E501 twice in ipapython/ipautil.py because IMO breaking string >>> constant into multiple lines does not help readability. >>> >>> Updated patches also attached. >> >> We are almost there, but NACK for now. >> >> 1) Following attempt to install IPA explodes. Please note that >> dom-245.idm.lab.eng.brq.redhat.com DNS domain is delegated to the IPA server >> before installation is started, so it gives 'timeout' or possibly SERVFAIL. >> >> # ipa-server-install --ds-password=root4lab --admin-password=root4lab >> --setup-dns --forwarder=10.38.5.26 --no-reverse --allow-zone-overlap >> --domain=dom-245.idm.lab.eng.brq.redhat.com >> --realm=DOM-245.IDM.LAB.ENG.BRQ.REDHAT.COM >> [...] >> >> Configuring DNS (named) >>[1/11]: generating rndc key file >>[2/11]: adding DNS container >>[3/11]: setting up our zone >>[error] InvocationError: DNS check for domain >> dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after >> 30.000453949 seconds. Please make sure that the domain is properly delegated >> to this IPA server. >> ipa.ipapython.install.cli.install_tool(Server): ERRORDNS check for domain >> dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after >> 30.000453949 seconds. Please make sure that the domain is properly delegated >> to this IPA server. >> ipa.ipapython.install.cli.install_tool(Server): ERRORThe >> ipa-server-install command failed. See /var/log/ipaserver-install.log for >> more >> information >> >> 2015-12-09T17:15:37Z DEBUG File >> "/usr/lib/python2.7/site-packages/ipapython/admintool.py", line 171, in >> execute >> return_value = self.run() >>File "/usr/lib/python2.7/site-pa
Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
On 09/12/15 18:55, Petr Spacek wrote: On 9.12.2015 13:37, David Kupka wrote: On 08/12/15 15:24, Petr Spacek wrote: On 8.12.2015 12:19, David Kupka wrote: On 08/12/15 08:56, Petr Spacek wrote: On 7.12.2015 14:41, David Kupka wrote: +def is_host_resolvable(fqdn): +if not isinstance(fqdn, DNSName): +fqdn = DNSName(fqdn) +for rdtype in (rdatatype.A, rdatatype.): +try: +resolver.query(fqdn.make_absolute(), rdtype) +except DNSException: +continue +else: +return True + +return False NACK, you are re-introducing duplicate function which was removed in 498471e4aed1367b72cd74d15811d0584a6ee268. Please amend the patch ASAP to use new verify_host_resolvable() function so I can test it and get it into 4.3. Updated patches attached. Hmm, my review script screams: Detected base branch: origin/master Checks will be made against following base commit: 848912a add missing /ipaplatform/constants.py to .gitignore Writing API to API.txt ./ipalib/plugins/dns.py:2127:9: E124 closing bracket does not match visual indentation ./ipalib/plugins/dns.py:2711:13: E128 continuation line under-indented for visual indent ./ipalib/plugins/dns.py:2713:9: E124 closing bracket does not match visual indentation ./ipalib/plugins/dns.py:2716:13: E128 continuation line under-indented for visual indent ./ipapython/ipautil.py:928:1: E302 expected 2 blank lines, found 1 ./ipapython/ipautil.py:948:17: E128 continuation line under-indented for visual indent ./ipapython/ipautil.py:956:1: E302 expected 2 blank lines, found 1 ./ipapython/ipautil.py:997:80: E501 line too long (83 > 79 characters) ./ipapython/ipautil.py:998:80: E501 line too long (83 > 79 characters) ./ipaserver/install/bindinstance.py:49:9: E128 continuation line under-indented for visual indent ./ipaserver/install/bindinstance.py:292:80: E501 line too long (80 > 79 characters) ./ipaserver/install/bindinstance.py:438:19: E128 continuation line under-indented for visual indent ./ipaserver/install/server/common.py:271:63: E261 at least two spaces before inline comment ./ipaserver/install/server/common.py:271:80: E501 line too long (90 > 79 characters) ERROR: PEP8 --diff failed, continuing ... ERROR: API.txt was changed without a change in VERSION, continuing ... Summary of detected errors: ERROR: PEP8 --diff failed ERROR: API.txt was changed without a change in VERSION Please fix it :-) Make make this more pleasant for you, you can use (and review) my attached patch. It adds 'review' target to Makefile, it will do the same checks as I do. Unfortunately your tool does not work for me (output w/ -o xtrace attached). Anyway I have incremented API version and fixed most of the pep8 errors except for E124 twice in ipalib/plugins/dns.py as it seems to be convention in the file and E501 twice in ipapython/ipautil.py because IMO breaking string constant into multiple lines does not help readability. Updated patches also attached. We are almost there, but NACK for now. 1) Following attempt to install IPA explodes. Please note that dom-245.idm.lab.eng.brq.redhat.com DNS domain is delegated to the IPA server before installation is started, so it gives 'timeout' or possibly SERVFAIL. # ipa-server-install --ds-password=root4lab --admin-password=root4lab --setup-dns --forwarder=10.38.5.26 --no-reverse --allow-zone-overlap --domain=dom-245.idm.lab.eng.brq.redhat.com --realm=DOM-245.IDM.LAB.ENG.BRQ.REDHAT.COM [...] Configuring DNS (named) [1/11]: generating rndc key file [2/11]: adding DNS container [3/11]: setting up our zone [error] InvocationError: DNS check for domain dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after 30.000453949 seconds. Please make sure that the domain is properly delegated to this IPA server. ipa.ipapython.install.cli.install_tool(Server): ERRORDNS check for domain dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after 30.000453949 seconds. Please make sure that the domain is properly delegated to this IPA server. ipa.ipapython.install.cli.install_tool(Server): ERRORThe ipa-server-install command failed. See /var/log/ipaserver-install.log for more information 2015-12-09T17:15:37Z DEBUG File "/usr/lib/python2.7/site-packages/ipapython/admintool.py", line 171, in execute return_value = self.run() File "/usr/lib/python2.7/site-packages/ipapython/install/cli.py", line 318, in run cfgr.run() File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 310, in run self.execute() File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 332, in execute for nothing in self._executor(): File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 372, in __runner self._handle_exception(exc_info) File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 394, in _handle_exception six.reraise(*exc_info) File
Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
On 8.12.2015 12:19, David Kupka wrote: > On 08/12/15 08:56, Petr Spacek wrote: >> On 7.12.2015 14:41, David Kupka wrote: >>> +def is_host_resolvable(fqdn): >>> +if not isinstance(fqdn, DNSName): >>> +fqdn = DNSName(fqdn) >>> +for rdtype in (rdatatype.A, rdatatype.): >>> +try: >>> +resolver.query(fqdn.make_absolute(), rdtype) >>> +except DNSException: >>> +continue >>> +else: >>> +return True >>> + >>> +return False >>> >> >> NACK, you are re-introducing duplicate function which was removed in >> 498471e4aed1367b72cd74d15811d0584a6ee268. >> >> Please amend the patch ASAP to use new verify_host_resolvable() function so I >> can test it and get it into 4.3. >> > Updated patches attached. Hmm, my review script screams: Detected base branch: origin/master Checks will be made against following base commit: 848912a add missing /ipaplatform/constants.py to .gitignore Writing API to API.txt ./ipalib/plugins/dns.py:2127:9: E124 closing bracket does not match visual indentation ./ipalib/plugins/dns.py:2711:13: E128 continuation line under-indented for visual indent ./ipalib/plugins/dns.py:2713:9: E124 closing bracket does not match visual indentation ./ipalib/plugins/dns.py:2716:13: E128 continuation line under-indented for visual indent ./ipapython/ipautil.py:928:1: E302 expected 2 blank lines, found 1 ./ipapython/ipautil.py:948:17: E128 continuation line under-indented for visual indent ./ipapython/ipautil.py:956:1: E302 expected 2 blank lines, found 1 ./ipapython/ipautil.py:997:80: E501 line too long (83 > 79 characters) ./ipapython/ipautil.py:998:80: E501 line too long (83 > 79 characters) ./ipaserver/install/bindinstance.py:49:9: E128 continuation line under-indented for visual indent ./ipaserver/install/bindinstance.py:292:80: E501 line too long (80 > 79 characters) ./ipaserver/install/bindinstance.py:438:19: E128 continuation line under-indented for visual indent ./ipaserver/install/server/common.py:271:63: E261 at least two spaces before inline comment ./ipaserver/install/server/common.py:271:80: E501 line too long (90 > 79 characters) ERROR: PEP8 --diff failed, continuing ... ERROR: API.txt was changed without a change in VERSION, continuing ... Summary of detected errors: ERROR: PEP8 --diff failed ERROR: API.txt was changed without a change in VERSION Please fix it :-) Make make this more pleasant for you, you can use (and review) my attached patch. It adds 'review' target to Makefile, it will do the same checks as I do. -- Petr^2 Spacek make lint is running ... make[1]: warning: jobserver unavailable: using -j1. Add '+' to parent make rule. Building IPA 4.2.90.201512081417GIT8d05032 ~/pkg/ipa/git ~/pkg/ipa/git/install/po install/po/tmp.pot.update: warning: Charset "CHARSET" is not a portable encoding name. Message conversion to user's charset might not work. ~/pkg/ipa/git/install/po tmp.pot updated tmp.pot: 3112 entries, 3155 msgid, 0 msgstr, 0 warnings 0 errors Detected base branch: origin/master Checks will be made against following base commit: 848912a add missing /ipaplatform/constants.py to .gitignore Writing API to API.txt ./ipalib/plugins/dns.py:2127:9: E124 closing bracket does not match visual indentation ./ipalib/plugins/dns.py:2711:13: E128 continuation line under-indented for visual indent ./ipalib/plugins/dns.py:2713:9: E124 closing bracket does not match visual indentation ./ipalib/plugins/dns.py:2716:13: E128 continuation line under-indented for visual indent ./ipapython/ipautil.py:928:1: E302 expected 2 blank lines, found 1 ./ipapython/ipautil.py:948:17: E128 continuation line under-indented for visual indent ./ipapython/ipautil.py:956:1: E302 expected 2 blank lines, found 1 ./ipapython/ipautil.py:997:80: E501 line too long (83 > 79 characters) ./ipapython/ipautil.py:998:80: E501 line too long (83 > 79 characters) ./ipaserver/install/bindinstance.py:49:9: E128 continuation line under-indented for visual indent ./ipaserver/install/bindinstance.py:292:80: E501 line too long (80 > 79 characters) ./ipaserver/install/bindinstance.py:438:19: E128 continuation line under-indented for visual indent ./ipaserver/install/server/common.py:271:63: E261 at least two spaces before inline comment ./ipaserver/install/server/common.py:271:80: E501 line too long (90 > 79 characters) ERROR: PEP8 --diff failed, continuing ... ERROR: API.txt was changed without a change in VERSION, continuing ... Summary of detected errors: ERROR: PEP8 --diff failed ERROR: API.txt was changed without a change in VERSION Please see /tmp/tmp.WGHe4ApoK3.log From 52393bf2bb0ad74dbe37e496b1fd41a6ab22bd90 Mon Sep 17 00:00:00 2001 From: Petr Spacek Date: Tue, 8 Dec 2015 12:06:33 +0100 Subject: [PATCH] Add 'review' target for make. It automates following tasks: - check if ACI.txt and API.txt are up-to-date - check if VERSION was changed if API was changed - pep8 --diff does not produce new errors when
Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
On 08/12/15 08:56, Petr Spacek wrote:
On 7.12.2015 14:41, David Kupka wrote:
+def is_host_resolvable(fqdn):
+if not isinstance(fqdn, DNSName):
+fqdn = DNSName(fqdn)
+for rdtype in (rdatatype.A, rdatatype.):
+try:
+resolver.query(fqdn.make_absolute(), rdtype)
+except DNSException:
+continue
+else:
+return True
+
+return False
NACK, you are re-introducing duplicate function which was removed in
498471e4aed1367b72cd74d15811d0584a6ee268.
Please amend the patch ASAP to use new verify_host_resolvable() function so I
can test it and get it into 4.3.
Updated patches attached.
--
David Kupka
From 6927ea57fe73ad9dfd64d432aa18fd7b3ecda084 Mon Sep 17 00:00:00 2001
From: David Kupka
Date: Wed, 2 Dec 2015 13:17:13 +
Subject: [PATCH] dns: do not add (forward)zone if it is already resolvable.
Check if the zone user wants to add is already resolvable and refuse to
create it if yes. --skip-overlap-check and --force options suppress this check.
https://fedorahosted.org/freeipa/ticket/5087
---
API.txt | 7 +++--
ipalib/plugins/dns.py | 32 ---
ipapython/ipautil.py | 87 +--
3 files changed, 117 insertions(+), 9 deletions(-)
diff --git a/API.txt b/API.txt
index 60c98c31aa85d6c8879cd145f3d84188d4fea5b7..3a9fb65a386a2a6529b8cd241642446c135471f2 100644
--- a/API.txt
+++ b/API.txt
@@ -959,7 +959,7 @@ output: Entry('result', , Gettext('A dictionary representing an LDA
output: Output('summary', (, ), None)
output: PrimaryKey('value', None, None)
command: dnsforwardzone_add
-args: 1,8,3
+args: 1,9,3
arg: DNSNameParam('idnsname', attribute=True, cli_name='name', multivalue=False, only_absolute=True, primary_key=True, required=True)
option: Str('addattr*', cli_name='addattr', exclude='webui')
option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
@@ -968,6 +968,7 @@ option: StrEnum('idnsforwardpolicy', attribute=True, cli_name='forward_policy',
option: Str('name_from_ip', attribute=False, cli_name='name_from_ip', multivalue=False, required=False)
option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
option: Str('setattr*', cli_name='setattr', exclude='webui')
+option: Flag('skip_overlap_check', autofill=True, default=False)
option: Str('version?', exclude='webui')
output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
output: Output('summary', (, ), None)
@@ -1366,7 +1367,7 @@ output: Entry('result', , Gettext('A dictionary representing an LDA
output: Output('summary', (, ), None)
output: PrimaryKey('value', None, None)
command: dnszone_add
-args: 1,26,3
+args: 1,28,3
arg: DNSNameParam('idnsname', attribute=True, cli_name='name', multivalue=False, only_absolute=True, primary_key=True, required=True)
option: Str('addattr*', cli_name='addattr', exclude='webui')
option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
@@ -1393,6 +1394,8 @@ option: Str('name_from_ip', attribute=False, cli_name='name_from_ip', multivalue
option: Str('nsec3paramrecord', attribute=True, cli_name='nsec3param_rec', multivalue=False, pattern='^\\d+ \\d+ \\d+ (([0-9a-fA-F]{2})+|-)$', required=False)
option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
option: Str('setattr*', cli_name='setattr', exclude='webui')
+option: Flag('skip_nameserver_check', autofill=True, default=False)
+option: Flag('skip_overlap_check', autofill=True, default=False)
option: Str('version?', exclude='webui')
output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
output: Output('summary', (, ), None)
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index 67947360eb207de31ed114bb630705c409b2f9a9..9cad9cfb8b4175cc92778b2df057621ca055e58f 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -53,8 +53,7 @@ from ipalib.util import (normalize_zonemgr,
validate_dnssec_zone_forwarder_step1,
validate_dnssec_zone_forwarder_step2,
verify_host_resolvable)
-
-from ipapython.ipautil import CheckedIPAddress
+from ipapython.ipautil import CheckedIPAddress, check_zone_overlap
from ipapython.dnsutil import DNSName
if six.PY3:
@@ -2121,6 +2120,13 @@ class DNSZoneBase(LDAPObject):
class DNSZoneBase_add(LDAPCreate):
+takes_options = LDAPCreate.takes_options + (
+Flag('skip_overlap_check',
+ doc=_('Force DNS zone creation even if it will overlap with '
+ 'an existing zone.')
+),
+)
+
has_output_params = LDAPCreate.has_output_params + dnszone_output_params
def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
@@ -2140,6 +2146,12 @@ class DNSZoneBase_add(LDAPCreate):
entry_attrs['idnszone
Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
On 7.12.2015 14:41, David Kupka wrote: > +def is_host_resolvable(fqdn): > +if not isinstance(fqdn, DNSName): > +fqdn = DNSName(fqdn) > +for rdtype in (rdatatype.A, rdatatype.): > +try: > +resolver.query(fqdn.make_absolute(), rdtype) > +except DNSException: > +continue > +else: > +return True > + > +return False > NACK, you are re-introducing duplicate function which was removed in 498471e4aed1367b72cd74d15811d0584a6ee268. Please amend the patch ASAP to use new verify_host_resolvable() function so I can test it and get it into 4.3. -- 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
Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
On 07/12/15 14:06, David Kupka wrote:
On 09/09/15 13:39, Petr Spacek wrote:
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
Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
On 09/09/15 13:39, Petr Spacek wrote:
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-i
Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
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 i
Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
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.
--
David Kupka
From 816ee3102ee0603450f897f8f6bfed4d5460636c Mon Sep 17 00:00:00 2001
From: David Kupka
Date: Fri, 21 Aug 2015 13:25:34 +0200
Subject: [PATCH 2/2] dns: Check if domain already exists.
Raise an error when the domain already exists. This can be ove
Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
On 28/08/15 10:03, 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 :-))
IIUC it is usually handled in pre_callback because framework does not
provide any other mechanism preprocess and validate options.
+
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 ...
Ok, will update this.
+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.
--
David Kupka
--
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
Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
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.
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?
--
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
Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
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.
--
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
Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
On 25/08/15 14:39, David Kupka wrote:
On 25/08/15 10:37, David Kupka wrote:
On 24/08/15 16:51, Martin Basti wrote:
On 08/20/2015 10:28 AM, David Kupka wrote:
On 31/07/15 13:32, Martin Basti wrote:
On 30/07/15 14:38, Martin Basti wrote:
On 29/07/15 16:12, David Kupka wrote:
https://fedorahosted.org/freeipa/ticket/5087
NACK
You forgot to update API.txt file
Thanks for catching that. Updated patch attached.
I'm just curious, what is the reason to check if forward zone exists?
IMO forwardzone must exists somewhere as the master zone. I don't
think
we should check forwardzones, this may give too many false positive
errors.
AIUI if the zone exist somewhere and is resolvable there is no need to
add it as a forward zone. If user for some reason want to do it he's
hiding the original zone and we should not allow this (without
--force).
Note: Petr2 agreed with David's solution
LGTM, works as expected, but this patch prevents users to add
conflicting zones via webUI (there is no --force field).
We should improve webUI together with this patch.
Martin^2
Martin^2
The '--force' option was not in WebUI before even though it was in API.
IMO we should not expose '--force' options in WebUI at all.
Added similar options to ipa-{server,dns}-install and reworked the patch
to not duplicate the code.
Updated patch and one new attached.
Updated patch attached.
--
David Kupka
From 660e4dcc7bb1b828b24515d89b4f6dad4f86b7bf Mon Sep 17 00:00:00 2001
From: David Kupka
Date: Thu, 2 Jul 2015 15:10:40 +0200
Subject: [PATCH 1/2] dns: do not add (forward)zone if it is already
resolvable.
Check if the zone user wants to add is already resolvable and refuse to
create it if yes. --skip-overlap-check and --force options suppress this check.
https://fedorahosted.org/freeipa/ticket/5087
---
API.txt | 8 ++--
ipalib/plugins/dns.py | 33 -
ipapython/ipautil.py | 14 ++
3 files changed, 48 insertions(+), 7 deletions(-)
diff --git a/API.txt b/API.txt
index afd5017bee2bc1eed54497ccd504b92619ff7a58..6fc1c3e5b2b9af48ff12803654340f91e826a507 100644
--- a/API.txt
+++ b/API.txt
@@ -960,15 +960,17 @@ output: Entry('result', , Gettext('A dictionary representing an LDA
output: Output('summary', (, ), None)
output: PrimaryKey('value', None, None)
command: dnsforwardzone_add
-args: 1,8,3
+args: 1,10,3
arg: DNSNameParam('idnsname', attribute=True, cli_name='name', multivalue=False, only_absolute=True, primary_key=True, required=True)
option: Str('addattr*', cli_name='addattr', exclude='webui')
option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
+option: Flag('force', autofill=True, default=False)
option: Str('idnsforwarders', attribute=True, cli_name='forwarder', csv=True, multivalue=True, required=False)
option: StrEnum('idnsforwardpolicy', attribute=True, cli_name='forward_policy', multivalue=False, required=False, values=(u'only', u'first', u'none'))
option: Str('name_from_ip', attribute=False, cli_name='name_from_ip', multivalue=False, required=False)
option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
option: Str('setattr*', cli_name='setattr', exclude='webui')
+option: Flag('skip_overlap_check', autofill=True, default=False)
option: Str('version?', exclude='webui')
output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
output: Output('summary', (, ), None)
@@ -1367,7 +1369,7 @@ output: Entry('result', , Gettext('A dictionary representing an LDA
output: Output('summary', (, ), None)
output: PrimaryKey('value', None, None)
command: dnszone_add
-args: 1,26,3
+args: 1,28,3
arg: DNSNameParam('idnsname', attribute=True, cli_name='name', multivalue=False, only_absolute=True, primary_key=True, required=True)
option: Str('addattr*', cli_name='addattr', exclude='webui')
option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
@@ -1394,6 +1396,8 @@ option: Str('name_from_ip', attribute=False, cli_name='name_from_ip', multivalue
option: Str('nsec3paramrecord', attribute=True, cli_name='nsec3param_rec', multivalue=False, pattern='^\\d+ \\d+ \\d+ (([0-9a-fA-F]{2})+|-)$', required=False)
option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
option: Str('setattr*', cli_name='setattr', exclude='webui')
+option: Flag('skip_nameserver_check', autofill=True, default=False)
+option: Flag('skip_overlap_check', autofill=True, default=False)
option: Str('version?', exclude='webui')
output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
output: Output('summary', (, ), None)
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index 23871a58f441cdc598d49c76035da243031b49d7..8bdea7ab5a46a366021c258788057e13c74be41a 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -50,7 +50,7 @@ from ipalib.util import (no
Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.
On 25/08/15 10:37, David Kupka wrote:
On 24/08/15 16:51, Martin Basti wrote:
On 08/20/2015 10:28 AM, David Kupka wrote:
On 31/07/15 13:32, Martin Basti wrote:
On 30/07/15 14:38, Martin Basti wrote:
On 29/07/15 16:12, David Kupka wrote:
https://fedorahosted.org/freeipa/ticket/5087
NACK
You forgot to update API.txt file
Thanks for catching that. Updated patch attached.
I'm just curious, what is the reason to check if forward zone exists?
IMO forwardzone must exists somewhere as the master zone. I don't think
we should check forwardzones, this may give too many false positive
errors.
AIUI if the zone exist somewhere and is resolvable there is no need to
add it as a forward zone. If user for some reason want to do it he's
hiding the original zone and we should not allow this (without --force).
Note: Petr2 agreed with David's solution
LGTM, works as expected, but this patch prevents users to add
conflicting zones via webUI (there is no --force field).
We should improve webUI together with this patch.
Martin^2
Martin^2
The '--force' option was not in WebUI before even though it was in API.
IMO we should not expose '--force' options in WebUI at all.
Added similar options to ipa-{server,dns}-install and reworked the patch
to not duplicate the code.
Updated patch and one new attached.
--
David Kupka
From ab7aa126a68bcea95f707a78ca1f776270d3b5ec Mon Sep 17 00:00:00 2001
From: David Kupka
Date: Thu, 2 Jul 2015 15:10:40 +0200
Subject: [PATCH 1/2] dns: do not add (forward)zone if it is already
resolvable.
Check if the zone user wants to add is already resolvable and refuse to
create it if yes. --skip-overlap-check and --force options suppress this check.
https://fedorahosted.org/freeipa/ticket/5087
---
API.txt | 8 ++--
ipalib/plugins/dns.py | 33 -
ipapython/ipautil.py | 17 +
3 files changed, 51 insertions(+), 7 deletions(-)
diff --git a/API.txt b/API.txt
index 6ab30ddab41715fdbccb4f37aa1852621bca62b4..f6db1e99fca2acf410308c5fdaa13b40c43df933 100644
--- a/API.txt
+++ b/API.txt
@@ -960,15 +960,17 @@ output: Entry('result', , Gettext('A dictionary representing an LDA
output: Output('summary', (, ), None)
output: PrimaryKey('value', None, None)
command: dnsforwardzone_add
-args: 1,8,3
+args: 1,10,3
arg: DNSNameParam('idnsname', attribute=True, cli_name='name', multivalue=False, only_absolute=True, primary_key=True, required=True)
option: Str('addattr*', cli_name='addattr', exclude='webui')
option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
+option: Flag('force', autofill=True, default=False)
option: Str('idnsforwarders', attribute=True, cli_name='forwarder', csv=True, multivalue=True, required=False)
option: StrEnum('idnsforwardpolicy', attribute=True, cli_name='forward_policy', multivalue=False, required=False, values=(u'only', u'first', u'none'))
option: Str('name_from_ip', attribute=False, cli_name='name_from_ip', multivalue=False, required=False)
option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
option: Str('setattr*', cli_name='setattr', exclude='webui')
+option: Flag('skip_overlap_check', autofill=True, default=False)
option: Str('version?', exclude='webui')
output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
output: Output('summary', (, ), None)
@@ -1367,7 +1369,7 @@ output: Entry('result', , Gettext('A dictionary representing an LDA
output: Output('summary', (, ), None)
output: PrimaryKey('value', None, None)
command: dnszone_add
-args: 1,26,3
+args: 1,28,3
arg: DNSNameParam('idnsname', attribute=True, cli_name='name', multivalue=False, only_absolute=True, primary_key=True, required=True)
option: Str('addattr*', cli_name='addattr', exclude='webui')
option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
@@ -1394,6 +1396,8 @@ option: Str('name_from_ip', attribute=False, cli_name='name_from_ip', multivalue
option: Str('nsec3paramrecord', attribute=True, cli_name='nsec3param_rec', multivalue=False, pattern='^\\d+ \\d+ \\d+ (([0-9a-fA-F]{2})+|-)$', required=False)
option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
option: Str('setattr*', cli_name='setattr', exclude='webui')
+option: Flag('skip_nameserver_check', autofill=True, default=False)
+option: Flag('skip_overlap_check', autofill=True, default=False)
option: Str('version?', exclude='webui')
output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
output: Output('summary', (, ), None)
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index 512a653c3cc8ee641debec0d20f58e17eff08266..bcd3793bd94f3836f5127fce153ae29b2cbfd151 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -50,7 +50,7 @@ from ipalib.util import (normalize_zonemgr,
validate_dnssec_zon
