Re: [Freeipa-devel] [PATCH] 18 Parse netmasks in IP addresses passed to server install

2011-06-07 Thread Jakub Hrozek
On 06/06/2011 12:28 PM, Jan Cholasta wrote:
 On 2.6.2011 08:23, Jakub Hrozek wrote:
 On 06/02/2011 08:18 AM, Jakub Hrozek wrote:
 On 05/23/2011 08:00 AM, Jan Cholasta wrote:
 On 22.5.2011 18:28, Jakub Hrozek wrote:
 On 05/20/2011 08:27 PM, Jan Cholasta wrote:
 TODO: Clean unreachable code paths off of ipa-server-install (?)

 In general I agree even though I don't know exactly what code you have
 in mind -- if the code is dead there's no reason to keep it.

 I've noticed that e.g. if the hostname can't be resolved, verify_fqdn
 raises an exception, so some of the checks below the ip =
 resolve_host(host_name) line in ipa-server-install are unnecessary,
 but
 I'm not yet sure if I'm not missing something.


 TODO: Workarounds for netaddr bugs (?)

 Are these bugs reported upstream? I know you mentioned some in an
 earlier e-mail, just wondering if they are the same.

 Long term, it might be better to fix them in netaddr rather than
 working
 around them.

 Yes, they're the same and are already fixed (according to the netaddr
 bug tracker), but there's no release with the fixes yet (or it's not in
 Fedora). There are not any big issues that I'm aware of, it's just that
 if you specify incorrect netmask with an IPv4 address, the error
 message
 isn't very helpful to the user:

 netaddr.IPNetwork('192.168.1.1/33')
 ...
 UnboundLocalError: local variable 'ip' referenced before assignment


 Jakub


 Honza



 I cherry-picked a patch for that issue from upstream and built a fixed
 python-netaddr:
 https://admin.fedoraproject.org/updates/python-netaddr-0.7.5-3.fc15

 Please test and add karma :-)

 
 Done.
 

The update went stable today in case you wanted to Require: the fixed
version



signature.asc
Description: OpenPGP digital signature
___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 18 Parse netmasks in IP addresses passed to server install

2011-06-06 Thread Jan Cholasta

On 2.6.2011 08:23, Jakub Hrozek wrote:

On 06/02/2011 08:18 AM, Jakub Hrozek wrote:

On 05/23/2011 08:00 AM, Jan Cholasta wrote:

On 22.5.2011 18:28, Jakub Hrozek wrote:

On 05/20/2011 08:27 PM, Jan Cholasta wrote:

TODO: Clean unreachable code paths off of ipa-server-install (?)


In general I agree even though I don't know exactly what code you have
in mind -- if the code is dead there's no reason to keep it.


I've noticed that e.g. if the hostname can't be resolved, verify_fqdn
raises an exception, so some of the checks below the ip =
resolve_host(host_name) line in ipa-server-install are unnecessary, but
I'm not yet sure if I'm not missing something.




TODO: Workarounds for netaddr bugs (?)


Are these bugs reported upstream? I know you mentioned some in an
earlier e-mail, just wondering if they are the same.

Long term, it might be better to fix them in netaddr rather than
working
around them.


Yes, they're the same and are already fixed (according to the netaddr
bug tracker), but there's no release with the fixes yet (or it's not in
Fedora). There are not any big issues that I'm aware of, it's just that
if you specify incorrect netmask with an IPv4 address, the error message
isn't very helpful to the user:

netaddr.IPNetwork('192.168.1.1/33')
...
UnboundLocalError: local variable 'ip' referenced before assignment



Jakub



Honza





I cherry-picked a patch for that issue from upstream and built a fixed
python-netaddr:
https://admin.fedoraproject.org/updates/python-netaddr-0.7.5-3.fc15

Please test and add karma :-)



Done.

--
Jan Cholasta

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 18 Parse netmasks in IP addresses passed to server install

2011-06-02 Thread Jakub Hrozek

On 06/02/2011 08:18 AM, Jakub Hrozek wrote:

On 05/23/2011 08:00 AM, Jan Cholasta wrote:

On 22.5.2011 18:28, Jakub Hrozek wrote:

On 05/20/2011 08:27 PM, Jan Cholasta wrote:

TODO: Clean unreachable code paths off of ipa-server-install (?)


