Skip to content

Commit

Permalink
Validate interface name length in CLI
Browse files Browse the repository at this point in the history
  • Loading branch information
@arfeigin
arfeigin committed Jun 16, 2024
1 parent d0856af commit fd71cc5
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 50 deletions.
23 changes: 12 additions & 11 deletions config/main.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
from sonic_yang_cfg_generator import SonicYangCfgDbGenerator
from utilities_common import util_base
from swsscommon import swsscommon
from swsscommon.swsscommon import SonicV2Connector, ConfigDBConnector
from swsscommon.swsscommon import SonicV2Connector, ConfigDBConnector, validate_interface_name_length, iface_name_max_length
from utilities_common.db import Db
from utilities_common.intf_filter import parse_interface_in_filter
from utilities_common import bgp_util
Expand Down Expand Up @@ -102,7 +102,6 @@

CFG_PORTCHANNEL_PREFIX = "PortChannel"
CFG_PORTCHANNEL_PREFIX_LEN = 11
CFG_PORTCHANNEL_NAME_TOTAL_LEN_MAX = 15
CFG_PORTCHANNEL_MAX_VAL = 9999
CFG_PORTCHANNEL_NO="<0-9999>"

Expand Down Expand Up @@ -429,7 +428,7 @@ def is_portchannel_name_valid(portchannel_name):
if (portchannel_name[CFG_PORTCHANNEL_PREFIX_LEN:].isdigit() is False or
int(portchannel_name[CFG_PORTCHANNEL_PREFIX_LEN:]) > CFG_PORTCHANNEL_MAX_VAL) :
return False
if len(portchannel_name) > CFG_PORTCHANNEL_NAME_TOTAL_LEN_MAX:
if not validate_interface_name_length(portchannel_name):
return False
return True

Expand Down Expand Up @@ -2291,8 +2290,8 @@ def add_portchannel(ctx, portchannel_name, min_links, fallback, fast_rate):
db = ValidatedConfigDBConnector(ctx.obj['db'])
if ADHOC_VALIDATION:
if is_portchannel_name_valid(portchannel_name) != True:
ctx.fail("{} is invalid!, name should have prefix '{}' and suffix '{}'"
.format(portchannel_name, CFG_PORTCHANNEL_PREFIX, CFG_PORTCHANNEL_NO))
ctx.fail("{} is invalid!, name should have prefix '{}' and suffix '{}' and its length should not exceed {} characters"
.format(portchannel_name, CFG_PORTCHANNEL_PREFIX, CFG_PORTCHANNEL_NO, IFNAMSIZ))
if is_portchannel_present_in_db(db, portchannel_name):
ctx.fail("{} already exists!".format(portchannel_name)) # TODO: MISSING CONSTRAINT IN YANG MODEL

Expand Down Expand Up @@ -5714,8 +5713,8 @@ def add_vrf(ctx, vrf_name):
config_db = ValidatedConfigDBConnector(ctx.obj['config_db'])
if not vrf_name.startswith("Vrf") and not (vrf_name == 'mgmt') and not (vrf_name == 'management'):
ctx.fail("'vrf_name' must begin with 'Vrf' or named 'mgmt'/'management' in case of ManagementVRF.")
if len(vrf_name) > 15:
ctx.fail("'vrf_name' is too long!")
if not validate_interface_name_length(portchannel_name):
ctx.fail("'vrf_name' length should not exceed {} characters".format(IFNAMSIZ))
if is_vrf_exists(config_db, vrf_name):
ctx.fail("VRF {} already exists!".format(vrf_name))
elif (vrf_name == 'mgmt' or vrf_name == 'management'):
Expand All @@ -5734,8 +5733,8 @@ def del_vrf(ctx, vrf_name):
config_db = ValidatedConfigDBConnector(ctx.obj['config_db'])
if not vrf_name.startswith("Vrf") and not (vrf_name == 'mgmt') and not (vrf_name == 'management'):
ctx.fail("'vrf_name' must begin with 'Vrf' or named 'mgmt'/'management' in case of ManagementVRF.")
if len(vrf_name) > 15:
ctx.fail("'vrf_name' is too long!")
if not validate_interface_name_length(portchannel_name):
ctx.fail("'vrf_name' length should not exceed {} characters".format(IFNAMSIZ))
syslog_table = config_db.get_table("SYSLOG_SERVER")
syslog_vrf_dev = "mgmt" if vrf_name == "management" else vrf_name
for syslog_entry, syslog_data in syslog_table.items():
Expand Down Expand Up @@ -6739,8 +6738,8 @@ def add_loopback(ctx, loopback_name):
config_db = ValidatedConfigDBConnector(ctx.obj['db'])
if ADHOC_VALIDATION:
if is_loopback_name_valid(loopback_name) is False:
ctx.fail("{} is invalid, name should have prefix '{}' and suffix '{}' "
.format(loopback_name, CFG_LOOPBACK_PREFIX, CFG_LOOPBACK_NO))
ctx.fail("{} is invalid, name should have prefix '{}' and suffix '{}' and should not exceed {} characters"
.format(loopback_name, CFG_LOOPBACK_PREFIX, CFG_LOOPBACK_NO, IFNAMSIZ))

