From 5dc956fbe11718dff7f4b4b6284cdaef7ddc2b50 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Fri, 28 Feb 2020 15:07:59 -0500 Subject: [PATCH 01/10] First stab at external authentication support --- docs/configuration/optional-settings.md | 48 +++++++++++++++++++++++++ netbox/netbox/configuration.example.py | 8 +++++ netbox/netbox/settings.py | 16 +++++++-- netbox/utilities/auth_backends.py | 47 +++++++++++++++++++++++- netbox/utilities/middleware.py | 9 +++++ 5 files changed, 124 insertions(+), 4 deletions(-) diff --git a/docs/configuration/optional-settings.md b/docs/configuration/optional-settings.md index cbe01728c..5b4fd1d14 100644 --- a/docs/configuration/optional-settings.md +++ b/docs/configuration/optional-settings.md @@ -291,6 +291,54 @@ When determining the primary IP address for a device, IPv6 is preferred over IPv --- +## REMOTE_AUTH_ENABLED + +Default: `False` + +NetBox can be configured to support remote user authentication by inferring user authentication from an HTTP header set by the HTTP reverse proxy (e.g. nginx or Apache). Set this to `True` to enable this functionality. (Local authenitcation will still take effect as a fallback.) + +--- + +## REMOTE_AUTH_BACKEND + +Default: `'utilities.auth_backends.RemoteUserBackend'` + +Python path to the custom [Django authentication backend]() to use for external user authentication, if not using NetBox's built-in backend. (Requires `REMOTE_AUTH_ENABLED`.) + +--- + +## REMOTE_AUTH_HEADER + +Default: `'HTTP_REMOTE_USER'` + +When remote user authentication is in use, this is the name of the HTTP header which informs NetBox of the currently authenticated user. (Requires `REMOTE_AUTH_ENABLED`.) + +--- + +## REMOTE_AUTH_AUTO_CREATE_USER + +Default: `True` + +If true, NetBox will automatically create local accounts for users authenticated via a remote service. (Requires `REMOTE_AUTH_ENABLED`.) + +--- + +## REMOTE_AUTH_DEFAULT_GROUPS + +Default: `[]` (Empty list) + +The list of groups to assign a new user account when created using remote authentication. (Requires `REMOTE_AUTH_ENABLED`.) + +--- + +## REMOTE_AUTH_DEFAULT_PERMISSIONS + +Default: `[]` (Empty list) + +The list of permissions to assign a new user account when created using remote authentication. (Requires `REMOTE_AUTH_ENABLED`.) + +--- + ## REPORTS_ROOT Default: $BASE_DIR/netbox/reports/ diff --git a/netbox/netbox/configuration.example.py b/netbox/netbox/configuration.example.py index 7002def9b..1aada7c2d 100644 --- a/netbox/netbox/configuration.example.py +++ b/netbox/netbox/configuration.example.py @@ -179,6 +179,14 @@ PAGINATE_COUNT = 50 # prefer IPv4 instead. PREFER_IPV4 = False +# Remote authentication support +REMOTE_AUTH_ENABLED = False +REMOTE_AUTH_BACKEND = 'utilities.auth_backends.RemoteUserBackend' +REMOTE_AUTH_HEADER = 'HTTP_REMOTE_USER' +REMOTE_AUTH_AUTO_CREATE_USER = True +REMOTE_AUTH_DEFAULT_GROUPS = [] +REMOTE_AUTH_DEFAULT_PERMISSIONS = [] + # The file path where custom reports will be stored. A trailing slash is not needed. Note that the default value of # this setting is derived from the installed location. # REPORTS_ROOT = '/opt/netbox/netbox/reports' diff --git a/netbox/netbox/settings.py b/netbox/netbox/settings.py index bdd83723d..ed3974312 100644 --- a/netbox/netbox/settings.py +++ b/netbox/netbox/settings.py @@ -93,6 +93,12 @@ NAPALM_TIMEOUT = getattr(configuration, 'NAPALM_TIMEOUT', 30) NAPALM_USERNAME = getattr(configuration, 'NAPALM_USERNAME', '') PAGINATE_COUNT = getattr(configuration, 'PAGINATE_COUNT', 50) PREFER_IPV4 = getattr(configuration, 'PREFER_IPV4', False) +REMOTE_AUTH_AUTO_CREATE_USER = getattr(configuration, 'REMOTE_AUTH_AUTO_CREATE_USER', False) +REMOTE_AUTH_BACKEND = getattr(configuration, 'REMOTE_AUTH_BACKEND', 'utilities.auth_backends.RemoteUserBackend') +REMOTE_AUTH_DEFAULT_GROUPS = getattr(configuration, 'REMOTE_AUTH_DEFAULT_GROUPS', []) +REMOTE_AUTH_DEFAULT_PERMISSIONS = getattr(configuration, 'REMOTE_AUTH_DEFAULT_PERMISSIONS', []) +REMOTE_AUTH_ENABLED = getattr(configuration, 'REMOTE_AUTH_ENABLED', False) +REMOTE_AUTH_HEADER = getattr(configuration, 'REMOTE_AUTH_HEADER', 'HTTP_REMOTE_USER') REPORTS_ROOT = getattr(configuration, 'REPORTS_ROOT', os.path.join(BASE_DIR, 'reports')).rstrip('/') SCRIPTS_ROOT = getattr(configuration, 'SCRIPTS_ROOT', os.path.join(BASE_DIR, 'scripts')).rstrip('/') SESSION_FILE_PATH = getattr(configuration, 'SESSION_FILE_PATH', None) @@ -258,7 +264,7 @@ INSTALLED_APPS = [ ] # Middleware -MIDDLEWARE = ( +MIDDLEWARE = [ 'debug_toolbar.middleware.DebugToolbarMiddleware', 'django_prometheus.middleware.PrometheusBeforeMiddleware', 'corsheaders.middleware.CorsMiddleware', @@ -274,7 +280,9 @@ MIDDLEWARE = ( 'utilities.middleware.APIVersionMiddleware', 'extras.middleware.ObjectChangeMiddleware', 'django_prometheus.middleware.PrometheusAfterMiddleware', -) +] +if REMOTE_AUTH_ENABLED: + MIDDLEWARE.append('utilities.middleware.RemoteUserMiddleware') ROOT_URLCONF = 'netbox.urls' @@ -297,10 +305,12 @@ TEMPLATES = [ }, ] -# Authentication +# Set up authentication backends AUTHENTICATION_BACKENDS = [ 'utilities.auth_backends.ViewExemptModelBackend', ] +if REMOTE_AUTH_ENABLED: + AUTHENTICATION_BACKENDS.insert(0, REMOTE_AUTH_BACKEND) # Internationalization LANGUAGE_CODE = 'en-us' diff --git a/netbox/utilities/auth_backends.py b/netbox/utilities/auth_backends.py index 54541b0b5..52c3454f1 100644 --- a/netbox/utilities/auth_backends.py +++ b/netbox/utilities/auth_backends.py @@ -1,5 +1,8 @@ +import logging + from django.conf import settings -from django.contrib.auth.backends import ModelBackend +from django.contrib.auth.backends import ModelBackend, RemoteUserBackend as RemoteUserBackend_ +from django.contrib.auth.models import Group, Permission class ViewExemptModelBackend(ModelBackend): @@ -26,3 +29,45 @@ class ViewExemptModelBackend(ModelBackend): pass return super().has_perm(user_obj, perm, obj) + + +class RemoteUserBackend(ViewExemptModelBackend, RemoteUserBackend_): + """ + Custom implementation of Django's RemoteUserBackend which provides configuration hooks for basic customization. + """ + @property + def create_unknown_user(self): + return bool(settings.REMOTE_AUTH_AUTO_CREATE_USER) + + def configure_user(self, request, user): + logger = logging.getLogger('netbox.authentication.RemoteUserBackend') + + # Assign default groups to the user + group_list = [] + for name in settings.REMOTE_AUTH_DEFAULT_GROUPS: + try: + group_list.append(Group.objects.get(name=name)) + except Group.DoesNotExist: + logging.error("Could not assign group {name} to remotely-authenticated user {user}: Group not found") + if group_list: + user.groups.add(*group_list) + logger.debug(f"Assigned groups to remotely-authenticated user {user}: {group_list}") + + # Assign default permissions to the user + permissions_list = [] + for permission_name in settings.REMOTE_AUTH_DEFAULT_PERMISSIONS: + try: + app_label, codename = permission_name.split('.') + permissions_list.append( + Permission.objects.get(content_type__app_label=app_label, codename=codename) + ) + except (ValueError, Permission.DoesNotExist): + logging.error( + "Invalid permission name: '{permission_name}'. Permissions must be in the form " + "._. (Example: dcim.add_site)" + ) + if permissions_list: + user.user_permissions.add(*permissions_list) + logger.debug(f"Assigned permissions to remotely-authenticated user {user}: {permissions_list}") + + return user diff --git a/netbox/utilities/middleware.py b/netbox/utilities/middleware.py index 564771821..a4fd1a254 100644 --- a/netbox/utilities/middleware.py +++ b/netbox/utilities/middleware.py @@ -1,6 +1,7 @@ from urllib import parse from django.conf import settings +from django.contrib.auth.middleware import RemoteUserMiddleware as RemoteUserMiddleware_ from django.db import ProgrammingError from django.http import Http404, HttpResponseRedirect from django.urls import reverse @@ -30,6 +31,14 @@ class LoginRequiredMiddleware(object): return self.get_response(request) +class RemoteUserMiddleware(RemoteUserMiddleware_): + """ + Custom implementation of Django's RemoteUserMiddleware which allows for a user-configurable HTTP header name. + """ + force_logout_if_no_header = False + header = settings.REMOTE_AUTH_HEADER + + class APIVersionMiddleware(object): """ If the request is for an API endpoint, include the API version as a response header. From 8c6d35645d0a86bd741d864c270b1d489d7d7409 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Tue, 10 Mar 2020 16:56:57 -0400 Subject: [PATCH 02/10] Remote auth cleanup --- docs/configuration/optional-settings.md | 2 +- netbox/utilities/middleware.py | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/configuration/optional-settings.md b/docs/configuration/optional-settings.md index dc15ed4da..4015badf4 100644 --- a/docs/configuration/optional-settings.md +++ b/docs/configuration/optional-settings.md @@ -319,7 +319,7 @@ NetBox can be configured to support remote user authentication by inferring user Default: `'utilities.auth_backends.RemoteUserBackend'` -Python path to the custom [Django authentication backend]() to use for external user authentication, if not using NetBox's built-in backend. (Requires `REMOTE_AUTH_ENABLED`.) +Python path to the custom [Django authentication backend](https://docs.djangoproject.com/en/stable/topics/auth/customizing/) to use for external user authentication, if not using NetBox's built-in backend. (Requires `REMOTE_AUTH_ENABLED`.) --- diff --git a/netbox/utilities/middleware.py b/netbox/utilities/middleware.py index 8e2b1993b..8141be53f 100644 --- a/netbox/utilities/middleware.py +++ b/netbox/utilities/middleware.py @@ -37,7 +37,10 @@ class RemoteUserMiddleware(RemoteUserMiddleware_): Custom implementation of Django's RemoteUserMiddleware which allows for a user-configurable HTTP header name. """ force_logout_if_no_header = False - header = settings.REMOTE_AUTH_HEADER + + @property + def header(self): + return settings.REMOTE_AUTH_HEADER class APIVersionMiddleware(object): From 90144ccd9abca5bc416835399bb2e783ffba43f8 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Tue, 10 Mar 2020 16:57:34 -0400 Subject: [PATCH 03/10] Add tests for remote authentication configuration --- netbox/netbox/tests/test_authentication.py | 124 +++++++++++++++++++++ 1 file changed, 124 insertions(+) create mode 100644 netbox/netbox/tests/test_authentication.py diff --git a/netbox/netbox/tests/test_authentication.py b/netbox/netbox/tests/test_authentication.py new file mode 100644 index 000000000..7c9f42bff --- /dev/null +++ b/netbox/netbox/tests/test_authentication.py @@ -0,0 +1,124 @@ +from django.contrib.auth.models import Group, Permission, User +from django.test import Client, TestCase +from django.test.utils import override_settings +from django.urls import reverse + + +class ExternalAuthenticationTestCase(TestCase): + + @override_settings( + REMOTE_AUTH_ENABLED=True, + LOGIN_REQUIRED=True + ) + def test_remote_auth(self): + """ + Test enabling remote authentication with the default configuration. + """ + headers = { + 'HTTP_REMOTE_USER': 'remoteuser1', + } + + self.client = Client() + response = self.client.get(reverse('home'), follow=True, **headers) + self.assertEqual(response.status_code, 200) + + user = User.objects.get(username='remoteuser1') + self.assertEqual(int(self.client.session['_auth_user_id']), user.pk, msg='Authentication failed') + + @override_settings( + REMOTE_AUTH_ENABLED=True, + REMOTE_AUTH_HEADER='HTTP_FOO', + LOGIN_REQUIRED=True + ) + def test_remote_auth_custom_header(self): + """ + Test enabling remote authentication with a custom HTTP header. + """ + headers = { + 'HTTP_FOO': 'remoteuser1', + } + + self.client = Client() + response = self.client.get(reverse('home'), follow=True, **headers) + self.assertEqual(response.status_code, 200) + + user = User.objects.get(username='remoteuser1') + self.assertEqual(int(self.client.session['_auth_user_id']), user.pk, msg='Authentication failed') + + @override_settings( + REMOTE_AUTH_ENABLED=True, + REMOTE_AUTH_AUTO_CREATE_USER=False, + LOGIN_REQUIRED=True + ) + def test_remote_auth_no_auto_create(self): + """ + Test enabling remote authentication with automatic user creation disabled. + """ + headers = { + 'HTTP_REMOTE_USER': 'remoteuser1', + } + + self.client = Client() + + # First attempt should fail as the user does not exist + self.client.get(reverse('home'), **headers) + self.assertNotIn('_auth_user_id', self.client.session) + + # Create the user locally and try again + user = User.objects.create(username='remoteuser1') + response = self.client.get(reverse('home'), follow=True, **headers) + self.assertEqual(response.status_code, 200) + self.assertEqual(int(self.client.session['_auth_user_id']), user.pk, msg='Authentication failed') + + @override_settings( + REMOTE_AUTH_ENABLED=True, + REMOTE_AUTH_DEFAULT_GROUPS=['Group 1', 'Group 2'], + LOGIN_REQUIRED=True + ) + def test_remote_auth_default_groups(self): + """ + Test enabling remote authentication with the default configuration. + """ + headers = { + 'HTTP_REMOTE_USER': 'remoteuser1', + } + + # Create required groups + groups = ( + Group(name='Group 1'), + Group(name='Group 2'), + Group(name='Group 3'), + ) + Group.objects.bulk_create(groups) + + self.client = Client() + response = self.client.get(reverse('home'), follow=True, **headers) + self.assertEqual(response.status_code, 200) + + user = User.objects.get(username='remoteuser1') + self.assertEqual(int(self.client.session['_auth_user_id']), user.pk, msg='Authentication failed') + self.assertListEqual( + [groups[0], groups[1]], + list(user.groups.all()) + ) + + @override_settings( + REMOTE_AUTH_ENABLED=True, + REMOTE_AUTH_DEFAULT_PERMISSIONS=['dcim.add_site', 'dcim.change_site'], + LOGIN_REQUIRED=True + ) + def test_remote_auth_default_permissions(self): + """ + Test enabling remote authentication with the default configuration. + """ + headers = { + 'HTTP_REMOTE_USER': 'remoteuser1', + } + + self.client = Client() + response = self.client.get(reverse('home'), follow=True, **headers) + self.assertEqual(response.status_code, 200) + + user = User.objects.get(username='remoteuser1') + self.assertEqual(int(self.client.session['_auth_user_id']), user.pk, msg='Authentication failed') + self.assertTrue(user.has_perms(['dcim.add_site', 'dcim.change_site'])) From 7ffc00159e978e6ab0aeb1eac1451e78181b46ab Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Wed, 11 Mar 2020 11:10:26 -0400 Subject: [PATCH 04/10] Tweak settings/middleware to support testing; improve tests --- netbox/netbox/settings.py | 6 +- netbox/netbox/tests/test_authentication.py | 99 +++++++++++++++------- netbox/utilities/auth_backends.py | 2 +- netbox/utilities/middleware.py | 8 ++ 4 files changed, 78 insertions(+), 37 deletions(-) diff --git a/netbox/netbox/settings.py b/netbox/netbox/settings.py index b330c8660..3c24b061a 100644 --- a/netbox/netbox/settings.py +++ b/netbox/netbox/settings.py @@ -277,13 +277,12 @@ MIDDLEWARE = [ 'django.middleware.clickjacking.XFrameOptionsMiddleware', 'django.middleware.security.SecurityMiddleware', 'utilities.middleware.ExceptionHandlingMiddleware', + 'utilities.middleware.RemoteUserMiddleware', 'utilities.middleware.LoginRequiredMiddleware', 'utilities.middleware.APIVersionMiddleware', 'extras.middleware.ObjectChangeMiddleware', 'django_prometheus.middleware.PrometheusAfterMiddleware', ] -if REMOTE_AUTH_ENABLED: - MIDDLEWARE.append('utilities.middleware.RemoteUserMiddleware') ROOT_URLCONF = 'netbox.urls' @@ -308,10 +307,9 @@ TEMPLATES = [ # Set up authentication backends AUTHENTICATION_BACKENDS = [ + REMOTE_AUTH_BACKEND, 'utilities.auth_backends.ViewExemptModelBackend', ] -if REMOTE_AUTH_ENABLED: - AUTHENTICATION_BACKENDS.insert(0, REMOTE_AUTH_BACKEND) # Internationalization LANGUAGE_CODE = 'en-us' diff --git a/netbox/netbox/tests/test_authentication.py b/netbox/netbox/tests/test_authentication.py index 7c9f42bff..42cddb082 100644 --- a/netbox/netbox/tests/test_authentication.py +++ b/netbox/netbox/tests/test_authentication.py @@ -1,4 +1,5 @@ -from django.contrib.auth.models import Group, Permission, User +from django.conf import settings +from django.contrib.auth.models import Group, User from django.test import Client, TestCase from django.test.utils import override_settings from django.urls import reverse @@ -6,11 +7,17 @@ from django.urls import reverse class ExternalAuthenticationTestCase(TestCase): + @classmethod + def setUpTestData(cls): + cls.user = User.objects.create(username='remoteuser1') + + def setUp(self): + self.client = Client() + @override_settings( - REMOTE_AUTH_ENABLED=True, LOGIN_REQUIRED=True ) - def test_remote_auth(self): + def test_remote_auth_disabled(self): """ Test enabling remote authentication with the default configuration. """ @@ -18,12 +25,31 @@ class ExternalAuthenticationTestCase(TestCase): 'HTTP_REMOTE_USER': 'remoteuser1', } - self.client = Client() + self.assertFalse(settings.REMOTE_AUTH_ENABLED) + self.assertEqual(settings.REMOTE_AUTH_HEADER, 'HTTP_REMOTE_USER') + + # Client should not be authenticated + response = self.client.get(reverse('home'), follow=True, **headers) + self.assertNotIn('_auth_user_id', self.client.session) + + @override_settings( + REMOTE_AUTH_ENABLED=True, + LOGIN_REQUIRED=True + ) + def test_remote_auth_enabled(self): + """ + Test enabling remote authentication with the default configuration. + """ + headers = { + 'HTTP_REMOTE_USER': 'remoteuser1', + } + + self.assertTrue(settings.REMOTE_AUTH_ENABLED) + self.assertEqual(settings.REMOTE_AUTH_HEADER, 'HTTP_REMOTE_USER') + response = self.client.get(reverse('home'), follow=True, **headers) self.assertEqual(response.status_code, 200) - - user = User.objects.get(username='remoteuser1') - self.assertEqual(int(self.client.session['_auth_user_id']), user.pk, msg='Authentication failed') + self.assertEqual(int(self.client.session.get('_auth_user_id')), self.user.pk, msg='Authentication failed') @override_settings( REMOTE_AUTH_ENABLED=True, @@ -38,40 +64,40 @@ class ExternalAuthenticationTestCase(TestCase): 'HTTP_FOO': 'remoteuser1', } - self.client = Client() + self.assertTrue(settings.REMOTE_AUTH_ENABLED) + self.assertEqual(settings.REMOTE_AUTH_HEADER, 'HTTP_FOO') + response = self.client.get(reverse('home'), follow=True, **headers) self.assertEqual(response.status_code, 200) - - user = User.objects.get(username='remoteuser1') - self.assertEqual(int(self.client.session['_auth_user_id']), user.pk, msg='Authentication failed') + self.assertEqual(int(self.client.session.get('_auth_user_id')), self.user.pk, msg='Authentication failed') @override_settings( REMOTE_AUTH_ENABLED=True, - REMOTE_AUTH_AUTO_CREATE_USER=False, + REMOTE_AUTH_AUTO_CREATE_USER=True, LOGIN_REQUIRED=True ) - def test_remote_auth_no_auto_create(self): + def test_remote_auth_auto_create(self): """ Test enabling remote authentication with automatic user creation disabled. """ headers = { - 'HTTP_REMOTE_USER': 'remoteuser1', + 'HTTP_REMOTE_USER': 'remoteuser2', } - self.client = Client() + self.assertTrue(settings.REMOTE_AUTH_ENABLED) + self.assertTrue(settings.REMOTE_AUTH_AUTO_CREATE_USER) + self.assertEqual(settings.REMOTE_AUTH_HEADER, 'HTTP_REMOTE_USER') - # First attempt should fail as the user does not exist - self.client.get(reverse('home'), **headers) - self.assertNotIn('_auth_user_id', self.client.session) - - # Create the user locally and try again - user = User.objects.create(username='remoteuser1') response = self.client.get(reverse('home'), follow=True, **headers) self.assertEqual(response.status_code, 200) - self.assertEqual(int(self.client.session['_auth_user_id']), user.pk, msg='Authentication failed') + + # Local user should have been automatically created + new_user = User.objects.get(username='remoteuser2') + self.assertEqual(int(self.client.session.get('_auth_user_id')), new_user.pk, msg='Authentication failed') @override_settings( REMOTE_AUTH_ENABLED=True, + REMOTE_AUTH_AUTO_CREATE_USER=True, REMOTE_AUTH_DEFAULT_GROUPS=['Group 1', 'Group 2'], LOGIN_REQUIRED=True ) @@ -80,9 +106,14 @@ class ExternalAuthenticationTestCase(TestCase): Test enabling remote authentication with the default configuration. """ headers = { - 'HTTP_REMOTE_USER': 'remoteuser1', + 'HTTP_REMOTE_USER': 'remoteuser2', } + self.assertTrue(settings.REMOTE_AUTH_ENABLED) + self.assertTrue(settings.REMOTE_AUTH_AUTO_CREATE_USER) + self.assertEqual(settings.REMOTE_AUTH_HEADER, 'HTTP_REMOTE_USER') + self.assertEqual(settings.REMOTE_AUTH_DEFAULT_GROUPS, ['Group 1', 'Group 2']) + # Create required groups groups = ( Group(name='Group 1'), @@ -91,19 +122,19 @@ class ExternalAuthenticationTestCase(TestCase): ) Group.objects.bulk_create(groups) - self.client = Client() response = self.client.get(reverse('home'), follow=True, **headers) self.assertEqual(response.status_code, 200) - user = User.objects.get(username='remoteuser1') - self.assertEqual(int(self.client.session['_auth_user_id']), user.pk, msg='Authentication failed') + new_user = User.objects.get(username='remoteuser2') + self.assertEqual(int(self.client.session.get('_auth_user_id')), new_user.pk, msg='Authentication failed') self.assertListEqual( [groups[0], groups[1]], - list(user.groups.all()) + list(new_user.groups.all()) ) @override_settings( REMOTE_AUTH_ENABLED=True, + REMOTE_AUTH_AUTO_CREATE_USER=True, REMOTE_AUTH_DEFAULT_PERMISSIONS=['dcim.add_site', 'dcim.change_site'], LOGIN_REQUIRED=True ) @@ -112,13 +143,17 @@ class ExternalAuthenticationTestCase(TestCase): Test enabling remote authentication with the default configuration. """ headers = { - 'HTTP_REMOTE_USER': 'remoteuser1', + 'HTTP_REMOTE_USER': 'remoteuser2', } - self.client = Client() + self.assertTrue(settings.REMOTE_AUTH_ENABLED) + self.assertTrue(settings.REMOTE_AUTH_AUTO_CREATE_USER) + self.assertEqual(settings.REMOTE_AUTH_HEADER, 'HTTP_REMOTE_USER') + self.assertEqual(settings.REMOTE_AUTH_DEFAULT_PERMISSIONS, ['dcim.add_site', 'dcim.change_site']) + response = self.client.get(reverse('home'), follow=True, **headers) self.assertEqual(response.status_code, 200) - user = User.objects.get(username='remoteuser1') - self.assertEqual(int(self.client.session['_auth_user_id']), user.pk, msg='Authentication failed') - self.assertTrue(user.has_perms(['dcim.add_site', 'dcim.change_site'])) + new_user = User.objects.get(username='remoteuser2') + self.assertEqual(int(self.client.session.get('_auth_user_id')), new_user.pk, msg='Authentication failed') + self.assertTrue(new_user.has_perms(['dcim.add_site', 'dcim.change_site'])) diff --git a/netbox/utilities/auth_backends.py b/netbox/utilities/auth_backends.py index 52c3454f1..6e968a241 100644 --- a/netbox/utilities/auth_backends.py +++ b/netbox/utilities/auth_backends.py @@ -37,7 +37,7 @@ class RemoteUserBackend(ViewExemptModelBackend, RemoteUserBackend_): """ @property def create_unknown_user(self): - return bool(settings.REMOTE_AUTH_AUTO_CREATE_USER) + return settings.REMOTE_AUTH_AUTO_CREATE_USER def configure_user(self, request, user): logger = logging.getLogger('netbox.authentication.RemoteUserBackend') diff --git a/netbox/utilities/middleware.py b/netbox/utilities/middleware.py index 8141be53f..d86be752b 100644 --- a/netbox/utilities/middleware.py +++ b/netbox/utilities/middleware.py @@ -42,6 +42,14 @@ class RemoteUserMiddleware(RemoteUserMiddleware_): def header(self): return settings.REMOTE_AUTH_HEADER + def process_request(self, request): + + # Bypass middleware if remote authentication is not enabled + if not settings.REMOTE_AUTH_ENABLED: + return + + return super().process_request(request) + class APIVersionMiddleware(object): """ From 2b33e91e2cdf9cd77e89d71f51325d26ce9a6f89 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Wed, 11 Mar 2020 11:27:47 -0400 Subject: [PATCH 05/10] Changelog for #2328 --- docs/release-notes/version-2.8.md | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/docs/release-notes/version-2.8.md b/docs/release-notes/version-2.8.md index 38cda9414..035a67b74 100644 --- a/docs/release-notes/version-2.8.md +++ b/docs/release-notes/version-2.8.md @@ -1,14 +1,30 @@ -# v2.8.0 (FUTURE) +# NetBox v2.8 -## Enhancements +## v2.8.0 (FUTURE) + +### New Features + +#### Remote Authentication Support ([#2328](https://github.com/netbox-community/netbox/issues/2328)) + +Several new configuration parameters provide support for authenticating an incoming request based on the value of a specific HTTP header. This can be leveraged to employ remote authentication via an nginx or Apache plugin, directing NetBox to create and configure a local user account as needed. The configuration parameters are: + +* `REMOTE_AUTH_ENABLED` - Enables remote authentication (disabled by default) +* `REMOTE_AUTH_HEADER` - The name of the HTTP header which conveys the username +* `REMOTE_AUTH_AUTO_CREATE_USER` - Enables the automatic creation of new users (disabled by default) +* `REMOTE_AUTH_DEFAULT_GROUPS` - A list of groups to assign newly created users +* `REMOTE_AUTH_DEFAULT_PERMISSIONS` - A list of permissions to assign newly created users + +If further customization of remote authentication is desired (for instance, if you want to pass group/permission information via HTTP headers as well), NetBox allows you to inject a custom [Django authentication backend](https://docs.djangoproject.com/en/stable/topics/auth/customizing/) to retain full control over the authentication and configuration of remote users. + +### Enhancements * [#4195](https://github.com/netbox-community/netbox/issues/4195) - Enabled application logging (see [logging configuration](../configuration/optional-settings.md#logging)) -## API Changes +### API Changes * dcim.Rack: The `/api/dcim/racks//units/` endpoint has been replaced with `/api/dcim/racks//elevation/`. * The `id__in` filter has been removed. Use the format `?id=1&id=2` instead. ([#4313](https://github.com/netbox-community/netbox/issues/4313)) -## Other Changes +### Other Changes * [#4081](https://github.com/netbox-community/netbox/issues/4081) - The `family` field has been removed from the Aggregate, Prefix, and IPAddress models From 84de0458aa440ee8b79fa76484ac345c67bbbcf5 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Wed, 11 Mar 2020 14:40:29 -0400 Subject: [PATCH 06/10] Implement nested RackGroups --- netbox/dcim/api/serializers.py | 3 +- netbox/dcim/filters.py | 10 +++++ netbox/dcim/forms.py | 38 ++++++++++++++-- .../dcim/migrations/0101_nested_rackgroups.py | 43 ++++++++++++++++++ .../0102_nested_rackgroups_rebuild.py | 21 +++++++++ netbox/dcim/models/__init__.py | 31 ++++++++++++- netbox/dcim/tables.py | 11 +++-- netbox/dcim/tests/test_api.py | 29 +++++++----- netbox/dcim/tests/test_filters.py | 44 ++++++++++++++----- netbox/dcim/tests/test_views.py | 12 +++-- netbox/dcim/views.py | 8 +++- 11 files changed, 213 insertions(+), 37 deletions(-) create mode 100644 netbox/dcim/migrations/0101_nested_rackgroups.py create mode 100644 netbox/dcim/migrations/0102_nested_rackgroups_rebuild.py diff --git a/netbox/dcim/api/serializers.py b/netbox/dcim/api/serializers.py index 5483904f5..85ff1895c 100644 --- a/netbox/dcim/api/serializers.py +++ b/netbox/dcim/api/serializers.py @@ -96,11 +96,12 @@ class SiteSerializer(TaggitSerializer, CustomFieldModelSerializer): class RackGroupSerializer(ValidatedModelSerializer): site = NestedSiteSerializer() + parent = NestedRackGroupSerializer(required=False, allow_null=True) rack_count = serializers.IntegerField(read_only=True) class Meta: model = RackGroup - fields = ['id', 'name', 'slug', 'site', 'rack_count'] + fields = ['id', 'name', 'slug', 'site', 'parent', 'rack_count'] class RackRoleSerializer(ValidatedModelSerializer): diff --git a/netbox/dcim/filters.py b/netbox/dcim/filters.py index a268e3114..3a21beb47 100644 --- a/netbox/dcim/filters.py +++ b/netbox/dcim/filters.py @@ -153,6 +153,16 @@ class RackGroupFilterSet(BaseFilterSet, NameSlugSearchFilterSet): to_field_name='slug', label='Site (slug)', ) + parent_id = django_filters.ModelMultipleChoiceFilter( + queryset=RackGroup.objects.all(), + label='Rack group (ID)', + ) + parent = django_filters.ModelMultipleChoiceFilter( + field_name='parent__slug', + queryset=RackGroup.objects.all(), + to_field_name='slug', + label='Rack group (slug)', + ) class Meta: model = RackGroup diff --git a/netbox/dcim/forms.py b/netbox/dcim/forms.py index 88d72eeda..1a6d60b86 100644 --- a/netbox/dcim/forms.py +++ b/netbox/dcim/forms.py @@ -386,7 +386,17 @@ class RackGroupForm(BootstrapMixin, forms.ModelForm): site = DynamicModelChoiceField( queryset=Site.objects.all(), widget=APISelect( - api_url="/api/dcim/sites/" + api_url="/api/dcim/sites/", + filter_for={ + 'parent': 'site_id', + } + ) + ) + parent = DynamicModelChoiceField( + queryset=RackGroup.objects.all(), + required=False, + widget=APISelect( + api_url="/api/dcim/rack-groups/" ) ) slug = SlugField() @@ -394,7 +404,7 @@ class RackGroupForm(BootstrapMixin, forms.ModelForm): class Meta: model = RackGroup fields = ( - 'site', 'name', 'slug', + 'site', 'parent', 'name', 'slug', ) @@ -407,6 +417,15 @@ class RackGroupCSVForm(forms.ModelForm): 'invalid_choice': 'Site not found.', } ) + parent = forms.ModelChoiceField( + queryset=RackGroup.objects.all(), + required=False, + to_field_name='name', + help_text='Name of parent rack group', + error_messages={ + 'invalid_choice': 'Rack group not found.', + } + ) class Meta: model = RackGroup @@ -426,7 +445,8 @@ class RackGroupFilterForm(BootstrapMixin, forms.Form): api_url="/api/dcim/regions/", value_field="slug", filter_for={ - 'site': 'region' + 'site': 'region', + 'parent': 'region', } ) ) @@ -437,6 +457,18 @@ class RackGroupFilterForm(BootstrapMixin, forms.Form): widget=APISelectMultiple( api_url="/api/dcim/sites/", value_field="slug", + filter_for={ + 'parent': 'site', + } + ) + ) + parent = DynamicModelMultipleChoiceField( + queryset=RackGroup.objects.all(), + to_field_name='slug', + required=False, + widget=APISelectMultiple( + api_url="/api/dcim/rack-groups/", + value_field="slug", ) ) diff --git a/netbox/dcim/migrations/0101_nested_rackgroups.py b/netbox/dcim/migrations/0101_nested_rackgroups.py new file mode 100644 index 000000000..dd5f8af26 --- /dev/null +++ b/netbox/dcim/migrations/0101_nested_rackgroups.py @@ -0,0 +1,43 @@ +from django.db import migrations, models +import django.db.models.deletion +import mptt.fields + + +class Migration(migrations.Migration): + + dependencies = [ + ('dcim', '0100_mptt_remove_indexes'), + ] + + operations = [ + migrations.AddField( + model_name='rackgroup', + name='parent', + field=mptt.fields.TreeForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='children', to='dcim.RackGroup'), + ), + migrations.AddField( + model_name='rackgroup', + name='level', + field=models.PositiveIntegerField(default=0, editable=False), + preserve_default=False, + ), + migrations.AddField( + model_name='rackgroup', + name='lft', + field=models.PositiveIntegerField(default=1, editable=False), + preserve_default=False, + ), + migrations.AddField( + model_name='rackgroup', + name='rght', + field=models.PositiveIntegerField(default=2, editable=False), + preserve_default=False, + ), + # tree_id will be set to a valid value during the following migration (which needs to be a separate migration) + migrations.AddField( + model_name='rackgroup', + name='tree_id', + field=models.PositiveIntegerField(db_index=True, default=0, editable=False), + preserve_default=False, + ), + ] diff --git a/netbox/dcim/migrations/0102_nested_rackgroups_rebuild.py b/netbox/dcim/migrations/0102_nested_rackgroups_rebuild.py new file mode 100644 index 000000000..411bb3abb --- /dev/null +++ b/netbox/dcim/migrations/0102_nested_rackgroups_rebuild.py @@ -0,0 +1,21 @@ +from django.db import migrations + + +def rebuild_mptt(apps, schema_editor): + RackGroup = apps.get_model('dcim', 'RackGroup') + for i, rackgroup in enumerate(RackGroup.objects.all(), start=1): + RackGroup.objects.filter(pk=rackgroup.pk).update(tree_id=i) + + +class Migration(migrations.Migration): + + dependencies = [ + ('dcim', '0101_nested_rackgroups'), + ] + + operations = [ + migrations.RunPython( + code=rebuild_mptt, + reverse_code=migrations.RunPython.noop + ), + ] diff --git a/netbox/dcim/models/__init__.py b/netbox/dcim/models/__init__.py index af785f0d2..1dbfdb76b 100644 --- a/netbox/dcim/models/__init__.py +++ b/netbox/dcim/models/__init__.py @@ -283,7 +283,7 @@ class Site(ChangeLoggedModel, CustomFieldModel): # Racks # -class RackGroup(ChangeLoggedModel): +class RackGroup(MPTTModel, ChangeLoggedModel): """ Racks can be grouped as subsets within a Site. The scope of a group will depend on how Sites are defined. For example, if a Site spans a corporate campus, a RackGroup might be defined to represent each building within that @@ -298,8 +298,16 @@ class RackGroup(ChangeLoggedModel): on_delete=models.CASCADE, related_name='rack_groups' ) + parent = TreeForeignKey( + to='self', + on_delete=models.CASCADE, + related_name='children', + blank=True, + null=True, + db_index=True + ) - csv_headers = ['site', 'name', 'slug'] + csv_headers = ['site', 'parent', 'name', 'slug'] class Meta: ordering = ['site', 'name'] @@ -308,6 +316,9 @@ class RackGroup(ChangeLoggedModel): ['site', 'slug'], ] + class MPTTMeta: + order_insertion_by = ['name'] + def __str__(self): return self.name @@ -317,10 +328,26 @@ class RackGroup(ChangeLoggedModel): def to_csv(self): return ( self.site, + self.parent.name if self.parent else '', self.name, self.slug, ) + def to_objectchange(self, action): + # Remove MPTT-internal fields + return ObjectChange( + changed_object=self, + object_repr=str(self), + action=action, + object_data=serialize_object(self, exclude=['level', 'lft', 'rght', 'tree_id']) + ) + + def clean(self): + + # Parent RackGroup (if any) must belong to the same Site + if self.parent and self.parent.site != self.site: + raise ValidationError(f"Parent rack group ({self.parent}) must belong to the same site ({self.site})") + class RackRole(ChangeLoggedModel): """ diff --git a/netbox/dcim/tables.py b/netbox/dcim/tables.py index bc91dd70c..ebda79dc0 100644 --- a/netbox/dcim/tables.py +++ b/netbox/dcim/tables.py @@ -11,13 +11,13 @@ from .models import ( VirtualChassis, ) -REGION_LINK = """ +MPTT_LINK = """ {% if record.get_children %} {% else %} {% endif %} - {{ record.name }} + {{ record.name }} """ @@ -214,7 +214,7 @@ def get_component_template_actions(model_name): class RegionTable(BaseTable): pk = ToggleColumn() - name = tables.TemplateColumn(template_code=REGION_LINK, orderable=False) + name = tables.TemplateColumn(template_code=MPTT_LINK, orderable=False) site_count = tables.Column(verbose_name='Sites') slug = tables.Column(verbose_name='Slug') actions = tables.TemplateColumn( @@ -250,7 +250,10 @@ class SiteTable(BaseTable): class RackGroupTable(BaseTable): pk = ToggleColumn() - name = tables.LinkColumn() + name = tables.TemplateColumn( + template_code=MPTT_LINK, + orderable=False + ) site = tables.LinkColumn( viewname='dcim:site', args=[Accessor('site.slug')], diff --git a/netbox/dcim/tests/test_api.py b/netbox/dcim/tests/test_api.py index a0fb442f3..ddb9c0b52 100644 --- a/netbox/dcim/tests/test_api.py +++ b/netbox/dcim/tests/test_api.py @@ -349,9 +349,11 @@ class RackGroupTest(APITestCase): self.site1 = Site.objects.create(name='Test Site 1', slug='test-site-1') self.site2 = Site.objects.create(name='Test Site 2', slug='test-site-2') - self.rackgroup1 = RackGroup.objects.create(site=self.site1, name='Test Rack Group 1', slug='test-rack-group-1') - self.rackgroup2 = RackGroup.objects.create(site=self.site1, name='Test Rack Group 2', slug='test-rack-group-2') - self.rackgroup3 = RackGroup.objects.create(site=self.site1, name='Test Rack Group 3', slug='test-rack-group-3') + self.parent_rackgroup1 = RackGroup.objects.create(site=self.site1, name='Parent Rack Group 1', slug='parent-rack-group-1') + self.parent_rackgroup2 = RackGroup.objects.create(site=self.site2, name='Parent Rack Group 2', slug='parent-rack-group-2') + self.rackgroup1 = RackGroup.objects.create(site=self.site1, name='Rack Group 1', slug='rack-group-1', parent=self.parent_rackgroup1) + self.rackgroup2 = RackGroup.objects.create(site=self.site1, name='Rack Group 2', slug='rack-group-2', parent=self.parent_rackgroup1) + self.rackgroup3 = RackGroup.objects.create(site=self.site1, name='Rack Group 3', slug='rack-group-3', parent=self.parent_rackgroup1) def test_get_rackgroup(self): @@ -365,7 +367,7 @@ class RackGroupTest(APITestCase): url = reverse('dcim-api:rackgroup-list') response = self.client.get(url, **self.header) - self.assertEqual(response.data['count'], 3) + self.assertEqual(response.data['count'], 5) def test_list_rackgroups_brief(self): @@ -380,20 +382,22 @@ class RackGroupTest(APITestCase): def test_create_rackgroup(self): data = { - 'name': 'Test Rack Group 4', - 'slug': 'test-rack-group-4', + 'name': 'Rack Group 4', + 'slug': 'rack-group-4', 'site': self.site1.pk, + 'parent': self.parent_rackgroup1.pk, } url = reverse('dcim-api:rackgroup-list') response = self.client.post(url, data, format='json', **self.header) self.assertHttpStatus(response, status.HTTP_201_CREATED) - self.assertEqual(RackGroup.objects.count(), 4) + self.assertEqual(RackGroup.objects.count(), 6) rackgroup4 = RackGroup.objects.get(pk=response.data['id']) self.assertEqual(rackgroup4.name, data['name']) self.assertEqual(rackgroup4.slug, data['slug']) self.assertEqual(rackgroup4.site_id, data['site']) + self.assertEqual(rackgroup4.parent_id, data['parent']) def test_create_rackgroup_bulk(self): @@ -402,16 +406,19 @@ class RackGroupTest(APITestCase): 'name': 'Test Rack Group 4', 'slug': 'test-rack-group-4', 'site': self.site1.pk, + 'parent': self.parent_rackgroup1.pk, }, { 'name': 'Test Rack Group 5', 'slug': 'test-rack-group-5', 'site': self.site1.pk, + 'parent': self.parent_rackgroup1.pk, }, { 'name': 'Test Rack Group 6', 'slug': 'test-rack-group-6', 'site': self.site1.pk, + 'parent': self.parent_rackgroup1.pk, }, ] @@ -419,7 +426,7 @@ class RackGroupTest(APITestCase): response = self.client.post(url, data, format='json', **self.header) self.assertHttpStatus(response, status.HTTP_201_CREATED) - self.assertEqual(RackGroup.objects.count(), 6) + self.assertEqual(RackGroup.objects.count(), 8) self.assertEqual(response.data[0]['name'], data[0]['name']) self.assertEqual(response.data[1]['name'], data[1]['name']) self.assertEqual(response.data[2]['name'], data[2]['name']) @@ -430,17 +437,19 @@ class RackGroupTest(APITestCase): 'name': 'Test Rack Group X', 'slug': 'test-rack-group-x', 'site': self.site2.pk, + 'parent': self.parent_rackgroup2.pk, } url = reverse('dcim-api:rackgroup-detail', kwargs={'pk': self.rackgroup1.pk}) response = self.client.put(url, data, format='json', **self.header) self.assertHttpStatus(response, status.HTTP_200_OK) - self.assertEqual(RackGroup.objects.count(), 3) + self.assertEqual(RackGroup.objects.count(), 5) rackgroup1 = RackGroup.objects.get(pk=response.data['id']) self.assertEqual(rackgroup1.name, data['name']) self.assertEqual(rackgroup1.slug, data['slug']) self.assertEqual(rackgroup1.site_id, data['site']) + self.assertEqual(rackgroup1.parent_id, data['parent']) def test_delete_rackgroup(self): @@ -448,7 +457,7 @@ class RackGroupTest(APITestCase): response = self.client.delete(url, **self.header) self.assertHttpStatus(response, status.HTTP_204_NO_CONTENT) - self.assertEqual(RackGroup.objects.count(), 2) + self.assertEqual(RackGroup.objects.count(), 4) class RackRoleTest(APITestCase): diff --git a/netbox/dcim/tests/test_filters.py b/netbox/dcim/tests/test_filters.py index 02de313fc..edd366a58 100644 --- a/netbox/dcim/tests/test_filters.py +++ b/netbox/dcim/tests/test_filters.py @@ -186,12 +186,21 @@ class RackGroupTestCase(TestCase): ) Site.objects.bulk_create(sites) - rack_groups = ( - RackGroup(name='Rack Group 1', slug='rack-group-1', site=sites[0]), - RackGroup(name='Rack Group 2', slug='rack-group-2', site=sites[1]), - RackGroup(name='Rack Group 3', slug='rack-group-3', site=sites[2]), + parent_rack_groups = ( + RackGroup(name='Parent Rack Group 1', slug='parent-rack-group-1', site=sites[0]), + RackGroup(name='Parent Rack Group 2', slug='parent-rack-group-2', site=sites[1]), + RackGroup(name='Parent Rack Group 3', slug='parent-rack-group-3', site=sites[2]), ) - RackGroup.objects.bulk_create(rack_groups) + for rackgroup in parent_rack_groups: + rackgroup.save() + + rack_groups = ( + RackGroup(name='Rack Group 1', slug='rack-group-1', site=sites[0], parent=parent_rack_groups[0]), + RackGroup(name='Rack Group 2', slug='rack-group-2', site=sites[1], parent=parent_rack_groups[1]), + RackGroup(name='Rack Group 3', slug='rack-group-3', site=sites[2], parent=parent_rack_groups[2]), + ) + for rackgroup in rack_groups: + rackgroup.save() def test_id(self): id_list = self.queryset.values_list('id', flat=True)[:2] @@ -209,15 +218,22 @@ class RackGroupTestCase(TestCase): def test_region(self): regions = Region.objects.all()[:2] params = {'region_id': [regions[0].pk, regions[1].pk]} - self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) + self.assertEqual(self.filterset(params, self.queryset).qs.count(), 4) params = {'region': [regions[0].slug, regions[1].slug]} - self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) + self.assertEqual(self.filterset(params, self.queryset).qs.count(), 4) def test_site(self): sites = Site.objects.all()[:2] params = {'site_id': [sites[0].pk, sites[1].pk]} - self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) + self.assertEqual(self.filterset(params, self.queryset).qs.count(), 4) params = {'site': [sites[0].slug, sites[1].slug]} + self.assertEqual(self.filterset(params, self.queryset).qs.count(), 4) + + def test_parent(self): + parent_groups = RackGroup.objects.filter(name__startswith='Parent')[:2] + params = {'parent_id': [parent_groups[0].pk, parent_groups[1].pk]} + self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) + params = {'parent': [parent_groups[0].slug, parent_groups[1].slug]} self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) @@ -280,7 +296,8 @@ class RackTestCase(TestCase): RackGroup(name='Rack Group 2', slug='rack-group-2', site=sites[1]), RackGroup(name='Rack Group 3', slug='rack-group-3', site=sites[2]), ) - RackGroup.objects.bulk_create(rack_groups) + for rackgroup in rack_groups: + rackgroup.save() rack_roles = ( RackRole(name='Rack Role 1', slug='rack-role-1'), @@ -432,7 +449,8 @@ class RackReservationTestCase(TestCase): RackGroup(name='Rack Group 2', slug='rack-group-2', site=sites[1]), RackGroup(name='Rack Group 3', slug='rack-group-3', site=sites[2]), ) - RackGroup.objects.bulk_create(rack_groups) + for rackgroup in rack_groups: + rackgroup.save() racks = ( Rack(name='Rack 1', site=sites[0], group=rack_groups[0]), @@ -1146,7 +1164,8 @@ class DeviceTestCase(TestCase): RackGroup(name='Rack Group 2', slug='rack-group-2', site=sites[1]), RackGroup(name='Rack Group 3', slug='rack-group-3', site=sites[2]), ) - RackGroup.objects.bulk_create(rack_groups) + for rackgroup in rack_groups: + rackgroup.save() racks = ( Rack(name='Rack 1', site=sites[0], group=rack_groups[0]), @@ -2559,7 +2578,8 @@ class PowerPanelTestCase(TestCase): RackGroup(name='Rack Group 2', slug='rack-group-2', site=sites[1]), RackGroup(name='Rack Group 3', slug='rack-group-3', site=sites[2]), ) - RackGroup.objects.bulk_create(rack_groups) + for rackgroup in rack_groups: + rackgroup.save() power_panels = ( PowerPanel(name='Power Panel 1', site=sites[0], rack_group=rack_groups[0]), diff --git a/netbox/dcim/tests/test_views.py b/netbox/dcim/tests/test_views.py index 2263a472b..d7434c808 100644 --- a/netbox/dcim/tests/test_views.py +++ b/netbox/dcim/tests/test_views.py @@ -122,11 +122,13 @@ class RackGroupTestCase(ViewTestCases.OrganizationalObjectViewTestCase): site = Site(name='Site 1', slug='site-1') site.save() - RackGroup.objects.bulk_create([ + rack_groups = ( RackGroup(name='Rack Group 1', slug='rack-group-1', site=site), RackGroup(name='Rack Group 2', slug='rack-group-2', site=site), RackGroup(name='Rack Group 3', slug='rack-group-3', site=site), - ]) + ) + for rackgroup in rack_groups: + rackgroup.save() cls.form_data = { 'name': 'Rack Group X', @@ -231,7 +233,8 @@ class RackTestCase(ViewTestCases.PrimaryObjectViewTestCase): RackGroup(name='Rack Group 1', slug='rack-group-1', site=sites[0]), RackGroup(name='Rack Group 2', slug='rack-group-2', site=sites[1]) ) - RackGroup.objects.bulk_create(rackgroups) + for rackgroup in rackgroups: + rackgroup.save() rackroles = ( RackRole(name='Rack Role 1', slug='rack-role-1'), @@ -1570,7 +1573,8 @@ class PowerPanelTestCase(ViewTestCases.PrimaryObjectViewTestCase): RackGroup(name='Rack Group 1', slug='rack-group-1', site=sites[0]), RackGroup(name='Rack Group 2', slug='rack-group-2', site=sites[1]), ) - RackGroup.objects.bulk_create(rackgroups) + for rackgroup in rackgroups: + rackgroup.save() PowerPanel.objects.bulk_create(( PowerPanel(site=sites[0], rack_group=rackgroups[0], name='Power Panel 1'), diff --git a/netbox/dcim/views.py b/netbox/dcim/views.py index a639849ba..1e5ae9772 100644 --- a/netbox/dcim/views.py +++ b/netbox/dcim/views.py @@ -266,7 +266,13 @@ class SiteBulkDeleteView(PermissionRequiredMixin, BulkDeleteView): class RackGroupListView(PermissionRequiredMixin, ObjectListView): permission_required = 'dcim.view_rackgroup' - queryset = RackGroup.objects.prefetch_related('site').annotate(rack_count=Count('racks')) + queryset = RackGroup.objects.add_related_count( + RackGroup.objects.all(), + Rack, + 'group', + 'rack_count', + cumulative=True + ).prefetch_related('site') filterset = filters.RackGroupFilterSet filterset_form = forms.RackGroupFilterForm table = tables.RackGroupTable From c42613cf4d727cb3f0780ec6fd0921ee3097148b Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Wed, 11 Mar 2020 14:57:48 -0400 Subject: [PATCH 07/10] Update filter fields --- netbox/dcim/filters.py | 37 ++++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/netbox/dcim/filters.py b/netbox/dcim/filters.py index 3a21beb47..e4ddf792b 100644 --- a/netbox/dcim/filters.py +++ b/netbox/dcim/filters.py @@ -204,15 +204,18 @@ class RackFilterSet(BaseFilterSet, TenancyFilterSet, CustomFieldFilterSet, Creat to_field_name='slug', label='Site (slug)', ) - group_id = django_filters.ModelMultipleChoiceFilter( + group_id = TreeNodeMultipleChoiceFilter( queryset=RackGroup.objects.all(), - label='Group (ID)', + field_name='group', + lookup_expr='in', + label='Rack group (ID)', ) - group = django_filters.ModelMultipleChoiceFilter( - field_name='group__slug', + group = TreeNodeMultipleChoiceFilter( queryset=RackGroup.objects.all(), + field_name='group', + lookup_expr='in', to_field_name='slug', - label='Group', + label='Rack group (slug)', ) status = django_filters.MultipleChoiceFilter( choices=RackStatusChoices, @@ -272,16 +275,18 @@ class RackReservationFilterSet(BaseFilterSet, TenancyFilterSet): to_field_name='slug', label='Site (slug)', ) - group_id = django_filters.ModelMultipleChoiceFilter( + group_id = TreeNodeMultipleChoiceFilter( + queryset=RackGroup.objects.all(), field_name='rack__group', - queryset=RackGroup.objects.all(), - label='Group (ID)', + lookup_expr='in', + label='Rack group (ID)', ) - group = django_filters.ModelMultipleChoiceFilter( - field_name='rack__group__slug', + group = TreeNodeMultipleChoiceFilter( queryset=RackGroup.objects.all(), + field_name='rack__group', + lookup_expr='in', to_field_name='slug', - label='Group', + label='Rack group (slug)', ) user_id = django_filters.ModelMultipleChoiceFilter( queryset=User.objects.all(), @@ -561,9 +566,10 @@ class DeviceFilterSet( to_field_name='slug', label='Site name (slug)', ) - rack_group_id = django_filters.ModelMultipleChoiceFilter( - field_name='rack__group', + rack_group_id = TreeNodeMultipleChoiceFilter( queryset=RackGroup.objects.all(), + field_name='rack__group', + lookup_expr='in', label='Rack group (ID)', ) rack_id = django_filters.ModelMultipleChoiceFilter( @@ -1253,9 +1259,10 @@ class PowerPanelFilterSet(BaseFilterSet): to_field_name='slug', label='Site name (slug)', ) - rack_group_id = django_filters.ModelMultipleChoiceFilter( - field_name='rack_group', + rack_group_id = TreeNodeMultipleChoiceFilter( queryset=RackGroup.objects.all(), + field_name='rack_group', + lookup_expr='in', label='Rack group (ID)', ) From 45f6ea211d107652393b6f9a0d12709e3e2b7b82 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Wed, 11 Mar 2020 20:20:04 -0400 Subject: [PATCH 08/10] Implement support for nested TenantGroups --- netbox/tenancy/api/serializers.py | 3 +- netbox/tenancy/filters.py | 38 ++++++++++------ netbox/tenancy/forms.py | 18 +++++++- .../migrations/0007_nested_tenantgroups.py | 43 +++++++++++++++++++ .../0008_nested_tenantgroups_rebuild.py | 21 +++++++++ netbox/tenancy/models.py | 29 +++++++++++-- netbox/tenancy/tables.py | 25 +++++++++-- netbox/tenancy/views.py | 8 +++- 8 files changed, 163 insertions(+), 22 deletions(-) create mode 100644 netbox/tenancy/migrations/0007_nested_tenantgroups.py create mode 100644 netbox/tenancy/migrations/0008_nested_tenantgroups_rebuild.py diff --git a/netbox/tenancy/api/serializers.py b/netbox/tenancy/api/serializers.py index 7599029c5..ec5e60a34 100644 --- a/netbox/tenancy/api/serializers.py +++ b/netbox/tenancy/api/serializers.py @@ -12,11 +12,12 @@ from .nested_serializers import * # class TenantGroupSerializer(ValidatedModelSerializer): + parent = NestedTenantGroupSerializer(required=False, allow_null=True) tenant_count = serializers.IntegerField(read_only=True) class Meta: model = TenantGroup - fields = ['id', 'name', 'slug', 'tenant_count'] + fields = ['id', 'name', 'slug', 'parent', 'tenant_count'] class TenantSerializer(TaggitSerializer, CustomFieldModelSerializer): diff --git a/netbox/tenancy/filters.py b/netbox/tenancy/filters.py index dc1635392..12e852879 100644 --- a/netbox/tenancy/filters.py +++ b/netbox/tenancy/filters.py @@ -2,7 +2,7 @@ import django_filters from django.db.models import Q from extras.filters import CustomFieldFilterSet, CreatedUpdatedFilterSet -from utilities.filters import BaseFilterSet, NameSlugSearchFilterSet, TagFilter +from utilities.filters import BaseFilterSet, NameSlugSearchFilterSet, TagFilter, TreeNodeMultipleChoiceFilter from .models import Tenant, TenantGroup @@ -14,6 +14,16 @@ __all__ = ( class TenantGroupFilterSet(BaseFilterSet, NameSlugSearchFilterSet): + parent_id = django_filters.ModelMultipleChoiceFilter( + queryset=TenantGroup.objects.all(), + label='Tenant group (ID)', + ) + parent = django_filters.ModelMultipleChoiceFilter( + field_name='parent__slug', + queryset=TenantGroup.objects.all(), + to_field_name='slug', + label='Tenant group group (slug)', + ) class Meta: model = TenantGroup @@ -25,15 +35,18 @@ class TenantFilterSet(BaseFilterSet, CustomFieldFilterSet, CreatedUpdatedFilterS method='search', label='Search', ) - group_id = django_filters.ModelMultipleChoiceFilter( + group_id = TreeNodeMultipleChoiceFilter( queryset=TenantGroup.objects.all(), - label='Group (ID)', + field_name='group', + lookup_expr='in', + label='Tenant group (ID)', ) - group = django_filters.ModelMultipleChoiceFilter( - field_name='group__slug', + group = TreeNodeMultipleChoiceFilter( queryset=TenantGroup.objects.all(), + field_name='group', + lookup_expr='in', to_field_name='slug', - label='Group (slug)', + label='Tenant group (slug)', ) tag = TagFilter() @@ -56,16 +69,17 @@ class TenancyFilterSet(django_filters.FilterSet): """ An inheritable FilterSet for models which support Tenant assignment. """ - tenant_group_id = django_filters.ModelMultipleChoiceFilter( - field_name='tenant__group__id', + tenant_group_id = TreeNodeMultipleChoiceFilter( queryset=TenantGroup.objects.all(), - to_field_name='id', + field_name='tenant__group', + lookup_expr='in', label='Tenant Group (ID)', ) - tenant_group = django_filters.ModelMultipleChoiceFilter( - field_name='tenant__group__slug', + tenant_group = TreeNodeMultipleChoiceFilter( queryset=TenantGroup.objects.all(), + field_name='tenant__group', to_field_name='slug', + lookup_expr='in', label='Tenant Group (slug)', ) tenant_id = django_filters.ModelMultipleChoiceFilter( @@ -73,8 +87,8 @@ class TenancyFilterSet(django_filters.FilterSet): label='Tenant (ID)', ) tenant = django_filters.ModelMultipleChoiceFilter( - field_name='tenant__slug', queryset=Tenant.objects.all(), + field_name='tenant__slug', to_field_name='slug', label='Tenant (slug)', ) diff --git a/netbox/tenancy/forms.py b/netbox/tenancy/forms.py index 5b828b661..9b8fc59da 100644 --- a/netbox/tenancy/forms.py +++ b/netbox/tenancy/forms.py @@ -16,16 +16,32 @@ from .models import Tenant, TenantGroup # class TenantGroupForm(BootstrapMixin, forms.ModelForm): + parent = DynamicModelChoiceField( + queryset=TenantGroup.objects.all(), + required=False, + widget=APISelect( + api_url="/api/tenancy/tenant-groups/" + ) + ) slug = SlugField() class Meta: model = TenantGroup fields = [ - 'name', 'slug', + 'parent', 'name', 'slug', ] class TenantGroupCSVForm(forms.ModelForm): + parent = forms.ModelChoiceField( + queryset=TenantGroup.objects.all(), + required=False, + to_field_name='name', + help_text='Name of parent tenant group', + error_messages={ + 'invalid_choice': 'Tenant group not found.', + } + ) slug = SlugField() class Meta: diff --git a/netbox/tenancy/migrations/0007_nested_tenantgroups.py b/netbox/tenancy/migrations/0007_nested_tenantgroups.py new file mode 100644 index 000000000..4278b3409 --- /dev/null +++ b/netbox/tenancy/migrations/0007_nested_tenantgroups.py @@ -0,0 +1,43 @@ +from django.db import migrations, models +import django.db.models.deletion +import mptt.fields + + +class Migration(migrations.Migration): + + dependencies = [ + ('tenancy', '0006_custom_tag_models'), + ] + + operations = [ + migrations.AddField( + model_name='tenantgroup', + name='parent', + field=mptt.fields.TreeForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='children', to='tenancy.TenantGroup'), + ), + migrations.AddField( + model_name='tenantgroup', + name='level', + field=models.PositiveIntegerField(default=0, editable=False), + preserve_default=False, + ), + migrations.AddField( + model_name='tenantgroup', + name='lft', + field=models.PositiveIntegerField(default=1, editable=False), + preserve_default=False, + ), + migrations.AddField( + model_name='tenantgroup', + name='rght', + field=models.PositiveIntegerField(default=2, editable=False), + preserve_default=False, + ), + # tree_id will be set to a valid value during the following migration (which needs to be a separate migration) + migrations.AddField( + model_name='tenantgroup', + name='tree_id', + field=models.PositiveIntegerField(db_index=True, default=0, editable=False), + preserve_default=False, + ), + ] diff --git a/netbox/tenancy/migrations/0008_nested_tenantgroups_rebuild.py b/netbox/tenancy/migrations/0008_nested_tenantgroups_rebuild.py new file mode 100644 index 000000000..e31a75d36 --- /dev/null +++ b/netbox/tenancy/migrations/0008_nested_tenantgroups_rebuild.py @@ -0,0 +1,21 @@ +from django.db import migrations + + +def rebuild_mptt(apps, schema_editor): + TenantGroup = apps.get_model('tenancy', 'TenantGroup') + for i, tenantgroup in enumerate(TenantGroup.objects.all(), start=1): + TenantGroup.objects.filter(pk=tenantgroup.pk).update(tree_id=i) + + +class Migration(migrations.Migration): + + dependencies = [ + ('tenancy', '0007_nested_tenantgroups'), + ] + + operations = [ + migrations.RunPython( + code=rebuild_mptt, + reverse_code=migrations.RunPython.noop + ), + ] diff --git a/netbox/tenancy/models.py b/netbox/tenancy/models.py index 9fa7f23ea..1a02184cd 100644 --- a/netbox/tenancy/models.py +++ b/netbox/tenancy/models.py @@ -1,10 +1,12 @@ from django.contrib.contenttypes.fields import GenericRelation from django.db import models from django.urls import reverse +from mptt.models import MPTTModel, TreeForeignKey from taggit.managers import TaggableManager -from extras.models import CustomFieldModel, TaggedItem +from extras.models import CustomFieldModel, ObjectChange, TaggedItem from utilities.models import ChangeLoggedModel +from utilities.utils import serialize_object __all__ = ( @@ -13,7 +15,7 @@ __all__ = ( ) -class TenantGroup(ChangeLoggedModel): +class TenantGroup(MPTTModel, ChangeLoggedModel): """ An arbitrary collection of Tenants. """ @@ -24,12 +26,23 @@ class TenantGroup(ChangeLoggedModel): slug = models.SlugField( unique=True ) + parent = TreeForeignKey( + to='self', + on_delete=models.CASCADE, + related_name='children', + blank=True, + null=True, + db_index=True + ) - csv_headers = ['name', 'slug'] + csv_headers = ['name', 'slug', 'parent'] class Meta: ordering = ['name'] + class MPTTMeta: + order_insertion_by = ['name'] + def __str__(self): return self.name @@ -40,6 +53,16 @@ class TenantGroup(ChangeLoggedModel): return ( self.name, self.slug, + self.parent.name if self.parent else '', + ) + + def to_objectchange(self, action): + # Remove MPTT-internal fields + return ObjectChange( + changed_object=self, + object_repr=str(self), + action=action, + object_data=serialize_object(self, exclude=['level', 'lft', 'rght', 'tree_id']) ) diff --git a/netbox/tenancy/tables.py b/netbox/tenancy/tables.py index af4fb34c0..adf73dc41 100644 --- a/netbox/tenancy/tables.py +++ b/netbox/tenancy/tables.py @@ -3,6 +3,16 @@ import django_tables2 as tables from utilities.tables import BaseTable, ToggleColumn from .models import Tenant, TenantGroup +MPTT_LINK = """ +{% if record.get_children %} + +{% else %} + +{% endif %} + {{ record.name }} + +""" + TENANTGROUP_ACTIONS = """ @@ -27,11 +37,18 @@ COL_TENANT = """ class TenantGroupTable(BaseTable): pk = ToggleColumn() - name = tables.LinkColumn(verbose_name='Name') - tenant_count = tables.Column(verbose_name='Tenants') - slug = tables.Column(verbose_name='Slug') + name = tables.TemplateColumn( + template_code=MPTT_LINK, + orderable=False + ) + tenant_count = tables.Column( + verbose_name='Tenants' + ) + slug = tables.Column() actions = tables.TemplateColumn( - template_code=TENANTGROUP_ACTIONS, attrs={'td': {'class': 'text-right noprint'}}, verbose_name='' + template_code=TENANTGROUP_ACTIONS, + attrs={'td': {'class': 'text-right noprint'}}, + verbose_name='' ) class Meta(BaseTable.Meta): diff --git a/netbox/tenancy/views.py b/netbox/tenancy/views.py index 0319a20b0..afc363cd6 100644 --- a/netbox/tenancy/views.py +++ b/netbox/tenancy/views.py @@ -20,7 +20,13 @@ from .models import Tenant, TenantGroup class TenantGroupListView(PermissionRequiredMixin, ObjectListView): permission_required = 'tenancy.view_tenantgroup' - queryset = TenantGroup.objects.annotate(tenant_count=Count('tenants')) + queryset = TenantGroup.objects.add_related_count( + TenantGroup.objects.all(), + Tenant, + 'group', + 'tenant_count', + cumulative=True + ) table = tables.TenantGroupTable From b5d57262f9c5feede98123aff2f9c46c9085c45e Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Wed, 11 Mar 2020 21:14:53 -0400 Subject: [PATCH 09/10] Update tests for nested TenantGroups --- netbox/circuits/tests/test_filters.py | 3 +- netbox/dcim/tests/test_filters.py | 12 ++- netbox/extras/tests/test_api.py | 6 +- netbox/extras/tests/test_filters.py | 3 +- netbox/ipam/tests/test_filters.py | 12 ++- netbox/tenancy/tests/test_api.py | 92 +++++++++++++-------- netbox/tenancy/tests/test_filters.py | 37 ++++++--- netbox/tenancy/tests/test_views.py | 21 +++-- netbox/virtualization/tests/test_filters.py | 6 +- 9 files changed, 126 insertions(+), 66 deletions(-) diff --git a/netbox/circuits/tests/test_filters.py b/netbox/circuits/tests/test_filters.py index 8ce679501..346d2ad80 100644 --- a/netbox/circuits/tests/test_filters.py +++ b/netbox/circuits/tests/test_filters.py @@ -139,7 +139,8 @@ class CircuitTestCase(TestCase): TenantGroup(name='Tenant group 2', slug='tenant-group-2'), TenantGroup(name='Tenant group 3', slug='tenant-group-3'), ) - TenantGroup.objects.bulk_create(tenant_groups) + for tenantgroup in tenant_groups: + tenantgroup.save() tenants = ( Tenant(name='Tenant 1', slug='tenant-1', group=tenant_groups[0]), diff --git a/netbox/dcim/tests/test_filters.py b/netbox/dcim/tests/test_filters.py index edd366a58..3986d0892 100644 --- a/netbox/dcim/tests/test_filters.py +++ b/netbox/dcim/tests/test_filters.py @@ -81,7 +81,8 @@ class SiteTestCase(TestCase): TenantGroup(name='Tenant group 2', slug='tenant-group-2'), TenantGroup(name='Tenant group 3', slug='tenant-group-3'), ) - TenantGroup.objects.bulk_create(tenant_groups) + for tenantgroup in tenant_groups: + tenantgroup.save() tenants = ( Tenant(name='Tenant 1', slug='tenant-1', group=tenant_groups[0]), @@ -311,7 +312,8 @@ class RackTestCase(TestCase): TenantGroup(name='Tenant group 2', slug='tenant-group-2'), TenantGroup(name='Tenant group 3', slug='tenant-group-3'), ) - TenantGroup.objects.bulk_create(tenant_groups) + for tenantgroup in tenant_groups: + tenantgroup.save() tenants = ( Tenant(name='Tenant 1', slug='tenant-1', group=tenant_groups[0]), @@ -471,7 +473,8 @@ class RackReservationTestCase(TestCase): TenantGroup(name='Tenant group 2', slug='tenant-group-2'), TenantGroup(name='Tenant group 3', slug='tenant-group-3'), ) - TenantGroup.objects.bulk_create(tenant_groups) + for tenantgroup in tenant_groups: + tenantgroup.save() tenants = ( Tenant(name='Tenant 1', slug='tenant-1', group=tenant_groups[0]), @@ -1187,7 +1190,8 @@ class DeviceTestCase(TestCase): TenantGroup(name='Tenant group 2', slug='tenant-group-2'), TenantGroup(name='Tenant group 3', slug='tenant-group-3'), ) - TenantGroup.objects.bulk_create(tenant_groups) + for tenantgroup in tenant_groups: + tenantgroup.save() tenants = ( Tenant(name='Tenant 1', slug='tenant-1', group=tenant_groups[0]), diff --git a/netbox/extras/tests/test_api.py b/netbox/extras/tests/test_api.py index abc2c684f..6871b2654 100644 --- a/netbox/extras/tests/test_api.py +++ b/netbox/extras/tests/test_api.py @@ -402,8 +402,10 @@ class ConfigContextTest(APITestCase): role2 = DeviceRole.objects.create(name='Test Role 2', slug='test-role-2') platform1 = Platform.objects.create(name='Test Platform 1', slug='test-platform-1') platform2 = Platform.objects.create(name='Test Platform 2', slug='test-platform-2') - tenantgroup1 = TenantGroup.objects.create(name='Test Tenant Group 1', slug='test-tenant-group-1') - tenantgroup2 = TenantGroup.objects.create(name='Test Tenant Group 2', slug='test-tenant-group-2') + tenantgroup1 = TenantGroup(name='Test Tenant Group 1', slug='test-tenant-group-1') + tenantgroup1.save() + tenantgroup2 = TenantGroup(name='Test Tenant Group 2', slug='test-tenant-group-2') + tenantgroup2.save() tenant1 = Tenant.objects.create(name='Test Tenant 1', slug='test-tenant-1') tenant2 = Tenant.objects.create(name='Test Tenant 2', slug='test-tenant-2') tag1 = Tag.objects.create(name='Test Tag 1', slug='test-tag-1') diff --git a/netbox/extras/tests/test_filters.py b/netbox/extras/tests/test_filters.py index ab559cf73..126414cfd 100644 --- a/netbox/extras/tests/test_filters.py +++ b/netbox/extras/tests/test_filters.py @@ -128,7 +128,8 @@ class ConfigContextTestCase(TestCase): TenantGroup(name='Tenant Group 2', slug='tenant-group-2'), TenantGroup(name='Tenant Group 3', slug='tenant-group-3'), ) - TenantGroup.objects.bulk_create(tenant_groups) + for tenantgroup in tenant_groups: + tenantgroup.save() tenants = ( Tenant(name='Tenant 1', slug='tenant-1'), diff --git a/netbox/ipam/tests/test_filters.py b/netbox/ipam/tests/test_filters.py index 333b80bc8..5bfbb30d9 100644 --- a/netbox/ipam/tests/test_filters.py +++ b/netbox/ipam/tests/test_filters.py @@ -20,7 +20,8 @@ class VRFTestCase(TestCase): TenantGroup(name='Tenant group 2', slug='tenant-group-2'), TenantGroup(name='Tenant group 3', slug='tenant-group-3'), ) - TenantGroup.objects.bulk_create(tenant_groups) + for tenantgroup in tenant_groups: + tenantgroup.save() tenants = ( Tenant(name='Tenant 1', slug='tenant-1', group=tenant_groups[0]), @@ -222,7 +223,8 @@ class PrefixTestCase(TestCase): TenantGroup(name='Tenant group 2', slug='tenant-group-2'), TenantGroup(name='Tenant group 3', slug='tenant-group-3'), ) - TenantGroup.objects.bulk_create(tenant_groups) + for tenantgroup in tenant_groups: + tenantgroup.save() tenants = ( Tenant(name='Tenant 1', slug='tenant-1', group=tenant_groups[0]), @@ -379,7 +381,8 @@ class IPAddressTestCase(TestCase): TenantGroup(name='Tenant group 2', slug='tenant-group-2'), TenantGroup(name='Tenant group 3', slug='tenant-group-3'), ) - TenantGroup.objects.bulk_create(tenant_groups) + for tenantgroup in tenant_groups: + tenantgroup.save() tenants = ( Tenant(name='Tenant 1', slug='tenant-1', group=tenant_groups[0]), @@ -593,7 +596,8 @@ class VLANTestCase(TestCase): TenantGroup(name='Tenant group 2', slug='tenant-group-2'), TenantGroup(name='Tenant group 3', slug='tenant-group-3'), ) - TenantGroup.objects.bulk_create(tenant_groups) + for tenantgroup in tenant_groups: + tenantgroup.save() tenants = ( Tenant(name='Tenant 1', slug='tenant-1', group=tenant_groups[0]), diff --git a/netbox/tenancy/tests/test_api.py b/netbox/tenancy/tests/test_api.py index 495cb250d..1767c8f28 100644 --- a/netbox/tenancy/tests/test_api.py +++ b/netbox/tenancy/tests/test_api.py @@ -28,23 +28,34 @@ class TenantGroupTest(APITestCase): super().setUp() - self.tenantgroup1 = TenantGroup.objects.create(name='Test Tenant Group 1', slug='test-tenant-group-1') - self.tenantgroup2 = TenantGroup.objects.create(name='Test Tenant Group 2', slug='test-tenant-group-2') - self.tenantgroup3 = TenantGroup.objects.create(name='Test Tenant Group 3', slug='test-tenant-group-3') + self.parent_tenant_groups = ( + TenantGroup(name='Parent Tenant Group 1', slug='parent-tenant-group-1'), + TenantGroup(name='Parent Tenant Group 2', slug='parent-tenant-group-2'), + ) + for tenantgroup in self.parent_tenant_groups: + tenantgroup.save() + + self.tenant_groups = ( + TenantGroup(name='Tenant Group 1', slug='tenant-group-1', parent=self.parent_tenant_groups[0]), + TenantGroup(name='Tenant Group 2', slug='tenant-group-2', parent=self.parent_tenant_groups[0]), + TenantGroup(name='Tenant Group 3', slug='tenant-group-3', parent=self.parent_tenant_groups[0]), + ) + for tenantgroup in self.tenant_groups: + tenantgroup.save() def test_get_tenantgroup(self): - url = reverse('tenancy-api:tenantgroup-detail', kwargs={'pk': self.tenantgroup1.pk}) + url = reverse('tenancy-api:tenantgroup-detail', kwargs={'pk': self.tenant_groups[0].pk}) response = self.client.get(url, **self.header) - self.assertEqual(response.data['name'], self.tenantgroup1.name) + self.assertEqual(response.data['name'], self.tenant_groups[0].name) def test_list_tenantgroups(self): url = reverse('tenancy-api:tenantgroup-list') response = self.client.get(url, **self.header) - self.assertEqual(response.data['count'], 3) + self.assertEqual(response.data['count'], 5) def test_list_tenantgroups_brief(self): @@ -59,33 +70,38 @@ class TenantGroupTest(APITestCase): def test_create_tenantgroup(self): data = { - 'name': 'Test Tenant Group 4', - 'slug': 'test-tenant-group-4', + 'name': 'Tenant Group 4', + 'slug': 'tenant-group-4', + 'parent': self.parent_tenant_groups[0].pk, } url = reverse('tenancy-api:tenantgroup-list') response = self.client.post(url, data, format='json', **self.header) self.assertHttpStatus(response, status.HTTP_201_CREATED) - self.assertEqual(TenantGroup.objects.count(), 4) + self.assertEqual(TenantGroup.objects.count(), 6) tenantgroup4 = TenantGroup.objects.get(pk=response.data['id']) self.assertEqual(tenantgroup4.name, data['name']) self.assertEqual(tenantgroup4.slug, data['slug']) + self.assertEqual(tenantgroup4.parent_id, data['parent']) def test_create_tenantgroup_bulk(self): data = [ { - 'name': 'Test Tenant Group 4', - 'slug': 'test-tenant-group-4', + 'name': 'Tenant Group 4', + 'slug': 'tenant-group-4', + 'parent': self.parent_tenant_groups[0].pk, }, { - 'name': 'Test Tenant Group 5', - 'slug': 'test-tenant-group-5', + 'name': 'Tenant Group 5', + 'slug': 'tenant-group-5', + 'parent': self.parent_tenant_groups[0].pk, }, { - 'name': 'Test Tenant Group 6', - 'slug': 'test-tenant-group-6', + 'name': 'Tenant Group 6', + 'slug': 'tenant-group-6', + 'parent': self.parent_tenant_groups[0].pk, }, ] @@ -93,7 +109,7 @@ class TenantGroupTest(APITestCase): response = self.client.post(url, data, format='json', **self.header) self.assertHttpStatus(response, status.HTTP_201_CREATED) - self.assertEqual(TenantGroup.objects.count(), 6) + self.assertEqual(TenantGroup.objects.count(), 8) self.assertEqual(response.data[0]['name'], data[0]['name']) self.assertEqual(response.data[1]['name'], data[1]['name']) self.assertEqual(response.data[2]['name'], data[2]['name']) @@ -101,26 +117,28 @@ class TenantGroupTest(APITestCase): def test_update_tenantgroup(self): data = { - 'name': 'Test Tenant Group X', - 'slug': 'test-tenant-group-x', + 'name': 'Tenant Group X', + 'slug': 'tenant-group-x', + 'parent': self.parent_tenant_groups[1].pk, } - url = reverse('tenancy-api:tenantgroup-detail', kwargs={'pk': self.tenantgroup1.pk}) + url = reverse('tenancy-api:tenantgroup-detail', kwargs={'pk': self.tenant_groups[0].pk}) response = self.client.put(url, data, format='json', **self.header) self.assertHttpStatus(response, status.HTTP_200_OK) - self.assertEqual(TenantGroup.objects.count(), 3) + self.assertEqual(TenantGroup.objects.count(), 5) tenantgroup1 = TenantGroup.objects.get(pk=response.data['id']) self.assertEqual(tenantgroup1.name, data['name']) self.assertEqual(tenantgroup1.slug, data['slug']) + self.assertEqual(tenantgroup1.parent_id, data['parent']) def test_delete_tenantgroup(self): - url = reverse('tenancy-api:tenantgroup-detail', kwargs={'pk': self.tenantgroup1.pk}) + url = reverse('tenancy-api:tenantgroup-detail', kwargs={'pk': self.tenant_groups[0].pk}) response = self.client.delete(url, **self.header) self.assertHttpStatus(response, status.HTTP_204_NO_CONTENT) - self.assertEqual(TenantGroup.objects.count(), 2) + self.assertEqual(TenantGroup.objects.count(), 4) class TenantTest(APITestCase): @@ -129,18 +147,26 @@ class TenantTest(APITestCase): super().setUp() - self.tenantgroup1 = TenantGroup.objects.create(name='Test Tenant Group 1', slug='test-tenant-group-1') - self.tenantgroup2 = TenantGroup.objects.create(name='Test Tenant Group 2', slug='test-tenant-group-2') - self.tenant1 = Tenant.objects.create(name='Test Tenant 1', slug='test-tenant-1', group=self.tenantgroup1) - self.tenant2 = Tenant.objects.create(name='Test Tenant 2', slug='test-tenant-2', group=self.tenantgroup1) - self.tenant3 = Tenant.objects.create(name='Test Tenant 3', slug='test-tenant-3', group=self.tenantgroup1) + self.tenant_groups = ( + TenantGroup(name='Tenant Group 1', slug='tenant-group-1'), + TenantGroup(name='Tenant Group 2', slug='tenant-group-2'), + ) + for tenantgroup in self.tenant_groups: + tenantgroup.save() + + self.tenants = ( + Tenant(name='Test Tenant 1', slug='test-tenant-1', group=self.tenant_groups[0]), + Tenant(name='Test Tenant 2', slug='test-tenant-2', group=self.tenant_groups[0]), + Tenant(name='Test Tenant 3', slug='test-tenant-3', group=self.tenant_groups[0]), + ) + Tenant.objects.bulk_create(self.tenants) def test_get_tenant(self): - url = reverse('tenancy-api:tenant-detail', kwargs={'pk': self.tenant1.pk}) + url = reverse('tenancy-api:tenant-detail', kwargs={'pk': self.tenants[0].pk}) response = self.client.get(url, **self.header) - self.assertEqual(response.data['name'], self.tenant1.name) + self.assertEqual(response.data['name'], self.tenants[0].name) def test_list_tenants(self): @@ -164,7 +190,7 @@ class TenantTest(APITestCase): data = { 'name': 'Test Tenant 4', 'slug': 'test-tenant-4', - 'group': self.tenantgroup1.pk, + 'group': self.tenant_groups[0].pk, } url = reverse('tenancy-api:tenant-list') @@ -208,10 +234,10 @@ class TenantTest(APITestCase): data = { 'name': 'Test Tenant X', 'slug': 'test-tenant-x', - 'group': self.tenantgroup2.pk, + 'group': self.tenant_groups[1].pk, } - url = reverse('tenancy-api:tenant-detail', kwargs={'pk': self.tenant1.pk}) + url = reverse('tenancy-api:tenant-detail', kwargs={'pk': self.tenants[0].pk}) response = self.client.put(url, data, format='json', **self.header) self.assertHttpStatus(response, status.HTTP_200_OK) @@ -223,7 +249,7 @@ class TenantTest(APITestCase): def test_delete_tenant(self): - url = reverse('tenancy-api:tenant-detail', kwargs={'pk': self.tenant1.pk}) + url = reverse('tenancy-api:tenant-detail', kwargs={'pk': self.tenants[0].pk}) response = self.client.delete(url, **self.header) self.assertHttpStatus(response, status.HTTP_204_NO_CONTENT) diff --git a/netbox/tenancy/tests/test_filters.py b/netbox/tenancy/tests/test_filters.py index de2c0886d..bb1ac889c 100644 --- a/netbox/tenancy/tests/test_filters.py +++ b/netbox/tenancy/tests/test_filters.py @@ -11,12 +11,21 @@ class TenantGroupTestCase(TestCase): @classmethod def setUpTestData(cls): - groups = ( - TenantGroup(name='Tenant Group 1', slug='tenant-group-1'), - TenantGroup(name='Tenant Group 2', slug='tenant-group-2'), - TenantGroup(name='Tenant Group 3', slug='tenant-group-3'), + parent_tenant_groups = ( + TenantGroup(name='Parent Tenant Group 1', slug='parent-tenant-group-1'), + TenantGroup(name='Parent Tenant Group 2', slug='parent-tenant-group-2'), + TenantGroup(name='Parent Tenant Group 3', slug='parent-tenant-group-3'), ) - TenantGroup.objects.bulk_create(groups) + for tenantgroup in parent_tenant_groups: + tenantgroup.save() + + tenant_groups = ( + TenantGroup(name='Tenant Group 1', slug='tenant-group-1', parent=parent_tenant_groups[0]), + TenantGroup(name='Tenant Group 2', slug='tenant-group-2', parent=parent_tenant_groups[1]), + TenantGroup(name='Tenant Group 3', slug='tenant-group-3', parent=parent_tenant_groups[2]), + ) + for tenantgroup in tenant_groups: + tenantgroup.save() def test_id(self): id_list = self.queryset.values_list('id', flat=True)[:2] @@ -31,6 +40,13 @@ class TenantGroupTestCase(TestCase): params = {'slug': ['tenant-group-1', 'tenant-group-2']} self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) + def test_parent(self): + parent_groups = TenantGroup.objects.filter(name__startswith='Parent')[:2] + params = {'parent_id': [parent_groups[0].pk, parent_groups[1].pk]} + self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) + params = {'parent': [parent_groups[0].slug, parent_groups[1].slug]} + self.assertEqual(self.filterset(params, self.queryset).qs.count(), 2) + class TenantTestCase(TestCase): queryset = Tenant.objects.all() @@ -39,17 +55,18 @@ class TenantTestCase(TestCase): @classmethod def setUpTestData(cls): - groups = ( + tenant_groups = ( TenantGroup(name='Tenant Group 1', slug='tenant-group-1'), TenantGroup(name='Tenant Group 2', slug='tenant-group-2'), TenantGroup(name='Tenant Group 3', slug='tenant-group-3'), ) - TenantGroup.objects.bulk_create(groups) + for tenantgroup in tenant_groups: + tenantgroup.save() tenants = ( - Tenant(name='Tenant 1', slug='tenant-1', group=groups[0]), - Tenant(name='Tenant 2', slug='tenant-2', group=groups[1]), - Tenant(name='Tenant 3', slug='tenant-3', group=groups[2]), + Tenant(name='Tenant 1', slug='tenant-1', group=tenant_groups[0]), + Tenant(name='Tenant 2', slug='tenant-2', group=tenant_groups[1]), + Tenant(name='Tenant 3', slug='tenant-3', group=tenant_groups[2]), ) Tenant.objects.bulk_create(tenants) diff --git a/netbox/tenancy/tests/test_views.py b/netbox/tenancy/tests/test_views.py index 27e2c1591..5b47f8080 100644 --- a/netbox/tenancy/tests/test_views.py +++ b/netbox/tenancy/tests/test_views.py @@ -8,11 +8,13 @@ class TenantGroupTestCase(ViewTestCases.OrganizationalObjectViewTestCase): @classmethod def setUpTestData(cls): - TenantGroup.objects.bulk_create([ + tenant_groups = ( TenantGroup(name='Tenant Group 1', slug='tenant-group-1'), TenantGroup(name='Tenant Group 2', slug='tenant-group-2'), TenantGroup(name='Tenant Group 3', slug='tenant-group-3'), - ]) + ) + for tenanantgroup in tenant_groups: + tenanantgroup.save() cls.form_data = { 'name': 'Tenant Group X', @@ -33,22 +35,23 @@ class TenantTestCase(ViewTestCases.PrimaryObjectViewTestCase): @classmethod def setUpTestData(cls): - tenantgroups = ( + tenant_groups = ( TenantGroup(name='Tenant Group 1', slug='tenant-group-1'), TenantGroup(name='Tenant Group 2', slug='tenant-group-2'), ) - TenantGroup.objects.bulk_create(tenantgroups) + for tenanantgroup in tenant_groups: + tenanantgroup.save() Tenant.objects.bulk_create([ - Tenant(name='Tenant 1', slug='tenant-1', group=tenantgroups[0]), - Tenant(name='Tenant 2', slug='tenant-2', group=tenantgroups[0]), - Tenant(name='Tenant 3', slug='tenant-3', group=tenantgroups[0]), + Tenant(name='Tenant 1', slug='tenant-1', group=tenant_groups[0]), + Tenant(name='Tenant 2', slug='tenant-2', group=tenant_groups[0]), + Tenant(name='Tenant 3', slug='tenant-3', group=tenant_groups[0]), ]) cls.form_data = { 'name': 'Tenant X', 'slug': 'tenant-x', - 'group': tenantgroups[1].pk, + 'group': tenant_groups[1].pk, 'description': 'A new tenant', 'comments': 'Some comments', 'tags': 'Alpha,Bravo,Charlie', @@ -62,5 +65,5 @@ class TenantTestCase(ViewTestCases.PrimaryObjectViewTestCase): ) cls.bulk_edit_data = { - 'group': tenantgroups[1].pk, + 'group': tenant_groups[1].pk, } diff --git a/netbox/virtualization/tests/test_filters.py b/netbox/virtualization/tests/test_filters.py index 9b61896e9..db5935be9 100644 --- a/netbox/virtualization/tests/test_filters.py +++ b/netbox/virtualization/tests/test_filters.py @@ -105,7 +105,8 @@ class ClusterTestCase(TestCase): TenantGroup(name='Tenant group 2', slug='tenant-group-2'), TenantGroup(name='Tenant group 3', slug='tenant-group-3'), ) - TenantGroup.objects.bulk_create(tenant_groups) + for tenantgroup in tenant_groups: + tenantgroup.save() tenants = ( Tenant(name='Tenant 1', slug='tenant-1', group=tenant_groups[0]), @@ -231,7 +232,8 @@ class VirtualMachineTestCase(TestCase): TenantGroup(name='Tenant group 2', slug='tenant-group-2'), TenantGroup(name='Tenant group 3', slug='tenant-group-3'), ) - TenantGroup.objects.bulk_create(tenant_groups) + for tenantgroup in tenant_groups: + tenantgroup.save() tenants = ( Tenant(name='Tenant 1', slug='tenant-1', group=tenant_groups[0]), From 997247ee77ee36ab938431a01695681b281a6892 Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Wed, 11 Mar 2020 21:22:06 -0400 Subject: [PATCH 10/10] Update changelog for #1754, #3939 --- docs/release-notes/version-2.8.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/release-notes/version-2.8.md b/docs/release-notes/version-2.8.md index 035a67b74..e3ed0291d 100644 --- a/docs/release-notes/version-2.8.md +++ b/docs/release-notes/version-2.8.md @@ -18,6 +18,8 @@ If further customization of remote authentication is desired (for instance, if y ### Enhancements +* [#1754](https://github.com/netbox-community/netbox/issues/1754) - Added support for nested rack groups +* [#3939](https://github.com/netbox-community/netbox/issues/3939) - Added support for nested tenant groups * [#4195](https://github.com/netbox-community/netbox/issues/4195) - Enabled application logging (see [logging configuration](../configuration/optional-settings.md#logging)) ### API Changes