Inventory source misc bug fixes (#917)

* DeviceModel: fix port type to 'int'

Signed-off-by: LucaNicosia <[email protected]>

* native source: do not set up default port

The default port must be set up depending on the device data, which
is not accessible within the source.
The default port assignment will be performed in the function
`set_device` of base_source

Signed-off-by: LucaNicosia <[email protected]>

* base_source: default port + inventory keys fix

Sometimes sources doesn't return the device port:
- netbox: never return the port
- native: if not specified on the host

If the port is not specified, the inventory key (which is computed using
namespace, address and port) will be incomplete.
In order to fix this issue, we correctly set the port to these items
and recreate the keys with the correct port

Signed-off-by: LucaNicosia <[email protected]>

* new tests + test fixes

Signed-off-by: LucaNicosia <[email protected]>

* unrelated test fix

this test passes locally, but somehow it doesn't on github.
setting the env-var to an empty value should fix this issue

Signed-off-by: LucaNicosia <[email protected]>

---------

Signed-off-by: LucaNicosia <[email protected]>

Author: LucaNicosia

suzieq/poller/controller/source/base_source.py

@@ -210,7 +210,7 @@ def set_device(self, inventory: Dict[str, Dict]):
         slow_host = False
         per_cmd_auth = True
         retries_on_auth_fail = 0
-        port = None
+        dev_port = None
         devtype = None
 
         if self._device:
@@ -231,21 +231,25 @@ def set_device(self, inventory: Dict[str, Dict]):
             slow_host = self._device.get('slow-host', False)
             per_cmd_auth = self._device.get('per-cmd-auth', True)
             retries_on_auth_fail = self._device.get('retries-on-auth-fail', 0)
-            port = self._device.get('port')
+            dev_port = self._device.get('port')
             devtype = self._device.get('devtype')
 
-        for node in inventory.values():
+        node_keys_to_remove = []
+        nodes_to_add = {}
+        for node_name, node in inventory.items():
             transport_tmp = node.get('transport') or transport or 'ssh'
             ignore_known_hosts_tmp = node.get('ignore_known_hosts')
+            missing_node_port = not (node_port := node.get('port'))
+            if missing_node_port:
+                node_port = dev_port or _DEFAULT_PORTS.get(transport_tmp)
             node.update({
                 'jump_host': node.get('jump_host') or jump_host,
                 'jump_host_key_file': node.get('jump_host_key_file')
                 or jump_host_key_file,
                 'ignore_known_hosts': ignore_known_hosts_tmp if
                 ignore_known_hosts_tmp is not None else ignore_known_hosts,
                 'transport': transport_tmp,
-                'port': node.get('port') or port or
-                _DEFAULT_PORTS.get(transport_tmp),
+                'port': node_port,
                 'devtype': node.get('devtype') or devtype,
                 'slow_host': node.get('slow_host', '') or slow_host,
                 'per_cmd_auth': ((node.get('per_cmd_auth', '') != '')
@@ -254,6 +258,25 @@ def set_device(self, inventory: Dict[str, Dict]):
                                                    -1) != -1) or
                                          retries_on_auth_fail)
             })
+            if missing_node_port:
+                # An inventory key is composed by
+                # '{namespace}.{address}.{port}'. If the node port is missing,
+                # the key generated is not complete.
+                # We need to assign the node to a correct key. Since we are
+                # looping on the items in the inventory, we cannot update it
+                # here. We need to do it after the loop is completed, so we are
+                # storing in "node_keys_to_remove" the invalid key and in
+                # "nodes_to_add" the current node assigned to the correct key
+                node_keys_to_remove.append(node_name)
+                new_key = f"{node['namespace']}.{node['address']}.{node_port}"
+                nodes_to_add[new_key] = node
+
+        if node_keys_to_remove:
+            for k in node_keys_to_remove:
+                del inventory[k]
+
+        if nodes_to_add:
+            inventory.update(nodes_to_add)
 
     def _validate_device(self):
         if self._device:

suzieq/poller/controller/source/native.py

@@ -12,8 +12,6 @@
 
 logger = logging.getLogger(__name__)
 
