YANG validation for ConfigDB Updates: MIRROR_SESSION use case (#2430)

sonic-net · Mar 7, 2023 · 2fc2b82 · 2fc2b82
1 parent e16bdaa
commit 2fc2b82
Show file tree

Hide file tree

Showing 3 changed files with 129 additions and 20 deletions.
diff --git a/config/main.py b/config/main.py
@@ -2354,25 +2354,35 @@ def add_erspan(session_name, src_ip, dst_ip, dscp, ttl, gre_type, queue, policer
         session_info['gre_type'] = gre_type
 
     session_info = gather_session_info(session_info, policer, queue, src_port, direction)
+    ctx = click.get_current_context()
 
     """
     For multi-npu platforms we need to program all front asic namespaces
     """
     namespaces = multi_asic.get_all_namespaces()
     if not namespaces['front_ns']:
-        config_db = ConfigDBConnector()
+        config_db = ValidatedConfigDBConnector(ConfigDBConnector())
         config_db.connect()
-        if validate_mirror_session_config(config_db, session_name, None, src_port, direction) is False:
-            return
-        config_db.set_entry("MIRROR_SESSION", session_name, session_info)
+        if ADHOC_VALIDATION: 
+            if validate_mirror_session_config(config_db, session_name, None, src_port, direction) is False:
+                return
+        try:
+            config_db.set_entry("MIRROR_SESSION", session_name, session_info)
+        except ValueError as e:
+            ctx.fail("Invalid ConfigDB. Error: {}".format(e))
+
     else:
         per_npu_configdb = {}
         for front_asic_namespaces in namespaces['front_ns']:
-            per_npu_configdb[front_asic_namespaces] = ConfigDBConnector(use_unix_socket_path=True, namespace=front_asic_namespaces)
+            per_npu_configdb[front_asic_namespaces] = ValidatedConfigDBConnector(ConfigDBConnector(use_unix_socket_path=True, namespace=front_asic_namespaces))
             per_npu_configdb[front_asic_namespaces].connect()
-            if validate_mirror_session_config(per_npu_configdb[front_asic_namespaces], session_name, None, src_port, direction) is False:
-                return
-            per_npu_configdb[front_asic_namespaces].set_entry("MIRROR_SESSION", session_name, session_info)
+            if ADHOC_VALIDATION:
+                if validate_mirror_session_config(per_npu_configdb[front_asic_namespaces], session_name, None, src_port, direction) is False:
+                    return
+            try:
+                per_npu_configdb[front_asic_namespaces].set_entry("MIRROR_SESSION", session_name, session_info)
+            except ValueError as e:
+                ctx.fail("Invalid ConfigDB. Error: {}".format(e))
 
 @mirror_session.group(cls=clicommon.AbbreviationGroup, name='span')
 @click.pass_context
@@ -2404,25 +2414,34 @@ def add_span(session_name, dst_port, src_port, direction, queue, policer):
             }
 
     session_info = gather_session_info(session_info, policer, queue, src_port, direction)
+    ctx = click.get_current_context()
 
     """
     For multi-npu platforms we need to program all front asic namespaces
     """
     namespaces = multi_asic.get_all_namespaces()
     if not namespaces['front_ns']:
-        config_db = ConfigDBConnector()
+        config_db = ValidatedConfigDBConnector(ConfigDBConnector())
         config_db.connect()
-        if validate_mirror_session_config(config_db, session_name, dst_port, src_port, direction) is False:
-            return
-        config_db.set_entry("MIRROR_SESSION", session_name, session_info)
+        if ADHOC_VALIDATION:
+            if validate_mirror_session_config(config_db, session_name, dst_port, src_port, direction) is False:
+                return
+        try:
+            config_db.set_entry("MIRROR_SESSION", session_name, session_info)
+        except ValueError as e:
+            ctx.fail("Invalid ConfigDB. Error: {}".format(e))
     else:
         per_npu_configdb = {}
         for front_asic_namespaces in namespaces['front_ns']:
-            per_npu_configdb[front_asic_namespaces] = ConfigDBConnector(use_unix_socket_path=True, namespace=front_asic_namespaces)
+            per_npu_configdb[front_asic_namespaces] = ValidatedConfigDBConnector(ConfigDBConnector(use_unix_socket_path=True, namespace=front_asic_namespaces))
             per_npu_configdb[front_asic_namespaces].connect()
-            if validate_mirror_session_config(per_npu_configdb[front_asic_namespaces], session_name, dst_port, src_port, direction) is False:
-                return
-            per_npu_configdb[front_asic_namespaces].set_entry("MIRROR_SESSION", session_name, session_info)
+            if ADHOC_VALIDATION:
+                if validate_mirror_session_config(per_npu_configdb[front_asic_namespaces], session_name, dst_port, src_port, direction) is False:
+                    return
+            try:
+                per_npu_configdb[front_asic_namespaces].set_entry("MIRROR_SESSION", session_name, session_info)
+            except ValueError as e:
+                ctx.fail("Invalid ConfigDB. Error: {}".format(e))
 
 
 @mirror_session.command()
@@ -2434,16 +2453,23 @@ def remove(session_name):
     For multi-npu platforms we need to program all front asic namespaces
     """
     namespaces = multi_asic.get_all_namespaces()
+    ctx = click.get_current_context()
     if not namespaces['front_ns']:
-        config_db = ConfigDBConnector()
+        config_db = ValidatedConfigDBConnector(ConfigDBConnector())
         config_db.connect()
-        config_db.set_entry("MIRROR_SESSION", session_name, None)
+        try:
+            config_db.set_entry("MIRROR_SESSION", session_name, None)
+        except JsonPatchConflict as e:
+            ctx.fail("Invalid ConfigDB. Error: {}".format(e))
     else:
         per_npu_configdb = {}
         for front_asic_namespaces in namespaces['front_ns']:
-            per_npu_configdb[front_asic_namespaces] = ConfigDBConnector(use_unix_socket_path=True, namespace=front_asic_namespaces)
+            per_npu_configdb[front_asic_namespaces] = ValidatedConfigDBConnector(ConfigDBConnector(use_unix_socket_path=True, namespace=front_asic_namespaces))
             per_npu_configdb[front_asic_namespaces].connect()
-            per_npu_configdb[front_asic_namespaces].set_entry("MIRROR_SESSION", session_name, None)
+            try:
+                per_npu_configdb[front_asic_namespaces].set_entry("MIRROR_SESSION", session_name, None)
+            except JsonPatchConflict as e:
+                ctx.fail("Invalid ConfigDB. Error: {}".format(e))
 
 #
 # 'pfcwd' group ('config pfcwd ...')

diff --git a/tests/config_mirror_session_test.py b/tests/config_mirror_session_test.py
@@ -1,7 +1,11 @@
 import pytest
 import config.main as config
+import jsonpatch
 from unittest import mock
 from click.testing import CliRunner
+from mock import patch
+from jsonpatch import JsonPatchConflict
+from sonic_py_common import multi_asic
 
 ERR_MSG_IP_FAILURE = "does not appear to be an IPv4 or IPv6 network"
 ERR_MSG_IP_VERSION_FAILURE = "not a valid IPv4 address"
@@ -172,7 +176,34 @@ def test_mirror_session_erspan_add():
         mocked.assert_called_with("test_session", "100.1.1.1", "2.2.2.2", 8, 63, 0, 0, None, None, None)
 
 
+@patch("validated_config_db_connector.device_info.is_yang_config_validation_enabled", mock.Mock(return_value=True))
+@patch("config.validated_config_db_connector.ValidatedConfigDBConnector.validated_set_entry", mock.Mock(side_effect=ValueError))
+def test_mirror_session_erspan_add_invalid_yang_validation():
+    config.ADHOC_VALIDATION = False
+    runner = CliRunner()
+    result = runner.invoke(
+            config.config.commands["mirror_session"].commands["erspan"].commands["add"],
+            ["test_session", "100.1.1.1", "2.2.2.2", "8", "63", "10", "100"])
+    print(result.output)
+    assert "Invalid ConfigDB. Error" in result.output
+
+
+@patch("config.main.ConfigDBConnector", spec=True, connect=mock.Mock())
+@patch("config.main.multi_asic.get_all_namespaces", mock.Mock(return_value={'front_ns': 'sample_ns'}))
+@patch("validated_config_db_connector.device_info.is_yang_config_validation_enabled", mock.Mock(return_value=True))
+@patch("config.validated_config_db_connector.ValidatedConfigDBConnector.validated_set_entry", mock.Mock(side_effect=ValueError))
+def test_mirror_session_erspan_add_multi_asic_invalid_yang_validation(mock_db_connector):
+    config.ADHOC_VALIDATION = False
+    runner = CliRunner()
+    result = runner.invoke(
+            config.config.commands["mirror_session"].commands["erspan"].commands["add"],
+            ["test_session", "100.1.1.1", "2.2.2.2", "8", "63", "10", "100"])
+    print(result.output)
+    assert "Invalid ConfigDB. Error" in result.output
+
+
 def test_mirror_session_span_add():
+    config.ADHOC_VALIDATION = True
     runner = CliRunner()
 
     # Verify invalid queue
@@ -273,3 +304,54 @@ def test_mirror_session_span_add():
 
         mocked.assert_called_with("test_session", "Ethernet0", "Ethernet4", "rx", 0, None)
 
+
+@patch("config.main.ConfigDBConnector", spec=True, connect=mock.Mock())
+@patch("config.main.multi_asic.get_all_namespaces", mock.Mock(return_value={'front_ns': 'sample_ns'}))
+@patch("validated_config_db_connector.device_info.is_yang_config_validation_enabled", mock.Mock(return_value=True))
+@patch("config.validated_config_db_connector.ValidatedConfigDBConnector.validated_set_entry", mock.Mock(side_effect=ValueError))
+def test_mirror_session_span_add_multi_asic_invalid_yang_validation(mock_db_connector):
+    config.ADHOC_VALIDATION = False
+    runner = CliRunner()
+    result = runner.invoke(
+            config.config.commands["mirror_session"].commands["span"].commands["add"],
+            ["test_session", "Ethernet0", "Ethernet4", "rx", "0"])
+    print(result.output)
+    assert "Invalid ConfigDB. Error" in result.output
+
+
+@patch("validated_config_db_connector.device_info.is_yang_config_validation_enabled", mock.Mock(return_value=True))
+@patch("config.validated_config_db_connector.ValidatedConfigDBConnector.validated_set_entry", mock.Mock(side_effect=ValueError))
+def test_mirror_session_span_add_invalid_yang_validation():
+    config.ADHOC_VALIDATION = False
+    runner = CliRunner()
+    result = runner.invoke(
+            config.config.commands["mirror_session"].commands["span"].commands["add"],
+            ["test_session", "Ethernet0", "Ethernet4", "rx", "0"])
+    print(result.output)
+    assert "Invalid ConfigDB. Error" in result.output
+
+
+@patch("config.main.multi_asic.get_all_namespaces", mock.Mock(return_value={'front_ns': 'sample_ns'}))
+@patch("validated_config_db_connector.device_info.is_yang_config_validation_enabled", mock.Mock(return_value=True))
+@patch("config.main.ConfigDBConnector", spec=True, connect=mock.Mock())
+@patch("config.validated_config_db_connector.ValidatedConfigDBConnector.validated_set_entry", mock.Mock(side_effect=JsonPatchConflict))
+def test_mirror_session_remove_multi_asic_invalid_yang_validation(mock_db_connector):
+    config.ADHOC_VALIDATION = False
+    runner = CliRunner()
+    result = runner.invoke(
+            config.config.commands["mirror_session"].commands["remove"],
+            ["mrr_sample"])
+    print(result.output)
+    assert "Invalid ConfigDB. Error" in result.output
+
+
+@patch("validated_config_db_connector.device_info.is_yang_config_validation_enabled", mock.Mock(return_value=True))
+@patch("config.validated_config_db_connector.ValidatedConfigDBConnector.validated_set_entry", mock.Mock(side_effect=JsonPatchConflict))
+def test_mirror_session_remove_invalid_yang_validation():
+    config.ADHOC_VALIDATION = False
+    runner = CliRunner()
+    result = runner.invoke(
+            config.config.commands["mirror_session"].commands["remove"],
+            ["mrr_sample"])
+    print(result.output)
+    assert "Invalid ConfigDB. Error" in result.output
diff --git a/tests/config_snmp_test.py b/tests/config_snmp_test.py
@@ -118,6 +118,7 @@ def setup_class(cls):
 
     # Add snmp community tests
     def test_config_snmp_community_add_new_community_ro(self):
+        config.ADHOC_VALIDATION = True
         db = Db()
         runner = CliRunner()
         with mock.patch('utilities_common.cli.run_command') as mock_run_command: