Re: [Freeipa-devel] [PATCH 0146] Fix internal errors in host-add and other commands caused by DNS resolutio
On 01.07.2016 10:37, Martin Basti wrote: On 01.07.2016 09:05, Petr Spacek wrote: On 30.6.2016 21:23, Petr Spacek wrote: Hello, Fix internal errors in host-add and other commands caused by DNS resolution Previously resolver was returning CheckedIPAddress objects. This internal server error in cases where DNS actually returned reserved IP addresses. Now the resolver is returning UnsafeIPAddress objects which do syntactic checks but do not filter IP addresses. >From now on we can decide if some IP address should be accepted as-is or if it needs to be contrained to some subset of IP addresses using CheckedIPAddress class. This regression was caused by changes for https://fedorahosted.org/freeipa/ticket/5710 I've split parser and checks into separate classes. Attached script CheckedIPAddressRefactoring.py uses python-hypothesis to compare results from old and new implementations. It seems that all valid inputs return the very same results. The new implementation is a bit stricter when it comes to invalid inputs (parse_netmask=False & addr=IPNetwork instance) but as far as I can tell this case could not happen in current IPA anyway. ipa-server-install, ipa-client-install, ipa-replica-install, and ipa-ca-install on replica seem to work. DNS records for ipa-ca were properly updated after replica installation. Also installation on server without A/ record in DNS and subsequent ipa-dns-install worked just fine. My bad, I forgot to attach cleanup patch 147 which is prerequisite for 146. (Sorry for the numbering.) ACK master: * ce1f9ca51bd91ed66233c1bac7eb05fac9c855c7 Remove unused is_local(), interface, and defaultnet from CheckedIPAddress * 5e78b54d7c532bec0ee5a4ce3f1b6d6c94d17c51 Fix internal errors in host-add and other commands caused by DNS resolution I will review 4.3 later ACK ipa-4-3: * 0db277eb224b92319aede31d1840db781c10 Remove unused is_local(), interface, and defaultnet from CheckedIPAddress * b8d5881ba93b00653ba42c61369f19ca27fb7a64 Fix internal errors in host-add and other commands caused by DNS resolution -- 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 0146] Fix internal errors in host-add and other commands caused by DNS resolutio
On 01.07.2016 10:37, Martin Basti wrote: On 01.07.2016 09:05, Petr Spacek wrote: On 30.6.2016 21:23, Petr Spacek wrote: Hello, Fix internal errors in host-add and other commands caused by DNS resolution Previously resolver was returning CheckedIPAddress objects. This internal server error in cases where DNS actually returned reserved IP addresses. Now the resolver is returning UnsafeIPAddress objects which do syntactic checks but do not filter IP addresses. >From now on we can decide if some IP address should be accepted as-is or if it needs to be contrained to some subset of IP addresses using CheckedIPAddress class. This regression was caused by changes for https://fedorahosted.org/freeipa/ticket/5710 I've split parser and checks into separate classes. Attached script CheckedIPAddressRefactoring.py uses python-hypothesis to compare results from old and new implementations. It seems that all valid inputs return the very same results. The new implementation is a bit stricter when it comes to invalid inputs (parse_netmask=False & addr=IPNetwork instance) but as far as I can tell this case could not happen in current IPA anyway. ipa-server-install, ipa-client-install, ipa-replica-install, and ipa-ca-install on replica seem to work. DNS records for ipa-ca were properly updated after replica installation. Also installation on server without A/ record in DNS and subsequent ipa-dns-install worked just fine. My bad, I forgot to attach cleanup patch 147 which is prerequisite for 146. (Sorry for the numbering.) ACK master: * ce1f9ca51bd91ed66233c1bac7eb05fac9c855c7 Remove unused is_local(), interface, and defaultnet from CheckedIPAddress * 5e78b54d7c532bec0ee5a4ce3f1b6d6c94d17c51 Fix internal errors in host-add and other commands caused by DNS resolution I will review 4.3 later ACK ipa-4-3: * 0db277eb224b92319aede31d1840db781c10 Remove unused is_local(), interface, and defaultnet from CheckedIPAddress * b8d5881ba93b00653ba42c61369f19ca27fb7a64 Fix internal errors in host-add and other commands caused by DNS resolution -- 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 0146] Fix internal errors in host-add and other commands caused by DNS resolutio
On 01.07.2016 09:05, Petr Spacek wrote: On 30.6.2016 21:23, Petr Spacek wrote: Hello, Fix internal errors in host-add and other commands caused by DNS resolution Previously resolver was returning CheckedIPAddress objects. This internal server error in cases where DNS actually returned reserved IP addresses. Now the resolver is returning UnsafeIPAddress objects which do syntactic checks but do not filter IP addresses. >From now on we can decide if some IP address should be accepted as-is or if it needs to be contrained to some subset of IP addresses using CheckedIPAddress class. This regression was caused by changes for https://fedorahosted.org/freeipa/ticket/5710 I've split parser and checks into separate classes. Attached script CheckedIPAddressRefactoring.py uses python-hypothesis to compare results from old and new implementations. It seems that all valid inputs return the very same results. The new implementation is a bit stricter when it comes to invalid inputs (parse_netmask=False & addr=IPNetwork instance) but as far as I can tell this case could not happen in current IPA anyway. ipa-server-install, ipa-client-install, ipa-replica-install, and ipa-ca-install on replica seem to work. DNS records for ipa-ca were properly updated after replica installation. Also installation on server without A/ record in DNS and subsequent ipa-dns-install worked just fine. My bad, I forgot to attach cleanup patch 147 which is prerequisite for 146. (Sorry for the numbering.) ACK master: * ce1f9ca51bd91ed66233c1bac7eb05fac9c855c7 Remove unused is_local(), interface, and defaultnet from CheckedIPAddress * 5e78b54d7c532bec0ee5a4ce3f1b6d6c94d17c51 Fix internal errors in host-add and other commands caused by DNS resolution I will review 4.3 later -- 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 0146] Fix internal errors in host-add and other commands caused by DNS resolutio
On 30.6.2016 21:23, Petr Spacek wrote:
> Hello,
>
> Fix internal errors in host-add and other commands caused by DNS resolution
>
> Previously resolver was returning CheckedIPAddress objects. This
> internal server error in cases where DNS actually returned reserved IP
> addresses.
>
> Now the resolver is returning UnsafeIPAddress objects which do syntactic
> checks but do not filter IP addresses.
>
>>From now on we can decide if some IP address should be accepted as-is or
> if it needs to be contrained to some subset of IP addresses using
> CheckedIPAddress class.
>
> This regression was caused by changes for
> https://fedorahosted.org/freeipa/ticket/5710
>
>
>
> I've split parser and checks into separate classes. Attached script
> CheckedIPAddressRefactoring.py uses python-hypothesis to compare results from
> old and new implementations. It seems that all valid inputs return the very
> same results. The new implementation is a bit stricter when it comes to
> invalid inputs (parse_netmask=False & addr=IPNetwork instance) but as far as I
> can tell this case could not happen in current IPA anyway.
>
> ipa-server-install, ipa-client-install, ipa-replica-install, and
> ipa-ca-install on replica seem to work. DNS records for ipa-ca were properly
> updated after replica installation. Also installation on server without A/
> record in DNS and subsequent ipa-dns-install worked just fine.
My bad, I forgot to attach cleanup patch 147 which is prerequisite for 146.
(Sorry for the numbering.)
--
Petr^2 Spacek
From 8fd4b5a37c4144328ae5a56b6fd7f8dd21d556ae Mon Sep 17 00:00:00 2001
From: Petr Spacek
Date: Thu, 30 Jun 2016 13:57:52 +0200
Subject: [PATCH] Remove unused is_local(), interface, and defaultnet from
CheckedIPAddress
All these were unused so I'm removing them to keep the code clean and
easier to read. At this point it is clear that only difference between
netaddr.IPAddress and CheckedIPAddress is prefixlen attribute.
---
ipapython/ipautil.py | 9 -
1 file changed, 9 deletions(-)
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index c7e20c5102efaa006c10d4c3af849bc259da43e7..8506bf2d58a35cb128e44cee34c3fb62d7a0c5e4 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -87,13 +87,10 @@ class CheckedIPAddress(netaddr.IPAddress):
if isinstance(addr, CheckedIPAddress):
super(CheckedIPAddress, self).__init__(addr, flags=self.netaddr_ip_flags)
self.prefixlen = addr.prefixlen
-self.defaultnet = addr.defaultnet
-self.interface = addr.interface
return
net = None
iface = None
-defnet = False
if isinstance(addr, netaddr.IPNetwork):
net = addr
@@ -161,7 +158,6 @@ class CheckedIPAddress(netaddr.IPAddress):
raise ValueError('No network interface matches the provided IP address and netmask')
if net is None:
-defnet = True
if addr.version == 4:
net = netaddr.IPNetwork(netaddr.cidr_abbrev_to_verbose(str(addr)))
elif addr.version == 6:
@@ -174,11 +170,6 @@ class CheckedIPAddress(netaddr.IPAddress):
super(CheckedIPAddress, self).__init__(addr, flags=self.netaddr_ip_flags)
self.prefixlen = net.prefixlen
-self.defaultnet = defnet
-self.interface = iface
-
-def is_local(self):
-return self.interface is not None
def valid_ip(addr):
return netaddr.valid_ipv4(addr) or netaddr.valid_ipv6(addr)
--
2.7.4
--
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
