Conversation
bb-Ricardo
reviewed
Sep 14, 2026
Comment on lines
+45
to
+75
| # NetBox exempts addresses with these roles from its uniqueness check, they are | ||
| # meant to exist on more than one interface at the same time | ||
| non_unique_ip_roles = ("anycast", "vip", "vrrp", "hsrp", "glbp", "carp") | ||
|
|
||
| # stub function to implement a finish call for each source | ||
| def finish(self): | ||
| pass | ||
|
|
||
| @staticmethod | ||
| def get_ip_address_role(ip_object): | ||
| """ | ||
| Returns the role of an IP address object. The NetBox API reports it as a dict, | ||
| an object this program created itself carries the plain value. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| ip_object: NBIPAddress | ||
| IP address object to get the role from | ||
|
|
||
| Returns | ||
| ------- | ||
| (str, None): role of this IP address or None if unset | ||
| """ | ||
|
|
||
| role = grab(ip_object, "data.role") | ||
|
|
||
| if isinstance(role, dict): | ||
| return role.get("value") | ||
|
|
||
| return role | ||
|
|
Owner
There was a problem hiding this comment.
as this code is specific to NBIPAddress I would add it as object method like ip_object.get_role(). What do you think?
Collaborator
Author
There was a problem hiding this comment.
Good call — done. Moved it onto NBIPAddress as get_role(), right next to get_interface()/get_device_vm(), so the role normalization lives with the object instead of the shared base. Both call sites now read ip.get_role(), and I added a couple of unit tests for the dict-vs-plain-value cases.
bb-Ricardo
requested changes
Sep 14, 2026
semx
force-pushed
the
fix/non-unique-ip-roles
branch
from
September 15, 2026 06:59
180ccec to
55621a0
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the anycast half of #344.
The anycast branch logs that the address can be assigned to multiple interfaces at the same time and then skips the assignment, so the second VM sharing the address never gets it. Now this interface gets an object of its own carrying the same role, and the object already on the other interface is left alone.
Two things came out of it. The check only knew
anycast, but NetBox exemptsvip,vrrp,hsrp,glbpandcarpfrom its uniqueness rule for the same reason, so all six are handled. And it read the role asdata.role.value, which only matches what the API returns - an object we created ourselves holds the plain string, so our own anycast addresses were not recognised on a later run. Both shapes are read now. The removal path had the same narrow check and uses the same set.The role is only set on an object we create, never written onto an existing one, since that is the user's to decide.
Plain duplicates without such a role still produce the old warning. That is the VLAN case from the first post in #344, and @Azmodeszer said he can live with it. It needs a config switch to change, tell me if you want one and I'll add it.
tests/test_ip_non_unique_roles.py: 7 of 8 fail on development, all pass here, including a control that a second run reuses the object instead of creating another one. Suite 112.