Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

500 Error when saving IP Address when SLAAC status does not exist. #1035

Closed
FragmentedPacket opened this issue Oct 27, 2021 · 5 comments · Fixed by #2070
Closed

500 Error when saving IP Address when SLAAC status does not exist. #1035

FragmentedPacket opened this issue Oct 27, 2021 · 5 comments · Fixed by #2070
Assignees
Labels
type: bug Something isn't working as expected type: documentation Improvements or additions to documentation

Comments

@FragmentedPacket
Copy link
Contributor

Environment

  • Python version: 3.6
  • Nautobot version: 1.1.4

Steps to Reproduce

  1. Delete SLAAC status from Nautobot
  2. Attempt to create an IP address (IPv4/IPv6)
  3. Receive a 500 error

Expected Behavior

The IP address gets created successfully

Observed Behavior

2021-10-27 15:38:14,189 ERROR [django.request:230] log 30452 139695917877184 Internal Server Error: /ipam/ip-addresses/add/
Traceback (most recent call last):
  File "/opt/nautobot/lib64/python3.6/site-packages/django/core/handlers/exception.py", line 47, in inner
    response = get_response(request)
  File "/opt/nautobot/lib64/python3.6/site-packages/django/core/handlers/base.py", line 181, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/opt/nautobot/lib64/python3.6/site-packages/django/views/generic/base.py", line 70, in view
    return self.dispatch(request, *args, **kwargs)
  File "/opt/nautobot/lib64/python3.6/site-packages/nautobot/core/views/generic.py", line 266, in dispatch
    return super().dispatch(request, *args, **kwargs)
  File "/opt/nautobot/lib64/python3.6/site-packages/nautobot/utilities/views.py", line 94, in dispatch
    return super().dispatch(request, *args, **kwargs)
  File "/opt/nautobot/lib64/python3.6/site-packages/django/views/generic/base.py", line 98, in dispatch
    return handler(request, *args, **kwargs)
  File "/opt/nautobot/lib64/python3.6/site-packages/nautobot/core/views/generic.py", line 293, in post
    if form.is_valid():
  File "/opt/nautobot/lib64/python3.6/site-packages/django/forms/forms.py", line 177, in is_valid
    return self.is_bound and not self.errors
  File "/opt/nautobot/lib64/python3.6/site-packages/django/forms/forms.py", line 172, in errors
    self.full_clean()
  File "/opt/nautobot/lib64/python3.6/site-packages/django/forms/forms.py", line 376, in full_clean
    self._post_clean()
  File "/opt/nautobot/lib64/python3.6/site-packages/django/forms/models.py", line 405, in _post_clean
    self.instance.full_clean(exclude=exclude, validate_unique=False)
  File "/opt/nautobot/lib64/python3.6/site-packages/django/db/models/base.py", line 1216, in full_clean
    self.clean()
  File "/opt/nautobot/lib64/python3.6/site-packages/nautobot/extras/plugins/validators.py", line 20, in wrapper
    model_clean_func(model_instance)
  File "/opt/nautobot/lib64/python3.6/site-packages/nautobot/ipam/models.py", line 891, in clean
    if self.status == IPAddress.STATUS_SLAAC and self.family != 6:
  File "/opt/nautobot/lib64/python3.6/site-packages/django/utils/functional.py", line 61, in _get_
    return self.fget(cls)
  File "/opt/nautobot/lib64/python3.6/site-packages/nautobot/ipam/models.py", line 848, in STATUS_SLAAC
    cls.__status_slaac = Status.objects.get_for_model(IPAddress).get(slug="slaac")
  File "/opt/nautobot/lib64/python3.6/site-packages/cacheops/query.py", line 353, in get
    return qs._no_monkey.get(qs, *args, **kwargs)
  File "/opt/nautobot/lib64/python3.6/site-packages/django/db/models/query.py", line 431, in get
    self.model._meta.object_name
@glennmatthews glennmatthews added the type: bug Something isn't working as expected label Oct 27, 2021
@FragmentedPacket
Copy link
Contributor Author

Appears to be due to the SLAAC check within the clean method.

cls.__status_slaac = Status.objects.get_for_model(IPAddress).get(slug="slaac")

@jathanism
Copy link
Contributor

jathanism commented Oct 27, 2021

This isn't technically a bug since this is working as intended. This is a required status. But it's also surprising and should be better documented so it is in actuality a bug.

And this isn't the only "required" status:

$ grep -A1 -r "def STATUS_" nautobot
nautobot/dcim/models/cables.py:    def STATUS_CONNECTED(cls):
nautobot/dcim/models/cables.py-        """Return a cached "connected" `Status` object for later reference."""
--
nautobot/ipam/models.py:    def STATUS_CONTAINER(cls):
nautobot/ipam/models.py-        """Return a cached "container" `Status` object for later reference."""
--
nautobot/ipam/models.py:    def STATUS_SLAAC(cls):
nautobot/ipam/models.py-        """Return a cached "slaac" `Status` object for later reference."""

@FragmentedPacket May I ask why the status SLAAC was deleted in this particular instance?

In any case, this topic literally just came up the other day when we were doing backlog grooming, in that for pre-populated database objects like Status instances, we should think deliberately about how to designate certain ones as "protected". We attempted to implement this pre-1.0 and ran into issues. It's kind of a quirky problem since we're auto-populating database objects on a new instance of Nautobot.

In the interim at least, we could better document this and think about how to at least provide useful messaging around the error case?

@jathanism jathanism added the type: documentation Improvements or additions to documentation label Oct 27, 2021
@FragmentedPacket
Copy link
Contributor Author

This was caused by a penetration test that ran in an environment and wiped out those objects. I would say that isn't a normal circumstance, but the designation of "protected" objects that are relied on in other places would be great to prevent these scenarios from happening due to internal code relying on them.

Documentation would be great, but not entirely sure it would help in the case of troubleshooting, but we're familiar with the code so we figured it out relatively quickly so someone else that may not be more familiar may look at docs for more info.

Maybe we can have a wrapper method for getting a status for a model that relies on a built-in and then returns a better message error message?

@whitej6
Copy link
Contributor

whitej6 commented Oct 28, 2021

I would second that as well, the error raised in the UI/API made me think it came from the selected status and we were only able to diagnose the issue with the traceback.

Protecting these dependent objects would prevent a user not reading documentation or a robot web scanner wreaking havoc.

@whitej6
Copy link
Contributor

whitej6 commented Dec 16, 2021

Related to #1169

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: bug Something isn't working as expected type: documentation Improvements or additions to documentation
Projects
No open projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants