From cc592c203563712df8c62d51ab6d2de72a155ddf Mon Sep 17 00:00:00 2001 From: Wen Liang Date: Wed, 20 Dec 2023 16:42:19 -0500 Subject: [PATCH] feat: Support blackhole, prohibit and unreachable route types It is useful and common practice to configure the routes with blackhole, prohibit, and unreachable route types when users have BGP routing setups. Notice that this feature is only for nm provider using `network_connections` variable. Configuring blackhole, prohibit, and unreachable route types is also supported by using `network_state` since nmstate version 2.2.20 (the setting name is `route-type`). Resolves: https://issues.redhat.com/browse/RHEL-19579 Signed-off-by: Wen Liang --- README.md | 16 +- examples/route_type_support.yml | 51 +++++ library/network_connections.py | 32 ++- .../network_lsr/argument_validator.py | 20 ++ tests/ensure_provider_tests.py | 5 + tests/playbooks/tests_route_type.yml | 194 ++++++++++++++++++ tests/tests_route_type_nm.yml | 45 ++++ tests/unit/test_network_connections.py | 35 ++++ 8 files changed, 391 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 e0ca5fa6..b849e5f7 100644 --- a/README.md +++ b/README.md @@ -586,13 +586,17 @@ 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`. + the following entries: `gateway`, `metric`, `network`, `prefix`, `table` and `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`. - Note that Classless inter-domain routing (CIDR) notation or network mask notation - are not supported yet. + 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`. The optional `type` key supports the values + `blackhole`, `prohibit`, and `unreachable`. + See [man 8 ip-route](https://man7.org/linux/man-pages/man8/ip-route.8.html#DESCRIPTION) + for their definition. Routes with these types do not support a gateway. If the type + is not specified, the route is considered as a unicast route. Note that the classless + inter-domain routing(CIDR) notation or the network mask notation are not supported + for the `network` key. - `routing_rule` diff --git a/examples/route_type_support.yml b/examples/route_type_support.yml new file mode 100644 index 00000000..1550321e --- /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 + type: blackhole + table: 30200 + - network: 198.53.100.12 + prefix: 32 + metric: 24 + type: unreachable + table: 30200 + - network: 198.53.100.10 + prefix: 32 + metric: 30 + type: prohibit + table: 30200 + - network: 2001:db8::4 + prefix: 128 + metric: 2 + type: blackhole + table: 30600 + - network: 2001:db8::6 + prefix: 128 + metric: 4 + type: prohibit + table: 30600 diff --git a/library/network_connections.py b/library/network_connections.py index 43915c92..cb95d680 100644 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -1255,6 +1255,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["type"]: + NM.IPRoute.set_attribute( + new_route, "type", Util.GLib().Variant("s", r["type"]) + ) if r["table"]: NM.IPRoute.set_attribute( new_route, "table", Util.GLib().Variant.new_uint32(r["table"]) @@ -2193,6 +2197,18 @@ def run_action_up(self, idx): ############################################################################### +def version_to_tuple(version): + """ + Translates the dot-separated version string to a tuple + + :param version: The dot-separated version string + :return: the version tuple + """ + version_list = version.split(".") + version_tuple = tuple(map(int, version_list)) + return version_tuple + + class Cmd_nm(Cmd): def __init__(self, **kwargs): Cmd.__init__(self, **kwargs) @@ -2222,7 +2238,21 @@ def run_prepare(self): names = {} for idx, connection in enumerate(self.connections): self._check_ethtool_setting_support(idx, connection) - + if connection.get("ip", {}): + for route in connection["ip"]["route"]: + if route["type"]: + # The special route type prohibit, blackhole and unreachable + # are only supported in NM since version 1.36.0 + nm_client_version = self._nm_provider.get_client_version() + if version_to_tuple(nm_client_version) < (1, 36, 0): + self.log_fatal( + idx, + "route type {0} is only supported in NM since 1.36.0 " + "but the NM client version is {1}".format( + route["type"], + nm_client_version, + ), + ) name = connection["name"] if not name: if not connection["persistent_state"] == "absent": diff --git a/module_utils/network_lsr/argument_validator.py b/module_utils/network_lsr/argument_validator.py index bb11f200..867ca356 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( + "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["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 gateway '%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["type"] is not None: + raise ValidationError.from_connection( + idx, + "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 bd52cd35..e454d046 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: \ +blackhole, prohibit 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 00000000..d1a25f7f --- /dev/null +++ b/tests/playbooks/tests_route_type.yml @@ -0,0 +1,194 @@ +# 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 + type: blackhole + table: 30200 + - network: 198.53.100.10 + prefix: 32 + metric: 30 + type: prohibit + table: 30200 + - network: 198.53.100.12 + prefix: 32 + metric: 24 + type: unreachable + table: 30200 + - network: 2001:db8::4 + prefix: 128 + metric: 2 + type: blackhole + table: 30600 + - network: 2001:db8::6 + prefix: 128 + metric: 4 + 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("prohibit 198.53.100.10 + proto static scope link metric 30") + - route_table_30200.stdout is search("unreachable 198.53.100.12 + proto static scope link metric 24") + 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: Removing some 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: 2001:db8::4 + prefix: 128 + metric: 2 + type: blackhole + table: 30600 + - network: 2001:db8::6 + prefix: 128 + metric: 4 + type: prohibit + table: 30600 + + - name: Get the routes from the route table 30200 after removing routes + command: ip route show table 30200 + register: table_30200 + changed_when: false + + - name: Get the routes from the route table 30600 after removing routes + command: ip -6 route show table 30600 + register: table_30600 + changed_when: false + + - name: Assert that the route table 30200 contains the specified unicast + 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") + msg: "the route table 30200 does not exist or does not contain the + specified unicast route" + + - name: Assert that the route table 30200 does not contain the type routes + assert: + that: + - table_30200.stdout is not search("blackhole 198.53.100.18 + proto static scope link metric 20") + - table_30200.stdout is not search("prohibit 198.53.100.10 + proto static scope link metric 30") + - table_30200.stdout is not search("unreachable 198.53.100.12 + proto static scope link metric 24") + msg: "the route table 30200 contains the type routes" + + - name: Assert that the route table 30600 still contains the type routes + assert: + that: + - table_30600.stdout is search("blackhole 2001:db8::4 + dev lo proto static metric 2 pref medium") + - 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 + type routes" + +- 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 00000000..2977be79 --- /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 +# yamllint disable rule:line-length +- name: Run playbook 'playbooks/tests_route_type.yml' with nm as provider + hosts: all + 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: blackhole, prohibit 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 4d8cd653..84a421da 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()), + "type": r.get_attribute("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["type"]: + NM.IPRoute.set_attribute( + new_route, + "type", + Util.GLib().Variant("s", r["type"]), + ) if r["table"]: NM.IPRoute.set_attribute( new_route, @@ -1135,6 +1142,7 @@ def test_routes(self): "prefix": 24, "gateway": None, "metric": -1, + "type": None, "table": None, } ], @@ -1475,6 +1483,7 @@ def test_vlan(self): "prefix": 24, "gateway": None, "metric": -1, + "type": None, "table": None, } ], @@ -1624,6 +1633,7 @@ def test_macvlan(self): "prefix": 24, "gateway": None, "metric": -1, + "type": None, "table": None, } ], @@ -1686,6 +1696,7 @@ def test_macvlan(self): "prefix": 24, "gateway": None, "metric": -1, + "type": None, "table": None, } ], @@ -2648,6 +2659,7 @@ def test_route_metric_prefix(self): "prefix": 24, "gateway": None, "metric": 545, + "type": None, "table": None, }, { @@ -2656,6 +2668,7 @@ def test_route_metric_prefix(self): "prefix": 30, "gateway": None, "metric": -1, + "type": None, "table": None, }, ], @@ -2752,6 +2765,7 @@ def test_route_v6(self): "prefix": 24, "gateway": None, "metric": 545, + "type": None, "table": None, }, { @@ -2760,6 +2774,7 @@ def test_route_v6(self): "prefix": 30, "gateway": None, "metric": -1, + "type": None, "table": None, }, { @@ -2768,6 +2783,7 @@ def test_route_v6(self): "prefix": 64, "gateway": None, "metric": -1, + "type": None, "table": None, }, ], @@ -2905,6 +2921,7 @@ def test_route_without_interface_name(self): "prefix": 24, "gateway": None, "metric": 545, + "type": None, "table": None, }, { @@ -2913,6 +2930,7 @@ def test_route_without_interface_name(self): "prefix": 30, "gateway": None, "metric": -1, + "type": None, "table": None, }, { @@ -2921,6 +2939,7 @@ def test_route_without_interface_name(self): "prefix": 64, "gateway": None, "metric": -1, + "type": None, "table": None, }, ], @@ -4966,6 +4985,22 @@ def test_table_not_found_when_validate_route_tables(self): self.connection_index, ) + def test_type_route_with_gateway(self): + """ + Test that the route type route can not have a gateway + """ + + self.test_connections[0]["ip"]["route"][0]["type"] = "blackhole" + self.assertRaisesRegex( + ValidationError, + "a {0} route can not have a gateway '{1}'".format( + self.test_connections[0]["ip"]["route"][0]["type"], + self.test_connections[0]["ip"]["route"][0]["gateway"], + ), + self.validator.validate, + self.test_connections, + ) + class TestValidatorRoutingRules(Python26CompatTestCase): def setUp(self):