From 7b882174c2d7f7c21e91d994b57a1e3fa7cfb26a Mon Sep 17 00:00:00 2001 From: d-dashkov Date: Tue, 9 Feb 2021 11:57:17 +0200 Subject: [PATCH 01/15] Added ip check to config int Signed-off-by: d-dashkov --- config/main.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/config/main.py b/config/main.py index 3ad1e317a1..21296d9f04 100644 --- a/config/main.py +++ b/config/main.py @@ -2652,9 +2652,12 @@ def add(ctx, interface_name, ip_addr, gw): return try: - net = ipaddress.ip_network(ip_addr, strict=False) - if '/' not in ip_addr: - ip_addr = str(net) + split_ip_mask = ip_addr.split("/") + # Check if the IP address is correct or throw a ValueError exception + ipaddress.ip_address(split_ip_mask[0]) + # Check the correctness of the mask + if len(split_ip_mask) < 2 or not int(split_ip_mask[1]) in range(31): + ctx.fail("ip mask is not valid") if interface_name == 'eth0': From 5f25663fcd96ad2d332765e4cd9cbe75227a3bb4 Mon Sep 17 00:00:00 2001 From: d-dashkov Date: Tue, 9 Feb 2021 12:25:09 +0200 Subject: [PATCH 02/15] Fixed checking the correct mask Signed-off-by: d-dashkov --- config/main.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/config/main.py b/config/main.py index 21296d9f04..ab1fd838b0 100644 --- a/config/main.py +++ b/config/main.py @@ -2652,12 +2652,18 @@ def add(ctx, interface_name, ip_addr, gw): return try: + net = ipaddress.ip_network(ip_addr, strict=False) + if '/' not in ip_addr: + ip_addr = str(net) + split_ip_mask = ip_addr.split("/") - # Check if the IP address is correct or throw a ValueError exception - ipaddress.ip_address(split_ip_mask[0]) - # Check the correctness of the mask - if len(split_ip_mask) < 2 or not int(split_ip_mask[1]) in range(31): - ctx.fail("ip mask is not valid") + # Check if the IP is correct and return the IP or try to fix it or throw a ValueError exception. + ip_addr = str(ipaddress.ip_address(split_ip_mask[0])) + # Check the correctness of the mask and add it to IP. + if len(split_ip_mask) < 2 or len(split_ip_mask[1]) > 2 or not int(split_ip_mask[1]) in range(31): + ctx.fail("ip mask is not valid.") + else: + ip_addr += '/' + split_ip_mask[1] if interface_name == 'eth0': From 9e22c2c0caecc80f00e6ef035903d75a2bffcb07 Mon Sep 17 00:00:00 2001 From: d-dashkov Date: Tue, 9 Feb 2021 12:28:51 +0200 Subject: [PATCH 03/15] Added checking to config ip del Signed-off-by: d-dashkov --- config/main.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/config/main.py b/config/main.py index ab1fd838b0..f5a776bc7b 100644 --- a/config/main.py +++ b/config/main.py @@ -2726,6 +2726,15 @@ def remove(ctx, interface_name, ip_addr): if '/' not in ip_addr: ip_addr = str(net) + split_ip_mask = ip_addr.split("/") + # Check if the IP is correct and return the IP or try to fix it or throw a ValueError exception. + ip_addr = str(ipaddress.ip_address(split_ip_mask[0])) + # Check the correctness of the mask and add it to IP. + if len(split_ip_mask) < 2 or len(split_ip_mask[1]) > 2 or not int(split_ip_mask[1]) in range(31): + ctx.fail("ip mask is not valid.") + else: + ip_addr += '/' + split_ip_mask[1] + if interface_name == 'eth0': config_db.set_entry("MGMT_INTERFACE", (interface_name, ip_addr), None) mgmt_ip_restart_services() From 3f287f7e44abf7fd561d82f99c0b9e676e3b8ab3 Mon Sep 17 00:00:00 2001 From: d-dashkov Date: Tue, 9 Feb 2021 12:36:58 +0200 Subject: [PATCH 04/15] Refactored validation function Signed-off-by: d-dashkov --- config/main.py | 30 ++++++++++++++---------------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/config/main.py b/config/main.py index f5a776bc7b..7ddad92d55 100644 --- a/config/main.py +++ b/config/main.py @@ -889,6 +889,18 @@ def validate_mirror_session_config(config_db, session_name, dst_port, src_port, return True +def validate_ip_address(ctx, ip_addr): + split_ip_mask = ip_addr.split("/") + # Check if the IP is correct and return the IP or try to fix it or throw a ValueError exception. + ip_addr = str(ipaddress.ip_address(split_ip_mask[0])) + # Check the correctness of the mask and add it to IP. + if len(split_ip_mask) < 2 or len(split_ip_mask[1]) > 2 or not int(split_ip_mask[1]) in range(32): + ctx.fail("ip mask is not valid.") + else: + ip_addr += '/' + split_ip_mask[1] + + return ip_addr + def update_sonic_environment(): """Prepare sonic environment variable using SONiC environment template file. """ @@ -2656,14 +2668,7 @@ def add(ctx, interface_name, ip_addr, gw): if '/' not in ip_addr: ip_addr = str(net) - split_ip_mask = ip_addr.split("/") - # Check if the IP is correct and return the IP or try to fix it or throw a ValueError exception. - ip_addr = str(ipaddress.ip_address(split_ip_mask[0])) - # Check the correctness of the mask and add it to IP. - if len(split_ip_mask) < 2 or len(split_ip_mask[1]) > 2 or not int(split_ip_mask[1]) in range(31): - ctx.fail("ip mask is not valid.") - else: - ip_addr += '/' + split_ip_mask[1] + ip_addr = validate_ip_address(ctx, ip_addr) if interface_name == 'eth0': @@ -2726,14 +2731,7 @@ def remove(ctx, interface_name, ip_addr): if '/' not in ip_addr: ip_addr = str(net) - split_ip_mask = ip_addr.split("/") - # Check if the IP is correct and return the IP or try to fix it or throw a ValueError exception. - ip_addr = str(ipaddress.ip_address(split_ip_mask[0])) - # Check the correctness of the mask and add it to IP. - if len(split_ip_mask) < 2 or len(split_ip_mask[1]) > 2 or not int(split_ip_mask[1]) in range(31): - ctx.fail("ip mask is not valid.") - else: - ip_addr += '/' + split_ip_mask[1] + ip_addr = validate_ip_address(ctx, ip_addr) if interface_name == 'eth0': config_db.set_entry("MGMT_INTERFACE", (interface_name, ip_addr), None) From cb3cba1447a759aa23c8a03ff80174acfb7a50ee Mon Sep 17 00:00:00 2001 From: d-dashkov Date: Tue, 16 Feb 2021 11:51:41 +0200 Subject: [PATCH 05/15] Updated for IPv6 Signed-off-by: d-dashkov --- config/main.py | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/config/main.py b/config/main.py index 7ddad92d55..d028e68c2d 100644 --- a/config/main.py +++ b/config/main.py @@ -890,16 +890,23 @@ def validate_mirror_session_config(config_db, session_name, dst_port, src_port, return True def validate_ip_address(ctx, ip_addr): - split_ip_mask = ip_addr.split("/") - # Check if the IP is correct and return the IP or try to fix it or throw a ValueError exception. - ip_addr = str(ipaddress.ip_address(split_ip_mask[0])) - # Check the correctness of the mask and add it to IP. - if len(split_ip_mask) < 2 or len(split_ip_mask[1]) > 2 or not int(split_ip_mask[1]) in range(32): - ctx.fail("ip mask is not valid.") - else: - ip_addr += '/' + split_ip_mask[1] + mask_range = 32 + split_ip_mask = ip_addr.split("/") + + try: + ip_addr = str(ipaddress.IPv4Address(split_ip_mask[0])) + except ipaddress.AddressValueError: + ip_addr = str(ipaddress.IPv6Address(split_ip_mask[0])) + mask_range = 65 + + # Check the correctness of the mask and add it to IP. + split_ip_mask[1] = int(split_ip_mask[1]) + if not split_ip_mask[1] in range(mask_range): + ctx.fail("ip mask is not valid.") + else: + ip_addr += '/' + str(split_ip_mask[1]) - return ip_addr + return ip_addr def update_sonic_environment(): """Prepare sonic environment variable using SONiC environment template file. From d8d8d2e6fdf96565c03ef18bfb05634b6fcd2719 Mon Sep 17 00:00:00 2001 From: d-dashkov Date: Wed, 17 Feb 2021 00:59:19 +0200 Subject: [PATCH 06/15] Replaced exeption Signed-off-by: d-dashkov --- config/main.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/config/main.py b/config/main.py index d028e68c2d..42e9d8f796 100644 --- a/config/main.py +++ b/config/main.py @@ -893,11 +893,10 @@ def validate_ip_address(ctx, ip_addr): mask_range = 32 split_ip_mask = ip_addr.split("/") - try: - ip_addr = str(ipaddress.IPv4Address(split_ip_mask[0])) - except ipaddress.AddressValueError: - ip_addr = str(ipaddress.IPv6Address(split_ip_mask[0])) + ip_obj = ipaddress.ip_address(split_ip_mask[0]) + if type(ip_obj) is ipaddress.IPv6Address: mask_range = 65 + ip_addr = str(ip_obj) # Check the correctness of the mask and add it to IP. split_ip_mask[1] = int(split_ip_mask[1]) From 9361fb6510f979980e1d13a8f49520a719dfa61a Mon Sep 17 00:00:00 2001 From: d-dashkov Date: Tue, 23 Feb 2021 00:04:35 +0200 Subject: [PATCH 07/15] Added UT for ip config Signed-off-by: d-dashkov --- tests/ip_config_test.py | 154 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 154 insertions(+) create mode 100644 tests/ip_config_test.py diff --git a/tests/ip_config_test.py b/tests/ip_config_test.py new file mode 100644 index 0000000000..8ab60c8dc6 --- /dev/null +++ b/tests/ip_config_test.py @@ -0,0 +1,154 @@ +import os +import traceback +from unittest import mock + +from click.testing import CliRunner + +import config.main as config +import show.main as show +from utilities_common.db import Db + + +class TestConfigIP(object): + @classmethod + def setup_class(cls): + os.environ['UTILITIES_UNIT_TESTING'] = "1" + print("SETUP") + + ''' Tests for IPv4 ''' + + def test_add_del_interface_valid_ipv4(self): + db = Db() + runner = CliRunner() + obj = {'config_db':db.cfgdb} + + # config int ip add Ethernet64 10.10.10.1/24 + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet64", "10.10.10.1/24"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code == 0 + assert ('Ethernet64', '10.10.10.1/24') in db.cfgdb.get_table('INTERFACE') + + # config int ip remove Ethernet64 10.10.10.1/24 + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet64", "10.10.10.1/24"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code != 0 + assert ('Ethernet64', '10.10.10.1/24') not in db.cfgdb.get_table('INTERFACE') + + def test_add_interface_invalid_ipv4(self): + db = Db() + runner = CliRunner() + obj = {'config_db':db.cfgdb} + + # config int ip add Ethernet64 10000.10.10.1/24 + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet64", "10000.10.10.1/24"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code != 0 + assert "Error: 'ip_addr' is not valid." in result.output + + def test_add_interface_ipv4_invalid_mask(self): + db = Db() + runner = CliRunner() + obj = {'config_db':db.cfgdb} + + # config int ip add Ethernet64 10.10.10.1 + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet64", "10.10.10.1"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code != 0 + assert "Error: ip mask is not valid." in result.output + + def test_add_del_interface_ipv4_with_leading_zeros(self): + db = Db() + runner = CliRunner() + obj = {'config_db':db.cfgdb} + + # config int ip add Ethernet68 10.10.10.002/24 + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet68", "10.10.10.002/24"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code == 0 + assert ('Ethernet68', '10.10.10.2/24') in db.cfgdb.get_table('INTERFACE') + + # config int ip remove Ethernet68 10.10.10.001/24 + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet68", "10.10.10.002/24"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code != 0 + assert ('Ethernet68', '10.10.10.2/24') not in db.cfgdb.get_table('INTERFACE') + + ''' Tests for IPv6 ''' + + def test_add_del_interface_valid_ipv6(self): + db = Db() + runner = CliRunner() + obj = {'config_db':db.cfgdb} + + # config int ip add Ethernet72 2001:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34 + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet72", "2001:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code == 0 + assert ('Ethernet72', '2001:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34') in db.cfgdb.get_table('INTERFACE') + + # config int ip remove Ethernet72 2001:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34 + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet72", "2001:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code != 0 + assert ('Ethernet72', '2001:1db8:11a3:19d7:1f34:8a2e:17a0:765d/34') not in db.cfgdb.get_table('INTERFACE') + + def test_add_interface_invalid_ipv6(self): + db = Db() + runner = CliRunner() + obj = {'config_db':db.cfgdb} + + # config int ip add Ethernet72 20001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/34 + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet72", "20001:0db8:11a3:19d7:1f34:8a2e:17a0:765d/34"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code != 0 + assert "Error: 'ip_addr' is not valid." in result.output + + def test_add_interface_ipv6_invalid_mask(self): + db = Db() + runner = CliRunner() + obj = {'config_db':db.cfgdb} + + # config int ip add Ethernet72 2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet72", "2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code != 0 + assert "Error: ip mask is not valid." in result.output + + def test_add_del_interface_ipv6_with_leading_zeros(self): + db = Db() + runner = CliRunner() + obj = {'config_db':db.cfgdb} + + # config int ip del Ethernet68 2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/34 + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet68", "2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/34"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code == 0 + assert ('Ethernet68', '2001:db8:11a3:9d7:1f34:8a2e:7a0:765d/34') in db.cfgdb.get_table('INTERFACE') + + # config int ip remove Ethernet68 2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/34 + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet68", "2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/34"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code != 0 + assert ('Ethernet68', '2001:db8:11a3:9d7:1f34:8a2e:7a0:765d/34') not in db.cfgdb.get_table('INTERFACE') + + def test_add_del_interface_shortened_ipv6_with_leading_zeros(self): + db = Db() + runner = CliRunner() + obj = {'config_db':db.cfgdb} + + # config int ip del Ethernet68 3000::001/64 + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet68", "3000::001/64"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code == 0 + assert ('Ethernet68', '3000::1/64') in db.cfgdb.get_table('INTERFACE') + + # config int ip remove Ethernet68 3000::001/64 + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet68", "3000::001/64"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code != 0 + assert ('Ethernet68', '3000::1/64') not in db.cfgdb.get_table('INTERFACE') + + @classmethod + def teardown_class(cls): + os.environ['UTILITIES_UNIT_TESTING'] = "0" + print("TEARDOWN") \ No newline at end of file From 98d7843fa6ad151a65811400d731cd8e6e5e5fac Mon Sep 17 00:00:00 2001 From: d-dashkov Date: Mon, 22 Mar 2021 16:57:42 +0200 Subject: [PATCH 08/15] Refactored ip functions Signed-off-by: d-dashkov --- config/main.py | 147 ++++++++++++++++++++++--------------------------- 1 file changed, 66 insertions(+), 81 deletions(-) diff --git a/config/main.py b/config/main.py index 42e9d8f796..135c1c7a8e 100644 --- a/config/main.py +++ b/config/main.py @@ -890,22 +890,21 @@ def validate_mirror_session_config(config_db, session_name, dst_port, src_port, return True def validate_ip_address(ctx, ip_addr): - mask_range = 32 - split_ip_mask = ip_addr.split("/") - - ip_obj = ipaddress.ip_address(split_ip_mask[0]) - if type(ip_obj) is ipaddress.IPv6Address: - mask_range = 65 - ip_addr = str(ip_obj) - - # Check the correctness of the mask and add it to IP. - split_ip_mask[1] = int(split_ip_mask[1]) - if not split_ip_mask[1] in range(mask_range): - ctx.fail("ip mask is not valid.") - else: - ip_addr += '/' + str(split_ip_mask[1]) + try: + split_ip_mask = ip_addr.split("/") + + # Checking if the IP address is correct. + ip_obj = ipaddress.ip_address(split_ip_mask[0]) - return ip_addr + # Checking if the mask is correct + split_ip_mask[1] = int(split_ip_mask[1]) + if not split_ip_mask[1] in range(33 if isinstance(ip_obj, ipaddress.IPv4Address) else 65): + ctx.fail("ip mask is not valid.") + + return str(ip_obj) + '/' + str(split_ip_mask[1]) + + except ValueError: + ctx.fail("ip address is not valid.") def update_sonic_environment(): """Prepare sonic environment variable using SONiC environment template file. @@ -2669,50 +2668,43 @@ def add(ctx, interface_name, ip_addr, gw): click.echo("Interface {} is a member of vlan\nAborting!".format(interface_name)) return - try: - net = ipaddress.ip_network(ip_addr, strict=False) - if '/' not in ip_addr: - ip_addr = str(net) - - ip_addr = validate_ip_address(ctx, ip_addr) - - if interface_name == 'eth0': - - # Configuring more than 1 IPv4 or more than 1 IPv6 address fails. - # Allow only one IPv4 and only one IPv6 address to be configured for IPv6. - # If a row already exist, overwrite it (by doing delete and add). - mgmtintf_key_list = _get_all_mgmtinterface_keys() - - for key in mgmtintf_key_list: - # For loop runs for max 2 rows, once for IPv4 and once for IPv6. - # No need to capture the exception since the ip_addr is already validated earlier - ip_input = ipaddress.ip_interface(ip_addr) - current_ip = ipaddress.ip_interface(key[1]) - if (ip_input.version == current_ip.version): - # If user has configured IPv4/v6 address and the already available row is also IPv4/v6, delete it here. - config_db.set_entry("MGMT_INTERFACE", ("eth0", key[1]), None) - - # Set the new row with new value - if not gw: - config_db.set_entry("MGMT_INTERFACE", (interface_name, ip_addr), {"NULL": "NULL"}) - else: - config_db.set_entry("MGMT_INTERFACE", (interface_name, ip_addr), {"gwaddr": gw}) - mgmt_ip_restart_services() + ip_addr = validate_ip_address(ctx, ip_addr) - return + if interface_name == 'eth0': - table_name = get_interface_table_name(interface_name) - if table_name == "": - ctx.fail("'interface_name' is not valid. Valid names [Ethernet/PortChannel/Vlan/Loopback]") - interface_entry = config_db.get_entry(table_name, interface_name) - if len(interface_entry) == 0: - if table_name == "VLAN_SUB_INTERFACE": - config_db.set_entry(table_name, interface_name, {"admin_status": "up"}) - else: - config_db.set_entry(table_name, interface_name, {"NULL": "NULL"}) - config_db.set_entry(table_name, (interface_name, ip_addr), {"NULL": "NULL"}) - except ValueError: - ctx.fail("'ip_addr' is not valid.") + # Configuring more than 1 IPv4 or more than 1 IPv6 address fails. + # Allow only one IPv4 and only one IPv6 address to be configured for IPv6. + # If a row already exist, overwrite it (by doing delete and add). + mgmtintf_key_list = _get_all_mgmtinterface_keys() + + for key in mgmtintf_key_list: + # For loop runs for max 2 rows, once for IPv4 and once for IPv6. + # No need to capture the exception since the ip_addr is already validated earlier + ip_input = ipaddress.ip_interface(ip_addr) + current_ip = ipaddress.ip_interface(key[1]) + if (ip_input.version == current_ip.version): + # If user has configured IPv4/v6 address and the already available row is also IPv4/v6, delete it here. + config_db.set_entry("MGMT_INTERFACE", ("eth0", key[1]), None) + + # Set the new row with new value + if not gw: + config_db.set_entry("MGMT_INTERFACE", (interface_name, ip_addr), {"NULL": "NULL"}) + else: + config_db.set_entry("MGMT_INTERFACE", (interface_name, ip_addr), {"gwaddr": gw}) + mgmt_ip_restart_services() + + return + + table_name = get_interface_table_name(interface_name) + if table_name == "": + ctx.fail("'interface_name' is not valid. Valid names [Ethernet/PortChannel/Vlan/Loopback]") + interface_entry = config_db.get_entry(table_name, interface_name) + if len(interface_entry) == 0: + if table_name == "VLAN_SUB_INTERFACE": + config_db.set_entry(table_name, interface_name, {"admin_status": "up"}) + else: + config_db.set_entry(table_name, interface_name, {"NULL": "NULL"}) + config_db.set_entry(table_name, (interface_name, ip_addr), {"NULL": "NULL"}) # # 'del' subcommand @@ -2732,33 +2724,26 @@ def remove(ctx, interface_name, ip_addr): if interface_name is None: ctx.fail("'interface_name' is None!") - try: - net = ipaddress.ip_network(ip_addr, strict=False) - if '/' not in ip_addr: - ip_addr = str(net) + ip_addr = validate_ip_address(ctx, ip_addr) - ip_addr = validate_ip_address(ctx, ip_addr) - - if interface_name == 'eth0': - config_db.set_entry("MGMT_INTERFACE", (interface_name, ip_addr), None) - mgmt_ip_restart_services() - return + if interface_name == 'eth0': + config_db.set_entry("MGMT_INTERFACE", (interface_name, ip_addr), None) + mgmt_ip_restart_services() + return - table_name = get_interface_table_name(interface_name) - if table_name == "": - ctx.fail("'interface_name' is not valid. Valid names [Ethernet/PortChannel/Vlan/Loopback]") - config_db.set_entry(table_name, (interface_name, ip_addr), None) - interface_dependent = interface_ipaddr_dependent_on_interface(config_db, interface_name) - if len(interface_dependent) == 0 and is_interface_bind_to_vrf(config_db, interface_name) is False: - config_db.set_entry(table_name, interface_name, None) + table_name = get_interface_table_name(interface_name) + if table_name == "": + ctx.fail("'interface_name' is not valid. Valid names [Ethernet/PortChannel/Vlan/Loopback]") + config_db.set_entry(table_name, (interface_name, ip_addr), None) + interface_dependent = interface_ipaddr_dependent_on_interface(config_db, interface_name) + if len(interface_dependent) == 0 and is_interface_bind_to_vrf(config_db, interface_name) is False: + config_db.set_entry(table_name, interface_name, None) - if multi_asic.is_multi_asic(): - command = "sudo ip netns exec {} ip neigh flush dev {} {}".format(ctx.obj['namespace'], interface_name, ip_addr) - else: - command = "ip neigh flush dev {} {}".format(interface_name, ip_addr) - clicommon.run_command(command) - except ValueError: - ctx.fail("'ip_addr' is not valid.") + if multi_asic.is_multi_asic(): + command = "sudo ip netns exec {} ip neigh flush dev {} {}".format(ctx.obj['namespace'], interface_name, ip_addr) + else: + command = "ip neigh flush dev {} {}".format(interface_name, ip_addr) + clicommon.run_command(command) # From b9d0d4809939ce38306ec42c035082b315ac8648 Mon Sep 17 00:00:00 2001 From: d-dashkov Date: Mon, 22 Mar 2021 19:40:53 +0200 Subject: [PATCH 09/15] Updated mask validation and UT Signed-off-by: d-dashkov --- config/main.py | 20 +++++++++----------- tests/ip_config_test.py | 14 +++++++------- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/config/main.py b/config/main.py index 135c1c7a8e..5afb904462 100644 --- a/config/main.py +++ b/config/main.py @@ -890,22 +890,20 @@ def validate_mirror_session_config(config_db, session_name, dst_port, src_port, return True def validate_ip_address(ctx, ip_addr): + split_ip_mask = ip_addr.split("/") + # Checking if the IP address is correct. try: - split_ip_mask = ip_addr.split("/") - - # Checking if the IP address is correct. ip_obj = ipaddress.ip_address(split_ip_mask[0]) - - # Checking if the mask is correct - split_ip_mask[1] = int(split_ip_mask[1]) - if not split_ip_mask[1] in range(33 if isinstance(ip_obj, ipaddress.IPv4Address) else 65): - ctx.fail("ip mask is not valid.") - - return str(ip_obj) + '/' + str(split_ip_mask[1]) - except ValueError: ctx.fail("ip address is not valid.") + # Checking if the mask is correct + mask_range = 33 if isinstance(ip_obj, ipaddress.IPv4Address) else 65 + if len(split_ip_mask) != 2 or not int(split_ip_mask[1]) in range(1, mask_range): + ctx.fail("ip mask is not valid.") + + return str(ip_obj) + '/' + str(int(split_ip_mask[1])) + def update_sonic_environment(): """Prepare sonic environment variable using SONiC environment template file. """ diff --git a/tests/ip_config_test.py b/tests/ip_config_test.py index 8ab60c8dc6..7af6e64fb0 100644 --- a/tests/ip_config_test.py +++ b/tests/ip_config_test.py @@ -40,18 +40,18 @@ def test_add_interface_invalid_ipv4(self): obj = {'config_db':db.cfgdb} # config int ip add Ethernet64 10000.10.10.1/24 - result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet64", "10000.10.10.1/24"], obj=obj) + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet64", "10000.10.10.1/24"], obj=obj) print(result.exit_code, result.output) assert result.exit_code != 0 - assert "Error: 'ip_addr' is not valid." in result.output + assert "Error: ip address is not valid." in result.output def test_add_interface_ipv4_invalid_mask(self): db = Db() runner = CliRunner() obj = {'config_db':db.cfgdb} - # config int ip add Ethernet64 10.10.10.1 - result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet64", "10.10.10.1"], obj=obj) + # config int ip add Ethernet64 10.10.10.1/37 + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet64", "10.10.10.1/37"], obj=obj) print(result.exit_code, result.output) assert result.exit_code != 0 assert "Error: ip mask is not valid." in result.output @@ -101,15 +101,15 @@ def test_add_interface_invalid_ipv6(self): result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet72", "20001:0db8:11a3:19d7:1f34:8a2e:17a0:765d/34"], obj=obj) print(result.exit_code, result.output) assert result.exit_code != 0 - assert "Error: 'ip_addr' is not valid." in result.output + assert "Error: ip address is not valid." in result.output def test_add_interface_ipv6_invalid_mask(self): db = Db() runner = CliRunner() obj = {'config_db':db.cfgdb} - # config int ip add Ethernet72 2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d - result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet72", "2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d"], obj=obj) + # config int ip add Ethernet72 2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/67 + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet72", "2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/67"], obj=obj) print(result.exit_code, result.output) assert result.exit_code != 0 assert "Error: ip mask is not valid." in result.output From 2aa389f53c7c6e6ce5ac95e6c76ef5056f2f2cfa Mon Sep 17 00:00:00 2001 From: d-dashkov Date: Mon, 22 Mar 2021 20:33:29 +0200 Subject: [PATCH 10/15] Made function generic Signed-off-by: d-dashkov --- config/main.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/config/main.py b/config/main.py index 5afb904462..86a38b2992 100644 --- a/config/main.py +++ b/config/main.py @@ -895,12 +895,14 @@ def validate_ip_address(ctx, ip_addr): try: ip_obj = ipaddress.ip_address(split_ip_mask[0]) except ValueError: - ctx.fail("ip address is not valid.") + click.echo("Error: ip address is not valid.") + return False # Checking if the mask is correct mask_range = 33 if isinstance(ip_obj, ipaddress.IPv4Address) else 65 if len(split_ip_mask) != 2 or not int(split_ip_mask[1]) in range(1, mask_range): - ctx.fail("ip mask is not valid.") + click.echo("Error: ip mask is not valid.") + return False return str(ip_obj) + '/' + str(int(split_ip_mask[1])) @@ -2667,6 +2669,8 @@ def add(ctx, interface_name, ip_addr, gw): return ip_addr = validate_ip_address(ctx, ip_addr) + if ip_addr is False: + return if interface_name == 'eth0': @@ -2723,6 +2727,8 @@ def remove(ctx, interface_name, ip_addr): ctx.fail("'interface_name' is None!") ip_addr = validate_ip_address(ctx, ip_addr) + if ip_addr is False: + return if interface_name == 'eth0': config_db.set_entry("MGMT_INTERFACE", (interface_name, ip_addr), None) From 0c39c0fb58f567e077410bd521d54fd6c7371211 Mon Sep 17 00:00:00 2001 From: d-dashkov Date: Mon, 22 Mar 2021 20:46:50 +0200 Subject: [PATCH 11/15] Fixed UT Signed-off-by: d-dashkov --- tests/ip_config_test.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/ip_config_test.py b/tests/ip_config_test.py index 7af6e64fb0..5489360170 100644 --- a/tests/ip_config_test.py +++ b/tests/ip_config_test.py @@ -42,7 +42,7 @@ def test_add_interface_invalid_ipv4(self): # config int ip add Ethernet64 10000.10.10.1/24 result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet64", "10000.10.10.1/24"], obj=obj) print(result.exit_code, result.output) - assert result.exit_code != 0 + assert result.exit_code == 0 assert "Error: ip address is not valid." in result.output def test_add_interface_ipv4_invalid_mask(self): @@ -53,7 +53,7 @@ def test_add_interface_ipv4_invalid_mask(self): # config int ip add Ethernet64 10.10.10.1/37 result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet64", "10.10.10.1/37"], obj=obj) print(result.exit_code, result.output) - assert result.exit_code != 0 + assert result.exit_code == 0 assert "Error: ip mask is not valid." in result.output def test_add_del_interface_ipv4_with_leading_zeros(self): @@ -100,7 +100,7 @@ def test_add_interface_invalid_ipv6(self): # config int ip add Ethernet72 20001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/34 result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet72", "20001:0db8:11a3:19d7:1f34:8a2e:17a0:765d/34"], obj=obj) print(result.exit_code, result.output) - assert result.exit_code != 0 + assert result.exit_code == 0 assert "Error: ip address is not valid." in result.output def test_add_interface_ipv6_invalid_mask(self): @@ -111,7 +111,7 @@ def test_add_interface_ipv6_invalid_mask(self): # config int ip add Ethernet72 2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/67 result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet72", "2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/67"], obj=obj) print(result.exit_code, result.output) - assert result.exit_code != 0 + assert result.exit_code == 0 assert "Error: ip mask is not valid." in result.output def test_add_del_interface_ipv6_with_leading_zeros(self): @@ -120,13 +120,13 @@ def test_add_del_interface_ipv6_with_leading_zeros(self): obj = {'config_db':db.cfgdb} # config int ip del Ethernet68 2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/34 - result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet68", "2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/34"], obj=obj) + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet68", "2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/34"], obj=obj) print(result.exit_code, result.output) assert result.exit_code == 0 assert ('Ethernet68', '2001:db8:11a3:9d7:1f34:8a2e:7a0:765d/34') in db.cfgdb.get_table('INTERFACE') # config int ip remove Ethernet68 2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/34 - result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet68", "2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/34"], obj=obj) + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet68", "2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/34"], obj=obj) print(result.exit_code, result.output) assert result.exit_code != 0 assert ('Ethernet68', '2001:db8:11a3:9d7:1f34:8a2e:7a0:765d/34') not in db.cfgdb.get_table('INTERFACE') From 3d1abffdc23de7bff6be3fc445587a853abcd9e6 Mon Sep 17 00:00:00 2001 From: d-dashkov Date: Thu, 25 Mar 2021 10:57:07 +0200 Subject: [PATCH 12/15] Fixed ipv6 mask Signed-off-by: d-dashkov --- config/main.py | 25 +++++++++++++++---------- tests/ip_config_test.py | 4 ++-- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/config/main.py b/config/main.py index 86a38b2992..61fbf55b8b 100644 --- a/config/main.py +++ b/config/main.py @@ -890,22 +890,27 @@ def validate_mirror_session_config(config_db, session_name, dst_port, src_port, return True def validate_ip_address(ctx, ip_addr): - split_ip_mask = ip_addr.split("/") - # Checking if the IP address is correct. try: + split_ip_mask = ip_addr.split("/") + # Checking if the IP address is correct. ip_obj = ipaddress.ip_address(split_ip_mask[0]) + + # Checking if the mask is correct + mask_range = 33 if isinstance(ip_obj, ipaddress.IPv4Address) else 129 + # If mask is not specified, set to default value + if len(split_ip_mask) < 2: + split_ip_mask.append(32 if isinstance(ip_obj, ipaddress.IPv4Address) else 128) + + if not int(split_ip_mask[1]) in range(1, mask_range): + click.echo("Error: ip mask is not valid.") + return False + + return str(ip_obj) + '/' + str(int(split_ip_mask[1])) + except ValueError: click.echo("Error: ip address is not valid.") return False - # Checking if the mask is correct - mask_range = 33 if isinstance(ip_obj, ipaddress.IPv4Address) else 65 - if len(split_ip_mask) != 2 or not int(split_ip_mask[1]) in range(1, mask_range): - click.echo("Error: ip mask is not valid.") - return False - - return str(ip_obj) + '/' + str(int(split_ip_mask[1])) - def update_sonic_environment(): """Prepare sonic environment variable using SONiC environment template file. """ diff --git a/tests/ip_config_test.py b/tests/ip_config_test.py index 5489360170..3fe5dd3e35 100644 --- a/tests/ip_config_test.py +++ b/tests/ip_config_test.py @@ -108,8 +108,8 @@ def test_add_interface_ipv6_invalid_mask(self): runner = CliRunner() obj = {'config_db':db.cfgdb} - # config int ip add Ethernet72 2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/67 - result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet72", "2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/67"], obj=obj) + # config int ip add Ethernet72 2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/200 + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet72", "2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/200"], obj=obj) print(result.exit_code, result.output) assert result.exit_code == 0 assert "Error: ip mask is not valid." in result.output From 72f369611777dd9a3ce86853a0fe9c6558820910 Mon Sep 17 00:00:00 2001 From: d-dashkov Date: Wed, 26 May 2021 17:24:57 +0300 Subject: [PATCH 13/15] Rename function Signed-off-by: d-dashkov --- config/main.py | 124 ++++++++++++++++++++-------------------- tests/ip_config_test.py | 2 +- 2 files changed, 63 insertions(+), 63 deletions(-) diff --git a/config/main.py b/config/main.py index 82c2a90c7b..bed39319e6 100644 --- a/config/main.py +++ b/config/main.py @@ -777,8 +777,7 @@ def validate_mirror_session_config(config_db, session_name, dst_port, src_port, return True -<<<<<<< HEAD -def validate_ip_address(ctx, ip_addr): +def validate_ip_mask(ctx, ip_addr): try: split_ip_mask = ip_addr.split("/") # Checking if the IP address is correct. @@ -799,7 +798,7 @@ def validate_ip_address(ctx, ip_addr): except ValueError: click.echo("Error: ip address is not valid.") return False -======= + def cli_sroute_to_config(ctx, command_str, strict_nh = True): if len(command_str) < 2 or len(command_str) > 9: ctx.fail("argument is not in pattern prefix [vrf ] nexthop <[vrf ] >|>!") @@ -859,7 +858,6 @@ def cli_sroute_to_config(ctx, command_str, strict_nh = True): key = ip_prefix return key, config_entry ->>>>>>> source/master def update_sonic_environment(): """Prepare sonic environment variable using SONiC environment template file. @@ -3453,45 +3451,51 @@ def add(ctx, interface_name, ip_addr, gw): click.echo("Interface {} is a member of vlan\nAborting!".format(interface_name)) return - ip_addr = validate_ip_address(ctx, ip_addr) - if ip_addr is False: - return + try: + net = ipaddress.ip_network(ip_addr, strict=False) + if '/' not in ip_addr: + ip_addr = str(net) - if interface_name == 'eth0': - - # Configuring more than 1 IPv4 or more than 1 IPv6 address fails. - # Allow only one IPv4 and only one IPv6 address to be configured for IPv6. - # If a row already exist, overwrite it (by doing delete and add). - mgmtintf_key_list = _get_all_mgmtinterface_keys() - - for key in mgmtintf_key_list: - # For loop runs for max 2 rows, once for IPv4 and once for IPv6. - # No need to capture the exception since the ip_addr is already validated earlier - ip_input = ipaddress.ip_interface(ip_addr) - current_ip = ipaddress.ip_interface(key[1]) - if (ip_input.version == current_ip.version): - # If user has configured IPv4/v6 address and the already available row is also IPv4/v6, delete it here. - config_db.set_entry("MGMT_INTERFACE", ("eth0", key[1]), None) - - # Set the new row with new value - if not gw: - config_db.set_entry("MGMT_INTERFACE", (interface_name, ip_addr), {"NULL": "NULL"}) - else: - config_db.set_entry("MGMT_INTERFACE", (interface_name, ip_addr), {"gwaddr": gw}) - mgmt_ip_restart_services() + if not validate_ip_mask(ctx, ip_addr): + return - return + if interface_name == 'eth0': + + # Configuring more than 1 IPv4 or more than 1 IPv6 address fails. + # Allow only one IPv4 and only one IPv6 address to be configured for IPv6. + # If a row already exist, overwrite it (by doing delete and add). + mgmtintf_key_list = _get_all_mgmtinterface_keys() + + for key in mgmtintf_key_list: + # For loop runs for max 2 rows, once for IPv4 and once for IPv6. + # No need to capture the exception since the ip_addr is already validated earlier + ip_input = ipaddress.ip_interface(ip_addr) + current_ip = ipaddress.ip_interface(key[1]) + if (ip_input.version == current_ip.version): + # If user has configured IPv4/v6 address and the already available row is also IPv4/v6, delete it here. + config_db.set_entry("MGMT_INTERFACE", ("eth0", key[1]), None) + + # Set the new row with new value + if not gw: + config_db.set_entry("MGMT_INTERFACE", (interface_name, ip_addr), {"NULL": "NULL"}) + else: + config_db.set_entry("MGMT_INTERFACE", (interface_name, ip_addr), {"gwaddr": gw}) + mgmt_ip_restart_services() - table_name = get_interface_table_name(interface_name) - if table_name == "": - ctx.fail("'interface_name' is not valid. Valid names [Ethernet/PortChannel/Vlan/Loopback]") - interface_entry = config_db.get_entry(table_name, interface_name) - if len(interface_entry) == 0: - if table_name == "VLAN_SUB_INTERFACE": - config_db.set_entry(table_name, interface_name, {"admin_status": "up"}) - else: - config_db.set_entry(table_name, interface_name, {"NULL": "NULL"}) - config_db.set_entry(table_name, (interface_name, ip_addr), {"NULL": "NULL"}) + return + + table_name = get_interface_table_name(interface_name) + if table_name == "": + ctx.fail("'interface_name' is not valid. Valid names [Ethernet/PortChannel/Vlan/Loopback]") + interface_entry = config_db.get_entry(table_name, interface_name) + if len(interface_entry) == 0: + if table_name == "VLAN_SUB_INTERFACE": + config_db.set_entry(table_name, interface_name, {"admin_status": "up"}) + else: + config_db.set_entry(table_name, interface_name, {"NULL": "NULL"}) + config_db.set_entry(table_name, (interface_name, ip_addr), {"NULL": "NULL"}) + except ValueError: + ctx.fail("'ip_addr' is not valid.") # # 'del' subcommand @@ -3511,24 +3515,19 @@ def remove(ctx, interface_name, ip_addr): if interface_name is None: ctx.fail("'interface_name' is None!") - ip_addr = validate_ip_address(ctx, ip_addr) - if ip_addr is False: - return + try: + net = ipaddress.ip_network(ip_addr, strict=False) + if '/' not in ip_addr: + ip_addr = str(net) + + if not validate_ip_mask(ctx, ip_addr): + return - if interface_name == 'eth0': - config_db.set_entry("MGMT_INTERFACE", (interface_name, ip_addr), None) - mgmt_ip_restart_services() - return + if interface_name == 'eth0': + config_db.set_entry("MGMT_INTERFACE", (interface_name, ip_addr), None) + mgmt_ip_restart_services() + return -<<<<<<< HEAD - table_name = get_interface_table_name(interface_name) - if table_name == "": - ctx.fail("'interface_name' is not valid. Valid names [Ethernet/PortChannel/Vlan/Loopback]") - config_db.set_entry(table_name, (interface_name, ip_addr), None) - interface_dependent = interface_ipaddr_dependent_on_interface(config_db, interface_name) - if len(interface_dependent) == 0 and is_interface_bind_to_vrf(config_db, interface_name) is False: - config_db.set_entry(table_name, interface_name, None) -======= table_name = get_interface_table_name(interface_name) if table_name == "": ctx.fail("'interface_name' is not valid. Valid names [Ethernet/PortChannel/Vlan/Loopback]") @@ -3555,13 +3554,14 @@ def remove(ctx, interface_name, ip_addr): interface_dependent = interface_ipaddr_dependent_on_interface(config_db, interface_name) if len(interface_dependent) == 0 and is_interface_bind_to_vrf(config_db, interface_name) is False: config_db.set_entry(table_name, interface_name, None) ->>>>>>> source/master - if multi_asic.is_multi_asic(): - command = "sudo ip netns exec {} ip neigh flush dev {} {}".format(ctx.obj['namespace'], interface_name, ip_addr) - else: - command = "ip neigh flush dev {} {}".format(interface_name, ip_addr) - clicommon.run_command(command) + if multi_asic.is_multi_asic(): + command = "sudo ip netns exec {} ip neigh flush dev {} {}".format(ctx.obj['namespace'], interface_name, ip_addr) + else: + command = "ip neigh flush dev {} {}".format(interface_name, ip_addr) + clicommon.run_command(command) + except ValueError: + ctx.fail("'ip_addr' is not valid.") # diff --git a/tests/ip_config_test.py b/tests/ip_config_test.py index 3fe5dd3e35..57f382bdcf 100644 --- a/tests/ip_config_test.py +++ b/tests/ip_config_test.py @@ -151,4 +151,4 @@ def test_add_del_interface_shortened_ipv6_with_leading_zeros(self): @classmethod def teardown_class(cls): os.environ['UTILITIES_UNIT_TESTING'] = "0" - print("TEARDOWN") \ No newline at end of file + print("TEARDOWN") From 31cc683540e5dd0e9c10e7a12df53eb781020a47 Mon Sep 17 00:00:00 2001 From: d-dashkov Date: Wed, 26 May 2021 18:14:28 +0300 Subject: [PATCH 14/15] Fix tests Signed-off-by: d-dashkov --- tests/ip_config_test.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/ip_config_test.py b/tests/ip_config_test.py index 57f382bdcf..eb81d3bdd2 100644 --- a/tests/ip_config_test.py +++ b/tests/ip_config_test.py @@ -42,7 +42,7 @@ def test_add_interface_invalid_ipv4(self): # config int ip add Ethernet64 10000.10.10.1/24 result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet64", "10000.10.10.1/24"], obj=obj) print(result.exit_code, result.output) - assert result.exit_code == 0 + assert result.exit_code != 0 assert "Error: ip address is not valid." in result.output def test_add_interface_ipv4_invalid_mask(self): @@ -53,7 +53,7 @@ def test_add_interface_ipv4_invalid_mask(self): # config int ip add Ethernet64 10.10.10.1/37 result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet64", "10.10.10.1/37"], obj=obj) print(result.exit_code, result.output) - assert result.exit_code == 0 + assert result.exit_code != 0 assert "Error: ip mask is not valid." in result.output def test_add_del_interface_ipv4_with_leading_zeros(self): @@ -67,7 +67,7 @@ def test_add_del_interface_ipv4_with_leading_zeros(self): assert result.exit_code == 0 assert ('Ethernet68', '10.10.10.2/24') in db.cfgdb.get_table('INTERFACE') - # config int ip remove Ethernet68 10.10.10.001/24 + # config int ip remove Ethernet68 10.10.10.002/24 result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["remove"], ["Ethernet68", "10.10.10.002/24"], obj=obj) print(result.exit_code, result.output) assert result.exit_code != 0 @@ -100,7 +100,7 @@ def test_add_interface_invalid_ipv6(self): # config int ip add Ethernet72 20001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/34 result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet72", "20001:0db8:11a3:19d7:1f34:8a2e:17a0:765d/34"], obj=obj) print(result.exit_code, result.output) - assert result.exit_code == 0 + assert result.exit_code != 0 assert "Error: ip address is not valid." in result.output def test_add_interface_ipv6_invalid_mask(self): From 78f5c13d27f494e4facc798d8eee322e998071e7 Mon Sep 17 00:00:00 2001 From: d-dashkov Date: Wed, 26 May 2021 19:56:59 +0300 Subject: [PATCH 15/15] Update logic and tests Signed-off-by: d-dashkov --- config/main.py | 46 +++++++++++++++++++---------------------- tests/ip_config_test.py | 15 ++++++++------ 2 files changed, 30 insertions(+), 31 deletions(-) diff --git a/config/main.py b/config/main.py index bed39319e6..9420b4990e 100644 --- a/config/main.py +++ b/config/main.py @@ -778,26 +778,20 @@ def validate_mirror_session_config(config_db, session_name, dst_port, src_port, return True def validate_ip_mask(ctx, ip_addr): - try: - split_ip_mask = ip_addr.split("/") - # Checking if the IP address is correct. - ip_obj = ipaddress.ip_address(split_ip_mask[0]) - - # Checking if the mask is correct - mask_range = 33 if isinstance(ip_obj, ipaddress.IPv4Address) else 129 - # If mask is not specified, set to default value - if len(split_ip_mask) < 2: - split_ip_mask.append(32 if isinstance(ip_obj, ipaddress.IPv4Address) else 128) - - if not int(split_ip_mask[1]) in range(1, mask_range): - click.echo("Error: ip mask is not valid.") - return False + split_ip_mask = ip_addr.split("/") + # Check if the IP address is correct or if there are leading zeros. + ip_obj = ipaddress.ip_address(split_ip_mask[0]) - return str(ip_obj) + '/' + str(int(split_ip_mask[1])) + # Check if the mask is correct + mask_range = 33 if isinstance(ip_obj, ipaddress.IPv4Address) else 129 + # If mask is not specified + if len(split_ip_mask) < 2: + return 0 - except ValueError: - click.echo("Error: ip address is not valid.") - return False + if not int(split_ip_mask[1]) in range(1, mask_range): + return 0 + + return str(ip_obj) + '/' + str(int(split_ip_mask[1])) def cli_sroute_to_config(ctx, command_str, strict_nh = True): if len(command_str) < 2 or len(command_str) > 9: @@ -3456,8 +3450,9 @@ def add(ctx, interface_name, ip_addr, gw): if '/' not in ip_addr: ip_addr = str(net) - if not validate_ip_mask(ctx, ip_addr): - return + ip_addr = validate_ip_mask(ctx, ip_addr) + if not ip_addr: + raise ValueError('') if interface_name == 'eth0': @@ -3495,7 +3490,7 @@ def add(ctx, interface_name, ip_addr, gw): config_db.set_entry(table_name, interface_name, {"NULL": "NULL"}) config_db.set_entry(table_name, (interface_name, ip_addr), {"NULL": "NULL"}) except ValueError: - ctx.fail("'ip_addr' is not valid.") + ctx.fail("ip address or mask is not valid.") # # 'del' subcommand @@ -3519,9 +3514,10 @@ def remove(ctx, interface_name, ip_addr): net = ipaddress.ip_network(ip_addr, strict=False) if '/' not in ip_addr: ip_addr = str(net) - - if not validate_ip_mask(ctx, ip_addr): - return + + ip_addr = validate_ip_mask(ctx, ip_addr) + if not ip_addr: + raise ValueError('') if interface_name == 'eth0': config_db.set_entry("MGMT_INTERFACE", (interface_name, ip_addr), None) @@ -3561,7 +3557,7 @@ def remove(ctx, interface_name, ip_addr): command = "ip neigh flush dev {} {}".format(interface_name, ip_addr) clicommon.run_command(command) except ValueError: - ctx.fail("'ip_addr' is not valid.") + ctx.fail("ip address or mask is not valid.") # diff --git a/tests/ip_config_test.py b/tests/ip_config_test.py index eb81d3bdd2..d08a03ca8f 100644 --- a/tests/ip_config_test.py +++ b/tests/ip_config_test.py @@ -8,6 +8,9 @@ import show.main as show from utilities_common.db import Db +ERROR_MSG = ''' +Error: ip address or mask is not valid. +''' class TestConfigIP(object): @classmethod @@ -43,7 +46,7 @@ def test_add_interface_invalid_ipv4(self): result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet64", "10000.10.10.1/24"], obj=obj) print(result.exit_code, result.output) assert result.exit_code != 0 - assert "Error: ip address is not valid." in result.output + assert ERROR_MSG in result.output def test_add_interface_ipv4_invalid_mask(self): db = Db() @@ -54,7 +57,7 @@ def test_add_interface_ipv4_invalid_mask(self): result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet64", "10.10.10.1/37"], obj=obj) print(result.exit_code, result.output) assert result.exit_code != 0 - assert "Error: ip mask is not valid." in result.output + assert ERROR_MSG in result.output def test_add_del_interface_ipv4_with_leading_zeros(self): db = Db() @@ -72,7 +75,7 @@ def test_add_del_interface_ipv4_with_leading_zeros(self): print(result.exit_code, result.output) assert result.exit_code != 0 assert ('Ethernet68', '10.10.10.2/24') not in db.cfgdb.get_table('INTERFACE') - + ''' Tests for IPv6 ''' def test_add_del_interface_valid_ipv6(self): @@ -101,7 +104,7 @@ def test_add_interface_invalid_ipv6(self): result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet72", "20001:0db8:11a3:19d7:1f34:8a2e:17a0:765d/34"], obj=obj) print(result.exit_code, result.output) assert result.exit_code != 0 - assert "Error: ip address is not valid." in result.output + assert ERROR_MSG in result.output def test_add_interface_ipv6_invalid_mask(self): db = Db() @@ -111,8 +114,8 @@ def test_add_interface_ipv6_invalid_mask(self): # config int ip add Ethernet72 2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/200 result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet72", "2001:0db8:11a3:09d7:1f34:8a2e:07a0:765d/200"], obj=obj) print(result.exit_code, result.output) - assert result.exit_code == 0 - assert "Error: ip mask is not valid." in result.output + assert result.exit_code != 0 + assert ERROR_MSG in result.output def test_add_del_interface_ipv6_with_leading_zeros(self): db = Db()