Fix failed proxy retrieval on login (#171)

tests/test_compat.py

@@ -0,0 +1,87 @@
+import pytest
+from packaging.version import Version
+
+from zabbix_cli import compat
+
+
+def test_packaging_version_release_sanity():
+    """Ensures that the `Version.release` tuple is in the correct format and
+    supports users running pre-release versions of Zabbix."""
+    assert Version("7.0.0").release == (7, 0, 0)
+    # Test that all types of pre-releases evaluate to the same release tuple
+    for pr in ["a1", "b1", "rc1", "alpha1", "beta1"]:
+        assert Version(f"7.0.0{pr}").release == (7, 0, 0)
+        assert Version(f"7.0.0{pr}").release >= (7, 0, 0)
+        assert Version(f"7.0.0{pr}").release <= (7, 0, 0)
+
+
+@pytest.mark.parametrize(
+        "version, expect",
+        [
+            # TODO (pederhan): decide on a set of versions we test against
+            # instead of coming up with them on the fly, such as here.
+            # Do we test against only major versions or minor versions as well?
+            (Version("7.0.0"), "proxyid"),
+            (Version("6.0.0"), "proxy_hostid"),
+            (Version("5.0.0"), "proxy_hostid"),
+            (Version("3.0.0"), "proxy_hostid"),
+            (Version("2.0.0"), "proxy_hostid"),
+            (Version("1.0.0"), "proxy_hostid"),
+        ],
+)
+def test_host_proxyid(version: Version, expect: str):
+    assert compat.host_proxyid(version) == expect
+
+
+@pytest.mark.parametrize(
+        "version, expect",
+        [
+            (Version("7.0.0"), "username"),
+            (Version("6.0.0"), "username"),
+            (Version("6.2.0"), "username"),
+            (Version("6.4.0"), "username"),
+            (Version("5.4.0"), "username"),
+            (Version("5.4.1"), "username"),
+            (Version("5.3.9"), "user"),
+            (Version("5.2.0"), "user"),
+            (Version("5.2"), "user"),
+            (Version("5.0"), "user"),
+            (Version("4.0"), "user"),
+            (Version("2.0"), "user"),
+        ],
+)
+def test_login_user_name(version: Version, expect: str):
+    assert compat.login_user_name(version) == expect
+
+
+@pytest.mark.parametrize(
+        "version, expect",
+        [
+            (Version("7.0.0"), "name"),
+            (Version("6.0.0"), "host"),
+            (Version("5.0.0"), "host"),
+            (Version("3.0.0"), "host"),
+            (Version("2.0.0"), "host"),
+            (Version("1.0.0"), "host"),
+        ],
+)
+def test_proxy_name(version: Version, expect: str):
+    assert compat.proxy_name(version) == expect
+
+@pytest.mark.parametrize(
+        "version, expect",
+        [
+            (Version("7.0.0"), "username"),
+            (Version("6.4.0"), "username"),
+            (Version("6.0.0"), "username"),
+            # NOTE: special case here where we use "alias" instead of "username"
+            # even though it was deprecated in 5.4.0 (matches historical zabbix_cli behavior)
+            (Version("5.4.0"), "alias"),
+            (Version("5.0.0"), "alias"),
+            (Version("3.0.0"), "alias"),
+            (Version("2.0.0"), "alias"),
+            (Version("1.0.0"), "alias"),
+        ],
+)
+def test_user_name(version: Version, expect: str):
+    assert compat.user_name(version) == expect

zabbix_cli/cli.py

@@ -36,9 +36,8 @@
 import textwrap
 import time
 
-from packaging.version import Version
-
 import zabbix_cli
+from zabbix_cli import compat
 import zabbix_cli.apiutils
 import zabbix_cli.utils
 from zabbix_cli.prettytable import ALL, FRAME, PrettyTable
@@ -902,8 +901,8 @@ def do_show_host(self, args):
                 available = host['interfaces'][0]['available']
             else:
                 available = host['available']
-            proxy = self.zapi.proxy.get(proxyids=host['proxy_hostid'])
-            proxy_name = proxy[0]['host'] if proxy else ""
+            proxy = self.zapi.proxy.get(proxyids=host[compat.host_proxyid(self.zabbix_version)])
+            proxy_name = proxy[0][compat.proxy_name(self.zabbix_version)] if proxy else ""
             if self.output_format == 'json':
                 result_columns[result_columns_key] = {'hostid': host['hostid'],
                                                       'host': host['host'],
@@ -2801,7 +2800,7 @@ def do_create_host(self, args):
             query = {
                 'host': host,
                 'groups': hostgroups_list,
-                'proxy_hostid': proxy_hostid,
+                compat.host_proxyid(self.zabbix_version): proxy_hostid,
                 'status': int(host_status),
                 'interfaces': interface_list,
                 'inventory_mode': 1,
@@ -4837,10 +4836,11 @@ def do_update_host_proxy(self, args):
                 #
                 # Update proxy used to monitor the host
                 #
-
-                self.zapi.host.update(hostid=hostid,
-                                      proxy_hostid=proxy_id)
-
+                query = {
+                    'hostid': hostid,
+                    compat.host_proxyid(self.zabbix_version): proxy_id
+                }
+                self.zapi.host.update(**query)
                 logger.info('Proxy for hostname (%s) changed to (%s)', hostname, proxy)
                 self.generate_feedback('Done', 'Proxy for hostname (' + hostname + ') changed to (' + str(proxy) + ')')
 
@@ -6762,14 +6762,17 @@ def do_move_proxy_hosts(self, args):
             search_spec = {}
 
         try:
-            result = self.zapi.host.get(output='hostid',
-                                        filter={'proxy_hostid': proxy_src_id},
-                                        search=search_spec,
-                                        # Somewhat unintuitively, False here
-                                        # would implicitly add wildcards around
-                                        # the given host_filter.  Force explicit
-                                        # wildcards to avoid surprises.
-                                        searchWildcardsEnabled=True)
+            query = {
+                "output": 'hostid',
+                "filter": {compat.host_proxyid(self.zabbix_version): proxy_src_id},
+                "search": search_spec,
+                # Somewhat unintuitively, False here
+                # would implicitly add wildcards around
+                # the given host_filter.  Force explicit
+                # wildcards to avoid surprises.
+                "searchWildcardsEnabled": True
+            }
+            result = self.zapi.host.get(**query)
         except Exception as e:
             logger.error('Problems getting host list from SRC proxy %s - %s', proxy_src, e)
             self.generate_feedback('Error', 'Problems getting host list from SRC proxy %s' + proxy_src)
@@ -6781,7 +6784,7 @@ def do_move_proxy_hosts(self, args):
             if host_count > 0:
                 query = {
                     "hosts": result,
-                    "proxy_hostid": proxy_dst_id
+                    compat.host_proxyid(self.zabbix_version): proxy_dst_id
                 }
                 self.zapi.host.massupdate(**query)
 
@@ -6927,7 +6930,7 @@ def do_load_balance_proxy_hosts(self, args):
 
                 query = {
                     "hosts": hostid_list,
-                    "proxy_hostid": proxy_specs[proxy]['id']
+                    compat.host_proxyid(self.zabbix_version): proxy_specs[proxy]['id']
                 }
 
                 result = self.zapi.host.massupdate(**query)
@@ -7380,7 +7383,6 @@ def get_proxy_id(self, proxy):
         Get the proxyid for a proxy server
         """
         if proxy != '':
-
             data = self.zapi.proxy.get(filter={"host": proxy})
 
             if data != []:
@@ -7413,7 +7415,7 @@ def get_random_proxyid(self, proxy_pattern):
             data = self.zapi.proxy.get(output=['proxyid', 'host'])
 
             for proxy in data:
-                if match_pattern.match(proxy['host']):
+                if match_pattern.match(proxy[compat.proxy_name(self.zabbix_version)]):
                     proxy_list.append(proxy['proxyid'])
 
         proxy_list_len = len(proxy_list)
@@ -7507,7 +7509,7 @@ def populate_proxyid_cache(self):
         data = self.zapi.proxy.get(output=['proxyid', 'host'])
 
         for proxy in data:
-            temp_dict[proxy['proxyid']] = proxy['host']
+            temp_dict[proxy['proxyid']] = proxy[compat.proxy_name(self.zabbix_version)]
 
         return temp_dict
 

zabbix_cli/compat.py

@@ -0,0 +1,55 @@
+"""Compatibility functions for different Zabbix versions."""
+
+from packaging.version import Version
+
+# TODO (pederhan): rewrite these functions as some sort of declarative data
+# structure that can be used to determine correct parameters based on version
+# if we end up with a lot of these functions. For now, this is fine.
+# OR we could turn it into a mixin class?
+
+# Compatibility methods for Zabbix API objects properties and method parameter names (same thing)
+# Returns the appropriate property name for the given Zabbix version.
+# 
+# FORMAT: <object>_<property>
+# EXAMPLE: user_name() (User object, name property)
+#
+# DEV NOTE: All functions follow the same pattern: 
+# Early return if the version is older than the version where the property 
+# was deprecated, otherwise return the new property name as the default.
+
+
+def host_proxyid(version: Version) -> str:
+    # https://support.zabbix.com/browse/ZBXNEXT-8500
+    # https://www.zabbix.com/documentation/7.0/en/manual/api/changes#host
+    if version.release < (7, 0, 0):
+        return "proxy_hostid"
+    return "proxyid" 
+
+
+def login_user_name(version: Version) -> str:
+    # https://support.zabbix.com/browse/ZBXNEXT-8085
+    # Deprecated in 5.4.0, removed in 6.4.0
+    # login uses different parameter names than the User object before 6.4
+    # From 6.4 and onwards, login and user.<method> use the same parameter names
+    # See: user_name
+    if version.release < (5, 4, 0):
+        return "user"
+    return "username"
+
+
+def proxy_name(version: Version) -> str:
+    # https://support.zabbix.com/browse/ZBXNEXT-8500
+    # https://www.zabbix.com/documentation/7.0/en/manual/api/changes#proxy
+    if version.release < (7, 0, 0):
+        return "host"
+    return "name"
+
+
+def user_name(version: Version) -> str:
+    # https://support.zabbix.com/browse/ZBXNEXT-8085
+    # Deprecated in 5.4, removed in 6.4
+    # However, historically we have used "alias" as the parameter name
+    # pre-6.0.0, so we maintain that behavior here
+    if version.release < (6, 0, 0):
+        return "alias"
+    return "username"

zabbix_cli/pyzabbix.py

@@ -17,6 +17,8 @@
 import requests
 from packaging.version import Version
 
+from zabbix_cli import compat
+
 
 class _NullHandler(logging.Handler):
     def emit(self, record):
@@ -99,7 +101,7 @@ def login(self, user='', password='', auth_token=''):
 
         # The username kwarg was called "user" in Zabbix 5.2 and earlier.
         # This sets the correct kwarg for the version of Zabbix we're using.
-        user_kwarg = {user_param_from_version(self.version): user}
+        user_kwarg = {compat.login_user_name(self.version): user}
 
         if auth_token == '':
 
@@ -217,13 +219,13 @@ def do_request(self, method, params=None):
 
         return response_json
 
-    def __getattr__(self, attr):
+    def __getattr__(self, attr: str):
         """Dynamically create an object class (ie: host)"""
         return ZabbixAPIObjectClass(attr, self)
 
 
 class ZabbixAPIObjectClass(object):
-    def __init__(self, name, parent):
+    def __init__(self, name: str, parent: ZabbixAPI):
         self.name = name
         self.parent = parent
 
@@ -240,3 +242,21 @@ def fn(*args, **kwargs):
             )['result']
 
         return fn
+
+    def get(self, *args, **kwargs):
+        """Provides per-endpoint overrides for the 'get' method"""
+        if self.name == "proxy":
+            # The proxy.get method changed from "host" to "name" in Zabbix 7.0
+            # https://www.zabbix.com/documentation/6.0/en/manual/api/reference/proxy/get
+            # https://www.zabbix.com/documentation/7.0/en/manual/api/reference/proxy/get
+            output_kwargs = kwargs.get("output", None)
+            params = ["name", "host"]
+            if isinstance(output_kwargs, list) and any(p in output_kwargs for p in params):
+                for param in params:
+                    try:
+                        output_kwargs.remove(param)
+                    except ValueError:
+                        pass
+                output_kwargs.append(compat.proxy_name(self.parent.version))
+                kwargs["output"] = output_kwargs
+        return self.__getattr__('get')(*args, **kwargs)