New GraphQL filters (AND, OR and NOT) don't work as intended

[NetaliDev]

Deployment Type

Self-hosted

NetBox Version

v4.0.0

Python Version

3.11

Steps to Reproduce

Let's take the OR-Filter as an example: As described in the docs, I would like to filter for VMs that are in the active and in the staged status:

{
  virtual_machine_list(filters: {status: "staged", OR: {status: "active"}}) {
    name
    status
  }
}

Returns:

{
  "data": {
    "virtual_machine_list": []
  }
}

As seen, this query returns an empty result, while distinct queries for staged and active VMs return results:

{
  virtual_machine_list(filters: {status: "staged"}) {
    name
    status
  }
}

Returns:

{
  "data": {
    "virtual_machine_list": [
      {
        "name": "vm3",
        "status": "staged"
      },
      {
        "name": "vm4",
        "status": "staged"
      }
    ]
  }
}

And

{
  virtual_machine_list(filters: {status: "active"}) {
    name
    status
  }
}

Returns:

{
  "data": {
    "virtual_machine_list": [
      {
        "name": "vm1",
        "status": "active"
      },
      {
        "name": "vm2",
        "status": "active"
      }
    ]
  }
}

Or another example: the NOT-Filter: let's assume that I want to filter for all VMs that are not active:

{
  virtual_machine_list(filters: {NOT: {status: "active"}}) {
    name
    status
  }
}

But for some reason, this only returns active VMs:

{
  "data": {
    "virtual_machine_list": [
      {
        "name": "vm1",
        "status": "active"
      },
      {
        "name": "vm2",
        "status": "active"
      }
    ]
  }
}

Expected Behavior

That the OR-Filter returns active and staged VMs and that the NOT-Filter returns everything except active VMs.

Observed Behavior

Described above.

[jeffgdotorg]

I'm able to reproduce this problem on a fresh 4.0.0 install with two VMs with status "active" and two VMs with status "staged". My level of GraphQL sophistication is currently low, so I might have missed something subtle.

[jeffgdotorg]

Thanks for reporting a problem you've encountered in NetBox. My role in this moment is limited to triage, so you may get questions from the developers who understand these things more deeply.

[jeremystretch]

I'm not sure this filter logic is meant to be supported currently. I know we've had to do some extensive customization with the GraphQL filters to support django-filters integration. Maybe @arthanson can weigh in?

[NetaliDev]

In my opinion, we have a regression here if the new filter logic is not yet supported, because the support for the __n suffix for filters has been dropped in 4.0 and with the NOT filter not working, I currently don't see a way to filter for example all VMs that are not active (like the scenario described in the original bug report).

[arthanson]

Dug into this, the issue here is Strawberry requires Q objects to be returned from the filter functions in order to do the AND, OR, NOT operations, but when we call django-filter we get back a queryset. I don't see any built-in way to get a Q object from django-filter. It should be possible to basically duplicate some of what django-filter is doing and take a list of query-params and create a Q object from them. That should be basically what django-filter is doing internally before it uses them to generate the queryset.

[NetaliDev]

What is the status of this now? I would like to emphasize once again that this is a regression, as it is now no longer possible to use NOT filters, which in my view restricts the GraphQL API quite a lot.

[jeremystretch]

What is the status of this now?

As you can see from the assigned labels, this issue needs an owner. @NetaliDev would you like to volunteer to own this issue?

[NetaliDev]

Sorry, but I don't know enough about GraphQL, Strawberry and especially the netbox codebase to fix this. I just had the task to migrate our GraphQL queries at work to netbox 4.X and stumbled over this issue.

[arthanson]

Note: Make sure to test below syntax as well:

{
  interface_list(filters: {device: ["fceos21", "ceos1"]}) {
    name
  }
}

[OliElli]

+1 here, I just found that the NOT filter doesn't work in 4.0.7

Query that works in 3.x: {device_list (status__n: "active", tenant: "tenant-name-here") {name status}}
Query that does NOT work in Netbox 4.x: {device_list(filters: {tenant: "tenant-name-here", NOT: {status: "active"}}) {status name}}

Because the AND and OR args are also broken I can't even workaround by selecting all statuses but the one I want

[arthanson]

The basic issue is strawberry filtering uses Q objects, django-filters returns filtersets which makes them incompatible. None of the solutions I see look very good and all would take a lot of work.

Graphene interfaces with Django-filter and uses filtersets. Code is at https://github.com/graphql-python/graphene-django/tree/main/graphene_django/filter. About 8 files of code.

Strawberry is all based off of Q objects. Code is at https://github.com/strawberry-graphql/strawberry-django/blob/main/strawberry_django/filters.py. Strawberry documentation on filters is at: https://strawberry.rocks/docs/django/guide/filters

From what I can see, this is also tied into the GraphQL parsing framework, so if we tried to use Django-filter we would also probably have to make patches to that code as well.

We are also using Strawberry Legacy Filtering which needs to be removed (https://strawberry.rocks/docs/django/guide/filters#legacy-filtering) this would need changes to the auto type decorator (https://github.com/netbox-community/netbox/blob/develop/netbox/netbox/graphql/filter_mixins.py#L131).

I can think of several different potential solutions (Option 4 might potentially be the best option?)

1. Basically replicate filter handling in Django-graphene - override strawberry_django.process_filters to work with django-filters basically replacing strawberry's filter handling with a new one that is compatible with Django-filters.

   • pros: No changes to existing NetBox filter code. Plugin filtering / GraphQL code would remain compatible.
   • cons: Have to replicate all graphene filter handling code which was mentioned to be bug-prone. A lot of work needed for POC. Not compatible with any future Strawberry filtering enhancements and probably brittle for future updates to Strawberry. Lots of potential for edge-case bugs.

2. Change NetBox filterset functions to return Q objects (or provide sub-functions returning Q objects) which should make them compatible with Strawberry lookup code. (I think this might be the most straight-forward solution)

   Functions like (https://github.com/netbox-community/netbox/blob/develop/netbox/dcim/filtersets.py#L1152).
   So it is not possible to convert a QuerySet to a Q object, Django-filter is pretty much built around QuerySet. Many use Q objects - but some (https://github.com/netbox-community/netbox/blob/develop/netbox/dcim/filtersets.py#L607) are just query set - although exclude could be converted to a negative Q object ~Q(...)

   • pros: Keeps filtering code in one place. Quick to do a POC and could actually implement it filter by filter. Less potential for edge-case bugs.
   • cons: Have to modify NetBox filtering code. Plugin filtering code would need to be changed to support this.

3. Can override the default filter method (https://strawberry.rocks/docs/django/guide/filters#overriding-the-default-filter-method) to just call the filter set, but then would need to handle the (AND, OR, NOT, ...) parsing ourselves.

   • pros: No changes to NetBox filter code. Plugin filtering / GraphQL code would remain compatible.
   • cons: A lot of work needed for POC. Most brittle for future Strawberry versions. More potential for edge-case bugs.

4. [Potential Best Option?] Write custom Strawberry filters for each of the filterset methods. This is what Strawberry is sort-of designed for, but it would require replicating / duplicating all the special NetBox filter handling in a Strawberry compatible way. Similar to 2 but leaves the existing filter code (non GraphQL) untouched.

   • pros: No changes to NetBox filter code. Can just be added, no POC needed. Less potential for edge-case bugs.
   • cons: Need to duplicate filtering logic specifically for GraphQL and plugins would need to do the same. Not DRY.

   Note: Could create the Strawberry filters as Q objects as a first pass, then migrate those over to the filters code, thus doing item 2 above, but would allow time to test the filters in the real world.

Support for Django-filter has been requested in Strawberry, but it doesn't look like it will be implemented (strawberry-graphql/strawberry-django#448).

5. Move back to Graphene / abandoning Strawberry.

   • pros: Known entity and we know filtering works. Could be done fairly easily by rolling back to old code.
   • cons: graphene-django is still semi-dead with about 1 commit every 2-3 months (see: https://github.com/graphql-python/graphene-django/commits/main/). Has minor issues / lack of full support for newer versions of Django.

[jeremystretch]

I think we have at least a partial fix for this in PR #17638. The PR only impacts choice fields at the moment but can be extended as needed to accommodate other field types as well.

It would be very helpful if users impacted by this bug could check out the PR branch and confirm whether it resolves the issues for them, as well as any outstanding issues remain with GraphQL filtering. Thanks!