From 957cdab92009f19fce2855ca186057b4b9ebeda7 Mon Sep 17 00:00:00 2001 From: Thomas Foks Date: Mon, 12 Oct 2026 09:50:34 +0200 Subject: [PATCH 1/2] fix(VMware): resolve cluster_scope_id_relation to the scoped object The scope id from cluster_scope_id_relation was passed to the cluster as the configured string. NBCluster only accepts a Site, SiteGroup, Location or Region object as scope_id, so every cluster logged "Invalid data type for 'scope_id'" and was left with a scope_type and no scope. Look up the object of the configured scope type by its NetBox id. Log an error and leave the scope unset if the type is unsupported, the id is not a number or no such object exists in NetBox. Co-Authored-By: Claude Opus 5.5 --- module/sources/vmware/connection.py | 24 ++++++++++++++++---- tests/test_cluster_scope.py | 34 ++++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/module/sources/vmware/connection.py b/module/sources/vmware/connection.py index 6c5d255..e2bb94d 100644 --- a/module/sources/vmware/connection.py +++ b/module/sources/vmware/connection.py @@ -34,6 +34,7 @@ from module.common.support import normalize_mac_address from module.netbox.inventory import NetBoxInventory from module.netbox import * +from module.netbox.object_classes import NetBoxMappings vsphere_automation_sdk_available = True try: @@ -2441,10 +2442,25 @@ def add_cluster(self, obj): if version.parse(self.inventory.netbox_api_version) >= version.parse("4.2.0"): # set the scope type and id if they are defined if scope_type is not None: - data["scope_type"] = scope_type - data["scope_id"] = scope_id - log.debug(f"Cluster '{full_cluster_name}' (or {name}) has scope type '{scope_type}' " - f"and scope id '{scope_id}'.") + # the configured scope id is a NetBox id, the cluster needs the object it points to + scope_object = None + scope_class = NetBoxMappings().get(scope_type) + if scope_class is None: + log.error(f"Unsupported scope type '{scope_type}' for cluster '{full_cluster_name}'") + elif scope_id is None or not str(scope_id).strip().isdigit(): + log.error(f"Invalid scope id '{scope_id}' for cluster '{full_cluster_name}', " + f"must be the NetBox id of a {scope_class.name}") + else: + scope_object = self.inventory.get_by_id(scope_class, nb_id=int(scope_id)) + if scope_object is None: + log.error(f"Unable to find {scope_class.name} with id '{scope_id}' in NetBox " + f"for cluster '{full_cluster_name}'") + + if scope_object is not None: + data["scope_type"] = scope_type + data["scope_id"] = scope_object + log.debug(f"Cluster '{full_cluster_name}' (or {name}) has scope type '{scope_type}' " + f"and scope id '{scope_id}'.") elif site_name is not None: # NetBox wants the id of the scoped object, so the site has to be a real # object here. A plain dict is sent as is and rejected with diff --git a/tests/test_cluster_scope.py b/tests/test_cluster_scope.py index 3bf9b32..053231f 100644 --- a/tests/test_cluster_scope.py +++ b/tests/test_cluster_scope.py @@ -12,7 +12,7 @@ id of the scoped object), and a scope read back from NetBox as an id has to resolve to that object again, otherwise every run reports a change. """ -from module.netbox.object_classes import NBCluster, NBClusterType, NBSite +from module.netbox.object_classes import NBCluster, NBClusterType, NBSite, NBSiteGroup def test_cluster_scope_is_resolved_from_an_id(inventory): @@ -58,3 +58,35 @@ def test_source_scopes_a_cluster_with_the_site_object(vcsim, inventory, load_con scope = cluster.data.get("scope_id") assert isinstance(scope, NBSite), f"scope_id is {type(scope).__name__}, NetBox needs an object it can turn into an id" assert scope.get_display_name() == "site1" + + +def sync_with_scope_relation(inventory, load_config, vmware_settings, scope_id): + from module.sources import instantiate_sources + + load_config(vmware_settings + "cluster_scope_type_relation = .* = dcim.sitegroup\n" + f"cluster_scope_id_relation = .* = {scope_id}\n") + source = instantiate_sources()[0] + assert source.init_successful + inventory.resolve_relations() + source.apply() + + clusters = list(inventory.get_all_items(NBCluster)) + assert clusters, "no cluster was synced" + return clusters + + +def test_source_scopes_a_cluster_with_the_configured_scope_id(vcsim, inventory, load_config, vmware_settings): + """The configured scope id is a NetBox id, the cluster has to carry the object it points to""" + site_group = inventory.add_object(NBSiteGroup, data={"name": "group1"}, read_from_netbox=True) + site_group.nb_id = 3 + + for cluster in sync_with_scope_relation(inventory, load_config, vmware_settings, 3): + assert cluster.data.get("scope_type") == "dcim.sitegroup" + assert cluster.data.get("scope_id") is site_group, \ + f"scope_id is {cluster.data.get('scope_id')!r}, the configured id was not resolved" + + +def test_source_skips_a_scope_id_missing_in_netbox(vcsim, inventory, load_config, vmware_settings): + for cluster in sync_with_scope_relation(inventory, load_config, vmware_settings, 99): + assert cluster.data.get("scope_id") is None + assert cluster.data.get("scope_type") is None, "scope type was set without a scope" From 98869627e2d686120c3a0d2d9e1a0000148efe7a Mon Sep 17 00:00:00 2001 From: Thomas Foks Date: Mon, 12 Oct 2026 12:49:35 +0200 Subject: [PATCH 2/2] fix(VMware): accept only cluster scope types and decimal scope ids MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up: - check the configured scope type against the four cluster scope types. Any other netbox-sync type (i.e. dcim.device) passed the check before and ended in two "Invalid data type" errors. - use str.isdecimal(), str.isdigit() accepts characters like '³' which int() rejects, which stopped the whole run. - add tests for a non-numeric id, a superscript digit and an unsupported type. - note in both option descriptions that cluster scopes need NetBox >= 4.2. Co-Authored-By: Claude Opus 5.5 --- module/sources/vmware/config.py | 4 +++ module/sources/vmware/connection.py | 8 +++--- settings-example.ini | 8 +++--- tests/test_cluster_scope.py | 42 ++++++++++++++++++++++++++--- 4 files changed, 52 insertions(+), 10 deletions(-) diff --git a/module/sources/vmware/config.py b/module/sources/vmware/config.py index 771eef4..3a768e5 100644 --- a/module/sources/vmware/config.py +++ b/module/sources/vmware/config.py @@ -178,6 +178,8 @@ def __init__(self): This is done with a comma separated key = value list. Can be set to "" to not assign a scope type. Note: this does not remove scope types from existing clusters in NetBox. + Note: cluster scopes require NetBox 4.2 or newer, the option is ignored + with older versions. key: defines a cluster name as regex value: defines the NetBox scope type name (use quotes if name contains commas) """, @@ -188,6 +190,8 @@ def __init__(self): The scope id is the NetBox ID of the scope type. This is done with a comma separated key = value list. To be used in combination with the 'cluster_scope_type_relation'. + Note: cluster scopes require NetBox 4.2 or newer, the option is ignored + with older versions. key: defines a cluster name as regex value: defines the NetBox scope id (use quotes if name contains commas) """, diff --git a/module/sources/vmware/connection.py b/module/sources/vmware/connection.py index e2bb94d..49a01d0 100644 --- a/module/sources/vmware/connection.py +++ b/module/sources/vmware/connection.py @@ -2444,10 +2444,12 @@ def add_cluster(self, obj): if scope_type is not None: # the configured scope id is a NetBox id, the cluster needs the object it points to scope_object = None + scope_classes = [NBSite, NBSiteGroup, NBLocation, NBRegion] scope_class = NetBoxMappings().get(scope_type) - if scope_class is None: - log.error(f"Unsupported scope type '{scope_type}' for cluster '{full_cluster_name}'") - elif scope_id is None or not str(scope_id).strip().isdigit(): + if scope_class not in scope_classes: + log.error(f"Unsupported scope type '{scope_type}' for cluster '{full_cluster_name}', must be " + f"one of: {', '.join(NetBoxMappings().scopes_object_types(scope_classes))}") + elif scope_id is None or not str(scope_id).strip().isdecimal(): log.error(f"Invalid scope id '{scope_id}' for cluster '{full_cluster_name}', " f"must be the NetBox id of a {scope_class.name}") else: diff --git a/settings-example.ini b/settings-example.ini index d8640c2..49992e6 100644 --- a/settings-example.ini +++ b/settings-example.ini @@ -241,14 +241,16 @@ password = super-secret ; This option defines the scope type for a cluster. The scope type can be 'dcim.site', ; 'dcim.sitegroup', 'dcim.location' or 'dcim.region'. This is done with a comma separated ; key = value list. Can be set to "" to not assign a scope type. Note: this does not -; remove scope types from existing clusters in NetBox. key: defines a cluster name as +; remove scope types from existing clusters in NetBox. Note: cluster scopes require NetBox +; 4.2 or newer, the option is ignored with older versions. key: defines a cluster name as ; regex value: defines the NetBox scope type name (use quotes if name contains commas) ;cluster_scope_type_relation = Cluster_NYC = dcim.site, Cluster_FFM = dcim.sitegroup, Cluster_BER = dcim.location ; This option defines the scope id for a cluster. The scope id is the NetBox ID of the ; scope type. This is done with a comma separated key = value list. To be used in -; combination with the 'cluster_scope_type_relation'. key: defines a cluster name as regex -; value: defines the NetBox scope id (use quotes if name contains commas) +; combination with the 'cluster_scope_type_relation'. Note: cluster scopes require NetBox +; 4.2 or newer, the option is ignored with older versions. key: defines a cluster name as +; regex value: defines the NetBox scope id (use quotes if name contains commas) ;cluster_scope_id_relation = Cluster_NYC = 1, Cluster_FFM.* = 2, Cluster_BER = 7 ; This option defines which cluster/host/VM belongs to which tenant. diff --git a/tests/test_cluster_scope.py b/tests/test_cluster_scope.py index 053231f..a43eb4a 100644 --- a/tests/test_cluster_scope.py +++ b/tests/test_cluster_scope.py @@ -12,6 +12,10 @@ id of the scoped object), and a scope read back from NetBox as an id has to resolve to that object again, otherwise every run reports a change. """ +import logging + +import pytest + from module.netbox.object_classes import NBCluster, NBClusterType, NBSite, NBSiteGroup @@ -60,10 +64,11 @@ def test_source_scopes_a_cluster_with_the_site_object(vcsim, inventory, load_con assert scope.get_display_name() == "site1" -def sync_with_scope_relation(inventory, load_config, vmware_settings, scope_id): + +def sync_with_scope_relation(inventory, load_config, vmware_settings, scope_id, scope_type="dcim.sitegroup"): from module.sources import instantiate_sources - load_config(vmware_settings + "cluster_scope_type_relation = .* = dcim.sitegroup\n" + load_config(vmware_settings + f"cluster_scope_type_relation = .* = {scope_type}\n" f"cluster_scope_id_relation = .* = {scope_id}\n") source = instantiate_sources()[0] assert source.init_successful @@ -75,6 +80,12 @@ def sync_with_scope_relation(inventory, load_config, vmware_settings, scope_id): return clusters +def assert_only_errors(caplog, expected): + errors = [r.getMessage() for r in caplog.records if r.levelno >= logging.ERROR] + assert errors, f"no error logged, expected '{expected}'" + assert all(expected in message for message in errors), errors + + def test_source_scopes_a_cluster_with_the_configured_scope_id(vcsim, inventory, load_config, vmware_settings): """The configured scope id is a NetBox id, the cluster has to carry the object it points to""" site_group = inventory.add_object(NBSiteGroup, data={"name": "group1"}, read_from_netbox=True) @@ -86,7 +97,30 @@ def test_source_scopes_a_cluster_with_the_configured_scope_id(vcsim, inventory, f"scope_id is {cluster.data.get('scope_id')!r}, the configured id was not resolved" -def test_source_skips_a_scope_id_missing_in_netbox(vcsim, inventory, load_config, vmware_settings): - for cluster in sync_with_scope_relation(inventory, load_config, vmware_settings, 99): +@pytest.mark.parametrize("scope_id, error", [ + ("99", "Unable to find site group with id '99'"), + ("group1", "Invalid scope id 'group1'"), + # str.isdigit() accepts it, int() does not + ("\u00b3", "Invalid scope id '\u00b3'"), +], ids=["missing", "name", "superscript"]) +def test_source_skips_an_invalid_scope_id(vcsim, inventory, load_config, vmware_settings, caplog, scope_id, error): + site_group = inventory.add_object(NBSiteGroup, data={"name": "group1"}, read_from_netbox=True) + site_group.nb_id = 3 + + clusters = sync_with_scope_relation(inventory, load_config, vmware_settings, scope_id) + + for cluster in clusters: assert cluster.data.get("scope_id") is None assert cluster.data.get("scope_type") is None, "scope type was set without a scope" + assert_only_errors(caplog, error) + + +def test_source_skips_an_unsupported_scope_type(vcsim, inventory, load_config, vmware_settings, caplog): + """netbox-sync knows 'dcim.device', but a cluster can't be scoped to it""" + clusters = sync_with_scope_relation(inventory, load_config, vmware_settings, 3, scope_type="dcim.device") + + for cluster in clusters: + assert cluster.data.get("scope_id") is None + assert cluster.data.get("scope_type") is None + # without the check it ends in two 'Invalid data type' errors per cluster + assert_only_errors(caplog, "Unsupported scope type 'dcim.device'")