From 75bb60fe4f22b2c0831e7b31e5675df0cd01ff7d Mon Sep 17 00:00:00 2001 From: isabelmsft <67024108+isabelmsft@users.noreply.github.com> Date: Tue, 7 Mar 2023 14:42:50 -0800 Subject: [PATCH] YANG validation for ConfigDB Updates: MIRROR_SESSION use case (#2430) --- config/main.py | 66 ++++++++++++++++------- tests/config_mirror_session_test.py | 82 +++++++++++++++++++++++++++++ tests/config_snmp_test.py | 1 + 3 files changed, 129 insertions(+), 20 deletions(-) diff --git a/config/main.py b/config/main.py index 89ae45a2e3..029a8b5d68 100644 --- a/config/main.py +++ b/config/main.py @@ -2355,25 +2355,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 @@ -2405,25 +2415,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() @@ -2435,16 +2454,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 index 5585cab87a..ccbc196b50 100644 --- 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 index 096f21cb80..76f5675690 100644 --- 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: