From 4ba7bb3139e5b0d1f8478c34a610b440263e138a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Antti=20Myyr=C3=A4?= <antti.myyra@upcloud.com>
Date: Mon, 18 Sep 2023 15:06:04 +0300
Subject: [PATCH 1/5] feat(inventory): Inventory support for labels & server
 metadata

---
 plugins/inventory/upcloud.py                 | 51 +++++++++-
 tests/unit/plugins/inventory/test_upcloud.py | 99 +++++++++++++++++++-
 2 files changed, 145 insertions(+), 5 deletions(-)

diff --git a/plugins/inventory/upcloud.py b/plugins/inventory/upcloud.py
index 133f613..9849263 100644
--- a/plugins/inventory/upcloud.py
+++ b/plugins/inventory/upcloud.py
@@ -6,8 +6,8 @@
       - Antti Myyrä (@ajmyyra)
     short_description: Ansible dynamic inventory plugin for UpCloud.
     requirements:
-        - python >= 3.4
-        - upcloud-api >= 2.0.0
+        - python >= 3.7
+        - upcloud-api >= 2.5.0
     description:
         - Reads inventories from UpCloud API.
         - Uses a YAML configuration file that ends with upcloud.(yml|yaml).
@@ -55,6 +55,12 @@
           type: list
           elements: str
           required: false
+        labels:
+          description: Populate inventory with instances having these labels
+          default: []
+          type: list
+          elements: str
+          required: false
         states:
           description: Populate inventory with instances with these states.
           default: []
@@ -78,8 +84,9 @@
 password: YOUR_PASSWORD
 zones:
   - nl-ams1
-tags:
-  - database
+labels:
+  - role=prod
+  - foo
 
 # Group by a zone with prefix e.g. "upcloud_zone_us-nyc1"
 # and state with prefix e.g. "server_state_running"