lo_intfs = [k for k, v in config_db.get_table('LOOPBACK_INTERFACE').items() if type(k) != tuple]
if loopback_name in lo_intfs:
Expand Down Expand Up @@ -7487,6 +7486,8 @@ def add_subinterface(ctx, subinterface_name, vid):

if interface_alias is None:
ctx.fail("{} invalid subinterface".format(interface_alias))
if not validate_interface_name_length(portchannel_name):
ctx.fail("Subinterface name length should not exceed {} characters".format(IFNAMSIZ))

if interface_alias.startswith("Po") is True:
intf_table_name = CFG_PORTCHANNEL_PREFIX
Expand Down
14 changes: 9 additions & 5 deletions config/vxlan.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@

from jsonpatch import JsonPatchConflict
from .validated_config_db_connector import ValidatedConfigDBConnector
from swsscommon.swsscommon import validate_interface_name_length

ADHOC_VALIDATION = True
IFNAMSIZ = 16
#
# 'vxlan' group ('config vxlan ...')
#
Expand All @@ -23,7 +25,9 @@ def add_vxlan(db, vxlan_name, src_ip):

if ADHOC_VALIDATION:
if not clicommon.is_ipaddress(src_ip):
ctx.fail("{} invalid src ip address".format(src_ip))
ctx.fail("{} invalid src ip address".format(src_ip))
if not validate_interface_name_length(vxlan_name):
ctx.fail("'vxlan_name' length should not exceed {} characters".format(IFNAMSIZ))

vxlan_keys = db.cfgdb.get_keys('VXLAN_TUNNEL')
if not vxlan_keys:
Expand All @@ -32,7 +36,7 @@ def add_vxlan(db, vxlan_name, src_ip):
vxlan_count = len(vxlan_keys)

if(vxlan_count > 0):
ctx.fail("VTEP already configured.")
ctx.fail("VTEP already configured.")

fvs = {'src_ip': src_ip}
try:
Expand All @@ -59,7 +63,7 @@ def del_vxlan(db, vxlan_name):
vxlan_count = len(vxlan_keys)

if(vxlan_count > 0):
ctx.fail("Please delete the EVPN NVO configuration.")
ctx.fail("Please delete the EVPN NVO configuration.")

vxlan_keys = db.cfgdb.get_keys("VXLAN_TUNNEL_MAP|*")
if not vxlan_keys:
Expand All @@ -68,7 +72,7 @@ def del_vxlan(db, vxlan_name):
vxlan_count = len(vxlan_keys)

if(vxlan_count > 0):
ctx.fail("Please delete all VLAN VNI mappings.")
ctx.fail("Please delete all VLAN VNI mappings.")

vnet_table = db.cfgdb.get_table('VNET')
vnet_keys = vnet_table.keys()
Expand Down Expand Up @@ -100,7 +104,7 @@ def add_vxlan_evpn_nvo(db, nvo_name, vxlan_name):
vxlan_count = len(vxlan_keys)

