subject:"\[Freeipa\-devel\] \[PATCH 0058, 0064\] dns\: do not add \(forward\)zone if it is already resolvable."

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

2015-12-14 Thread David Kupka


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.

2015-12-14 Thread Petr Spacek

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.

2015-12-14 Thread David Kupka


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

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

2015-12-14 Thread David Kupka


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

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

2015-12-13 Thread Petr Spacek

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

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

2015-12-13 Thread David Kupka


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', (,

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

2015-12-11 Thread David Kupka


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

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

2015-12-11 Thread Petr Spacek

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.

2015-12-10 Thread Petr Spacek

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

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

2015-12-10 Thread David Kupka


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.

2015-12-08 Thread David Kupka


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):

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

2015-12-08 Thread Petr Spacek

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

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

2015-12-07 Thread Petr Spacek

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.

2015-12-07 Thread David Kupka


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.


*

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

2015-12-07 Thread David Kupka


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)

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

2015-09-09 Thread Petr Spacek

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

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

2015-09-08 Thread David Kupka


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

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

2015-09-08 Thread David Kupka


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.

2015-08-28 Thread Martin Basti




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 = uDNS 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.

2015-08-28 Thread Petr Spacek

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 = uDNS 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.

2015-08-25 Thread David Kupka


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 dku...@redhat.com
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', type 'dict', Gettext('A dictionary representing an LDA
 output: Output('summary', (type 'unicode', type 'NoneType'), 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', type 'dict', Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('summary', (type 'unicode', type 'NoneType'), None)
@@ -1367,7 +1369,7 @@ output: Entry('result', type 'dict', Gettext('A dictionary representing an LDA
 output: Output('summary', (type 'unicode', type 'NoneType'), 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', type 'dict', Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('summary', (type 'unicode', type 'NoneType'), None)
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

Re: [Freeipa-devel] [PATCH 0058, 0064] dns: do not add (forward)zone if it is already resolvable.

21 matches

Site Navigation

Mail list logo

Footer information