@@ -92,10 +99,14 @@
 """
 
 import os
+from typing import List
 from ansible.errors import AnsibleError
 from ansible.module_utils._text import to_native
 from ansible.plugins.inventory import BaseInventoryPlugin, Constructable
 from ansible.release import __version__
+from ansible.utils.display import Display
+
+display = Display()
 
 try:
     import upcloud_api
@@ -145,6 +156,7 @@ def _get_servers(self):
 
     def _filter_servers(self):
         if self.get_option("zones"):
+            display.vv("Choosing servers by zone")
             tmp = []
             for server in self.servers:
                 if server.zone in self.get_option("zones"):
@@ -153,6 +165,7 @@ def _filter_servers(self):
             self.servers = tmp
 
         if self.get_option("states"):
+            display.vv("Choosing servers by server state")
             tmp = []
             for server in self.servers:
                 if server.state in self.get_option("states"):
@@ -161,6 +174,7 @@ def _filter_servers(self):
             self.servers = tmp
 
         if self.get_option("tags"):
+            display.vv("Choosing servers by tags")
             tmp = []
             for server in self.servers:
                 disqualified = False
@@ -173,7 +187,25 @@ def _filter_servers(self):
 
             self.servers = tmp
 
+        if self.get_option("labels"):
+            display.vv("Choosing servers by labels")
+            tmp = []
+            for server in self.servers:
+                disqualified = False
+                for wanted_label in self.get_option("labels"):
+                    server_labels = _parse_server_labels(server.labels['label'])
+                    for server_label in server_labels:
+                        display.vvvv(f"Comparing {wanted_label} against {server_label}")
+                        if wanted_label not in server_label:
+                            disqualified = True
+
+                if not disqualified:
+                    tmp.append(server)
+
+            self.servers = tmp
+
         if self.get_option("network"):
+            display.vv("Choosing servers by network")
             try:
                 self.network = self._fetch_network_details(self.get_option("network"))
             except UpCloudAPIError as exp:
@@ -198,6 +230,8 @@ def _set_server_attributes(self, server):
         self.inventory.set_variable(server.hostname, "firewall", to_native(server_details.firewall))
         self.inventory.set_variable(server.hostname, "plan", to_native(server.plan))
         self.inventory.set_variable(server.hostname, "tags", list(server_details.tags))
+        self.inventory.set_variable(server.hostname, "metadata", to_native(server_details.metadata))
+        self.inventory.set_variable(server.hostname, "labels", list(_parse_server_labels(server.labels["label"])))
 
         ipv4_addrs = []
         ipv6_addrs = []
@@ -287,3 +321,12 @@ def parse(self, inventory, loader, path, cache=True):
         self._read_config_data(path)
 
         self._populate()
+
+
+def _parse_server_labels(labels: List):
+    processed = []
+
+    for label in labels:
+        processed.append(f"{label['key']}={label['value']}")
+
+    return processed
diff --git a/tests/unit/plugins/inventory/test_upcloud.py b/tests/unit/plugins/inventory/test_upcloud.py
index 27b8ca5..8b6eedd 100644
--- a/tests/unit/plugins/inventory/test_upcloud.py
+++ b/tests/unit/plugins/inventory/test_upcloud.py
@@ -16,6 +16,7 @@ class Server:
         'core_number',
         'firewall',
         'hostname',
+        'metadata',
         'memory_amount',
         'nic_model',
         'title',
@@ -63,6 +64,9 @@ def get_servers():
             'core_number': '2',
             'created': 1599136169,
             'hostname': 'server1',
+            'labels': {
+                'label': []
+            },
             'license': 0,
             'memory_amount': '4096',
             'plan': '2xCPU-4GB',
@@ -79,6 +83,14 @@ def get_servers():
             'core_number': '1',
             'created': 1598526425,
             'hostname': 'server2',
+            'labels': {
+                'label': [
+                    {
+                        'key': 'foo',
+                        'value': 'bar'
+                    }
+                ]
+            },
             'license': 0,
             'memory_amount': '2048',
             'plan': '1xCPU-2GB',
@@ -90,6 +102,25 @@ def get_servers():
             'uuid': '004d5201-e2ff-4325-7ac6-a274f1c517b7',
             'zone': 'nl-ams1',
             'tags': []
+        },
+        {
+            'core_number': '1',
+            'created': 1598526319,
+            'hostname': 'server3',
+            'labels': {
+                'label': []
+            },
+            'license': 0,
+            'memory_amount': '2048',
+            'plan': '1xCPU-2GB',
+            'plan_ipv4_bytes': '0',
+            'plan_ipv6_bytes': '0',
+            'simple_backup': 'no',
+            'state': 'started',
+            'title': 'Server #3',
+            'uuid': '0003295f-343a-44a2-8080-fb8196a6802a',
+            'zone': 'nl-ams1',
+            'tags': []
         }
     ]
 
@@ -163,6 +194,14 @@ def get_server2_details():
         'created': 1598526425,
         'firewall': 'on',
         'hostname': 'server2',
+        'labels': {
+            'label': [
+                {
+                    'key': 'foo',
+                    'value': 'bar'
+                }
+            ]
+        },
         'license': 0,
         'memory_amount': '2048',
         'metadata': 'no',
@@ -207,6 +246,57 @@ def get_server2_details():
     })
 
 
+def get_server3_details():
+    return Server(**{
+        'boot_order': 'disk',
+        'core_number': '1',
+        'created': 1598526319,
+        'firewall': 'on',
+        'hostname': 'server3',
+        'license': 0,
+        'memory_amount': '2048',
+        'metadata': 'yes',
+        'networking': {
+            'interfaces': {
+                'interface': [
+                    {
+                        'bootable': 'no',
+                        'index': 1,
+                        'ip_addresses': {
+                            'ip_address': [
+                                {
+                                    'address': '172.16.0.3',
+                                    'family': 'IPv4',
+                                    'floating': 'no'
+                                }
+                            ]
+                        },
+                        'mac': '3b:a6:ba:4a:4b:10',
+                        'network': '035146a5-7a85-408b-b1f8-21925164a7d3',
+                        'source_ip_filtering': 'yes',
+                        'type': 'private'
+                    }
+                ]
+            }
+        },
+        'nic_model': 'virtio',
+        'plan': '1xCPU-2GB',
+        'plan_ipv4_bytes': '0',
+        'plan_ipv6_bytes': '0',
+        'remote_access_enabled': 'no',
+        'remote_access_password': 'fooBar',
+        'remote_access_type': 'vnc',
+        'simple_backup': 'no',
+        'state': 'started',
+        'timezone': 'UTC',
+        'title': 'Server #3',
+        'uuid': '0003295f-343a-44a2-8080-fb8196a6802a',
+        'video_model': 'vga',
+        'zone': 'nl-ams1',
+        'tags': [],
+    })
+
+
 def _mock_initialize_client():
     return
 
@@ -225,7 +315,9 @@ def get_option(option):
 
 def test_populate_hostvars(inventory, mocker):
     inventory._fetch_servers = mocker.MagicMock(side_effect=get_servers)
-    inventory._fetch_server_details = mocker.MagicMock(side_effects=[get_server1_details, get_server2_details])
+    inventory._fetch_server_details = mocker.MagicMock(
+        side_effects=[get_server1_details, get_server2_details, get_server3_details]
+    )
     inventory.get_option = mocker.MagicMock(side_effect=get_option)
 
     inventory._initialize_upcloud_client = _mock_initialize_client
@@ -235,7 +327,12 @@ def test_populate_hostvars(inventory, mocker):
 
     host1 = inventory.inventory.get_host('server1')
     host2 = inventory.inventory.get_host('server2')
+    host3 = inventory.inventory.get_host('server3')
 
     assert host1.vars['id'] == "00229adf-0e46-49b5-a8f7-cbd638d11f6a"
     assert host1.vars['state'] == "started"
     assert host2.vars['plan'] == "1xCPU-2GB"
+    assert len(host2.vars['labels']) == 1
+    assert host2.vars['labels'][0] == "foo=bar"
+    assert host3.vars['id'] == "0003295f-343a-44a2-8080-fb8196a6802a"
+    assert len(host3.vars['labels']) == 0

From 6a66a668720660b7d78f487026780692b32ebbb6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Antti=20Myyr=C3=A4?= <antti.myyra@upcloud.com>
Date: Thu, 21 Sep 2023 18:33:31 +0300
Subject: [PATCH 2/5] feat(groups & server interfaces): Filter by server
 groups, skip servers with when wanted network interface does not exist

---
 README.md                    |  10 +-
 plugins/inventory/upcloud.py | 208 +++++++++++++++++++++++++----------
 2 files changed, 153 insertions(+), 65 deletions(-)

diff --git a/README.md b/README.md
index 80b8dd8..7697f8a 100644
--- a/README.md
+++ b/README.md
@@ -22,7 +22,7 @@ UpCloud Collection requires [UpCloud API's Python bindings](https://pypi.org/pro
 newer in order to work. It can be installed from the Python Package Index with the `pip` tool:
 
 ```bash
-pip3 install upcloud-api>=2.0.0
+pip3 install upcloud-api>=2.5.0
 ```
 
 The collection itself can be installed with the `ansible-galaxy` command that comes with the Ansible package:
@@ -67,12 +67,14 @@ You can filter based on multiple data points:
 plugin: community.upcloud.upcloud
 zones:
   - fi-hel2
-tags:
-  - app
-  - db
+labels:
+  - role=prod
+  - foo
 states:
   - started
+connect_with: private_ipv4
 network: 035a0a8a-7704-4da5-820d-129fc8232714
+server_group: Group name or UUID
 ```
 
 Servers can also be grouped by status, zone etc by specifying them as `keyed_groups`.
diff --git a/plugins/inventory/upcloud.py b/plugins/inventory/upcloud.py
index 9849263..df92dd9 100644
--- a/plugins/inventory/upcloud.py
+++ b/plugins/inventory/upcloud.py
@@ -35,7 +35,7 @@
             type: str
             required: false
         connect_with:
-            description: Connect to the server with the specified choice.
+            description: Connect to the server with the specified choice. Server is skipped if chosen type is not available.
             default: public_ipv4
             type: str
             choices:
@@ -43,42 +43,48 @@
                 - public_ipv6
                 - hostname
                 - private_ipv4
+                - utility_ipv4
+        server_group:
+            description: Populate inventory with instances in this server group (UUID or title)
+            default: ""
+            type: str
+            required: false
         zones:
-          description: Populate inventory with instances in these zones.
-          default: []
-          type: list
-          elements: str
-          required: false
+            description: Populate inventory with instances in these zones.
+            default: []
+            type: list
+            elements: str
+            required: false
         tags:
-          description: Populate inventory with instances with these tags.
-          default: []
-          type: list
-          elements: str
-          required: false
+            description: Populate inventory with instances with these tags.
+            default: []
+            type: list
+            elements: str
+            required: false
         labels:
-          description: Populate inventory with instances having these labels
-          default: []
-          type: list
-          elements: str
-          required: false
+            description: Populate inventory with instances having all these labels
+            default: []
+            type: list
+            elements: str
+            required: false
         states:
-          description: Populate inventory with instances with these states.
-          default: []
-          type: list
-          elements: str
-          required: false
+            description: Populate inventory with instances with these states.
+            default: []
+            type: list
+            elements: str
+            required: false
         network:
-          description: Populate inventory with instances which are attached to this network name or UUID.
-          default: ""
-          type: str
-          required: false
+            description: Populate inventory with instances which are attached to this network name or UUID.
+            default: ""
+            type: str
+            required: false
 '''
 
 EXAMPLES = r"""
 # Minimal example. `UPCLOUD_USERNAME` and `UPCLOUD_PASSWORD` are available as environment variables.
 plugin: community.upcloud.upcloud
 
