[Python-Dev] PEP 3144 review.

[R. David Murray]

On Tue, 29 Sep 2009 at 14:19, Glyph Lefkowitz wrote:
> In addition to the somebody-must-have-mentioned-this-already feeling that I
> got, I hesitated to post this message because it doesn't actually seem that
> important to me.  While I'm firmly convinced that Network.ip is a design
> mistake, it's not like the rest of Python, or for that matter any software,
> is completely perfect.  In fact I think this mistake is significantly less
> bad than some of the others already present in Python.  If Peter remains
> unconvinced, I do think that we should put it in the stdlib, move on, and
> get to fixing some of the other stuff we agree needs fixing rather than
> continuing to re-hash this.  Primarily because, as far as I can tell, if
> hashing and equality are defined the way that everyone seems to be agreeing
> they be defined (ignoring the .ip attribute) then those of us who think .ip
> is a design error can use the library and safely ignore it completely.

I think Glyph is petty much on target here.  I think I dislike the .ip
attribute more than he does, but I'm willing to let go of it now
that the equality issue has been addressed so that the class acts
like a Network object _except_ for the ip attribute.

I do want to show why I think the ip attribute is a bad idea, using
a concrete example, rather than the hand waving we've done up to this
point.  This is a stripped down example of something that could be a
real application.  I started writing this to show what the difference
between using the ipaddr classes versus the completely separate address
and network classes would look like, but ended up with something where
there would be very little difference in the final program.

Given this simple configuration file:

demo.ini
--------------------------------------------------------------
[gatewayrouter]
inside =  e1/0 192.168.1.1/24
outside = e1/1 172.16.200.1/23
dmz =     e1/2 192.168.2.1/24

[dmzhosts]
webserver =  172.16.200.2 192.168.2.30 22 80 442
mailserver = 172.16.200.3 182.168.2.31 22 25 587
--------------------------------------------------------------


Here is a (runable) program to generate the skeleton of a Cisco router
configuration file from that configuration (I've left out a bunch
of additinoal Cisco configuration I'd include if this were a real
application):

--------------------------------------------------------------
import ipaddr
import ConfigParser

config = ConfigParser.SafeConfigParser()
config.read('demo.ini')
output = open('gatewayrouter.config', 'w')


class InterfaceData(object):

     def __init__(self, interfacespec):
         self.interfacename, ipwithnetmask = interfacespec.split()
         self.network = ipaddr.IPv4Network(ipwithnetmask)
         self.ip = self.network.ip
         self.netmask = self.network.netmask


class DMZHost(object):

     def __init__(self, hostdata):
         data = hostdata.split()
         outsideip, insideip = data[0:2]
         self.ports = data[2:]
         self.outsideip = ipaddr.IPv4Address(outsideip)
         self.insideip = ipaddr.IPv4Address(insideip)


interfaces = {}
for name in ('inside', 'outside', 'dmz'):
     interfacedata = InterfaceData(config.get('gatewayrouter', name))
     interfaces[name] = interfacedata
     globals()[name] = interfacedata.network

dmzhosts = {}
for host, hostdata in config.items('dmzhosts'):
     dmzhosts[host] = DMZHost(hostdata)

for idata in interfaces.values():
     print >>output, 'interface {}'.format(idata.interfacename)
     print >>output, 'ip address {} {}'.format(idata.ip, idata.netmask)

print >>output, 'ip extended access-list outside_in'
for host in dmzhosts.values():
     for port in host.ports:
         print >>output, 'permit ip any host {} eq {}'.format(
             host.outsideip, port)
for icmp in ('echo', 'echo-reply', 'host-unreachable', 'time-exceeded'):
     print >>output, 'permit icmp any host {} {}'.format(
         interfaces['inside'].ip, icmp)
     print >>output, 'permit icmp any {} {} {}'.format(
         dmz.network, dmz.hostmask, icmp)

print >>output, 'ip extended access-list nat'
print >>output, 'permit ip {} {} any'.format(inside.network, inside.hostmask)
print >>output, 'permit ip {} {} any'.format(dmz.network, dmz.hostmask)

print >>output, ('ip nat inside source access-list nat '
     'interface {} overload').format(interfaces['outside'].interfacename)
for host in dmzhosts.values():
     print >>output, 'ip nat inside source static {} {}'.format(
         host.insideip, host.outsideip)
--------------------------------------------------------------

There are several things to note about this code.  The first is that I
wanted my own wrapper classes to deal with the data, IPv4Network/Address
by themselves weren't enough, nor would an IPWithNetmask class have been.
I also notice that I immediately pulled out the 'ip' address from the
IPv4Network object to a top level attribute in my wrapper, so for me
a ParseIPWithMask that returned separate IPAddress and an IPNetwork
classes would have been more natural.  On the other hand, I also pulled
out the 'netmask', which I would still have needed to do.

There's one place in this code where the inclusion of the 'ip' information
in the IPNetwork class could have been used.  In the line that allows ICMP
traffic to the router's outside port, I could have written 'inside.ip'
instead of interfaces['inside'].ip.  I think that would have confused me
when I came back to read the code later, though, since 'inside' is
otherwise a network object.

But the real 'ah ha' moment in writing this program came when I wrote
the lines for the 'nat' access list.  I first wrote the line like
this:

print >>output, 'permit ip {} {} any'.format(inside.ip, inside.hostmask)

because I wanted the base IP of the inside network to go into the
access-list statement.  The code would have run, but it would have
produced incorrect output.  It also felt unnatural to write '.network'
there, since the object, in my mind, _was_ a network, which is presumably
why I wrote 'inside.ip' at first, before catching myself.  But that I
can certainly live with, just as I can live with '.ip' if I have to :)

--David