T1435 plus other dhcp/dhcpv6-server enhancements

- T1435: dhcp-server: make ip-address optional in static-mapping
- remove [REQUIRED] from dhcpv6-server static-mapping identifier and ipv6-address
- verify if static-mapping ipv6-address is in subnet
- make help and error messages in conf-mode more descriptive
- remove regex ^$ anchors (implied in re.fullmatch)

4 files changed, 64 insertions, 43 deletions

diff --git a/interface-definitions/dhcp-server.xml b/interface-definitions/dhcp-server.xml
index 87999f496..7d42294e8 100644
--- a/interface-definitions/dhcp-server.xml
+++ b/interface-definitions/dhcp-server.xml
@@ -46,9 +46,9 @@
             <properties>
               <help>DHCP shared network name [REQUIRED]</help>
               <constraint>
-                <regex>^[-_a-zA-Z0-9.]+$</regex>
+                <regex>[-_a-zA-Z0-9.]+</regex>
               </constraint>
-              <constraintErrorMessage>Invalid DHCP pool name</constraintErrorMessage>
+              <constraintErrorMessage>Invalid shared network name. May only contain letters, numbers and .-_</constraintErrorMessage>
             </properties>
             <children>
               <leafNode name="authoritative">
@@ -151,7 +151,7 @@
                   </leafNode>
                   <leafNode name="exclude">
                     <properties>
-                      <help>IP address that needs to be excluded from DHCP lease range</help>
+                      <help>IP address to exclude from DHCP lease range</help>
                       <valueHelp>
                         <format>ipv4</format>
                         <description>IPv4 address to exclude from lease range</description>
@@ -183,9 +183,9 @@
                         <properties>
                           <help>DHCP failover peer name [REQUIRED]</help>
                           <constraint>
-                            <regex>^[-_a-zA-Z0-9.]+$</regex>
+                            <regex>[-_a-zA-Z0-9.]+</regex>
                           </constraint>
-                          <constraintErrorMessage>Invalid failover peer name</constraintErrorMessage>
+                          <constraintErrorMessage>Invalid failover peer name. May only contain letters, numbers and .-_</constraintErrorMessage>
                         </properties>
                       </leafNode>
                       <leafNode name="peer-address">
@@ -193,7 +193,7 @@
                           <help>IP address of failover peer [REQUIRED]</help>
                           <valueHelp>
                             <format>ipv4</format>
-                            <description>IPv4 address to exclude from lease range</description>
+                            <description>IPv4 address of failover peer</description>
                           </valueHelp>
                           <constraint>
                             <validator name="ipv4-address"/>
@@ -225,12 +225,12 @@
                       <help>Lease timeout in seconds (default: 86400)</help>
                       <valueHelp>
                         <format>0-4294967295</format>
-                        <description>DHCP lease time in seconds must be between 0 and 4294967295 (49 days)</description>
+                        <description>DHCP lease time in seconds</description>
                       </valueHelp>
                       <constraint>
                         <validator name="numeric" argument="--range 0-4294967295"/>
                       </constraint>
-                      <constraintErrorMessage>DHCP lease time must be 0 to 4294967295</constraintErrorMessage>
+                      <constraintErrorMessage>DHCP lease time must be between 0 and 4294967295 (49 days)</constraintErrorMessage>
                     </properties>
                   </leafNode>
                   <leafNode name="ntp-server">
@@ -288,9 +288,9 @@
                     <properties>
                       <help>DHCP lease range</help>
                       <constraint>
-                        <regex>^[-_a-zA-Z0-9.]+$</regex>
+                        <regex>[-_a-zA-Z0-9.]+</regex>
                       </constraint>
-                      <constraintErrorMessage>Invalid DHCP lease range name</constraintErrorMessage>
+                      <constraintErrorMessage>Invalid DHCP lease range name. May only contain letters, numbers and .-_</constraintErrorMessage>
                     </properties>
                     <children>
                       <leafNode name="start">
@@ -321,22 +321,22 @@
                   </tagNode>
                   <tagNode name="static-mapping">
                     <properties>
-                      <help>Static mapping for specified address type</help>
+                      <help>Name of static mapping</help>
                       <constraint>
-                        <regex>^[-_a-zA-Z0-9.]+$</regex>
+                        <regex>[-_a-zA-Z0-9.]+</regex>
                       </constraint>
-                      <constraintErrorMessage>Invalid static-mapping name</constraintErrorMessage>
+                      <constraintErrorMessage>Invalid static mapping name. May only contain letters, numbers and .-_</constraintErrorMessage>
                     </properties>
                     <children>
                       <leafNode name="disable">
                         <properties>
-                          <help>Option to disable static-mapping</help>
+                          <help>Option to disable static mapping</help>
                           <valueless/>
                         </properties>
                       </leafNode>
                       <leafNode name="ip-address">
                         <properties>
-                          <help>Static mapping for specified IP address [REQUIRED]</help>
+                          <help>Fixed IP address of static mapping</help>
                           <valueHelp>
                             <format>ipv4</format>
                             <description>IPv4 address used in static mapping</description>
@@ -348,7 +348,7 @@
                       </leafNode>
                       <leafNode name="mac-address">
                         <properties>
-                          <help>Static mapping for specified MAC address [REQUIRED]</help>
+                          <help>MAC address of static mapping [REQUIRED]</help>
                           <valueHelp>
                             <format>h:h:h:h:h:h</format>
                             <description>MAC address used in static mapping [REQUIRED]</description>
@@ -358,6 +358,7 @@
                       <leafNode name="static-mapping-parameters">
                         <properties>
                           <help>Additional static-mapping parameters for DHCP server.
+                Will be placed inside the "host" block of the mapping.
                 You must use the syntax of dhcpd.conf in this text-field.
                 Using this without proper knowledge may result in a crashed
                 DHCP server. Check system log to look for errors.</help>
@@ -414,10 +415,14 @@
                   <leafNode name="time-offset">
                     <properties>
                       <help>Offset of the client's subnet in seconds from Coordinated Universal Time (UTC)</help>
+                      <valueHelp>
+                        <format>[-]N</format>
+                        <description>Time offset (number, may be negative)</description>
+                      </valueHelp>
                       <constraint>
-                        <regex>^-?[0-9]+$</regex>
+                        <regex>-?[0-9]+</regex>
                       </constraint>
-                      <constraintErrorMessage>Invalid time offset valuee</constraintErrorMessage>
+                      <constraintErrorMessage>Invalid time offset value</constraintErrorMessage>
                     </properties>
                   </leafNode>
                   <leafNode name="time-server">
diff --git a/interface-definitions/dhcpv6-server.xml b/interface-definitions/dhcpv6-server.xml
index e18a58608..28b56a64d 100644
--- a/interface-definitions/dhcpv6-server.xml
+++ b/interface-definitions/dhcpv6-server.xml
@@ -32,9 +32,9 @@
             <properties>
               <help>DHCPv6 shared network name [REQUIRED]</help>
               <constraint>
-                <regex>^[-_a-zA-Z0-9.]+$</regex>
+                <regex>[-_a-zA-Z0-9.]+</regex>
               </constraint>
-              <constraintErrorMessage>Invalid DHCPv6 pool name</constraintErrorMessage>
+              <constraintErrorMessage>Invalid DHCPv6 shared network name. May only contain letters, numbers and .-_</constraintErrorMessage>
             </properties>
             <children>
               <leafNode name="disable">
@@ -112,9 +112,9 @@
                     <properties>
                       <help>Domain name for client to search</help>
                       <constraint>
-                        <regex>^[-_a-zA-Z0-9.]+$</regex>
+                        <regex>[-_a-zA-Z0-9.]+</regex>
                       </constraint>
-                      <constraintErrorMessage>Invalid domain name syntax</constraintErrorMessage>
+                      <constraintErrorMessage>Invalid domain name. May only contain letters, numbers and .-_</constraintErrorMessage>
                       <multi/>
                     </properties>
                   </leafNode>
@@ -157,9 +157,9 @@
                     <properties>
                       <help>NIS domain name for client to use</help>
                       <constraint>
-                        <regex>^[-_a-zA-Z0-9.]+$</regex>
+                        <regex>[-_a-zA-Z0-9.]+</regex>
                       </constraint>
-                      <constraintErrorMessage>Invalid NIS domain name syntax</constraintErrorMessage>
+                      <constraintErrorMessage>Invalid NIS domain name</constraintErrorMessage>
                     </properties>
                   </leafNode>
                   <leafNode name="nis-server">
@@ -179,9 +179,9 @@
                     <properties>
                       <help>NIS+ domain name for client to use</help>
                       <constraint>
-                        <regex>^[-_a-zA-Z0-9.]+$</regex>
+                        <regex>[-_a-zA-Z0-9.]+</regex>
                       </constraint>
-                      <constraintErrorMessage>Invalid NIS+ domain name syntax</constraintErrorMessage>
+                      <constraintErrorMessage>Invalid NIS+ domain name. May only contain letters, numbers and .-_</constraintErrorMessage>
                     </properties>
                   </leafNode>
                   <leafNode name="nisplus-server">
@@ -260,9 +260,9 @@
                     <properties>
                       <help>SIP server name</help>
                       <constraint>
-                        <regex>^[-_a-zA-Z0-9.]+$</regex>
+                        <regex>[-_a-zA-Z0-9.]+</regex>
                       </constraint>
-                      <constraintErrorMessage>Invalid SIP server name syntax</constraintErrorMessage>
+                      <constraintErrorMessage>Invalid SIP server name. May only contain letters, numbers and .-_</constraintErrorMessage>
                       <multi/>
                     </properties>
                   </leafNode>
@@ -281,7 +281,7 @@
                       <constraint>
                         <regex>[-_a-zA-Z0-9.]+</regex>
                       </constraint>
-                      <constraintErrorMessage>Invalid static-mapping name. May only contain letters, numbers and .-_</constraintErrorMessage>
+                      <constraintErrorMessage>Invalid static mapping name. May only contain letters, numbers and .-_</constraintErrorMessage>
                     </properties>
                     <children>
                       <leafNode name="disable">
@@ -292,7 +292,7 @@
                       </leafNode>
                       <leafNode name="identifier">
                         <properties>
-                          <help>Client identifier (DUID) for this static mapping [REQUIRED]</help>
+                          <help>Client identifier (DUID) for this static mapping</help>
                           <valueHelp>
                             <format>h[[:h]...]</format>
                             <description>DUID: colon-separated hex list (as used by isc-dhcp option dhcpv6.client-id)</description>
@@ -300,14 +300,15 @@
                           <constraint>
                             <regex>([0-9A-Fa-f]{1,2}[:])*([0-9A-Fa-f]{1,2})</regex>
                           </constraint>
+                          <constraintErrorMessage>Invalid DUID. Must be in the format h[[:h]...] where each \"h\" is 1 to 2 hex characters.</constraintErrorMessage>
                         </properties>
                       </leafNode>
                       <leafNode name="ipv6-address">
                         <properties>
-                          <help>Client IPv6 address for this static mapping [REQUIRED]</help>
+                          <help>Client IPv6 address for this static mapping</help>
                           <valueHelp>
                             <format>ipv6</format>
-                            <description>IPv6 address for this static mapping [REQUIRED]</description>
+                            <description>IPv6 address for this static mapping</description>
                           </valueHelp>
                           <constraint>
                             <validator name="ipv6-address"/>
