From 05e42b0e46595ad2b5693a7c23ab4455fa7c22cb Mon Sep 17 00:00:00 2001 From: d-dashkov Date: Fri, 22 Jan 2021 16:53:57 +0200 Subject: [PATCH 1/4] Check entry name is in db Signed-off-by: d-dashkov --- config/main.py | 50 ++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 46 insertions(+), 4 deletions(-) diff --git a/config/main.py b/config/main.py index 5f3fa24c86..d6840a421e 100755 --- a/config/main.py +++ b/config/main.py @@ -303,6 +303,42 @@ def interface_alias_to_name(config_db, interface_alias): # portchannel is passed in as argument, which does not have an alias return interface_alias if sub_intf_sep_idx == -1 else interface_alias + VLAN_SUB_INTERFACE_SEPARATOR + vlan_id +def loopback_name_is_valid(config_db, loopback_name): + """Check if the loopback name is valid + """ + # If the input parameter config_db is None, try DEFAULT_NAMESPACE. + if config_db is None: + namespace = DEFAULT_NAMESPACE + config_db = ConfigDBConnector(use_unix_socket_path=True, namespace=namespace) + + config_db.connect() + loopback_dict = config_db.get_table('LOOPBACK_INTERFACE') + + if loopback_name is not None: + if loopback_dict: + for loopback_dict_keys in loopback_dict.keys(): + if loopback_name == loopback_dict_keys: + return True + return False + +def vlan_name_is_valid(config_db, vlan_name): + """Check if the vlan name is valid + """ + # If the input parameter config_db is None, try DEFAULT_NAMESPACE. + if config_db is None: + namespace = DEFAULT_NAMESPACE + config_db = ConfigDBConnector(use_unix_socket_path=True, namespace=namespace) + + config_db.connect() + vlan_dict = config_db.get_table('VLAN') + + if vlan_name is not None: + if vlan_dict: + for vlan_dict_keys in vlan_dict.keys(): + if vlan_name == vlan_dict_keys: + return True + return False + def interface_name_is_valid(config_db, interface_name): """Check if the interface name is valid """ @@ -2473,9 +2509,12 @@ def add(ctx, interface_name, ip_addr, gw): return - table_name = get_interface_table_name(interface_name) - if table_name == "": + if not (interface_name_is_valid(config_db, interface_name) + or vlan_name_is_valid(config_db, interface_name) + or loopback_name_is_valid(config_db, interface_name)): ctx.fail("'interface_name' is not valid. Valid names [Ethernet/PortChannel/Vlan/Loopback]") + + table_name = get_interface_table_name(interface_name) interface_entry = config_db.get_entry(table_name, interface_name) if len(interface_entry) == 0: if table_name == "VLAN_SUB_INTERFACE": @@ -2514,9 +2553,12 @@ def remove(ctx, interface_name, ip_addr): mgmt_ip_restart_services() return - table_name = get_interface_table_name(interface_name) - if table_name == "": + if not (interface_name_is_valid(config_db, interface_name) + or vlan_name_is_valid(config_db, interface_name) + or loopback_name_is_valid(config_db, interface_name)): ctx.fail("'interface_name' is not valid. Valid names [Ethernet/PortChannel/Vlan/Loopback]") + + table_name = get_interface_table_name(interface_name) 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: From f81a5fede21eeadc32a63a4728c9dd2dbd0efa88 Mon Sep 17 00:00:00 2001 From: d-dashkov Date: Fri, 22 Jan 2021 17:13:03 +0200 Subject: [PATCH 2/4] Replace if statement Signed-off-by: d-dashkov --- config/main.py | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/config/main.py b/config/main.py index d6840a421e..90b695447b 100755 --- a/config/main.py +++ b/config/main.py @@ -314,11 +314,10 @@ def loopback_name_is_valid(config_db, loopback_name): config_db.connect() loopback_dict = config_db.get_table('LOOPBACK_INTERFACE') - if loopback_name is not None: - if loopback_dict: - for loopback_dict_keys in loopback_dict.keys(): - if loopback_name == loopback_dict_keys: - return True + if loopback_name is not None and loopback_dict: + for loopback_dict_keys in loopback_dict.keys(): + if loopback_name == loopback_dict_keys: + return True return False def vlan_name_is_valid(config_db, vlan_name): @@ -332,11 +331,10 @@ def vlan_name_is_valid(config_db, vlan_name): config_db.connect() vlan_dict = config_db.get_table('VLAN') - if vlan_name is not None: - if vlan_dict: - for vlan_dict_keys in vlan_dict.keys(): - if vlan_name == vlan_dict_keys: - return True + if vlan_name is not None and vlan_dict: + for vlan_dict_keys in vlan_dict.keys(): + if vlan_name == vlan_dict_keys: + return True return False def interface_name_is_valid(config_db, interface_name): From 9c9c5340983577293997b8d7bd1e822d067ead8f Mon Sep 17 00:00:00 2001 From: Nick Date: Fri, 8 Oct 2021 17:59:02 +0300 Subject: [PATCH 3/4] Add unit-tests. Signed-off-by: Mykola Horb --- tests/config_int_ip_test.py | 42 ++++++++++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/tests/config_int_ip_test.py b/tests/config_int_ip_test.py index 6968fcbe45..bd5efedf28 100644 --- a/tests/config_int_ip_test.py +++ b/tests/config_int_ip_test.py @@ -14,6 +14,10 @@ sys.path.insert(0, test_path) mock_db_path = os.path.join(test_path, "int_ip_input") +ERROR_MSG_WRONG_INTERFACE_NAME = '''Usage: add [OPTIONS] +Try "add --help" for help. + +Error: \'interface_name\' is not valid. Valid names [Ethernet/PortChannel/Vlan/Loopback]''' class TestIntIp(object): @pytest.fixture(scope="class", autouse=True) @@ -155,4 +159,40 @@ def test_config_int_ip_rem_static_multiasic( print(result.exit_code, result.output) assert result.exit_code != 0 assert "Error: Cannot remove the last IP entry of interface Ethernet8. A static ipv6 route is still bound to the RIF." in result.output - assert mock_run_command.call_count == 0 \ No newline at end of file + assert mock_run_command.call_count == 0 + + +class TestConfigIP_wrong_name(object): + @classmethod + def setup_class(cls): + os.environ['UTILITIES_UNIT_TESTING'] = "1" + print("SETUP") + + def test_add_interface_invalid_vlan(self): + db = Db() + runner = CliRunner() + obj = {'config_db':db.cfgdb} + import config.main as config + + #Try to set wrong VLAN: config int ip add Vlan100500 100.50.20.1/24 + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Vlan100500", "100.50.20.1/24"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code != 0 + assert ERROR_MSG_WRONG_INTERFACE_NAME in result.output + + def test_add_interface_invalid_name(self): + db = Db() + runner = CliRunner() + obj = {'config_db':db.cfgdb} + import config.main as config + + #Try to set IP on wrong interface: config int ip add Ethernet2abc 100.50.20.1/24 + result = runner.invoke(config.config.commands["interface"].commands["ip"].commands["add"], ["Ethernet2abc", "100.50.20.1/24"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code != 0 + assert ERROR_MSG_WRONG_INTERFACE_NAME in result.output + + @classmethod + def teardown_class(cls): + os.environ['UTILITIES_UNIT_TESTING'] = "0" + print("TEARDOWN") From 473b6b17053b74d32036a5370ab7aa1322c7643a Mon Sep 17 00:00:00 2001 From: d-dashkov Date: Wed, 29 Dec 2021 23:26:02 +0200 Subject: [PATCH 4/4] Remove validation on remove Signed-off-by: d-dashkov --- config/main.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/config/main.py b/config/main.py index fdabc4466f..6395efbfde 100755 --- a/config/main.py +++ b/config/main.py @@ -3537,11 +3537,6 @@ def remove(ctx, interface_name, ip_addr): mgmt_ip_restart_services() return - if not (interface_name_is_valid(config_db, interface_name) - or vlan_name_is_valid(config_db, interface_name) - or loopback_name_is_valid(config_db, interface_name)): - ctx.fail("'interface_name' is not valid. Valid names [Ethernet/PortChannel/Vlan/Loopback]") - table_name = get_interface_table_name(interface_name) interface_dependent = interface_ipaddr_dependent_on_interface(config_db, interface_name) # If we deleting the last IP entry of the interface, check whether a static route present for the RIF