if(vxlan_count > 0):
ctx.fail("EVPN NVO already configured")
ctx.fail("EVPN NVO already configured")

if len(db.cfgdb.get_entry('VXLAN_TUNNEL', vxlan_name)) == 0:
ctx.fail("VTEP {} not configured".format(vxlan_name))
Expand Down
6 changes: 6 additions & 0 deletions tests/config_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2354,6 +2354,12 @@ def test_add_loopback_with_invalid_name_adhoc_validation(self):
assert result.exit_code != 0
assert "Error: Loopbax1 is invalid, name should have prefix 'Loopback' and suffix '<0-999>'" in result.output

result = runner.invoke(config.config.commands["loopback"].commands["add"], ["Loopback0000"], obj=obj)
print(result.exit_code)
print(result.output)
assert result.exit_code != 0
assert "Error: Loopback0000 is invalid, name should have prefix 'Loopback' and suffix '<0-999>' and should not exceed 11 characters" in result.output

def test_del_nonexistent_loopback_adhoc_validation(self):
config.ADHOC_VALIDATION = True
runner = CliRunner()
Expand Down
12 changes: 9 additions & 3 deletions tests/portchannel_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def test_add_portchannel_with_invalid_name_yang_validation(self):
print(result.output)
assert result.exit_code != 0
assert "Error: PortChan005 is invalid!, name should have prefix 'PortChannel' and suffix '<0-9999>'" in result.output

def test_add_portchannel_with_invalid_name_adhoc_validation(self):
config.ADHOC_VALIDATION = True
runner = CliRunner()
Expand All @@ -46,7 +46,13 @@ def test_add_portchannel_with_invalid_name_adhoc_validation(self):
print(result.exit_code)
print(result.output)
assert result.exit_code != 0
assert "Error: PortChan005 is invalid!, name should have prefix 'PortChannel' and suffix '<0-9999>'" in result.output
assert "Error: PortChan005 is invalid!, name should have prefix 'PortChannel' and suffix '<0-9999>' and its length should not exceed 15 characters" in result.output

result = runner.invoke(config.config.commands["portchannel"].commands["add"], ["PortChanl00000"], obj=obj)
print(result.exit_code)
print(result.output)
assert result.exit_code != 0
assert "Error: PortChanl00000 is invalid!, name should have prefix 'PortChannel' and suffix '<0-9999>' and its length should not exceed 15 characters" in result.output

@patch("config.validated_config_db_connector.ValidatedConfigDBConnector.validated_set_entry", mock.Mock(side_effect=JsonPatchConflict))
@patch("validated_config_db_connector.device_info.is_yang_config_validation_enabled", mock.Mock(return_value=True))
Expand Down Expand Up @@ -119,7 +125,7 @@ def test_add_portchannel_with_invalid_fast_rate(self, fast_rate):
runner = CliRunner()
db = Db()
obj = {'db':db.cfgdb}

# add a portchannel with invalid fats rate
result = runner.invoke(config.config.commands["portchannel"].commands["add"], ["PortChannel0005", "--fast-rate", fast_rate], obj=obj)
print(result.exit_code)
Expand Down
20 changes: 4 additions & 16 deletions tests/subintf_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ def test_add_del_subintf_short_name(self):
runner = CliRunner()
db = Db()
obj = {'db':db.cfgdb}

result = runner.invoke(config.config.commands["subinterface"].commands["add"], ["Eth0.102", "1002"], obj=obj)
print(result.exit_code, result.output)
assert result.exit_code == 0
Expand Down Expand Up @@ -53,35 +53,23 @@ def test_add_del_subintf_with_long_name(self):
runner = CliRunner()
db = Db()
obj = {'db':db.cfgdb}

result = runner.invoke(config.config.commands["subinterface"].commands["add"], ["Ethernet0.102"], obj=obj)
print(result.exit_code, result.output)
assert result.exit_code == 0
assert ('Ethernet0.102') in db.cfgdb.get_table('VLAN_SUB_INTERFACE')
assert db.cfgdb.get_table('VLAN_SUB_INTERFACE')['Ethernet0.102']['admin_status'] == 'up'

result = runner.invoke(config.config.commands["subinterface"].commands["add"], ["PortChannel0004.104"], obj=obj)
print(result.exit_code, result.output)
assert result.exit_code == 0
assert ('PortChannel0004.104') in db.cfgdb.get_table('VLAN_SUB_INTERFACE')
assert db.cfgdb.get_table('VLAN_SUB_INTERFACE')['PortChannel0004.104']['admin_status'] == 'up'

result = runner.invoke(config.config.commands["subinterface"].commands["del"], ["Ethernet0.102"], obj=obj)
print(result.exit_code, result.output)
assert result.exit_code == 0
assert ('Ethernet0.102') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE')

result = runner.invoke(config.config.commands["subinterface"].commands["del"], ["PortChannel0004.104"], obj=obj)
print(result.exit_code, result.output)
assert result.exit_code == 0
assert ('PortChannel0004.104') not in db.cfgdb.get_table('VLAN_SUB_INTERFACE')


def test_add_existing_subintf_again(self):
runner = CliRunner()
db = Db()
obj = {'db':db.cfgdb}

result = runner.invoke(config.config.commands["subinterface"].commands["add"], ["Ethernet0.102"], obj=obj)
print(result.exit_code, result.output)
assert result.exit_code == 0
Expand All @@ -104,7 +92,7 @@ def test_delete_non_existing_subintf(self):
runner = CliRunner()
db = Db()
obj = {'db':db.cfgdb}

result = runner.invoke(config.config.commands["subinterface"].commands["del"], ["Ethernet0.102"], obj=obj)
print(result.exit_code, result.output)
assert result.exit_code != 0
Expand Down
8 changes: 8 additions & 0 deletions tests/vlan_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,14 @@ def test_config_vlan_del_member_with_invalid_port(self):
assert result.exit_code != 0
assert "Error: Invalid VLAN ID 4097 (2-4094)" in result.output

def test_config_vlan_add_member_with_invalid_long_name(self):
runner = CliRunner()
result = runner.invoke(config.config.commands["vlan"].commands["member"].commands["add"], ["123456789012", "Ethernet4"])
print(result.exit_code)
print(result.output)
assert result.exit_code != 0
assert "Error: Invalid VLAN ID 123456789012 (1-4094)" in result.output

def test_config_vlan_add_member_with_nonexist_vlanid(self):
runner = CliRunner()
result = runner.invoke(config.config.commands["vlan"].commands["member"].commands["add"], ["1001", "Ethernet4"])
Expand Down
38 changes: 23 additions & 15 deletions tests/vrf_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def test_vrf_show(self):
Loopback0
Po0002.101
"""

result = runner.invoke(show.cli.commands['vrf'], [], obj=db)
dbconnector.dedicated_dbs = {}
assert result.exit_code == 0
Expand All @@ -65,7 +65,7 @@ def test_vrf_bind_unbind(self):
Loopback0
Po0002.101
"""

result = runner.invoke(show.cli.commands['vrf'], [], obj=db)
dbconnector.dedicated_dbs = {}
assert result.exit_code == 0
Expand All @@ -81,7 +81,7 @@ def test_vrf_bind_unbind(self):
assert result.exit_code == 0
assert 'Ethernet4' not in db.cfgdb.get_table('INTERFACE')
assert result.output == expected_output_unbind

expected_output_unbind = "Interface Loopback0 IP disabled and address(es) removed due to unbinding VRF.\n"

result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["unbind"], ["Loopback0"], obj=vrf_obj)
Expand All @@ -108,7 +108,7 @@ def test_vrf_bind_unbind(self):
assert result.exit_code == 0
assert 'PortChannel002' not in db.cfgdb.get_table('PORTCHANNEL_INTERFACE')
assert result.output == expected_output_unbind

vrf_obj = {'config_db':db.cfgdb, 'namespace':DEFAULT_NAMESPACE}
state_db = SonicV2Connector(use_unix_socket_path=True, namespace='')
state_db.connect(state_db.STATE_DB, False)
Expand All @@ -117,7 +117,7 @@ def test_vrf_bind_unbind(self):
vrf_obj['state_db'] = state_db

expected_output_unbind = "Interface Eth36.10 IP disabled and address(es) removed due to unbinding VRF.\n"
T1 = threading.Thread( target = self.update_statedb, args = (state_db, db.db.STATE_DB, _hash))
T1 = threading.Thread( target = self.update_statedb, args = (state_db, db.db.STATE_DB, _hash))
T1.start()
result = runner.invoke(config.config.commands["interface"].commands["vrf"].commands["unbind"], ["Eth36.10"], obj=vrf_obj)
T1.join()
Expand Down Expand Up @@ -203,7 +203,7 @@ def test_vrf_bind_unbind(self):
Loopback0
Po0002.101
"""

result = runner.invoke(show.cli.commands['vrf'], [], obj=db)
dbconnector.dedicated_dbs = {}
assert result.exit_code == 0
Expand All @@ -213,24 +213,24 @@ def test_vrf_add_del(self):
runner = CliRunner()
db = Db()
vrf_obj = {'config_db':db.cfgdb, 'namespace':db.db.namespace}

result = runner.invoke(config.config.commands["vrf"].commands["add"], ["Vrf100"], obj=vrf_obj)
assert ('Vrf100') in db.cfgdb.get_table('VRF')
assert result.exit_code == 0

result = runner.invoke(config.config.commands["vrf"].commands["add"], ["Vrf1"], obj=vrf_obj)
assert "VRF Vrf1 already exists!" in result.output
assert ('Vrf1') in db.cfgdb.get_table('VRF')
assert result.exit_code != 0

expected_output_del = "VRF Vrf1 deleted and all associated IP addresses removed.\n"
result = runner.invoke(config.config.commands["vrf"].commands["del"], ["Vrf1"], obj=vrf_obj)
assert result.exit_code == 0
assert result.output == expected_output_del
assert ('Vrf1') not in db.cfgdb.get_table('VRF')

result = runner.invoke(config.config.commands["vrf"].commands["del"], ["Vrf200"], obj=vrf_obj)
assert result.exit_code != 0
assert result.exit_code != 0
assert ('Vrf200') not in db.cfgdb.get_table('VRF')
assert "VRF Vrf200 does not exist!" in result.output

Expand All @@ -245,25 +245,33 @@ def test_invalid_vrf_name(self):
assert result.exit_code != 0
assert ('vrf-blue') not in db.cfgdb.get_table('VRF')
assert expected_output in result.output

result = runner.invoke(config.config.commands["vrf"].commands["add"], ["VRF2"], obj=obj)
assert result.exit_code != 0
assert ('VRF2') not in db.cfgdb.get_table('VRF')
assert expected_output in result.output

result = runner.invoke(config.config.commands["vrf"].commands["add"], ["VrF10"], obj=obj)
assert result.exit_code != 0
assert ('VrF10') not in db.cfgdb.get_table('VRF')
assert expected_output in result.output

result = runner.invoke(config.config.commands["vrf"].commands["del"], ["vrf-blue"], obj=obj)
assert result.exit_code != 0
assert expected_output in result.output

result = runner.invoke(config.config.commands["vrf"].commands["del"], ["VRF2"], obj=obj)
assert result.exit_code != 0
assert expected_output in result.output

result = runner.invoke(config.config.commands["vrf"].commands["del"], ["VrF10"], obj=obj)
assert result.exit_code != 0
assert expected_output in result.output

expected_output = """\
Error: 'vrf_name' length should not exceed 16 characters
"""
result = runner.invoke(config.config.commands["vrf"].commands["add"], ["VrfNameTooLong!!!"], obj=obj)
assert result.exit_code != 0
assert ('VrfNameTooLong!!!') not in db.cfgdb.get_table('VRF')
assert expected_output in result.output

0 comments on commit fd71cc5

Please sign in to comment.