From 9ffc40402728f99df3685a64040a4cfe153ab6fd Mon Sep 17 00:00:00 2001 From: Jeremy Stretch Date: Mon, 6 Apr 2020 11:44:38 -0400 Subject: [PATCH] Add tests for plugin configuration, min/max version --- netbox/extras/plugins/__init__.py | 34 +++++++++++++ netbox/extras/tests/dummy_plugin/__init__.py | 2 + netbox/extras/tests/test_plugins.py | 53 +++++++++++++++++++- netbox/netbox/configuration.testing.py | 4 ++ netbox/netbox/settings.py | 29 ++--------- 5 files changed, 96 insertions(+), 26 deletions(-) diff --git a/netbox/extras/plugins/__init__.py b/netbox/extras/plugins/__init__.py index 6e515dd5b..df98b1a65 100644 --- a/netbox/extras/plugins/__init__.py +++ b/netbox/extras/plugins/__init__.py @@ -1,7 +1,10 @@ import collections import inspect +from pkg_resources import parse_version from django.apps import AppConfig +from django.conf import settings +from django.core.exceptions import ImproperlyConfigured from django.template.loader import get_template from django.utils.module_loading import import_string @@ -70,6 +73,37 @@ class PluginConfig(AppConfig): except ImportError: pass + @classmethod + def validate(cls, user_config): + + # Enforce version constraints + current_version = parse_version(settings.VERSION) + if cls.min_version is not None: + min_version = parse_version(cls.min_version) + if current_version < min_version: + raise ImproperlyConfigured( + f"Plugin {cls.__module__} requires NetBox minimum version {cls.min_version}." + ) + if cls.max_version is not None: + max_version = parse_version(cls.max_version) + if current_version > max_version: + raise ImproperlyConfigured( + f"Plugin {cls.__module__} requires NetBox maximum version {cls.max_version}." + ) + + # Verify required configuration settings + for setting in cls.required_settings: + if setting not in user_config: + raise ImproperlyConfigured( + f"Plugin {cls.__module__} requires '{setting}' to be present in the PLUGINS_CONFIG section of " + f"configuration.py." + ) + + # Apply default configuration values + for setting, value in cls.default_settings.items(): + if setting not in user_config: + user_config[setting] = value + # # Template content injection diff --git a/netbox/extras/tests/dummy_plugin/__init__.py b/netbox/extras/tests/dummy_plugin/__init__.py index eaaff4f5e..63f7d308e 100644 --- a/netbox/extras/tests/dummy_plugin/__init__.py +++ b/netbox/extras/tests/dummy_plugin/__init__.py @@ -7,6 +7,8 @@ class DummyPluginConfig(PluginConfig): version = '0.0' description = 'For testing purposes only' base_url = 'dummy-plugin' + min_version = '1.0' + max_version = '9.0' middleware = [ 'extras.tests.dummy_plugin.middleware.DummyMiddleware' ] diff --git a/netbox/extras/tests/test_plugins.py b/netbox/extras/tests/test_plugins.py index dba6308b9..94c0a85f3 100644 --- a/netbox/extras/tests/test_plugins.py +++ b/netbox/extras/tests/test_plugins.py @@ -1,13 +1,15 @@ from unittest import skipIf from django.conf import settings +from django.core.exceptions import ImproperlyConfigured from django.test import Client, TestCase, override_settings from django.urls import reverse from extras.registry import registry +from extras.tests.dummy_plugin import config as dummy_config -@skipIf('extras.tests.dummy_plugin.DummyPluginConfig' not in settings.PLUGINS, "dummy_plugin not in settings.PLUGINS") +@skipIf('extras.tests.dummy_plugin' not in settings.PLUGINS, "dummy_plugin not in settings.PLUGINS") class PluginTest(TestCase): def test_config(self): @@ -77,3 +79,52 @@ class PluginTest(TestCase): Check that plugin middleware is registered. """ self.assertIn('extras.tests.dummy_plugin.middleware.DummyMiddleware', settings.MIDDLEWARE) + + @override_settings(VERSION='0.9') + def test_min_version(self): + """ + Check enforcement of minimum NetBox version. + """ + with self.assertRaises(ImproperlyConfigured): + dummy_config.validate({}) + + @override_settings(VERSION='10.0') + def test_max_version(self): + """ + Check enforcement of maximum NetBox version. + """ + with self.assertRaises(ImproperlyConfigured): + dummy_config.validate({}) + + def test_required_settings(self): + """ + Validate enforcement of required settings. + """ + class DummyConfigWithRequiredSettings(dummy_config): + required_settings = ['foo'] + + # Validation should pass when all required settings are present + DummyConfigWithRequiredSettings.validate({'foo': True}) + + # Validation should fail when a required setting is missing + with self.assertRaises(ImproperlyConfigured): + DummyConfigWithRequiredSettings.validate({}) + + def test_default_settings(self): + """ + Validate population of default config settings. + """ + class DummyConfigWithDefaultSettings(dummy_config): + default_settings = { + 'bar': 123, + } + + # Populate the default value if setting has not been specified + user_config = {} + DummyConfigWithDefaultSettings.validate(user_config) + self.assertEqual(user_config['bar'], 123) + + # Don't overwrite specified values + user_config = {'bar': 456} + DummyConfigWithDefaultSettings.validate(user_config) + self.assertEqual(user_config['bar'], 456) diff --git a/netbox/netbox/configuration.testing.py b/netbox/netbox/configuration.testing.py index 09d5362ab..c0cd6b3ea 100644 --- a/netbox/netbox/configuration.testing.py +++ b/netbox/netbox/configuration.testing.py @@ -18,6 +18,10 @@ PLUGINS = [ 'extras.tests.dummy_plugin', ] +PLUGINS_CONFIG = { + 'foo': True, +} + REDIS = { 'tasks': { 'HOST': 'localhost', diff --git a/netbox/netbox/settings.py b/netbox/netbox/settings.py index c172aeaa9..a6a55cf4b 100644 --- a/netbox/netbox/settings.py +++ b/netbox/netbox/settings.py @@ -10,7 +10,6 @@ from urllib.parse import urlsplit from django.contrib.messages import constants as messages from django.core.exceptions import ImproperlyConfigured, ValidationError from django.core.validators import URLValidator -from pkg_resources import parse_version # @@ -658,36 +657,16 @@ for plugin_name in PLUGINS: f"__init__.py file and point to the PluginConfig subclass." ) - # Check version constraints - parsed_min_version = parse_version(plugin_config.min_version or VERSION) - parsed_max_version = parse_version(plugin_config.max_version or VERSION) - if plugin_config.min_version and plugin_config.max_version and parsed_min_version > parsed_max_version: - raise ImproperlyConfigured(f"Plugin {plugin_name} specifies invalid version constraints!") - if plugin_config.min_version and parsed_min_version > parse_version(VERSION): - raise ImproperlyConfigured(f"Plugin {plugin_name} requires NetBox minimum version {plugin_config.min_version}!") - if plugin_config.max_version and parsed_max_version < parse_version(VERSION): - raise ImproperlyConfigured(f"Plugin {plugin_name} requires NetBox maximum version {plugin_config.max_version}!") + # Validate user-provided configuration settings and assign defaults + if plugin_name not in PLUGINS_CONFIG: + PLUGINS_CONFIG[plugin_name] = {} + plugin_config.validate(PLUGINS_CONFIG[plugin_name]) # Add middleware plugin_middleware = plugin_config.middleware if plugin_middleware and type(plugin_middleware) in (list, tuple): MIDDLEWARE.extend(plugin_middleware) - # Verify required configuration settings - if plugin_name not in PLUGINS_CONFIG: - PLUGINS_CONFIG[plugin_name] = {} - for setting in plugin_config.required_settings: - if setting not in PLUGINS_CONFIG[plugin_name]: - raise ImproperlyConfigured( - f"Plugin {plugin_name} requires '{setting}' to be present in the PLUGINS_CONFIG section of " - f"configuration.py." - ) - - # Apply default configuration values - for setting, value in plugin_config.default_settings.items(): - if setting not in PLUGINS_CONFIG[plugin_name]: - PLUGINS_CONFIG[plugin_name][setting] = value - # Apply cacheops config if type(plugin_config.caching_config) is not dict: raise ImproperlyConfigured(f"Plugin {plugin_name} caching_config must be a dictionary.")