Skip to content

Commit

Permalink
[VRF]Adding CLI checks to ensure Vrf is valid in interface bind and s…
Browse files Browse the repository at this point in the history
…tatic route commands (sonic-net#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.
  • Loading branch information
dgsudharsan authored and yxieca committed Sep 1, 2022
1 parent 87ec859 commit 5892f6d
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 0 deletions.
21 changes: 21 additions & 0 deletions config/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
"""
Expand Down Expand Up @@ -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")
Expand All @@ -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]
Expand All @@ -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]
Expand Down Expand Up @@ -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]")
Expand Down
35 changes: 35 additions & 0 deletions tests/ip_config_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,20 @@

ERROR_MSG = "Error: IP address is not valid"

INVALID_VRF_MSG ="""\
Usage: bind [OPTIONS] <interface_name> <vrf_name>
Try "bind --help" for help.
Error: VRF Vrf2 does not exist!
"""

INVALID_MGMT_VRF_MSG ="""\
Usage: bind [OPTIONS] <interface_name> <vrf_name>
Try "bind --help" for help.
Error: VRF mgmt does not exist!
"""

class TestConfigIP(object):
@classmethod
def setup_class(cls):
Expand Down Expand Up @@ -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"
Expand Down
13 changes: 13 additions & 0 deletions tests/static_routes_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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': ''}

Expand All @@ -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)
Expand Down Expand Up @@ -251,13 +258,17 @@ 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)
print(result.exit_code, result.output)
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)
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 5892f6d

Please sign in to comment.