From de82d5ac71819877b4e31bd0115bdaade028bb78 Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Tue, 24 Jun 2025 13:52:43 +0200 Subject: [PATCH 01/34] Remove duplicates from the list of hostgroups --- modules/device.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/modules/device.py b/modules/device.py index e61cede..ee8b666 100644 --- a/modules/device.py +++ b/modules/device.py @@ -135,6 +135,9 @@ class PhysicalDevice: self.hostgroups = [hg.generate(f) for f in hg_format] else: self.hostgroups.append(hg.generate(hg_format)) + self.hostgroups = list(set(self.hostgroups)) + self.logger.debug(f"Host {self.name}: Should be member " + f"of groups: {self.hostgroups}") def set_template(self, prefer_config_context, overrule_custom): """Set Template""" From 753633e7d2673683e8b54df2674c30f98d3f0302 Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Tue, 24 Jun 2025 15:01:45 +0200 Subject: [PATCH 02/34] Added checks for empty list of hostgroups, improved some logging --- modules/device.py | 27 +++++++++++++++------------ modules/hostgroups.py | 14 ++++++++------ netbox_zabbix_sync.py | 2 ++ 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/modules/device.py b/modules/device.py index ee8b666..26bac0e 100644 --- a/modules/device.py +++ b/modules/device.py @@ -135,9 +135,14 @@ class PhysicalDevice: self.hostgroups = [hg.generate(f) for f in hg_format] else: self.hostgroups.append(hg.generate(hg_format)) - self.hostgroups = list(set(self.hostgroups)) - self.logger.debug(f"Host {self.name}: Should be member " - f"of groups: {self.hostgroups}") + # Remove dyuplicates and None values + self.hostgroups = list(filter(None, list(set(self.hostgroups)))) + if self.hostgroups: + self.logger.debug(f"Host {self.name}: Should be member " + f"of groups: {self.hostgroups}") + return True + return False + def set_template(self, prefer_config_context, overrule_custom): """Set Template""" @@ -180,8 +185,6 @@ class PhysicalDevice: self.logger.warning(e) raise TemplateError(e) - - def get_templates_context(self): """Get Zabbix templates from the device context""" if "zabbix" not in self.config_context: @@ -301,7 +304,8 @@ class PhysicalDevice: "name": zbx_template["name"], } ) - e = f"Host {self.name}: found template {zbx_template['name']}" + e = (f"Host {self.name}: Found template '{zbx_template['name']}' " + f"(ID:{zbx_template['templateid']})") self.logger.debug(e) # Return error should the template not be found in Zabbix if not template_match: @@ -324,7 +328,7 @@ class PhysicalDevice: if group["name"] == hg: self.group_ids.append({"groupid": group["groupid"]}) e = ( - f"Host {self.name}: matched group " + f"Host {self.name}: Matched group " f"\"{group['name']}\" (ID:{group['groupid']})" ) self.logger.debug(e) @@ -506,7 +510,6 @@ class PhysicalDevice: templateids.append({"templateid": template["templateid"]}) # Set interface, group and template configuration interfaces = self.setInterfaceDetails() - groups = self.group_ids # Set Zabbix proxy if defined self.setProxy(proxies) # Set basic data for host creation @@ -515,7 +518,7 @@ class PhysicalDevice: "name": self.visible_name, "status": self.zabbix_state, "interfaces": interfaces, - "groups": groups, + "groups": self.group_ids, "templates": templateids, "description": description, "inventory_mode": self.inventory_mode, @@ -544,7 +547,7 @@ class PhysicalDevice: # Set NetBox custom field to hostID value. self.nb.custom_fields[config["device_cf"]] = int(self.zabbix_id) self.nb.save() - msg = f"Host {self.name}: Created host in Zabbix." + msg = f"Host {self.name}: Created host in Zabbix. (ID:{self.zabbix_id})" self.logger.info(msg) self.create_journal_entry("success", msg) else: @@ -944,8 +947,8 @@ class PhysicalDevice: tmpls_from_zabbix.pop(pos) succesfull_templates.append(nb_tmpl) self.logger.debug( - f"Host {self.name}: template " - f"{nb_tmpl['name']} is present in Zabbix." + f"Host {self.name}: Template " + f"'{nb_tmpl['name']}' is present in Zabbix." ) break if ( diff --git a/modules/hostgroups.py b/modules/hostgroups.py index 8bbec86..cad31e1 100644 --- a/modules/hostgroups.py +++ b/modules/hostgroups.py @@ -93,6 +93,7 @@ class Hostgroup: format_options["cluster"] = self.nb.cluster.name format_options["cluster_type"] = self.nb.cluster.type.name self.format_options = format_options + self.logger.debug(f"Host {self.name}: Resolved properties for use in hostgroups: {self.format_options}") def set_nesting( self, nested_sitegroup_flag, nested_region_flag, nb_groups, nb_regions @@ -135,17 +136,18 @@ class Hostgroup: hostgroup_value = self.format_options[hg_item] if hostgroup_value: hg_output.append(hostgroup_value) + else: + self.logger.info(f"Host {self.name}: Used field '{hg_item}' has no value.") # Check if the hostgroup is populated with at least one item. if bool(hg_output): return "/".join(hg_output) msg = ( - f"Unable to generate hostgroup for host {self.name}." - " Not enough valid items. This is most likely" - " due to the use of custom fields that are empty" - " or an invalid hostgroup format." + f"Host {self.name}: Generating hostgroup name for '{hg_format}' failed. " + f"This is most likely due to fields that have no value." ) - self.logger.error(msg) - raise HostgroupError(msg) + self.logger.warning(msg) + return None + #raise HostgroupError(msg) def list_formatoptions(self): """ diff --git a/netbox_zabbix_sync.py b/netbox_zabbix_sync.py index d9ff71b..1515041 100755 --- a/netbox_zabbix_sync.py +++ b/netbox_zabbix_sync.py @@ -212,6 +212,8 @@ def main(arguments): config["hostgroup_format"], netbox_site_groups, netbox_regions) # Check if a valid hostgroup has been found for this VM. if not device.hostgroups: + logger.warning(f"Host {device.name}: Host has no valid " + f"hostgroups, Skipping this host...") continue device.set_inventory(nb_device) device.set_usermacros() From 2a3d5863029431ac70355d899c704ea5ac2b5d8f Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Tue, 24 Jun 2025 15:06:52 +0200 Subject: [PATCH 03/34] corrected typo --- modules/device.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/device.py b/modules/device.py index 26bac0e..472384c 100644 --- a/modules/device.py +++ b/modules/device.py @@ -135,7 +135,7 @@ class PhysicalDevice: self.hostgroups = [hg.generate(f) for f in hg_format] else: self.hostgroups.append(hg.generate(hg_format)) - # Remove dyuplicates and None values + # Remove duplicates and None values self.hostgroups = list(filter(None, list(set(self.hostgroups)))) if self.hostgroups: self.logger.debug(f"Host {self.name}: Should be member " From 906c7198630e727065eb66da22e6c30e9e218a37 Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Tue, 24 Jun 2025 15:16:39 +0200 Subject: [PATCH 04/34] corrected linting errors --- modules/device.py | 2 +- modules/hostgroups.py | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/modules/device.py b/modules/device.py index 472384c..524be55 100644 --- a/modules/device.py +++ b/modules/device.py @@ -138,7 +138,7 @@ class PhysicalDevice: # Remove duplicates and None values self.hostgroups = list(filter(None, list(set(self.hostgroups)))) if self.hostgroups: - self.logger.debug(f"Host {self.name}: Should be member " + self.logger.debug(f"Host {self.name}: Should be member " f"of groups: {self.hostgroups}") return True return False diff --git a/modules/hostgroups.py b/modules/hostgroups.py index cad31e1..f98c09b 100644 --- a/modules/hostgroups.py +++ b/modules/hostgroups.py @@ -11,6 +11,7 @@ class Hostgroup: Takes type (vm or dev) and NB object""" # pylint: disable=too-many-arguments, disable=too-many-positional-arguments + # pylint: disable=logging-fstring-interpolation def __init__( self, obj_type, @@ -93,7 +94,8 @@ class Hostgroup: format_options["cluster"] = self.nb.cluster.name format_options["cluster_type"] = self.nb.cluster.type.name self.format_options = format_options - self.logger.debug(f"Host {self.name}: Resolved properties for use in hostgroups: {self.format_options}") + self.logger.debug(f"Host {self.name}: Resolved properties for use " + f"in hostgroups: {self.format_options}") def set_nesting( self, nested_sitegroup_flag, nested_region_flag, nb_groups, nb_regions @@ -142,7 +144,7 @@ class Hostgroup: if bool(hg_output): return "/".join(hg_output) msg = ( - f"Host {self.name}: Generating hostgroup name for '{hg_format}' failed. " + f"Host {self.name}: Generating hostgroup name for '{hg_format}' failed. " f"This is most likely due to fields that have no value." ) self.logger.warning(msg) From 435fd1fa7822b995a227e0bdace41bd54a49d193 Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Tue, 24 Jun 2025 17:09:23 +0200 Subject: [PATCH 05/34] Fixed issues with tag mapping --- modules/device.py | 8 ++++---- modules/tags.py | 6 ++++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/modules/device.py b/modules/device.py index 524be55..bdde4a1 100644 --- a/modules/device.py +++ b/modules/device.py @@ -428,16 +428,16 @@ class PhysicalDevice: tags = ZabbixTags( self.nb, self._tag_map(), - config['tag_sync'], - config['tag_lower'], + tag_sync=config['tag_sync'], + tag_lower=config['tag_lower'], tag_name=config['tag_name'], tag_value=config['tag_value'], logger=self.logger, host=self.name, ) - if tags.sync is False: + if config['tag_sync'] is False: self.tags = [] - + return False self.tags = tags.generate() return True diff --git a/modules/tags.py b/modules/tags.py index 441ebe2..659966c 100644 --- a/modules/tags.py +++ b/modules/tags.py @@ -15,7 +15,7 @@ class ZabbixTags: self, nb, tag_map, - tag_sync, + tag_sync=False, tag_lower=True, tag_name=None, tag_value=None, @@ -130,4 +130,6 @@ class ZabbixTags: if t: tags.append(t) - return remove_duplicates(tags, sortkey="tag") + tags = remove_duplicates(tags, sortkey="tag") + self.logger.debug(f"Host {self.name}: Resolved tags: {tags}") + return tags From 9933c97e949d0731ff668394626a7bb89fb59d84 Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Tue, 24 Jun 2025 17:28:57 +0200 Subject: [PATCH 06/34] improved debug logging --- modules/device.py | 1 + modules/tools.py | 5 +++-- modules/usermacros.py | 5 ++++- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/modules/device.py b/modules/device.py index bdde4a1..aa323fb 100644 --- a/modules/device.py +++ b/modules/device.py @@ -229,6 +229,7 @@ class PhysicalDevice: self.inventory = field_mapper( self.name, self._inventory_map(), nbdevice, self.logger ) + self.logger.debug(f"Host {self.name}: Resolved inventory: {self.inventory}") return True def isCluster(self): diff --git a/modules/tools.py b/modules/tools.py index 823410e..ed327a7 100644 --- a/modules/tools.py +++ b/modules/tools.py @@ -159,7 +159,7 @@ def sanatize_log_output(data): """ Used for the update function to Zabbix which shows the data that its using to update the host. - Removes and sensitive data from the input. + Removes any sensitive data from the input. """ if not isinstance(data, dict): return data @@ -168,7 +168,8 @@ def sanatize_log_output(data): if "macros" in data: for macro in sanitized_data["macros"]: # Check if macro is secret type - if not macro["type"] == str(1): + if not (macro["type"] == str(1) or + macro["type"] == 1): continue macro["value"] = "********" # Check for interface data diff --git a/modules/usermacros.py b/modules/usermacros.py index 6d396c8..1e23213 100644 --- a/modules/usermacros.py +++ b/modules/usermacros.py @@ -6,7 +6,7 @@ All of the Zabbix Usermacro related configuration from logging import getLogger from re import match -from modules.tools import field_mapper +from modules.tools import field_mapper, sanatize_log_output class ZabbixUsermacros: @@ -98,6 +98,7 @@ class ZabbixUsermacros: Generate full set of Usermacros """ macros = [] + data={} # Parse the field mapper for usermacros if self.usermacro_map: self.logger.debug(f"Host {self.nb.name}: Starting usermacro mapper") @@ -119,4 +120,6 @@ class ZabbixUsermacros: m = self.render_macro(macro, properties) if m: macros.append(m) + data={'macros': macros} + self.logger.debug(f"Host {self.name}: Resolved macros: {sanatize_log_output(data)}") return macros From c2b25e0cd2747b8a5d04bd0c8ba5ee38ad873ece Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Tue, 24 Jun 2025 17:35:10 +0200 Subject: [PATCH 07/34] fixed linting --- modules/device.py | 2 +- modules/tools.py | 2 +- modules/usermacros.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/device.py b/modules/device.py index aa323fb..28e78b4 100644 --- a/modules/device.py +++ b/modules/device.py @@ -229,7 +229,7 @@ class PhysicalDevice: self.inventory = field_mapper( self.name, self._inventory_map(), nbdevice, self.logger ) - self.logger.debug(f"Host {self.name}: Resolved inventory: {self.inventory}") + self.logger.debug(f"Host {self.name}: Resolved inventory: {self.inventory}") return True def isCluster(self): diff --git a/modules/tools.py b/modules/tools.py index ed327a7..f3d27cc 100644 --- a/modules/tools.py +++ b/modules/tools.py @@ -168,7 +168,7 @@ def sanatize_log_output(data): if "macros" in data: for macro in sanitized_data["macros"]: # Check if macro is secret type - if not (macro["type"] == str(1) or + if not (macro["type"] == str(1) or macro["type"] == 1): continue macro["value"] = "********" diff --git a/modules/usermacros.py b/modules/usermacros.py index 1e23213..fd95eab 100644 --- a/modules/usermacros.py +++ b/modules/usermacros.py @@ -121,5 +121,5 @@ class ZabbixUsermacros: if m: macros.append(m) data={'macros': macros} - self.logger.debug(f"Host {self.name}: Resolved macros: {sanatize_log_output(data)}") + self.logger.debug(f"Host {self.name}: Resolved macros: {sanatize_log_output(data)}") return macros From 1de0b0781bcf71d5ea2edacc59c0a1f329f3770b Mon Sep 17 00:00:00 2001 From: TheNetworkGuy Date: Tue, 24 Jun 2025 20:44:59 +0200 Subject: [PATCH 08/34] Removed default for hostgroups and fixed bug for hostgroup attributes which do not exist --- modules/device.py | 3 +++ modules/hostgroups.py | 5 ----- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/modules/device.py b/modules/device.py index 28e78b4..c3baa17 100644 --- a/modules/device.py +++ b/modules/device.py @@ -564,6 +564,9 @@ class PhysicalDevice: final_data = [] # Check if the hostgroup is in a nested format and check each parent for hostgroup in self.hostgroups: + # Check if hostgroup is string. If Nonetype skip hostgroup + if not isinstance(hostgroup, str): + continue for pos in range(len(hostgroup.split("/"))): zabbix_hg = hostgroup.rsplit("/", pos)[0] if self.lookupZabbixHostgroup(hostgroups, zabbix_hg): diff --git a/modules/hostgroups.py b/modules/hostgroups.py index f98c09b..d5306a3 100644 --- a/modules/hostgroups.py +++ b/modules/hostgroups.py @@ -108,11 +108,6 @@ class Hostgroup: def generate(self, hg_format=None): """Generate hostgroup based on a provided format""" - # Set format to default in case its not specified - if not hg_format: - hg_format = ( - "site/manufacturer/role" if self.type == "dev" else "cluster/role" - ) # Split all given names hg_output = [] hg_items = hg_format.split("/") From a522c989298429a3dd165943f15b1e6f706c9f15 Mon Sep 17 00:00:00 2001 From: TheNetworkGuy Date: Tue, 24 Jun 2025 20:50:04 +0200 Subject: [PATCH 09/34] Removed default None for hg_format making a hostgroup format input required. --- modules/hostgroups.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/hostgroups.py b/modules/hostgroups.py index d5306a3..38c44c0 100644 --- a/modules/hostgroups.py +++ b/modules/hostgroups.py @@ -106,7 +106,7 @@ class Hostgroup: "region": {"flag": nested_region_flag, "data": nb_regions}, } - def generate(self, hg_format=None): + def generate(self, hg_format): """Generate hostgroup based on a provided format""" # Split all given names hg_output = [] From 6d4f1ac0a54ebed6883f8f882bb27ffc5c0cef20 Mon Sep 17 00:00:00 2001 From: TheNetworkGuy Date: Tue, 24 Jun 2025 21:28:13 +0200 Subject: [PATCH 10/34] Added hostgroup tests --- tests/test_hostgroups.py | 80 +++++++++++----- tests/test_list_hostgroup_formats.py | 137 +++++++++++++++++++++++++++ 2 files changed, 193 insertions(+), 24 deletions(-) create mode 100644 tests/test_list_hostgroup_formats.py diff --git a/tests/test_hostgroups.py b/tests/test_hostgroups.py index 1e652ec..995d26c 100644 --- a/tests/test_hostgroups.py +++ b/tests/test_hostgroups.py @@ -163,10 +163,6 @@ class TestHostgroups(unittest.TestCase): """Test different hostgroup formats for devices.""" hostgroup = Hostgroup("dev", self.mock_device, "4.0", self.mock_logger) - # Default format: site/manufacturer/role - default_result = hostgroup.generate() - self.assertEqual(default_result, "TestSite/TestManufacturer/TestRole") - # Custom format: site/region custom_result = hostgroup.generate("site/region") self.assertEqual(custom_result, "TestSite/TestRegion") @@ -180,7 +176,7 @@ class TestHostgroups(unittest.TestCase): hostgroup = Hostgroup("vm", self.mock_vm, "4.0", self.mock_logger) # Default format: cluster/role - default_result = hostgroup.generate() + default_result = hostgroup.generate("cluster/role") self.assertEqual(default_result, "TestCluster/TestRole") # Custom format: site/tenant @@ -251,7 +247,7 @@ class TestHostgroups(unittest.TestCase): hostgroup = Hostgroup("dev", minimal_device, "4.0", self.mock_logger) # Generate with default format - result = hostgroup.generate() + result = hostgroup.generate("site/manufacturer/role") # Site is missing, so only manufacturer and role should be included self.assertEqual(result, "MinimalManufacturer/MinimalRole") @@ -259,24 +255,6 @@ class TestHostgroups(unittest.TestCase): with self.assertRaises(HostgroupError): hostgroup.generate("site/nonexistent/role") - def test_hostgroup_missing_required_attributes(self): - """Test handling when no valid hostgroup can be generated.""" - # Create a VM with minimal attributes that won't satisfy any format - minimal_vm = MagicMock() - minimal_vm.name = "minimal-vm" - minimal_vm.site = None - minimal_vm.tenant = None - minimal_vm.platform = None - minimal_vm.role = None - minimal_vm.cluster = None - minimal_vm.custom_fields = {} - - hostgroup = Hostgroup("vm", minimal_vm, "4.0", self.mock_logger) - - # With default format of cluster/role, both are None, so should raise an error - with self.assertRaises(HostgroupError): - hostgroup.generate() - def test_nested_region_hostgroups(self): """Test hostgroup generation with nested regions.""" # Mock the build_path function to return a predictable result @@ -334,6 +312,60 @@ class TestHostgroups(unittest.TestCase): calls = [call.write(f"The following options are available for host test-device"), call.write('\n')] mock_stdout.assert_has_calls(calls, any_order=True) + + def test_vm_list_based_hostgroup_format(self): + """Test VM hostgroup generation with a list-based format.""" + hostgroup = Hostgroup("vm", self.mock_vm, "4.0", self.mock_logger) + + # Test with a list of format strings + format_list = ["platform", "role", "cluster_type/cluster"] + + # Generate hostgroups for each format in the list + hostgroups = [] + for fmt in format_list: + result = hostgroup.generate(fmt) + if result: # Only add non-None results + hostgroups.append(result) + + # Verify each expected hostgroup is generated + self.assertEqual(len(hostgroups), 3) # Should have 3 hostgroups + self.assertIn("TestPlatform", hostgroups) + self.assertIn("TestRole", hostgroups) + self.assertIn("TestClusterType/TestCluster", hostgroups) + + def test_nested_format_splitting(self): + """Test that formats with slashes correctly split and resolve each component.""" + hostgroup = Hostgroup("vm", self.mock_vm, "4.0", self.mock_logger) + + # Test a format with slashes that should be split + complex_format = "cluster_type/cluster" + result = hostgroup.generate(complex_format) + + # Verify the format is correctly split and each component resolved + self.assertEqual(result, "TestClusterType/TestCluster") + + def test_multiple_hostgroup_formats_device(self): + """Test device hostgroup generation with multiple formats.""" + hostgroup = Hostgroup("dev", self.mock_device, "4.0", self.mock_logger) + + # Test with various formats that would be in a list + formats = [ + "site", + "manufacturer/role", + "platform/location", + "tenant_group/tenant" + ] + + # Generate and check each format + results = {} + for fmt in formats: + results[fmt] = hostgroup.generate(fmt) + + # Verify results + self.assertEqual(results["site"], "TestSite") + self.assertEqual(results["manufacturer/role"], "TestManufacturer/TestRole") + self.assertEqual(results["platform/location"], "TestPlatform/TestLocation") + self.assertEqual(results["tenant_group/tenant"], "TestTenantGroup/TestTenant") if __name__ == "__main__": diff --git a/tests/test_list_hostgroup_formats.py b/tests/test_list_hostgroup_formats.py new file mode 100644 index 0000000..9b8cc21 --- /dev/null +++ b/tests/test_list_hostgroup_formats.py @@ -0,0 +1,137 @@ +"""Tests for list-based hostgroup formats in configuration.""" +import unittest +from unittest.mock import MagicMock, patch +from modules.hostgroups import Hostgroup +from modules.exceptions import HostgroupError +from modules.tools import verify_hg_format + + +class TestListHostgroupFormats(unittest.TestCase): + """Test class for list-based hostgroup format functionality.""" + + def setUp(self): + """Set up test fixtures.""" + # Create mock logger + self.mock_logger = MagicMock() + + # Create mock device + self.mock_device = MagicMock() + self.mock_device.name = "test-device" + + # Set up site information + site = MagicMock() + site.name = "TestSite" + + # Set up region information + region = MagicMock() + region.name = "TestRegion" + region.__str__.return_value = "TestRegion" + site.region = region + + # Set device site + self.mock_device.site = site + + # Set up role information + self.mock_device_role = MagicMock() + self.mock_device_role.name = "TestRole" + self.mock_device_role.__str__.return_value = "TestRole" + self.mock_device.role = self.mock_device_role + + # Set up rack information + rack = MagicMock() + rack.name = "TestRack" + self.mock_device.rack = rack + + # Set up platform information + platform = MagicMock() + platform.name = "TestPlatform" + self.mock_device.platform = platform + + # Device-specific properties + device_type = MagicMock() + manufacturer = MagicMock() + manufacturer.name = "TestManufacturer" + device_type.manufacturer = manufacturer + self.mock_device.device_type = device_type + + # Create mock VM + self.mock_vm = MagicMock() + self.mock_vm.name = "test-vm" + + # Reuse site from device + self.mock_vm.site = site + + # Set up role for VM + self.mock_vm.role = self.mock_device_role + + # Set up platform for VM + self.mock_vm.platform = platform + + # VM-specific properties + cluster = MagicMock() + cluster.name = "TestCluster" + cluster_type = MagicMock() + cluster_type.name = "TestClusterType" + cluster.type = cluster_type + self.mock_vm.cluster = cluster + + def test_verify_list_based_hostgroup_format(self): + """Test verification of list-based hostgroup formats.""" + # List format with valid items + valid_format = ["region", "site", "rack"] + + # List format with nested path + valid_nested_format = ["region", "site/rack"] + + # List format with invalid item + invalid_format = ["region", "invalid_item", "rack"] + + # Should not raise exception for valid formats + verify_hg_format(valid_format, hg_type="dev", logger=self.mock_logger) + verify_hg_format(valid_nested_format, hg_type="dev", logger=self.mock_logger) + + # Should raise exception for invalid format + with self.assertRaises(HostgroupError): + verify_hg_format(invalid_format, hg_type="dev", logger=self.mock_logger) + + def test_simulate_hostgroup_generation_from_config(self): + """Simulate how the main script would generate hostgroups from list-based config.""" + # Mock configuration with list-based hostgroup format + config_format = ["region", "site", "rack"] + hostgroup = Hostgroup("dev", self.mock_device, "4.0", self.mock_logger) + + # Simulate the main script's hostgroup generation process + hostgroups = [] + for fmt in config_format: + result = hostgroup.generate(fmt) + if result: + hostgroups.append(result) + + # Check results + self.assertEqual(len(hostgroups), 3) + self.assertIn("TestRegion", hostgroups) + self.assertIn("TestSite", hostgroups) + self.assertIn("TestRack", hostgroups) + + def test_vm_hostgroup_format_from_config(self): + """Test VM hostgroup generation with list-based format.""" + # Mock VM configuration with mixed format + config_format = ["platform", "role", "cluster_type/cluster"] + hostgroup = Hostgroup("vm", self.mock_vm, "4.0", self.mock_logger) + + # Simulate the main script's hostgroup generation process + hostgroups = [] + for fmt in config_format: + result = hostgroup.generate(fmt) + if result: + hostgroups.append(result) + + # Check results + self.assertEqual(len(hostgroups), 3) + self.assertIn("TestPlatform", hostgroups) + self.assertIn("TestRole", hostgroups) + self.assertIn("TestClusterType/TestCluster", hostgroups) + + +if __name__ == "__main__": + unittest.main() From 4a53b53789d9af793e18fdae9b911d87e61d7375 Mon Sep 17 00:00:00 2001 From: TheNetworkGuy Date: Tue, 24 Jun 2025 21:28:32 +0200 Subject: [PATCH 11/34] Removed previous patch for Nonetype hostgroups and made a proper fix by refactoring the set_hostgroup() function and removing it from virtual_machines.py --- modules/device.py | 6 ++---- modules/hostgroups.py | 1 - modules/virtual_machine.py | 20 +------------------- 3 files changed, 3 insertions(+), 24 deletions(-) diff --git a/modules/device.py b/modules/device.py index c3baa17..03ca191 100644 --- a/modules/device.py +++ b/modules/device.py @@ -48,6 +48,7 @@ class PhysicalDevice: self.zbx_template_names = [] self.zbx_templates = [] self.hostgroups = [] + self.hostgroup_type = "dev" self.tenant = nb.tenant self.config_context = nb.config_context self.zbxproxy = None @@ -121,7 +122,7 @@ class PhysicalDevice: """Set the hostgroup for this device""" # Create new Hostgroup instance hg = Hostgroup( - "dev", + self.hostgroup_type, self.nb, self.nb_api_version, logger=self.logger, @@ -564,9 +565,6 @@ class PhysicalDevice: final_data = [] # Check if the hostgroup is in a nested format and check each parent for hostgroup in self.hostgroups: - # Check if hostgroup is string. If Nonetype skip hostgroup - if not isinstance(hostgroup, str): - continue for pos in range(len(hostgroup.split("/"))): zabbix_hg = hostgroup.rsplit("/", pos)[0] if self.lookupZabbixHostgroup(hostgroups, zabbix_hg): diff --git a/modules/hostgroups.py b/modules/hostgroups.py index 38c44c0..213b4cf 100644 --- a/modules/hostgroups.py +++ b/modules/hostgroups.py @@ -144,7 +144,6 @@ class Hostgroup: ) self.logger.warning(msg) return None - #raise HostgroupError(msg) def list_formatoptions(self): """ diff --git a/modules/virtual_machine.py b/modules/virtual_machine.py index e0f7abb..847375d 100644 --- a/modules/virtual_machine.py +++ b/modules/virtual_machine.py @@ -16,6 +16,7 @@ class VirtualMachine(PhysicalDevice): super().__init__(*args, **kwargs) self.hostgroup = None self.zbx_template_names = None + self.hostgroup_type = "vm" def _inventory_map(self): """use VM inventory maps""" @@ -29,25 +30,6 @@ class VirtualMachine(PhysicalDevice): """use VM tag maps""" return config["vm_tag_map"] - def set_hostgroup(self, hg_format, nb_site_groups, nb_regions): - """Set the hostgroup for this device""" - # Create new Hostgroup instance - hg = Hostgroup( - "vm", - self.nb, - self.nb_api_version, - logger=self.logger, - nested_sitegroup_flag=config["traverse_site_groups"], - nested_region_flag=config["traverse_regions"], - nb_groups=nb_site_groups, - nb_regions=nb_regions, - ) - # Generate hostgroup based on hostgroup format - if isinstance(hg_format, list): - self.hostgroups = [hg.generate(f) for f in hg_format] - else: - self.hostgroups.append(hg.generate(hg_format)) - def set_vm_template(self): """Set Template for VMs. Overwrites default class to skip a lookup of custom fields.""" From 29a54e5a86e6fca5b1858cac5f6c5b71fd512628 Mon Sep 17 00:00:00 2001 From: TheNetworkGuy Date: Tue, 24 Jun 2025 21:29:36 +0200 Subject: [PATCH 12/34] Removed unused hostgroup import since the hostgroup generate function function has been moved to devices.py --- modules/virtual_machine.py | 1 - 1 file changed, 1 deletion(-) diff --git a/modules/virtual_machine.py b/modules/virtual_machine.py index 847375d..8c52033 100644 --- a/modules/virtual_machine.py +++ b/modules/virtual_machine.py @@ -2,7 +2,6 @@ """Module that hosts all functions for virtual machine processing""" from modules.device import PhysicalDevice from modules.exceptions import InterfaceConfigError, SyncInventoryError, TemplateError -from modules.hostgroups import Hostgroup from modules.interface import ZabbixInterface from modules.config import load_config # Load config From 5923682d48b8836d339ae2c05f826adf80af65cd Mon Sep 17 00:00:00 2001 From: TheNetworkGuy Date: Tue, 24 Jun 2025 21:42:46 +0200 Subject: [PATCH 13/34] Fixes workflows to be executed 2 times. --- .github/workflows/quality.yml | 1 - .github/workflows/run_tests.yml | 1 - 2 files changed, 2 deletions(-) diff --git a/.github/workflows/quality.yml b/.github/workflows/quality.yml index 8dfbe3f..81cad97 100644 --- a/.github/workflows/quality.yml +++ b/.github/workflows/quality.yml @@ -2,7 +2,6 @@ name: Pylint Quality control on: - push: pull_request: workflow_call: diff --git a/.github/workflows/run_tests.yml b/.github/workflows/run_tests.yml index 6a16a64..589fc47 100644 --- a/.github/workflows/run_tests.yml +++ b/.github/workflows/run_tests.yml @@ -2,7 +2,6 @@ name: Pytest code testing on: - push: pull_request: workflow_call: From e4a1a17ded7a911a52b360a7bf61cd6142f11434 Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Wed, 25 Jun 2025 10:43:47 +0200 Subject: [PATCH 14/34] Logging improvements --- modules/device.py | 80 +++++++++++++++++++++---------------------- modules/tags.py | 8 ++--- modules/tools.py | 10 +++--- modules/usermacros.py | 8 ++--- netbox_zabbix_sync.py | 14 ++++---- 5 files changed, 59 insertions(+), 61 deletions(-) diff --git a/modules/device.py b/modules/device.py index 03ca191..61a11c6 100644 --- a/modules/device.py +++ b/modules/device.py @@ -98,7 +98,7 @@ class PhysicalDevice: self.zabbix_id = self.nb.custom_fields[config["device_cf"]] else: e = f'Host {self.name}: Custom field {config["device_cf"]} not present' - self.logger.warning(e) + self.logger.error(e) raise SyncInventoryError(e) # Validate hostname format. @@ -226,7 +226,7 @@ class PhysicalDevice: return False self.inventory = {} if config["inventory_sync"] and self.inventory_mode in [0, 1]: - self.logger.debug(f"Host {self.name}: Starting inventory mapper") + self.logger.debug(f"Host {self.name}: Starting inventory mapper.") self.inventory = field_mapper( self.name, self._inventory_map(), nbdevice, self.logger ) @@ -248,14 +248,14 @@ class PhysicalDevice: f"Unable to proces {self.name} for cluster calculation: " f"not part of a cluster." ) - self.logger.warning(e) + self.logger.info(e) raise SyncInventoryError(e) if not self.nb.virtual_chassis.master: e = ( f"{self.name} is part of a NetBox virtual chassis which does " "not have a master configured. Skipping for this reason." ) - self.logger.error(e) + self.logger.warning(e) raise SyncInventoryError(e) return self.nb.virtual_chassis.master.id @@ -267,14 +267,14 @@ class PhysicalDevice: """ masterid = self.getClusterMaster() if masterid == self.id: - self.logger.debug( + self.logger.info( f"Host {self.name} is primary cluster member. " f"Modifying hostname from {self.name} to " + f"{self.nb.virtual_chassis.name}." ) self.name = self.nb.virtual_chassis.name return True - self.logger.debug(f"Host {self.name} is non-primary cluster member.") + self.logger.info(f"Host {self.name} is non-primary cluster member.") return False def zbxTemplatePrepper(self, templates): @@ -286,7 +286,7 @@ class PhysicalDevice: # Check if there are templates defined if not self.zbx_template_names: e = f"Host {self.name}: No templates found" - self.logger.info(e) + self.logger.warning(e) raise SyncInventoryError() # Set variable to empty list self.zbx_templates = [] @@ -477,7 +477,7 @@ class PhysicalDevice: # If the proxy name matches if proxy["name"] == proxy_name: self.logger.debug( - f"Host {self.name}: using {proxy['type']}" f" {proxy_name}" + f"Host {self.name}: using {proxy['type']}" f" '{proxy_name}'" ) self.zbxproxy = proxy return True @@ -640,8 +640,6 @@ class PhysicalDevice: ) self.logger.warning(e) raise SyncInventoryError(e) - #if self.group_ids: - # self.group_ids.append(self.pri_group_id) # Prepare templates and proxy config self.zbxTemplatePrepper(templates) @@ -674,10 +672,10 @@ class PhysicalDevice: raise SyncInventoryError(e) host = host[0] if host["host"] == self.name: - self.logger.debug(f"Host {self.name}: hostname in-sync.") + self.logger.debug(f"Host {self.name}: Hostname in-sync.") else: - self.logger.warning( - f"Host {self.name}: hostname OUT of sync. " + self.logger.info( + f"Host {self.name}: Hostname OUT of sync. " f"Received value: {host['host']}" ) self.updateZabbixHost(host=self.name) @@ -685,17 +683,17 @@ class PhysicalDevice: # Execute check depending on wether the name is special or not if self.use_visible_name: if host["name"] == self.visible_name: - self.logger.debug(f"Host {self.name}: visible name in-sync.") + self.logger.debug(f"Host {self.name}: Visible name in-sync.") else: - self.logger.warning( - f"Host {self.name}: visible name OUT of sync." + self.logger.info( + f"Host {self.name}: Visible name OUT of sync." f" Received value: {host['name']}" ) self.updateZabbixHost(name=self.visible_name) # Check if the templates are in-sync if not self.zbx_template_comparer(host["parentTemplates"]): - self.logger.warning(f"Host {self.name}: template(s) OUT of sync.") + self.logger.info(f"Host {self.name}: Template(s) OUT of sync.") # Prepare Templates for API parsing templateids = [] for template in self.zbx_templates: @@ -705,7 +703,7 @@ class PhysicalDevice: templates_clear=host["parentTemplates"], templates=templateids ) else: - self.logger.debug(f"Host {self.name}: template(s) in-sync.") + self.logger.debug(f"Host {self.name}: Template(s) in-sync.") # Check if Zabbix version is 6 or higher. Issue #93 group_dictname = "hostgroups" @@ -714,15 +712,15 @@ class PhysicalDevice: # Check if hostgroups match if (sorted(host[group_dictname], key=itemgetter('groupid')) == sorted(self.group_ids, key=itemgetter('groupid'))): - self.logger.debug(f"Host {self.name}: hostgroups in-sync.") + self.logger.debug(f"Host {self.name}: Hostgroups in-sync.") else: - self.logger.warning(f"Host {self.name}: hostgroups OUT of sync.") + self.logger.info(f"Host {self.name}: Hostgroups OUT of sync.") self.updateZabbixHost(groups=self.group_ids) if int(host["status"]) == self.zabbix_state: - self.logger.debug(f"Host {self.name}: status in-sync.") + self.logger.debug(f"Host {self.name}: Status in-sync.") else: - self.logger.warning(f"Host {self.name}: status OUT of sync.") + self.logger.info(f"Host {self.name}: Status OUT of sync.") self.updateZabbixHost(status=str(self.zabbix_state)) # Check if a proxy has been defined @@ -730,13 +728,13 @@ class PhysicalDevice: # Check if proxy or proxy group is defined if (self.zbxproxy["idtype"] in host and host[self.zbxproxy["idtype"]] == self.zbxproxy["id"]): - self.logger.debug(f"Host {self.name}: proxy in-sync.") + self.logger.debug(f"Host {self.name}: Proxy in-sync.") # Backwards compatibility for Zabbix <= 6 elif "proxy_hostid" in host and host["proxy_hostid"] == self.zbxproxy["id"]: - self.logger.debug(f"Host {self.name}: proxy in-sync.") + self.logger.debug(f"Host {self.name}: Proxy in-sync.") # Proxy does not match, update Zabbix else: - self.logger.warning(f"Host {self.name}: proxy OUT of sync.") + self.logger.info(f"Host {self.name}: Proxy OUT of sync.") # Zabbix <= 6 patch if not str(self.zabbix.version).startswith("7"): self.updateZabbixHost(proxy_hostid=self.zbxproxy["id"]) @@ -759,7 +757,7 @@ class PhysicalDevice: if proxy_power and proxy_set: # Zabbix <= 6 fix self.logger.warning( - f"Host {self.name}: no proxy is configured in NetBox " + f"Host {self.name}: No proxy is configured in NetBox " "but is configured in Zabbix. Removing proxy config in Zabbix" ) if "proxy_hostid" in host and bool(host["proxy_hostid"]): @@ -773,26 +771,26 @@ class PhysicalDevice: # Checks if a proxy has been defined in Zabbix and if proxy_power config has been set if proxy_set and not proxy_power: # Display error message - self.logger.error( - f"Host {self.name} is configured " + self.logger.warning( + f"Host {self.name}: Is configured " f"with proxy in Zabbix but not in NetBox. The" " -p flag was ommited: no " "changes have been made." ) if not proxy_set: - self.logger.debug(f"Host {self.name}: proxy in-sync.") + self.logger.debug(f"Host {self.name}: Proxy in-sync.") # Check host inventory mode if str(host["inventory_mode"]) == str(self.inventory_mode): self.logger.debug(f"Host {self.name}: inventory_mode in-sync.") else: - self.logger.warning(f"Host {self.name}: inventory_mode OUT of sync.") + self.logger.info(f"Host {self.name}: inventory_mode OUT of sync.") self.updateZabbixHost(inventory_mode=str(self.inventory_mode)) if config["inventory_sync"] and self.inventory_mode in [0, 1]: # Check host inventory mapping if host["inventory"] == self.inventory: - self.logger.debug(f"Host {self.name}: inventory in-sync.") + self.logger.debug(f"Host {self.name}: Inventory in-sync.") else: - self.logger.warning(f"Host {self.name}: inventory OUT of sync.") + self.logger.info(f"Host {self.name}: Inventory OUT of sync.") self.updateZabbixHost(inventory=self.inventory) # Check host usermacros @@ -815,18 +813,18 @@ class PhysicalDevice: netbox_macros.sort(key=filter_with_macros) # Check if both lists are the same if host["macros"] == netbox_macros: - self.logger.debug(f"Host {self.name}: usermacros in-sync.") + self.logger.debug(f"Host {self.name}: Usermacros in-sync.") else: - self.logger.warning(f"Host {self.name}: usermacros OUT of sync.") + self.logger.info(f"Host {self.name}: Usermacros OUT of sync.") # Update Zabbix with NetBox usermacros self.updateZabbixHost(macros=self.usermacros) # Check host tags if config['tag_sync']: if remove_duplicates(host["tags"], sortkey="tag") == self.tags: - self.logger.debug(f"Host {self.name}: tags in-sync.") + self.logger.debug(f"Host {self.name}: Tags in-sync.") else: - self.logger.warning(f"Host {self.name}: tags OUT of sync.") + self.logger.info(f"Host {self.name}: Tags OUT of sync.") self.updateZabbixHost(tags=self.tags) # If only 1 interface has been found @@ -864,11 +862,11 @@ class PhysicalDevice: updates[key] = item if updates: # If interface updates have been found: push to Zabbix - self.logger.warning(f"Host {self.name}: Interface OUT of sync.") + self.logger.info(f"Host {self.name}: Interface OUT of sync.") if "type" in updates: # Changing interface type not supported. Raise exception. e = ( - f"Host {self.name}: changing interface type to " + f"Host {self.name}: Changing interface type to " f"{str(updates['type'])} is not supported." ) self.logger.error(e) @@ -878,7 +876,7 @@ class PhysicalDevice: try: # API call to Zabbix self.zabbix.hostinterface.update(updates) - e = (f"Host {self.name}: updated interface " + e = (f"Host {self.name}: Updated interface " f"with data {sanatize_log_output(updates)}.") self.logger.info(e) self.create_journal_entry("info", e) @@ -888,11 +886,11 @@ class PhysicalDevice: raise SyncExternalError(msg) from e else: # If no updates are found, Zabbix interface is in-sync - e = f"Host {self.name}: interface in-sync." + e = f"Host {self.name}: Interface in-sync." self.logger.debug(e) else: e = ( - f"Host {self.name} has unsupported interface configuration." + f"Host {self.name}: Has unsupported interface configuration." f" Host has total of {len(host['interfaces'])} interfaces. " "Manual intervention required." ) diff --git a/modules/tags.py b/modules/tags.py index 659966c..f341abd 100644 --- a/modules/tags.py +++ b/modules/tags.py @@ -76,7 +76,7 @@ class ZabbixTags: else: tag["tag"] = tag_name else: - self.logger.warning(f"Tag {tag_name} is not a valid tag name, skipping.") + self.logger.warning(f"Tag '{tag_name}' is not a valid tag name, skipping.") return False if self.validate_value(tag_value): @@ -85,8 +85,8 @@ class ZabbixTags: else: tag["value"] = tag_value else: - self.logger.warning( - f"Tag {tag_name} has an invalid value: '{tag_value}', skipping." + self.logger.info( + f"Tag '{tag_name}' has an invalid value: '{tag_value}', skipping." ) return False return tag @@ -99,7 +99,7 @@ class ZabbixTags: tags = [] # Parse the field mapper for tags if self.tag_map: - self.logger.debug(f"Host {self.nb.name}: Starting tag mapper") + self.logger.debug(f"Host {self.nb.name}: Starting tag mapper.") field_tags = field_mapper(self.nb.name, self.tag_map, self.nb, self.logger) for tag, value in field_tags.items(): t = self.render_tag(tag, value) diff --git a/modules/tools.py b/modules/tools.py index f3d27cc..c49f5dd 100644 --- a/modules/tools.py +++ b/modules/tools.py @@ -71,20 +71,20 @@ def field_mapper(host, mapper, nbdevice, logger): data[zbx_field] = str(value) elif not value: # empty value should just be an empty string for API compatibility - logger.debug( + logger.info( f"Host {host}: NetBox lookup for " - f"'{nb_field}' returned an empty value" + f"'{nb_field}' returned an empty value." ) data[zbx_field] = "" else: # Value is not a string or numeral, probably not what the user expected. - logger.error( + logger.info( f"Host {host}: Lookup for '{nb_field}'" " returned an unexpected type: it will be skipped." ) logger.debug( f"Host {host}: Field mapping complete. " - f"Mapped {len(list(filter(None, data.values())))} field(s)" + f"Mapped {len(list(filter(None, data.values())))} field(s)." ) return data @@ -151,7 +151,7 @@ def verify_hg_format(hg_format, device_cfs=None, vm_cfs=None, hg_type="dev", log f"Hostgroup item {hg_object} is not valid. Make sure you" " use valid items and separate them with '/'." ) - logger.error(e) + logger.warning(e) raise HostgroupError(e) diff --git a/modules/usermacros.py b/modules/usermacros.py index fd95eab..cfa8082 100644 --- a/modules/usermacros.py +++ b/modules/usermacros.py @@ -57,7 +57,7 @@ class ZabbixUsermacros: macro["macro"] = str(macro_name) if isinstance(macro_properties, dict): if not "value" in macro_properties: - self.logger.warning(f"Host {self.name}: Usermacro {macro_name} has " + self.logger.info(f"Host {self.name}: Usermacro {macro_name} has " "no value in Netbox, skipping.") return False macro["value"] = macro_properties["value"] @@ -83,11 +83,11 @@ class ZabbixUsermacros: macro["description"] = "" else: - self.logger.warning(f"Host {self.name}: Usermacro {macro_name} " + self.logger.info(f"Host {self.name}: Usermacro {macro_name} " "has no value, skipping.") return False else: - self.logger.error( + self.logger.warning( f"Host {self.name}: Usermacro {macro_name} is not a valid usermacro name, skipping." ) return False @@ -101,7 +101,7 @@ class ZabbixUsermacros: data={} # Parse the field mapper for usermacros if self.usermacro_map: - self.logger.debug(f"Host {self.nb.name}: Starting usermacro mapper") + self.logger.debug(f"Host {self.nb.name}: Starting usermacro mapper.") field_macros = field_mapper( self.nb.name, self.usermacro_map, self.nb, self.logger ) diff --git a/netbox_zabbix_sync.py b/netbox_zabbix_sync.py index 1515041..3e783e8 100755 --- a/netbox_zabbix_sync.py +++ b/netbox_zabbix_sync.py @@ -143,7 +143,7 @@ def main(arguments): try: vm = VirtualMachine(nb_vm, zabbix, netbox_journals, nb_version, config["create_journal"], logger) - logger.debug(f"Host {vm.name}: started operations on VM.") + logger.debug(f"Host {vm.name}: Started operations on VM.") vm.set_vm_template() # Check if a valid template has been found for this VM. if not vm.zbx_template_names: @@ -162,12 +162,12 @@ def main(arguments): # Delete device from Zabbix # and remove hostID from NetBox. vm.cleanup() - logger.debug(f"VM {vm.name}: cleanup complete") + logger.info(f"VM {vm.name}: cleanup complete") continue # Device has been added to NetBox # but is not in Activate state logger.info( - f"VM {vm.name}: skipping since this VM is " + f"VM {vm.name}: Skipping since this VM is " f"not in the active state." ) continue @@ -202,7 +202,7 @@ def main(arguments): # Set device instance set data such as hostgroup and template information. device = PhysicalDevice(nb_device, zabbix, netbox_journals, nb_version, config["create_journal"], logger) - logger.debug(f"Host {device.name}: started operations on device.") + logger.debug(f"Host {device.name}: Started operations on device.") device.set_template(config["templates_config_context"], config["templates_config_context_overrule"]) # Check if a valid template has been found for this VM. @@ -229,7 +229,7 @@ def main(arguments): # Device is secondary in cluster. # Don't continue with this device. e = ( - f"Device {device.name}: is part of cluster " + f"Device {device.name}: Is part of cluster " f"but not primary. Skipping this host..." ) logger.info(e) @@ -245,7 +245,7 @@ def main(arguments): # Device has been added to NetBox # but is not in Activate state logger.info( - f"Device {device.name}: skipping since this device is " + f"Device {device.name}: Skipping since this device is " f"not in the active state." ) continue @@ -282,7 +282,7 @@ if __name__ == "__main__": description="A script to sync Zabbix with NetBox device data." ) parser.add_argument( - "-v", "--verbose", help="Turn on debugging.", action="store_true" + "-v", "--verbose", help="Turn on verbose logging.", action="store_true" ) parser.add_argument( "-vv", "--debug", help="Turn on debugging.", action="store_true" From e0ec3c063275d4f4fa2ba8e8ab86e0f05478c037 Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Wed, 25 Jun 2025 10:54:39 +0200 Subject: [PATCH 15/34] updated usermacro test for new loglevels --- tests/test_usermacros.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_usermacros.py b/tests/test_usermacros.py index 28305af..5c2b6a4 100644 --- a/tests/test_usermacros.py +++ b/tests/test_usermacros.py @@ -88,7 +88,7 @@ class TestZabbixUsermacros(unittest.TestCase): macros = ZabbixUsermacros(self.nb, {}, False, logger=self.logger) result = macros.render_macro("{$FOO}", {"type": "text"}) self.assertFalse(result) - self.logger.warning.assert_called() + self.logger.info.assert_called() def test_render_macro_str(self): macros = ZabbixUsermacros(self.nb, {}, False, logger=self.logger) @@ -102,7 +102,7 @@ class TestZabbixUsermacros(unittest.TestCase): macros = ZabbixUsermacros(self.nb, {}, False, logger=self.logger) result = macros.render_macro("FOO", "bar") self.assertFalse(result) - self.logger.error.assert_called() + self.logger.warning.assert_called() def test_generate_from_map(self): nb = DummyNB(memory="bar", role="baz") From 57c7f83e6a5fb7c9a37c03a2fe79cf5d0830c6da Mon Sep 17 00:00:00 2001 From: Wouter de Bruijn Date: Wed, 25 Jun 2025 13:56:41 +0200 Subject: [PATCH 16/34] =?UTF-8?q?=F0=9F=94=8A=20Removed=20f-strings=20usag?= =?UTF-8?q?e=20from=20logs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- modules/device.py | 191 +++++++++++++++++++++++------------------- modules/hostgroups.py | 11 ++- modules/tags.py | 9 +- modules/tools.py | 89 +++++++++++--------- modules/usermacros.py | 29 +++++-- netbox_zabbix_sync.py | 93 ++++++++++++-------- 6 files changed, 246 insertions(+), 176 deletions(-) diff --git a/modules/device.py b/modules/device.py index 61a11c6..dcae69c 100644 --- a/modules/device.py +++ b/modules/device.py @@ -26,6 +26,7 @@ from modules.config import load_config config = load_config() + class PhysicalDevice: # pylint: disable=too-many-instance-attributes, too-many-arguments, too-many-positional-arguments """ @@ -97,7 +98,7 @@ class PhysicalDevice: if config["device_cf"] in self.nb.custom_fields: self.zabbix_id = self.nb.custom_fields[config["device_cf"]] else: - e = f'Host {self.name}: Custom field {config["device_cf"]} not present' + e = f"Host {self.name}: Custom field {config['device_cf']} not present" self.logger.error(e) raise SyncInventoryError(e) @@ -111,9 +112,10 @@ class PhysicalDevice: self.visible_name = self.nb.name self.use_visible_name = True self.logger.info( - f"Host {self.visible_name} contains special characters. " - f"Using {self.name} as name for the NetBox object " - f"and using {self.visible_name} as visible name in Zabbix." + "Host %s contains special characters. Using %s as name for the NetBox object and using %s as visible name in Zabbix.", + self.visible_name, + self.name, + self.visible_name, ) else: pass @@ -126,8 +128,8 @@ class PhysicalDevice: self.nb, self.nb_api_version, logger=self.logger, - nested_sitegroup_flag=config['traverse_site_groups'], - nested_region_flag=config['traverse_regions'], + nested_sitegroup_flag=config["traverse_site_groups"], + nested_region_flag=config["traverse_regions"], nb_groups=nb_site_groups, nb_regions=nb_regions, ) @@ -139,12 +141,12 @@ class PhysicalDevice: # Remove duplicates and None values self.hostgroups = list(filter(None, list(set(self.hostgroups)))) if self.hostgroups: - self.logger.debug(f"Host {self.name}: Should be member " - f"of groups: {self.hostgroups}") + self.logger.debug( + "Host %s: Should be member of groups: %s", self.name, self.hostgroups + ) return True return False - def set_template(self, prefer_config_context, overrule_custom): """Set Template""" self.zbx_template_names = None @@ -210,9 +212,10 @@ class PhysicalDevice: # Set inventory mode. Default is disabled (see class init function). if config["inventory_mode"] == "disabled": if config["inventory_sync"]: - self.logger.error(f"Host {self.name}: Unable to map NetBox inventory to Zabbix. " - "Inventory sync is enabled in " - "config but inventory mode is disabled.") + self.logger.error( + "Host %s: Unable to map NetBox inventory to Zabbix. Inventory sync is enabled in config but inventory mode is disabled", + self.name, + ) return True if config["inventory_mode"] == "manual": self.inventory_mode = 0 @@ -220,17 +223,20 @@ class PhysicalDevice: self.inventory_mode = 1 else: self.logger.error( - f"Host {self.name}: Specified value for inventory mode in" - f" config is not valid. Got value {config['inventory_mode']}" + "Host %s: Specified value for inventory mode in config is not valid. Got value %s", + self.name, + config["inventory_mode"], ) return False self.inventory = {} if config["inventory_sync"] and self.inventory_mode in [0, 1]: - self.logger.debug(f"Host {self.name}: Starting inventory mapper.") + self.logger.debug("Host %s: Starting inventory mapper.", self.name) self.inventory = field_mapper( self.name, self._inventory_map(), nbdevice, self.logger ) - self.logger.debug(f"Host {self.name}: Resolved inventory: {self.inventory}") + self.logger.debug( + "Host %s: Resolved inventory: %s", self.name, self.inventory + ) return True def isCluster(self): @@ -268,13 +274,14 @@ class PhysicalDevice: masterid = self.getClusterMaster() if masterid == self.id: self.logger.info( - f"Host {self.name} is primary cluster member. " - f"Modifying hostname from {self.name} to " - + f"{self.nb.virtual_chassis.name}." + "Host %s is primary cluster member. Modifying hostname from %s to %s.", + self.name, + self.name, + self.nb.virtual_chassis.name, ) self.name = self.nb.virtual_chassis.name return True - self.logger.info(f"Host {self.name} is non-primary cluster member.") + self.logger.info("Host %s is non-primary cluster member.", self.name) return False def zbxTemplatePrepper(self, templates): @@ -306,8 +313,10 @@ class PhysicalDevice: "name": zbx_template["name"], } ) - e = (f"Host {self.name}: Found template '{zbx_template['name']}' " - f"(ID:{zbx_template['templateid']})") + e = ( + f"Host {self.name}: Found template '{zbx_template['name']}' " + f"(ID:{zbx_template['templateid']})" + ) self.logger.debug(e) # Return error should the template not be found in Zabbix if not template_match: @@ -331,7 +340,7 @@ class PhysicalDevice: self.group_ids.append({"groupid": group["groupid"]}) e = ( f"Host {self.name}: Matched group " - f"\"{group['name']}\" (ID:{group['groupid']})" + f'"{group["name"]}" (ID:{group["groupid"]})' ) self.logger.debug(e) if len(self.group_ids) == len(self.hostgroups): @@ -412,7 +421,7 @@ class PhysicalDevice: macros = ZabbixUsermacros( self.nb, self._usermacro_map(), - config['usermacro_sync'], + config["usermacro_sync"], logger=self.logger, host=self.name, ) @@ -430,14 +439,14 @@ class PhysicalDevice: tags = ZabbixTags( self.nb, self._tag_map(), - tag_sync=config['tag_sync'], - tag_lower=config['tag_lower'], - tag_name=config['tag_name'], - tag_value=config['tag_value'], + tag_sync=config["tag_sync"], + tag_lower=config["tag_lower"], + tag_name=config["tag_name"], + tag_value=config["tag_value"], logger=self.logger, host=self.name, ) - if config['tag_sync'] is False: + if config["tag_sync"] is False: self.tags = [] return False self.tags = tags.generate() @@ -477,12 +486,12 @@ class PhysicalDevice: # If the proxy name matches if proxy["name"] == proxy_name: self.logger.debug( - f"Host {self.name}: using {proxy['type']}" f" '{proxy_name}'" + "Host %s: using {proxy['type']} '%s'", self.name, proxy_name ) self.zbxproxy = proxy return True self.logger.warning( - f"Host {self.name}: unable to find proxy {proxy_name}" + "Host %s: unable to find proxy %s", self.name, proxy_name ) return False @@ -554,7 +563,7 @@ class PhysicalDevice: self.create_journal_entry("success", msg) else: self.logger.error( - f"Host {self.name}: Unable to add to Zabbix. Host already present." + "Host %s: Unable to add to Zabbix. Host already present.", self.name ) def createZabbixHostgroup(self, hostgroups): @@ -612,7 +621,9 @@ class PhysicalDevice: ) self.logger.error(e) raise SyncExternalError(e) from None - self.logger.info(f"Host {self.name}: updated with data {sanatize_log_output(kwargs)}.") + self.logger.info( + "Host %s: updated with data %s.", self.name, sanatize_log_output(kwargs) + ) self.create_journal_entry("info", "Updated host in Zabbix with latest NB data.") def ConsistencyCheck( @@ -623,7 +634,7 @@ class PhysicalDevice: Checks if Zabbix object is still valid with NetBox parameters. """ # If group is found or if the hostgroup is nested - if not self.setZabbixGroupID(groups): # or len(self.hostgroups.split("/")) > 1: + if not self.setZabbixGroupID(groups): # or len(self.hostgroups.split("/")) > 1: if create_hostgroups: # Script is allowed to create a new hostgroup new_groups = self.createZabbixHostgroup(groups) @@ -672,28 +683,30 @@ class PhysicalDevice: raise SyncInventoryError(e) host = host[0] if host["host"] == self.name: - self.logger.debug(f"Host {self.name}: Hostname in-sync.") + self.logger.debug("Host %s: Hostname in-sync.", self.name) else: self.logger.info( - f"Host {self.name}: Hostname OUT of sync. " - f"Received value: {host['host']}" + "Host %s: Hostname OUT of sync. Received value: %s", + self.name, + host["host"], ) self.updateZabbixHost(host=self.name) # Execute check depending on wether the name is special or not if self.use_visible_name: if host["name"] == self.visible_name: - self.logger.debug(f"Host {self.name}: Visible name in-sync.") + self.logger.debug("Host %s: Visible name in-sync.", self.name) else: self.logger.info( - f"Host {self.name}: Visible name OUT of sync." - f" Received value: {host['name']}" + "Host %s: Visible name OUT of sync. Received value: %s", + self.name, + host["name"], ) self.updateZabbixHost(name=self.visible_name) # Check if the templates are in-sync if not self.zbx_template_comparer(host["parentTemplates"]): - self.logger.info(f"Host {self.name}: Template(s) OUT of sync.") + self.logger.info("Host %s: Template(s) OUT of sync.", self.name) # Prepare Templates for API parsing templateids = [] for template in self.zbx_templates: @@ -703,38 +716,41 @@ class PhysicalDevice: templates_clear=host["parentTemplates"], templates=templateids ) else: - self.logger.debug(f"Host {self.name}: Template(s) in-sync.") + self.logger.debug("Host %s: Template(s) in-sync.", self.name) # Check if Zabbix version is 6 or higher. Issue #93 group_dictname = "hostgroups" if str(self.zabbix.version).startswith(("6", "5")): group_dictname = "groups" # Check if hostgroups match - if (sorted(host[group_dictname], key=itemgetter('groupid')) == - sorted(self.group_ids, key=itemgetter('groupid'))): - self.logger.debug(f"Host {self.name}: Hostgroups in-sync.") + if sorted(host[group_dictname], key=itemgetter("groupid")) == sorted( + self.group_ids, key=itemgetter("groupid") + ): + self.logger.debug("Host %s: Hostgroups in-sync.", self.name) else: - self.logger.info(f"Host {self.name}: Hostgroups OUT of sync.") + self.logger.info("Host %s: Hostgroups OUT of sync.", self.name) self.updateZabbixHost(groups=self.group_ids) if int(host["status"]) == self.zabbix_state: - self.logger.debug(f"Host {self.name}: Status in-sync.") + self.logger.debug("Host %s: Status in-sync.", self.name) else: - self.logger.info(f"Host {self.name}: Status OUT of sync.") + self.logger.info("Host %s: Status OUT of sync.", self.name) self.updateZabbixHost(status=str(self.zabbix_state)) # Check if a proxy has been defined if self.zbxproxy: # Check if proxy or proxy group is defined - if (self.zbxproxy["idtype"] in host and - host[self.zbxproxy["idtype"]] == self.zbxproxy["id"]): - self.logger.debug(f"Host {self.name}: Proxy in-sync.") + if ( + self.zbxproxy["idtype"] in host + and host[self.zbxproxy["idtype"]] == self.zbxproxy["id"] + ): + self.logger.debug("Host %s: Proxy in-sync.", self.name) # Backwards compatibility for Zabbix <= 6 elif "proxy_hostid" in host and host["proxy_hostid"] == self.zbxproxy["id"]: - self.logger.debug(f"Host {self.name}: Proxy in-sync.") + self.logger.debug("Host %s: Proxy in-sync.", self.name) # Proxy does not match, update Zabbix else: - self.logger.info(f"Host {self.name}: Proxy OUT of sync.") + self.logger.info("Host %s: Proxy OUT of sync.", self.name) # Zabbix <= 6 patch if not str(self.zabbix.version).startswith("7"): self.updateZabbixHost(proxy_hostid=self.zbxproxy["id"]) @@ -757,8 +773,8 @@ class PhysicalDevice: if proxy_power and proxy_set: # Zabbix <= 6 fix self.logger.warning( - f"Host {self.name}: No proxy is configured in NetBox " - "but is configured in Zabbix. Removing proxy config in Zabbix" + "Host %s: No proxy is configured in NetBox but is configured in Zabbix. Removing proxy config in Zabbix", + self.name, ) if "proxy_hostid" in host and bool(host["proxy_hostid"]): self.updateZabbixHost(proxy_hostid=0) @@ -772,59 +788,59 @@ class PhysicalDevice: if proxy_set and not proxy_power: # Display error message self.logger.warning( - f"Host {self.name}: Is configured " - f"with proxy in Zabbix but not in NetBox. The" - " -p flag was ommited: no " - "changes have been made." + "Host %s: Is configured with proxy in Zabbix but not in NetBox. The -p flag was ommited: no changes have been made.", + self.name, ) if not proxy_set: - self.logger.debug(f"Host {self.name}: Proxy in-sync.") + self.logger.debug("Host %s: Proxy in-sync.", self.name) # Check host inventory mode if str(host["inventory_mode"]) == str(self.inventory_mode): - self.logger.debug(f"Host {self.name}: inventory_mode in-sync.") + self.logger.debug("Host %s: inventory_mode in-sync.", self.name) else: - self.logger.info(f"Host {self.name}: inventory_mode OUT of sync.") + self.logger.info("Host %s: inventory_mode OUT of sync.", self.name) self.updateZabbixHost(inventory_mode=str(self.inventory_mode)) if config["inventory_sync"] and self.inventory_mode in [0, 1]: # Check host inventory mapping if host["inventory"] == self.inventory: - self.logger.debug(f"Host {self.name}: Inventory in-sync.") + self.logger.debug("Host %s: Inventory in-sync.", self.name) else: - self.logger.info(f"Host {self.name}: Inventory OUT of sync.") + self.logger.info("Host %s: Inventory OUT of sync.", self.name) self.updateZabbixHost(inventory=self.inventory) # Check host usermacros - if config['usermacro_sync']: + if config["usermacro_sync"]: # Make a full copy synce we dont want to lose the original value # of secret type macros from Netbox netbox_macros = deepcopy(self.usermacros) # Set the sync bit - full_sync_bit = bool(str(config['usermacro_sync']).lower() == "full") + full_sync_bit = bool(str(config["usermacro_sync"]).lower() == "full") for macro in netbox_macros: # If the Macro is a secret and full sync is NOT activated if macro["type"] == str(1) and not full_sync_bit: # Remove the value as the Zabbix api does not return the value key # This is required when you want to do a diff between both lists macro.pop("value") + # Sort all lists def filter_with_macros(macro): return macro["macro"] + host["macros"].sort(key=filter_with_macros) netbox_macros.sort(key=filter_with_macros) # Check if both lists are the same if host["macros"] == netbox_macros: - self.logger.debug(f"Host {self.name}: Usermacros in-sync.") + self.logger.debug("Host %s: Usermacros in-sync.", self.name) else: - self.logger.info(f"Host {self.name}: Usermacros OUT of sync.") + self.logger.info("Host %s: Usermacros OUT of sync.", self.name) # Update Zabbix with NetBox usermacros self.updateZabbixHost(macros=self.usermacros) # Check host tags - if config['tag_sync']: + if config["tag_sync"]: if remove_duplicates(host["tags"], sortkey="tag") == self.tags: - self.logger.debug(f"Host {self.name}: Tags in-sync.") + self.logger.debug("Host %s: Tags in-sync.", self.name) else: - self.logger.info(f"Host {self.name}: Tags OUT of sync.") + self.logger.info("Host %s: Tags OUT of sync.", self.name) self.updateZabbixHost(tags=self.tags) # If only 1 interface has been found @@ -862,7 +878,7 @@ class PhysicalDevice: updates[key] = item if updates: # If interface updates have been found: push to Zabbix - self.logger.info(f"Host {self.name}: Interface OUT of sync.") + self.logger.info("Host %s: Interface OUT of sync.", self.name) if "type" in updates: # Changing interface type not supported. Raise exception. e = ( @@ -876,26 +892,27 @@ class PhysicalDevice: try: # API call to Zabbix self.zabbix.hostinterface.update(updates) - e = (f"Host {self.name}: Updated interface " - f"with data {sanatize_log_output(updates)}.") - self.logger.info(e) - self.create_journal_entry("info", e) + err_msg = ( + f"Host {self.name}: Updated interface " + f"with data {sanatize_log_output(updates)}." + ) + self.logger.info(err_msg) + self.create_journal_entry("info", err_msg) except APIRequestError as e: msg = f"Zabbix returned the following error: {str(e)}." self.logger.error(msg) raise SyncExternalError(msg) from e else: # If no updates are found, Zabbix interface is in-sync - e = f"Host {self.name}: Interface in-sync." - self.logger.debug(e) + self.logger.debug("Host %s: Interface in-sync.", self.name) else: - e = ( + err_msg = ( f"Host {self.name}: Has unsupported interface configuration." f" Host has total of {len(host['interfaces'])} interfaces. " "Manual intervention required." ) - self.logger.error(e) - raise SyncInventoryError(e) + self.logger.error(err_msg) + raise SyncInventoryError(err_msg) def create_journal_entry(self, severity, message): """ @@ -906,7 +923,7 @@ class PhysicalDevice: # Check if the severity is valid if severity not in ["info", "success", "warning", "danger"]: self.logger.warning( - f"Value {severity} not valid for NB journal entries." + "Value %s not valid for NB journal entries.", severity ) return False journal = { @@ -917,12 +934,11 @@ class PhysicalDevice: } try: self.nb_journals.create(journal) - self.logger.debug(f"Host {self.name}: Created journal entry in NetBox") + self.logger.debug("Host %s: Created journal entry in NetBox", self.name) return True except NetboxRequestError as e: self.logger.warning( - "Unable to create journal entry for " - f"{self.name}: NB returned {e}" + "Unable to create journal entry for %s: NB returned {e}", self.name ) return False return False @@ -947,8 +963,9 @@ class PhysicalDevice: tmpls_from_zabbix.pop(pos) succesfull_templates.append(nb_tmpl) self.logger.debug( - f"Host {self.name}: Template " - f"'{nb_tmpl['name']}' is present in Zabbix." + "Host %s: Template '%s' is present in Zabbix.", + self.name, + nb_tmpl["name"], ) break if ( diff --git a/modules/hostgroups.py b/modules/hostgroups.py index 213b4cf..58bb057 100644 --- a/modules/hostgroups.py +++ b/modules/hostgroups.py @@ -94,8 +94,11 @@ class Hostgroup: format_options["cluster"] = self.nb.cluster.name format_options["cluster_type"] = self.nb.cluster.type.name self.format_options = format_options - self.logger.debug(f"Host {self.name}: Resolved properties for use " - f"in hostgroups: {self.format_options}") + self.logger.debug( + "Host %s: Resolved properties for use in hostgroups: %s", + self.name, + self.format_options, + ) def set_nesting( self, nested_sitegroup_flag, nested_region_flag, nb_groups, nb_regions @@ -134,7 +137,9 @@ class Hostgroup: if hostgroup_value: hg_output.append(hostgroup_value) else: - self.logger.info(f"Host {self.name}: Used field '{hg_item}' has no value.") + self.logger.info( + "Host %s: Used field '%s' has no value.", self.name, hg_item + ) # Check if the hostgroup is populated with at least one item. if bool(hg_output): return "/".join(hg_output) diff --git a/modules/tags.py b/modules/tags.py index f341abd..835490c 100644 --- a/modules/tags.py +++ b/modules/tags.py @@ -3,6 +3,7 @@ """ All of the Zabbix Usermacro related configuration """ + from logging import getLogger from modules.tools import field_mapper, remove_duplicates @@ -76,7 +77,7 @@ class ZabbixTags: else: tag["tag"] = tag_name else: - self.logger.warning(f"Tag '{tag_name}' is not a valid tag name, skipping.") + self.logger.warning("Tag '%s' is not a valid tag name, skipping.", tag_name) return False if self.validate_value(tag_value): @@ -86,7 +87,7 @@ class ZabbixTags: tag["value"] = tag_value else: self.logger.info( - f"Tag '{tag_name}' has an invalid value: '{tag_value}', skipping." + "Tag '%s' has an invalid value: '%s', skipping.", tag_name, tag_value ) return False return tag @@ -99,7 +100,7 @@ class ZabbixTags: tags = [] # Parse the field mapper for tags if self.tag_map: - self.logger.debug(f"Host {self.nb.name}: Starting tag mapper.") + self.logger.debug("Host %s: Starting tag mapper.", self.nb.name) field_tags = field_mapper(self.nb.name, self.tag_map, self.nb, self.logger) for tag, value in field_tags.items(): t = self.render_tag(tag, value) @@ -131,5 +132,5 @@ class ZabbixTags: tags.append(t) tags = remove_duplicates(tags, sortkey="tag") - self.logger.debug(f"Host {self.name}: Resolved tags: {tags}") + self.logger.debug("Host %s: Resolved tags: %s", self.name, tags) return tags diff --git a/modules/tools.py b/modules/tools.py index c49f5dd..ae7a12b 100644 --- a/modules/tools.py +++ b/modules/tools.py @@ -1,6 +1,8 @@ """A collection of tools used by several classes""" + from modules.exceptions import HostgroupError + def convert_recordset(recordset): """Converts netbox RedcordSet to list of dicts.""" recordlist = [] @@ -72,19 +74,22 @@ def field_mapper(host, mapper, nbdevice, logger): elif not value: # empty value should just be an empty string for API compatibility logger.info( - f"Host {host}: NetBox lookup for " - f"'{nb_field}' returned an empty value." + "Host %s: NetBox lookup for '%s' returned an empty value.", + host, + nb_field, ) data[zbx_field] = "" else: # Value is not a string or numeral, probably not what the user expected. logger.info( - f"Host {host}: Lookup for '{nb_field}'" - " returned an unexpected type: it will be skipped." + "Host %s: Lookup for '%s' returned an unexpected type: it will be skipped.", + host, + nb_field, ) logger.debug( - f"Host {host}: Field mapping complete. " - f"Mapped {len(list(filter(None, data.values())))} field(s)." + "Host %s: Field mapping complete. Mapped %s field(s).", + host, + len(list(filter(None, data.values()))), ) return data @@ -101,7 +106,9 @@ def remove_duplicates(input_list, sortkey=None): return output_list -def verify_hg_format(hg_format, device_cfs=None, vm_cfs=None, hg_type="dev", logger=None): +def verify_hg_format( + hg_format, device_cfs=None, vm_cfs=None, hg_type="dev", logger=None +): """ Verifies hostgroup field format """ @@ -109,44 +116,51 @@ def verify_hg_format(hg_format, device_cfs=None, vm_cfs=None, hg_type="dev", log device_cfs = [] if not vm_cfs: vm_cfs = [] - allowed_objects = {"dev": ["location", - "rack", - "role", - "manufacturer", - "region", - "site", - "site_group", - "tenant", - "tenant_group", - "platform", - "cluster"] - ,"vm": ["cluster_type", - "role", - "manufacturer", - "region", - "site", - "site_group", - "tenant", - "tenant_group", - "cluster", - "device", - "platform"] - ,"cfs": {"dev": [], "vm": []} - } + allowed_objects = { + "dev": [ + "location", + "rack", + "role", + "manufacturer", + "region", + "site", + "site_group", + "tenant", + "tenant_group", + "platform", + "cluster", + ], + "vm": [ + "cluster_type", + "role", + "manufacturer", + "region", + "site", + "site_group", + "tenant", + "tenant_group", + "cluster", + "device", + "platform", + ], + "cfs": {"dev": [], "vm": []}, + } for cf in device_cfs: - allowed_objects['cfs']['dev'].append(cf.name) + allowed_objects["cfs"]["dev"].append(cf.name) for cf in vm_cfs: - allowed_objects['cfs']['vm'].append(cf.name) + allowed_objects["cfs"]["vm"].append(cf.name) hg_objects = [] - if isinstance(hg_format,list): + if isinstance(hg_format, list): for f in hg_format: hg_objects = hg_objects + f.split("/") else: hg_objects = hg_format.split("/") hg_objects = sorted(set(hg_objects)) for hg_object in hg_objects: - if (hg_object not in allowed_objects[hg_type] and - hg_object not in allowed_objects['cfs'][hg_type]): + if ( + hg_object not in allowed_objects[hg_type] + and hg_object not in allowed_objects["cfs"][hg_type] + ): e = ( f"Hostgroup item {hg_object} is not valid. Make sure you" " use valid items and separate them with '/'." @@ -168,8 +182,7 @@ def sanatize_log_output(data): if "macros" in data: for macro in sanitized_data["macros"]: # Check if macro is secret type - if not (macro["type"] == str(1) or - macro["type"] == 1): + if not (macro["type"] == str(1) or macro["type"] == 1): continue macro["value"] = "********" # Check for interface data diff --git a/modules/usermacros.py b/modules/usermacros.py index cfa8082..acf8725 100644 --- a/modules/usermacros.py +++ b/modules/usermacros.py @@ -3,6 +3,7 @@ """ All of the Zabbix Usermacro related configuration """ + from logging import getLogger from re import match @@ -57,8 +58,11 @@ class ZabbixUsermacros: macro["macro"] = str(macro_name) if isinstance(macro_properties, dict): if not "value" in macro_properties: - self.logger.info(f"Host {self.name}: Usermacro {macro_name} has " - "no value in Netbox, skipping.") + self.logger.info( + "Host %s: Usermacro %s has no value in Netbox, skipping.", + self.name, + macro_name, + ) return False macro["value"] = macro_properties["value"] @@ -83,12 +87,17 @@ class ZabbixUsermacros: macro["description"] = "" else: - self.logger.info(f"Host {self.name}: Usermacro {macro_name} " - "has no value, skipping.") + self.logger.info( + "Host %s: Usermacro %s has no value, skipping.", + self.name, + macro_name, + ) return False else: self.logger.warning( - f"Host {self.name}: Usermacro {macro_name} is not a valid usermacro name, skipping." + "Host %s: Usermacro %s is not a valid usermacro name, skipping.", + self.name, + macro_name, ) return False return macro @@ -98,10 +107,10 @@ class ZabbixUsermacros: Generate full set of Usermacros """ macros = [] - data={} + data = {} # Parse the field mapper for usermacros if self.usermacro_map: - self.logger.debug(f"Host {self.nb.name}: Starting usermacro mapper.") + self.logger.debug("Host %s: Starting usermacro mapper.", self.nb.name) field_macros = field_mapper( self.nb.name, self.usermacro_map, self.nb, self.logger ) @@ -120,6 +129,8 @@ class ZabbixUsermacros: m = self.render_macro(macro, properties) if m: macros.append(m) - data={'macros': macros} - self.logger.debug(f"Host {self.name}: Resolved macros: {sanatize_log_output(data)}") + data = {"macros": macros} + self.logger.debug( + "Host %s: Resolved macros: %s", self.name, sanatize_log_output(data) + ) return macros diff --git a/netbox_zabbix_sync.py b/netbox_zabbix_sync.py index 3e783e8..79fa27e 100755 --- a/netbox_zabbix_sync.py +++ b/netbox_zabbix_sync.py @@ -2,6 +2,7 @@ # pylint: disable=invalid-name, logging-not-lazy, too-many-locals, logging-fstring-interpolation """NetBox to Zabbix sync script.""" + import argparse import logging import ssl @@ -67,15 +68,15 @@ def main(arguments): try: # Get NetBox version nb_version = netbox.version - logger.debug(f"NetBox version is {nb_version}.") + logger.debug("NetBox version is %s.", nb_version) except RequestsConnectionError: logger.error( - f"Unable to connect to NetBox with URL {netbox_host}." - " Please check the URL and status of NetBox." + "Unable to connect to NetBox with URL %s. Please check the URL and status of NetBox.", + netbox_host, ) sys.exit(1) except NBRequestError as e: - logger.error(f"NetBox error: {e}") + logger.error("NetBox error: %s", e) sys.exit(1) # Check if the provided Hostgroup layout is valid device_cfs = [] @@ -83,14 +84,18 @@ def main(arguments): device_cfs = list( netbox.extras.custom_fields.filter(type="text", content_types="dcim.device") ) - verify_hg_format(config["hostgroup_format"], - device_cfs=device_cfs, hg_type="dev", logger=logger) + verify_hg_format( + config["hostgroup_format"], device_cfs=device_cfs, hg_type="dev", logger=logger + ) if config["sync_vms"]: vm_cfs = list( - netbox.extras.custom_fields.filter(type="text", - content_types="virtualization.virtualmachine") + netbox.extras.custom_fields.filter( + type="text", content_types="virtualization.virtualmachine" + ) + ) + verify_hg_format( + config["vm_hostgroup_format"], vm_cfs=vm_cfs, hg_type="vm", logger=logger ) - verify_hg_format(config["vm_hostgroup_format"], vm_cfs=vm_cfs, hg_type="vm", logger=logger) # Set Zabbix API try: ssl_ctx = ssl.create_default_context() @@ -120,7 +125,8 @@ def main(arguments): netbox_vms = [] if config["sync_vms"]: netbox_vms = list( - netbox.virtualization.virtual_machines.filter(**config["nb_vm_filter"])) + netbox.virtualization.virtual_machines.filter(**config["nb_vm_filter"]) + ) netbox_site_groups = convert_recordset((netbox.dcim.site_groups.all())) netbox_regions = convert_recordset(netbox.dcim.regions.all()) netbox_journals = netbox.extras.journal_entries @@ -141,15 +147,22 @@ def main(arguments): # Go through all NetBox devices for nb_vm in netbox_vms: try: - vm = VirtualMachine(nb_vm, zabbix, netbox_journals, nb_version, - config["create_journal"], logger) - logger.debug(f"Host {vm.name}: Started operations on VM.") + vm = VirtualMachine( + nb_vm, + zabbix, + netbox_journals, + nb_version, + config["create_journal"], + logger, + ) + logger.debug("Host %s: Started operations on VM.", vm.name) vm.set_vm_template() # Check if a valid template has been found for this VM. if not vm.zbx_template_names: continue - vm.set_hostgroup(config["vm_hostgroup_format"], - netbox_site_groups, netbox_regions) + vm.set_hostgroup( + config["vm_hostgroup_format"], netbox_site_groups, netbox_regions + ) # Check if a valid hostgroup has been found for this VM. if not vm.hostgroups: continue @@ -162,13 +175,12 @@ def main(arguments): # Delete device from Zabbix # and remove hostID from NetBox. vm.cleanup() - logger.info(f"VM {vm.name}: cleanup complete") + logger.info("VM %s: cleanup complete", vm.name) continue # Device has been added to NetBox # but is not in Activate state logger.info( - f"VM {vm.name}: Skipping since this VM is " - f"not in the active state." + "VM %s: Skipping since this VM is not in the active state.", vm.name ) continue # Check if the VM is in the disabled state @@ -200,20 +212,31 @@ def main(arguments): for nb_device in netbox_devices: try: # Set device instance set data such as hostgroup and template information. - device = PhysicalDevice(nb_device, zabbix, netbox_journals, nb_version, - config["create_journal"], logger) - logger.debug(f"Host {device.name}: Started operations on device.") - device.set_template(config["templates_config_context"], - config["templates_config_context_overrule"]) + device = PhysicalDevice( + nb_device, + zabbix, + netbox_journals, + nb_version, + config["create_journal"], + logger, + ) + logger.debug("Host %s: Started operations on device.", device.name) + device.set_template( + config["templates_config_context"], + config["templates_config_context_overrule"], + ) # Check if a valid template has been found for this VM. if not device.zbx_template_names: continue device.set_hostgroup( - config["hostgroup_format"], netbox_site_groups, netbox_regions) + config["hostgroup_format"], netbox_site_groups, netbox_regions + ) # Check if a valid hostgroup has been found for this VM. if not device.hostgroups: - logger.warning(f"Host {device.name}: Host has no valid " - f"hostgroups, Skipping this host...") + logger.warning( + "Host %s: Host has no valid hostgroups, Skipping this host...", + device.name, + ) continue device.set_inventory(nb_device) device.set_usermacros() @@ -223,16 +246,16 @@ def main(arguments): if device.isCluster() and config["clustering"]: # Check if device is primary or secondary if device.promoteMasterDevice(): - e = f"Device {device.name}: is " f"part of cluster and primary." - logger.info(e) + logger.info( + "Device %s: is part of cluster and primary.", device.name + ) else: # Device is secondary in cluster. # Don't continue with this device. - e = ( - f"Device {device.name}: Is part of cluster " - f"but not primary. Skipping this host..." + logger.info( + "Device %s: Is part of cluster but not primary. Skipping this host...", + device.name, ) - logger.info(e) continue # Checks if device is in cleanup state if device.status in config["zabbix_device_removal"]: @@ -240,13 +263,13 @@ def main(arguments): # Delete device from Zabbix # and remove hostID from NetBox. device.cleanup() - logger.info(f"Device {device.name}: cleanup complete") + logger.info("Device %s: cleanup complete", device.name) continue # Device has been added to NetBox # but is not in Activate state logger.info( - f"Device {device.name}: Skipping since this device is " - f"not in the active state." + "Device %s: Skipping since this device is not in the active state.", + device.name, ) continue # Check if the device is in the disabled state From e71856068985e39b237cbac17f5808cf2cfdc1ed Mon Sep 17 00:00:00 2001 From: Wouter de Bruijn Date: Wed, 25 Jun 2025 16:37:44 +0200 Subject: [PATCH 17/34] =?UTF-8?q?=F0=9F=9A=A8=20Line=20length=20fixes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- modules/device.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/modules/device.py b/modules/device.py index dcae69c..cf0e2a2 100644 --- a/modules/device.py +++ b/modules/device.py @@ -112,7 +112,8 @@ class PhysicalDevice: self.visible_name = self.nb.name self.use_visible_name = True self.logger.info( - "Host %s contains special characters. Using %s as name for the NetBox object and using %s as visible name in Zabbix.", + "Host %s contains special characters." + "Using %s as name for the NetBox object and using %s as visible name in Zabbix.", self.visible_name, self.name, self.visible_name, @@ -213,7 +214,8 @@ class PhysicalDevice: if config["inventory_mode"] == "disabled": if config["inventory_sync"]: self.logger.error( - "Host %s: Unable to map NetBox inventory to Zabbix. Inventory sync is enabled in config but inventory mode is disabled", + "Host %s: Unable to map NetBox inventory to Zabbix." + "Inventory sync is enabled in config but inventory mode is disabled", self.name, ) return True @@ -773,7 +775,8 @@ class PhysicalDevice: if proxy_power and proxy_set: # Zabbix <= 6 fix self.logger.warning( - "Host %s: No proxy is configured in NetBox but is configured in Zabbix. Removing proxy config in Zabbix", + "Host %s: No proxy is configured in NetBox but is configured in Zabbix." + "Removing proxy config in Zabbix", self.name, ) if "proxy_hostid" in host and bool(host["proxy_hostid"]): @@ -788,7 +791,8 @@ class PhysicalDevice: if proxy_set and not proxy_power: # Display error message self.logger.warning( - "Host %s: Is configured with proxy in Zabbix but not in NetBox. The -p flag was ommited: no changes have been made.", + "Host %s: Is configured with proxy in Zabbix but not in NetBox." + "The -p flag was ommited: no changes have been made.", self.name, ) if not proxy_set: @@ -938,7 +942,9 @@ class PhysicalDevice: return True except NetboxRequestError as e: self.logger.warning( - "Unable to create journal entry for %s: NB returned {e}", self.name + "Unable to create journal entry for %s: NB returned %s", + self.name, + e, ) return False return False From 98c13919c570a63564240186a4cac5094231e035 Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Wed, 25 Jun 2025 16:50:17 +0200 Subject: [PATCH 18/34] Added support for hardcoded strings in hostgroups --- modules/hostgroups.py | 31 ++++++++++++++++++------------- modules/tools.py | 1 + 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/modules/hostgroups.py b/modules/hostgroups.py index 58bb057..6da099a 100644 --- a/modules/hostgroups.py +++ b/modules/hostgroups.py @@ -117,19 +117,24 @@ class Hostgroup: for hg_item in hg_items: # Check if requested data is available as option for this host if hg_item not in self.format_options: - # Check if a custom field exists with this name - cf_data = self.custom_field_lookup(hg_item) - # CF does not exist - if not cf_data["result"]: - msg = ( - f"Unable to generate hostgroup for host {self.name}. " - f"Item type {hg_item} not supported." - ) - self.logger.error(msg) - raise HostgroupError(msg) - # CF data is populated - if cf_data["cf"]: - hg_output.append(cf_data["cf"]) + if hg_item.startswith(("'", '"')) and hg_item.endswith(("'", '"')): + hg_item = hg_item.strip("\'") + hg_item = hg_item.strip('\"') + hg_output.append(hg_item) + else: + # Check if a custom field exists with this name + cf_data = self.custom_field_lookup(hg_item) + # CF does not exist + if not cf_data["result"]: + msg = ( + f"Unable to generate hostgroup for host {self.name}. " + f"Item type {hg_item} not supported." + ) + self.logger.error(msg) + raise HostgroupError(msg) + # CF data is populated + if cf_data["cf"]: + hg_output.append(cf_data["cf"]) continue # Check if there is a value associated to the variable. # For instance, if a device has no location, do not use it with hostgroup calculation diff --git a/modules/tools.py b/modules/tools.py index ae7a12b..dacab20 100644 --- a/modules/tools.py +++ b/modules/tools.py @@ -160,6 +160,7 @@ def verify_hg_format( if ( hg_object not in allowed_objects[hg_type] and hg_object not in allowed_objects["cfs"][hg_type] + and not hg_object.startswith(('"',"'")) ): e = ( f"Hostgroup item {hg_object} is not valid. Make sure you" From 3910e0de2d04a779d79a0aeee9c355657319b4d4 Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Wed, 25 Jun 2025 16:54:12 +0200 Subject: [PATCH 19/34] Updated docs --- README.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/README.md b/README.md index ef280cc..169aca0 100644 --- a/README.md +++ b/README.md @@ -212,6 +212,17 @@ in `config.py` the script will render a full region path of all parent regions for the hostgroup name. `traverse_site_groups` controls the same behaviour for site_groups. +**Hardcoded text** + +You can add hardcoded text in the hostgroup format by using quotes, this will +insert the literal text: + +```python +hostgroup_format = "'MyDevices'/location/role" +``` + +In this case, the prefix MyDevices will be used for all generated groups. + **Custom fields** You can use the value of custom fields for hostgroup generation. This allows From e82c098e26603d07fc67aff9ed6ccd9c9e101c8c Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Wed, 25 Jun 2025 17:00:04 +0200 Subject: [PATCH 20/34] corrected linting error --- modules/hostgroups.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/hostgroups.py b/modules/hostgroups.py index 6da099a..954c218 100644 --- a/modules/hostgroups.py +++ b/modules/hostgroups.py @@ -120,7 +120,7 @@ class Hostgroup: if hg_item.startswith(("'", '"')) and hg_item.endswith(("'", '"')): hg_item = hg_item.strip("\'") hg_item = hg_item.strip('\"') - hg_output.append(hg_item) + hg_output.append(hg_item) else: # Check if a custom field exists with this name cf_data = self.custom_field_lookup(hg_item) From 161b310ba379afdd91be7240549823205db85e41 Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Wed, 25 Jun 2025 17:07:46 +0200 Subject: [PATCH 21/34] corrected linting error --- modules/hostgroups.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/hostgroups.py b/modules/hostgroups.py index 954c218..5916c0a 100644 --- a/modules/hostgroups.py +++ b/modules/hostgroups.py @@ -120,7 +120,7 @@ class Hostgroup: if hg_item.startswith(("'", '"')) and hg_item.endswith(("'", '"')): hg_item = hg_item.strip("\'") hg_item = hg_item.strip('\"') - hg_output.append(hg_item) + hg_output.append(hg_item) else: # Check if a custom field exists with this name cf_data = self.custom_field_lookup(hg_item) From c58a3e8dd50b10d994b0e35eaf30cd6ee03334f4 Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Thu, 26 Jun 2025 09:48:25 +0200 Subject: [PATCH 22/34] Update README.md Replaced dependency pyzabbix with zabbix-utils as this was changed a few months ago. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 169aca0..e6cabfa 100644 --- a/README.md +++ b/README.md @@ -51,7 +51,7 @@ pip. ```sh # Packages: pynetbox -pyzabbix +zabbix-utils # Install them through requirements.txt from a venv: virtualenv .venv From 9259e736173769796868909a1c6f4f9a89e6aa90 Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Mon, 8 Sep 2025 14:44:46 +0200 Subject: [PATCH 23/34] Added option to extend site information for devices and vms. --- .gitignore | 1 + config.py.example | 18 +++++++++++------- modules/config.py | 1 + modules/device.py | 1 + 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/.gitignore b/.gitignore index c515fe3..0cf4953 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,6 @@ *.log .venv +.env config.py Pipfile Pipfile.lock diff --git a/config.py.example b/config.py.example index e4082e6..869da30 100644 --- a/config.py.example +++ b/config.py.example @@ -53,6 +53,12 @@ hostgroup_format = "site/manufacturer/role" traverse_regions = False traverse_site_groups = False +## Extended site properteis +# By default, NetBox will only return basic site info for any device or VM. +# By setting `extended_site_properties` to True, the script will query NetBox for additiopnal site info. +# Be aware that this will increase the number of API queries to NetBox. +extended_site_properties = False + ## Filtering # Custom device filter, variable must be present but can be left empty with no filtering. # A couple of examples: @@ -87,8 +93,8 @@ device_inventory_map = { "asset_tag": "asset_tag", "virtual_chassis/name": "chassis", "status/label": "deployment_status", "location/name": "location", - "latitude": "location_lat", - "longitude": "location_lon", + "site/latitude": "location_lat", + "site/longitude": "location_lon", "comments": "notes", "name": "name", "rack/name": "site_rack", @@ -112,19 +118,19 @@ usermacro_sync = False # device usermacro_map to map NetBox fields to usermacros. device_usermacro_map = {"serial": "{$HW_SERIAL}", "role/name": "{$DEV_ROLE}", - "url": "{$NB_URL}", + "display_url": "{$NB_URL}", "id": "{$NB_ID}"} # virtual machine usermacro_map to map NetBox fields to usermacros. vm_usermacro_map = {"memory": "{$TOTAL_MEMORY}", "role/name": "{$DEV_ROLE}", - "url": "{$NB_URL}", + "display_url": "{$NB_URL}", "id": "{$NB_ID}"} # To sync host tags to Zabbix, set to True. tag_sync = False -# Setting tag_lower to True will lower capital letters ain tag names and values +# Setting tag_lower to True will lower capital letters in tag names and values # This is more inline with the Zabbix way of working with tags. # # You can however set this to False to ensure capital letters are synced to Zabbix tags. @@ -132,8 +138,6 @@ tag_lower = True # We can sync NetBox device/VM tags to Zabbix, but as NetBox tags don't follow the key/value # pattern, we need to specify a tag name to register the NetBox tags in Zabbix. -# -# # # If tag_name is set to False, we won't sync NetBox device/VM tags to Zabbix. tag_name = 'NetBox' diff --git a/modules/config.py b/modules/config.py index a105892..43bd775 100644 --- a/modules/config.py +++ b/modules/config.py @@ -31,6 +31,7 @@ DEFAULT_CONFIG = { "nb_vm_filter": {"name__n": "null"}, "inventory_mode": "disabled", "inventory_sync": False, + "extended_site_properties": False, "device_inventory_map": { "asset_tag": "asset_tag", "virtual_chassis/name": "chassis", diff --git a/modules/device.py b/modules/device.py index cf0e2a2..6b00846 100644 --- a/modules/device.py +++ b/modules/device.py @@ -2,6 +2,7 @@ """ Device specific handeling for NetBox to Zabbix """ +from pprint import pprint from copy import deepcopy from logging import getLogger From 79e82c4365f2dde8a6aa9c5dd281807fc8dd62cf Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Mon, 8 Sep 2025 14:47:48 +0200 Subject: [PATCH 24/34] Added option to extend site information for devices and vms. --- modules/device.py | 1 - netbox_zabbix_sync.py | 6 ++++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/modules/device.py b/modules/device.py index 6b00846..cf0e2a2 100644 --- a/modules/device.py +++ b/modules/device.py @@ -2,7 +2,6 @@ """ Device specific handeling for NetBox to Zabbix """ -from pprint import pprint from copy import deepcopy from logging import getLogger diff --git a/netbox_zabbix_sync.py b/netbox_zabbix_sync.py index 79fa27e..3a03f66 100755 --- a/netbox_zabbix_sync.py +++ b/netbox_zabbix_sync.py @@ -166,6 +166,9 @@ def main(arguments): # Check if a valid hostgroup has been found for this VM. if not vm.hostgroups: continue + if config["extended_site_properties"] and nb_vm.site: + logger.debug("VM %s: extending site information.", vm.name) + vm.site=(convert_recordset(netbox.dcim.sites.filter(id=nb_vm.site.id))) vm.set_inventory(nb_vm) vm.set_usermacros() vm.set_tags() @@ -238,6 +241,9 @@ def main(arguments): device.name, ) continue + if config["extended_site_properties"] and nb_device.site: + logger.debug("Device %s: extending site information.", device.name) + device.site=(convert_recordset(netbox.dcim.sites.filter(id=nb_device.site.id))) device.set_inventory(nb_device) device.set_usermacros() device.set_tags() From 18f52c1d40087904da6166e4338428f0869d107d Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Tue, 9 Sep 2025 09:36:58 +0200 Subject: [PATCH 25/34] Added documentation for extended site properties --- README.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/README.md b/README.md index e6cabfa..477f41c 100644 --- a/README.md +++ b/README.md @@ -289,6 +289,27 @@ hostgroup_format = "mycustomfieldname" NetBox-Zabbix-sync - ERROR - ESXI1 has no reliable hostgroup. This is most likely due to the use of custom fields that are empty. ``` +### Extended site properties + +By default, NetBox will only return the following properties under the 'site' key for a device: + +- site id +- (api) url +- display name +- name +- slug +- description + +However, NetBox-Zabbix-Sync allows you to extend these site properties with the full site information +so you can use this data in inventory fields, tags and usermacros. + +To enable this functionality, enable the following setting in your configuration file: + +`extended_site_properties = True` + +Keep in mind that enabling this option will increase the number of API calls to your NetBox instance, +this might impact performance on large syncs. + ### Device status By setting a status on a NetBox device you determine how the host is added (or From b5d7596de768d60d5d25990e663c3f999821ae9c Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Tue, 9 Sep 2025 10:00:53 +0200 Subject: [PATCH 26/34] Reverted device inventory map to work with default configuration --- config.py.example | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/config.py.example b/config.py.example index 869da30..cf0cf6b 100644 --- a/config.py.example +++ b/config.py.example @@ -93,8 +93,8 @@ device_inventory_map = { "asset_tag": "asset_tag", "virtual_chassis/name": "chassis", "status/label": "deployment_status", "location/name": "location", - "site/latitude": "location_lat", - "site/longitude": "location_lon", + "latitude": "location_lat", + "longitude": "location_lon", "comments": "notes", "name": "name", "rack/name": "site_rack", @@ -102,6 +102,8 @@ device_inventory_map = { "asset_tag": "asset_tag", "device_type/model": "type", "device_type/manufacturer/name": "vendor", "oob_ip/address": "oob_ip" } +# Replace latitude and longitude with site/latitude and and site/longitude to use +# site geo data. Enable extended_site_properties for this to work! # We also support inventory mapping on Virtual Machines. vm_inventory_map = { "status/label": "deployment_status", From 5810cbe621def08a6c01ace4dc50671403e9a040 Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Thu, 11 Sep 2025 17:20:05 +0200 Subject: [PATCH 27/34] First working version of proxy by custom fields --- README.md | 39 ++++++++++++++++++++++++++++++++++----- modules/config.py | 2 ++ modules/device.py | 20 ++++++++++++++++++-- 3 files changed, 54 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 477f41c..9616983 100644 --- a/README.md +++ b/README.md @@ -414,9 +414,9 @@ Tags can be synced from the following sources: Syncing tags will override any tags that were set manually on the host, making NetBox the single source-of-truth for managing tags. -To enable syncing, turn on tag_sync in the config file. +To enable syncing, turn on `tag_sync` in the config file. By default, this script will modify tag names and tag values to lowercase. -You can change this behaviour by setting tag_lower to False. +You can change this behaviour by setting `tag_lower` to `False`. ```python tag_sync = True @@ -429,7 +429,8 @@ As NetBox doesn't follow the tag/value pattern for tags, we will need a tag name set to register the netbox tags. By default the tag name is "NetBox", but you can change this to whatever you want. -The value for the tag can be set to 'name', 'display', or 'slug', which refers to the property of the NetBox tag object that will be used as the value in Zabbix. +The value for the tag can be set to 'name', 'display', or 'slug', which refers to the +property of the NetBox tag object that will be used as the value in Zabbix. ```python tag_name = 'NetBox' @@ -512,7 +513,7 @@ Through this method, it is possible to define the following types of usermacros: 2. Secret 3. Vault -The default macro type is text if no `type` and `value` have been set. +The default macro type is text, if no `type` and `value` have been set. It is also possible to create usermacros with [context](https://www.zabbix.com/documentation/7.0/en/manual/config/macros/user_macros_context). @@ -632,7 +633,8 @@ python3 netbox_zabbix_sync.py ### Zabbix proxy -You can set the proxy for a device using the 'proxy' key in config context. +#### Config Context +You can set the proxy for a device using the `proxy` key in config context. ```json { @@ -673,6 +675,33 @@ In the example above the host will use the group on Zabbix 7. On Zabbix 6 and below the host will use the proxy. Zabbix 7 will use the proxy value when omitting the proxy_group value. +#### Custom Field + +Alternatively, you can use a custom field for assigning a device or VM to +a Zabbix proxy or proxy group. The custom fields can be assigned to both +Devices and VMs. + +You can also assign these custom fields to a site to allow all devices/VMs +in that site to be confured with the same proxy or proxy group. +In order for this to work, `extended_site_properties` needs to be enabled in +the configuation as well. + +To use the custom fields for proxy configuration, configure one or both +of the following settings in the configuration file with the actual names of your +custom fields: + +```python +proxy_cf = "zabbix_proxy" +proxy_group_cf = "zabbix_proxy_group" +``` + +As with config context proxy configuration, proxy group will take precedence over +proxy when configured. +Proxy settings configured on the device or VM will take precedence over any site configuration. + +If the custom fields have no value but the proxy or proxy group is configured in config context, +that setting will be used. + ### Set interface parameters within NetBox When adding a new device, you can set the interface type with custom context. By diff --git a/modules/config.py b/modules/config.py index 43bd775..b1afcfb 100644 --- a/modules/config.py +++ b/modules/config.py @@ -16,6 +16,8 @@ DEFAULT_CONFIG = { "templates_config_context_overrule": False, "template_cf": "zabbix_template", "device_cf": "zabbix_hostid", + "proxy_cf": False, + "proxy_group_cf" : False, "clustering": False, "create_hostgroups": True, "create_journal": False, diff --git a/modules/device.py b/modules/device.py index cf0e2a2..ad99e9d 100644 --- a/modules/device.py +++ b/modules/device.py @@ -458,9 +458,14 @@ class PhysicalDevice: """ Sets proxy or proxy group if this value has been defined in config context + or custom fields. input: List of all proxies and proxy groups in standardized format """ + proxy_name = None + + #!!! FIX this check to still work if only CF is configurd. + # check if the key Zabbix is defined in the config context if "zabbix" not in self.nb.config_context: return False @@ -477,10 +482,21 @@ class PhysicalDevice: # Only insert groups in front of list for Zabbix7 proxy_types.insert(0, "proxy_group") for proxy_type in proxy_types: - # Check if the key exists in NetBox CC - if proxy_type in self.nb.config_context["zabbix"]: + # Check if we should use custom fields for proxy config + field_config = "proxy_cf" if proxy_type=="proxy" else "proxy_group_cf" + if config[field_config]: + if config[field_config] in self.nb.custom_fields: + if self.nb.custom_fields[config[field_config]]: + proxy_name = self.nb.custom_fields[config[field_config]] + elif config[field_config] in self.nb.site.custom_fields: + if self.nb.site.custom_fields[config[field_config]]: + proxy_name = self.nb.site.custom_fields[config[field_config]] + + # Otherwise check if the proxy is configured in NetBox CC + if not proxy_name and proxy_type in self.nb.config_context["zabbix"]: proxy_name = self.nb.config_context["zabbix"][proxy_type] # go through all proxies + if proxy_name: for proxy in proxy_list: # If the proxy does not match the type, ignore and continue if not proxy["type"] == proxy_type: From 17ba97be458c94e38c0fa46a9f9c74a1d1a32d3d Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Thu, 11 Sep 2025 17:26:05 +0200 Subject: [PATCH 28/34] Minor update on README --- README.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 9616983..e4b9f38 100644 --- a/README.md +++ b/README.md @@ -682,7 +682,7 @@ a Zabbix proxy or proxy group. The custom fields can be assigned to both Devices and VMs. You can also assign these custom fields to a site to allow all devices/VMs -in that site to be confured with the same proxy or proxy group. +in that site to be configured with the same proxy or proxy group. In order for this to work, `extended_site_properties` needs to be enabled in the configuation as well. @@ -696,8 +696,9 @@ proxy_group_cf = "zabbix_proxy_group" ``` As with config context proxy configuration, proxy group will take precedence over -proxy when configured. -Proxy settings configured on the device or VM will take precedence over any site configuration. +standalone proxy when configured. +Proxy settings configured on the device or VM will in their turn take precedence +over any site configuration. If the custom fields have no value but the proxy or proxy group is configured in config context, that setting will be used. From 7d9bb9f637563575efa003ce76aafb0287673c14 Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Fri, 12 Sep 2025 10:21:42 +0200 Subject: [PATCH 29/34] Refactoring --- modules/device.py | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/modules/device.py b/modules/device.py index ad99e9d..a636523 100644 --- a/modules/device.py +++ b/modules/device.py @@ -462,25 +462,16 @@ class PhysicalDevice: input: List of all proxies and proxy groups in standardized format """ - proxy_name = None - - #!!! FIX this check to still work if only CF is configurd. - - # check if the key Zabbix is defined in the config context - if "zabbix" not in self.nb.config_context: - return False - if ( - "proxy" in self.nb.config_context["zabbix"] - and not self.nb.config_context["zabbix"]["proxy"] - ): - return False # Proxy group takes priority over a proxy due # to it being HA and therefore being more reliable # Includes proxy group fix since Zabbix <= 6 should ignore this proxy_types = ["proxy"] + proxy_name = None if str(self.zabbix.version).startswith("7"): # Only insert groups in front of list for Zabbix7 proxy_types.insert(0, "proxy_group") + + # loop through supported proxy-types for proxy_type in proxy_types: # Check if we should use custom fields for proxy config field_config = "proxy_cf" if proxy_type=="proxy" else "proxy_group_cf" @@ -493,7 +484,8 @@ class PhysicalDevice: proxy_name = self.nb.site.custom_fields[config[field_config]] # Otherwise check if the proxy is configured in NetBox CC - if not proxy_name and proxy_type in self.nb.config_context["zabbix"]: + if (not proxy_name and "zabbix" in self.nb.config_context and + proxy_type in self.nb.config_context["zabbix"]): proxy_name = self.nb.config_context["zabbix"][proxy_type] # go through all proxies if proxy_name: From 123b243f5692ffc2cf7f93e7c755b34e151064ce Mon Sep 17 00:00:00 2001 From: Wouter de Bruijn Date: Fri, 12 Sep 2025 10:48:29 +0200 Subject: [PATCH 30/34] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Improved=20Zabbix=20?= =?UTF-8?q?version=20check=20for=20proxy=20group=20insertion?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- modules/device.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/modules/device.py b/modules/device.py index a636523..f1b63c7 100644 --- a/modules/device.py +++ b/modules/device.py @@ -7,6 +7,7 @@ from copy import deepcopy from logging import getLogger from re import search from operator import itemgetter +from typing import Any from zabbix_utils import APIRequestError from pynetbox import RequestError as NetboxRequestError @@ -454,7 +455,7 @@ class PhysicalDevice: self.tags = tags.generate() return True - def setProxy(self, proxy_list): + def _setProxy(self, proxy_list: list[dict[str, Any]]) -> bool: """ Sets proxy or proxy group if this value has been defined in config context @@ -467,7 +468,8 @@ class PhysicalDevice: # Includes proxy group fix since Zabbix <= 6 should ignore this proxy_types = ["proxy"] proxy_name = None - if str(self.zabbix.version).startswith("7"): + + if self.zabbix.version >= 7.0: # Only insert groups in front of list for Zabbix7 proxy_types.insert(0, "proxy_group") @@ -488,6 +490,7 @@ class PhysicalDevice: proxy_type in self.nb.config_context["zabbix"]): proxy_name = self.nb.config_context["zabbix"][proxy_type] # go through all proxies + if proxy_name: for proxy in proxy_list: # If the proxy does not match the type, ignore and continue @@ -500,6 +503,7 @@ class PhysicalDevice: ) self.zbxproxy = proxy return True + self.logger.warning( "Host %s: unable to find proxy %s", self.name, proxy_name ) @@ -532,7 +536,7 @@ class PhysicalDevice: # Set interface, group and template configuration interfaces = self.setInterfaceDetails() # Set Zabbix proxy if defined - self.setProxy(proxies) + self._setProxy(proxies) # Set basic data for host creation create_data = { "host": self.name, @@ -664,7 +668,7 @@ class PhysicalDevice: # Prepare templates and proxy config self.zbxTemplatePrepper(templates) - self.setProxy(proxies) + self._setProxy(proxies) # Get host object from Zabbix host = self.zabbix.host.get( filter={"hostid": self.zabbix_id}, From 422d343c1f52ac5d32f1fee6b4abc1ab714cfea2 Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Fri, 12 Sep 2025 14:11:38 +0200 Subject: [PATCH 31/34] * Added support for object and select custom fields in host groups and proxy config. * Corrected error when `full_proxy_sync` was not set and a host no longer uses a proxy. --- modules/device.py | 12 ++++++------ modules/hostgroups.py | 4 ++-- modules/tools.py | 15 ++++++++++++++- netbox_zabbix_sync.py | 4 ++-- 4 files changed, 24 insertions(+), 11 deletions(-) diff --git a/modules/device.py b/modules/device.py index f1b63c7..fa62ec0 100644 --- a/modules/device.py +++ b/modules/device.py @@ -21,7 +21,7 @@ from modules.exceptions import ( from modules.hostgroups import Hostgroup from modules.interface import ZabbixInterface from modules.tags import ZabbixTags -from modules.tools import field_mapper, remove_duplicates, sanatize_log_output +from modules.tools import field_mapper, cf_to_string, remove_duplicates, sanatize_log_output from modules.usermacros import ZabbixUsermacros from modules.config import load_config @@ -480,17 +480,17 @@ class PhysicalDevice: if config[field_config]: if config[field_config] in self.nb.custom_fields: if self.nb.custom_fields[config[field_config]]: - proxy_name = self.nb.custom_fields[config[field_config]] + proxy_name = cf_to_string(self.nb.custom_fields[config[field_config]]) elif config[field_config] in self.nb.site.custom_fields: if self.nb.site.custom_fields[config[field_config]]: - proxy_name = self.nb.site.custom_fields[config[field_config]] + proxy_name = cf_to_string(self.nb.site.custom_fields[config[field_config]]) # Otherwise check if the proxy is configured in NetBox CC if (not proxy_name and "zabbix" in self.nb.config_context and proxy_type in self.nb.config_context["zabbix"]): proxy_name = self.nb.config_context["zabbix"][proxy_type] - # go through all proxies - + + # If a proxy name was found, loop through all proxies to find a match if proxy_name: for proxy in proxy_list: # If the proxy does not match the type, ignore and continue @@ -804,7 +804,7 @@ class PhysicalDevice: # Display error message self.logger.warning( "Host %s: Is configured with proxy in Zabbix but not in NetBox." - "The -p flag was ommited: no changes have been made.", + "full_proxy_sync is not set: no changes have been made.", self.name, ) if not proxy_set: diff --git a/modules/hostgroups.py b/modules/hostgroups.py index 5916c0a..785e172 100644 --- a/modules/hostgroups.py +++ b/modules/hostgroups.py @@ -3,7 +3,7 @@ from logging import getLogger from modules.exceptions import HostgroupError -from modules.tools import build_path +from modules.tools import build_path, cf_to_string class Hostgroup: @@ -134,7 +134,7 @@ class Hostgroup: raise HostgroupError(msg) # CF data is populated if cf_data["cf"]: - hg_output.append(cf_data["cf"]) + hg_output.append(cf_to_string(cf_data["cf"])) continue # Check if there is a value associated to the variable. # For instance, if a device has no location, do not use it with hostgroup calculation diff --git a/modules/tools.py b/modules/tools.py index dacab20..efa6922 100644 --- a/modules/tools.py +++ b/modules/tools.py @@ -50,6 +50,19 @@ def proxy_prepper(proxy_list, proxy_group_list): return output +def cf_to_string(cf, key="name", logger=None): + """ + Converts a dict custom fields to string + """ + if isinstance(cf, dict): + if key: + return cf[key] + else: + logger.error("Conversion of custom field failed, '%s' not found in cf dict.", key) + return None + return cf + + def field_mapper(host, mapper, nbdevice, logger): """ Maps NetBox field data to Zabbix properties. @@ -204,4 +217,4 @@ def sanatize_log_output(data): continue # A macro is not used, so we sanitize the value. sanitized_data["details"][key] = "********" - return sanitized_data + return sanitized_data \ No newline at end of file diff --git a/netbox_zabbix_sync.py b/netbox_zabbix_sync.py index 3a03f66..08e507f 100755 --- a/netbox_zabbix_sync.py +++ b/netbox_zabbix_sync.py @@ -82,7 +82,7 @@ def main(arguments): device_cfs = [] vm_cfs = [] device_cfs = list( - netbox.extras.custom_fields.filter(type="text", content_types="dcim.device") + netbox.extras.custom_fields.filter(type=["text","object","select"], content_types="dcim.device") ) verify_hg_format( config["hostgroup_format"], device_cfs=device_cfs, hg_type="dev", logger=logger @@ -90,7 +90,7 @@ def main(arguments): if config["sync_vms"]: vm_cfs = list( netbox.extras.custom_fields.filter( - type="text", content_types="virtualization.virtualmachine" + type=["text","object","select"], content_types="virtualization.virtualmachine" ) ) verify_hg_format( From bc12064b6a60a9b322677acdcf3480fbedddaf7c Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Fri, 12 Sep 2025 14:27:06 +0200 Subject: [PATCH 32/34] corrected linting error --- netbox_zabbix_sync.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/netbox_zabbix_sync.py b/netbox_zabbix_sync.py index 08e507f..c2e0865 100755 --- a/netbox_zabbix_sync.py +++ b/netbox_zabbix_sync.py @@ -82,7 +82,8 @@ def main(arguments): device_cfs = [] vm_cfs = [] device_cfs = list( - netbox.extras.custom_fields.filter(type=["text","object","select"], content_types="dcim.device") + netbox.extras.custom_fields.filter(type=["text","object","select"], + content_types="dcim.device") ) verify_hg_format( config["hostgroup_format"], device_cfs=device_cfs, hg_type="dev", logger=logger @@ -90,7 +91,8 @@ def main(arguments): if config["sync_vms"]: vm_cfs = list( netbox.extras.custom_fields.filter( - type=["text","object","select"], content_types="virtualization.virtualmachine" + type=["text","object","select"], + content_types="virtualization.virtualmachine" ) ) verify_hg_format( From c27505b9272cac8402a266e3dc6b0a5baf2c8532 Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Fri, 12 Sep 2025 14:39:11 +0200 Subject: [PATCH 33/34] corrected linting errors and a minor bug in cf_to_string --- modules/device.py | 14 +++++++------- modules/tools.py | 9 ++++----- netbox_zabbix_sync.py | 4 ++-- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/modules/device.py b/modules/device.py index fa62ec0..3a68afc 100644 --- a/modules/device.py +++ b/modules/device.py @@ -468,25 +468,25 @@ class PhysicalDevice: # Includes proxy group fix since Zabbix <= 6 should ignore this proxy_types = ["proxy"] proxy_name = None - + if self.zabbix.version >= 7.0: # Only insert groups in front of list for Zabbix7 proxy_types.insert(0, "proxy_group") - # loop through supported proxy-types + # loop through supported proxy-types for proxy_type in proxy_types: # Check if we should use custom fields for proxy config - field_config = "proxy_cf" if proxy_type=="proxy" else "proxy_group_cf" - if config[field_config]: + field_config = "proxy_cf" if proxy_type=="proxy" else "proxy_group_cf" + if config[field_config]: if config[field_config] in self.nb.custom_fields: if self.nb.custom_fields[config[field_config]]: proxy_name = cf_to_string(self.nb.custom_fields[config[field_config]]) elif config[field_config] in self.nb.site.custom_fields: if self.nb.site.custom_fields[config[field_config]]: - proxy_name = cf_to_string(self.nb.site.custom_fields[config[field_config]]) + proxy_name = cf_to_string(self.nb.site.custom_fields[config[field_config]]) # Otherwise check if the proxy is configured in NetBox CC - if (not proxy_name and "zabbix" in self.nb.config_context and + if (not proxy_name and "zabbix" in self.nb.config_context and proxy_type in self.nb.config_context["zabbix"]): proxy_name = self.nb.config_context["zabbix"][proxy_type] @@ -503,7 +503,7 @@ class PhysicalDevice: ) self.zbxproxy = proxy return True - + self.logger.warning( "Host %s: unable to find proxy %s", self.name, proxy_name ) diff --git a/modules/tools.py b/modules/tools.py index efa6922..023cef8 100644 --- a/modules/tools.py +++ b/modules/tools.py @@ -55,11 +55,10 @@ def cf_to_string(cf, key="name", logger=None): Converts a dict custom fields to string """ if isinstance(cf, dict): - if key: + if key in cf: return cf[key] - else: - logger.error("Conversion of custom field failed, '%s' not found in cf dict.", key) - return None + logger.error("Conversion of custom field failed, '%s' not found in cf dict.", key) + return None return cf @@ -217,4 +216,4 @@ def sanatize_log_output(data): continue # A macro is not used, so we sanitize the value. sanitized_data["details"][key] = "********" - return sanitized_data \ No newline at end of file + return sanitized_data diff --git a/netbox_zabbix_sync.py b/netbox_zabbix_sync.py index c2e0865..4ee2ba0 100755 --- a/netbox_zabbix_sync.py +++ b/netbox_zabbix_sync.py @@ -82,7 +82,7 @@ def main(arguments): device_cfs = [] vm_cfs = [] device_cfs = list( - netbox.extras.custom_fields.filter(type=["text","object","select"], + netbox.extras.custom_fields.filter(type=["text","object","select"], content_types="dcim.device") ) verify_hg_format( @@ -91,7 +91,7 @@ def main(arguments): if config["sync_vms"]: vm_cfs = list( netbox.extras.custom_fields.filter( - type=["text","object","select"], + type=["text","object","select"], content_types="virtualization.virtualmachine" ) ) From 37774cfec36a6f34b6791a3df5dbdfb4deba3637 Mon Sep 17 00:00:00 2001 From: Raymond Kuiper Date: Fri, 12 Sep 2025 14:40:53 +0200 Subject: [PATCH 34/34] More linting fixes --- modules/device.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/device.py b/modules/device.py index 3a68afc..13e74aa 100644 --- a/modules/device.py +++ b/modules/device.py @@ -468,7 +468,7 @@ class PhysicalDevice: # Includes proxy group fix since Zabbix <= 6 should ignore this proxy_types = ["proxy"] proxy_name = None - + if self.zabbix.version >= 7.0: # Only insert groups in front of list for Zabbix7 proxy_types.insert(0, "proxy_group")