Merge pull request #2789 from vyos/mergify/bp/sagitta/pr-2777

T5688: Changed 'range' to multi in 'client-ip-pool' for accell-ppp (backport #2777)

10 files changed, 85 insertions, 107 deletions

diff --git a/data/templates/accel-ppp/config_ip_pool.j2 b/data/templates/accel-ppp/config_ip_pool.j2
index 6ac04e1a1..8e66486e6 100644
--- a/data/templates/accel-ppp/config_ip_pool.j2
+++ b/data/templates/accel-ppp/config_ip_pool.j2
@@ -12,16 +12,20 @@ gw-ip-address={{ gateway_address }}
 {%     endif %}
 {%     for pool in ordered_named_pools %}
 {%         for pool_name, pool_config in pool.items() %}
-{%             set iprange_str = pool_config.range %}
-{%             set iprange_list = pool_config.range.split('-') %}
-{%             if iprange_list | length == 2 %}
-{%                 set last_ip_oct = iprange_list[1].split('.') %}
-{%                 set iprange_str = iprange_list[0] + '-' + last_ip_oct[last_ip_oct | length - 1] %}
-{%             endif %}
-{%             if pool_config.next_pool is vyos_defined %}
+{%             if pool_config.range is vyos_defined %}
+{%                 for range in pool_config.range %}
+{%                     set iprange_str = range %}
+{%                     set iprange_list = range.split('-') %}
+{%                     if iprange_list | length == 2 %}
+{%                         set last_ip_oct = iprange_list[1].split('.') %}
+{%                         set iprange_str = iprange_list[0] + '-' + last_ip_oct[last_ip_oct | length - 1] %}
+{%                     endif %}
+{%                     if loop.last and pool_config.next_pool is vyos_defined %}
 {{ iprange_str }},name={{ pool_name }},next={{ pool_config.next_pool }}
-{%             else %}
+{%                     else %}
 {{ iprange_str }},name={{ pool_name }}
+{%                     endif %}
+{%                 endfor %}
 {%             endif %}
 {%         endfor %}
 {%     endfor %}
diff --git a/interface-definitions/include/accel-ppp/client-ip-pool.xml.i b/interface-definitions/include/accel-ppp/client-ip-pool.xml.i
index 71fe69f8d..b30a5ee01 100644
--- a/interface-definitions/include/accel-ppp/client-ip-pool.xml.i
+++ b/interface-definitions/include/accel-ppp/client-ip-pool.xml.i
@@ -27,11 +27,15 @@
           <validator name="ipv4-host"/>
           <validator name="ipv4-range-mask"  argument="-m 24 -r"/>
         </constraint>
+        <multi/>
       </properties>
     </leafNode>
     <leafNode name="next-pool">
       <properties>
         <help>Next pool name</help>
+        <completionHelp>
+          <path>${COMP_WORDS[@]:1:${#COMP_WORDS[@]}-4}</path>
+        </completionHelp>
         <valueHelp>
           <format>txt</format>
           <description>Name of IP pool</description>
diff --git a/interface-definitions/include/accel-ppp/default-pool.xml.i b/interface-definitions/include/accel-ppp/default-pool.xml.i
index a08b066b1..e06642c37 100644
--- a/interface-definitions/include/accel-ppp/default-pool.xml.i
+++ b/interface-definitions/include/accel-ppp/default-pool.xml.i
@@ -2,6 +2,9 @@
 <leafNode name="default-pool">
   <properties>
     <help>Default client IP pool name</help>
+    <completionHelp>
+      <path>${COMP_WORDS[@]:1:${#COMP_WORDS[@]}-3} client-ip-pool</path>
+    </completionHelp>
     <valueHelp>
       <format>txt</format>
       <description>Default IP pool</description>
diff --git a/src/conf_mode/vpn_l2tp.py b/src/conf_mode/vpn_l2tp.py
index 03a27d3cd..1a91951b4 100755
--- a/src/conf_mode/vpn_l2tp.py
+++ b/src/conf_mode/vpn_l2tp.py
@@ -71,7 +71,7 @@ def verify(l2tp):
             raise ConfigError('DA/CoE server key required!')
 
     if dict_search('authentication.mode', l2tp) in ['local', 'noauth']:
-        if not l2tp['client_ip_pool'] and not l2tp['client_ipv6_pool']:
+        if not dict_search('client_ip_pool', l2tp) and not dict_search('client_ipv6_pool', l2tp):
             raise ConfigError(
                 "L2TP local auth mode requires local client-ip-pool or client-ipv6-pool to be configured!")
         if dict_search('client_ip_pool', l2tp) and not dict_search('default_pool', l2tp):
diff --git a/src/migration-scripts/ipoe-server/1-to-2 b/src/migration-scripts/ipoe-server/1-to-2
index c8cec6835..11d7911e9 100755
--- a/src/migration-scripts/ipoe-server/1-to-2
+++ b/src/migration-scripts/ipoe-server/1-to-2
@@ -57,7 +57,7 @@ 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)
+        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']):
diff --git a/src/migration-scripts/l2tp/4-to-5 b/src/migration-scripts/l2tp/4-to-5
index 496dc83d6..3176f895a 100755
--- a/src/migration-scripts/l2tp/4-to-5
+++ b/src/migration-scripts/l2tp/4-to-5
@@ -24,7 +24,7 @@ import os
 from sys import argv
 from sys import exit
 from vyos.configtree import ConfigTree
-
+from vyos.base import Warning
 
 if len(argv) < 2:
     print("Must specify file name!")
@@ -45,33 +45,33 @@ if not config.exists(pool_base):
     exit(0)
 default_pool = ''
 range_pool_name = 'default-range-pool'
-subnet_base_name = 'default-subnet-pool'
-number = 1
-subnet_pool_name = f'{subnet_base_name}-{number}'
-prev_subnet_pool = subnet_pool_name
-if config.exists(pool_base + ['subnet']):
-    default_pool = subnet_pool_name
-    for subnet in config.return_values(pool_base + ['subnet']):
-        config.set(pool_base + [subnet_pool_name, 'range'], value=subnet)
-        if prev_subnet_pool != subnet_pool_name:
-            config.set(pool_base + [prev_subnet_pool, 'next-pool'],
-                       value=subnet_pool_name)
-            prev_subnet_pool = subnet_pool_name
-        number += 1
-        subnet_pool_name = f'{subnet_base_name}-{number}'
-
-    config.delete(pool_base + ['subnet'])
 
 if config.exists(pool_base + ['start']) and config.exists(pool_base + ['stop']):
+    def is_legalrange(ip1: str, ip2: str, mask: str):
+        from ipaddress import IPv4Interface
+        interface1 = IPv4Interface(f'{ip1}/{mask}')
+
+        interface2 = IPv4Interface(f'{ip2}/{mask}')
+        return interface1.network.network_address == interface2.network.network_address and interface2.ip > interface1.ip
+
     start_ip = config.return_value(pool_base + ['start'])
     stop_ip = config.return_value(pool_base + ['stop'])
-    ip_range = f'{start_ip}-{stop_ip}'
+    if is_legalrange(start_ip, stop_ip,'24'):
+        ip_range = f'{start_ip}-{stop_ip}'
+        config.set(pool_base + [range_pool_name, 'range'], value=ip_range, replace=False)
+        default_pool = range_pool_name
+    else:
+        Warning(
+            f'L2TP client-ip-pool range start-ip:{start_ip} and stop-ip:{stop_ip} can not be migrated.')
+
     config.delete(pool_base + ['start'])
     config.delete(pool_base + ['stop'])
-    config.set(pool_base + [range_pool_name, 'range'], value=ip_range)
-    if default_pool:
-        config.set(pool_base + [range_pool_name, 'next-pool'],
-                   value=default_pool)
+
+if config.exists(pool_base + ['subnet']):
+    for subnet in config.return_values(pool_base + ['subnet']):
+        config.set(pool_base + [range_pool_name, 'range'], value=subnet, replace=False)
+
+    config.delete(pool_base + ['subnet'])
     default_pool = range_pool_name
 
 if default_pool:
diff --git a/src/migration-scripts/pppoe-server/6-to-7 b/src/migration-scripts/pppoe-server/6-to-7
index d856c1f34..b94ce57f9 100755
--- a/src/migration-scripts/pppoe-server/6-to-7
+++ b/src/migration-scripts/pppoe-server/6-to-7
@@ -29,7 +29,7 @@ import os
 from sys import argv
 from sys import exit
 from vyos.configtree import ConfigTree
-
+from vyos.base import Warning
 
 if len(argv) < 2:
     print("Must specify file name!")
@@ -48,38 +48,35 @@ if not config.exists(base):
 
 if not config.exists(pool_base):
     exit(0)
+
 default_pool = ''
 range_pool_name = 'default-range-pool'
 
-subnet_base_name = 'default-subnet-pool'
-number = 1
-subnet_pool_name = f'{subnet_base_name}-{number}'
-prev_subnet_pool = subnet_pool_name
 #Default nameless pools migrations
-if config.exists(pool_base + ['subnet']):
-    default_pool = subnet_pool_name
-    for subnet in config.return_values(pool_base + ['subnet']):
-        config.set(pool_base + [subnet_pool_name, 'range'], value=subnet)
-        if prev_subnet_pool != subnet_pool_name:
-            config.set(pool_base + [prev_subnet_pool, 'next-pool'],
-                       value=subnet_pool_name)
-            prev_subnet_pool = subnet_pool_name
-        number += 1
-        subnet_pool_name = f'{subnet_base_name}-{number}'
-
-    config.delete(pool_base + ['subnet'])
-
 if config.exists(pool_base + ['start']) and config.exists(pool_base + ['stop']):
+    def is_legalrange(ip1: str, ip2: str, mask: str):
+        from ipaddress import IPv4Interface
+        interface1 = IPv4Interface(f'{ip1}/{mask}')
+        interface2 = IPv4Interface(f'{ip2}/{mask}')
+        return interface1.network.network_address == interface2.network.network_address and interface2.ip > interface1.ip
+
     start_ip = config.return_value(pool_base + ['start'])
     stop_ip = config.return_value(pool_base + ['stop'])
-    ip_range = f'{start_ip}-{stop_ip}'
+    if is_legalrange(start_ip, stop_ip, '24'):
+        ip_range = f'{start_ip}-{stop_ip}'
+        config.set(pool_base + [range_pool_name, 'range'], value=ip_range, replace=False)
+        default_pool = range_pool_name
+    else:
+        Warning(
+            f'PPPoE client-ip-pool range start-ip:{start_ip} and stop-ip:{stop_ip} can not be migrated.')
     config.delete(pool_base + ['start'])
     config.delete(pool_base + ['stop'])
-    config.set(pool_base + [range_pool_name, 'range'], value=ip_range)
-    if default_pool:
-        config.set(pool_base + [range_pool_name, 'next-pool'],
-                   value=default_pool)
+
+if config.exists(pool_base + ['subnet']):
     default_pool = range_pool_name
+    for subnet in config.return_values(pool_base + ['subnet']):
+        config.set(pool_base + [range_pool_name, 'range'], value=subnet, replace=False)
+    config.delete(pool_base + ['subnet'])
 
 gateway = ''
 if config.exists(base + ['gateway-address']):
@@ -97,7 +94,7 @@ if config.exists(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)
+            config.set(pool_base + [pool_name, 'range'], value=subnet, replace=False)
         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)
diff --git a/src/migration-scripts/pptp/2-to-3 b/src/migration-scripts/pptp/2-to-3
index 98dc5c2a6..091cb68ec 100755
--- a/src/migration-scripts/pptp/2-to-3
+++ b/src/migration-scripts/pptp/2-to-3
@@ -23,7 +23,7 @@ import os
 from sys import argv
 from sys import exit
 from vyos.configtree import ConfigTree
-
+from vyos.base import Warning
 
 if len(argv) < 2:
     print("Must specify file name!")
@@ -46,13 +46,24 @@ if not config.exists(pool_base):
 range_pool_name = 'default-range-pool'
 
 if config.exists(pool_base + ['start']) and config.exists(pool_base + ['stop']):
+    def is_legalrange(ip1: str, ip2: str, mask: str):
+        from ipaddress import IPv4Interface
+        interface1 = IPv4Interface(f'{ip1}/{mask}')
+        interface2 = IPv4Interface(f'{ip2}/{mask}')
+        return interface1.network.network_address == interface2.network.network_address and interface2.ip > interface1.ip
+
     start_ip = config.return_value(pool_base + ['start'])
     stop_ip = config.return_value(pool_base + ['stop'])
-    ip_range = f'{start_ip}-{stop_ip}'
+    if is_legalrange(start_ip, stop_ip, '24'):
+        ip_range = f'{start_ip}-{stop_ip}'
+        config.set(pool_base + [range_pool_name, 'range'], value=ip_range, replace=False)
+        config.set(base + ['default-pool'], value=range_pool_name)
+    else:
+        Warning(
+            f'PPTP client-ip-pool range start-ip:{start_ip} and stop-ip:{stop_ip} can not be migrated.')
+
     config.delete(pool_base + ['start'])
     config.delete(pool_base + ['stop'])
-    config.set(pool_base + [range_pool_name, 'range'], value=ip_range)
-    config.set(base + ['default-pool'], value=range_pool_name)
 # format as tag node
 config.set_tag(pool_base)
 
diff --git a/src/migration-scripts/sstp/4-to-5 b/src/migration-scripts/sstp/4-to-5
index 3a86c79ec..95e482713 100755
--- a/src/migration-scripts/sstp/4-to-5
+++ b/src/migration-scripts/sstp/4-to-5
@@ -43,21 +43,12 @@ if not config.exists(base):
 if not config.exists(pool_base):
     exit(0)
 
-subnet_base_name = 'default-subnet-pool'
-number = 1
-subnet_pool_name = f'{subnet_base_name}-{number}'
-prev_subnet_pool = subnet_pool_name
+range_pool_name = 'default-range-pool'
+
 if config.exists(pool_base + ['subnet']):
-    default_pool = subnet_pool_name
+    default_pool = range_pool_name
     for subnet in config.return_values(pool_base + ['subnet']):
-        config.set(pool_base + [subnet_pool_name, 'range'], value=subnet)
-        if prev_subnet_pool != subnet_pool_name:
-            config.set(pool_base + [prev_subnet_pool, 'next-pool'],
-                       value=subnet_pool_name)
-            prev_subnet_pool = subnet_pool_name
-        number += 1
-        subnet_pool_name = f'{subnet_base_name}-{number}'
-
+        config.set(pool_base + [range_pool_name, 'range'], value=subnet, replace=False)
     config.delete(pool_base + ['subnet'])
     config.set(base + ['default-pool'], value=default_pool)
 # format as tag node
diff --git a/src/validators/ipv4-range-mask b/src/validators/ipv4-range-mask
index 7bb4539af..9373328ff 100755
--- a/src/validators/ipv4-range-mask
+++ b/src/validators/ipv4-range-mask
@@ -1,12 +1,5 @@
 #!/bin/bash
 
-# snippet from https://stackoverflow.com/questions/10768160/ip-address-converter
-ip2dec () {
-    local a b c d ip=$@
-    IFS=. read -r a b c d <<< "$ip"
-    printf '%d\n' "$((a * 256 ** 3 + b * 256 ** 2 + c * 256 + d))"
-}
-
 error_exit() {
   echo "Error: $1 is not a valid IPv4 address range or these IPs are not under /$2"
   exit 1
@@ -22,37 +15,12 @@ do
         r) range=${OPTARG}
     esac
 done
-if [[ "${range}" =~ "-" ]]&&[[ ! -z ${mask} ]]; then
-  # This only works with real bash (<<<) - split IP addresses into array with
-  # hyphen as delimiter
-  readarray -d - -t strarr <<< ${range}
-
-  ipaddrcheck --is-ipv4-single ${strarr[0]}
-  if [ $? -gt 0 ]; then
-    error_exit ${range} ${mask}
-  fi
 
-  ipaddrcheck --is-ipv4-single ${strarr[1]}
+if [[ "${range}" =~ "-" ]]&&[[ ! -z ${mask} ]]; then
+  ipaddrcheck --range-prefix-length ${mask} --is-ipv4-range ${range}
   if [ $? -gt 0 ]; then
     error_exit ${range} ${mask}
   fi
-
-  ${vyos_validators_dir}/numeric --range 0-32 ${mask} > /dev/null
-   if [ $? -ne 0 ]; then
-     error_exit ${range} ${mask}
-   fi
-
-  is_in_24=$( grepcidr ${strarr[0]}"/"${mask} <(echo ${strarr[1]}) )
-  if [ -z $is_in_24 ]; then
-    error_exit ${range} ${mask}
-  fi
-
-  start=$(ip2dec ${strarr[0]})
-  stop=$(ip2dec ${strarr[1]})
-  if [ $start -ge $stop ]; then
-    error_exit ${range} ${mask}
-  fi
-
   exit 0
 fi