dropbox / nsot

Network Source of Truth is an open source IPAM and network inventory database
https://nsot.readthedocs.io
Other
400 stars 66 forks source link

Swagger networks urls have an erroneous ) in them for pk parameter #152

Open TheSkorm opened 8 years ago

TheSkorm commented 8 years ago

See picture for an example. I had a little dig around to see if I could find the cause however couldn't. Some guidance on how to fix this would be appreciated.

screen shot 2016-02-20 at 5 29 12 pm
jathanism commented 8 years ago

Hmm... That's quirky. We recently had to adjust the regex pattern for pk lookups to support natural key lookups on the views for Network objects . So that's the first place I would look.

If you want to look around, start here at nsot.api.views.NetworkViewSet.lookup_value_regex:

https://github.com/dropbox/nsot/blob/master/nsot/api/views.py#L462

This affects what ends up being a part of the URL dispatch pattern that can be inspected here:

https://github.com/dropbox/nsot/blob/master/nsot/api/urls.py#L40

You can inspect what the generated URL patterns are by doing this:

$ nsot-server shell_plus
# Shell Plus Model Imports
from django.contrib.admin.models import LogEntry
from django.contrib.auth.models import Group, Permission
from django.contrib.contenttypes.models import ContentType
from django.contrib.sessions.models import Session
from nsot.models import Assignment, Attribute, Change, Device, Interface, Network, Site, User, Value
# Shell Plus Django Imports
from django.utils import timezone
from django.conf import settings
from django.core.cache import cache
from django.db.models import Avg, Count, F, Max, Min, Sum, Q, Prefetch
from django.core.urlresolvers import reverse
from django.db import transaction
fPython 2.7.3 (default, Jun 22 2015, 19:33:41)
Type "copyright", "credits" or "license" for more information.

IPython 3.1.0 -- An enhanced Interactive Python.
?         -> Introduction and overview of IPython's features.
%quickref -> Quick reference.
help      -> Python's own help system.
object?   -> Details about 'object', use 'object??' for extra details.

In [1]: from nsot.api import urls

In [2]: urls.urlpatterns[0].url_patterns
Out[2]:
[<RegexURLPattern site-list ^sites/$>,
 <RegexURLPattern site-detail ^sites/(?P<pk>[^/.]+)/$>,
 <RegexURLPattern attribute-list ^attributes/$>,
 <RegexURLPattern attribute-query ^attributes/query/$>,
 <RegexURLPattern attribute-detail ^attributes/(?P<pk>[^/.]+)/$>,
 <RegexURLPattern change-list ^changes/$>,
 <RegexURLPattern change-detail ^changes/(?P<pk>[^/.]+)/$>,
 <RegexURLPattern device-list ^devices/$>,
 <RegexURLPattern device-query ^devices/query/$>,
 <RegexURLPattern device-detail ^devices/(?P<pk>[^/.]+)/$>,
 <RegexURLPattern device-interfaces ^devices/(?P<pk>[^/.]+)/interfaces/$>,
 <RegexURLPattern interface-list ^interfaces/$>,
 <RegexURLPattern interface-query ^interfaces/query/$>,
 <RegexURLPattern interface-detail ^interfaces/(?P<pk>[^/.]+)/$>,
 <RegexURLPattern interface-addresses ^interfaces/(?P<pk>[^/.]+)/addresses/$>,
 <RegexURLPattern interface-assignments ^interfaces/(?P<pk>[^/.]+)/assignments/$>,
 <RegexURLPattern interface-networks ^interfaces/(?P<pk>[^/.]+)/networks/$>,
 <RegexURLPattern network-list ^networks/$>,
 <RegexURLPattern network-query ^networks/query/$>,
 <RegexURLPattern network-reserved ^networks/reserved/$>,
 <RegexURLPattern network-detail ^networks/(?P<pk>[a-fA-F0-9:.]+(?:\/\d+)?)/$>,
 <RegexURLPattern network-ancestors ^networks/(?P<pk>[a-fA-F0-9:.]+(?:\/\d+)?)/ancestors/$>,
 <RegexURLPattern network-assignments ^networks/(?P<pk>[a-fA-F0-9:.]+(?:\/\d+)?)/assignments/$>,
 <RegexURLPattern network-children ^networks/(?P<pk>[a-fA-F0-9:.]+(?:\/\d+)?)/children/$>,
 <RegexURLPattern network-descendents ^networks/(?P<pk>[a-fA-F0-9:.]+(?:\/\d+)?)/descendents/$>,
 <RegexURLPattern network-next-address ^networks/(?P<pk>[a-fA-F0-9:.]+(?:\/\d+)?)/next_address/$>,
 <RegexURLPattern network-next-network ^networks/(?P<pk>[a-fA-F0-9:.]+(?:\/\d+)?)/next_network/$>,
 <RegexURLPattern network-parent ^networks/(?P<pk>[a-fA-F0-9:.]+(?:\/\d+)?)/parent/$>,
 <RegexURLPattern network-root ^networks/(?P<pk>[a-fA-F0-9:.]+(?:\/\d+)?)/root/$>,
 <RegexURLPattern network-siblings ^networks/(?P<pk>[a-fA-F0-9:.]+(?:\/\d+)?)/siblings/$>,
 <RegexURLPattern network-subnets ^networks/(?P<pk>[a-fA-F0-9:.]+(?:\/\d+)?)/subnets/$>,
 <RegexURLPattern network-supernets ^networks/(?P<pk>[a-fA-F0-9:.]+(?:\/\d+)?)/supernets/$>,
 <RegexURLPattern user-list ^users/$>,
 <RegexURLPattern user-detail ^users/(?P<pk>[^/.]+)/$>,
 <RegexURLPattern user-rotate-secret-key ^users/(?P<pk>[^/.]+)/rotate_secret_key/$>,
 <RegexURLPattern value-list ^values/$>,
 <RegexURLPattern value-detail ^values/(?P<pk>[^/.]+)/$>]

If it's not somewhere with this? We'd need to dig into how the django-rest-swagger lib is generating the Swagger stuff viewed at /docs/.

TheSkorm commented 8 years ago

Played around with the lookup_value_regex. Removing the non capturing group fixes the swagger issue but naturally breaks the API. Will need to investigate django-rest-swagger I think

jathanism commented 8 years ago

@TheSkorm I was troubleshooting this for a different reason and ended up tracking it down to this:

https://github.com/jason-kane/django-rest-swagger/blob/master/rest_framework_swagger/urlparser.py#L109

Which is calling django.contrib.admindocs.views.simplify_regex!

jathanism commented 8 years ago
In [1]: from django.contrib.admindocs.views import simplify_regex

In [2]: pattern = u'^networks/(?P<pk>[a-fA-F0-9:.]+(?:\\/\\d+)?)/$'

In [3]: prefix = u'^api/^'

In [4]: simplify_regex(prefix + pattern)
Out[4]: u'/api/networks/<pk>)/'