Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prefix in different VRF wrongly sees global prefix as its parent #11715

Closed
candlerb opened this issue Feb 9, 2023 · 4 comments · Fixed by #12448
Closed

Prefix in different VRF wrongly sees global prefix as its parent #11715

candlerb opened this issue Feb 9, 2023 · 4 comments · Fixed by #12448
Assignees
Labels
status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application

Comments

@candlerb
Copy link
Contributor

candlerb commented Feb 9, 2023

NetBox version

v3.4.4

Python version

3.8

Steps to Reproduce

  1. Create VRF "testvrf"
  2. Create prefix 192.168.0.0/24 in global VRF
  3. Create prefix 192.168.0.0/28 in "testvrf"
  4. Browse to 192.168.0.0/28 (which is where you'll be right after creation)

Expected Behavior

192.168.0.0/28@testvrf and 192.168.0.0/24 should be independent from each other, as they are in different VRFs.

Observed Behavior

192.168.0.0/28@testvrf sees 192.168.0.0/24 as its parent.

image

However, 192.168.0.0/24 does not see 192.168.0.0/28@testvrf as its child.

This is inconsistent: this child<->parent relationship should either work in both ways, or neither.

In this case, I think it should be neither: otherwise 192.168.0.0/24 could see many different prefixes and IP addresses in different VRFs as its children, which would mix lots of VRFs together in the same view.

EDIT: the current behaviour does appear to be intentional in the code:

        # Parent prefixes table
        parent_prefixes = Prefix.objects.restrict(request.user, 'view').filter(
            Q(vrf=instance.vrf) | Q(vrf__isnull=True)
@candlerb candlerb added the type: bug A confirmed report of unexpected behavior in the application label Feb 9, 2023
@jsenecal
Copy link
Contributor

The current behavior is indeed intentional. This allows for Container prefixes in the Global VRF to nest under it smaller prefixes in the table views.

Can you clarify what you would like fixed in that case, if anything ?

@jsenecal jsenecal added the status: under review Further discussion is needed to determine this issue's scope and/or implementation label Feb 10, 2023
@candlerb
Copy link
Contributor Author

If A is the parent of B, then B should be the child of A. This isn't the case currently.

If you want to make a container prefix for prefixes in VRF X, then the container should also be within VRF X (IMO).

@bobbwest
Copy link

Worse still, if there are two 192.168.0.0/24 prefixes, one in global, one in the VRF "testvrf", which one would be the parent of 192.168.0.0/28@testvrf ? Both? It should be the one in the same VRF.

VRFs are separate routing domains; there's no reason for a prefix in global to show up in a VRF. In the rare cases when route leaking is happening between VRFs, then I think that the prefix should be listed in each VRF with a note as to how it is appearing there.

@DanSheps
Copy link
Member

The current behavior is indeed intentional. This allows for Container prefixes in the Global VRF to nest under it smaller prefixes in the table views.

Can you clarify what you would like fixed in that case, if anything ?

It is only suppose to work this way if it is set as "Container" for status.

@DanSheps DanSheps added status: accepted This issue has been accepted for implementation and removed status: under review Further discussion is needed to determine this issue's scope and/or implementation labels May 2, 2023
@DanSheps DanSheps self-assigned this May 2, 2023
DanSheps added a commit that referenced this issue May 3, 2023
jeremystretch added a commit that referenced this issue May 5, 2023
* Fixes: #11715 - Fix Parent Prefix table display of global vrf prefixes that are **not** containers.

* Combine AND into a single Q object

Co-authored-by: Jeremy Stretch <[email protected]>

---------

Co-authored-by: Jeremy Stretch <[email protected]>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants