flow-accounting: T4105: drop "sflow agent-address auto"

The implementation of the "auto" option to specify the sflow/netflow
agent-address is very error prone. The current implementation will determine
the IP address used for the "auto" value as follow:

Get BGP router-id
1) If not found use OSPF router-id
2) If not found use OSPFv3 router-id
3) If not found use "the first IP address found on the system

Well, what is the "first IP address found"? Also this changes if DHCP is in use.
Also another disadvantage is when the BGP/OSPF/OSPFv3 router-id is changed,
the agent-address is not updated upon the next reboot of the system.

This task is about removing the "auto" keyword from the CLI at all and make it
either entirely configurable by the user and hardcode the value in CLI, or not
use this at all.

If "auto" is specified we will query the system in the above order and set the
proper router-id in the CLI. If none can be found the CLI node is removed.

4 files changed, 35 insertions, 35 deletions

diff --git a/interface-definitions/flow-accounting-conf.xml.in b/interface-definitions/flow-accounting-conf.xml.in
index 7b110e733..ba5c70979 100644
--- a/interface-definitions/flow-accounting-conf.xml.in
+++ b/interface-definitions/flow-accounting-conf.xml.in
@@ -363,19 +363,13 @@
                     <script>${vyos_completion_dir}/list_local_ips.sh --ipv4</script>
                   </completionHelp>
                   <valueHelp>
-                    <format>auto</format>
-                    <description>auto select sFlow agent-address (default)</description>
-                  </valueHelp>
-                  <valueHelp>
                     <format>ipv4</format>
                     <description>sFlow IPv4 agent address</description>
                   </valueHelp>
                   <constraint>
                     <validator name="ipv4-address"/>
-                    <regex>^auto$</regex>
                   </constraint>
                 </properties>
-                <defaultValue>auto</defaultValue>
               </leafNode>
               <leafNode name="sampling-rate">
                 <properties>
diff --git a/smoketest/configs/bgp-big-as-cloud b/smoketest/configs/bgp-big-as-cloud
index 694243d1e..10660ec87 100644
--- a/smoketest/configs/bgp-big-as-cloud
+++ b/smoketest/configs/bgp-big-as-cloud
@@ -1819,6 +1819,12 @@ system {
             }
             version 9
         }
+        sflow {
+            agent-address auto
+            server 1.2.3.4 {
+                port 1234
+            }
+        }
         syslog-facility daemon
     }
     host-name vyos
diff --git a/src/conf_mode/flow_accounting_conf.py b/src/conf_mode/flow_accounting_conf.py
index 9467e805c..86fbd96b1 100755
--- a/src/conf_mode/flow_accounting_conf.py
+++ b/src/conf_mode/flow_accounting_conf.py
@@ -43,29 +43,6 @@ iptables_nflog_chain = 'VYATTA_CT_PREROUTING_HOOK'
 egress_iptables_nflog_table = 'mangle'
 egress_iptables_nflog_chain = 'FORWARD'
 
-# get sFlow agent-ip if agent-address is "auto" (default behaviour)
-def _sflow_default_agentip(config):
-    # check if any of BGP, OSPF, OSPFv3 protocols are configured and use router-id from there
-    if config.exists('protocols bgp'):
-        bgp_router_id = config.return_value("protocols bgp {} parameters router-id".format(config.list_nodes('protocols bgp')[0]))
-        if bgp_router_id:
-            return bgp_router_id
-    if config.return_value('protocols ospf parameters router-id'):
-        return config.return_value('protocols ospf parameters router-id')
-    if config.return_value('protocols ospfv3 parameters router-id'):
-        return config.return_value('protocols ospfv3 parameters router-id')
-
-    # if router-id was not found, use first available ip of any interface
-    for iface in Section.interfaces():
-        for address in Interface(iface).get_addr():
-            # return an IP, if this is not loopback
-            regex_filter = re.compile('^(?!(127)|(::1)|(fe80))(?P<ipaddr>[a-f\d\.:]+)/\d+$')
-            if regex_filter.search(address):
-                return regex_filter.search(address).group('ipaddr')
-
-    # return nothing by default
-    return None
-
 # get iptables rule dict for chain in table
 def _iptables_get_nflog(chain, table):
     # define list with rules
