netboxlabs / netbox-branching

Official NetBox Labs plugin that implements git-like branching functionality for NetBox
http://netboxlabs.com
Other
63 stars 1 forks source link

Exception when trying to clone a NetBox DNS zone #89

Closed peteeckel closed 1 month ago

peteeckel commented 2 months ago

Plugin Version

0.4.0

NetBox Version

4.1.0

Python Version

3.12.1

Steps to Reproduce

  1. Create a DNS zone in NetBox DNS
  2. Create and activate a new branch in NetBox Branching
  3. Clone the zone just created

Expected Behavior

The zone is cloned in the branch.

Observed Behavior

Environment:

Request Method: POST
Request URL: https://netbox.dev.hindenburgring.com/plugins/netbox-dns/zones/add/?view=3&name=0.0.10.in-addr.arpa&status=active&nameservers=1&nameservers=2&default_ttl=86400&soa_ttl=86400&soa_mname=1&soa_rname=hostmaster.example.com&soa_refresh=172800&soa_retry=7200&soa_expire=2592000&soa_minimum=3600

Django Version: 5.0.9
Python Version: 3.12.1
Installed Applications:
['django.contrib.auth',
 'django.contrib.contenttypes',
 'django.contrib.sessions',
 'django.contrib.messages',
 'django.contrib.staticfiles',
 'django.contrib.humanize',
 'django.forms',
 'corsheaders',
 'debug_toolbar',
 'django_filters',
 'django_htmx',
 'django_tables2',
 'django_prometheus',
 'strawberry_django',
 'mptt',
 'rest_framework',
 'social_django',
 'taggit',
 'timezone_field',
 'core',
 'account',
 'circuits',
 'dcim',
 'ipam',
 'extras',
 'tenancy',
 'users',
 'utilities',
 'virtualization',
 'vpn',
 'wireless',
 'django_rq',
 'drf_spectacular',
 'drf_spectacular_sidecar',
 'netbox_dns.DNSConfig',
 'netbox_branching.AppConfig']
Installed Middleware:
['strawberry_django.middlewares.debug_toolbar.DebugToolbarMiddleware',
 'corsheaders.middleware.CorsMiddleware',
 'django.contrib.sessions.middleware.SessionMiddleware',
 'django.middleware.locale.LocaleMiddleware',
 'django.middleware.common.CommonMiddleware',
 'django.middleware.csrf.CsrfViewMiddleware',
 'django.contrib.auth.middleware.AuthenticationMiddleware',
 'django.contrib.messages.middleware.MessageMiddleware',
 'django.middleware.clickjacking.XFrameOptionsMiddleware',
 'django.middleware.security.SecurityMiddleware',
 'django_htmx.middleware.HtmxMiddleware',
 'netbox.middleware.RemoteUserMiddleware',
 'netbox.middleware.CoreMiddleware',
 'netbox.middleware.MaintenanceModeMiddleware',
 'netbox_branching.middleware.BranchMiddleware']

Traceback (most recent call last):
  File "/opt/netbox/lib64/python3.12/site-packages/django/core/handlers/exception.py", line 55, in inner
    response = get_response(request)
               ^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/lib64/python3.12/site-packages/django/core/handlers/base.py", line 197, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/lib64/python3.12/site-packages/django/views/generic/base.py", line 104, in view
    return self.dispatch(request, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/netbox/netbox/views/generic/object_views.py", line 180, in dispatch
    return super().dispatch(request, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/netbox/netbox/views/generic/base.py", line 26, in dispatch
    return super().dispatch(request, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/netbox/utilities/views.py", line 125, in dispatch
    return super().dispatch(request, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/netbox/utilities/views.py", line 39, in dispatch
    return super().dispatch(request, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/lib64/python3.12/site-packages/django/views/generic/base.py", line 143, in dispatch
    return handler(request, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/netbox/netbox/views/generic/object_views.py", line 276, in post
    obj = form.save()
          ^^^^^^^^^^^
  File "/vagrant/netbox-dns/netbox_dns/forms/zone.py", line 119, in save
    zone = super().save(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/lib64/python3.12/site-packages/django/forms/models.py", line 552, in save
    self.instance.save()
    ^^^^^^^^^^^^^^^^^^^^
  File "/vagrant/netbox-dns/netbox_dns/models/zone.py", line 756, in save
    address_record.save(
    ^
  File "/vagrant/netbox-dns/netbox_dns/models/record.py", line 818, in save
    super().save(*args, **kwargs)
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/lib64/python3.12/site-packages/django/db/models/base.py", line 822, in save
    self.save_base(
    ^
  File "/opt/netbox/lib64/python3.12/site-packages/django/db/models/base.py", line 924, in save_base
    post_save.send(
    ^
  File "/opt/netbox/lib64/python3.12/site-packages/django/dispatch/dispatcher.py", line 189, in send
    response = receiver(signal=self, sender=sender, **named)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/netbox/core/signals.py", line 94, in handle_changed_object
    objectchange.save()
    ^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/netbox/core/models/change_logging.py", line 136, in save
    return super().save(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/lib64/python3.12/site-packages/django/db/models/base.py", line 822, in save
    self.save_base(
    ^
  File "/opt/netbox/lib64/python3.12/site-packages/django/db/models/base.py", line 924, in save_base
    post_save.send(
    ^
  File "/opt/netbox/lib64/python3.12/site-packages/django/dispatch/dispatcher.py", line 189, in send
    response = receiver(signal=self, sender=sender, **named)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/lib64/python3.12/site-packages/netbox_branching/signal_receivers.py", line 92, in record_change_diff
    diff.save()
    ^^^^^^^^^^^
  File "/opt/netbox/lib64/python3.12/site-packages/netbox_branching/models/changes.py", line 160, in save
    self._update_conflicts()
    ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/netbox/lib64/python3.12/site-packages/netbox_branching/models/changes.py", line 175, in _update_conflicts
    k for k, v in self.original.items()
                  ^^^^^^^^^^^^^^^^^^^

Exception Type: AttributeError at /plugins/netbox-dns/zones/add/
Exception Value: 'NoneType' object has no attribute 'items'

After reloading it seems that the new zone has been created anyway.

cruse1977 commented 2 months ago

also for consideration - #56. (may/may not be related)

peteeckel commented 2 months ago

That's possible, although here the error occurs in post_save(), while #56 affects merging.

stiltzkin10 commented 1 month ago

I ran in to this problem too. I think it boils down to signal_receivers.py:L93 setting ChangeDiff.original = None in some cases:

diff = ChangeDiff(
    branch=branch,
    object=instance.changed_object,
    action=instance.action,
    original=instance.prechange_data_clean or None,
    modified=instance.postchange_data_clean or None,
    current=current_data or None,
    last_updated=timezone.now(),
)

None gets saved and then _update_conflicts() tries to call items() on None.

Here's one way to quickly re-produce the issue:

from netbox_branching.models import Branch, ChangeDiff
from dcim.models import Interface
from netbox_branching.utilities import activate_branch
from django.utils import timezone
branch = Branch(name='Test 1')
branch.schema_id = 'test1'
branch.save(provision=False)
branch.provision(user=None)

with activate_branch(branch):
  interface, success = Interface.objects.get_or_create(name='Ethernet1')
  interface.description = "foobar"
  diff = ChangeDiff(
    branch=branch,
    object=interface,
    action="update",
    original=None,
    modified=None,
    current=None,
    last_updated=timezone.now(),
  )
  interface.save()

diff.save()

Output:

(venv) unit@5788bbbb9a0c:/opt/netbox/netbox$ /opt/netbox/netbox/manage.py shell
🧬 loaded config '/etc/netbox/config/configuration.py'
🧬 loaded config '/etc/netbox/config/extra.py'
🧬 loaded config '/etc/netbox/config/logging.py'
🧬 loaded config '/etc/netbox/config/plugins.py'
Python 3.12.3 (main, Jul 31 2024, 17:43:48) [GCC 13.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
(InteractiveConsole)
>>> from netbox_branching.models import Branch, ChangeDiff
>>> from dcim.models import Interface
>>> from netbox_branching.utilities import activate_branch
>>> from django.utils import timezone
>>> branch = Branch(name='Test 1')
>>> branch.schema_id = 'test1'
>>> branch.save(provision=False)
>>> branch.provision(user=None)
>>> with activate_branch(branch):
...   interface, success = Interface.objects.get_or_create(name='Ethernet1')
...   interface.description = "foobar"
...   diff = ChangeDiff(
...     branch=branch,
...     object=interface,
...     action="update",
...     original=None,
...     modified=None,
...     current=None,
...     last_updated=timezone.now(),
...   )
...   interface.save()
...
>>> diff.save()
Traceback (most recent call last):
  File "/usr/lib/python3.12/code.py", line 90, in runcode
    exec(code, self.locals)
  File "<console>", line 1, in <module>
  File "/opt/netbox/venv/lib/python3.12/site-packages/netbox_branching/models/changes.py", line 160, in save
    self._update_conflicts()
  File "/opt/netbox/venv/lib/python3.12/site-packages/netbox_branching/models/changes.py", line 175, in _update_conflicts
    k for k, v in self.original.items()
                  ^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'items'
jeremystretch commented 1 month ago

I'm not able to reproduce this using either netbox-branching v0.4.0 or the current develop branch: The new zone is created within the branch as expected. @peteeckel could you provide more detailed reproduction steps please?

peteeckel commented 1 month ago

Just tried with a fresh database, and now I can't reproduce it either - what about the minimised reproduction code that @stiltzkin10 contributed? The error is exactly the same.

jeremystretch commented 1 month ago

Arbitrary code does not prove a bug. The behavior must be reproducible in NetBox itself.

stiltzkin10 commented 1 month ago

I created a new plugin with this model:

from django.db import models
from django.urls import reverse
from netbox.models import NetBoxModel

from dcim.models import Site

class Branchingbug(NetBoxModel):
    name = models.CharField(max_length=100)

    class Meta:
        ordering = ("name",)

    def save(self, **kwargs):
        """Save."""

        site = Site.objects.get(name='MySite')
        site.description = "foobar"
        site.save()

        super().save(**kwargs)

    def __str__(self):
        return self.name

    def get_absolute_url(self):
        return reverse("plugins:branchingbug:branchingbug", args=[self.pk])

It works fine if I'm in the "Main" branch, but if I create a new branch and try to create a new instance of this plugin it crashes.

If I change the get() call to get_or_create() and let it create a new site it doesn't fail.

jeremystretch commented 1 month ago

This looks like the same root issue as #56: The model is making branch-agnostic queries.

Generally speaking, it's best to avoid making database queries (especially mutating queries) inside save(). If that's unavoidable, you'll need to ensure that the method's using keyword argument is passed through to ensure the correct database connection is used.

I'm going to close this out as it substantially overlaps with #56; let's continue any further relevant discussion under that issue.