update ipam reconcile query to handle deleted relationships correctly (…

…#5074)

* unit tests and a first fix

* add labels filter to IP nodes so we don't pick up DiffNodes

* update reconcile query for deleted edges

* allow deleting last prefix

* add changelog

backend/infrahub/core/query/ipam.py

@@ -359,7 +359,9 @@ async def query_init(self, db: InfrahubDatabase, **kwargs) -> None:
         ]
 
         namespace_query = """
+        // ------------------
         // Get IP Namespace
+        // ------------------
         MATCH (ip_namespace:%(namespace_kind)s {uuid: $namespace_id})-[r:IS_PART_OF]->(root:Root)
         WHERE %(branch_filter)s
         """ % {"branch_filter": branch_filter, "namespace_kind": self.params["namespace_kind"]}
@@ -368,14 +370,23 @@ async def query_init(self, db: InfrahubDatabase, **kwargs) -> None:
         if self.ip_uuid:
             self.params["node_uuid"] = self.ip_uuid
             get_node_by_id_query = """
+            // ------------------
             // Get IP Prefix node by UUID
+            // ------------------
             MATCH (ip_node {uuid: $node_uuid})
-            """
+            WHERE "%(ip_kind)s" IN labels(ip_node)
+            """ % {
+                "ip_kind": InfrahubKind.IPADDRESS
+                if isinstance(self.ip_value, IPAddressType)
+                else InfrahubKind.IPPREFIX,
+            }
             self.add_to_query(get_node_by_id_query)
 
         else:
             get_node_by_prefix_query = """
+            // ------------------
             // Get IP Prefix node by prefix value
+            // ------------------
             OPTIONAL MATCH ip_node_path = (:Root)<-[:IS_PART_OF]-(ip_node:%(ip_kind)s)
             -[:HAS_ATTRIBUTE]->(a:Attribute)-[:HAS_VALUE]->(aipn:%(ip_attribute_kind)s),
             (ip_namespace)-[:IS_RELATED]-(nsr:Relationship)
