From 9d99ede024218bf8d46584449e223c9e21270bca Mon Sep 17 00:00:00 2001 From: thatmattlove Date: Tue, 7 Sep 2021 16:20:36 -0700 Subject: [PATCH 1/4] Fixes #7202: Verify integrity of bundled assets in CI --- .github/workflows/ci.yml | 3 +++ docs/release-notes/version-3.0.md | 1 + scripts/verify-bundles.sh | 41 +++++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+) create mode 100755 scripts/verify-bundles.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d734ad2f0..c8e3f47ab 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -58,6 +58,9 @@ jobs: - name: Check UI ESLint, TypeScript, and Prettier Compliance run: yarn --cwd netbox/project-static validate + + - name: Validate Static Asset Integrity + run: scripts/verify-bundles.sh - name: Run tests run: coverage run --source="netbox/" netbox/manage.py test netbox/ diff --git a/docs/release-notes/version-3.0.md b/docs/release-notes/version-3.0.md index ac2ee021c..2389d6730 100644 --- a/docs/release-notes/version-3.0.md +++ b/docs/release-notes/version-3.0.md @@ -9,6 +9,7 @@ * [#7169](https://github.com/netbox-community/netbox/issues/7169) - Fix CSV import file upload * [#7176](https://github.com/netbox-community/netbox/issues/7176) - Fix issue where query parameters were duplicated across different forms of the same type * [#7193](https://github.com/netbox-community/netbox/issues/7193) - Fix prefix (flat) template issue when viewing child prefixes with prefixes available +* [#7202](https://github.com/netbox-community/netbox/issues/7202) - Verify integrity of static assets in CI --- diff --git a/scripts/verify-bundles.sh b/scripts/verify-bundles.sh new file mode 100755 index 000000000..e674de9bd --- /dev/null +++ b/scripts/verify-bundles.sh @@ -0,0 +1,41 @@ +#!/usr/bin/env bash + +# This script verifies the integrity of *bundled* static assets by re-running the bundling process +# and checking for changed files. Because bundle output should not change given the same source +# input, the bundle process shouldn't produce any changes. If they do, it's an indication that +# the dist files have been altered, or that dist files were not committed. In either case, tests +# should fail. + +PROJECT_STATIC="$PWD/netbox/project-static" +DIST="$PROJECT_STATIC/dist/" + +# Bundle static assets. +bundle() { + echo "Bundling static assets..." + yarn --cwd $PROJECT_STATIC bundle >/dev/null 2>&1 + if [[ $? != 0 ]]; then + echo "Error bundling static assets" + exit 1 + fi +} + +# See if any files have changed. +check_dist() { + local diff=$(git --no-pager diff $DIST) + if [[ $diff != "" ]]; then + local SHA=$(git rev-parse HEAD) + echo "Commit '$SHA' produced different static assets than were committed" + exit 1 + fi +} + +bundle +check_dist + +if [[ $? = 0 ]]; then + echo "Static asset check passed" + exit 0 +else + echo "Error checking static asset integrity" + exit 1 +fi From 0e8c6ee52298d3141ae8698d7f342a353e9d5439 Mon Sep 17 00:00:00 2001 From: jeremystretch Date: Wed, 8 Sep 2021 08:33:30 -0400 Subject: [PATCH 2/4] Changelog for #7189 --- docs/release-notes/version-3.0.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/release-notes/version-3.0.md b/docs/release-notes/version-3.0.md index e1bb57d72..84cd3994b 100644 --- a/docs/release-notes/version-3.0.md +++ b/docs/release-notes/version-3.0.md @@ -9,6 +9,7 @@ * [#7169](https://github.com/netbox-community/netbox/issues/7169) - Fix CSV import file upload * [#7176](https://github.com/netbox-community/netbox/issues/7176) - Fix issue where query parameters were duplicated across different forms of the same type * [#7188](https://github.com/netbox-community/netbox/issues/7188) - Fix issue where select fields with `null_option` did not render or send the null option +* [#7189](https://github.com/netbox-community/netbox/issues/7189) - Set connection factory for django-redis when Sentinel is in use * [#7193](https://github.com/netbox-community/netbox/issues/7193) - Fix prefix (flat) template issue when viewing child prefixes with prefixes available --- From 9226302742fdd557d74b5e747165e844de19f009 Mon Sep 17 00:00:00 2001 From: jeremystretch Date: Wed, 8 Sep 2021 09:38:23 -0400 Subject: [PATCH 3/4] Fixes #7209: Allow unlimited API results when MAX_PAGE_SIZE is disabled --- docs/release-notes/version-3.0.md | 1 + netbox/netbox/api/pagination.py | 21 ++++++++---- netbox/utilities/tests/test_api.py | 51 +++++++++++++++++++++++++++++- 3 files changed, 66 insertions(+), 7 deletions(-) diff --git a/docs/release-notes/version-3.0.md b/docs/release-notes/version-3.0.md index 84cd3994b..ea463dfd7 100644 --- a/docs/release-notes/version-3.0.md +++ b/docs/release-notes/version-3.0.md @@ -11,6 +11,7 @@ * [#7188](https://github.com/netbox-community/netbox/issues/7188) - Fix issue where select fields with `null_option` did not render or send the null option * [#7189](https://github.com/netbox-community/netbox/issues/7189) - Set connection factory for django-redis when Sentinel is in use * [#7193](https://github.com/netbox-community/netbox/issues/7193) - Fix prefix (flat) template issue when viewing child prefixes with prefixes available +* [#7209](https://github.com/netbox-community/netbox/issues/7209) - Allow unlimited API results when `MAX_PAGE_SIZE` is disabled --- diff --git a/netbox/netbox/api/pagination.py b/netbox/netbox/api/pagination.py index 77af755ce..e34cb27d0 100644 --- a/netbox/netbox/api/pagination.py +++ b/netbox/netbox/api/pagination.py @@ -34,13 +34,22 @@ class OptionalLimitOffsetPagination(LimitOffsetPagination): return list(queryset[self.offset:]) def get_limit(self, request): - limit = super().get_limit(request) + if self.limit_query_param: + try: + limit = int(request.query_params[self.limit_query_param]) + if limit < 0: + raise ValueError() + # Enforce maximum page size, if defined + if settings.MAX_PAGE_SIZE: + if limit == 0: + return settings.MAX_PAGE_SIZE + else: + return min(limit, settings.MAX_PAGE_SIZE) + return limit + except (KeyError, ValueError): + pass - # Enforce maximum page size - if settings.MAX_PAGE_SIZE: - limit = min(limit, settings.MAX_PAGE_SIZE) - - return limit + return self.default_limit def get_next_link(self): diff --git a/netbox/utilities/tests/test_api.py b/netbox/utilities/tests/test_api.py index 2cc9accaa..d06670ca9 100644 --- a/netbox/utilities/tests/test_api.py +++ b/netbox/utilities/tests/test_api.py @@ -1,7 +1,8 @@ import urllib.parse +from django.conf import settings from django.contrib.contenttypes.models import ContentType -from django.test import Client, TestCase +from django.test import Client, TestCase, override_settings from django.urls import reverse from rest_framework import status @@ -122,6 +123,54 @@ class WritableNestedSerializerTest(APITestCase): self.assertEqual(VLAN.objects.count(), 0) +class APIPaginationTestCase(APITestCase): + + @classmethod + def setUpTestData(cls): + cls.url = reverse('dcim-api:site-list') + + # Create a large number of Sites for testing + Site.objects.bulk_create([ + Site(name=f'Site {i}', slug=f'site-{i}') for i in range(1, 101) + ]) + + def test_default_page_size(self): + response = self.client.get(self.url, format='json', **self.header) + page_size = settings.PAGINATE_COUNT + self.assertLess(page_size, 100, "Default page size not sufficient for data set") + + self.assertEqual(response.data['count'], 100) + self.assertTrue(response.data['next'].endswith(f'?limit={page_size}&offset={page_size}')) + self.assertIsNone(response.data['previous']) + self.assertEqual(len(response.data['results']), page_size) + + def test_custom_page_size(self): + response = self.client.get(f'{self.url}?limit=10', format='json', **self.header) + + self.assertEqual(response.data['count'], 100) + self.assertTrue(response.data['next'].endswith(f'?limit=10&offset=10')) + self.assertIsNone(response.data['previous']) + self.assertEqual(len(response.data['results']), 10) + + @override_settings(MAX_PAGE_SIZE=20) + def test_max_page_size(self): + response = self.client.get(f'{self.url}?limit=0', format='json', **self.header) + + self.assertEqual(response.data['count'], 100) + self.assertTrue(response.data['next'].endswith(f'?limit=20&offset=20')) + self.assertIsNone(response.data['previous']) + self.assertEqual(len(response.data['results']), 20) + + @override_settings(MAX_PAGE_SIZE=0) + def test_max_page_size_disabled(self): + response = self.client.get(f'{self.url}?limit=0', format='json', **self.header) + + self.assertEqual(response.data['count'], 100) + self.assertIsNone(response.data['next']) + self.assertIsNone(response.data['previous']) + self.assertEqual(len(response.data['results']), 100) + + class APIDocsTestCase(TestCase): def setUp(self): From e12314ba60ae2ba0045cd49b9f2cedbe4bc283bf Mon Sep 17 00:00:00 2001 From: jeremystretch Date: Wed, 8 Sep 2021 09:57:53 -0400 Subject: [PATCH 4/4] Fix test user permissions for API pagination tests --- netbox/utilities/testing/api.py | 4 ++-- netbox/utilities/tests/test_api.py | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/netbox/utilities/testing/api.py b/netbox/utilities/testing/api.py index c67296c47..8498f93ae 100644 --- a/netbox/utilities/testing/api.py +++ b/netbox/utilities/testing/api.py @@ -39,13 +39,13 @@ class APITestCase(ModelTestCase): def setUp(self): """ - Create a superuser and token for API calls. + Create a user and token for API calls. """ # Create the test user and assign permissions self.user = User.objects.create_user(username='testuser') self.add_permissions(*self.user_permissions) self.token = Token.objects.create(user=self.user) - self.header = {'HTTP_AUTHORIZATION': 'Token {}'.format(self.token.key)} + self.header = {'HTTP_AUTHORIZATION': f'Token {self.token.key}'} def _get_view_namespace(self): return f'{self.view_namespace or self.model._meta.app_label}-api' diff --git a/netbox/utilities/tests/test_api.py b/netbox/utilities/tests/test_api.py index d06670ca9..5b711056a 100644 --- a/netbox/utilities/tests/test_api.py +++ b/netbox/utilities/tests/test_api.py @@ -124,6 +124,7 @@ class WritableNestedSerializerTest(APITestCase): class APIPaginationTestCase(APITestCase): + user_permissions = ('dcim.view_site',) @classmethod def setUpTestData(cls): @@ -139,6 +140,7 @@ class APIPaginationTestCase(APITestCase): page_size = settings.PAGINATE_COUNT self.assertLess(page_size, 100, "Default page size not sufficient for data set") + self.assertHttpStatus(response, status.HTTP_200_OK) self.assertEqual(response.data['count'], 100) self.assertTrue(response.data['next'].endswith(f'?limit={page_size}&offset={page_size}')) self.assertIsNone(response.data['previous']) @@ -147,6 +149,7 @@ class APIPaginationTestCase(APITestCase): def test_custom_page_size(self): response = self.client.get(f'{self.url}?limit=10', format='json', **self.header) + self.assertHttpStatus(response, status.HTTP_200_OK) self.assertEqual(response.data['count'], 100) self.assertTrue(response.data['next'].endswith(f'?limit=10&offset=10')) self.assertIsNone(response.data['previous']) @@ -156,6 +159,7 @@ class APIPaginationTestCase(APITestCase): def test_max_page_size(self): response = self.client.get(f'{self.url}?limit=0', format='json', **self.header) + self.assertHttpStatus(response, status.HTTP_200_OK) self.assertEqual(response.data['count'], 100) self.assertTrue(response.data['next'].endswith(f'?limit=20&offset=20')) self.assertIsNone(response.data['previous']) @@ -165,6 +169,7 @@ class APIPaginationTestCase(APITestCase): def test_max_page_size_disabled(self): response = self.client.get(f'{self.url}?limit=0', format='json', **self.header) + self.assertHttpStatus(response, status.HTTP_200_OK) self.assertEqual(response.data['count'], 100) self.assertIsNone(response.data['next']) self.assertIsNone(response.data['previous'])