diff --git a/src/conf_mode/dhcp_server.py b/src/conf_mode/dhcp_server.py
index 78927a847..6d88ea95a 100755
--- a/src/conf_mode/dhcp_server.py
+++ b/src/conf_mode/dhcp_server.py
@@ -187,7 +187,9 @@ shared-network {{ network.name }} {
         {%- for host in subnet.static_mapping %}
         {% if not host.disabled -%}
         host {% if host_decl_name -%} {{ host.name }} {%- else -%} {{ network.name }}_{{ host.name }} {%- endif %} {
+            {%- if host.ip_address %}
             fixed-address {{ host.ip_address }};
+            {%- endif %}
             hardware ethernet {{ host.mac_address }};
             {%- if host.static_parameters %}
             # The following {{ host.static_parameters | length }} line(s) were added as static-mapping-parameters in the CLI and have not been validated
@@ -728,22 +730,19 @@ def verify(dhcp):
                 raise ConfigError('No DHCP address range or active static-mapping set\n' \
                                   'for subnet {0}!'.format(subnet['network']))
 
-            # Static IP address mappings require both an IP address and MAC address
+            # Static mappings require just a MAC address (will use an IP from the dynamic pool if IP is not set)
             for mapping in subnet['static_mapping']:
-                # Static IP address must be configured
-                if not mapping['ip_address']:
-                    raise ConfigError('DHCP static lease IP address not specified for static mapping\n' \
-                                      '{0} under shared network name {1}!'.format(mapping['name'], network['name']))
 
-                # Static IP address must be in bound
-                if not ipaddress.ip_address(mapping['ip_address']) in ipaddress.ip_network(subnet['network']):
-                    raise ConfigError('DHCP static lease IP address {0} for static mapping {1}\n' \
-                                      'in shared network {2} is outside DHCP lease subnet {3}!' \
-                                      .format(mapping['ip_address'], mapping['name'], network['name'], subnet['network']))
+                if mapping['ip_address']:
+                    # Static IP address must be in bound
+                    if not ipaddress.ip_address(mapping['ip_address']) in ipaddress.ip_network(subnet['network']):
+                        raise ConfigError('DHCP static lease IP address {0} for static mapping {1}\n' \
+                                          'in shared network {2} is outside DHCP lease subnet {3}!' \
+                                          .format(mapping['ip_address'], mapping['name'], network['name'], subnet['network']))
 
                 # Static mapping requires MAC address
                 if not mapping['mac_address']:
-                     raise ConfigError('DHCP static lease MAC address not specified for static mapping\n' \
+                    raise ConfigError('DHCP static lease MAC address not specified for static mapping\n' \
                                        '{0} under shared network name {1}!'.format(mapping['name'], network['name']))
 
             # There must be one subnet connected to a listen interface.
diff --git a/src/conf_mode/dhcpv6_server.py b/src/conf_mode/dhcpv6_server.py
index f5117de53..d2769466e 100755
--- a/src/conf_mode/dhcpv6_server.py
+++ b/src/conf_mode/dhcpv6_server.py
@@ -94,8 +94,12 @@ shared-network {{ network.name }} {
         {%- for host in subnet.static_mapping %}
         {% if not host.disabled -%}
         host {{ network.name }}_{{ host.name }} {
+            {%- if host.client_identifier %}
             host-identifier option dhcp6.client-id {{ host.client_identifier }};
+            {%- endif %}
+            {%- if host.ipv6_address %}
             fixed-address6 {{ host.ipv6_address }};
+            {%- endif %}
         }
         {%- endif %}
         {%- endfor %}
@@ -384,7 +388,19 @@ def verify(dhcpv6):
                         raise ConfigError('DHCPv6 prefix {0} is not in subnet {1}\n' \
                                           'specified for shared network {2}!'.format(prefix['prefix'], subnet['network'], network['name']))
 
+            # Static mappings don't require anything (but check if IP is in subnet if it's set)
+            for mapping in subnet['static_mapping']:
+                if mapping['ipv6_address']:
+                    # Static address must be in subnet
+                    if not ipaddress.ip_address(mapping['ipv6_address']) in ipaddress.ip_network(subnet['network']):
+                        raise ConfigError('DHCPv6 static mapping IPv6 address {0} for static mapping {1}\n' \
+                                          'in shared network {2} is outside subnet {3}!' \
+                                          .format(mapping['ipv6_address'], mapping['name'], network['name'], subnet['network']))
+
         # DHCPv6 requires at least one configured address range or one static mapping
+        # (FIXME: is not actually checked right now?)
+
+        # There must be one subnet connected to a listen interface if network is not disabled.
         if not network['disabled']:
             if vyos.validate.is_subnet_connected(subnet['network']):
                 listen_ok = True