In general I agree even though I don't know exactly what code you have
in mind -- if the code is dead there's no reason to keep it.


I've noticed that e.g. if the hostname can't be resolved, verify_fqdn
raises an exception, so some of the checks below the ip =
resolve_host(host_name) line in ipa-server-install are unnecessary, but
I'm not yet sure if I'm not missing something.




TODO: Workarounds for netaddr bugs (?)


Are these bugs reported upstream? I know you mentioned some in an
earlier e-mail, just wondering if they are the same.

Long term, it might be better to fix them in netaddr rather than working
around them.


Yes, they're the same and are already fixed (according to the netaddr
bug tracker), but there's no release with the fixes yet (or it's not in
Fedora). There are not any big issues that I'm aware of, it's just that
if you specify incorrect netmask with an IPv4 address, the error message
isn't very helpful to the user:

netaddr.IPNetwork('192.168.1.1/33')
...
UnboundLocalError: local variable 'ip' referenced before assignment



Jakub



Honza





I cherry-picked a patch for that issue from upstream and built a fixed 
python-netaddr:

https://admin.fedoraproject.org/updates/python-netaddr-0.7.5-3.fc15

Please test and add karma :-)


___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 18 Parse netmasks in IP addresses passed to server install

2011-05-30 Thread Martin Kosek
On Fri, 2011-05-27 at 22:09 +0200, Jan Cholasta wrote:
 On 27.5.2011 18:59, Martin Kosek wrote:
  On Fri, 2011-05-27 at 16:47 +0200, Jan Cholasta wrote:
  On 24.5.2011 15:38, Jan Cholasta wrote:
  On 20.5.2011 20:27, Jan Cholasta wrote:
  On 10.5.2011 20:06, Jan Cholasta wrote:
  Parse netmasks in IP addresses passed to server install.
 
  ticket 1212
 
  Patch updated.
 
  TODO: Write unit test for ipapython.ipautil.CheckedIPAddress
  TODO: Clean unreachable code paths off of ipa-server-install (?)
  TODO: Workarounds for netaddr bugs (?)
 
 
 
  ___
  Freeipa-devel mailing list
  Freeipa-devel@redhat.com
  https://www.redhat.com/mailman/listinfo/freeipa-devel
 
  Fixed ipa-replica-prepare and added a unit test.
 
 
  Another update.
 
  Honza
 
  Can you please rebase your patches? My patch 070 fixing
  add_reverse_zone() function was pushed today. Unfortunately, it made
  your patches 18 and 3 not applicable.
 
 Done.
 
 
  You may want to look closer at the patch 070 as it is relevant to your
  patch set and also to make sure the fix is still functional after your
  set of patches.
 
 It seems it's ok.
 
 
  Thanks,
  Martin
 
 
 Honza
 

Everything seems to work fine, ACK.

Pushed to master.

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 18 Parse netmasks in IP addresses passed to server install

2011-05-27 Thread Jan Cholasta

On 24.5.2011 15:38, Jan Cholasta wrote:

On 20.5.2011 20:27, Jan Cholasta wrote:

On 10.5.2011 20:06, Jan Cholasta wrote:

Parse netmasks in IP addresses passed to server install.

ticket 1212


Patch updated.

TODO: Write unit test for ipapython.ipautil.CheckedIPAddress
TODO: Clean unreachable code paths off of ipa-server-install (?)
TODO: Workarounds for netaddr bugs (?)



___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Fixed ipa-replica-prepare and added a unit test.



Another update.

Honza

--
Jan Cholasta
From 9bf06f27c816f98281e6e33bed6725e0322727f0 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Fri, 27 May 2011 13:01:41 +0200
Subject: [PATCH] Parse netmasks in IP addresses passed to server install.

ticket 1212
---
 freeipa.spec.in  |1 +
 install/tools/ipa-dns-install|9 +++--
 install/tools/ipa-replica-install|6 +++-
 install/tools/ipa-replica-prepare|   17 +
 install/tools/ipa-server-install |   36 +--
 ipapython/config.py  |   13 ++-
 ipapython/ipautil.py |   67 ++
 ipaserver/install/installutils.py|   39 +---
 tests/test_ipapython/__init__.py |   22 +++
 tests/test_ipapython/test_ipautil.py |   56 
 10 files changed, 213 insertions(+), 53 deletions(-)
 create mode 100644 tests/test_ipapython/__init__.py
 create mode 100644 tests/test_ipapython/test_ipautil.py