-# Example with locations, types, groups, username and password
+# Example with username and password
 plugin: community.upcloud.upcloud
 username: YOUR_USERNAME
 password: YOUR_PASSWORD
@@ -88,6 +94,16 @@
   - role=prod
   - foo
 
+# Example with locations, labels and server_group
+plugin: community.upcloud.upcloud
+zones:
+  - es-mad1
+  - fi-hel2
+labels:
+  - role=prod
+  - foo
+server_group: group name or uuid
+
 # Group by a zone with prefix e.g. "upcloud_zone_us-nyc1"
 # and state with prefix e.g. "server_state_running"
 plugin: community.upcloud.upcloud
@@ -116,6 +132,11 @@
     UC_AVAILABLE = False
 
 
+class NoAvailableAddressException(Exception):
+    """Raised when requested address type is not available"""
+    pass
+
+
 class InventoryModule(BaseInventoryPlugin, Constructable):
     name = 'upcloud'
 
@@ -130,7 +151,7 @@ def _initialize_upcloud_client(self):
         )
 
         self.client = upcloud_api.CloudManager(self.username, self.password)
-        self.client.api.user_agent = "upcloud-ansible-inventory/{0}".format(__version__)
+        self.client.api.user_agent = f"upcloud-ansible-inventory/{__version__}"
 
         api_root_env = "UPCLOUD_API_ROOT"
         if os.getenv(api_root_env):
