Add golden config check (sonic-net#3770)

* Add golden config check

* fix format

* Addressing comment.

* fix tests.

* fix format

* fix common function

* Fix UT

* fix test

* reset mock

* fix ut

* fix ut

* remove unnecessary test

* Add negative test

* fix format

* remove empty dict check

* Add none check

* fix condition

* fix invalid golden

Author: xincunli-sonic

config/main.py

@@ -1392,26 +1392,41 @@ def multiasic_write_to_db(filename, load_sysinfo):
 
 def config_file_yang_validation(filename):
     config = read_json_file(filename)
+
+    # Check if the config is not a dictionary
+    if not isinstance(config, dict):
+        return False
+
+    # If the device is multi-ASIC, check if all required namespaces exist
+    if multi_asic.is_multi_asic():
+        required_namespaces = [HOST_NAMESPACE, *multi_asic.get_namespace_list()]
+        for ns in required_namespaces:
+            asic_name = HOST_NAMESPACE if ns == DEFAULT_NAMESPACE else ns
+            if asic_name not in config:
+                return False
+
     sy = sonic_yang.SonicYang(YANG_DIR)
     sy.loadYangModel()
     asic_list = [HOST_NAMESPACE]
     if multi_asic.is_multi_asic():
         asic_list.extend(multi_asic.get_namespace_list())
     for scope in asic_list:
         config_to_check = config.get(scope) if multi_asic.is_multi_asic() else config
-        try:
-            sy.loadData(configdbJson=config_to_check)
-            sy.validate_data_tree()
-        except sonic_yang.SonicYangException as e:
-            click.secho("{} fails YANG validation! Error: {}".format(filename, str(e)),
-                        fg='magenta')
-            raise click.Abort()
+        if config_to_check:
+            try:
+                sy.loadData(configdbJson=config_to_check)
+                sy.validate_data_tree()
+            except sonic_yang.SonicYangException as e:
+                click.secho("{} fails YANG validation! Error: {}".format(filename, str(e)),
+                            fg='magenta')
+                raise click.Abort()
 
         sy.tablesWithOutYang.pop('bgpraw', None)
         if len(sy.tablesWithOutYang):
             click.secho("Config tables are missing yang models: {}".format(str(sy.tablesWithOutYang.keys())),
                         fg='magenta')
             raise click.Abort()
+    return True
 
 
 # This is our main entrypoint - the main 'config' command
@@ -2048,7 +2063,10 @@ def load_minigraph(db, no_service_restart, traffic_shift_away, override_config,
                         fg='magenta')
             raise click.Abort()
 
-        config_file_yang_validation(golden_config_path)
+        if not config_file_yang_validation(golden_config_path):
+            click.secho("Invalid golden config file:'{}'!".format(golden_config_path),
+                        fg='magenta')
+            raise click.Abort()
 
         config_to_check = read_json_file(golden_config_path)
         # Dependency check golden config json
@@ -2057,7 +2075,8 @@ def load_minigraph(db, no_service_restart, traffic_shift_away, override_config,
             asic_list.extend(multi_asic.get_namespace_list())
         for scope in asic_list:
             host_config = config_to_check.get(scope) if multi_asic.is_multi_asic() else config_to_check
-            table_hard_dependency_check(host_config)
+            if host_config:
+                table_hard_dependency_check(host_config)
 
     # Stop services before config push
     if not no_service_restart:

tests/config_test.py

@@ -27,6 +27,7 @@
 
 import config.main as config
 import config.validated_config_db_connector as validated_config_db_connector
+from config.main import config_file_yang_validation
 
 # Add Test, module and script path.
 test_path = os.path.dirname(os.path.abspath(__file__))
@@ -966,6 +967,16 @@ def setup_class(cls):
         import config.main
         importlib.reload(config.main)
 
+    def read_json_file_side_effect(self, filename):
+        return {
+            'DEVICE_METADATA': {
+                'localhost': {
+                    'platform': 'x86_64-mlnx_msn2700-r0',
+                    'mac': '00:02:03:04:05:07'
+                }
+            }
+        }
+
     @mock.patch('sonic_py_common.device_info.get_paths_to_platform_and_hwsku_dirs', mock.MagicMock(return_value=("dummy_path", None)))
     @mock.patch('config.main.subprocess.check_call')
     def test_load_minigraph(self, mock_check_call, get_cmd_module, setup_single_broadcom_asic):
@@ -1135,12 +1146,9 @@ 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('config.main.read_json_file', mock.MagicMock(side_effect=read_json_file_side_effect)):
+                mock.patch('config.main.read_json_file', mock.MagicMock(side_effect=self.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"])
@@ -1152,18 +1160,32 @@ 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('config.main.read_json_file', mock.MagicMock(side_effect=read_json_file_side_effect)):
+                mock.patch('config.main.read_json_file', mock.MagicMock(side_effect=self.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_with_invalid_golden_config(self, get_cmd_module):
+        def is_file_side_effect(filename):
+            return True if 'golden_config' in filename else False
+
+        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(return_value=[])):
+            (config, show) = get_cmd_module
+            runner = CliRunner()
+            result = runner.invoke(config.config.commands["load_minigraph"], ["--override_config", "-y"])
+            assert result.exit_code != 0
+            assert "Invalid golden config file:" 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):
@@ -1234,11 +1256,9 @@ def test_load_minigraph_with_traffic_shift_away_with_golden_config(self, get_cmd
             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('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)):
+                    mock.patch('config.main.read_json_file', mock.MagicMock(
+                        side_effect=self.read_json_file_side_effect)):
                 (config, show) = get_cmd_module
                 db = Db()
                 golden_config = {}
@@ -1251,6 +1271,51 @@ def read_json_file_side_effect(filename):
                 assert "TSA" in result.output
                 assert "[WARNING] Golden configuration may override Traffic-shift-away state" in result.output
 
+    def test_config_file_yang_validation(self):
+        # Test with empty config
+        with mock.patch('config.main.read_json_file', return_value="") as mock_read_json_file:
+            with mock.patch('config.main.sonic_yang.SonicYang.loadYangModel') as mock_load_yang_model:
+                assert not config_file_yang_validation('dummy_file.json')
+                mock_read_json_file.assert_called_once_with('dummy_file.json')
+                mock_load_yang_model.assert_not_called()
+
+        # Test with non-dict config
+        with mock.patch('config.main.read_json_file', return_value=[]) as mock_read_json_file:
+            with mock.patch('config.main.sonic_yang.SonicYang.loadYangModel') as mock_load_yang_model:
+                assert not config_file_yang_validation('dummy_file.json')
+                mock_read_json_file.assert_called_once_with('dummy_file.json')
+                mock_load_yang_model.assert_not_called()
+
+        # Test with missing namespaces in multi-ASIC config
+        with mock.patch('config.main.read_json_file', return_value={'localhost': {}}) as mock_read_json_file:
+            with mock.patch('config.main.multi_asic.is_multi_asic', return_value=True):
+                with mock.patch('config.main.multi_asic.get_namespace_list', return_value=['asic0', 'asic1']):
+                    with mock.patch('config.main.sonic_yang.SonicYang.loadYangModel') as mock_load_yang_model:
+                        assert not config_file_yang_validation('dummy_file.json')
+                        mock_read_json_file.assert_called_once_with('dummy_file.json')
+                        mock_load_yang_model.assert_not_called()
+
+        # Test with valid config
+        valid_config = {
+                'DEVICE_METADATA': {
+                    'localhost': {
+                        'platform': 'x86_64-mlnx_msn2700-r0',
+                        'mac': '00:02:03:04:05:07'
+                    }
+                }
+            }
+
+        with mock.patch('config.main.read_json_file', return_value=valid_config) as mock_read_json_file, \
+                mock.patch('config.main.multi_asic.is_multi_asic', return_value=False), \
+                mock.patch('config.main.sonic_yang.SonicYang.loadYangModel') as mock_load_yang_model, \
+                mock.patch('config.main.sonic_yang.SonicYang.loadData') as mock_load_data, \
+                mock.patch('config.main.sonic_yang.SonicYang.validate_data_tree') as mock_validate_data_tree:
+            assert config_file_yang_validation('dummy_file.json')
+            mock_read_json_file.assert_called_once_with('dummy_file.json')
+            mock_load_yang_model.assert_called_once()
+            mock_load_data.assert_called_once_with(configdbJson=valid_config)
+            mock_validate_data_tree.assert_called_once()
+
     @classmethod
     def teardown_class(cls):
         os.environ['UTILITIES_UNIT_TESTING'] = "0"