From 687de40fb2c9f9e4419413cd18a1ce4b9edbdca9 Mon Sep 17 00:00:00 2001 From: Sola Babatunde Date: Tue, 12 May 2026 11:07:28 +0100 Subject: [PATCH 1/2] IHS-119: Fix upsert not working for numberpool and attribute in hfid --- changelog/339.fixed.md | 1 + .../python-sdk/guides/resource-manager.mdx | 56 ++++++++++++ infrahub_sdk/node/node.py | 44 +++++++++- tests/unit/sdk/pool/conftest.py | 20 +++++ .../unit/sdk/pool/test_attribute_from_pool.py | 85 +++++++++++++++++++ 5 files changed, 205 insertions(+), 1 deletion(-) create mode 100644 changelog/339.fixed.md diff --git a/changelog/339.fixed.md b/changelog/339.fixed.md new file mode 100644 index 00000000..4543386c --- /dev/null +++ b/changelog/339.fixed.md @@ -0,0 +1 @@ +Calling `.save(allow_upsert=True)` on a node whose human-friendly identifier contains a CoreNumberPool-sourced attribute now raises a clear `ValidationError` instead of crashing with an opaque backend error. diff --git a/docs/docs/python-sdk/guides/resource-manager.mdx b/docs/docs/python-sdk/guides/resource-manager.mdx index 97cb4b90..64e3b43e 100644 --- a/docs/docs/python-sdk/guides/resource-manager.mdx +++ b/docs/docs/python-sdk/guides/resource-manager.mdx @@ -309,3 +309,59 @@ query { ``` Notice that we have one IP address allocated by the Resource manager in the test branch. The query in the main branch shows us this allocation, indicating that it has been allocated and the resource cannot be allocated again. However, the IP address does not exist itself within the main branch. + +## CoreNumberPool and attribute allocation + +`CoreNumberPool` allocates integer values (such as VLAN IDs or AS numbers) directly to node attributes. The pool assigns the integer value at the moment the node is created on the server. + +```python +vlan = await client.create( + kind="InfraVLAN", + name="VLAN-100", + vlan_id={"from_pool": {"id": pool_id}}, +) +await vlan.save() +``` + +### Limitation: `allow_upsert=True` with a pool-sourced HFID attribute + +`CoreNumberPool` assigns the integer value at server creation time. The SDK does not know the assigned value before the node exists. When a node's human-friendly identifier (HFID) includes a pool-sourced attribute, the SDK cannot construct the HFID needed to look up an existing node for upsert. + +:::warning + +Calling `save(allow_upsert=True)` on a node whose HFID contains a `CoreNumberPool`-sourced attribute raises `ValidationError` before any network call is made. + +```python +# Schema has human_friendly_id: ["vlan_id__value"] +vlan = await client.create( + kind="InfraVLAN", + name="VLAN-100", + vlan_id={"from_pool": {"id": pool_id}}, +) + +# This raises ValidationError — the pool-assigned vlan_id is unknown client-side +await vlan.save(allow_upsert=True) +``` + +**Alternatives:** + +- **Two-step pattern** — create the node first, then update it in a separate call: + + ```python + await vlan.save() # creates node, pool assigns vlan_id + # later, if you need to update: + vlan.name.value = "VLAN-100-updated" + await vlan.save() # now _existing=True, calls update + ``` + +- **Explicit id** — if you already know the node's UUID, set `node.id` before saving. The upsert will use the UUID directly and skip HFID lookup: + + ```python + vlan.id = "known-uuid" + await vlan.save(allow_upsert=True) # guard bypassed + ``` + +- **Deterministic identifier** — if possible, design your schema so the HFID uses a non-pool attribute (for example, a human-assigned `name`) and keep `vlan_id` out of the HFID. + +::: + diff --git a/infrahub_sdk/node/node.py b/infrahub_sdk/node/node.py index 6155fd2f..5ee35598 100644 --- a/infrahub_sdk/node/node.py +++ b/infrahub_sdk/node/node.py @@ -7,7 +7,13 @@ from typing import TYPE_CHECKING, Any, BinaryIO, overload from ..constants import InfrahubClientMode -from ..exceptions import FeatureNotSupportedError, NodeNotFoundError, ResourceNotDefinedError, SchemaNotFoundError +from ..exceptions import ( + FeatureNotSupportedError, + NodeNotFoundError, + ResourceNotDefinedError, + SchemaNotFoundError, + ValidationError, +) from ..file_handler import FileHandler, FileHandlerBase, FileHandlerSync, PreparedFile, sha1_of_source from ..graphql import Mutation, Query from ..schema import ( @@ -973,6 +979,24 @@ async def save( timeout: int | None = None, request_context: RequestContext | None = None, ) -> None: + if allow_upsert and not self.id: + for hfid_path in self._schema.human_friendly_id or []: + attr_name = hfid_path.split("__")[0] + try: + attr = self._get_attribute(attr_name) + except ResourceNotDefinedError: + continue + if attr.is_from_pool_attribute(): + raise ValidationError( + identifier=attr_name, + message=( + f"Attribute '{attr_name}' is sourced from a CoreNumberPool and is part of " + "this node's human-friendly identifier. Upsert cannot resolve the HFID " + "without a concrete value. Use an explicit id, or create the node first " + "and update it in a separate call." + ), + ) + if self._existing is False or allow_upsert is True: await self.create(allow_upsert=allow_upsert, timeout=timeout, request_context=request_context) else: @@ -1944,6 +1968,24 @@ def save( timeout: int | None = None, request_context: RequestContext | None = None, ) -> None: + if allow_upsert and not self.id: + for hfid_path in self._schema.human_friendly_id or []: + attr_name = hfid_path.split("__")[0] + try: + attr = self._get_attribute(attr_name) + except ResourceNotDefinedError: + continue + if attr.is_from_pool_attribute(): + raise ValidationError( + identifier=attr_name, + message=( + f"Attribute '{attr_name}' is sourced from a CoreNumberPool and is part of " + "this node's human-friendly identifier. Upsert cannot resolve the HFID " + "without a concrete value. Use an explicit id, or create the node first " + "and update it in a separate call." + ), + ) + if self._existing is False or allow_upsert is True: self.create(allow_upsert=allow_upsert, timeout=timeout, request_context=request_context) else: diff --git a/tests/unit/sdk/pool/conftest.py b/tests/unit/sdk/pool/conftest.py index 9d0fd245..43ab113c 100644 --- a/tests/unit/sdk/pool/conftest.py +++ b/tests/unit/sdk/pool/conftest.py @@ -114,3 +114,23 @@ async def ipprefix_pool_schema() -> NodeSchemaAPI: ], } return NodeSchema(**data).convert_api() + + +@pytest.fixture +async def vlan_schema_with_pool_hfid() -> NodeSchemaAPI: + """VLAN schema where vlan_id (NumberPool-sourced) is part of the human_friendly_id.""" + data: dict[str, Any] = { + "name": "VLAN", + "namespace": "Infra", + "label": "VLAN", + "default_filter": "name__value", + "order_by": ["name__value"], + "display_labels": ["name__value"], + "human_friendly_id": ["vlan_id__value"], + "attributes": [ + {"name": "name", "kind": "Text", "unique": True}, + {"name": "vlan_id", "kind": "Number"}, + ], + "relationships": [], + } + return NodeSchema(**data).convert_api() diff --git a/tests/unit/sdk/pool/test_attribute_from_pool.py b/tests/unit/sdk/pool/test_attribute_from_pool.py index 18ec619e..b024e7ca 100644 --- a/tests/unit/sdk/pool/test_attribute_from_pool.py +++ b/tests/unit/sdk/pool/test_attribute_from_pool.py @@ -10,9 +10,14 @@ from typing import TYPE_CHECKING, Any +import pytest + +from infrahub_sdk.exceptions import ValidationError from infrahub_sdk.node import InfrahubNode, InfrahubNodeSync if TYPE_CHECKING: + from pytest_httpx import HTTPXMock + from infrahub_sdk import InfrahubClient, InfrahubClientSync from infrahub_sdk.schema import NodeSchemaAPI @@ -201,3 +206,83 @@ async def test_attribute_with_pool_node_generates_mutation_query( mutation_query = vlan._generate_mutation_query() assert mutation_query["object"]["vlan_id"] == {"value": None} + + +UPSERT_MOCK_RESPONSE = { + "data": { + "InfraVLANUpsert": { + "ok": True, + "object": {"id": "mock-vlan-uuid", "vlan_id": {"value": 100}}, + } + } +} + + +@pytest.mark.httpx_mock(assert_all_responses_were_requested=False) +async def test_save_upsert_no_error_before_fix( + client: InfrahubClient, + vlan_schema_with_pool_hfid: NodeSchemaAPI, + httpx_mock: HTTPXMock, +) -> None: + """FAILS before the guard is added: ValidationError must be raised when HFID attr is NumberPool-sourced.""" + httpx_mock.add_response(method="POST", json=UPSERT_MOCK_RESPONSE) + node = InfrahubNode( + client=client, + schema=vlan_schema_with_pool_hfid, + data={"name": "Test VLAN", "vlan_id": {"from_pool": {"id": POOL_ID}}}, + ) + + with pytest.raises(ValidationError, match="vlan_id"): + await node.save(allow_upsert=True) + + +async def test_save_upsert_raises_when_numberpool_attr_in_hfid( + client: InfrahubClient, + vlan_schema_with_pool_hfid: NodeSchemaAPI, +) -> None: + """save(allow_upsert=True) raises ValidationError naming the pool-sourced HFID attribute.""" + node = InfrahubNode( + client=client, + schema=vlan_schema_with_pool_hfid, + data={"name": "Test VLAN", "vlan_id": {"from_pool": {"id": POOL_ID}}}, + ) + + with pytest.raises(ValidationError, match="vlan_id"): + await node.save(allow_upsert=True) + + +async def test_save_upsert_proceeds_when_explicit_id_set( + client: InfrahubClient, + vlan_schema_with_pool_hfid: NodeSchemaAPI, + httpx_mock: HTTPXMock, +) -> None: + """Guard is bypassed when an explicit node id is already set.""" + httpx_mock.add_response(method="POST", json=UPSERT_MOCK_RESPONSE) + node = InfrahubNode( + client=client, + schema=vlan_schema_with_pool_hfid, + data={"name": "Test VLAN", "vlan_id": {"from_pool": {"id": POOL_ID}}}, + ) + node.id = "existing-node-uuid" + + await node.save(allow_upsert=True) + + assert node.id == "mock-vlan-uuid" + + +async def test_save_upsert_proceeds_when_numberpool_attr_not_in_hfid( + client: InfrahubClient, + vlan_schema: NodeSchemaAPI, + httpx_mock: HTTPXMock, +) -> None: + """Guard does not fire when the pool-sourced attribute is not part of the HFID.""" + httpx_mock.add_response(method="POST", json=UPSERT_MOCK_RESPONSE) + node = InfrahubNode( + client=client, + schema=vlan_schema, + data={"name": "Test VLAN", "vlan_id": {"from_pool": {"id": POOL_ID}}}, + ) + + await node.save(allow_upsert=True) + + assert node.id == "mock-vlan-uuid" From 98047b9b3c32613717c34503771054b5fbde7241 Mon Sep 17 00:00:00 2001 From: Sola Babatunde Date: Tue, 12 May 2026 11:15:46 +0100 Subject: [PATCH 2/2] update docs --- docs/docs/python-sdk/guides/resource-manager.mdx | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/docs/python-sdk/guides/resource-manager.mdx b/docs/docs/python-sdk/guides/resource-manager.mdx index 64e3b43e..8a42b6e9 100644 --- a/docs/docs/python-sdk/guides/resource-manager.mdx +++ b/docs/docs/python-sdk/guides/resource-manager.mdx @@ -364,4 +364,3 @@ await vlan.save(allow_upsert=True) - **Deterministic identifier** — if possible, design your schema so the HFID uses a non-pool attribute (for example, a human-assigned `name`) and keep `vlan_id` out of the HFID. ::: -