@@ -151,6 +172,9 @@ def _fetch_server_details(self, uuid):
     def _fetch_network_details(self, uuid):
         return self.client.get_network(uuid)
 
+    def _fetch_server_groups(self):
+        return self.client.api.get_request("/server-group/")
+
     def _get_servers(self):
         self.servers = self._fetch_servers()
 
@@ -194,6 +218,9 @@ def _filter_servers(self):
                 disqualified = False
                 for wanted_label in self.get_option("labels"):
                     server_labels = _parse_server_labels(server.labels['label'])
+                    if len(server_labels) == 0:
+                        disqualified = True
+
                     for server_label in server_labels:
                         display.vvvv(f"Comparing {wanted_label} against {server_label}")
                         if wanted_label not in server_label:
@@ -215,28 +242,58 @@ def _filter_servers(self):
             if getattr(self.network, "servers"):
                 for server in self.servers:
                     for net_server in self.network.servers["server"]:
-                        if server.uuid == net_server.uuid:
+                        if server.uuid == net_server["uuid"]:
                             tmp.append(server)
 
             self.servers = tmp
 
-    def _set_server_attributes(self, server):
+        if self.get_option("server_group"):
+            display.vv("Choosing servers by server group")
+            wanted_group = self.get_option("server_group")
+
+            try:
+                raw_groups = self._fetch_server_groups()
+                groups = raw_groups["server_groups"]["server_group"]
+            except UpCloudAPIError as exp:
+                raise AnsibleError(str(exp))
+
+            tmp = []
+            server_group = None
+            for group in groups:
+                if str(wanted_group).lower() in [group["uuid"].lower(), group["title"].lower()]:
+                    server_group = group["uuid"]
+                    break
+
+            if not server_group:
+                raise AnsibleError(f"Requested server group {wanted_group} does not exist")
+
+            for server in self.servers:
+                if server.server_group == server_group:
+                    tmp.append(server)
+
+            self.servers = tmp
+
+    def _get_server_attributes(self, server):
         server_details = self._fetch_server_details(server.uuid)
 
