From 2e9009e66247c700e2c97a4d47e3a93cbf8e7e38 Mon Sep 17 00:00:00 2001 From: jingwenxie Date: Tue, 18 Jun 2024 12:37:49 +0800 Subject: [PATCH] [config] no op if Golden Config is invalid (#3367) ADO: 27941719 What I did Improve Golden Config workflow and make sure no op if invalid config detected How I did it Add table dependency check right after Golden Config path is enabled How to verify it Unit test --- config/main.py | 8 +++++++ tests/config_test.py | 50 +++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/config/main.py b/config/main.py index 2833a8b535..01d93dff42 100644 --- a/config/main.py +++ b/config/main.py @@ -1953,6 +1953,14 @@ def load_minigraph(db, no_service_restart, traffic_shift_away, override_config, fg='magenta') raise click.Abort() + # Dependency check golden config json + config_to_check = read_json_file(golden_config_path) + if multi_asic.is_multi_asic(): + host_config = config_to_check.get('localhost', {}) + else: + host_config = config_to_check + table_hard_dependency_check(host_config) + #Stop services before config push if not no_service_restart: log.log_notice("'load_minigraph' stopping services...") diff --git a/tests/config_test.py b/tests/config_test.py index 00c9847a38..db62bf3249 100644 --- a/tests/config_test.py +++ b/tests/config_test.py @@ -987,8 +987,13 @@ def is_file_side_effect(filename): def test_load_minigraph_with_specified_golden_config_path(self, get_cmd_module): def is_file_side_effect(filename): return True if 'golden_config' in filename else False + + def read_json_file_side_effect(filename): + return {} + with mock.patch("utilities_common.cli.run_command", mock.MagicMock(side_effect=mock_run_command_side_effect)) as mock_run_command, \ - mock.patch('os.path.isfile', mock.MagicMock(side_effect=is_file_side_effect)): + mock.patch('os.path.isfile', mock.MagicMock(side_effect=is_file_side_effect)), \ + mock.patch('config.main.read_json_file', mock.MagicMock(side_effect=read_json_file_side_effect)): (config, show) = get_cmd_module runner = CliRunner() result = runner.invoke(config.config.commands["load_minigraph"], ["--override_config", "--golden_config_path", "golden_config.json", "-y"]) @@ -999,14 +1004,48 @@ def is_file_side_effect(filename): def test_load_minigraph_with_default_golden_config_path(self, get_cmd_module): def is_file_side_effect(filename): return True if 'golden_config' in filename else False + + def read_json_file_side_effect(filename): + return {} + with mock.patch("utilities_common.cli.run_command", mock.MagicMock(side_effect=mock_run_command_side_effect)) as mock_run_command, \ - mock.patch('os.path.isfile', mock.MagicMock(side_effect=is_file_side_effect)): + mock.patch('os.path.isfile', mock.MagicMock(side_effect=is_file_side_effect)), \ + mock.patch('config.main.read_json_file', mock.MagicMock(side_effect=read_json_file_side_effect)): (config, show) = get_cmd_module runner = CliRunner() result = runner.invoke(config.config.commands["load_minigraph"], ["--override_config", "-y"]) assert result.exit_code == 0 assert "config override-config-table /etc/sonic/golden_config_db.json" in result.output + @mock.patch('sonic_py_common.device_info.get_paths_to_platform_and_hwsku_dirs', + mock.MagicMock(return_value=("dummy_path", None))) + def test_load_minigraph_hard_dependency_check(self, get_cmd_module): + def is_file_side_effect(filename): + return True if 'golden_config' in filename else False + + def read_json_file_side_effect(filename): + return { + "AAA": { + "authentication": { + "login": "tacacs+" + } + }, + "TACPLUS": { + "global": { + "passkey": "" + } + } + } + + with mock.patch("utilities_common.cli.run_command", mock.MagicMock(side_effect=mock_run_command_side_effect)), \ + mock.patch('os.path.isfile', mock.MagicMock(side_effect=is_file_side_effect)), \ + mock.patch('config.main.read_json_file', mock.MagicMock(side_effect=read_json_file_side_effect)): + (config, _) = get_cmd_module + runner = CliRunner() + result = runner.invoke(config.config.commands["load_minigraph"], ["--override_config", "-y"]) + assert result.exit_code != 0 + assert "Authentication with 'tacacs+' is not allowed when passkey not exits." in result.output + @mock.patch('sonic_py_common.device_info.get_paths_to_platform_and_hwsku_dirs', mock.MagicMock(return_value=("dummy_path", None))) def test_load_minigraph_with_traffic_shift_away(self, get_cmd_module): with mock.patch("utilities_common.cli.run_command", mock.MagicMock(side_effect=mock_run_command_side_effect)) as mock_run_command: @@ -1024,7 +1063,12 @@ def test_load_minigraph_with_traffic_shift_away_with_golden_config(self, get_cmd with mock.patch("utilities_common.cli.run_command", mock.MagicMock(side_effect=mock_run_command_side_effect)) as mock_run_command: def is_file_side_effect(filename): return True if 'golden_config' in filename else False - with mock.patch('os.path.isfile', mock.MagicMock(side_effect=is_file_side_effect)): + + def read_json_file_side_effect(filename): + return {} + + with mock.patch('os.path.isfile', mock.MagicMock(side_effect=is_file_side_effect)), \ + mock.patch('config.main.read_json_file', mock.MagicMock(side_effect=read_json_file_side_effect)): (config, show) = get_cmd_module db = Db() golden_config = {}