[base] Fix hierarchical duplicate IP validation #116

[Stoiicc]

Checklist

• I have read the OpenWISP Contributing Guidelines.
• I have manually tested the changes proposed in this pull request.
• I have written new test cases for new code and/or updated existing tests for changes to existing code.
• I have updated the documentation.

Reference to Existing Issue

Closes #116 .

Description of Changes

Previously, AbstractIpAddress.clean() only checked for duplicate IP addresses within the same subnet.
As discussed in issue #116, this allows invalid configurations where:

An IP exists in a child subnet, and
The same IP is created again in a parent subnet.

This PR introduces hierarchical subnet duplicate detection by:

1. Identifying all related subnets

2. A subnet is considered related if its network contains the IP being validated.
   This includes:

• Parent subnets
• Child subnets
• Overlapping subnets
• The subnet itself

3. Validating duplicates across all related subnets

If the same IP exists in any related subnet, a ValidationError is raised.

[nemesifier]

Thanks @Stoiicc for tackling #116. The intent is right: an IP that exists in a child subnet should not be addable again in an overlapping parent subnet. But the implementation has a correctness bug and a serious performance problem, so it cannot merge as-is.

1. It ignores organization scoping (cross-tenant false conflicts)

for subnet in load_model("openwisp_ipam", "Subnet").objects.all():
    ...
duplicate = IpAddress.objects.filter(
    ip_address=self.ip_address, subnet_id__in=related_subnet_ids
).exclude(pk=self.pk).exists()

This scans every subnet in the database regardless of organization. So 10.0.1.10 in organization A's subnet would conflict with 10.0.1.10 in organization B's overlapping subnet, even though they are independent tenants. That breaks multi-tenant isolation. The check must be scoped to the relevant organization, consistently with how the rest of openwisp-ipam handles org scoping.

2. Full table scan in Python on every save

Subnet.objects.all() loads every subnet into Python and runs ip_network(...) per row on every IpAddress.clean(). On any non-trivial IPAM this is very slow. Please push the work into the database: filter candidate subnets by org and ideally test containment with a query, or at minimum restrict the loop to the small set of subnets that could plausibly contain this IP.

3. Test and style

tests/test_hierarchy_validation.py imports the concrete models directly rather than using load_model, has no trailing newline, inconsistent spacing, and calls both full_clean() and save() inside assertRaises (the save is unreachable). Please align it with the existing test style and cover the cross-org case explicitly (same IP in two different orgs must be allowed).

Correct goal, but fix the organization scoping (this is a blocker), make the lookup a bounded DB query, and clean up the test.

[openwisp-companion]

Hi @Stoiicc 👋,

This is a friendly reminder that this pull request has had no activity for 7 days since changes were requested.

We'd love to see this contribution merged! Please take a moment to:

• Address the review feedback
• Push your changes
• Let us know if you have any questions or need clarification

If you're busy or need more time, no worries! Just leave a comment to let us know you're still working on it.

Note: within 7 more days, the linked issue will be unassigned to allow other contributors to work on it.

Thank you for your contribution! 🙏

[openwisp-companion]

Hi @Stoiicc 👋,

This pull request has been marked as stale due to 15 days of inactivity after changes were requested.

As a result, any linked issues are being unassigned from you so other contributors can pick them up.

However, you can still continue working on this PR! If you push new commits or respond to the review feedback:

• The issue will be reassigned to you
• Your contribution is still very welcome

If you need more time or have questions about the requested changes, please let us know. We're happy to help! 🤝