-        self.inventory.set_variable(server.hostname, "id", to_native(server.uuid))
-        self.inventory.set_variable(server.hostname, "hostname", to_native(server.hostname))
-        self.inventory.set_variable(server.hostname, "state", to_native(server.state))
-        self.inventory.set_variable(server.hostname, "zone", to_native(server.zone))
-        self.inventory.set_variable(server.hostname, "firewall", to_native(server_details.firewall))
-        self.inventory.set_variable(server.hostname, "plan", to_native(server.plan))
-        self.inventory.set_variable(server.hostname, "tags", list(server_details.tags))
-        self.inventory.set_variable(server.hostname, "metadata", to_native(server_details.metadata))
-        self.inventory.set_variable(server.hostname, "labels", list(_parse_server_labels(server.labels["label"])))
+        def _new_attribute(key, attribute):
+            return {"key": key, "attribute": attribute}
+
+        attributes = [
+            _new_attribute("id", to_native(server.uuid)),
+            _new_attribute("hostname", to_native(server.hostname)),
+            _new_attribute("state", to_native(server.state)), _new_attribute("zone", to_native(server.zone)),
+            _new_attribute("firewall", to_native(server_details.firewall)),
+            _new_attribute("plan", to_native(server.plan)), _new_attribute("tags", list(server_details.tags)),
+            _new_attribute("metadata", to_native(server_details.metadata)),
+            _new_attribute("labels", list(_parse_server_labels(server.labels["label"]))),
+            _new_attribute("server_group", to_native(server_details.server_group))
+        ]
 
         ipv4_addrs = []
         ipv6_addrs = []
         publ_addrs = []
-        priv_addrs = []
+        util_addrs = []
         for iface in server_details.networking["interfaces"]["interface"]:
             for addr in iface["ip_addresses"]["ip_address"]:
                 address = addr.get("address")
@@ -245,41 +302,55 @@ def _set_server_attributes(self, server):
                     ipv4_addrs.append(address)
                 else:
                     ipv6_addrs.append(address)
-
                 if iface.get("type") == "public":
                     publ_addrs.append(address)
-                else:
-                    priv_addrs.append(address)
+                if iface.get("type") == "utility":
+                    util_addrs.append(address)
+
+        public_ipv4 = list(set(ipv4_addrs) & set(publ_addrs))
+        public_ipv6 = list(set(ipv6_addrs) & set(publ_addrs))
+
+        # We default to IPv4 when available
+        if len(public_ipv4) > 0:
+            attributes.append(_new_attribute("public_ip", to_native(public_ipv4[0])))
+        elif len(public_ipv6) > 0:
+            attributes.append(_new_attribute("public_ip", to_native(public_ipv4[0])))
+
+        if len(util_addrs) > 0:
+            attributes.append(_new_attribute("utility_ip", to_native(util_addrs[0])))
 
         connect_with = self.get_option("connect_with")
         if connect_with == "public_ipv4":
-            possible_addresses = list(set(ipv4_addrs) & set(publ_addrs))
-            if len(possible_addresses) == 0:
-                raise AnsibleError(
-                    "No available public IPv4 addresses for server {0} ({1})".format(server.uuid, server.hostname)
-                )
-            self.inventory.set_variable(server.hostname, "ansible_host", to_native(possible_addresses[0]))
+            if len(public_ipv4) == 0:
+                raise NoAvailableAddressException(
+                    f"No available public IPv4 addresses for server {server.uuid} ({server.hostname})")
+            attributes.append(_new_attribute("ansible_host", to_native(public_ipv4[0])))
+
         elif connect_with == "public_ipv6":
-            possible_addresses = list(set(ipv6_addrs) & set(publ_addrs))
-            if len(possible_addresses) == 0:
-                raise AnsibleError(
-                    "No available public IPv6 addresses for server {0} ({1})".format(server.uuid, server.hostname)
-                )
-            self.inventory.set_variable(server.hostname, "ansible_host", to_native(possible_addresses[0]))
+            if len(public_ipv6) == 0:
+                raise NoAvailableAddressException(
+                    f"No available public IPv6 addresses for server {server.uuid} ({server.hostname})")
+            attributes.append(_new_attribute("ansible_host", to_native(public_ipv6[0])))
+        elif connect_with == "utility_ipv4":
+            if len(util_addrs) == 0:
+                raise NoAvailableAddressException(
+                    f"No available utility addresses for server {server.uuid} ({server.hostname})")
+            attributes.append(_new_attribute("ansible_host", to_native(util_addrs[0])))
         elif connect_with == "hostname":
