From 5892f6dce142bae6cf533330b6f542ad1c6174c1 Mon Sep 17 00:00:00 2001 From: Sudharsan Dhamal Gopalarathnam Date: Mon, 29 Aug 2022 06:26:23 -0700 Subject: [PATCH] [VRF]Adding CLI checks to ensure Vrf is valid in interface bind and static route commands (#2333) - What I did Added CLI checks to verify if Vrf is created and valid when used in interface bind and static route commands - How I did it Check for presence of Vrf in VRF table or in case if it is mgmt vrf, verify if mgmt vrf is enabled/ - How to verify it Added UT to verify it. --- config/main.py | 21 +++++++++++++++++++++ tests/ip_config_test.py | 35 +++++++++++++++++++++++++++++++++++ tests/static_routes_test.py | 13 +++++++++++++ 3 files changed, 69 insertions(+) diff --git a/config/main.py b/config/main.py index 62df0d7389..ac4d41da12 100644 --- a/config/main.py +++ b/config/main.py @@ -365,6 +365,19 @@ def get_interface_ipaddresses(config_db, interface_name): return ipaddresses +def is_vrf_exists(config_db, vrf_name): + """Check if VRF exists + """ + keys = config_db.get_keys("VRF") + if vrf_name in keys: + return True + elif vrf_name == "mgmt": + entry = config_db.get_entry("MGMT_VRF_CONFIG", "vrf_global") + if entry and entry.get("mgmtVrfEnabled") == "true": + return True + + return False + def is_interface_bind_to_vrf(config_db, interface_name): """Get interface if bind to vrf or not """ @@ -910,6 +923,7 @@ def cli_sroute_to_config(ctx, command_str, strict_nh = True): nexthop_str = None config_entry = {} vrf_name = "" + config_db = ctx.obj['config_db'] if "nexthop" in command_str: idx = command_str.index("nexthop") @@ -922,6 +936,8 @@ def cli_sroute_to_config(ctx, command_str, strict_nh = True): if 'prefix' in prefix_str and 'vrf' in prefix_str: # prefix_str: ['prefix', 'vrf', Vrf-name, ip] vrf_name = prefix_str[2] + if not is_vrf_exists(config_db, vrf_name): + ctx.fail("VRF %s does not exist!"%(vrf_name)) ip_prefix = prefix_str[3] elif 'prefix' in prefix_str: # prefix_str: ['prefix', ip] @@ -933,6 +949,8 @@ def cli_sroute_to_config(ctx, command_str, strict_nh = True): if 'nexthop' in nexthop_str and 'vrf' in nexthop_str: # nexthop_str: ['nexthop', 'vrf', Vrf-name, ip] config_entry["nexthop"] = nexthop_str[3] + if not is_vrf_exists(config_db, nexthop_str[2]): + ctx.fail("VRF %s does not exist!"%(nexthop_str[2])) config_entry["nexthop-vrf"] = nexthop_str[2] elif 'nexthop' in nexthop_str and 'dev' in nexthop_str: # nexthop_str: ['nexthop', 'dev', ifname] @@ -4821,6 +4839,9 @@ def bind(ctx, interface_name, vrf_name): if interface_name is None: ctx.fail("'interface_name' is None!") + if not is_vrf_exists(config_db, vrf_name): + ctx.fail("VRF %s does not exist!"%(vrf_name)) + table_name = get_interface_table_name(interface_name) if table_name == "": ctx.fail("'interface_name' is not valid. Valid names [Ethernet/PortChannel/Vlan/Loopback]") diff --git a/tests/ip_config_test.py b/tests/ip_config_test.py index 872825fb13..c56b226c74 100644 --- a/tests/ip_config_test.py +++ b/tests/ip_config_test.py @@ -15,6 +15,20 @@ ERROR_MSG = "Error: IP address is not valid" +INVALID_VRF_MSG ="""\ +Usage: bind [OPTIONS] +Try "bind --help" for help. + +Error: VRF Vrf2 does not exist! +""" + +INVALID_MGMT_VRF_MSG ="""\ +Usage: bind [OPTIONS] +Try "bind --help" for help. + +Error: VRF mgmt does not exist! +""" + class TestConfigIP(object): @classmethod def setup_class(cls): @@ -234,6 +248,27 @@ def test_intf_vrf_bind_unbind(self): print(result.exit_code, result.output) assert result.exit_code == 0 + def test_intf_unknown_vrf_bind(self): + runner = CliRunner() + db = Db() + obj = {'config_db':db.cfgdb, 'namespace':db.db.namespace} + + result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["bind"], ["Ethernet64", "Vrf2"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code != 0 + assert result.output == INVALID_VRF_MSG + + result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["bind"], ["Ethernet64", "mgmt"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code != 0 + assert result.output == INVALID_MGMT_VRF_MSG + + result = runner.invoke(config.config.commands["vrf"].commands["add"], ["mgmt"], obj=obj) + print(result.exit_code, result.output) + result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["bind"], ["Ethernet64", "mgmt"], obj=obj) + print(result.exit_code, result.output) + assert result.exit_code == 0 + @classmethod def teardown_class(cls): os.environ['UTILITIES_UNIT_TESTING'] = "0" diff --git a/tests/static_routes_test.py b/tests/static_routes_test.py index 4d60c3ef0c..fc7371b344 100644 --- a/tests/static_routes_test.py +++ b/tests/static_routes_test.py @@ -93,6 +93,8 @@ def test_vrf_static_route(self): obj = {'config_db':db.cfgdb} # config route add prefix vrf Vrf-BLUE 2.2.3.4/32 nexthop 30.0.0.6 + result = runner.invoke(config.config.commands["vrf"].commands["add"], ["Vrf-BLUE"], obj=obj) + print(result.exit_code, result.output) result = runner.invoke(config.config.commands["route"].commands["add"], \ ["prefix", "vrf", "Vrf-BLUE", "2.2.3.4/32", "nexthop", "30.0.0.6"], obj=obj) print(result.exit_code, result.output) @@ -111,9 +113,12 @@ def test_dest_vrf_static_route(self): obj = {'config_db':db.cfgdb} # config route add prefix 3.2.3.4/32 nexthop vrf Vrf-RED 30.0.0.6 + result = runner.invoke(config.config.commands["vrf"].commands["add"], ["Vrf-RED"], obj=obj) + print(result.exit_code, result.output) result = runner.invoke(config.config.commands["route"].commands["add"], \ ["prefix", "3.2.3.4/32", "nexthop", "vrf", "Vrf-RED", "30.0.0.6"], obj=obj) print(result.exit_code, result.output) + print(db.cfgdb.get_table('STATIC_ROUTE')) assert ('3.2.3.4/32') in db.cfgdb.get_table('STATIC_ROUTE') assert db.cfgdb.get_entry('STATIC_ROUTE', '3.2.3.4/32') == {'nexthop': '30.0.0.6', 'nexthop-vrf': 'Vrf-RED', 'blackhole': 'false', 'distance': '0', 'ifname': ''} @@ -129,6 +134,8 @@ def test_multiple_nexthops_with_vrf_static_route(self): obj = {'config_db':db.cfgdb} ''' Add ''' + result = runner.invoke(config.config.commands["vrf"].commands["add"], ["Vrf-RED"], obj=obj) + print(result.exit_code, result.output) # config route add prefix 6.2.3.4/32 nexthop vrf Vrf-RED "30.0.0.6,30.0.0.7" result = runner.invoke(config.config.commands["route"].commands["add"], \ ["prefix", "6.2.3.4/32", "nexthop", "vrf", "Vrf-RED", "30.0.0.6,30.0.0.7"], obj=obj) @@ -251,6 +258,8 @@ def test_static_route_ECMP_nexthop_with_vrf(self): obj = {'config_db':db.cfgdb} ''' Add ''' + result = runner.invoke(config.config.commands["vrf"].commands["add"], ["Vrf-RED"], obj=obj) + print(result.exit_code, result.output) # config route add prefix 11.2.3.4/32 nexthop vrf Vrf-RED 30.0.0.5 result = runner.invoke(config.config.commands["route"].commands["add"], \ ["prefix", "11.2.3.4/32", "nexthop", "vrf", "Vrf-RED", "30.0.0.5"], obj=obj) @@ -258,6 +267,8 @@ def test_static_route_ECMP_nexthop_with_vrf(self): assert ('11.2.3.4/32') in db.cfgdb.get_table('STATIC_ROUTE') assert db.cfgdb.get_entry('STATIC_ROUTE', '11.2.3.4/32') == {'nexthop': '30.0.0.5', 'nexthop-vrf': 'Vrf-RED', 'blackhole': 'false', 'distance': '0', 'ifname': ''} + result = runner.invoke(config.config.commands["vrf"].commands["add"], ["Vrf-BLUE"], obj=obj) + print(result.exit_code, result.output) # config route add prefix 11.2.3.4/32 nexthop vrf Vrf-BLUE 30.0.0.6 result = runner.invoke(config.config.commands["route"].commands["add"], \ ["prefix", "11.2.3.4/32", "nexthop", "vrf", "Vrf-BLUE", "30.0.0.6"], obj=obj) @@ -292,6 +303,8 @@ def test_static_route_ECMP_mixed_nextfop(self): assert ('12.2.3.4/32') in db.cfgdb.get_table('STATIC_ROUTE') assert db.cfgdb.get_entry('STATIC_ROUTE', '12.2.3.4/32') == {'nexthop': '30.0.0.6', 'blackhole': 'false', 'distance': '0', 'ifname': '', 'nexthop-vrf': ''} + result = runner.invoke(config.config.commands["vrf"].commands["add"], ["Vrf-RED"], obj=obj) + print(result.exit_code, result.output) # config route add prefix 12.2.3.4/32 nexthop vrf Vrf-RED 30.0.0.7 result = runner.invoke(config.config.commands["route"].commands["add"], \ ["prefix", "12.2.3.4/32", "nexthop", "vrf", "Vrf-RED", "30.0.0.7"], obj=obj)