From 4ad1185536756890295fdd8b566fb65a75ef429d Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Sat, 6 Apr 2024 11:24:49 +0200 Subject: ipoe: T6205: error in migration script logic while renaming mac-address to mac node The problem was introduced in [1] but the config migrator part unfortunately was added to the wrong version [2]. As IPoE config version 0 was only active during the 1.3 development cycle and VyOS 1.3.0 was already released with config version 1 we can safely drop the migrator 0-to-1 and move the code to 1-to-2 to properly support upgrades from VyOS 1.3 -> 1.4 or newer. 1: https://github.com/vyos/vyos-1x/commit/05df2a5f021f0c7aab7c06db645d210858b6e98d#diff-08291bf77870abe3af8bbe3e8ce4bbf344fd0498b2c5c75a75aa7235d381c88eL168 2: https://github.com/vyos/vyos-1x/commit/05df2a5f021f0c7aab7c06db645d210858b6e98d#diff-b8bb58b75607d3653e74d82eff02442f9f3ab82698f160ba37858f7cdf6c79ccR44-R46 (cherry picked from commit a5ccc06c08d3a9696f1c03c8d0c7de78ce1fd3c5) --- src/conf_mode/service_ipoe-server.py | 4 +- src/migration-scripts/ipoe-server/0-to-1 | 71 -------------------------------- src/migration-scripts/ipoe-server/1-to-2 | 36 +++++++++++++++- 3 files changed, 36 insertions(+), 75 deletions(-) delete mode 100755 src/migration-scripts/ipoe-server/0-to-1 (limited to 'src') diff --git a/src/conf_mode/service_ipoe-server.py b/src/conf_mode/service_ipoe-server.py index 852b714eb..11e950782 100755 --- a/src/conf_mode/service_ipoe-server.py +++ b/src/conf_mode/service_ipoe-server.py @@ -68,8 +68,8 @@ def verify(ipoe): for interface, iface_config in ipoe['interface'].items(): verify_interface_exists(interface) if 'client_subnet' in iface_config and 'vlan' in iface_config: - raise ConfigError('Option "client-subnet" incompatible with "vlan"!' - 'Use "ipoe client-ip-pool" instead.') + raise ConfigError('Option "client-subnet" and "vlan" are mutually exclusive, ' + 'use "client-ip-pool" instead!') verify_accel_ppp_authentication(ipoe, local_users=False) verify_accel_ppp_ip_pool(ipoe) diff --git a/src/migration-scripts/ipoe-server/0-to-1 b/src/migration-scripts/ipoe-server/0-to-1 deleted file mode 100755 index a6dd46ac1..000000000 --- a/src/migration-scripts/ipoe-server/0-to-1 +++ /dev/null @@ -1,71 +0,0 @@ -#!/usr/bin/env python3 -# -# Copyright (C) 2022-2024 VyOS maintainers and contributors -# -# This program is free software; you can redistribute it and/or modify -# it under the terms of the GNU General Public License version 2 or later as -# published by the Free Software Foundation. -# -# This program 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 General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program. If not, see . - -# - T4703: merge vlan-id and vlan-range to vlan CLI node - -# L2|L3 -> l2|l3 -# mac-address -> mac -# network-mode -> mode - -from sys import argv, exit -from vyos.configtree import ConfigTree - -if len(argv) < 2: - print("Must specify file name!") - exit(1) - -file_name = argv[1] - -with open(file_name, 'r') as f: - config_file = f.read() - -config = ConfigTree(config_file) -base = ['service', 'ipoe-server'] -if not config.exists(base): - # Nothing to do - exit(0) - -if config.exists(base + ['authentication', 'interface']): - for interface in config.list_nodes(base + ['authentication', 'interface']): - config.rename(base + ['authentication', 'interface', interface, 'mac-address'], 'mac') - - mac_base = base + ['authentication', 'interface', interface, 'mac'] - for mac in config.list_nodes(mac_base): - vlan_config = mac_base + [mac, 'vlan-id'] - if config.exists(vlan_config): - config.rename(vlan_config, 'vlan') - -for interface in config.list_nodes(base + ['interface']): - base_path = base + ['interface', interface] - for vlan in ['vlan-id', 'vlan-range']: - if config.exists(base_path + [vlan]): - print(interface, vlan) - for tmp in config.return_values(base_path + [vlan]): - config.set(base_path + ['vlan'], value=tmp, replace=False) - config.delete(base_path + [vlan]) - - if config.exists(base_path + ['network-mode']): - tmp = config.return_value(base_path + ['network-mode']) - config.delete(base_path + ['network-mode']) - # Change L2|L3 to lower case l2|l3 - config.set(base_path + ['mode'], value=tmp.lower()) - -try: - with open(file_name, 'w') as f: - f.write(config.to_string()) -except OSError as e: - print("Failed to save the modified config: {}".format(e)) - exit(1) diff --git a/src/migration-scripts/ipoe-server/1-to-2 b/src/migration-scripts/ipoe-server/1-to-2 index 378702693..f1335b5a5 100755 --- a/src/migration-scripts/ipoe-server/1-to-2 +++ b/src/migration-scripts/ipoe-server/1-to-2 @@ -1,6 +1,6 @@ #!/usr/bin/env python3 # -# Copyright (C) 2023 VyOS maintainers and contributors +# Copyright (C) 2023-2024 VyOS maintainers and contributors # # This program is free software; you can redistribute it and/or modify # it under the terms of the GNU General Public License version 2 or later as @@ -14,6 +14,11 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . +# - T4703: merge vlan-id and vlan-range to vlan CLI node +# L2|L3 -> l2|l3 +# mac-address -> mac +# network-mode -> mode + # - changed cli of all named pools # - moved gateway-address from pool to global configuration with / netmask # gateway can exist without pool if radius is used @@ -39,12 +44,39 @@ with open(file_name, 'r') as f: config = ConfigTree(config_file) base = ['service', 'ipoe-server'] -pool_base = base + ['client-ip-pool'] + if not config.exists(base): exit(0) +if config.exists(base + ['authentication', 'interface']): + for interface in config.list_nodes(base + ['authentication', 'interface']): + config.rename(base + ['authentication', 'interface', interface, 'mac-address'], 'mac') + + mac_base = base + ['authentication', 'interface', interface, 'mac'] + for mac in config.list_nodes(mac_base): + vlan_config = mac_base + [mac, 'vlan-id'] + if config.exists(vlan_config): + config.rename(vlan_config, 'vlan') + +for interface in config.list_nodes(base + ['interface']): + base_path = base + ['interface', interface] + for vlan in ['vlan-id', 'vlan-range']: + if config.exists(base_path + [vlan]): + print(interface, vlan) + for tmp in config.return_values(base_path + [vlan]): + config.set(base_path + ['vlan'], value=tmp, replace=False) + config.delete(base_path + [vlan]) + + if config.exists(base_path + ['network-mode']): + tmp = config.return_value(base_path + ['network-mode']) + config.delete(base_path + ['network-mode']) + # Change L2|L3 to lower case l2|l3 + config.set(base_path + ['mode'], value=tmp.lower()) + +pool_base = base + ['client-ip-pool'] if not config.exists(pool_base): exit(0) + default_pool = '' gateway = '' -- cgit v1.2.3 From c2b0381a0c26d82288fcc39f9fba1f3c25132aec Mon Sep 17 00:00:00 2001 From: Christian Breunig Date: Sun, 7 Apr 2024 07:41:15 +0200 Subject: ipoe: T6205: fix conditional branch error in config migrator Commit a5ccc06c0 ("ipoe: T6205: error in migration script logic while renaming mac-address to mac node") added a conditional path into the config which could result in the migrated config not beeing written if precondition was not met. (cherry picked from commit 2bbded1e485614d40b2e95165629487537fd1757) --- src/migration-scripts/ipoe-server/1-to-2 | 67 +++++++++++++++----------------- 1 file changed, 32 insertions(+), 35 deletions(-) (limited to 'src') diff --git a/src/migration-scripts/ipoe-server/1-to-2 b/src/migration-scripts/ipoe-server/1-to-2 index f1335b5a5..6a7111541 100755 --- a/src/migration-scripts/ipoe-server/1-to-2 +++ b/src/migration-scripts/ipoe-server/1-to-2 @@ -62,7 +62,6 @@ for interface in config.list_nodes(base + ['interface']): base_path = base + ['interface', interface] for vlan in ['vlan-id', 'vlan-range']: if config.exists(base_path + [vlan]): - print(interface, vlan) for tmp in config.return_values(base_path + [vlan]): config.set(base_path + ['vlan'], value=tmp, replace=False) config.delete(base_path + [vlan]) @@ -74,40 +73,38 @@ for interface in config.list_nodes(base + ['interface']): config.set(base_path + ['mode'], value=tmp.lower()) pool_base = base + ['client-ip-pool'] -if not config.exists(pool_base): - exit(0) - -default_pool = '' -gateway = '' - -#named pool migration -namedpools_base = pool_base + ['name'] - -for pool_name in config.list_nodes(namedpools_base): - pool_path = namedpools_base + [pool_name] - if config.exists(pool_path + ['subnet']): - subnet = config.return_value(pool_path + ['subnet']) - config.set(pool_base + [pool_name, 'range'], value=subnet, replace=False) - # Get netmask from subnet - mask = subnet.split("/")[1] - if config.exists(pool_path + ['next-pool']): - next_pool = config.return_value(pool_path + ['next-pool']) - config.set(pool_base + [pool_name, 'next-pool'], value=next_pool) - if not default_pool: - default_pool = pool_name - if config.exists(pool_path + ['gateway-address']) and mask: - gateway = f'{config.return_value(pool_path + ["gateway-address"])}/{mask}' - config.set(base + ['gateway-address'], value=gateway, replace=False) - -if not default_pool and config.list_nodes(namedpools_base): - default_pool = config.list_nodes(namedpools_base)[0] - -config.delete(namedpools_base) - -if default_pool: - config.set(base + ['default-pool'], value=default_pool) -# format as tag node -config.set_tag(pool_base) +if config.exists(pool_base): + default_pool = '' + gateway = '' + + #named pool migration + namedpools_base = pool_base + ['name'] + + for pool_name in config.list_nodes(namedpools_base): + pool_path = namedpools_base + [pool_name] + if config.exists(pool_path + ['subnet']): + subnet = config.return_value(pool_path + ['subnet']) + config.set(pool_base + [pool_name, 'range'], value=subnet, replace=False) + # Get netmask from subnet + mask = subnet.split("/")[1] + if config.exists(pool_path + ['next-pool']): + next_pool = config.return_value(pool_path + ['next-pool']) + config.set(pool_base + [pool_name, 'next-pool'], value=next_pool) + if not default_pool: + default_pool = pool_name + if config.exists(pool_path + ['gateway-address']) and mask: + gateway = f'{config.return_value(pool_path + ["gateway-address"])}/{mask}' + config.set(base + ['gateway-address'], value=gateway, replace=False) + + if not default_pool and config.list_nodes(namedpools_base): + default_pool = config.list_nodes(namedpools_base)[0] + + config.delete(namedpools_base) + + if default_pool: + config.set(base + ['default-pool'], value=default_pool) + # format as tag node + config.set_tag(pool_base) try: with open(file_name, 'w') as f: -- cgit v1.2.3