-            self.inventory.set_variable(server.hostname, "ansible_host", to_native(server.hostname))
+            attributes.append(_new_attribute("ansible_host", to_native(server.hostname)))
         elif connect_with == "private_ipv4":
             if self.get_option("network"):
                 for iface in server_details.networking["interfaces"]["interface"]:
-                    if iface.network == self.network.uuid:
-                        self.inventory.set_variable(
-                            server.hostname,
+                    if iface["network"] == self.network.uuid:
+                        attributes.append(_new_attribute(
                             "ansible_host",
-                            to_native(iface["ip_addresses"]["ip_address"][0].get("address"))
+                            to_native(iface["ip_addresses"]["ip_address"][0].get("address")))
                         )
             else:
                 raise AnsibleError("You can only connect with private IPv4 if you specify a network")
 
+        return attributes
+
     def verify_file(self, path):
         """Return if a file can be used by this plugin"""
         return (
@@ -297,8 +368,21 @@ def _populate(self):
         self.inventory.add_group(group="upcloud")
 
         for server in self.servers:
+            display.vv(f"Evaluating server {server.uuid} ({server.hostname})")
+
+            try:
+                attributes = self._get_server_attributes(server)
+            except NoAvailableAddressException as e:
+                display.vv(str(e))
+                display.v(
+                    f"Skipping server {server.hostname} as it doesn't have requested connection "
+                    f"type ({self.get_option('connect_with')}) available"
+                )
+                continue
+
             self.inventory.add_host(server.hostname, group="upcloud")
-            self._set_server_attributes(server)
+            for attr in attributes:
+                self.inventory.set_variable(server.hostname, attr["key"], attr["attribute"])
 
             strict = self.get_option('strict')
 
@@ -316,7 +400,9 @@ def parse(self, inventory, loader, path, cache=True):
         super(InventoryModule, self).parse(inventory, loader, path, cache)
 
         if not UC_AVAILABLE:
-            raise AnsibleError("UpCloud dynamic inventory plugin requires upcloud-api Python module")
+            raise AnsibleError(
+                "UpCloud dynamic inventory plugin requires upcloud-api Python module, "
+                + "see https://pypi.org/project/upcloud-api/")
 
         self._read_config_data(path)
 

From 145d6b0a8596c558eaf99254da3994717632fc60 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Antti=20Myyr=C3=A4?= <antti.myyra@upcloud.com>
Date: Thu, 21 Sep 2023 18:41:56 +0300
Subject: [PATCH 3/5] chore(tests): Ignore GPLv3 check in sanity tests for
 Ansible 2.17

---
 tests/sanity/ignore-2.17.txt | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 tests/sanity/ignore-2.17.txt

diff --git a/tests/sanity/ignore-2.17.txt b/tests/sanity/ignore-2.17.txt
new file mode 100644
index 0000000..c37f47d
--- /dev/null
+++ b/tests/sanity/ignore-2.17.txt
@@ -0,0 +1 @@
+plugins/inventory/upcloud.py validate-modules:missing-gplv3-license

From 922044eb4bd8aa2682cc30b66eb7b3c67ff1e016 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Antti=20Myyr=C3=A4?= <antti.myyra@upcloud.com>
Date: Mon, 25 Sep 2023 14:01:32 +0300
Subject: [PATCH 4/5] chore(inventory): Change labels functionality to include
 anything matching

---
 plugins/inventory/upcloud.py | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/plugins/inventory/upcloud.py b/plugins/inventory/upcloud.py
index df92dd9..cf49941 100644
--- a/plugins/inventory/upcloud.py
+++ b/plugins/inventory/upcloud.py
@@ -62,7 +62,7 @@
             elements: str
             required: false
         labels:
-            description: Populate inventory with instances having all these labels
+            description: Populate inventory with instances with any of these labels, either just key or value ("foo" or "bar") or as a whole tag ("foo=bar")
             default: []
             type: list
             elements: str
@@ -215,19 +215,12 @@ def _filter_servers(self):
             display.vv("Choosing servers by labels")
             tmp = []
             for server in self.servers:
-                disqualified = False
                 for wanted_label in self.get_option("labels"):
                     server_labels = _parse_server_labels(server.labels['label'])
-                    if len(server_labels) == 0:
-                        disqualified = True
-
                     for server_label in server_labels:
                         display.vvvv(f"Comparing {wanted_label} against {server_label}")
-                        if wanted_label not in server_label:
-                            disqualified = True
-
-                if not disqualified:
-                    tmp.append(server)
+                        if wanted_label in server_label:
+                            tmp.append(server)
 
             self.servers = tmp
 

From 1a64a8639fd31149fea2048395a0136fa584099b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Antti=20Myyr=C3=A4?= <antti.myyra@upcloud.com>
Date: Mon, 25 Sep 2023 16:24:07 +0300
Subject: [PATCH 5/5] chore(test): Test inventory filtering

---
 plugins/inventory/upcloud.py                 |   2 +-
 tests/unit/plugins/inventory/test_upcloud.py | 161 +++++++++++++++++--
 2 files changed, 151 insertions(+), 12 deletions(-)

diff --git a/plugins/inventory/upcloud.py b/plugins/inventory/upcloud.py
index cf49941..e408e8f 100644
--- a/plugins/inventory/upcloud.py
+++ b/plugins/inventory/upcloud.py
@@ -218,7 +218,7 @@ def _filter_servers(self):
                 for wanted_label in self.get_option("labels"):
                     server_labels = _parse_server_labels(server.labels['label'])
                     for server_label in server_labels:
-                        display.vvvv(f"Comparing {wanted_label} against {server_label}")
+                        display.vvvv(f"Comparing wanted label {wanted_label} against labels {server_label} of server {server.hostname}")
                         if wanted_label in server_label:
                             tmp.append(server)
 
diff --git a/tests/unit/plugins/inventory/test_upcloud.py b/tests/unit/plugins/inventory/test_upcloud.py
index 8b6eedd..52535df 100644
--- a/tests/unit/plugins/inventory/test_upcloud.py
+++ b/tests/unit/plugins/inventory/test_upcloud.py
@@ -16,37 +16,61 @@ class Server:
         'core_number',
         'firewall',
         'hostname',
-        'metadata',
+        'labels',
         'memory_amount',
         'nic_model',
+        'plan',
+        'simple_backup',
         'title',
         'timezone',
         'video_model',
         'vnc',
         'vnc_password',
-        'plan',
     ]
 
     optional_fields = [
-        'plan',
-        'core_number',
-        'memory_amount',
+        'avoid_host',
         'boot_order',
+        'core_number',
         'firewall',
+        'labels',
+        'login_user',
+        'memory_amount',
+        'networking',
         'nic_model',
+        'password_delivery',
+        'plan',
+        'server_group',
+        'simple_backup',
         'timezone',
+        'metadata',
+        'user_data',
         'video_model',
         'vnc_password',
-        'password_delivery',
-        'avoid_host',
-        'login_user',
-        'user_data',
     ]
 
     def __init__(self, **entries):
         self.__dict__.update(entries)
 
 
+class Network:
+    """
+    Simple class representation of UpCloud Network instance for testing purposes.
+    """
+
+    ATTRIBUTES = {
+        'name': None,
+        'type': None,
+        'uuid': None,
+        'zone': None,
+        'ip_networks': None,
+        'servers': None,
+    }
+
+    def __init__(self, **entries):
+        self.__dict__.update(entries)
+
+
 @pytest.fixture()
 def inventory():
     r = InventoryModule()
@@ -108,7 +132,16 @@ def get_servers():
             'created': 1598526319,
             'hostname': 'server3',
             'labels': {
-                'label': []
+                'label': [
+                    {
+                        'key': 'foo',
+                        'value': 'bar'
+                    },
+                    {
+                        'key': 'private',
+                        'value': 'yes'
+                    }
+                ]
             },
             'license': 0,
             'memory_amount': '2048',
@@ -138,6 +171,9 @@ def get_server1_details():
         'created': 1599136169,
         'firewall': 'on',
         'hostname': 'server1',
+        'labels': {
+            'label': []
+        },
         'license': 0,
         'memory_amount': '4096',
         'metadata': 'no',
@@ -253,6 +289,18 @@ def get_server3_details():
         'created': 1598526319,
         'firewall': 'on',
         'hostname': 'server3',
+        'labels': {
+            'label': [
+                {
+                    'key': 'foo',
+                    'value': 'bar'
+                },
+                {
+                    'key': 'private',
+                    'value': 'yes'
+                }
+            ]
+        },
         'license': 0,
         'memory_amount': '2048',
         'metadata': 'yes',
@@ -297,6 +345,36 @@ def get_server3_details():
     })
 
 