diff --git a/freeipa.spec.in b/freeipa.spec.in
index b936616..fba2f31 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -188,6 +188,7 @@ Requires: python-kerberos = 1.1-3
 %endif
 Requires: authconfig
 Requires: gnupg
+Requires: iproute
 Requires: pyOpenSSL
 Requires: python-nss = 0.11
 Requires: python-lxml
diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install
index aac85bf..491585b 100755
--- a/install/tools/ipa-dns-install
+++ b/install/tools/ipa-dns-install
@@ -37,9 +37,10 @@ def parse_options():
   sensitive=True, help=admin password)
 parser.add_option(-d, --debug, dest=debug, action=store_true,
   default=False, help=print debugging information)
-parser.add_option(--ip-address, dest=ip_address, help=Master Server IP Address)
+parser.add_option(--ip-address, dest=ip_address,
+  type=ipnet, help=Master Server IP Address)
 parser.add_option(--forwarder, dest=forwarders, action=append,
-  help=Add a DNS forwarder)
+  type=ipaddr, help=Add a DNS forwarder)
 parser.add_option(--no-forwarders, dest=no_forwarders, action=store_true,
   default=False, help=Do not add any DNS forwarders, use root servers instead)
 parser.add_option(--no-reverse, dest=no_reverse,
@@ -130,12 +131,14 @@ def main():
 if options.ip_address:
 ip_address = options.ip_address
 else:
-ip_address = resolve_host(api.env.host)
+hostaddr = resolve_host(api.env.host)
+ip_address = hostaddr and ipautil.CheckedIPAddress(hostaddr)
 if not ip_address or not verify_ip_address(ip_address):
 if options.unattended:
 sys.exit(Unable to resolve IP address for host name)
 else:
 ip_address = read_ip_address(api.env.host, fstore)
+ip_address = str(ip_address)
 logging.debug(will use ip_address: %s\n, ip_address)
 
 if options.no_forwarders:
diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install
index 49df7fe..2727fa6 100755
--- a/install/tools/ipa-replica-install
+++ b/install/tools/ipa-replica-install
@@ -63,7 +63,7 @@ def parse_options():
 parser.add_option(--setup-dns, dest=setup_dns, action=store_true,
   default=False, help=configure bind with our zone)
 parser.add_option(--forwarder, dest=forwarders, action=append,
-  help=Add a DNS forwarder)
+  type=ipaddr, help=Add a DNS forwarder)
 parser.add_option(--no-forwarders, dest=no_forwarders, action=store_true,
   default=False, help=Do not add any DNS forwarders, use root servers instead)
 parser.add_option(--no-reverse, dest=no_reverse, action=store_true,
@@ -285,6 +285,8 @@ def install_bind(config, options):
 ip_address = resolve_host(config.host_name)
 if not ip_address:
 sys.exit(Unable to resolve IP address for host name)
+ip = installutils.parse_ip_address(ip_address)
+ip_address = str(ip)
 
 create_reverse = True
 if options.unattended:
@@ -320,6 +322,8 @@ def install_dns_records(config, options):
 ip_address = resolve_host(config.host_name)
 if not ip_address:
 sys.exit(Unable to resolve IP address for host name)
+ip = 

Re: [Freeipa-devel] [PATCH] 18 Parse netmasks in IP addresses passed to server install

2011-05-27 Thread Martin Kosek
On Fri, 2011-05-27 at 16:47 +0200, Jan Cholasta wrote:
 On 24.5.2011 15:38, Jan Cholasta wrote:
  On 20.5.2011 20:27, Jan Cholasta wrote:
  On 10.5.2011 20:06, Jan Cholasta wrote:
  Parse netmasks in IP addresses passed to server install.
 
  ticket 1212
 
  Patch updated.
 
  TODO: Write unit test for ipapython.ipautil.CheckedIPAddress
  TODO: Clean unreachable code paths off of ipa-server-install (?)
  TODO: Workarounds for netaddr bugs (?)
 
 
 
  ___
  Freeipa-devel mailing list
  Freeipa-devel@redhat.com
  https://www.redhat.com/mailman/listinfo/freeipa-devel
 
  Fixed ipa-replica-prepare and added a unit test.
 
 
 Another update.
 
 Honza

Can you please rebase your patches? My patch 070 fixing
add_reverse_zone() function was pushed today. Unfortunately, it made
your patches 18 and 3 not applicable.

You may want to look closer at the patch 070 as it is relevant to your
patch set and also to make sure the fix is still functional after your
set of patches.

Thanks,
Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 18 Parse netmasks in IP addresses passed to server install

2011-05-27 Thread Jan Cholasta

On 27.5.2011 18:59, Martin Kosek wrote:

On Fri, 2011-05-27 at 16:47 +0200, Jan Cholasta wrote:

On 24.5.2011 15:38, Jan Cholasta wrote:

On 20.5.2011 20:27, Jan Cholasta wrote:

On 10.5.2011 20:06, Jan Cholasta wrote:

Parse netmasks in IP addresses passed to server install.

ticket 1212


Patch updated.

TODO: Write unit test for ipapython.ipautil.CheckedIPAddress
TODO: Clean unreachable code paths off of ipa-server-install (?)
TODO: Workarounds for netaddr bugs (?)



___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Fixed ipa-replica-prepare and added a unit test.



Another update.

Honza


Can you please rebase your patches? My patch 070 fixing
add_reverse_zone() function was pushed today. Unfortunately, it made
your patches 18 and 3 not applicable.


Done.



You may want to look closer at the patch 070 as it is relevant to your
patch set and also to make sure the fix is still functional after your
set of patches.


It seems it's ok.



Thanks,
Martin



Honza

--
Jan Cholasta
From 49bc2ab0b1050f930161bc525f06516c9afbdacc Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Fri, 27 May 2011 20:17:22 +0200
Subject: [PATCH] Parse netmasks in IP addresses passed to server install.

ticket 1212
---
 freeipa.spec.in  |1 +
 install/tools/ipa-dns-install|9 +++--
 install/tools/ipa-replica-install|6 +++-
 install/tools/ipa-replica-prepare|   17 +
 install/tools/ipa-server-install |   36 +--
 ipapython/config.py  |   13 ++-
 ipapython/ipautil.py |   67 ++
 ipaserver/install/installutils.py|   39 +---
 tests/test_ipapython/__init__.py |   22 +++
 tests/test_ipapython/test_ipautil.py |   56 
 10 files changed, 213 insertions(+), 53 deletions(-)
 create mode 100644 tests/test_ipapython/__init__.py
 create mode 100644 tests/test_ipapython/test_ipautil.py

diff --git a/freeipa.spec.in b/freeipa.spec.in
index b936616..fba2f31 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -188,6 +188,7 @@ Requires: python-kerberos = 1.1-3
 %endif
 Requires: authconfig
 Requires: gnupg
+Requires: iproute
 Requires: pyOpenSSL
 Requires: python-nss = 0.11
 Requires: python-lxml
diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install
index a763297..e837919 100755
--- a/install/tools/ipa-dns-install
+++ b/install/tools/ipa-dns-install
@@ -37,9 +37,10 @@ def parse_options():
   sensitive=True, help=admin password)
 parser.add_option(-d, --debug, dest=debug, action=store_true,
   default=False, help=print debugging information)
-parser.add_option(--ip-address, dest=ip_address, help=Master Server IP Address)
+parser.add_option(--ip-address, dest=ip_address,
+  type=ipnet, help=Master Server IP Address)
 parser.add_option(--forwarder, dest=forwarders, action=append,
-  help=Add a DNS forwarder)
+  type=ipaddr, help=Add a DNS forwarder)
 parser.add_option(--no-forwarders, dest=no_forwarders, action=store_true,
   default=False, help=Do not add any DNS forwarders, use root servers instead)
 parser.add_option(--no-reverse, dest=no_reverse,
@@ -105,12 +106,14 @@ def main():
 if options.ip_address:
 ip_address = options.ip_address
 else:
-ip_address = resolve_host(api.env.host)
+hostaddr = resolve_host(api.env.host)
+ip_address = hostaddr and ipautil.CheckedIPAddress(hostaddr)
 if not ip_address or not verify_ip_address(ip_address):
 if options.unattended:
 sys.exit(Unable to resolve IP address for host name)
 else:
 ip_address = read_ip_address(api.env.host, fstore)
+ip_address = str(ip_address)
 logging.debug(will use ip_address: %s\n, ip_address)
 
 if options.no_forwarders:
diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install
index 293a0a0..6df5123 100755
--- a/install/tools/ipa-replica-install
+++ b/install/tools/ipa-replica-install
@@ -61,7 +61,7 @@ def parse_options():
 parser.add_option(--setup-dns, dest=setup_dns, action=store_true,
   default=False, help=configure bind with our zone)
 parser.add_option(--forwarder, dest=forwarders, action=append,
-  help=Add a DNS forwarder)
+  type=ipaddr, help=Add a DNS forwarder)
 parser.add_option(--no-forwarders, dest=no_forwarders, action=store_true,
   default=False, help=Do not add any DNS forwarders, use root servers instead)
 parser.add_option(--no-reverse, dest=no_reverse, action=store_true,
@@ -270,6 +270,8 @@ def install_bind(config, options):
 ip_address = 

Re: [Freeipa-devel] [PATCH] 18 Parse netmasks in IP addresses passed to server install

2011-05-24 Thread Jan Cholasta

On 20.5.2011 20:27, Jan Cholasta wrote:

On 10.5.2011 20:06, Jan Cholasta wrote:

Parse netmasks in IP addresses passed to server install.

ticket 1212


Patch updated.

TODO: Write unit test for ipapython.ipautil.CheckedIPAddress
TODO: Clean unreachable code paths off of ipa-server-install (?)
TODO: Workarounds for netaddr bugs (?)



___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Fixed ipa-replica-prepare and added a unit test.

--
Jan Cholasta
From 412dee8f9980523376d411ef166c73455a87e22b Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Tue, 24 May 2011 15:27:46 +0200
Subject: [PATCH] Parse netmasks in IP addresses passed to server install.

ticket 1212
---
 freeipa.spec.in  |1 +
 install/tools/ipa-dns-install|9 +++--
 install/tools/ipa-replica-install|4 ++-
 install/tools/ipa-replica-prepare|   15 
 install/tools/ipa-server-install |   36 +--
 ipapython/config.py  |   13 ++-
 ipapython/ipautil.py |   63 ++
 ipaserver/install/installutils.py|   39 ++---
 tests/test_ipapython/__init__.py |   22 
 tests/test_ipapython/test_ipautil.py |   56 ++
 10 files changed, 206 insertions(+), 52 deletions(-)
 create mode 100644 tests/test_ipapython/__init__.py
 create mode 100644 tests/test_ipapython/test_ipautil.py

diff --git a/freeipa.spec.in b/freeipa.spec.in
index b936616..fba2f31 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -188,6 +188,7 @@ Requires: python-kerberos = 1.1-3
 %endif
 Requires: authconfig
 Requires: gnupg
+Requires: iproute
 Requires: pyOpenSSL
 Requires: python-nss = 0.11
 Requires: python-lxml
diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install
index aac85bf..491585b 100755
--- a/install/tools/ipa-dns-install
+++ b/install/tools/ipa-dns-install
@@ -37,9 +37,10 @@ def parse_options():
   sensitive=True, help=admin password)
 parser.add_option(-d, --debug, dest=debug, action=store_true,
   default=False, help=print debugging information)
-parser.add_option(--ip-address, dest=ip_address, help=Master Server IP Address)
+parser.add_option(--ip-address, dest=ip_address,
+  type=ipnet, help=Master Server IP Address)
 parser.add_option(--forwarder, dest=forwarders, action=append,
-  help=Add a DNS forwarder)
+  type=ipaddr, help=Add a DNS forwarder)
 parser.add_option(--no-forwarders, dest=no_forwarders, action=store_true,
   default=False, help=Do not add any DNS forwarders, use root servers instead)
 parser.add_option(--no-reverse, dest=no_reverse,
@@ -130,12 +131,14 @@ def main():
 if options.ip_address:
 ip_address = options.ip_address
 else:
-ip_address = resolve_host(api.env.host)
+hostaddr = resolve_host(api.env.host)
+ip_address = hostaddr and ipautil.CheckedIPAddress(hostaddr)
 if not ip_address or not verify_ip_address(ip_address):
 if options.unattended:
 sys.exit(Unable to resolve IP address for host name)
 else:
 ip_address = read_ip_address(api.env.host, fstore)
+ip_address = str(ip_address)
 logging.debug(will use ip_address: %s\n, ip_address)
 
 if options.no_forwarders:
diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install
index 49df7fe..2b7c8ca 100755
--- a/install/tools/ipa-replica-install
+++ b/install/tools/ipa-replica-install
@@ -63,7 +63,7 @@ def parse_options():
 parser.add_option(--setup-dns, dest=setup_dns, action=store_true,
   default=False, help=configure bind with our zone)
 parser.add_option(--forwarder, dest=forwarders, action=append,
-  help=Add a DNS forwarder)
+  type=ipaddr, help=Add a DNS forwarder)
 parser.add_option(--no-forwarders, dest=no_forwarders, action=store_true,
   default=False, help=Do not add any DNS forwarders, use root servers instead)
 parser.add_option(--no-reverse, dest=no_reverse, action=store_true,
@@ -285,6 +285,8 @@ def install_bind(config, options):
 ip_address = resolve_host(config.host_name)
 if not ip_address:
 sys.exit(Unable to resolve IP address for host name)
+ip = installutils.parse_ip_address(ip_address)
+ip_address = str(ip)
 
 create_reverse = True
 if options.unattended:
diff --git a/install/tools/ipa-replica-prepare b/install/tools/ipa-replica-prepare
index e912235..759f588 100755
--- a/install/tools/ipa-replica-prepare
+++ b/install/tools/ipa-replica-prepare
@@ -24,7 +24,6 @@ import logging, tempfile, shutil, os, pwd
 import traceback
 from ConfigParser import 

Re: [Freeipa-devel] [PATCH] 18 Parse netmasks in IP addresses passed to server install

2011-05-23 Thread Jan Cholasta

On 22.5.2011 18:28, Jakub Hrozek wrote:

On 05/20/2011 08:27 PM, Jan Cholasta wrote:

TODO: Clean unreachable code paths off of ipa-server-install (?)


In general I agree even though I don't know exactly what code you have
in mind -- if the code is dead there's no reason to keep it.


I've noticed that e.g. if the hostname can't be resolved, verify_fqdn 
raises an exception, so some of the checks below the ip = 
resolve_host(host_name) line in ipa-server-install are unnecessary, but 
I'm not yet sure if I'm not missing something.





TODO: Workarounds for netaddr bugs (?)


Are these bugs reported upstream? I know you mentioned some in an
earlier e-mail, just wondering if they are the same.

Long term, it might be better to fix them in netaddr rather than working
around them.


Yes, they're the same and are already fixed (according to the netaddr 
bug tracker), but there's no release with the fixes yet (or it's not in 
Fedora). There are not any big issues that I'm aware of, it's just that 
if you specify incorrect netmask with an IPv4 address, the error message 
isn't very helpful to the user:


netaddr.IPNetwork('192.168.1.1/33')
...
UnboundLocalError: local variable 'ip' referenced before assignment



Jakub



Honza

--
Jan Cholasta

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 18 Parse netmasks in IP addresses passed to server install

2011-05-20 Thread Jan Cholasta

On 10.5.2011 20:06, Jan Cholasta wrote:

Parse netmasks in IP addresses passed to server install.

ticket 1212


Patch updated.

TODO: Write unit test for ipapython.ipautil.CheckedIPAddress
TODO: Clean unreachable code paths off of ipa-server-install (?)
TODO: Workarounds for netaddr bugs (?)

--
Jan Cholasta
From 778ea5b4177bd6dc5e8866e6c84adbc104095b1a Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Fri, 20 May 2011 20:19:25 +0200
Subject: [PATCH] Parse netmasks in IP addresses passed to server install.

ticket 1212
---
 freeipa.spec.in   |1 +
 install/tools/ipa-dns-install |9 +++--
 install/tools/ipa-replica-install |4 ++-
 install/tools/ipa-replica-prepare |   11 +++---
 install/tools/ipa-server-install  |   36 ++---
 ipapython/config.py   |   13 +++-
 ipapython/ipautil.py  |   63 +
 ipaserver/install/installutils.py |   39 ++
 8 files changed, 126 insertions(+), 50 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index b936616..fba2f31 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -188,6 +188,7 @@ Requires: python-kerberos = 1.1-3
 %endif
 Requires: authconfig
 Requires: gnupg
