From 60aabdc9587700a326c8e9df06f09ba0c60c5435 Mon Sep 17 00:00:00 2001 From: Alex Kudentsov <43482574+alexk37@users.noreply.github.com> Date: Thu, 8 Oct 2026 22:27:36 +0700 Subject: ospf: T9441: migrate "parameters opaque-lsa" to "capability opaque" "protocols ospf parameters opaque-lsa" and "protocols ospf capability opaque" are two CLI nodes for one FRR setting, FRR's "capability opaque". Keep "capability opaque" only. The migration script ospf/2-to-3 sets "capability opaque" where "parameters opaque-lsa" exists in the default VRF, and deletes the old node (and the "parameters" node when it is left empty). FRR refuses opaque LSAs outside the default VRF, so in a VRF instance the option never took effect: for a VRF the migration drops both "parameters opaque-lsa" and a plain "capability opaque" instead of moving the value into a node FRR would refuse at the next commit. The template renders the line from "capability opaque" only; it rendered the alias form "ospf opaque-lsa" for the old node, which frr-reload found missing in the running config on every reload: while only the old node was set, every commit that reloads FRR reset all adjacencies of that OSPF instance. Tests: a migration unit test (global instance, VRF instance, a VRF with "capability opaque" set directly, both nodes set together in a VRF, and global+VRF independence), the vrf-ospf config test (now carries "parameters opaque-lsa" in both the default instance and a VRF instance; the VRF instance is expected to lose it with no replacement), test_ospf_02 uses the new node, and test_ospf_21 checks the generated FRR config. --- src/migration-scripts/ospf/2-to-3 | 54 ++++++++ src/tests/test_ospf_migration.py | 258 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 312 insertions(+) create mode 100644 src/migration-scripts/ospf/2-to-3 create mode 100644 src/tests/test_ospf_migration.py (limited to 'src') diff --git a/src/migration-scripts/ospf/2-to-3 b/src/migration-scripts/ospf/2-to-3 new file mode 100644 index 000000000..271dbd58e --- /dev/null +++ b/src/migration-scripts/ospf/2-to-3 @@ -0,0 +1,54 @@ +# Copyright VyOS maintainers and contributors +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with this library. If not, see . + +# T9441: "parameters opaque-lsa" and "capability opaque" are two CLI nodes for +# the same FRR setting. Keep "capability opaque" only. FRR supports +# opaque LSAs only in the default VRF and refuses the setting in a VRF, +# so in VRF instances it never took effect: drop it there. + +from vyos.configtree import ConfigTree + + +def opaque_lsa_migration(config: ConfigTree, ospf_base: list, vrf: bool) -> None: + parameters = ospf_base + ['parameters'] + if config.exists(parameters + ['opaque-lsa']): + config.delete(parameters + ['opaque-lsa']) + + # do not leave an empty "parameters" node behind + if config.list_nodes(parameters) == []: + config.delete(parameters) + + if not vrf: + config.set(ospf_base + ['capability', 'opaque']) + + # FRR refuses "capability opaque" outside the default VRF; drop a value + # that would otherwise fail every commit (it never took effect anyway) + capability = ospf_base + ['capability'] + if vrf and config.exists(capability + ['opaque']): + config.delete(capability + ['opaque']) + if config.list_nodes(capability) == []: + config.delete(capability) + + +def migrate(config: ConfigTree) -> None: + # OSPF in the default VRF + opaque_lsa_migration(config, ['protocols', 'ospf'], vrf=False) + + # OSPF in every VRF + if config.exists(['vrf', 'name']): + for name in config.list_nodes(['vrf', 'name']): + opaque_lsa_migration( + config, ['vrf', 'name', name, 'protocols', 'ospf'], vrf=True + ) diff --git a/src/tests/test_ospf_migration.py b/src/tests/test_ospf_migration.py new file mode 100644 index 000000000..95a73cbe9 --- /dev/null +++ b/src/tests/test_ospf_migration.py @@ -0,0 +1,258 @@ +# Copyright VyOS maintainers and contributors +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with this library. If not, see . + +import importlib.machinery +import importlib.util +from unittest import TestCase + +from vyos.configtree import ConfigTree + +migration_script = 'src/migration-scripts/ospf/2-to-3' + +ospf = ['protocols', 'ospf'] +vrf_ospf = ['vrf', 'name', 'red', 'protocols', 'ospf'] + + +def load_migrate(): + loader = importlib.machinery.SourceFileLoader('ospf_2_to_3', migration_script) + spec = importlib.util.spec_from_loader(loader.name, loader) + module = importlib.util.module_from_spec(spec) + loader.exec_module(module) + return module.migrate + + +def opaque_lsa(base): + return base + ['parameters', 'opaque-lsa'] + + +def capability_opaque(base): + return base + ['capability', 'opaque'] + + +class TestOspfOpaqueLsaMigration(TestCase): + def setUp(self): + self.migrate = load_migrate() + + def test_no_ospf(self): + # must not raise when OSPF is not configured + config = ConfigTree('system {\n host-name vyos\n}\n') + self.migrate(config) + self.assertFalse(config.exists(ospf)) + self.assertFalse(config.exists(['vrf'])) + + def test_neither_node(self): + config = ConfigTree( + 'protocols {\n' + ' ospf {\n' + ' parameters {\n' + ' router-id 192.0.2.1\n' + ' }\n' + ' }\n' + '}\n' + ) + self.migrate(config) + self.assertFalse(config.exists(capability_opaque(ospf))) + self.assertEqual( + config.return_value(ospf + ['parameters', 'router-id']), '192.0.2.1' + ) + + def test_global_instance(self): + config = ConfigTree( + 'protocols {\n' + ' ospf {\n' + ' parameters {\n' + ' opaque-lsa\n' + ' router-id 192.0.2.1\n' + ' }\n' + ' }\n' + '}\n' + ) + self.migrate(config) + self.assertTrue(config.exists(capability_opaque(ospf))) + self.assertFalse(config.exists(opaque_lsa(ospf))) + # the other parameters stay + self.assertEqual( + config.return_value(ospf + ['parameters', 'router-id']), '192.0.2.1' + ) + + def test_empty_parameters_node_is_removed(self): + config = ConfigTree( + 'protocols {\n' + ' ospf {\n' + ' parameters {\n' + ' opaque-lsa\n' + ' }\n' + ' }\n' + '}\n' + ) + self.migrate(config) + self.assertTrue(config.exists(capability_opaque(ospf))) + self.assertFalse(config.exists(ospf + ['parameters'])) + + def test_vrf_instance(self): + # FRR refuses opaque LSAs outside the default VRF: the option never + # took effect there, so it is dropped, not turned into "capability + # opaque" (which FRR would refuse at commit time) + config = ConfigTree( + 'vrf {\n' + ' name red {\n' + ' protocols {\n' + ' ospf {\n' + ' parameters {\n' + ' opaque-lsa\n' + ' router-id 192.0.2.2\n' + ' }\n' + ' }\n' + ' }\n' + ' table 1000\n' + ' }\n' + '}\n' + ) + self.migrate(config) + self.assertFalse(config.exists(capability_opaque(vrf_ospf))) + self.assertFalse(config.exists(opaque_lsa(vrf_ospf))) + self.assertEqual( + config.return_value(vrf_ospf + ['parameters', 'router-id']), '192.0.2.2' + ) + # nothing is created in the default VRF + self.assertFalse(config.exists(ospf)) + + def test_vrf_capability_opaque_dropped(self): + # a VRF instance that already has "capability opaque" set directly + # (without "parameters opaque-lsa") is just as refused by FRR; drop it + config = ConfigTree( + 'vrf {\n' + ' name red {\n' + ' protocols {\n' + ' ospf {\n' + ' capability {\n' + ' opaque\n' + ' }\n' + ' parameters {\n' + ' router-id 192.0.2.2\n' + ' }\n' + ' }\n' + ' }\n' + ' table 1000\n' + ' }\n' + '}\n' + ) + self.migrate(config) + self.assertFalse(config.exists(capability_opaque(vrf_ospf))) + self.assertFalse(config.exists(vrf_ospf + ['capability'])) + self.assertEqual( + config.return_value(vrf_ospf + ['parameters', 'router-id']), '192.0.2.2' + ) + + def test_both_nodes_set_in_vrf(self): + # both the dead "parameters opaque-lsa" deletion and the "capability + # opaque" drop fire for the same VRF instance in one pass + config = ConfigTree( + 'vrf {\n' + ' name red {\n' + ' protocols {\n' + ' ospf {\n' + ' capability {\n' + ' opaque\n' + ' }\n' + ' parameters {\n' + ' opaque-lsa\n' + ' }\n' + ' }\n' + ' }\n' + ' table 1000\n' + ' }\n' + '}\n' + ) + self.migrate(config) + self.assertFalse(config.exists(capability_opaque(vrf_ospf))) + self.assertFalse(config.exists(opaque_lsa(vrf_ospf))) + self.assertFalse(config.exists(vrf_ospf + ['capability'])) + self.assertFalse(config.exists(vrf_ospf + ['parameters'])) + + def test_global_and_vrf_instances_are_independent(self): + # the default instance keeps the move to "capability opaque"; VRF + # "red" only loses the dead option; VRF "blue" has neither and must + # stay as it is + config = ConfigTree( + 'protocols {\n' + ' ospf {\n' + ' parameters {\n' + ' opaque-lsa\n' + ' }\n' + ' }\n' + '}\n' + 'vrf {\n' + ' name blue {\n' + ' protocols {\n' + ' ospf {\n' + ' parameters {\n' + ' router-id 192.0.2.3\n' + ' }\n' + ' }\n' + ' }\n' + ' }\n' + ' name red {\n' + ' protocols {\n' + ' ospf {\n' + ' parameters {\n' + ' opaque-lsa\n' + ' }\n' + ' }\n' + ' }\n' + ' }\n' + '}\n' + ) + self.migrate(config) + blue = ['vrf', 'name', 'blue', 'protocols', 'ospf'] + # capability opaque exists only in the default instance + self.assertTrue(config.exists(capability_opaque(ospf))) + self.assertFalse(config.exists(capability_opaque(vrf_ospf))) + self.assertFalse(config.exists(opaque_lsa(vrf_ospf))) + self.assertFalse(config.exists(capability_opaque(blue))) + self.assertEqual( + config.return_value(blue + ['parameters', 'router-id']), '192.0.2.3' + ) + + def test_both_nodes_set(self): + config = ConfigTree( + 'protocols {\n' + ' ospf {\n' + ' capability {\n' + ' opaque\n' + ' }\n' + ' parameters {\n' + ' opaque-lsa\n' + ' }\n' + ' }\n' + '}\n' + ) + self.migrate(config) + self.assertTrue(config.exists(capability_opaque(ospf))) + self.assertFalse(config.exists(opaque_lsa(ospf))) + + def test_only_capability_set(self): + config = ConfigTree( + 'protocols {\n' + ' ospf {\n' + ' capability {\n' + ' opaque\n' + ' }\n' + ' }\n' + '}\n' + ) + self.migrate(config) + self.assertTrue(config.exists(capability_opaque(ospf))) + self.assertFalse(config.exists(ospf + ['parameters'])) -- cgit v1.2.3 From 20a57113117b5db6d54c8da3eb08fa29b9398947 Mon Sep 17 00:00:00 2001 From: Alex Kudentsov <43482574+alexk37@users.noreply.github.com> Date: Tue, 13 Oct 2026 14:01:27 +0700 Subject: ospf: T9441: reject "capability opaque" in a VRF instance FRR refuses opaque LSAs outside the default VRF ("OSPF Opaque LSA is only supported in default VRF"). Before this, a "set protocols ospf capability opaque" under "vrf name protocols ospf" was accepted by the CLI and only failed later, at FRR's own reload, with the render already written and every other change in the commit already applied. Reject it in verify() before the commit reaches FRR, with a message that names the node and the VRF; the default VRF is unaffected. Test: test_ospf_23_vrf_capability_opaque sets "capability opaque" in a VRF instance and expects the commit to fail. --- smoketest/scripts/cli/test_protocols_ospf.py | 24 ++++++++++++++++++++++++ src/conf_mode/protocols_ospf.py | 7 +++++++ 2 files changed, 31 insertions(+) (limited to 'src') diff --git a/smoketest/scripts/cli/test_protocols_ospf.py b/smoketest/scripts/cli/test_protocols_ospf.py index da7ead42b..28568ecbb 100755 --- a/smoketest/scripts/cli/test_protocols_ospf.py +++ b/smoketest/scripts/cli/test_protocols_ospf.py @@ -792,5 +792,29 @@ class TestProtocolsOSPF(VyOSUnitTestSHIM.TestCase): self.cli_delete(vrf_base) + def test_ospf_23_vrf_capability_opaque(self): + # FRR refuses opaque LSAs outside the default VRF (T9441's migration + # drops a saved "capability opaque" there for the same reason); a new + # "set" must be rejected before FRR is reloaded, not left to FRR's + # own refusal + vrf = 'blue' + vrf_base = ['vrf', 'name', vrf] + vrf_ospf = vrf_base + ['protocols', 'ospf'] + self.cli_set(vrf_base + ['table', '1000']) + self.cli_set(vrf_ospf + ['parameters', 'router-id', '1.1.1.1']) + self.cli_set(vrf_ospf + ['capability', 'opaque']) + with self.assertRaises(ConfigSessionError): + self.cli_commit() + + self.cli_delete(vrf_ospf + ['capability']) + self.cli_commit() + + vrf_header = f'router ospf vrf {vrf}' + frrconfig = self.getFRRconfig(vrf_header, stop_section='^exit') + self.assertIn(' ospf router-id 1.1.1.1', frrconfig) + self.assertNotIn('capability opaque', frrconfig) + + self.cli_delete(vrf_base) + if __name__ == '__main__': unittest.main(verbosity=2, failfast=VyOSUnitTestSHIM.TestCase.debug_on()) diff --git a/src/conf_mode/protocols_ospf.py b/src/conf_mode/protocols_ospf.py index 47e1596a6..0f73a6c8a 100755 --- a/src/conf_mode/protocols_ospf.py +++ b/src/conf_mode/protocols_ospf.py @@ -52,6 +52,13 @@ def verify(config_dict): config_dict) or config_dict['ospf'] ospf['policy'] = config_dict['policy'] + # FRR supports opaque LSAs only in the default VRF + if vrf and 'opaque' in ospf.get('capability', {}): + raise ConfigError( + 'OSPF "capability opaque" is only supported in the ' + f'default VRF, not in VRF "{vrf}"!' + ) + verify_common_route_maps(ospf) # As we can have a default-information route-map, we need to validate it! -- cgit v1.2.3