From f807d3a024e99c02ab2246d753d30e82dbee1d19 Mon Sep 17 00:00:00 2001 From: Glenn Matthews Date: Wed, 1 Jul 2020 15:23:38 -0400 Subject: [PATCH 1/2] Don't ignore ImportErrors raised when loading a plugin. Fixes #4805 --- netbox/extras/plugins/__init__.py | 31 +++++++++++++++-------- netbox/extras/plugins/urls.py | 42 ++++++++++++++++++++----------- netbox/extras/plugins/views.py | 25 +++++++++++++----- 3 files changed, 66 insertions(+), 32 deletions(-) diff --git a/netbox/extras/plugins/__init__.py b/netbox/extras/plugins/__init__.py index ee7f59196..38fbc3b47 100644 --- a/netbox/extras/plugins/__init__.py +++ b/netbox/extras/plugins/__init__.py @@ -1,12 +1,13 @@ import collections +import importlib import inspect +import sys from packaging import 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 from extras.registry import registry from utilities.choices import ButtonColorChoices @@ -60,18 +61,26 @@ class PluginConfig(AppConfig): def ready(self): # Register template content - try: - template_extensions = import_string(f"{self.__module__}.{self.template_extensions}") - register_template_extensions(template_extensions) - except ImportError: - pass + module, attr = f"{self.__module__}.{self.template_extensions}".rsplit('.', 1) + spec = importlib.util.find_spec(module) + if spec is not None: + template_content = importlib.util.module_from_spec(spec) + sys.modules[module] = template_content + spec.loader.exec_module(template_content) + if hasattr(template_content, attr): + template_extensions = getattr(template_content, attr) + register_template_extensions(template_extensions) # Register navigation menu items (if defined) - try: - menu_items = import_string(f"{self.__module__}.{self.menu_items}") - register_menu_items(self.verbose_name, menu_items) - except ImportError: - pass + module, attr = f"{self.__module__}.{self.menu_items}".rsplit('.', 1) + spec = importlib.util.find_spec(module) + if spec is not None: + navigation = importlib.util.module_from_spec(spec) + sys.modules[module] = navigation + spec.loader.exec_module(navigation) + if hasattr(navigation, attr): + menu_items = getattr(navigation, attr) + register_menu_items(self.verbose_name, menu_items) @classmethod def validate(cls, user_config): diff --git a/netbox/extras/plugins/urls.py b/netbox/extras/plugins/urls.py index b4360dc9e..d5925f1c8 100644 --- a/netbox/extras/plugins/urls.py +++ b/netbox/extras/plugins/urls.py @@ -1,9 +1,11 @@ +import importlib +import sys + from django.apps import apps from django.conf import settings from django.conf.urls import include from django.contrib.admin.views.decorators import staff_member_required from django.urls import path -from django.utils.module_loading import import_string from . import views @@ -24,19 +26,29 @@ for plugin_path in settings.PLUGINS: base_url = getattr(app, 'base_url') or app.label # Check if the plugin specifies any base URLs - try: - urlpatterns = import_string(f"{plugin_path}.urls.urlpatterns") - plugin_patterns.append( - path(f"{base_url}/", include((urlpatterns, app.label))) - ) - except ImportError: - pass + spec = importlib.util.find_spec(f"{plugin_path}.urls") + if spec is not None: + # The plugin has a .urls module - import it + urls = importlib.util.module_from_spec(spec) + sys.modules[f"{plugin_path}.urls"] = urls + spec.loader.exec_module(urls) + if hasattr(urls, "urlpatterns"): + urlpatterns = urls.urlpatterns + plugin_patterns.append( + path(f"{base_url}/", include((urlpatterns, app.label))) + ) # Check if the plugin specifies any API URLs - try: - urlpatterns = import_string(f"{plugin_path}.api.urls.urlpatterns") - plugin_api_patterns.append( - path(f"{base_url}/", include((urlpatterns, f"{app.label}-api"))) - ) - except ImportError: - pass + spec = importlib.util.find_spec(f"{plugin_path}.api") + if spec is not None: + spec = importlib.util.find_spec(f"{plugin_path}.api.urls") + if spec is not None: + # The plugin has a .api.urls module - import it + api_urls = importlib.util.module_from_spec(spec) + sys.modules[f"{plugin_path}.api.urls"] = api_urls + spec.loader.exec_module(api_urls) + if hasattr(api_urls, "urlpatterns"): + urlpatterns = api_urls.urlpatterns + plugin_api_patterns.append( + path(f"{base_url}/", include((urlpatterns, f"{app.label}-api"))) + ) diff --git a/netbox/extras/plugins/views.py b/netbox/extras/plugins/views.py index 39aa692d7..cbbf28791 100644 --- a/netbox/extras/plugins/views.py +++ b/netbox/extras/plugins/views.py @@ -1,10 +1,11 @@ from collections import OrderedDict +import importlib +import sys from django.apps import apps from django.conf import settings from django.shortcuts import render from django.urls.exceptions import NoReverseMatch -from django.utils.module_loading import import_string from django.views.generic import View from rest_framework import permissions from rest_framework.response import Response @@ -60,11 +61,23 @@ class PluginsAPIRootView(APIView): @staticmethod def _get_plugin_entry(plugin, app_config, request, format): - try: - api_app_name = import_string(f"{plugin}.api.urls.app_name") - except (ImportError, ModuleNotFoundError): - # Plugin does not expose an API + # Check if the plugin specifies any API URLs + spec = importlib.util.find_spec(f"{plugin}.api") + if spec is None: + # There is no plugin.api module return None + spec = importlib.util.find_spec(f"{plugin}.api.urls") + if spec is None: + # There is no plugin.api.urls module + return None + # The plugin has a .api.urls module - import it + api_urls = importlib.util.module_from_spec(spec) + sys.modules[f"{plugin}.api.urls"] = api_urls + spec.loader.exec_module(api_urls) + if not hasattr(api_urls, "app_name"): + # The plugin api.urls does not declare an app_name string + return None + api_app_name = api_urls.app_name try: entry = (getattr(app_config, 'base_url', app_config.label), reverse( @@ -73,7 +86,7 @@ class PluginsAPIRootView(APIView): format=format )) except NoReverseMatch: - # The plugin does not include an api-root + # The plugin does not include an api-root url entry = None return entry From 0fd3c838613341fc5eb5c95086cff1dcabd8808d Mon Sep 17 00:00:00 2001 From: Glenn Matthews Date: Tue, 14 Jul 2020 17:15:17 -0400 Subject: [PATCH 2/2] Refactor repeated import code --- netbox/extras/plugins/__init__.py | 28 +++++++--------------- netbox/extras/plugins/urls.py | 39 ++++++++++--------------------- netbox/extras/plugins/utils.py | 33 ++++++++++++++++++++++++++ netbox/extras/plugins/views.py | 22 ++++------------- 4 files changed, 58 insertions(+), 64 deletions(-) create mode 100644 netbox/extras/plugins/utils.py diff --git a/netbox/extras/plugins/__init__.py b/netbox/extras/plugins/__init__.py index 38fbc3b47..159d1a023 100644 --- a/netbox/extras/plugins/__init__.py +++ b/netbox/extras/plugins/__init__.py @@ -1,7 +1,5 @@ import collections -import importlib import inspect -import sys from packaging import version from django.apps import AppConfig @@ -12,6 +10,8 @@ from django.template.loader import get_template from extras.registry import registry from utilities.choices import ButtonColorChoices +from extras.plugins.utils import import_object + # Initialize plugin registry stores registry['plugin_template_extensions'] = collections.defaultdict(list) @@ -61,26 +61,14 @@ class PluginConfig(AppConfig): def ready(self): # Register template content - module, attr = f"{self.__module__}.{self.template_extensions}".rsplit('.', 1) - spec = importlib.util.find_spec(module) - if spec is not None: - template_content = importlib.util.module_from_spec(spec) - sys.modules[module] = template_content - spec.loader.exec_module(template_content) - if hasattr(template_content, attr): - template_extensions = getattr(template_content, attr) - register_template_extensions(template_extensions) + template_extensions = import_object(f"{self.__module__}.{self.template_extensions}") + if template_extensions is not None: + register_template_extensions(template_extensions) # Register navigation menu items (if defined) - module, attr = f"{self.__module__}.{self.menu_items}".rsplit('.', 1) - spec = importlib.util.find_spec(module) - if spec is not None: - navigation = importlib.util.module_from_spec(spec) - sys.modules[module] = navigation - spec.loader.exec_module(navigation) - if hasattr(navigation, attr): - menu_items = getattr(navigation, attr) - register_menu_items(self.verbose_name, menu_items) + menu_items = import_object(f"{self.__module__}.{self.menu_items}") + if menu_items is not None: + register_menu_items(self.verbose_name, menu_items) @classmethod def validate(cls, user_config): diff --git a/netbox/extras/plugins/urls.py b/netbox/extras/plugins/urls.py index d5925f1c8..7ab293916 100644 --- a/netbox/extras/plugins/urls.py +++ b/netbox/extras/plugins/urls.py @@ -1,12 +1,11 @@ -import importlib -import sys - from django.apps import apps from django.conf import settings from django.conf.urls import include from django.contrib.admin.views.decorators import staff_member_required from django.urls import path +from extras.plugins.utils import import_object + from . import views # Initialize URL base, API, and admin URL patterns for plugins @@ -26,29 +25,15 @@ for plugin_path in settings.PLUGINS: base_url = getattr(app, 'base_url') or app.label # Check if the plugin specifies any base URLs - spec = importlib.util.find_spec(f"{plugin_path}.urls") - if spec is not None: - # The plugin has a .urls module - import it - urls = importlib.util.module_from_spec(spec) - sys.modules[f"{plugin_path}.urls"] = urls - spec.loader.exec_module(urls) - if hasattr(urls, "urlpatterns"): - urlpatterns = urls.urlpatterns - plugin_patterns.append( - path(f"{base_url}/", include((urlpatterns, app.label))) - ) + urlpatterns = import_object(f"{plugin_path}.urls.urlpatterns") + if urlpatterns is not None: + plugin_patterns.append( + path(f"{base_url}/", include((urlpatterns, app.label))) + ) # Check if the plugin specifies any API URLs - spec = importlib.util.find_spec(f"{plugin_path}.api") - if spec is not None: - spec = importlib.util.find_spec(f"{plugin_path}.api.urls") - if spec is not None: - # The plugin has a .api.urls module - import it - api_urls = importlib.util.module_from_spec(spec) - sys.modules[f"{plugin_path}.api.urls"] = api_urls - spec.loader.exec_module(api_urls) - if hasattr(api_urls, "urlpatterns"): - urlpatterns = api_urls.urlpatterns - plugin_api_patterns.append( - path(f"{base_url}/", include((urlpatterns, f"{app.label}-api"))) - ) + urlpatterns = import_object(f"{plugin_path}.api.urls.urlpatterns") + if urlpatterns is not None: + plugin_api_patterns.append( + path(f"{base_url}/", include((urlpatterns, f"{app.label}-api"))) + ) diff --git a/netbox/extras/plugins/utils.py b/netbox/extras/plugins/utils.py new file mode 100644 index 000000000..87240aba8 --- /dev/null +++ b/netbox/extras/plugins/utils.py @@ -0,0 +1,33 @@ +import importlib.util +import sys + + +def import_object(module_and_object): + """ + Import a specific object from a specific module by name, such as "extras.plugins.utils.import_object". + + Returns the imported object, or None if it doesn't exist. + """ + target_module_name, object_name = module_and_object.rsplit('.', 1) + module_hierarchy = target_module_name.split('.') + + # Iterate through the module hierarchy, checking for the existence of each successive submodule. + # We have to do this rather than jumping directly to calling find_spec(target_module_name) + # because find_spec will raise a ModuleNotFoundError if any parent module of target_module_name does not exist. + module_name = "" + for module_component in module_hierarchy: + module_name = f"{module_name}.{module_component}" if module_name else module_component + spec = importlib.util.find_spec(module_name) + if spec is None: + # No such module + return None + + # Okay, target_module_name exists. Load it if not already loaded + if target_module_name in sys.modules: + module = sys.modules[target_module_name] + else: + module = importlib.util.module_from_spec(spec) + sys.modules[target_module_name] = module + spec.loader.exec_module(module) + + return getattr(module, object_name, None) diff --git a/netbox/extras/plugins/views.py b/netbox/extras/plugins/views.py index cbbf28791..f30e0f539 100644 --- a/netbox/extras/plugins/views.py +++ b/netbox/extras/plugins/views.py @@ -1,6 +1,4 @@ from collections import OrderedDict -import importlib -import sys from django.apps import apps from django.conf import settings @@ -12,6 +10,8 @@ from rest_framework.response import Response from rest_framework.reverse import reverse from rest_framework.views import APIView +from extras.plugins.utils import import_object + class InstalledPluginsAdminView(View): """ @@ -62,22 +62,10 @@ class PluginsAPIRootView(APIView): @staticmethod def _get_plugin_entry(plugin, app_config, request, format): # Check if the plugin specifies any API URLs - spec = importlib.util.find_spec(f"{plugin}.api") - if spec is None: - # There is no plugin.api module + api_app_name = import_object(f"{plugin}.api.urls.app_name") + if api_app_name is None: + # Plugin does not expose an API return None - spec = importlib.util.find_spec(f"{plugin}.api.urls") - if spec is None: - # There is no plugin.api.urls module - return None - # The plugin has a .api.urls module - import it - api_urls = importlib.util.module_from_spec(spec) - sys.modules[f"{plugin}.api.urls"] = api_urls - spec.loader.exec_module(api_urls) - if not hasattr(api_urls, "app_name"): - # The plugin api.urls does not declare an app_name string - return None - api_app_name = api_urls.app_name try: entry = (getattr(app_config, 'base_url', app_config.label), reverse(