@@ -397,23 +408,51 @@ async def query_init(self, db: InfrahubDatabase, **kwargs) -> None:
             self.add_to_query(get_node_by_prefix_query)
 
         get_current_parent_query = """
+        // ------------------
         // Get prefix node's current parent, if it exists
-        OPTIONAL MATCH parent_prefix_path = (ip_node)-[:IS_RELATED]->(:Relationship {name: "parent__child"})-[:IS_RELATED]->(current_parent:%(ip_prefix_kind)s)
-        WHERE all(r IN relationships(parent_prefix_path) WHERE (%(branch_filter)s) and r.status = "active")
+        // ------------------
+        CALL {
+            WITH ip_node
+            OPTIONAL MATCH parent_prefix_path = (ip_node)-[r1:IS_RELATED]->(:Relationship {name: "parent__child"})-[r2:IS_RELATED]->(current_parent:%(ip_prefix_kind)s)
+            WHERE all(r IN relationships(parent_prefix_path) WHERE (%(branch_filter)s))
+            RETURN current_parent, (r1.status = "active" AND r2.status = "active") AS parent_is_active
+            ORDER BY r1.branch_level DESC, r2.branch_level DESC, r1.from DESC, r2.from DESC
+            LIMIT 1
+        }
+        WITH ip_namespace, ip_node, CASE WHEN parent_is_active THEN current_parent ELSE NULL END as current_parent
         """ % {
             "branch_filter": branch_filter,
             "ip_prefix_kind": InfrahubKind.IPPREFIX,
         }
         self.add_to_query(get_current_parent_query)
 
         get_current_children_query = """
+        // ------------------
         // Get prefix node's current prefix children, if any exist
-        OPTIONAL MATCH child_prefix_path = (ip_node)<-[:IS_RELATED]-(:Relationship {name: "parent__child"})<-[:IS_RELATED]-(current_prefix_child:%(ip_prefix_kind)s)
-        WHERE all(r IN relationships(child_prefix_path) WHERE (%(branch_filter)s) and r.status = "active")
+        // ------------------
+        CALL {
+            WITH ip_node
+            OPTIONAL MATCH child_prefix_path = (ip_node)<-[r1:IS_RELATED]-(:Relationship {name: "parent__child"})<-[r2:IS_RELATED]-(current_prefix_child:%(ip_prefix_kind)s)
+            WHERE all(r IN relationships(child_prefix_path) WHERE (%(branch_filter)s))
+            WITH current_prefix_child, (r1.status = "active" AND r2.status = "active") AS is_active
+            ORDER BY current_prefix_child.uuid, r1.branch_level DESC, r1.from DESC, r2.branch_level DESC, r2.from DESC
+            RETURN current_prefix_child, head(collect(is_active)) AS prefix_child_is_active
+        }
+        WITH ip_namespace, ip_node, current_parent, CASE WHEN prefix_child_is_active THEN current_prefix_child ELSE NULL END as current_prefix_child
         WITH ip_namespace, ip_node, current_parent, collect(current_prefix_child) AS current_prefix_children
+        // ------------------
         // Get prefix node's current address children, if any exist
-        OPTIONAL MATCH child_address_path = (ip_node)-[:IS_RELATED]-(:Relationship {name: "ip_prefix__ip_address"})-[:IS_RELATED]-(current_address_child:%(ip_address_kind)s)
-        WHERE all(r IN relationships(child_address_path) WHERE (%(branch_filter)s) and r.status = "active")
+        // ------------------
+        CALL {
+            WITH ip_node
+            OPTIONAL MATCH child_address_path = (ip_node)-[r1:IS_RELATED]-(:Relationship {name: "ip_prefix__ip_address"})-[r2:IS_RELATED]-(current_address_child:%(ip_address_kind)s)
+            WHERE all(r IN relationships(child_address_path) WHERE (%(branch_filter)s))
+            WITH current_address_child, (r1.status = "active" AND r2.status = "active") AS is_active
+            ORDER BY current_address_child.uuid, r1.branch_level DESC, r1.from DESC, r2.branch_level DESC, r2.from DESC
+            RETURN current_address_child, head(collect(is_active)) AS address_child_is_active
+
+        }
+        WITH ip_namespace, ip_node, current_parent, current_prefix_children, CASE WHEN address_child_is_active THEN current_address_child ELSE NULL END as current_address_child
         WITH ip_namespace, ip_node, current_parent, current_prefix_children, collect(current_address_child) AS current_address_children
         WITH ip_namespace, ip_node, current_parent, current_prefix_children + current_address_children AS current_children
         """ % {
@@ -424,7 +463,9 @@ async def query_init(self, db: InfrahubDatabase, **kwargs) -> None:
         self.add_to_query(get_current_children_query)
 
         get_new_parent_query = """
+        // ------------------
         // Identify the correct parent, if any, for the prefix node
+        // ------------------
         CALL {
             WITH ip_namespace
             OPTIONAL MATCH parent_path = (ip_namespace)-[pr1:IS_RELATED {status: "active"}]-(ns_rel:Relationship {name: "ip_namespace__ip_prefix"})
@@ -445,8 +486,8 @@ async def query_init(self, db: InfrahubDatabase, **kwargs) -> None:
             WITH maybe_new_parent, prefixlen, is_active
             RETURN maybe_new_parent, head(collect(prefixlen)) AS mnp_prefixlen, head(collect(is_active)) AS mnp_is_active
         }
-        WITH ip_namespace, ip_node, current_parent, current_children, maybe_new_parent, mnp_prefixlen, mnp_is_active
-        WHERE mnp_is_active OR maybe_new_parent IS NULL
+        WITH ip_namespace, ip_node, current_parent, current_children, mnp_prefixlen,
+            CASE WHEN mnp_is_active THEN maybe_new_parent ELSE NULL END AS maybe_new_parent
         WITH ip_namespace, ip_node, current_parent, current_children, maybe_new_parent, mnp_prefixlen
         ORDER BY ip_node.uuid, mnp_prefixlen DESC
         WITH ip_namespace, ip_node, current_parent, current_children, head(collect(maybe_new_parent)) as new_parent
@@ -458,13 +499,15 @@ async def query_init(self, db: InfrahubDatabase, **kwargs) -> None:
         self.add_to_query(get_new_parent_query)
 
         get_new_children_query = """
+        // ------------------
         // Identify the correct children, if any, for the prefix node
+        // ------------------
         CALL {
             // Get ALL possible children for the prefix node
             WITH ip_namespace, ip_node
             OPTIONAL MATCH child_path = (
-                 (ip_namespace)-[:IS_RELATED]
-                 -(ns_rel:Relationship)-[:IS_RELATED]
+                 (ip_namespace)-[r1:IS_RELATED]
+                 -(ns_rel:Relationship)-[r2:IS_RELATED]
                  -(maybe_new_child:Node)-[har:HAS_ATTRIBUTE]
                  ->(a:Attribute)-[hvr:HAS_VALUE]
                  ->(av:AttributeValue)
@@ -481,26 +524,30 @@ async def query_init(self, db: InfrahubDatabase, **kwargs) -> None:
             )
             AND av.version = $ip_version
             AND av.binary_address STARTS WITH $prefix_binary_host
-            AND all(r IN relationships(child_path) WHERE (%(branch_filter)s) AND r.status = "active")
+            AND all(r IN relationships(child_path) WHERE (%(branch_filter)s))
             WITH
                 maybe_new_child,
                 av AS mnc_attribute,
+                r1,
+                r2,
                 har,
                 hvr,
-                (har.status = "active" AND hvr.status = "active") AS is_active,
-                har.branch_level + hvr.branch_level AS branch_level
-            ORDER BY maybe_new_child.uuid, branch_level DESC, har.from DESC, hvr.from DESC
+                all(r in relationships(child_path) WHERE r.status = "active") AS is_active,
+                reduce(br_lvl = 0, r in relationships(child_path) | br_lvl + r.branch_level) AS branch_level
+            ORDER BY maybe_new_child.uuid, branch_level DESC, hvr.from DESC, har.from DESC, r2.from DESC, r1.from DESC
             WITH maybe_new_child, head(collect([mnc_attribute, is_active])) AS latest_mnc_details
             RETURN maybe_new_child, latest_mnc_details[0] AS latest_mnc_attribute, latest_mnc_details[1] AS mnc_is_active
         }
-        WITH ip_namespace, ip_node, current_parent, current_children, new_parent, maybe_new_child, latest_mnc_attribute, mnc_is_active
-        WHERE mnc_is_active = TRUE OR mnc_is_active IS NULL
+        WITH ip_namespace, ip_node, current_parent, current_children, new_parent, latest_mnc_attribute,
+            CASE WHEN mnc_is_active IN [TRUE, NULL] THEN maybe_new_child ELSE NULL END AS maybe_new_child
         WITH ip_namespace, ip_node, current_parent, current_children, new_parent, collect([maybe_new_child, latest_mnc_attribute]) AS maybe_children_ips
         WITH ip_namespace, ip_node, current_parent, current_children, new_parent, maybe_children_ips, range(0, size(maybe_children_ips) - 1) AS child_indices
         UNWIND child_indices as ind
         CALL {
+            // ------------------
             // Filter all possible children to remove those that have a more-specific parent
             // among the list of all possible children
+            // ------------------
             WITH ind, maybe_children_ips
             WITH ind, maybe_children_ips AS ips
             RETURN REDUCE(

backend/tests/unit/core/ipam/test_ipam_reconcile_query.py

@@ -4,6 +4,7 @@
 from infrahub.core.branch import Branch
 from infrahub.core.constants import InfrahubKind
 from infrahub.core.initialization import create_branch, create_ipam_namespace, get_default_ipnamespace
+from infrahub.core.manager import NodeManager
 from infrahub.core.node import Node
 from infrahub.core.query.ipam import IPPrefixReconcileQuery
 from infrahub.core.schema.schema_branch import SchemaBranch
@@ -236,6 +237,73 @@ async def test_ipprefix_reconcile_query_get_deleted_node_by_uuid(
     }
 
 
+async def test_ipprefix_reconcile_query_deleted_children_ignored_on_branch(
+    db: InfrahubDatabase, ip_dataset_01: dict[str, Node]
+):
+    branch = await create_branch(db=db, branch_name="branch2")
+
+    ns1_id = ip_dataset_01["ns1"].id
+    net140_branch = await NodeManager.get_one(db=db, branch=branch, id=ip_dataset_01["net140"].id)
+    await net140_branch.delete(db=db)
+    net145_branch = await NodeManager.get_one(db=db, branch=branch, id=ip_dataset_01["net145"].id)
+    await net145_branch.delete(db=db)
+    address_10_branch = await NodeManager.get_one(db=db, branch=branch, id=ip_dataset_01["address10"].id)
+    await address_10_branch.delete(db=db)
+
+    query = await IPPrefixReconcileQuery.init(
+        db=db,
+        branch=branch,
+        ip_value=ipaddress.ip_network(net140_branch.prefix.value),
+        node_uuid=net140_branch.id,
+        namespace=ns1_id,
+    )
+    await query.execute(db=db)
+
+    assert query.get_ip_node_uuid() == net140_branch.id
+    assert query.get_current_parent_uuid() is None
+    assert query.get_current_children_uuids() == []
+    assert query.get_calculated_parent_uuid() == ip_dataset_01["net146"].id
+    assert set(query.get_calculated_children_uuids()) == {
+        ip_dataset_01["net142"].id,
+        ip_dataset_01["net144"].id,
+        # should not be included b/c the address was deleted
+        # ip_dataset_01["net145"].id,
+        # ip_dataset_01["address10"].id
+    }
+
+
+async def test_ipprefix_reconcile_query_deleted_parent_ignored_on_branch(
+    db: InfrahubDatabase, ip_dataset_01: dict[str, Node]
+):
+    branch = await create_branch(db=db, branch_name="branch2")
+
+    ns1_id = ip_dataset_01["ns1"].id
+    net140_branch = await NodeManager.get_one(db=db, branch=branch, id=ip_dataset_01["net140"].id)
+    await net140_branch.delete(db=db)
+    net146_branch = await NodeManager.get_one(db=db, branch=branch, id=ip_dataset_01["net146"].id)
+    await net146_branch.delete(db=db)
+
+    query = await IPPrefixReconcileQuery.init(
+        db=db,
+        branch=branch,
+        ip_value=ipaddress.ip_network(net140_branch.prefix.value),
+        node_uuid=net140_branch.id,
+        namespace=ns1_id,
+    )
+    await query.execute(db=db)
+
+    assert query.get_ip_node_uuid() == net140_branch.id
+    assert query.get_current_parent_uuid() is None
+    assert query.get_current_children_uuids() == []
+    assert query.get_calculated_parent_uuid() is None
+    assert set(query.get_calculated_children_uuids()) == {
+        ip_dataset_01["net142"].id,
+        ip_dataset_01["net144"].id,
+        ip_dataset_01["net145"].id,
+        ip_dataset_01["address10"].id,
+    }
+
+
 async def test_branch_updates_respected(db: InfrahubDatabase, default_branch: Branch, ip_dataset_01):
     prefix_schema = registry.schema.get_node_schema(name="IpamIPPrefix", branch=default_branch)
     address_schema = registry.schema.get_node_schema(name="IpamIPAddress", branch=default_branch)

backend/tests/unit/graphql/mutations/test_ipam.py

@@ -5,6 +5,7 @@
 from infrahub.core import registry
 from infrahub.core.branch import Branch
 from infrahub.core.constants import InfrahubKind
+from infrahub.core.manager import NodeManager
 from infrahub.core.node import Node
 from infrahub.core.schema.schema_branch import SchemaBranch
 from infrahub.database import InfrahubDatabase
@@ -1125,3 +1126,46 @@ async def test_prefix_ancestors_descendants(
     assert children == [{"node": {"id": net16.id}}]
     descendants = prefix_details["descendants"]["edges"]
     assert descendants == [{"node": {"id": net16.id}}]
+
+
+async def test_delete_top_level_prefix(
+    db: InfrahubDatabase,
+    default_branch: Branch,
+    default_ipnamespace: Node,
+    register_core_models_schema: SchemaBranch,
+    register_ipam_schema: SchemaBranch,
+):
+    prefix_schema = registry.schema.get_node_schema(name="IpamIPPrefix", branch=default_branch)
+
+    ns1 = await Node.init(db=db, schema=InfrahubKind.NAMESPACE)
+    await ns1.new(db=db, name="ns1")
+    await ns1.save(db=db)
+    net8 = await Node.init(db=db, schema=prefix_schema)
+    await net8.new(db=db, prefix="10.0.0.0/8", ip_namespace=ns1)
+    await net8.save(db=db)
+    net10 = await Node.init(db=db, schema=prefix_schema)
+    await net10.new(db=db, prefix="10.0.0.0/10", parent=net8, ip_namespace=ns1)
+    await net10.save(db=db)
+
+    gql_params = prepare_graphql_params(db=db, include_subscription=False, branch=default_branch)
+    delete_top = await graphql(
+        schema=gql_params.schema,
+        source=DELETE_IPPREFIX,
+        context_value=gql_params.context,
+        variable_values={"id": str(net10.id)},
+    )
+    assert not delete_top.errors
+    assert delete_top.data["IpamIPPrefixDelete"]["ok"] is True
+
+    gql_params = prepare_graphql_params(db=db, include_subscription=False, branch=default_branch)
+    delete_last_prefix = await graphql(
+        schema=gql_params.schema,
+        source=DELETE_IPPREFIX,
+        context_value=gql_params.context,
+        variable_values={"id": str(net8.id)},
+    )
+    assert not delete_last_prefix.errors
+    assert delete_last_prefix.data["IpamIPPrefixDelete"]["ok"] is True
+
+    ip_prefixes = await NodeManager.query(db=db, branch=default_branch, schema="IpamIPPrefix")
+    assert len(ip_prefixes) == 0

changelog/+ipam-reconcile-delete.fixed.md

@@ -0,0 +1 @@
+Fix bug in IP reconciliation query around deleted nodes and relationships