+def get_network_details(uuid):
+    return Network(**{
+        "name": "Test private net",
+        "type": "private",
+        "uuid": uuid,
+        "zone": "nl-ams1",
+        "ip_networks": {
+            "ip_network": [
+                {
+                    "address": "172.16.0.0/24",
+                    "dhcp": "yes",
+                    "dhcp_default_route": "no",
+                    "dhcp_dns": [
+                        "172.16.0.10",
+                        "172.16.1.10"
+                    ],
+                    "family": "IPv4",
+                    "gateway": "172.16.0.1"
+                }
+            ]
+        },
+        "labels": [],
+        "servers": {
+            "server": [
+                {"uuid": "0003295f-343a-44a2-8080-fb8196a6802a", "title": "Server #2"}
+            ]
+        }
+    })
+
+
 def _mock_initialize_client():
     return
 
@@ -331,8 +409,69 @@ def test_populate_hostvars(inventory, mocker):
 
     assert host1.vars['id'] == "00229adf-0e46-49b5-a8f7-cbd638d11f6a"
     assert host1.vars['state'] == "started"
+    assert len(host1.vars['labels']) == 0
     assert host2.vars['plan'] == "1xCPU-2GB"
     assert len(host2.vars['labels']) == 1
     assert host2.vars['labels'][0] == "foo=bar"
     assert host3.vars['id'] == "0003295f-343a-44a2-8080-fb8196a6802a"
