Skip to content

Commit 179659c

Browse files
nemesifiernemesifier
committed
[openwrt] Improved multiple ip address output #59
Improved output of OpenWRT/LEDE UCI configuration when using multiple ip addresses. Cyclomatic complexity has been decreased thanks to the split of many instructions in separate methods. I tried to avoid changing the output of the library when using single ipv4 or ipv6 addresses: this should reduce the possibility of introducing bugs in systems that have been already deployed. Closes #59
1 parent d296c71 commit 179659c

3 files changed

Lines changed: 199 additions & 137 deletions

File tree

‎netjsonconfig/backends/openwrt/converters.py‎

Lines changed: 136 additions & 105 deletions
Original file line numberDiff line numberDiff line change
@@ -76,147 +76,178 @@ def to_intermediate(self):
7676
class Interfaces(BaseConverter):
7777
def to_intermediate(self):
7878
result = []
79-
# this line ensures interfaces are not entirely
80-
# ignored if they do not contain any address
81-
default_address = [{'proto': 'none'}]
8279
for interface in get_copy(self.netjson, 'interfaces'):
8380
i = 1
84-
is_bridge = False
8581
# determine uci logical interface name
86-
uci_name = interface.get('network', interface['name'])
87-
# convert dot and dashes to underscore
88-
uci_name = logical_name(uci_name)
89-
# determine if must be type bridge
90-
if interface.get('type') == 'bridge':
91-
is_bridge = True
92-
bridge_members = ' '.join(interface['bridge_members'])
93-
# ensure address list is not never empty, even when 'addresses' is []
94-
address_list = interface.get('addresses')
95-
if not address_list:
96-
address_list = default_address
97-
# address list defaults to empty list
82+
uci_name = logical_name(interface.get('network', interface['name']))
83+
address_list = self.__get_addresses(interface)
84+
interface = self.__get_interface(interface, uci_name)
85+
# create one or more "config interface" UCI blocks
9886
for address in address_list:
99-
# prepare new UCI interface directive
10087
uci_interface = deepcopy(interface)
101-
if 'network' in uci_interface:
102-
del uci_interface['network']
103-
if 'mac' in uci_interface:
104-
if interface.get('type') != 'wireless':
105-
uci_interface['macaddr'] = interface['mac']
106-
del uci_interface['mac']
107-
if 'autostart' in uci_interface:
108-
uci_interface['auto'] = interface['autostart']
109-
del uci_interface['autostart']
110-
if 'disabled' in uci_interface:
111-
uci_interface['enabled'] = not interface['disabled']
112-
del uci_interface['disabled']
113-
if 'addresses' in uci_interface:
114-
del uci_interface['addresses']
115-
if 'type' in uci_interface:
116-
del uci_interface['type']
117-
if 'wireless' in uci_interface:
118-
del uci_interface['wireless']
119-
# default values
120-
address_key = None
121-
address_value = None
122-
netmask = None
123-
proto = self.__get_proto(uci_interface, address)
124-
# add suffix if there is more than one config block
88+
# add suffix to logical name when
89+
# there is more than one interface
12590
if i > 1:
126-
name = '{name}_{i}'.format(name=uci_name, i=i)
127-
else:
128-
name = uci_name
129-
if address.get('family') == 'ipv4':
130-
address_key = 'ipaddr'
131-
elif address.get('family') == 'ipv6':
132-
address_key = 'ip6addr'
133-
proto = proto.replace('dhcp', 'dhcpv6')
134-
if address.get('address') and address.get('mask'):
135-
address_value = '{address}/{mask}'.format(**address)
136-
# do not use CIDR notation when using ipv4
137-
# see https://github.com/openwisp/netjsonconfig/issues/54
138-
if address.get('family') == 'ipv4':
139-
netmask = str(ip_interface(address_value).netmask)
140-
address_value = address['address']
141-
# update interface dict
91+
uci_interface['.name'] = '{name}_{i}'.format(name=uci_name, i=i)
14292
uci_interface.update({
143-
'.name': name,
144-
'.type': 'interface',
145-
'ifname': uci_interface.pop('name'),
146-
'proto': proto,
14793
'dns': self.__get_dns_servers(uci_interface, address),
148-
'dns_search': self.__get_dns_search(uci_interface, address)
94+
'dns_search': self.__get_dns_search(uci_interface, address),
95+
'proto': self.__get_proto(uci_interface, address),
14996
})
150-
# bridging
151-
if is_bridge:
152-
uci_interface['type'] = 'bridge'
153-
# put bridge members in ifname attribute
154-
if bridge_members:
155-
uci_interface['ifname'] = bridge_members
156-
# if no members, this is an empty bridge
157-
else:
158-
uci_interface['bridge_empty'] = True
159-
del uci_interface['ifname']
160-
# ensure type "bridge" is only given to one logical interface
161-
is_bridge = False
162-
# bridge has already been defined
163-
# but we need to add more references to it
164-
elif interface.get('type') == 'bridge':
165-
# openwrt adds "br-"" prefix to bridge interfaces
166-
# we need to take this into account when referring
167-
# to these physical names
168-
uci_interface['ifname'] = 'br-{0}'.format(interface['name'])
169-
# delete bridge_members attribtue if bridge is empty
170-
if uci_interface.get('bridge_members') is not None:
171-
del uci_interface['bridge_members']
172-
# add address if any (with correct option name)
173-
if address_key and address_value:
174-
uci_interface[address_key] = address_value
175-
# add netmask option (only for IPv4)
176-
if netmask:
177-
uci_interface['netmask'] = netmask
178-
# merge additional address fields (discard default ones first)
179-
address_copy = address.copy()
180-
for key in ['address', 'mask', 'proto', 'family']:
181-
if key in address_copy:
182-
del address_copy[key]
183-
uci_interface.update(address_copy)
184-
# append to interface list
97+
uci_interface = self.__get_bridge(uci_interface, i)
98+
if address:
99+
uci_interface.update(address)
185100
result.append(sorted_dict(uci_interface))
186101
i += 1
187102
return (('network', result),)
188103

