From 551f786fe403f8060a53ef96370f7486c45f96ee Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Fri, 21 Aug 2026 14:03:43 +0200 Subject: [PATCH] fix(graphql): fall back to client-side vendor scoping when NetBox rejects template filters NetBox 4.3.0 gave the GraphQL PowerOutletTemplateFilter the component mixin (device/device_id) instead of the template mixin (device_type/module_type), so vendor-scoped power_outlet_template_list queries are rejected by the schema on every released 4.3.x and 4.4.x. NetBox fixed the filter in netbox-community/netbox#20935, first shipped in v4.5.0, with no backport to 4.4.x. When a vendor-scoped component template query raises GraphQLSchemaError, get_component_templates now fetches the endpoint unfiltered once and scopes the records client-side against the vendor's device type and module type ids (string-normalized on both sides). Transport errors keep the existing retry and propagation behavior; only a schema rejection triggers the fallback, so the slow path cannot hide a flaky server. An asymmetric rejection (device leg accepted, module leg rejected) falls back for the module leg alone without fetching device type ids. Fixes #120 --- core/graphql_client.py | 85 ++++++++++++++---- tests/test_graphql_client.py | 163 +++++++++++++++++++++++++++++++++++ 2 files changed, 230 insertions(+), 18 deletions(-) diff --git a/core/graphql_client.py b/core/graphql_client.py index 62ef3086..953d4d59 100644 --- a/core/graphql_client.py +++ b/core/graphql_client.py @@ -792,6 +792,45 @@ def get_component_templates(self, endpoint_name, manufacturer_slug=None, on_page if component.module_types: parent_fields += "\n module_type { id }" + def _unfiltered_vendor_items(include_device_parent, include_module_parent): + all_items = self._query_component_endpoint( + list_key=list_key, + filter_clause="", + endpoint_name=endpoint_name, + fields=fields, + parent_fields=parent_fields, + on_page=on_page, + ) + + device_type_ids = set() + if include_device_parent: + device_types, _ = self.get_device_types(manufacturer_slugs=[manufacturer_slug]) + device_type_ids = {str(record.id) for record in device_types.values()} + + module_type_ids = set() + if include_module_parent: + module_types = self.get_module_types(manufacturer_slugs=[manufacturer_slug]) + module_type_ids = { + str(record.id) + for manufacturer_types in module_types.values() + for record in manufacturer_types.values() + } + + return [ + item + for item in all_items + if ( + include_device_parent + and item.get("device_type") + and str(item["device_type"].get("id")) in device_type_ids + ) + or ( + include_module_parent + and item.get("module_type") + and str(item["module_type"].get("id")) in module_type_ids + ) + ] + if manufacturer_slug is None: # Unfiltered query (original behavior) items = self._query_component_endpoint( @@ -807,23 +846,10 @@ def get_component_templates(self, endpoint_name, manufacturer_slug=None, on_page extra_vars = {"manufacturer_slug": manufacturer_slug} # Vendor-scoped: query device-type-filtered templates device_filter = "filters: {device_type: {manufacturer: {slug: {exact: $manufacturer_slug}}}}, " - device_items = self._query_component_endpoint( - list_key=list_key, - filter_clause=device_filter, - endpoint_name=endpoint_name, - fields=fields, - parent_fields=parent_fields, - on_page=on_page, - var_decl=var_decl, - extra_variables=extra_vars, - ) - - # If endpoint supports module_type, also query module-type-filtered templates - if component.module_types: - module_filter = "filters: {module_type: {manufacturer: {slug: {exact: $manufacturer_slug}}}}, " - module_items = self._query_component_endpoint( + try: + device_items = self._query_component_endpoint( list_key=list_key, - filter_clause=module_filter, + filter_clause=device_filter, endpoint_name=endpoint_name, fields=fields, parent_fields=parent_fields, @@ -831,8 +857,31 @@ def get_component_templates(self, endpoint_name, manufacturer_slug=None, on_page var_decl=var_decl, extra_variables=extra_vars, ) - items = device_items + module_items + except GraphQLSchemaError: + # Only a schema rejection may trigger this expensive unfiltered scan. + # It replaces both filtered legs because each template has one parent. + items = _unfiltered_vendor_items(True, component.module_types) else: - items = device_items + # If endpoint supports module_type, also query module-type-filtered templates + if component.module_types: + module_filter = "filters: {module_type: {manufacturer: {slug: {exact: $manufacturer_slug}}}}, " + try: + module_items = self._query_component_endpoint( + list_key=list_key, + filter_clause=module_filter, + endpoint_name=endpoint_name, + fields=fields, + parent_fields=parent_fields, + on_page=on_page, + var_decl=var_decl, + extra_variables=extra_vars, + ) + except GraphQLSchemaError: + # Only a schema rejection may trigger this expensive unfiltered scan. + # Keep only module parents because the device leg already succeeded. + module_items = _unfiltered_vendor_items(False, True) + items = device_items + module_items + else: + items = device_items return [_to_dotdict(item) for item in items] diff --git a/tests/test_graphql_client.py b/tests/test_graphql_client.py index 00e2b2dc..39d9376c 100644 --- a/tests/test_graphql_client.py +++ b/tests/test_graphql_client.py @@ -1980,6 +1980,108 @@ def _make_client(self): return NetBoxGraphQLClient("http://netbox.local", "tok") + @staticmethod + def _response(body): + response = MagicMock() + response.status_code = 200 + response.raise_for_status = MagicMock() + response.json.return_value = body + return response + + def _netbox_44_power_outlet_transport(self, reject_device_filter=True): + requests_seen = [] + component_records = [ + { + "id": "1", + "name": "Target device outlet", + "device_type": {"id": "101"}, + "module_type": None, + }, + { + "id": "2", + "name": "Target module outlet", + "device_type": None, + "module_type": {"id": "201"}, + }, + { + "id": "3", + "name": "Foreign device outlet", + "device_type": {"id": "102"}, + "module_type": None, + }, + { + "id": "4", + "name": "Foreign module outlet", + "device_type": None, + "module_type": {"id": "202"}, + }, + ] + device_types = [ + { + "id": 101, + "model": "Target device", + "slug": "target-device", + "manufacturer": {"id": 1, "name": "Acme", "slug": "acme"}, + } + ] + module_types = [ + { + "id": 201, + "model": "Target module", + "manufacturer": {"id": 1, "name": "Acme", "slug": "acme"}, + } + ] + + def post(_url, *, json, timeout): + query = json["query"] + variables = json["variables"] + offset = variables["pagination"]["offset"] + requests_seen.append((query, variables)) + + if "power_outlet_template_list" in query and "filters: {device_type:" in query: + if reject_device_filter: + return self._response( + { + "errors": [ + { + "message": ( + "Field 'device_type' is not defined by type " + "'PowerOutletTemplateFilter'. Did you mean 'device_id' or 'device'?" + ) + } + ] + } + ) + page = [component_records[0]] if offset == 0 else [] + return self._response({"data": {"power_outlet_template_list": page}}) + if "power_outlet_template_list" in query and "filters: {module_type:" in query: + return self._response( + { + "errors": [ + { + "message": ( + "Field 'module_type' is not defined by type " + "'PowerOutletTemplateFilter'. Did you mean 'module_id' or 'module'?" + ) + } + ] + } + ) + if "power_outlet_template_list" in query: + page = component_records if offset == 0 else [] + return self._response({"data": {"power_outlet_template_list": page}}) + if "device_type_list" in query: + assert variables["manufacturer_slug"] == "acme" + page = device_types if offset == 0 else [] + return self._response({"data": {"device_type_list": page}}) + if "module_type_list" in query: + assert variables["manufacturer_slug"] == "acme" + page = module_types if offset == 0 else [] + return self._response({"data": {"module_type_list": page}}) + raise AssertionError(f"Unexpected GraphQL query: {query}") + + return post, requests_seen + def test_unfiltered_query(self, mock_post): """Test that manufacturer_slug=None produces unfiltered behavior.""" data = { @@ -2058,6 +2160,67 @@ def test_vendor_scoped_two_queries(self, mock_post): assert "filters: {module_type: {manufacturer: {slug: {exact: $manufacturer_slug}}}}" in module_query assert module_vars["manufacturer_slug"] == "cisco" + def test_vendor_scope_falls_back_to_one_unfiltered_scan_for_rejected_filters(self, mock_post): + """A NetBox 4.4 filter rejection falls back to one client-scoped endpoint scan.""" + post, requests_seen = self._netbox_44_power_outlet_transport() + mock_post.side_effect = post + + client = self._make_client() + result = client.get_component_templates("power_outlet_templates", manufacturer_slug="acme") + + assert {record.name for record in result} == {"Target device outlet", "Target module outlet"} + component_requests = [ + (query, variables) for query, variables in requests_seen if "power_outlet_template_list" in query + ] + assert sum("filters: {device_type:" in query for query, _variables in component_requests) == 1 + assert not any("filters: {module_type:" in query for query, _variables in component_requests) + unfiltered_offsets = [ + variables["pagination"]["offset"] for query, variables in component_requests if "filters:" not in query + ] + assert unfiltered_offsets == [0, 4] + + def test_module_leg_schema_rejection_falls_back_without_device_scan(self, mock_post): + """A module-leg-only rejection scans unfiltered for module parents alone.""" + post, requests_seen = self._netbox_44_power_outlet_transport(reject_device_filter=False) + mock_post.side_effect = post + + client = self._make_client() + result = client.get_component_templates("power_outlet_templates", manufacturer_slug="acme") + + assert sorted(record.name for record in result) == ["Target device outlet", "Target module outlet"] + queries = [query for query, _variables in requests_seen] + assert not any("device_type_list" in query for query in queries) + component_requests = [ + (query, variables) for query, variables in requests_seen if "power_outlet_template_list" in query + ] + device_leg_offsets = [ + variables["pagination"]["offset"] + for query, variables in component_requests + if "filters: {device_type:" in query + ] + assert device_leg_offsets == [0, 1] + assert sum("filters: {module_type:" in query for query, _variables in component_requests) == 1 + unfiltered_offsets = [ + variables["pagination"]["offset"] for query, variables in component_requests if "filters:" not in query + ] + assert unfiltered_offsets == [0, 4] + + @pytest.mark.usefixtures("no_retry_backoff") + def test_transport_failure_does_not_trigger_vendor_scope_fallback(self, mock_post): + """An exhausted connection retry remains a transport error without an unfiltered scan.""" + from core.graphql_client import GraphQLError, GraphQLSchemaError + + mock_post.side_effect = requests.exceptions.ConnectionError("connection refused") + + client = self._make_client() + with pytest.raises(GraphQLError, match="connection refused") as excinfo: + client.get_component_templates("power_outlet_templates", manufacturer_slug="acme") + + assert not isinstance(excinfo.value, GraphQLSchemaError) + assert mock_post.call_count == 4 + queries = [call.kwargs["json"]["query"] for call in mock_post.call_args_list] + assert all("filters: {device_type:" in query for query in queries) + def test_vendor_scoped_device_bay_single_query(self, mock_post): """Test that device_bay_templates makes only one query (no module_type field).""" device_data = {