+Requires: iproute
 Requires: pyOpenSSL
 Requires: python-nss = 0.11
 Requires: python-lxml
diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install
index aac85bf..491585b 100755
--- a/install/tools/ipa-dns-install
+++ b/install/tools/ipa-dns-install
@@ -37,9 +37,10 @@ def parse_options():
   sensitive=True, help=admin password)
 parser.add_option(-d, --debug, dest=debug, action=store_true,
   default=False, help=print debugging information)
-parser.add_option(--ip-address, dest=ip_address, help=Master Server IP Address)
+parser.add_option(--ip-address, dest=ip_address,
+  type=ipnet, help=Master Server IP Address)
 parser.add_option(--forwarder, dest=forwarders, action=append,
-  help=Add a DNS forwarder)
+  type=ipaddr, help=Add a DNS forwarder)
 parser.add_option(--no-forwarders, dest=no_forwarders, action=store_true,
   default=False, help=Do not add any DNS forwarders, use root servers instead)
 parser.add_option(--no-reverse, dest=no_reverse,
@@ -130,12 +131,14 @@ def main():
 if options.ip_address:
 ip_address = options.ip_address
 else:
-ip_address = resolve_host(api.env.host)
+hostaddr = resolve_host(api.env.host)
+ip_address = hostaddr and ipautil.CheckedIPAddress(hostaddr)
 if not ip_address or not verify_ip_address(ip_address):
 if options.unattended:
 sys.exit(Unable to resolve IP address for host name)
 else:
 ip_address = read_ip_address(api.env.host, fstore)
+ip_address = str(ip_address)
 logging.debug(will use ip_address: %s\n, ip_address)
 
 if options.no_forwarders:
diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install
index 49df7fe..2b7c8ca 100755
--- a/install/tools/ipa-replica-install
+++ b/install/tools/ipa-replica-install
@@ -63,7 +63,7 @@ def parse_options():
 parser.add_option(--setup-dns, dest=setup_dns, action=store_true,
   default=False, help=configure bind with our zone)
 parser.add_option(--forwarder, dest=forwarders, action=append,
-  help=Add a DNS forwarder)
+  type=ipaddr, help=Add a DNS forwarder)
 parser.add_option(--no-forwarders, dest=no_forwarders, action=store_true,
   default=False, help=Do not add any DNS forwarders, use root servers instead)
 parser.add_option(--no-reverse, dest=no_reverse, action=store_true,
@@ -285,6 +285,8 @@ def install_bind(config, options):
 ip_address = resolve_host(config.host_name)
 if not ip_address:
 sys.exit(Unable to resolve IP address for host name)
+ip = installutils.parse_ip_address(ip_address)
+ip_address = str(ip)
 
 create_reverse = True
 if options.unattended:
diff --git a/install/tools/ipa-replica-prepare b/install/tools/ipa-replica-prepare
index e912235..04a6478 100755
--- a/install/tools/ipa-replica-prepare
+++ b/install/tools/ipa-replica-prepare
@@ -53,7 +53,7 @@ def parse_options():
 parser.add_option(-p, --password, dest=password, 
   help=Directory Manager (existing master) password)
 parser.add_option(--ip-address, dest=ip_address,
-  help=Add A and PTR records of the future replica)
+  type=ipnet, help=Add A and PTR records of the future replica)
 parser.add_option(--ca, dest=ca_file, default=/root/cacert.p12,
   help=Location of CA PKCS#12 file, default /root/cacert.p12)
 parser.add_option(--no-pkinit, dest=setup_pkinit, action=store_false,
@@ -425,10 +425,11 @@ 

[Freeipa-devel] [PATCH] 18 Parse netmasks in IP addresses passed to server install

2011-05-10 Thread Jan Cholasta

Split from patch 3.

https://fedorahosted.org/freeipa/ticket/1212

Honza

--
Jan Cholasta
From 68dacbd0b4ada4c2f02570c167c34c299ceec57e Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Tue, 10 May 2011 19:23:31 +0200
Subject: [PATCH] Parse netmasks in IP addresses passed to server install.

ticket 1212
---
 install/tools/ipa-dns-install |6 ++-
 install/tools/ipa-replica-install |2 +
 install/tools/ipa-replica-prepare |   11 --
 install/tools/ipa-server-install  |   30 +++--
 ipaserver/install/installutils.py |   65 
 tests/test_install/test_utils.py  |   55 +++
 6 files changed, 131 insertions(+), 38 deletions(-)
 create mode 100644 tests/test_install/test_utils.py

diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install
index aac85bf..256c670 100755
--- a/install/tools/ipa-dns-install
+++ b/install/tools/ipa-dns-install
@@ -131,11 +131,13 @@ def main():
 ip_address = options.ip_address
 else:
 ip_address = resolve_host(api.env.host)
-if not ip_address or not verify_ip_address(ip_address):
+ip = parse_ip_address(ip_address)
+if not verify_ip_address(ip):
 if options.unattended:
 sys.exit(Unable to resolve IP address for host name)
 else:
-ip_address = read_ip_address(api.env.host, fstore)
+ip = read_ip_address(api.env.host, fstore)
+ip_address = str(ip)
 logging.debug(will use ip_address: %s\n, ip_address)
 
 if options.no_forwarders:
diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install
index 64f1577..722da37 100755
--- a/install/tools/ipa-replica-install
+++ b/install/tools/ipa-replica-install
@@ -285,6 +285,8 @@ def install_bind(config, options):
 ip_address = resolve_host(config.host_name)
 if not ip_address:
 sys.exit(Unable to resolve IP address for host name)
+ip = installutils.parse_ip_address(ip_address)
+ip_address = str(ip)
 
 create_reverse = True
 if options.unattended:
diff --git a/install/tools/ipa-replica-prepare b/install/tools/ipa-replica-prepare
index e912235..6a4e413 100755
--- a/install/tools/ipa-replica-prepare
+++ b/install/tools/ipa-replica-prepare
@@ -425,10 +425,13 @@ def main():
 name = domain.pop(0)
 domain = ..join(domain)
 
-zone = add_zone(domain, nsaddr=options.ip_address)
-add_rr(zone, name, A, options.ip_address)
-add_reverse_zone(options.ip_address)
-add_ptr_rr(options.ip_address, replica_fqdn)
+ip = installutils.parse_ip_address(options.ip_address)
+ip_address = str(ip)
+
+zone = add_zone(domain, nsaddr=ip_address)
+add_rr(zone, name, A, ip_address)
+add_reverse_zone(ip_address)
+add_ptr_rr(ip_address, replica_fqdn)
 
 try:
 if not os.geteuid()==0:
diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index d50dc61..c256d0e 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -615,37 +615,33 @@ def main():
 domain_name = domain_name.lower()
 
 # Check we have a public IP that is associated with the hostname
-ip = resolve_host(host_name)
-if ip is None:
-if options.ip_address:
-ip = options.ip_address
-if ip is None and options.unattended:
+ip = parse_ip_address(resolve_host(host_name))
+ip_opt = parse_ip_address(options.ip_address)
+if not ip and ip_opt:
+ip = ip_opt
+if not ip and options.unattended:
 sys.exit(Unable to resolve IP address for host name)
 
 if not verify_ip_address(ip):
-ip = 
+ip = None
 if options.unattended:
 sys.exit(1)
 
-if options.ip_address and options.ip_address != ip:
-if options.setup_dns:
-if not verify_ip_address(options.ip_address):
-return 1
-ip = options.ip_address
-else:
+if ip_opt:
+if ip_opt != ip and not options.setup_dns:
 print sys.stderr, Error: the hostname resolves to an IP address that is different
 print sys.stderr, from the one provided on the command line.  Please fix your DNS
 print sys.stderr, or /etc/hosts file and restart the installation.
 return 1
 
-if options.unattended:
-if not ip:
-sys.exit(Unable to resolve IP address)
+ip = ip_opt
+if not verify_ip_address(ip):
+return 1
 
 if not ip:
 ip = read_ip_address(host_name, fstore)
-logging.debug(read ip_address: %s\n % ip)
-ip_address = ip
+logging.debug(read ip_address: %s\n % str(ip))
+ip_address = str(ip)
 
 print The IPA Master Server will be configured with
 print Hostname: + host_name
diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index 3868c4d..8ac24c9