104+
def __get_addresses(self, interface):
105+
"""
106+
converts NetJSON address to
107+
UCI intermediate data structure
108+
"""
109+
address_list = get_copy(interface, 'addresses')
110+
# do not ignore interfaces if they do not contain any address
111+
if not address_list:
112+
return [{'proto': 'none'}]
113+
result = []
114+
static = {}
115+
dhcp = []
116+
for address in address_list:
117+
family = address.get('family')
118+
# dhcp
119+
if address['proto'] == 'dhcp':
120+
address['proto'] = 'dhcp' if family == 'ipv4' else 'dhcpv6'
121+
dhcp.append(self.__del_address_keys(address))
122+
continue
123+
# static
124+
address_key = 'ipaddr' if family == 'ipv4' else 'ip6addr'
125+
static.setdefault(address_key, [])
126+
static[address_key].append('{address}/{mask}'.format(**address))
127+
static.update(self.__del_address_keys(address))
128+
if static:
129+
# do not use CIDR notation when using a single ipv4
130+
# see https://github.com/openwisp/netjsonconfig/issues/54
131+
if len(static.get('ipaddr', [])) == 1:
132+
network = ip_interface(static['ipaddr'][0])
133+
static['ipaddr'] = str(network.ip)
134+
static['netmask'] = str(network.netmask)
135+
# do not use lists when using a single ipv6 address
136+
# (avoids to change output of existing configuration)
137+
if len(static.get('ip6addr', [])) == 1:
138+
static['ip6addr'] = static['ip6addr'][0]
139+
result.append(static)
140+
if dhcp:
141+
result += dhcp
142+
return result
143+
144+
def __get_interface(self, interface, uci_name):
145+
"""
146+
converts NetJSON interface to
147+
UCI intermediate data structure
148+
"""
149+
interface.update({
150+
'.type': 'interface',
151+
'.name': uci_name,
152+
'ifname': interface.pop('name')
153+
})
154+
if 'network' in interface:
155+
del interface['network']
156+
if 'mac' in interface:
157+
# mac address of wireless interface must
158+
# be set in /etc/config/wireless, therfore
159+
# we can skip this in /etc/config/network
160+
if interface.get('type') != 'wireless':
161+
interface['macaddr'] = interface['mac']
162+
del interface['mac']
163+
if 'autostart' in interface:
164+
interface['auto'] = interface['autostart']
165+
del interface['autostart']
166+
if 'disabled' in interface:
167+
interface['enabled'] = not interface['disabled']
168+
del interface['disabled']
169+
if 'wireless' in interface:
170+
del interface['wireless']
171+
if 'addresses' in interface:
172+
del interface['addresses']
173+
return interface
174+
175+
_address_keys = ['address', 'mask', 'family']
176+
177+
def __del_address_keys(self, address):
178+
"""
179+
deletes NetJSON address keys
180+
"""
181+
for key in self._address_keys:
182+
if key in address:
183+
del address[key]
184+
return address
185+
186+
def __get_bridge(self, interface, i):
187+
"""
188+
converts NetJSON bridge to
189+
UCI intermediate data structure
190+
"""
191+
# ensure type "bridge" is only given to one logical interface
192+
if interface['type'] == 'bridge' and i < 2:
193+
bridge_members = ' '.join(interface.pop('bridge_members'))
194+
# put bridge members in ifname attribute
195+
if bridge_members:
196+
interface['ifname'] = bridge_members
197+
# if no members, this is an empty bridge
198+
else:
199+
interface['bridge_empty'] = True
200+
del interface['ifname']
201+
# bridge has already been defined
202+
# but we need to add more references to it
203+
elif interface['type'] == 'bridge' and i >= 2:
204+
# openwrt adds "br-" prefix to bridge interfaces
205+
# we need to take this into account when referring
206+
# to these physical names
207+
interface['ifname'] = 'br-{ifname}'.format(**interface)
208+
# do not repeat bridge attributes (they have already been processed)
209+
del interface['type']
210+
del interface['bridge_members']
211+
elif interface['type'] != 'bridge':
212+
del interface['type']
213+
return interface
214+
189215
def __get_proto(self, interface, address):
190216
"""
191-
determines interface "proto" option
217+
determines UCI interface "proto" option
192218
"""
219+
# proto defaults to static
220+
address_proto = address.pop('proto', 'static')
193221
if 'proto' not in interface:
194-
# proto defaults to static
195-
return address.get('proto', 'static')
222+
return address_proto
196223
else:
197-
# allow override
198-
return interface['proto']
224+
# allow override on interface level
225+
return interface.pop('proto')
199226

200227
def __get_dns_servers(self, uci, address):
228+
"""
229+
determines UCI interface "dns" option
230+
"""
201231
# allow override
202232
if 'dns' in uci:
203233
return uci['dns']
204234
# ignore if using DHCP or if "proto" is none
205-
if address['proto'] in ['dhcp', 'none']:
235+
if address['proto'] in ['dhcp', 'dhcpv6', 'none']:
206236
return None
207-
# general setting
208237
dns = self.netjson.get('dns_servers', None)
209238
if dns:
210239
return ' '.join(dns)
211240

212241
def __get_dns_search(self, uci, address):
242+
"""
243+
determines UCI interface "dns_search" option
244+
"""
213245
# allow override
214246
if 'dns_search' in uci:
215247
return uci['dns_search']
216248
# ignore if "proto" is none
217249
if address['proto'] == 'none':
218250
return None
219-
# general setting
220251
dns_search = self.netjson.get('dns_search', None)
221252
if dns_search:
222253
return ' '.join(dns_search)

‎runflake8‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,5 @@
11
#!/bin/bash
22
set -e
3-
complex="./netjsonconfig/backends/openwrt/converters.py"
4-
5-
flake8 --max-line-length=110 --max-complexity=26 "$complex" || exit 1
63
flake8 --max-line-length=110 \
74
--max-complexity=12 \
8-
--exclude=./docs/,./build/,./setup.py,$complex || exit 1
5+
--exclude=./docs/,./build/,./setup.py || exit 1

0 commit comments

Comments
 (0)