-    assert len(host3.vars['labels']) == 0
+    assert len(host3.vars['labels']) == 2
+
+
+def get_filtered_labeled_option(option):
+    options = {
+        'plugin': 'community.upcloud.upcloud',
+        'labels': ['foo=bar'],
+    }
+    return options.get(option)
+
+
+def test_filtering_with_labels(inventory, mocker):
+    inventory._fetch_servers = mocker.MagicMock(side_effect=get_servers)
+    inventory._fetch_server_details = mocker.MagicMock(
+        side_effects=[get_server1_details, get_server2_details, get_server3_details]
+    )
+    inventory.get_option = mocker.MagicMock(side_effect=get_filtered_labeled_option)
+
+    inventory._initialize_upcloud_client = _mock_initialize_client
+    inventory._test_upcloud_credentials = _mock_test_credentials
+
+    inventory._populate()
+
+    assert len(inventory.inventory.hosts) == 2
+    host2 = inventory.inventory.get_host('server2')
+    host3 = inventory.inventory.get_host('server3')
+
+    assert host2.vars['id'] == "004d5201-e2ff-4325-7ac6-a274f1c517b7"
+    assert host2.vars['labels'][0] == "foo=bar"
+    assert host3.vars['id'] == "0003295f-343a-44a2-8080-fb8196a6802a"
+    assert len(host3.vars['labels']) == 2
+    assert host3.vars['labels'][1] == "private=yes"
+
+
+def get_filtered_connect_with_option(option):
+    options = {
+        'plugin': 'community.upcloud.upcloud',
+        'connect_with': 'private_ipv4',
+        'network': '035146a5-7a85-408b-b1f8-21925164a7d3'
+    }
+    return options.get(option)
+
+
+def test_filtering_with_connect_with(inventory, mocker):
+    inventory._fetch_servers = mocker.MagicMock(side_effect=get_servers)
+    inventory._fetch_server_details = mocker.MagicMock(
+        side_effects=[get_server1_details, get_server2_details, get_server3_details]
+    )
+    inventory.get_option = mocker.MagicMock(side_effect=get_filtered_connect_with_option)
+
+    inventory._initialize_upcloud_client = _mock_initialize_client
+    inventory._test_upcloud_credentials = _mock_test_credentials
+
+    inventory._fetch_network_details = get_network_details
+
+    inventory._populate()
+
+    assert len(inventory.inventory.hosts) == 1
+    host3 = inventory.inventory.get_host('server3')
+
+    assert host3.vars['id'] == "0003295f-343a-44a2-8080-fb8196a6802a"