[Freeipa-devel] [PATCH] 3 Add ability to specify netmask with IP addresses during installation

[Jan Cholasta]

On 20.4.2011 22:08, Rob Crittenden wrote:
> Jan Cholasta wrote:
>> On 11.4.2011 12:48, Jan Cholasta wrote:
>>> On 8.4.2011 16:26, Rob Crittenden wrote:
>>>> Jan Cholasta wrote:
>>>>> On 29.3.2011 22:15, Rob Crittenden wrote:
>>>>>> Jan Cholasta wrote:
>>>>>>> Sorry, forgot to attach the patch.
>>>>>>>
>>>>>>
>>>>>> Is this why you have some blind excepts?
>>>>>>
>>>>>> installutils._IPAddressWithPrefix('192.168.0.1/33')
>>>>>> Traceback (most recent call last):
>>>>>> File "<stdin>", line 1, in <module>
>>>>>> File "ipaserver/install/installutils.py", line 167, in __init__
>>>>>> net = netaddr.IPNetwork(addr)
>>>>>> File "/usr/lib/python2.7/site-packages/netaddr/ip/__init__.py", line
>>>>>> 919, in __init__
>>>>>> implicit_prefix, flags)
>>>>>> File "/usr/lib/python2.7/site-packages/netaddr/ip/__init__.py", line
>>>>>> 782, in parse_ip_network
>>>>>> value = ip._value
>>>>>> UnboundLocalError: local variable 'ip' referenced before assignment
>>>>>>
>>>>>> We should get an upstream bug filed on python-netaddr about this.
>>>>>
>>>>> https://github.com/drkjam/netaddr/issues/closed#issue/5
>>>>> https://github.com/drkjam/netaddr/issues/closed#issue/6
>>>>> https://github.com/drkjam/netaddr/issues/closed#issue/8
>>>>>
>>>>> Apparently it's already been fixed for the next release.
>>>>>
>>>>> IMHO it's not much of an issue for us, because the exception gets
>>>>> caught
>>>>> in parse_ip_address and that's currently the only place where
>>>>> _IPAddressWithPrefix is used.
>>>>>
>>>>>>
>>>>>> Shoudl parse_ip_address() raise an exception on bad data rather than
>>>>>> returning 0.0.0.0?
>>>>>
>>>>> I've been down that road and it would need a rewrite of the fragile IP
>>>>> address handling logic of ipa-server-install, which is something I'd
>>>>> rather avoid.
>>>>>
>>>>>>
>>>>>> >>> installutils.parse_ip_address('355.555.3.3')
>>>>>> _IPAddressWithPrefix('0.0.0.0')
>>>>>>
>>>>>> or
>>>>>>
>>>>>> >>> installutils.parse_ip_address('192.168.0.1/55')
>>>>>> _IPAddressWithPrefix('0.0.0.0')
>>>>>>
>>>>>> Should it disallow net addresses like 192.168.0.0?
>>>>>
>>>>> If you mean network and broadcast addresses, it probably should. It
>>>>> might be a good idea to disallow localhost, multicast and/or
>>>>> link-local
>>>>> addresses too.
>>>>
>>>> Are you going to resubmit the patch with these added or should we
>>>> open a
>>>> separate ticket?
>>>
>>> I'm going to resubmit it. Right now it disallows loopback, IANA
>>> reserved, link-local, network, multicast and broadcast IP addresses.
>>> Does it make sense to also allow only IP addresses attached to one of
>>> the local network interfaces? Perhaps it would be sufficient just to
>>> print a warning. Or should we not care about that at all?
>>
>> Sending the updated patch.
>
> This looks ok, just one question. Should we add a dependency on the
> iproute package because of the /sbin/ip package?

Yes, we should.

>
> rob


-- 
Jan Cholasta