@@ -223,14 +200,16 @@ def verify(flow_config):
 
         # check agent-id for sFlow: we should avoid mixing IPv4 agent-id with IPv6 collectors and vice-versa
         for server in flow_config['sflow']['server']:
-            if flow_config['sflow']['agent_address'] != 'auto':
+            if 'agent_address' in flow_config['sflow']:
                 if ip_address(server).version != ip_address(flow_config['sflow']['agent_address']).version:
-                    raise ConfigError("Different IP address versions cannot be mixed in \"sflow agent-address\" and \"sflow server\". You need to set manually the same IP version for \"agent-address\" as for all sFlow servers")
+                    raise ConfigError('IPv4 and IPv6 addresses can not be mixed in "sflow agent-address" and "sflow '\
+                                      'server". You need to set the same IP version for both "agent-address" and '\
+                                      'all sFlow servers')
 
         if 'agent_address' in flow_config['sflow']:
-            agent_address = flow_config['sflow']['agent_address']
-            if agent_address != 'auto' and not is_addr_assigned(agent_address):
-                print(f'Warning: Configured "sflow agent-address" does not exist in the system!')
+            if not is_addr_assigned(agent_address):
+                tmp = flow_config['sflow']['agent_address']
+                print(f'Warning: Configured "sflow agent-address {tmp}" does not exist in the system!')
 
     # check NetFlow configuration
     if 'netflow' in flow_config:
@@ -241,7 +220,8 @@ def verify(flow_config):
         # check if configured netflow source-ip exist in the system
         if 'source_address' in flow_config['netflow']:
             if not is_addr_assigned(flow_config['netflow']['source_address']):
-                print(f'Warning: Configured "netflow source-address" does not exist on the system!')
+                tmp = flow_config['netflow']['source_address']
+                print(f'Warning: Configured "netflow source-address {tmp}" does not exist on the system!')
 
         # check if engine-id compatible with selected protocol version
         if 'engine_id' in flow_config['netflow']:
diff --git a/src/migration-scripts/flow-accounting/0-to-1 b/src/migration-scripts/flow-accounting/0-to-1
index 9e9d7132d..72cce77b0 100755
--- a/src/migration-scripts/flow-accounting/0-to-1
+++ b/src/migration-scripts/flow-accounting/0-to-1
@@ -16,6 +16,7 @@
 
 # T4099: flow-accounting: sync "source-ip" and "source-address" between netflow
 #        and sflow ion CLI
+# T4105: flow-accounting: drop "sflow agent-address auto"
 
 from sys import argv
 from vyos.configtree import ConfigTree
@@ -41,6 +42,25 @@ tmp = base + ['netflow', 'source-ip']
 if config.exists(tmp):
     config.rename(tmp, 'source-address')
 
+# T4105
+tmp = base + ['sflow', 'agent-address']
+if config.exists(tmp):
+    value = config.return_value(tmp)
+    if value == 'auto':
+        # delete the "auto"
+        config.delete(tmp)
+
+        # 1) check if BGP router-id is set
+        # 2) check if OSPF router-id is set
+        # 3) check if OSPFv3 router-id is set
+        router_id = None
+        for protocol in ['bgp', 'ospf', 'ospfv3']:
+            if config.exists(['protocols', protocol, 'parameters', 'router-id']):
+                router_id = config.return_value(['protocols', protocol, 'parameters', 'router-id'])
+                break
+        if router_id:
+            config.set(tmp, value=router_id)
+
 try:
     with open(file_name, 'w') as f:
         f.write(config.to_string())