Skip to content

Commit

Permalink
sonic-utilities: Update config reload() to verify formatting of an in…
Browse files Browse the repository at this point in the history 
…put file.

sonic-utilities: bugfix-9499
* Update config/main.py to verify if a config input file is properly formatted before writing it into confib_db
* Update tests/config_test.py to include test cases for invalid input file
* Add tests/config_reload_input/config_db_invalid.json as invalid input file used in tests/config_test.py

Signed-off-by: [email protected]
  • Loading branch information
@cchoate54
cchoate54 committed Nov 28, 2022
1 parent addae73 commit 46d3024
Show file tree
Hide file tree
Showing 3 changed files with 129 additions and 2 deletions.
9 changes: 9 additions & 0 deletions config/main.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1547,6 +1547,15 @@ def reload(db, filename, yes, load_sysinfo, no_service_restart, force, file_form
if len(cfg_files) != num_cfg_file:
click.echo("Input {} config file(s) separated by comma for multiple files ".format(num_cfg_file))
return

# Check if the file is properly formatted before proceeding.
for file in cfg_files:
try:
config_input = read_json_file(file)
except Exception as e:
click.secho("Bad format: json file broken. {}".format(str(e)),
fg='magenta')
sys.exit(1)

#Stop services before config push
if not no_service_restart:
Expand Down
62 changes: 62 additions & 0 deletions tests/config_reload_input/config_db_invalid.json
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
{
"DEVICE_METADATA": {
"localhost": {
"docker_routing_config_mode": "split",
"hostname": "sonic",
"hwsku": "Seastone-DX010-25-50",
"mac": "00:e0:ec:89:6e:48",
"platform": "x86_64-cel_seastone-r0",
"type": "ToRRouter"
}
}
"VLAN_MEMBER": {
"Vlan1000|Ethernet0": {
"tagging_mode": "untagged",
},
"Vlan1000|Ethernet4": {
"tagging_mode": "untagged"
},
"Vlan1000|Ethernet8": {
"tagging_mode": "untagged"
}
},
"VLAN": {
"Vlan1000": {
"vlanid": "1000",
"dhcp_servers": [
"192.0.0.1",
"192.0.0.2",
"192.0.0.3",
"192.0.0.4"
]
}
},
"PORT": {
"Ethernet0": {
"alias": "Eth1",
"lanes": "65, 66, 67, 68",
"description": "Ethernet0 100G link",
"speed": "100000"
},
"Ethernet4": {
"admin_status": "up",
"alias": "fortyGigE0/4",
"description": "Servers0:eth0",
"index": "1",
"lanes": "29,30,31,32",
"mtu": "9100",
"pfc_asym": "off",
"speed": "40000"
},
"Ethernet8": {
"admin_status": "up",
"alias": "fortyGigE0/8",
"description": "Servers1:eth0",
"index": "2",
"lanes": "33,34,35,36",
"mtu": "9100",
"pfc_asym": "off",
"speed": "40000"
}
}
}
60 changes: 58 additions & 2 deletions tests/config_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import traceback
import json
import jsonpatch
import shutil
import sys
import unittest
import ipaddress
Expand Down Expand Up @@ -88,6 +89,10 @@
Reloading Monit configuration ...
"""

RELOAD_CONFIG_DB_OUTPUT_INVALID = """\
Bad format: json file broken. Expecting ',' delimiter: line 12 column 5 (char 321)
"""

RELOAD_YANG_CFG_OUTPUT = """\
Stopping SONiC target ...
Running command: /usr/local/bin/sonic-cfggen -Y /tmp/config.json --write-to-db
Expand All @@ -104,6 +109,10 @@
Reloading Monit configuration ...
"""

RELOAD_MASIC_CONFIG_DB_OUTPUT_INVALID = """\
Bad format: json file broken. Expecting ',' delimiter: line 12 column 5 (char 321)
"""

reload_config_with_sys_info_command_output="""\
Running command: /usr/local/bin/sonic-cfggen -H -k Seastone-DX010-25-50 --write-to-db"""

Expand Down Expand Up @@ -195,6 +204,7 @@ def mock_run_command_side_effect_gnmi(*args, **kwargs):

class TestConfigReload(object):
dummy_cfg_file = os.path.join(os.sep, "tmp", "config.json")
dummy_cfg_file_contents = os.path.join(mock_db_path, "config_db.json")

@classmethod
def setup_class(cls):
Expand All @@ -206,7 +216,8 @@ def setup_class(cls):

import config.main
importlib.reload(config.main)
open(cls.dummy_cfg_file, 'w').close()
shutil.copyfile(cls.dummy_cfg_file_contents, cls.dummy_cfg_file)
open(cls.dummy_cfg_file, 'r').close()

def test_config_reload(self, get_cmd_module, setup_single_broadcom_asic):
with mock.patch("utilities_common.cli.run_command", mock.MagicMock(side_effect=mock_run_command_side_effect)) as mock_run_command:
Expand Down Expand Up @@ -479,14 +490,17 @@ def teardown_class(cls):

class TestReloadConfig(object):
dummy_cfg_file = os.path.join(os.sep, "tmp", "config.json")
dummy_cfg_file_contents = os.path.join(mock_db_path, "config_db.json")
dummy_cfg_file_invalid = os.path.join(mock_db_path, "config_db_invalid.json")

@classmethod
def setup_class(cls):
os.environ['UTILITIES_UNIT_TESTING'] = "1"
print("SETUP")
import config.main
importlib.reload(config.main)
open(cls.dummy_cfg_file, 'w').close()
shutil.copyfile(cls.dummy_cfg_file_contents, cls.dummy_cfg_file)
open(cls.dummy_cfg_file, 'r').close()

def test_reload_config(self, get_cmd_module, setup_single_broadcom_asic):
with mock.patch(
Expand All @@ -507,6 +521,25 @@ def test_reload_config(self, get_cmd_module, setup_single_broadcom_asic):
assert "\n".join([l.rstrip() for l in result.output.split('\n')]) \
== RELOAD_CONFIG_DB_OUTPUT

def test_reload_config_invalid_config_file(self, get_cmd_module, setup_single_broadcom_asic):
with mock.patch(
"utilities_common.cli.run_command",
mock.MagicMock(side_effect=mock_run_command_side_effect)
) as mock_run_command:
(config, show) = get_cmd_module
runner = CliRunner()

result = runner.invoke(
config.config.commands["reload"],
[self.dummy_cfg_file_invalid, '-y', '-f', "--disable_arp_cache"])

print(result.exit_code)
print(result.output)
traceback.print_tb(result.exc_info[2])
assert result.exit_code == 1
assert "\n".join([l.rstrip() for l in result.output.split('\n')]) \
== RELOAD_CONFIG_DB_OUTPUT_INVALID

def test_config_reload_disabled_service(self, get_cmd_module, setup_single_broadcom_asic):
with mock.patch(
"utilities_common.cli.run_command",
Expand Down Expand Up @@ -549,6 +582,29 @@ def test_reload_config_masic(self, get_cmd_module, setup_multi_broadcom_masic):
assert "\n".join([l.rstrip() for l in result.output.split('\n')]) \
== RELOAD_MASIC_CONFIG_DB_OUTPUT

def test_reload_config_masic_invalid(self, get_cmd_module, setup_multi_broadcom_masic):
with mock.patch(
"utilities_common.cli.run_command",
mock.MagicMock(side_effect=mock_run_command_side_effect)
) as mock_run_command:
(config, show) = get_cmd_module
runner = CliRunner()
# 3 config files: 1 for host and 2 for asic
cfg_files = "{},{},{}".format(
self.dummy_cfg_file,
self.dummy_cfg_file_invalid,
self.dummy_cfg_file)
result = runner.invoke(
config.config.commands["reload"],
[cfg_files, '-y', '-f', "--disable_arp_cache"])

print(result.exit_code)
print(result.output)
traceback.print_tb(result.exc_info[2])
assert result.exit_code == 1
assert "\n".join([l.rstrip() for l in result.output.split('\n')]) \
== RELOAD_MASIC_CONFIG_DB_OUTPUT_INVALID

def test_reload_yang_config(self, get_cmd_module,
setup_single_broadcom_asic):
with mock.patch(
Expand Down

0 comments on commit 46d3024

Please sign in to comment.