-_DEFAULT_PORTS = {'http': 80, 'https': 443, 'ssh': 22}
-
 
 class HostModel(BaseModel):
     """Model used to validate the hosts of a native inventory
@@ -22,7 +20,7 @@ class HostModel(BaseModel):
     password: Optional[str] = Field(default=None)
     keyfile: Optional[str] = Field(default=None)
     devtype: Optional[str] = Field(default=None)
-    port: str = Field(default=None)
+    port: Optional[str] = Field(default=None)
     address: str = Field(default=None)
     transport: str = Field(default=None)
     url: str
@@ -47,7 +45,7 @@ def validate_and_set(cls, url: str, values):
         except KeyError:
             raise ValueError(
                 f"Transport '{transport}' not supported for host {address}")
-        port = decoded_url.port or _DEFAULT_PORTS.get(transport)
+        port = decoded_url.port
         devtype = None
         keyfile = None
 

suzieq/poller/controller/utils/inventory_models.py

@@ -34,7 +34,7 @@ class DeviceModel(BaseModel):
     retries_on_auth_fail: Optional[int] = Field(alias='retries-on-auth-fail',
                                                 default=1)
     transport: Optional[PollerTransport]
-    port: Optional[str]
+    port: Optional[int]
     devtype: Optional[str]
 
     # pylint: disable=no-self-argument

tests/unit/poller/controller/data/exp-inventory.yaml

@@ -6,15 +6,15 @@ auths:
     password: strong-passoword
     key-passphrase: null
     type: null
-    username: null
+    username: user
 devices:
   dev0:
     devtype: null
     ignore-known-hosts: false
     jump-host: [email protected]
     jump-host-key-file: null
     name: dev0
-    port: null
+    port: 333
     slow-host: False
     per-cmd-auth: False
     retries-on-auth-fail: 0
@@ -24,28 +24,52 @@ namespaces:
   device: dev0
   name: native-ns
   source: native0
+- auth: auth0
+  device: null
+  name: native-default
+  source: native-default
 - auth: auth0
   device: null
   name: netbox-ns
   source: netbox0
 sources:
+  native-default:
+    hosts:
+    - address: 192.168.1.2
+      devtype: eos
+      keyfile: null
+      password: null
+      port: null
+      transport: https
+      url: https://192.168.1.2 devtype=eos
+      username: null
+    - address: 192.168.1.3
+      devtype: null
+      keyfile: null
+      password: null
+      port: null
+      transport: ssh
+      url: ssh://192.168.1.3
+      username: null
+    name: native-default
+    type: null
   native0:
     hosts:
     - address: 192.168.123.164
       devtype: eos
       keyfile: null
       password: null
-      port: 443
+      port: null
       transport: https
       url: https://[email protected] devtype=eos
       username: vagrant
     - address: 192.168.123.70
       devtype: null
       keyfile: null
       password: null
-      port: 22
+      port: 123
       transport: ssh
-      url: ssh://192.168.123.70 username=admin
+      url: ssh://192.168.123.70:123 username=admin
       username: admin
     name: native0
     type: null

tests/unit/poller/controller/data/inventory.yaml

@@ -2,29 +2,38 @@ sources:
 - name: native0
   hosts:
   - url: https://[email protected] devtype=eos
-  - url: ssh://192.168.123.70 username=admin
+  - url: ssh://192.168.123.70:123 username=admin
 - name: netbox0
   type: netbox
   token: MY-TOKEN
   url: http://localhost:8000
   tag: suzieq
+- name: native-default
+  hosts:
+  - url: https://192.168.1.2 devtype=eos
+  - url: ssh://192.168.1.3
 
 auths:
 - name: auth0
   password: plain:strong-passoword
+  username: user
 
 devices:
 - name: dev0
   transport: ssh
   jump-host: [email protected]
   per-cmd-auth: False
   retries-on-auth-fail: 0
+  port: 333
 
 namespaces:
 - name: native-ns
   device: dev0
   auth: auth0
   source: native0
+- name: native-default
+  auth: auth0
+  source: native-default
 - name: netbox-ns
   auth: auth0
   source: netbox0

tests/unit/poller/controller/sources/netbox/netbox_faker.py

@@ -274,7 +274,8 @@ def generate_data(self) -> Tuple[Dict, Dict]:
                     namespace = dev_site
                 else:
                     namespace = self._namespace
-                dev_key = f'{namespace}.{ip}'
+                port = 22
+                dev_key = f'{namespace}.{ip}.{port}'
                 exp_inventory[dev_key] = _BASE_INVENTORY_DEVICE.copy()
 
                 exp_inventory[dev_key].update({

tests/unit/poller/controller/sources/test_devices.py

@@ -49,7 +49,7 @@
         'transport': 'http',
         'port': 443
     },
-    'native-ns.192.168.123.171.22':
+    'native-ns.192.168.123.171':
     {
         'address': '192.168.123.171',
         'hostname': None,
@@ -101,7 +101,7 @@
         'transport': 'http',
         'port': 443
     },
-    'native-ns.192.168.123.171.22':
+    'native-ns.192.168.123.171.80':
     {
         'address': '192.168.123.171',
         'hostname': None,