Exit early if YANG validation fails in Golden Config (sonic-net#3490)

What I did Exit early if golden config fails YANG validation How I did it Check before stop service in load_minigraph How to verify it Unit test Previous command output (if the output of
mssonicbld · Sep 5, 2024 · 70e43b9 · 70e43b9
1 parent 1481250
commit 70e43b9
Show file tree

Hide file tree

Showing 5 changed files with 44 additions and 107 deletions.
diff --git a/config/main.py b/config/main.py
@@ -15,6 +15,7 @@
 import itertools
 import copy
 import tempfile
+import sonic_yang
 
 from jsonpatch import JsonPatchConflict
 from jsonpointer import JsonPointerException
@@ -57,7 +58,7 @@
 from . import vlan
 from . import vxlan
 from . import plugins
-from .config_mgmt import ConfigMgmtDPB, ConfigMgmt
+from .config_mgmt import ConfigMgmtDPB, ConfigMgmt, YANG_DIR
 from . import mclag
 from . import syslog
 from . import switchport
@@ -1964,8 +1965,22 @@ 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():
+            # Multiasic has not 100% fully validated. Thus pass here.
+            pass
+        else:
+            sy = sonic_yang.SonicYang(YANG_DIR)
+            sy.loadYangModel()
+            try:
+                sy.loadData(configdbJson=config_to_check)
+                sy.validate_data_tree()
+            except sonic_yang.SonicYangException as e:
+                click.secho("{} fails YANG validation! Error: {}".format(golden_config_path, str(e)),
+                            fg='magenta')
+                raise click.Abort()
+
+        # Dependency check golden config json
         if multi_asic.is_multi_asic():
             host_config = config_to_check.get('localhost', {})
         else:
@@ -2292,7 +2307,7 @@ def aaa_table_hard_dependency_check(config_json):
     tacacs_enable = "tacacs+" in aaa_authentication_login.split(",")
     tacplus_passkey = TACPLUS_TABLE.get("global", {}).get("passkey", "")
     if tacacs_enable and len(tacplus_passkey) == 0:
-        click.secho("Authentication with 'tacacs+' is not allowed when passkey not exits.", fg="magenta")
+        click.secho("Authentication with 'tacacs+' is not allowed when passkey not exists.", fg="magenta")
         sys.exit(1)
 
 

diff --git a/tests/config_override_input/golden_input_yang_failure.json b/tests/config_override_input/golden_input_yang_failure.json
diff --git a/tests/config_override_input/partial_config_override.json b/tests/config_override_input/partial_config_override.json
@@ -71,6 +71,30 @@
                 "stage": "ingress",
                 "type": "CTRLPLANE"
             }
+        },
+        "PORT": {
+            "Ethernet4": {
+                "admin_status": "up",
+                "alias": "fortyGigE0/4",
+                "description": "Servers0:eth0",
+                "index": "1",
+                "lanes": "29,30,31,32",
+                "mtu": "9100",
+                "pfc_asym": "off",
+                "speed": "40000",
+                "tpid": "0x8100"
+            },
+            "Ethernet8": {
+                "admin_status": "up",
+                "alias": "fortyGigE0/8",
+                "description": "Servers1:eth0",
+                "index": "2",
+                "lanes": "33,34,35,36",
+                "mtu": "9100",
+                "pfc_asym": "off",
+                "speed": "40000",
+                "tpid": "0x8100"
+            }
         }
     },
     "expected_config": {

diff --git a/tests/config_override_test.py b/tests/config_override_test.py
@@ -20,7 +20,6 @@
 EMPTY_TABLE_REMOVAL = os.path.join(DATA_DIR, "empty_table_removal.json")
 AAA_YANG_HARD_CHECK = os.path.join(DATA_DIR, "aaa_yang_hard_check.json")
 RUNNING_CONFIG_YANG_FAILURE = os.path.join(DATA_DIR, "running_config_yang_failure.json")
-GOLDEN_INPUT_YANG_FAILURE = os.path.join(DATA_DIR, "golden_input_yang_failure.json")
 FINAL_CONFIG_YANG_FAILURE = os.path.join(DATA_DIR, "final_config_yang_failure.json")
 MULTI_ASIC_MACSEC_OV = os.path.join(DATA_DIR, "multi_asic_macsec_ov.json")
 MULTI_ASIC_FEATURE_RM = os.path.join(DATA_DIR, "multi_asic_feature_rm.json")
@@ -179,7 +178,7 @@ def read_json_file_side_effect(filename):
                                        ['golden_config_db.json'], obj=db)
 
                 assert result.exit_code != 0
-                assert "Authentication with 'tacacs+' is not allowed when passkey not exits." in result.output
+                assert "Authentication with 'tacacs+' is not allowed when passkey not exists." in result.output
 
     def check_override_config_table(self, db, config, running_config,
                                     golden_config, expected_config):
@@ -233,17 +232,6 @@ def is_yang_config_validation_enabled_side_effect(filename):
             self.check_yang_verification_failure(
                 db, config, read_data['running_config'], read_data['golden_config'], "running config")
 
-    def test_golden_input_yang_failure(self):
-        def is_yang_config_validation_enabled_side_effect(filename):
-            return True
-        db = Db()
-        with open(GOLDEN_INPUT_YANG_FAILURE, "r") as f:
-            read_data = json.load(f)
-        with mock.patch('config.main.device_info.is_yang_config_validation_enabled',
-                        mock.MagicMock(side_effect=is_yang_config_validation_enabled_side_effect)):
-            self.check_yang_verification_failure(
-                db, config, read_data['running_config'], read_data['golden_config'], "config_input")
-
     def test_final_config_yang_failure(self):
         def is_yang_config_validation_enabled_side_effect(filename):
             return True

diff --git a/tests/config_test.py b/tests/config_test.py
@@ -1128,7 +1128,6 @@ def read_json_file_side_effect(filename):
                 },
                 "TACPLUS": {
                     "global": {
-                        "passkey": ""
                     }
                 }
             }
@@ -1140,7 +1139,7 @@ def read_json_file_side_effect(filename):
             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
+            assert "Authentication with 'tacacs+' is not allowed when passkey not exists." 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):