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.

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

Validate interface name length #25

Closed
wants to merge 55 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
fe29c8a
Validate interface name length in CLI
arfeigin Apr 3, 2024
914507e
Fix comments
arfeigin Apr 30, 2024
d3073d4
Fix typo
arfeigin May 13, 2024
777e88b
Use sonic-swss-common interface name length validation
arfeigin May 30, 2024
db7f6a9
Remove redundancy
arfeigin Jun 16, 2024
e3ccdfa
Improve Semgrep CI (#3259)
maipbui Apr 8, 2024
7c42081
[Mellanox]Fix Syntax Warning in config mlnx command (#3254)
dgsudharsan Apr 9, 2024
bed1808
Update intfutil and sfpshow to support DPC role (#3242)
vivekrnv Apr 11, 2024
7a13456
[Mellanox] added component versions to techsupport (#3264)
skr31 Apr 17, 2024
3a8673c
Add multi ASIC support for syslog rate limit feature (#3235)
Junchao-Mellanox Apr 18, 2024
dfc4418
[Mellanox] Support new platform SN5400 in generic configuration updat…
DavidZagury Apr 18, 2024
b22581c
Fix double hex to decimal conversion (#3267)
yuazhe Apr 19, 2024
e4fb2ac
Revert "Revert "route_check: Skip route checks if bgp feature is not …
anamehra Apr 19, 2024
0dcef84
[fast/warm-reboot] Retain TRANSCEIVER_INFO tables on fast/warm-reboot…
stepanblyschak Apr 19, 2024
b38898f
[chassis][voq]Add fabric monitoring commands. (#3239)
jfeng-arista Apr 22, 2024
0efca08
Add Multi ASIC support for apply-patch (#3249)
xincunli-sonic Apr 23, 2024
dccf863
Fix db_migrate.py show error and back trace while loading configurati…
liuh-80 Apr 25, 2024
a3ab1cb
Display target firmware version through CLI (#3274)
mihirpat1 Apr 25, 2024
a9e2118
[chassis][voq] Add fabric capacity monitoring cmds (#3255)
jfeng-arista Apr 26, 2024
318dc53
Migrate AAA table in db_migrator (#3284)
liuh-80 Apr 29, 2024
07f06a3
Update sonic-utilities to support new SKU Mellanox-SN5600-O128 (#3236)
stephenxs Apr 29, 2024
a81601b
Add Precommit and flake8 (#3287)
maipbui Apr 29, 2024
0fb743c
Run `ip neigh flush` before removing the IP address from the interfac…
saiarcot895 Apr 29, 2024
bbce555
[Mellanpx] Update SDK Sniffer default folder (#3276)
dprital May 7, 2024
aee2d04
Switchport Mode & CLI Modified Fix (#3247)
sabakram May 10, 2024
7f84136
Support ASIC/SDK health event (#3129)
stephenxs May 13, 2024
aa5f593
Migrate AAA table per-command authorization in db_migrator (#3296)
liuh-80 May 15, 2024
d6517f7
ThirdPartyContainerManagement(TPCM)_in_SonicPackageManager (#2815)
sg893052 May 15, 2024
453191f
Add LDAP feature CLI support (#3022)
davidpil2002 May 15, 2024
e8c0be2
[chassis][midplane] Add notification to Supervisor when LC is gracefu…
mlok-nokia May 15, 2024
d40ec4f
Add new cli to add a interface ip as secondary address (#3016)
shbalaku-microsoft May 15, 2024
bd63ad1
[show] Show running config when bgp is down (#3313)
wen587 May 16, 2024
c401b57
Add full configuration validation. (#3316)
xincunli-sonic May 17, 2024
12add72
[config]Improve config save cli to save to one file for multiasic (#3…
wen587 May 17, 2024
87f3d22
[chassis] Add show fabric rate command (#3297)
jfeng-arista May 20, 2024
f80c129
Modified reboot scripts to sync FSIO reads/writes to disk before OS-l…
assrinivasan May 21, 2024
9c7b62a
[Mellanox] Update sonic-utilities to support new SKU Mellanox-SN5600-…
DavidZagury May 22, 2024
564ba82
[sonic-package-manager] remove leftovers from featured on uninstall (…
stepanblyschak May 22, 2024
8373306
[build] Fix base OS compilation issue caused by incompatibility betwe…
oleksandrivantsiv May 23, 2024
67e044c
[Mellanox] Update NVIDIA header for "config/plugins/mlnx.py" file (#3…
dprital May 28, 2024
6858d5f
Update for the procedures for insertion/hot swap of Switch Fabric Mod…
JunhongMao May 29, 2024
8031686
Backup STATE_DB PORT_TABLE|Ethernet during warm-reboot (#3111)
mihirpat1 May 30, 2024
5eebb6f
[ci] Migrate ubuntu-20.04 agent pool to sonic-ubuntu-1c to fix S360 a…
liushilongbuaa May 31, 2024
84bd55d
[chassis][multi-asic] Make PFC commands use a class (#3057)
bktsim-arista May 31, 2024
208b42c
Rename sonic_ssd to sonic_storage matching corresponding sonic-platfo…
assrinivasan Jun 3, 2024
175098e
Add a check for ensuring mirror session ACLs are programmed to ASIC (…
ryanzhu706 Jun 3, 2024
bae59bd
Don't exit immediately if running a command under alias mode (#3353)
saiarcot895 Jun 4, 2024
6fdf8a3
[chassis][voq] Added support for Voq Counters(SAI_SWITCH_STAT_PACKET_…
saksarav-nokia Jun 5, 2024
aade985
Add W-ECMP CLI (#3253)
nazariig Jun 5, 2024
eaf75af
Fix show fabric monitor capacity command when the feature is disabled…
jfeng-arista Jun 5, 2024
49fc4cd
[consutil] Fix consule CLI and enhance unittest (#3360)
lizhijianrd Jun 9, 2024
2d33c0b
[DPB]Fixing return code of breakout command on failure (#3357)
dgsudharsan Jun 10, 2024
cdced93
Add Checkpoint and Rollback for Multi ASIC. (#3299)
xincunli-sonic Jun 14, 2024
e53989e
Validate interface name length in CLI
arfeigin Apr 3, 2024
f887ad8
Use sonic-swss-common interface name length validation
arfeigin May 30, 2024
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
25 changes: 14 additions & 11 deletions config/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
from utilities_common.intf_filter import parse_interface_in_filter
from utilities_common import bgp_util
import utilities_common.cli as clicommon
from utilities_common.helper import get_port_pbh_binding, get_port_acl_binding, update_config
from utilities_common.helper import get_port_pbh_binding, get_port_acl_binding, update_config, validate_interface_name_length
from utilities_common.general import load_db_config, load_module_from_source
from .validated_config_db_connector import ValidatedConfigDBConnector
import utilities_common.multi_asic as multi_asic_util
Expand Down Expand Up @@ -97,10 +97,11 @@

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>"

IFNAMSIZ = 16
arfeigin marked this conversation as resolved.
Show resolved Hide resolved

PORT_MTU = "mtu"
PORT_SPEED = "speed"
PORT_TPID = "tpid"
Expand Down Expand Up @@ -423,7 +424,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 validate_interface_name_length(portchannel_name) is False:
arfeigin marked this conversation as resolved.
Show resolved Hide resolved
return False
return True

Expand Down Expand Up @@ -2168,8 +2169,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 @@ -5549,8 +5550,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 validate_interface_name_length(vrf_name) is False:
arfeigin marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -5569,8 +5570,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 validate_interface_name_length(vrf_name) is False:
arfeigin marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -6574,8 +6575,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 @@ -7322,6 +7323,8 @@ def add_subinterface(ctx, subinterface_name, vid):

if interface_alias is None:
ctx.fail("{} invalid subinterface".format(interface_alias))
if validate_interface_name_length(subinterface_name) is False:
arfeigin marked this conversation as resolved.
Show resolved Hide resolved
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
3 changes: 3 additions & 0 deletions config/vxlan.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from .validated_config_db_connector import ValidatedConfigDBConnector

ADHOC_VALIDATION = True
IFNAMSIZ = 16
#
# 'vxlan' group ('config vxlan ...')
#
Expand All @@ -24,6 +25,8 @@ 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))
if len(vxlan_name) >= IFNAMSIZ:
ctx.fail("'vxlan_name' length should not exceed {} characters".format(IFNAMSIZ))
arfeigin marked this conversation as resolved.
Show resolved Hide resolved

vxlan_keys = db.cfgdb.get_keys('VXLAN_TUNNEL')
if not vxlan_keys:
Expand Down
8 changes: 7 additions & 1 deletion tests/config_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -2103,7 +2103,13 @@ def test_add_loopback_with_invalid_name_adhoc_validation(self):
print(result.exit_code)
print(result.output)
assert result.exit_code != 0
assert "Error: Loopbax1 is invalid, name should have prefix 'Loopback' and suffix '<0-999>'" in result.output
assert "Error: Loopbax1 is invalid, name should have prefix 'Loopback' and suffix '<0-999>' and should not exceed 11 characters" 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
Expand Down
8 changes: 7 additions & 1 deletion tests/portchannel_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
11 changes: 0 additions & 11 deletions tests/subintf_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,22 +60,11 @@ def test_add_del_subintf_with_long_name(self):
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'

arfeigin marked this conversation as resolved.
Show resolved Hide resolved
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')

arfeigin marked this conversation as resolved.
Show resolved Hide resolved

def test_add_existing_subintf_again(self):
runner = CliRunner()
Expand Down
8 changes: 8 additions & 0 deletions tests/vlan_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,14 @@ def test_config_vlan_add_member_with_invalid_vlanid(self):
print(result.output)
assert result.exit_code != 0
assert "Error: Invalid VLAN ID 4096 (1-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()
Expand Down
8 changes: 8 additions & 0 deletions tests/vrf_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -267,3 +267,11 @@ def test_invalid_vrf_name(self):
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
9 changes: 9 additions & 0 deletions utilities_common/helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
from .db import Db
import copy

IFNAMSIZ = 16

def get_port_acl_binding(db_wrap, port, ns):
"""
Verify if the port is not bound to any ACL Table
Expand Down Expand Up @@ -35,6 +37,13 @@ def get_port_acl_binding(db_wrap, port, ns):
return acl_tables


def validate_interface_name_length(iface_name):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arfeigin why do we need this part? Isn't it was moved to swss common lib?

"""
Verify that interface name length does not exceed IFNAMSIZ
"""
return True if len(iface_name) >= IFNAMSIZ else False
arfeigin marked this conversation as resolved.
Show resolved Hide resolved
arfeigin marked this conversation as resolved.
Show resolved Hide resolved


def get_port_pbh_binding(db_wrap, port, ns):
"""
Verify if the port is not bound to any PBH Table
Expand Down
Loading