18896 Replace STORAGE_BACKEND with STORAGES and support Script running from S3 (#18680)

base_requirements.txt

@@ -42,6 +42,10 @@ django-rich
 # https://github.com/rq/django-rq/blob/master/CHANGELOG.md
 django-rq
 
+# Provides a variety of storage backends
+# https://github.com/jschneier/django-storages/blob/master/CHANGELOG.rst
+django-storages
+
 # Abstraction models for rendering and paginating HTML tables
 # https://github.com/jieter/django-tables2/blob/master/CHANGELOG.md
 django-tables2

docs/administration/replicating-netbox.md

@@ -54,7 +54,7 @@ pg_dump --username netbox --password --host localhost -s netbox > netbox_schema.
 By default, NetBox stores uploaded files (such as image attachments) in its media directory. To fully replicate an instance of NetBox, you'll need to copy both the database and the media files.
 
 !!! note
-    These operations are not necessary if your installation is utilizing a [remote storage backend](../configuration/system.md#storage_backend).
+    These operations are not necessary if your installation is utilizing a [remote storage backend](../configuration/system.md#storages).
 
 ### Archive the Media Directory
 

docs/configuration/system.md

@@ -196,23 +196,46 @@ The dotted path to the desired search backend class. `CachedValueSearchBackend`
 
 ---
 
-## STORAGE_BACKEND
+## STORAGES
 
-Default: None (local storage)
+The backend storage engine for handling uploaded files such as [image attachments](../models/extras/imageattachment.md) and [custom scripts](../customization/custom-scripts.md). NetBox integrates with the [`django-storages`](https://django-storages.readthedocs.io/en/stable/) and [`django-storage-swift`](https://github.com/dennisv/django-storage-swift) libraries, which provide backends for several popular file storage services. If not configured, local filesystem storage will be used.
 
-The backend storage engine for handling uploaded files (e.g. image attachments). NetBox supports integration with the [`django-storages`](https://django-storages.readthedocs.io/en/stable/) and [`django-storage-swift`](https://github.com/dennisv/django-storage-swift) packages, which provide backends for several popular file storage services. If not configured, local filesystem storage will be used.
+By default, the following configuration is used:
 
-The configuration parameters for the specified storage backend are defined under the `STORAGE_CONFIG` setting.
+```python
+STORAGES = {
+    "default": {
+        "BACKEND": "django.core.files.storage.FileSystemStorage",
+    },
+    "staticfiles": {
+        "BACKEND": "django.contrib.staticfiles.storage.StaticFilesStorage",
+    },
+    "scripts": {
+        "BACKEND": "extras.storage.ScriptFileSystemStorage",
+    },
+}
+```
 
----
+Within the `STORAGES` dictionary, `"default"` is used for image uploads, "staticfiles" is for static files and `"scripts"` is used for custom scripts.
 
-## STORAGE_CONFIG
+If using a remote storage like S3, define the config as `STORAGES[key]["OPTIONS"]` for each storage item as needed. For example:
 
-Default: Empty
+```python
+STORAGES = { 
+    "scripts": { 
+        "BACKEND": "storages.backends.s3boto3.S3Boto3Storage", 
+        "OPTIONS": { 
+            'access_key': 'access key', 
+            'secret_key': 'secret key',
+        }
+    }, 
+}
+```
 
-A dictionary of configuration parameters for the storage backend configured as `STORAGE_BACKEND`. The specific parameters to be used here are specific to each backend; see the documentation for your selected backend ([`django-storages`](https://django-storages.readthedocs.io/en/stable/) or [`django-storage-swift`](https://github.com/dennisv/django-storage-swift)) for more detail.
+The specific configuration settings for each storage backend can be found in the [django-storages documentation](https://django-storages.readthedocs.io/en/latest/index.html).
 
-If `STORAGE_BACKEND` is not defined, this setting will be ignored.
+!!! note
+    Any keys defined in the `STORAGES` configuration parameter replace those in the default configuration. It is only necessary to define keys within the `STORAGES` for the specific backend(s) you wish to configure.
 
 ---
 

docs/customization/custom-scripts.md

@@ -140,6 +140,8 @@ The Script class provides two convenience methods for reading data from files:
 
 These two methods will load data in YAML or JSON format, respectively, from files within the local path (i.e. `SCRIPTS_ROOT`).
 
+**Note:** These convenience methods are deprecated and will be removed in NetBox v4.4.  These only work if running scripts within the local path, they will not work if using a storage other than ScriptFileSystemStorage.
+
 ## Logging
 
 The Script object provides a set of convenient functions for recording messages at different severity levels:

docs/installation/3-netbox.md

@@ -207,7 +207,7 @@ All Python packages required by NetBox are listed in `requirements.txt` and will
 
 ### Remote File Storage
 
-By default, NetBox will use the local filesystem to store uploaded files. To use a remote filesystem, install the [`django-storages`](https://django-storages.readthedocs.io/en/stable/) library and configure your [desired storage backend](../configuration/system.md#storage_backend) in `configuration.py`.
+By default, NetBox will use the local filesystem to store uploaded files. To use a remote filesystem, install the [`django-storages`](https://django-storages.readthedocs.io/en/stable/) library and configure your [desired storage backend](../configuration/system.md#storages) in `configuration.py`.
 
 ```no-highlight
 sudo sh -c "echo 'django-storages' >> /opt/netbox/local_requirements.txt"

netbox/core/models/data.py

@@ -357,17 +357,6 @@ class DataFile(models.Model):
 
         return is_modified
 
-    def write_to_disk(self, path, overwrite=False):
-        """
-        Write the object's data to disk at the specified path
-        """
-        # Check whether file already exists
-        if os.path.isfile(path) and not overwrite:
-            raise FileExistsError()
-
-        with open(path, 'wb+') as new_file:
-            new_file.write(self.data)
-
 
 class AutoSyncRecord(models.Model):
     """

netbox/core/models/files.py

@@ -1,13 +1,16 @@
 import logging
 import os
+from functools import cached_property
 
 from django.conf import settings
 from django.core.exceptions import ValidationError
 from django.db import models
+from django.core.files.storage import storages
 from django.urls import reverse
 from django.utils.translation import gettext as _
 
 from ..choices import ManagedFileRootPathChoices
+from extras.storage import ScriptFileSystemStorage
 from netbox.models.features import SyncedDataMixin
 from utilities.querysets import RestrictedQuerySet
 
@@ -76,15 +79,35 @@ class ManagedFile(SyncedDataMixin, models.Model):
         return os.path.join(self._resolve_root_path(), self.file_path)
 
     def _resolve_root_path(self):
-        return {
+        storage = self.storage
-            'scripts': settings.SCRIPTS_ROOT,
+        if isinstance(storage, ScriptFileSystemStorage):
-            'reports': settings.REPORTS_ROOT,
+            return {
-        }[self.file_root]
+                'scripts': settings.SCRIPTS_ROOT,
+                'reports': settings.REPORTS_ROOT,
+            }[self.file_root]
+        else:
+            return ""
 
     def sync_data(self):
         if self.data_file:
             self.file_path = os.path.basename(self.data_path)
-            self.data_file.write_to_disk(self.full_path, overwrite=True)
+            self._write_to_disk(self.full_path, overwrite=True)
+
+    def _write_to_disk(self, path, overwrite=False):
+        """
+        Write the object's data to disk at the specified path
+        """
+        # Check whether file already exists
+        storage = self.storage
+        if storage.exists(path) and not overwrite:
+            raise FileExistsError()
+
+        with storage.open(path, 'wb+') as new_file:
+            new_file.write(self.data)
+
+    @cached_property
+    def storage(self):
+        return storages.create_storage(storages.backends["scripts"])
 
     def clean(self):
         super().clean()
@@ -104,8 +127,9 @@ class ManagedFile(SyncedDataMixin, models.Model):
 
     def delete(self, *args, **kwargs):
         # Delete file from disk
+        storage = self.storage
         try:
-            os.remove(self.full_path)
+            storage.delete(self.full_path)
         except FileNotFoundError:
             pass
 

netbox/extras/forms/scripts.py

@@ -1,11 +1,18 @@
+import os
+
 from django import forms
+from django.conf import settings
+from django.core.files.storage import storages
 from django.utils.translation import gettext_lazy as _
 
+from core.forms import ManagedFileForm
 from extras.choices import DurationChoices
+from extras.storage import ScriptFileSystemStorage
 from utilities.forms.widgets import DateTimePicker, NumberWithOptions
 from utilities.datetime import local_now
 
 __all__ = (
+    'ScriptFileForm',
     'ScriptForm',
 )
 
@@ -55,3 +62,26 @@ class ScriptForm(forms.Form):
             self.cleaned_data['_schedule_at'] = local_now()
 
         return self.cleaned_data
+
+
+class ScriptFileForm(ManagedFileForm):
+    """
+    ManagedFileForm with a custom save method to use django-storages.
+    """
+    def save(self, *args, **kwargs):
+        # If a file was uploaded, save it to disk
+        if self.cleaned_data['upload_file']:
+            storage = storages.create_storage(storages.backends["scripts"])
+
+            filename = self.cleaned_data['upload_file'].name
+            if isinstance(storage, ScriptFileSystemStorage):
+                full_path = os.path.join(settings.SCRIPTS_ROOT, filename)
+            else:
+                full_path = filename
+
+            self.instance.file_path = full_path
+            data = self.cleaned_data['upload_file']
+            storage.save(filename, data)
+
+        # need to skip ManagedFileForm save method
+        return super(ManagedFileForm, self).save(*args, **kwargs)

netbox/extras/models/mixins.py

@@ -1,11 +1,31 @@
+import importlib.abc
+import importlib.util
 import os
-from importlib.machinery import SourceFileLoader
+import sys
+from django.core.files.storage import storages
 
 __all__ = (
     'PythonModuleMixin',
 )
 
 
+class CustomStoragesLoader(importlib.abc.Loader):
+    """
+    Custom loader for exec_module to use django-storages instead of the file system.
+    """
+    def __init__(self, filename):
+        self.filename = filename
+
+    def create_module(self, spec):
+        return None  # Use default module creation
+
+    def exec_module(self, module):
+        storage = storages.create_storage(storages.backends["scripts"])
+        with storage.open(self.filename, 'rb') as f:
+            code = f.read()
+        exec(code, module.__dict__)
+
+
 class PythonModuleMixin:
 
     def get_jobs(self, name):
@@ -33,6 +53,16 @@ class PythonModuleMixin:
             return name
 
     def get_module(self):
-        loader = SourceFileLoader(self.python_name, self.full_path)
+        """
-        module = loader.load_module()
+        Load the module using importlib, but use a custom loader to use django-storages
+        instead of the file system.
+        """
+        spec = importlib.util.spec_from_file_location(self.python_name, self.name)
+        if spec is None:
+            raise ModuleNotFoundError(f"Could not find module: {self.python_name}")
+        loader = CustomStoragesLoader(self.name)
+        module = importlib.util.module_from_spec(spec)
+        sys.modules[self.python_name] = module
+        loader.exec_module(module)
+
         return module

netbox/extras/scripts.py

@@ -2,10 +2,12 @@ import inspect
 import json
 import logging
 import os
+import re
 
 import yaml
 from django import forms
 from django.conf import settings
+from django.core.files.storage import storages
 from django.core.validators import RegexValidator
 from django.utils import timezone
 from django.utils.functional import classproperty
@@ -367,9 +369,46 @@ class BaseScript:
     def filename(self):
         return inspect.getfile(self.__class__)
 
+    def findsource(self, object):
+        storage = storages.create_storage(storages.backends["scripts"])
+        with storage.open(os.path.basename(self.filename), 'r') as f:
+            data = f.read()
+
+        # Break the source code into lines
+        lines = [line + '\n' for line in data.splitlines()]
+
+        # Find the class definition
+        name = object.__name__
+        pat = re.compile(r'^(\s*)class\s*' + name + r'\b')
+        # use the class definition with the least indentation
+        candidates = []
+        for i in range(len(lines)):
+            match = pat.match(lines[i])
+            if match:
+                if lines[i][0] == 'c':
+                    return lines, i
+
+                candidates.append((match.group(1), i))
+        if not candidates:
+            raise OSError('could not find class definition')
+
+        # Sort the candidates by whitespace, and by line number
+        candidates.sort()
+        return lines, candidates[0][1]
+
     @property
     def source(self):
-        return inspect.getsource(self.__class__)
+        # Can't use inspect.getsource() as it uses os to get the file
+        # inspect uses ast, but that is overkill for this as we only do
+        # classes.
+        object = self.__class__
+
+        try:
+            lines, lnum = self.findsource(object)
+            lines = inspect.getblock(lines[lnum:])
+            return ''.join(lines)
+        except OSError:
+            return ''
 
     @classmethod
     def _get_vars(cls):
@@ -524,7 +563,12 @@ class BaseScript:
     def load_yaml(self, filename):
         """
         Return data from a YAML file
+        TODO: DEPRECATED: Remove this method in v4.4
         """
+        self._log(
+            _("load_yaml is deprecated and will be removed in v4.4"),
+            level=LogLevelChoices.LOG_WARNING
+        )
         try:
             from yaml import CLoader as Loader
         except ImportError:
@@ -539,7 +583,12 @@ class BaseScript:
     def load_json(self, filename):
         """
         Return data from a JSON file
+        TODO: DEPRECATED: Remove this method in v4.4
         """
+        self._log(
+            _("load_json is deprecated and will be removed in v4.4"),
+            level=LogLevelChoices.LOG_WARNING
+        )
         file_path = os.path.join(settings.SCRIPTS_ROOT, filename)
         with open(file_path, 'r') as datafile:
             data = json.load(datafile)
@@ -555,7 +604,6 @@ class BaseScript:
         Run the report and save its results. Each test method will be executed in order.
         """
         self.logger.info("Running report")
-
         try:
             for test_name in self.tests:
                 self._current_test = test_name

netbox/extras/storage.py

@@ -0,0 +1,14 @@
+from django.conf import settings
+from django.core.files.storage import FileSystemStorage
+from django.utils.functional import cached_property
+
+
+class ScriptFileSystemStorage(FileSystemStorage):
+    """
+    Custom storage for scripts - for django-storages as the default one will
+    go off media-root and raise security errors as the scripts can be outside
+    the media-root directory.
+    """
+    @cached_property
+    def base_location(self):
+        return settings.SCRIPTS_ROOT

netbox/extras/views.py

@@ -12,7 +12,6 @@ from django.utils.translation import gettext as _
 from django.views.generic import View
 
 from core.choices import ManagedFileRootPathChoices
-from core.forms import ManagedFileForm
 from core.models import Job
 from core.tables import JobTable
 from dcim.models import Device, DeviceRole, Platform
@@ -1163,7 +1162,7 @@ class DashboardWidgetDeleteView(LoginRequiredMixin, View):
 @register_model_view(ScriptModule, 'edit')
 class ScriptModuleCreateView(generic.ObjectEditView):
     queryset = ScriptModule.objects.all()
-    form = ManagedFileForm
+    form = forms.ScriptFileForm
 
     def alter_object(self, obj, *args, **kwargs):
         obj.file_root = ManagedFileRootPathChoices.SCRIPTS

netbox/netbox/settings.py

@@ -17,6 +17,7 @@ from netbox.config import PARAMS as CONFIG_PARAMS
 from netbox.constants import RQ_QUEUE_DEFAULT, RQ_QUEUE_HIGH, RQ_QUEUE_LOW
 from netbox.plugins import PluginConfig
 from netbox.registry import registry
+import storages.utils  # type: ignore
 from utilities.release import load_release_data
 from utilities.string import trailing_slash
 
@@ -177,7 +178,8 @@ SESSION_COOKIE_PATH = CSRF_COOKIE_PATH
 SESSION_COOKIE_SECURE = getattr(configuration, 'SESSION_COOKIE_SECURE', False)
 SESSION_FILE_PATH = getattr(configuration, 'SESSION_FILE_PATH', None)
 STORAGE_BACKEND = getattr(configuration, 'STORAGE_BACKEND', None)
-STORAGE_CONFIG = getattr(configuration, 'STORAGE_CONFIG', {})
+STORAGE_CONFIG = getattr(configuration, 'STORAGE_CONFIG', None)
+STORAGES = getattr(configuration, 'STORAGES', {})
 TIME_ZONE = getattr(configuration, 'TIME_ZONE', 'UTC')
 TRANSLATION_ENABLED = getattr(configuration, 'TRANSLATION_ENABLED', True)
 
@@ -234,61 +236,64 @@ DATABASES = {
 # Storage backend
 #
 
+if STORAGE_BACKEND is not None:
+    if not STORAGES:
+        raise ImproperlyConfigured(
+            "STORAGE_BACKEND and STORAGES are both set, remove the deprecated STORAGE_BACKEND setting."
+        )
+    else:
+        warnings.warn(
+            "STORAGE_BACKEND is deprecated, use the new STORAGES setting instead."
+        )
+
+if STORAGE_CONFIG is not None:
+    warnings.warn(
+        "STORAGE_CONFIG is deprecated, use the new STORAGES setting instead."
+    )
+
 # Default STORAGES for Django
-STORAGES = {
+DEFAULT_STORAGES = {
     "default": {
         "BACKEND": "django.core.files.storage.FileSystemStorage",
     },
     "staticfiles": {
         "BACKEND": "django.contrib.staticfiles.storage.StaticFilesStorage",
     },
+    "scripts": {
+        "BACKEND": "extras.storage.ScriptFileSystemStorage",
+    },
 }
+STORAGES = DEFAULT_STORAGES | STORAGES
 
+# TODO: This code is deprecated and needs to be removed in the future
 if STORAGE_BACKEND is not None:
     STORAGES['default']['BACKEND'] = STORAGE_BACKEND
 
-    # django-storages
+# Monkey-patch django-storages to fetch settings from STORAGE_CONFIG
-    if STORAGE_BACKEND.startswith('storages.'):
+if STORAGE_CONFIG is not None:
-        try:
+    def _setting(name, default=None):
-            import storages.utils  # type: ignore
+        if name in STORAGE_CONFIG:
-        except ModuleNotFoundError as e:
+            return STORAGE_CONFIG[name]
-            if getattr(e, 'name') == 'storages':
+        return globals().get(name, default)
-                raise ImproperlyConfigured(
+    storages.utils.setting = _setting
-                    f"STORAGE_BACKEND is set to {STORAGE_BACKEND} but django-storages is not present. It can be "
-                    f"installed by running 'pip install django-storages'."
-                )
-            raise e
 
-        # Monkey-patch django-storages to fetch settings from STORAGE_CONFIG
+# django-storage-swift
-        def _setting(name, default=None):
+if STORAGE_BACKEND == 'swift.storage.SwiftStorage':
-            if name in STORAGE_CONFIG:
+    try:
-                return STORAGE_CONFIG[name]
+        import swift.utils  # noqa: F401
-            return globals().get(name, default)
+    except ModuleNotFoundError as e:
-        storages.utils.setting = _setting
+        if getattr(e, 'name') == 'swift':
-
+            raise ImproperlyConfigured(
-    # django-storage-swift
+                f"STORAGE_BACKEND is set to {STORAGE_BACKEND} but django-storage-swift is not present. "
-    elif STORAGE_BACKEND == 'swift.storage.SwiftStorage':
+                "It can be installed by running 'pip install django-storage-swift'."
-        try:
+            )
-            import swift.utils  # noqa: F401
+        raise e
-        except ModuleNotFoundError as e:
-            if getattr(e, 'name') == 'swift':
-                raise ImproperlyConfigured(
-                    f"STORAGE_BACKEND is set to {STORAGE_BACKEND} but django-storage-swift is not present. "
-                    "It can be installed by running 'pip install django-storage-swift'."
-                )
-            raise e
-
-        # Load all SWIFT_* settings from the user configuration
-        for param, value in STORAGE_CONFIG.items():
-            if param.startswith('SWIFT_'):
-                globals()[param] = value
-
-if STORAGE_CONFIG and STORAGE_BACKEND is None:
-    warnings.warn(
-        "STORAGE_CONFIG has been set in configuration.py but STORAGE_BACKEND is not defined. STORAGE_CONFIG will be "
-        "ignored."
-    )
 
+    # Load all SWIFT_* settings from the user configuration
+    for param, value in STORAGE_CONFIG.items():
+        if param.startswith('SWIFT_'):
+            globals()[param] = value
+# TODO: End of deprecated code
 
 #
 # Redis

requirements.txt

@@ -10,6 +10,7 @@ django-prometheus==2.3.1
 django-redis==5.4.0
 django-rich==1.13.0
 django-rq==3.0
+django-storages==1.14.4
 django-taggit==6.1.0
 django-tables2==2.7.5
 django-timezone-field==7.1