Skip to content

Commit e98bbb6

Browse files
akokhanakokhan
authored
Reworked IP validation in "config interface ip add/remove" command (#1709)
- Renamed validate_ip_mask() to is_valid_ip_interface() as per code style - Updated is_valid_ip_interface() to do not modify the IP address - Updated UTs per changes Signed-off-by: Andriy Kokhan <andriyx.kokhan@intel.com>
1 parent 866d1d7 commit e98bbb6

File tree

2 files changed

+31
-34
lines changed

2 files changed

+31
-34
lines changed

‎config/main.py‎

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -804,21 +804,26 @@ def validate_mirror_session_config(config_db, session_name, dst_port, src_port,
804804

805805
return True
806806

807-
def validate_ip_mask(ctx, ip_addr):
807+
def is_valid_ip_interface(ctx, ip_addr):
808808
split_ip_mask = ip_addr.split("/")
809+
if len(split_ip_mask) < 2:
810+
return False
811+
809812
# Check if the IP address is correct or if there are leading zeros.
810813
ip_obj = ipaddress.ip_address(split_ip_mask[0])
811814

812-
# Check if the mask is correct
813-
mask_range = 33 if isinstance(ip_obj, ipaddress.IPv4Address) else 129
814-
# If mask is not specified
815-
if len(split_ip_mask) < 2:
816-
return 0
815+
if isinstance(ip_obj, ipaddress.IPv4Address):
816+
# Since the IP address is used as a part of a key in Redis DB,
817+
# do not tolerate extra zeros in IPv4.
818+
if str(ip_obj) != split_ip_mask[0]:
819+
return False
817820

818-
if not int(split_ip_mask[1]) in range(1, mask_range):
819-
return 0
821+
# Check if the mask is correct
822+
net = ipaddress.ip_network(ip_addr, strict=False)
823+
if str(net.prefixlen) != split_ip_mask[1] or net.prefixlen == 0:
824+
return False
820825

821-
return str(ip_obj) + '/' + str(int(split_ip_mask[1]))
826+
return True
822827

823828
def cli_sroute_to_config(ctx, command_str, strict_nh = True):
824829
if len(command_str) < 2 or len(command_str) > 9:
@@ -3587,10 +3592,9 @@ def add(ctx, interface_name, ip_addr, gw):
35873592
try:
35883593
net = ipaddress.ip_network(ip_addr, strict=False)
35893594
if '/' not in ip_addr:
3590-
ip_addr = str(net)
3595+
ip_addr += '/' + str(net.prefixlen)
35913596

3592-
ip_addr = validate_ip_mask(ctx, ip_addr)
3593-
if not ip_addr:
3597+
if not is_valid_ip_interface(ctx, ip_addr):
35943598
raise ValueError('')
35953599

35963600
if interface_name == 'eth0':
@@ -3652,10 +3656,9 @@ def remove(ctx, interface_name, ip_addr):
36523656
try:
36533657
net = ipaddress.ip_network(ip_addr, strict=False)
36543658
if '/' not in ip_addr:
3655-
ip_addr = str(net)
3656-
3657-
ip_addr = validate_ip_mask(ctx, ip_addr)
3658-
if not ip_addr:
3659+
ip_addr += '/' + str(net.prefixlen)
3660+
3661+
if not is_valid_ip_interface(ctx, ip_addr):
36593662
raise ValueError('')
36603663

36613664
if interface_name == 'eth0':

‎tests/ip_config_test.py‎

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ def test_add_del_interface_valid_ipv4(self):
2626
obj = {'config_db':db.cfgdb}
2727

2828
# config int ip add Ethernet64 10.10.10.1/24
29-
result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet64", "10.10.10.1/24"], obj=obj)
29+
result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet64", "10.10.10.1/24"], obj=obj)
3030
print(result.exit_code, result.output)
3131
assert result.exit_code == 0
3232
assert ('Ethernet64', '10.10.10.1/24') in db.cfgdb.get_table('INTERFACE')
@@ -59,22 +59,16 @@ def test_add_interface_ipv4_invalid_mask(self):
5959
assert result.exit_code != 0
6060
assert ERROR_MSG in result.output
6161

62-
def test_add_del_interface_ipv4_with_leading_zeros(self):
62+
def test_add_interface_ipv4_with_leading_zeros(self):
6363
db = Db()
6464
runner = CliRunner()
6565
obj = {'config_db':db.cfgdb}
6666

6767
# config int ip add Ethernet68 10.10.10.002/24
6868
result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet68", "10.10.10.002/24"], obj=obj)
6969
print(result.exit_code, result.output)
70-
assert result.exit_code == 0
71-
assert ('Ethernet68', '10.10.10.2/24') in db.cfgdb.get_table('INTERFACE')
72-
73-
# config int ip remove Ethernet68 10.10.10.002/24
74-
result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet68", "10.10.10.002/24"], obj=obj)
75-
print(result.exit_code, result.output)
7670
assert result.exit_code != 0
77-
assert ('Ethernet68', '10.10.10.2/24') not in db.cfgdb.get_table('INTERFACE')
71+
assert ERROR_MSG in result.output
7872

7973
''' Tests for IPv6 '''
8074

@@ -84,13 +78,13 @@ def test_add_del_interface_valid_ipv6(self):
8478
obj = {'config_db':db.cfgdb}
8579

8680
# config int ip add Ethernet72 2001:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34
87-
result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet72", "2001:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34"], obj=obj)
81+
result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet72", "2001:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34"], obj=obj)
8882
print(result.exit_code, result.output)
8983
assert result.exit_code == 0
9084
assert ('Ethernet72', '2001:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34') in db.cfgdb.get_table('INTERFACE')
9185

9286
# config int ip remove Ethernet72 2001:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34
93-
result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet72", "2001:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34"], obj=obj)
87+
result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet72", "2001:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34"], obj=obj)
9488
print(result.exit_code, result.output)
9589
assert result.exit_code != 0
9690
assert ('Ethernet72', '2001:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34') not in db.cfgdb.get_table('INTERFACE')
@@ -122,34 +116,34 @@ def test_add_del_interface_ipv6_with_leading_zeros(self):
122116
runner = CliRunner()
123117
obj = {'config_db':db.cfgdb}
124118

125-
# config int ip del Ethernet68 2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/34
119+
# config int ip add Ethernet68 2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/34
126120
result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet68", "2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/34"], obj=obj)
127121
print(result.exit_code, result.output)
128122
assert result.exit_code == 0
129-
assert ('Ethernet68', '2001:db8:11a3:9d7:1f34:8a2e:7a0:765d/34') in db.cfgdb.get_table('INTERFACE')
123+
assert ('Ethernet68', '2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/34') in db.cfgdb.get_table('INTERFACE')
130124

131125
# config int ip remove Ethernet68 2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/34
132126
result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet68", "2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/34"], obj=obj)
133127
print(result.exit_code, result.output)
134128
assert result.exit_code != 0
135-
assert ('Ethernet68', '2001:db8:11a3:9d7:1f34:8a2e:7a0:765d/34') not in db.cfgdb.get_table('INTERFACE')
129+
assert ('Ethernet68', '2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/34') not in db.cfgdb.get_table('INTERFACE')
136130

137131
def test_add_del_interface_shortened_ipv6_with_leading_zeros(self):
138132
db = Db()
139133
runner = CliRunner()
140134
obj = {'config_db':db.cfgdb}
141135

142-
# config int ip del Ethernet68 3000::001/64
136+
# config int ip add Ethernet68 3000::001/64
143137
result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet68", "3000::001/64"], obj=obj)
144138
print(result.exit_code, result.output)
145139
assert result.exit_code == 0
146-
assert ('Ethernet68', '3000::1/64') in db.cfgdb.get_table('INTERFACE')
140+
assert ('Ethernet68', '3000::001/64') in db.cfgdb.get_table('INTERFACE')
147141

148142
# config int ip remove Ethernet68 3000::001/64
149-
result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet68", "3000::001/64"], obj=obj)
143+
result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet68", "3000::001/64"], obj=obj)
150144
print(result.exit_code, result.output)
151145
assert result.exit_code != 0
152-
assert ('Ethernet68', '3000::1/64') not in db.cfgdb.get_table('INTERFACE')
146+
assert ('Ethernet68', '3000::001/64') not in db.cfgdb.get_table('INTERFACE')
153147

154148
@classmethod
155149
def teardown_class(cls):

0 commit comments

Comments
 (0)