Django 4 support: Option 5, migrating to django-filter (v2) (#492)

* Fixes #487 by implementing option 5 in the issue.

Supports: python3.7+, django 3.2+. Tests included for this range.
Breaking changes: None

This implementation uses django-filter as intended. It sets a default filter backend and uses standard filter_class models where possible.

Exceptions take place in the following views:

HostList. Relies on manipulating the queryset, and said manipulation is also used in the filter-less HostDetail.
*Zone*List. The abstraction model for these views is based around propagating a filterset into the parent class.
Concerns:

This patch creates explicit mapping filters for JSONField and CIDRField. It is unclear how well-tested these are.

* Update pyproject.toml fix package building (for tox)

  - Required as we intriduced the toml file for ruff.
  - Sets some default keys.
  - Make tox.ini use the lint and coverage environments.
  - Make tox runs that use pythons and django show versions for both.

* A few cleanup fixes.

  - Uses filterset_class correctly.
  - Applies the filter via MregMixin.
  - Test basic filtering in hostgroups.

* Hack support CI-fields.

  - Requires manual lookup declarations.
  - See #489 (comment)

* Rebase to master, clean up imports a bit.

* Cleanup after rebase.

* Remove unused variable.

.github/workflows/test.yml

@@ -3,6 +3,7 @@ on:
     paths-ignore:
       - 'ci/**'
       - 'README.md'
+
   pull_request:
     types: [opened, reopened, synchronize]
   workflow_dispatch:
@@ -31,11 +32,11 @@ jobs:
       matrix:
         os: [ubuntu-latest]
         python-version:
-          - '3.7'
-          - '3.8'
-          - '3.9'
-          - '3.10'
-          - '3.11'
+          - "3.7"
+          - "3.8"
+          - "3.9"
+          - "3.10"
+          - "3.11"
     steps:
       - name: Checkout
         uses: actions/checkout@v3
@@ -57,16 +58,17 @@ jobs:
           # Needed to build the native python-ldap extension.
           sudo apt-get update
           sudo apt-get -y install libsasl2-dev libldap2-dev
-          pip install -r requirements.txt
-          pip install -r requirements-dev.txt
+          python -m pip install --upgrade pip
+          python -m pip install tox tox-gh-actions
+          python -m pip install -r requirements-test.txt
+      - name: Test with tox
+        run: tox -r
+        env:
+          MREG_DB_PASSWORD: postgres
       - name: Check migrations
         run: python manage.py makemigrations --check
       - name: Export OpenAPI schema
         run: python manage.py generateschema > openapi.yml
-      - name: Test
-        run: coverage run manage.py test -v2
-        env:
-          MREG_DB_PASSWORD: postgres
       - name: Upload OpenAPI schema
         if: matrix.python-version == '3.10'
         uses: actions/upload-artifact@v3
@@ -88,11 +90,11 @@ jobs:
       matrix:
         os: [ubuntu-latest]
         python-version:
-          - '3.7'
-          - '3.8'
-          - '3.9'
-          - '3.10'
-          - '3.11'
+          - "3.7"
+          - "3.8"
+          - "3.9"
+          - "3.10"
+          - "3.11"
     steps:
       - name: Checkout
         uses: actions/checkout@v3

hostpolicy/api/v1/tests.py

@@ -100,6 +100,27 @@ def test_create_with_date(self):
         post_data = {"name": "orange", "description": "Round and orange", "create_date": "2018-07-07"}
         self.assert_post('/api/v1/hostpolicy/atoms/', post_data)
 
+    def test_get_atoms_200_ok(self):
+        """Getting an existing entry should return 200"""
+        self.assert_get('/api/v1/hostpolicy/atoms/testatom1')
+        ret1 = self.assert_get('/api/v1/hostpolicy/atoms/?name=testatom1')
+        self.assertEqual(ret1.json()['count'], 1)
+        self.assertEqual(ret1.json()['results'][0]['name'], 'testatom1')
+
+        ret2 = self.assert_get('/api/v1/hostpolicy/atoms/')
+        self.assertEqual(ret2.json()['count'], 2)
+
+    def test_get_atoms_filtered_200_ok(self):
+        """Test filtering on atoms"""
+        ret2 = self.assert_get('/api/v1/hostpolicy/atoms/?name=testatom2')
+        self.assertEqual(ret2.json()['count'], 1)
+
+        ret2 = self.assert_get('/api/v1/hostpolicy/atoms/?name__contains=testatom2')
+        self.assertEqual(ret2.json()['count'], 1)
+
+        ret2 = self.assert_get('/api/v1/hostpolicy/atoms/?name__regex=.*testatom2.*')
+        self.assertEqual(ret2.json()['count'], 1)
+
 
 class HostPolicyAdminRights(MregAPITestCase):
     """Test that all of the API for HostPolicy are available for the admin group

hostpolicy/api/v1/views.py

@@ -2,7 +2,7 @@
 from rest_framework import status
 from rest_framework.response import Response
 
-from url_filter.filtersets import ModelFilterSet
+from django_filters import rest_framework as rest_filters
 
 from hostpolicy.models import HostPolicyAtom, HostPolicyRole
 from hostpolicy.api.permissions import IsSuperOrHostPolicyAdminOrReadOnly
@@ -20,15 +20,29 @@
 from . import serializers
 
 
-class HostPolicyAtomFilterSet(ModelFilterSet):
+# We can't use fields = '__all__' due to our use of LCI-fields:
+# https://github.com/unioslo/mreg/issues/489#issuecomment-1610209358
+# For the HostPolicyAtom model, this applies to the field "name"
+class HostPolicyAtomFilterSet(rest_filters.FilterSet):
     class Meta:
         model = HostPolicyAtom
+        fields = {
+            'name': ['exact', 'regex', 'contains'],
+        }
 
 
-class HostPolicyRoleFilterSet(ModelFilterSet):
+# We can't use fields = '__all__' due to our use of LCI-fields:
+# https://github.com/unioslo/mreg/issues/489#issuecomment-1610209358
+# For the HostPolicyRole model, this applies to the field "name"
+class HostPolicyRoleFilterSet(rest_filters.FilterSet):
     class Meta:
         model = HostPolicyRole
-
+        fields = {
+            'name': ['exact', 'regex', 'contains'],
+            'atoms__name': ['exact', 'regex', 'contains'],
+            'hosts__name': ['exact', 'regex', 'contains'],
+            'labels__name': ['exact', 'regex', 'contains'],
+        }
 
 class HostPolicyAtomLogMixin(HistoryLog):
 
@@ -61,10 +75,7 @@ class HostPolicyAtomList(HostPolicyAtomLogMixin, MregListCreateAPIView):
     serializer_class = serializers.HostPolicyAtomSerializer
     permission_classes = (IsSuperOrHostPolicyAdminOrReadOnly, )
     lookup_field = 'name'
-
-    def get_queryset(self):
-        qs = super().get_queryset()
-        return HostPolicyRoleFilterSet(data=self.request.GET, queryset=qs).filter()
+    filterset_class = HostPolicyAtomFilterSet
 
     def post(self, request, *args, **kwargs):
         if "name" in request.data:
@@ -96,10 +107,7 @@ class HostPolicyRoleList(HostPolicyRoleLogMixin, MregListCreateAPIView):
     serializer_class = serializers.HostPolicyRoleSerializer
     permission_classes = (IsSuperOrHostPolicyAdminOrReadOnly, )
     lookup_field = 'name'
-
-    def get_queryset(self):
-        qs = _role_prefetcher(super().get_queryset())
-        return HostPolicyRoleFilterSet(data=self.request.GET, queryset=qs).filter()
+    filterset_class = HostPolicyRoleFilterSet
 
     def post(self, request, *args, **kwargs):
         if "name" in request.data:

mreg/api/v1/filters.py

@@ -0,0 +1,203 @@
+from django_filters import rest_framework as filters
+
+from mreg.models.base import History
+from mreg.models.host import BACnetID, Host, HostGroup, Ipaddress, PtrOverride
+from mreg.models.network import (
+    Label,
+    NetGroupRegexPermission,
+    Network,
+    NetworkExcludedRange,
+)
+from mreg.models.resource_records import Cname, Hinfo, Loc, Mx, Naptr, Srv, Sshfp, Txt
+from mreg.models.zone import (
+    ForwardZone,
+    ForwardZoneDelegation,
+    NameServer,
+    ReverseZone,
+    ReverseZoneDelegation,
+)
+
+
+class JSONFieldExactFilter(filters.CharFilter):
+    pass
+
+
+class CIDRFieldExactFilter(filters.CharFilter):
+    pass
+
+
+class BACnetIDFilterSet(filters.FilterSet):
+    class Meta:
+        model = BACnetID
+        fields = "__all__"
+
+
+class CnameFilterSet(filters.FilterSet):
+    class Meta:
+        model = Cname
+        fields = "__all__"
+
+
+# Not that due to the email field being a CIEmailField, filtering on it
+# with lookups (email__contains=..., email__regex=..., etc) won't work.
+# This field is inherited from BaseZone.
+class ForwardZoneFilterSet(filters.FilterSet):
+    class Meta:
+        model = ForwardZone
+        fields = "__all__"
+
+
+class ForwardZoneDelegationFilterSet(filters.FilterSet):
+    class Meta:
+        model = ForwardZoneDelegation
+        fields = "__all__"
+
+
+class HinfoFilterSet(filters.FilterSet):
+    class Meta:
+        model = Hinfo
+        fields = "__all__"
+
+
+class HistoryFilterSet(filters.FilterSet):
+    data = JSONFieldExactFilter(field_name="data")
+
+    class Meta:
+        model = History
+        fields = "__all__"
+
+
+class HostFilterSet(filters.FilterSet):
+    class Meta:
+        model = Host
+        fields = "__all__"
+
+
+# We can't use fields = '__all__' due to our use of LCI-fields:
+# https://github.com/unioslo/mreg/issues/489#issuecomment-1610209358
+# For the HostGroup model, this applies to the field "name"
+class HostGroupFilterSet(filters.FilterSet):
+    class Meta:
+        model = HostGroup
+        fields = {
+            "name": ["exact", "regex", "contains"],
+            "description": ["exact", "regex", "contains"],
+            "owners__name": ["exact", "regex", "contains"],
+            "parent__name": ["exact", "regex", "contains"],
+            "hosts__name": ["exact", "regex", "contains"],
+        }
+
+
+class IpaddressFilterSet(filters.FilterSet):
+    class Meta:
+        model = Ipaddress
+        fields = "__all__"
+
+
+# We can't use fields = '__all__' due to our use of LCI-fields:
+# https://github.com/unioslo/mreg/issues/489#issuecomment-1610209358
+# For the Label model, this applies to the field "name"
+class LabelFilterSet(filters.FilterSet):
+    class Meta:
+        model = Label
+        fields = {
+            "name": ["exact", "regex", "contains"],
+            "description": ["exact", "regex", "contains"],
+        }
+
+
+class LocFilterSet(filters.FilterSet):
+    class Meta:
+        model = Loc
+        fields = "__all__"
+
+
+class MxFilterSet(filters.FilterSet):
+    class Meta:
+        model = Mx
+        fields = "__all__"
+
+
+class NameServerFilterSet(filters.FilterSet):
+    class Meta:
+        model = NameServer
+        fields = "__all__"
+
+
+class NaptrFilterSet(filters.FilterSet):
+    class Meta:
+        model = Naptr
+        fields = "__all__"
+
+
+class NetGroupRegexPermissionFilterSet(filters.FilterSet):
+    range = CIDRFieldExactFilter(field_name="range")
+
+    class Meta:
+        model = NetGroupRegexPermission
+        fields = "__all__"
+
+
+class NetworkFilterSet(filters.FilterSet):
+    network = CIDRFieldExactFilter(field_name="network")
+
+    class Meta:
+        model = Network
+        fields = "__all__"
+
+
+class NetworkExcludedRangeFilterSet(filters.FilterSet):
+    class Meta:
+        model = NetworkExcludedRange
+        fields = "__all__"
+
+
+class PtrOverrideFilterSet(filters.FilterSet):
+    class Meta:
+        model = PtrOverride
+        fields = "__all__"
+
+# Not that due to the email field being a CIEmailField, filtering on it
+# with lookups (email__contains=..., email__regex=..., etc) won't work.
+# This field is inherited from BaseZone.
+
+class ReverseZoneFilterSet(filters.FilterSet):
+    network = CIDRFieldExactFilter(field_name="network")
+
+    class Meta:
+        model = ReverseZone
+        fields = "__all__"
+
+
+class ReverseZoneDelegationFilterSet(filters.FilterSet):
+    class Meta:
+        model = ReverseZoneDelegation
+        fields = "__all__"
+
+# We can't use fields = '__all__' due to our use of LCI-fields:
+# https://github.com/unioslo/mreg/issues/489#issuecomment-1610209358
+# For the Srv model, this applies to the field "name"
+
+class SrvFilterSet(filters.FilterSet):
+    class Meta:
+        model = Srv
+        fields = {
+            "name": ["exact", "contains", "regex"],
+            "priority": ["exact", "lt", "gt"],
+            "weight": ["exact", "lt", "gt"],
+            "port": ["exact", "lt", "gt"],
+            "ttl": ["exact", "lt", "gt"],
+            "host__name": ["exact", "contains", "regex"],
+        }
+
+
+class SshfpFilterSet(filters.FilterSet):
+    class Meta:
+        model = Sshfp
+        fields = "__all__"
+
+
+class TxtFilterSet(filters.FilterSet):
+    class Meta:
+        model = Txt
+        fields = "__all__"

mreg/api/v1/tests/test_host_permissions.py

@@ -299,7 +299,6 @@ def test_can_create_hostname_with_prefix_underscore(self):
     def test_special_group_members_create_underscore(self):
         self.client = self.get_token_client(superuser=False, adminuser=True)
         self.add_user_to_groups('DNS_UNDERSCORE_GROUP')
-        path = '/api/v1/hosts/'
         data1 = {'name': '_host1.example.org', 'ipaddress': '10.0.0.1'}
         data2 = {'name': 'host2._sub.example.org', 'ipaddress': '10.0.0.2'}
         self.assert_post('/hosts/', data1)