From 86c18ec6fdd5ac1589bee834d3c75a926b28d449 Mon Sep 17 00:00:00 2001 From: Wen Liang Date: Wed, 20 Dec 2023 16:42:19 -0500 Subject: [PATCH] feat: Add support for the route_type When users have BGP routing setups, it is common practice to blackhole some less-specific routes in order to avoid routing loops, and the BGP router might insert a more specific route dynamically afterwards. Signed-off-by: Wen Liang --- README.md | 15 ++- examples/route_type_support.yml | 51 +++++++ library/network_connections.py | 4 + .../network_lsr/argument_validator.py | 20 +++ tests/ensure_provider_tests.py | 5 + tests/playbooks/tests_route_type.yml | 124 ++++++++++++++++++ tests/tests_route_type_nm.yml | 45 +++++++ tests/unit/test_network_connections.py | 35 +++++ 8 files changed, 292 insertions(+), 7 deletions(-) create mode 100644 examples/route_type_support.yml create mode 100644 tests/playbooks/tests_route_type.yml create mode 100644 tests/tests_route_type_nm.yml diff --git a/README.md b/README.md index e0ca5fa61..0697c35f0 100644 --- a/README.md +++ b/README.md @@ -586,13 +586,14 @@ The IP configuration supports the following options: Static route configuration can be specified via a list of routes given in the `route` option. The default value is an empty list. Each route is a dictionary with - the following entries: `network`, `prefix`, `gateway`, `metric` and `table`. - `network` and `prefix` specify the destination network. `table` supports both the - numeric table and named table. In order to specify the named table, the users have - to ensure the named table is properly defined in `/etc/iproute2/rt_tables` or - `/etc/iproute2/rt_tables.d/*.conf`. - Note that Classless inter-domain routing (CIDR) notation or network mask notation - are not supported yet. + the following entries: `network`, `prefix`, `gateway`, `metric`, `table` and + `route_type`. `network` and `prefix` specify the destination network. `table` + supports both the numeric table and named table. In order to specify the named + table, the users have to ensure the named table is properly defined in + `/etc/iproute2/rt_tables` or `/etc/iproute2/rt_tables.d/*.conf`. And the supported + `route_type` are `blackhole`, `prohibit`, `unreachable` and these `route_type` routes + can not have next hop (gateway) specified. Note that Classless inter-domain routing + (CIDR) notation or network mask notation are not supported yet. - `routing_rule` diff --git a/examples/route_type_support.yml b/examples/route_type_support.yml new file mode 100644 index 000000000..1c2c64c47 --- /dev/null +++ b/examples/route_type_support.yml @@ -0,0 +1,51 @@ +# SPDX-License-Identifier: BSD-3-Clause +--- +- name: Manage route type routes + hosts: all + tasks: + - name: Configure connection profile and route type routes + import_role: + name: linux-system-roles.network + vars: + network_connections: + - name: eth0 + interface_name: eth0 + state: up + type: ethernet + autoconnect: true + ip: + dhcp4: false + address: + - 198.51.100.3/26 + - 2001:db8::2/32 + route: + - network: 198.51.100.64 + prefix: 26 + gateway: 198.51.100.6 + metric: 4 + table: 30200 + - network: 198.53.100.18 + prefix: 32 + metric: 20 + route_type: blackhole + table: 30200 + - network: 198.53.100.12 + prefix: 32 + metric: 24 + route_type: unreachable + table: 30200 + - network: 198.53.100.10 + prefix: 32 + metric: 30 + route_type: prohibit + table: 30200 + - network: 2001:db8::4 + prefix: 128 + metric: 2 + route_type: blackhole + table: 30600 + - network: 2001:db8::6 + prefix: 128 + metric: 4 + route_type: prohibit + table: 30600 diff --git a/library/network_connections.py b/library/network_connections.py index 07b29ccf9..077bf4bf0 100644 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -1220,6 +1220,10 @@ def connection_create(self, connections, idx, connection_current=None): new_route = NM.IPRoute.new( r["family"], r["network"], r["prefix"], r["gateway"], r["metric"] ) + if r["route_type"]: + NM.IPRoute.set_attribute( + new_route, "type", Util.GLib().Variant("s", r["route_type"]) + ) if r["table"]: NM.IPRoute.set_attribute( new_route, "table", Util.GLib().Variant.new_uint32(r["table"]) diff --git a/module_utils/network_lsr/argument_validator.py b/module_utils/network_lsr/argument_validator.py index bb11f200f..4d5b26ded 100644 --- a/module_utils/network_lsr/argument_validator.py +++ b/module_utils/network_lsr/argument_validator.py @@ -674,6 +674,11 @@ def __init__(self, name, family=None, required=False): ArgValidatorNum( "metric", default_value=-1, val_min=-1, val_max=UINT32_MAX ), + ArgValidatorStr( + "route_type", + default_value=None, + enum_values=["blackhole", "prohibit", "unreachable"], + ), ArgValidatorRouteTable("table"), ], default_value=None, @@ -688,6 +693,7 @@ def _validate_post(self, value, name, result): result["family"] = family gateway = result["gateway"] + route_type = result["route_type"] if gateway is not None: if family != gateway["family"]: raise ValidationError( @@ -695,6 +701,12 @@ def _validate_post(self, value, name, result): "conflicting address family between network and gateway '%s'" % (gateway["address"]), ) + if route_type is not None: + raise ValidationError( + name, + "a %s route can not have a next hop '%s'" + % (route_type, gateway["address"]), + ) result["gateway"] = gateway["address"] prefix = result["prefix"] @@ -2607,6 +2619,14 @@ def _ipv6_is_not_configured(connection): "NetworkManger until NM 1.30", ) + if connection["ip"]["route"]: + if mode == self.VALIDATE_ONE_MODE_INITSCRIPTS: + for route in connection["ip"]["route"]: + if route["route_type"] is not None: + raise ValidationError.from_connection( + idx, + "route_type is not supported by initscripts", + ) if connection["ip"]["routing_rule"]: if mode == self.VALIDATE_ONE_MODE_INITSCRIPTS: raise ValidationError.from_connection( diff --git a/tests/ensure_provider_tests.py b/tests/ensure_provider_tests.py index 95d4a0650..b013bf49a 100755 --- a/tests/ensure_provider_tests.py +++ b/tests/ensure_provider_tests.py @@ -95,6 +95,11 @@ }, "playbooks/tests_reapply.yml": {}, "playbooks/tests_route_table.yml": {}, + "playbooks/tests_route_type.yml": { + MINIMUM_VERSION: "'1.36.0'", + "comment": "# NetworkManager 1.36.0 added support for special route types: \ +prohibit, blackhole and unreachable", + }, "playbooks/tests_routing_rules.yml": {}, # team interface is not supported on Fedora "playbooks/tests_team.yml": { diff --git a/tests/playbooks/tests_route_type.yml b/tests/playbooks/tests_route_type.yml new file mode 100644 index 000000000..c5bc14ccb --- /dev/null +++ b/tests/playbooks/tests_route_type.yml @@ -0,0 +1,124 @@ +# SPDX-License-Identifier: BSD-3-Clause +--- +- name: Play for testing route types + hosts: all + vars: + type: veth + interface: ethtest0 + + + tasks: + - name: "Set type={{ type }} and interface={{ interface }}" # noqa name + set_fact: + type: "{{ type }}" + interface: "{{ interface }}" + - name: Include the task 'show_interfaces.yml' + include_tasks: tasks/show_interfaces.yml + - name: Include the task 'manage_test_interface.yml' + include_tasks: tasks/manage_test_interface.yml + vars: + state: present + - name: Include the task 'assert_device_present.yml' + include_tasks: tasks/assert_device_present.yml + + - name: Configure connection profile and specify the route types in + static routes + import_role: + name: linux-system-roles.network + vars: + network_connections: + - name: "{{ interface }}" + interface_name: "{{ interface }}" + state: up + type: ethernet + autoconnect: true + ip: + dhcp4: false + address: + - 198.51.100.3/26 + - 2001:db8::2/32 + route: + - network: 198.51.100.64 + prefix: 26 + gateway: 198.51.100.6 + metric: 4 + table: 30200 + - network: 198.53.100.18 + prefix: 32 + metric: 20 + route_type: blackhole + table: 30200 + - network: 198.53.100.12 + prefix: 32 + metric: 24 + route_type: unreachable + table: 30200 + - network: 198.53.100.10 + prefix: 32 + metric: 30 + route_type: prohibit + table: 30200 + - network: 2001:db8::4 + prefix: 128 + metric: 2 + route_type: blackhole + table: 30600 + - network: 2001:db8::6 + prefix: 128 + metric: 4 + route_type: prohibit + table: 30600 + + - name: Get the routes from the route table 30200 + command: ip route show table 30200 + register: route_table_30200 + changed_when: false + + - name: Get the routes from the route table 30600 + command: ip -6 route show table 30600 + register: route_table_30600 + changed_when: false + + - name: Assert that the route table 30200 contains the specified route + assert: + that: + - route_table_30200.stdout is search("198.51.100.64/26 via + 198.51.100.6 dev ethtest0 proto static metric 4") + - route_table_30200.stdout is search("blackhole 198.53.100.18 + proto static scope link metric 20") + - route_table_30200.stdout is search("unreachable 198.53.100.12 + proto static scope link metric 24") + - route_table_30200.stdout is search("prohibit 198.53.100.10 + proto static scope link metric 30") + msg: "the route table 30200 does not exist or does not contain the + specified route" + + - name: Assert that the route table 30600 contains the specified route + assert: + that: + - route_table_30600.stdout is search("blackhole 2001:db8::4 + dev lo proto static metric 2 pref medium") + - route_table_30600.stdout is search("prohibit 2001:db8::6 + dev lo proto static metric 4 pref medium") + msg: "the route table 30600 does not exist or does not contain the + specified route" + +- name: Import the playbook 'down_profile+delete_interface.yml' + import_playbook: down_profile+delete_interface.yml + vars: + profile: "{{ interface }}" +# FIXME: assert profile/device down +- name: Import the playbook 'remove_profile.yml' + import_playbook: remove_profile.yml + vars: + profile: "{{ interface }}" +- name: Assert device and profile are absent + hosts: all + tasks: + - name: Include the task 'assert_profile_absent.yml' + include_tasks: tasks/assert_profile_absent.yml + vars: + profile: "{{ interface }}" + - name: Include the task 'assert_device_absent.yml' + include_tasks: tasks/assert_device_absent.yml +... diff --git a/tests/tests_route_type_nm.yml b/tests/tests_route_type_nm.yml new file mode 100644 index 000000000..874f5418e --- /dev/null +++ b/tests/tests_route_type_nm.yml @@ -0,0 +1,45 @@ +# SPDX-License-Identifier: BSD-3-Clause +# This file was generated by ensure_provider_tests.py +--- +# set network provider and gather facts +- hosts: all + # yamllint disable rule:line-length + name: Run playbook 'playbooks/tests_route_type.yml' with nm as provider + tasks: + - name: Include the task 'el_repo_setup.yml' + include_tasks: tasks/el_repo_setup.yml + - name: Set network provider to 'nm' + set_fact: + network_provider: nm + tags: + - always + + - name: Install NetworkManager and get NetworkManager version + when: + - ansible_distribution_major_version != '6' + tags: + - always + block: + - name: Install NetworkManager + package: + name: NetworkManager + state: present + use: "{{ (__network_is_ostree | d(false)) | + ternary('ansible.posix.rhel_rpm_ostree', omit) }}" + - name: Get package info + package_facts: + - name: Get NetworkManager version + set_fact: + networkmanager_version: "{{ + ansible_facts.packages['NetworkManager'][0]['version'] }}" + + +# The test requires or should run with NetworkManager, therefore it cannot run +# on RHEL/CentOS 6 +# NetworkManager 1.36.0 added support for special route types: prohibit, blackhole and unreachable +- name: Import the playbook 'playbooks/tests_route_type.yml' + import_playbook: playbooks/tests_route_type.yml + when: + - ansible_distribution_major_version != '6' + + - networkmanager_version is version('1.36.0', '>=') diff --git a/tests/unit/test_network_connections.py b/tests/unit/test_network_connections.py index 4d8cd653e..fa9539b3c 100644 --- a/tests/unit/test_network_connections.py +++ b/tests/unit/test_network_connections.py @@ -237,6 +237,7 @@ def assert_nm_connection_routes_expected(self, connection, route_list_expected): "prefix": int(r.get_prefix()), "gateway": r.get_next_hop(), "metric": int(r.get_metric()), + "route_type": r.get_attribute("route_type"), "table": r.get_attribute("table"), } for r in route_list_new @@ -282,6 +283,12 @@ def do_connections_validate_nm(self, input_connections, **kwargs): r["gateway"], r["metric"], ) + if r["route_type"]: + NM.IPRoute.set_attribute( + new_route, + "type", + Util.GLib().Variant("s", r["route_type"]), + ) if r["table"]: NM.IPRoute.set_attribute( new_route, @@ -1135,6 +1142,7 @@ def test_routes(self): "prefix": 24, "gateway": None, "metric": -1, + "route_type": None, "table": None, } ], @@ -1475,6 +1483,7 @@ def test_vlan(self): "prefix": 24, "gateway": None, "metric": -1, + "route_type": None, "table": None, } ], @@ -1624,6 +1633,7 @@ def test_macvlan(self): "prefix": 24, "gateway": None, "metric": -1, + "route_type": None, "table": None, } ], @@ -1686,6 +1696,7 @@ def test_macvlan(self): "prefix": 24, "gateway": None, "metric": -1, + "route_type": None, "table": None, } ], @@ -2648,6 +2659,7 @@ def test_route_metric_prefix(self): "prefix": 24, "gateway": None, "metric": 545, + "route_type": None, "table": None, }, { @@ -2656,6 +2668,7 @@ def test_route_metric_prefix(self): "prefix": 30, "gateway": None, "metric": -1, + "route_type": None, "table": None, }, ], @@ -2752,6 +2765,7 @@ def test_route_v6(self): "prefix": 24, "gateway": None, "metric": 545, + "route_type": None, "table": None, }, { @@ -2760,6 +2774,7 @@ def test_route_v6(self): "prefix": 30, "gateway": None, "metric": -1, + "route_type": None, "table": None, }, { @@ -2768,6 +2783,7 @@ def test_route_v6(self): "prefix": 64, "gateway": None, "metric": -1, + "route_type": None, "table": None, }, ], @@ -2905,6 +2921,7 @@ def test_route_without_interface_name(self): "prefix": 24, "gateway": None, "metric": 545, + "route_type": None, "table": None, }, { @@ -2913,6 +2930,7 @@ def test_route_without_interface_name(self): "prefix": 30, "gateway": None, "metric": -1, + "route_type": None, "table": None, }, { @@ -2921,6 +2939,7 @@ def test_route_without_interface_name(self): "prefix": 64, "gateway": None, "metric": -1, + "route_type": None, "table": None, }, ], @@ -4966,6 +4985,22 @@ def test_table_not_found_when_validate_route_tables(self): self.connection_index, ) + def test_route_type_route_with_next_hop(self): + """ + Test that the route type route can not have the next hop + """ + + self.test_connections[0]["ip"]["route"][0]["route_type"] = "blackhole" + self.assertRaisesRegex( + ValidationError, + "a {0} route can not have a next hop '{1}'".format( + self.test_connections[0]["ip"]["route"][0]["route_type"], + self.test_connections[0]["ip"]["route"][0]["gateway"], + ), + self.validator.validate, + self.test_connections, + ) + class TestValidatorRoutingRules(Python26CompatTestCase): def setUp(self):