Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Validate interface name length in CLI #26

Closed
wants to merge 1 commit into from
Closed

Validate interface name length in CLI #26

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 12 additions & 11 deletions config/main.py
View file
Open in desktop
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, ConfigDBPipeConnector
from swsscommon.swsscommon import SonicV2Connector, ConfigDBConnector, ConfigDBPipeConnector, 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 @@ -2400,8 +2399,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 @@ -5824,8 +5823,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(vrf_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 @@ -5844,8 +5843,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(vrf_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 @@ -6849,8 +6848,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 @@ -7597,6 +7596,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(interface_alias):
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
Open in desktop
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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2623,6 +2623,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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading