NetBox 3.7 compatibility

[peteeckel]

This is a quick fix to make the plugin compatible with NetBox 3.7.

Given the pre-alpha status of the plugin and the lack of automated tests I did not make any effort to check compatibility with earlier versions (though I would expect it to work down to NetBox 3.5), but these fixes to the job to get the plugin running on my 3.7.3 test instance.

[peteeckel]

Unfortunately the plugin is broken in some other places as well, so I'll need a bit more time to actually get it to work with recent NetBox releases.

[renatoalmeidaoliveira]

Thanks @peteeckel for contributing on this plugin :D. In the first release of this plugin I was thinking in a plugin that was backward compatible with all versions, but that requirement is making the code very ugly difficult to mantain, so maybe for the 3.7 compatibility release It might be great to remove all netbox version testing and add NetBoxModel features as well.

[peteeckel]

Hi Renato, thanks for the feedback!

I saw that you jumped through some hoops to ensure compatibility with a broad range of NetBox releases, but I think you're right in that will eventually just clutter up the code and create lots of ugliness, for not much - very few users actually still have NetBox 2.x running, and even in the 3.x releases I see a tendency towards using the last two or three minor releases at most.

With the release of NetBox 4.0 I'm planning a new release train for NetBox DNS, putting the old one into maintenance mode and just fixing bugs. That makes it possible to concentrate on the latest version of NetBox while not leaving users of the last few versions too much behind. Currently I can simultaneously support 3.5, 3.6 and 3.7, and that should be enough for most cases.

If you don't mind I'll prepare a full PR for 3.5-3.7, and if I find the time I'll also add the standard NetBox test suite tests (test_api, test_views and test_filtersets). I currently have a use case for your plugin, so I'm as interested as you to get it to work :-)

[renatoalmeidaoliveira]

Sure, I already started to make some changes and found a very unpleasant problem... If we just use NetBoxModel the TAG model creates a namespace collision with the reverse naming with IP.SERVICE.TAG, so I was thinking in use everything of NetBoxModel instead of TAG, or the other option would be change the Service class name, but that might break users in the older versions.... And when changing the models I got problems with the API's too

[renatoalmeidaoliveira]

@peteeckel pushed some changes fixing API calls, and changed the base class for the models.

[peteeckel]

Thanks, much better, but there are still some things missing ...

• There is no serializer for PenTest
• When you use the NetBoxModel base class there must be an ObjectChangeLogView URL for all classes
• There's still some backward compatibility clutter in urls.py - do you want to get rid of that altogether?

I'm not quite sure to what extent to invest work as we are obviously working on the same issues at the same time - I'll be glad to help, though, if we can find a way to proceed without doubling effort.

Best regards,

Peter.

[renatoalmeidaoliveira]

@peteeckel you kinda give me some motivation to fix thing up here :D, thanks :D. If you could help on the standart tests, I haven't experience with that kind of stuff in Django.

[peteeckel]

If you could help on the standart tests, I haven't experience with that kind of stuff in Django.

Sure, I'll start pushing out some tests as soon as I find the time. Glad to hear you're motivated to work on this plugin, it really looks useful and I have at least one application for it!

[peteeckel]

Hi @renatoalmeidaoliveira, I pushed a first set of standard tests (for the Service model), just for starters. Running them looks pretty horrible, but don't be worried, that's normal :-)

I could identify at least two problems at once:

• Some URLs are missing ... bulk update, import, etc. do not work because the endpoints are not defined in urls.py, and the views required for them are missing.
• Your filters inherit from django_filters.FilterSet instead of NetBoxModelFilterSet, which causes some fields to be considered filterable that in fact are not ... this can all be fixed by adjusting your models to the NetBox model classes.

Both issues can probably be fixed by adjusting the classes you inherit from. It's not wrong, it just doesn't match the NetBox tests in the standard test classes.

[renatoalmeidaoliveira]

@peteeckel thanks, started to move all views and templates to the new format, already done with Service model, and created the API for the PenTest model too, but I may need to add some views for it too

[peteeckel]

Hi @renatoalmeidaoliveira, I managed to get all tests to succeed now.

In some cases I needed to change the inheritance for Views, Forms etc. so the standard NetBox features all work - which is prerequisite for the tests to succeed (and in most cases for the GUI and API not to do strange things :-)). There still is some code cleanup to do, I only fixed the stuff that is absolutely necessary for the tests to work. As a side product, now bulk edit, delete and import are possible.

There are still some issues with fields being required or not - it's better to state explicitly whether a model field is nullable or may be blank than to rely on the defaults, which are sometimes unexpected. I made some decisions that you might want to review, for example I set the backup_profile field for Service to optional - there were mixed settings for this field, so I had to choose one.

So much for Service - there's still a lot to do, but we're making progress.

(netbox) [root@dns netbox]# /opt/netbox/netbox/manage.py test --keepdb nb_service.tests.service
Found 47 test(s).
Using existing test database for alias 'default'...
System check identified no issues (0 silenced).
...............................................
----------------------------------------------------------------------
Ran 47 tests in 3.026s

OK
Preserving test database for alias 'default'...

[renatoalmeidaoliveira]

@peteeckel you made the tags work :D :D. I need to change one of the migrations because it was referencing a model outside NBService, that way old NetBox vesions would fail. Removed the old views of 2x and changed the Service UI to use Cards too

[peteeckel]

Hi @renatoalmeidaoliveira, yes, tags can be a bit tricky when there are duplicate model names, but fortunately this is easy to fix.

Regarding the reference to NetBox core models, this is there for a reason, although you probably will never run into it. If you want to keep the depencency in the migration you can use an older one that still contains the models you need, but that's still available in the oldest version you want to support.

Generally, it is really difficult (and leads to very ugly code, as you already noted :-)) to support more than the last two or three older versions. Personally, I wouldn't bother - as soon as NetBox 4 is releast, many things will break anyway, so it's better to start with a clean slate and denote a max_version for a specific release and a min_version for the new one. You can still support both to a certain extent, but there's no use in cutting yourself off from new NetBox (or Python!) features.

[renatoalmeidaoliveira]

@peteeckel Got it, my dev environment is in 3.7.0 so that migration already breaks since 0107_cachedvalue_extras_cachedvalue_object was introdcted at 3.7.3 ....

[peteeckel]

Hi Renato, this should do ... referencing an older migration is the cleanest version IMHO. On a fresh install it's important for the migration that the order is maintained, so this is the safe bet.

[peteeckel]

Here we go :-)

(netbox) [root@dns netbox]# /opt/netbox/netbox/manage.py test --parallel --keepdb nb_service
Found 100 test(s).
Using existing test database for alias 'default'...
Using existing clone for alias 'default'...
Using existing clone for alias 'default'...
Using existing clone for alias 'default'...
Using existing clone for alias 'default'...
System check identified no issues (0 silenced).
....................................................................................................
----------------------------------------------------------------------
Ran 100 tests in 3.442s

OK
Preserving test database for alias 'default'...
Preserving test database for alias 'default'...
Preserving test database for alias 'default'...
Preserving test database for alias 'default'...
Preserving test database for alias 'default'...

Service and Application are fine now (at least with the GUI/API/filter tests, no model testing is done so far).

[renatoalmeidaoliveira]

@peteeckel removed all compatibility code including some copy of NetBox code customized for plugins that aren't needed any more. The only thing bugging me is with the Application model, since we doesn't have a intermediary model we can't add models to the Application without going to the ApplicationEdit view, and that king reduces the usability of the model, and the Chield View tables call thes Model Edit/Delete outside the Application Context

[peteeckel]

Hi @renatoalmeidaoliveira, looks good!

How do we want to proceed? In order to create the tests from the NetBox test suite, lots of views/forms/filters etc. for IC, Relation and PenTest are still missing. Does it make sense at all to create them, or would you prefer to only have a subset of the features (maybe they aren't even required for the functionality you intend) tested?

Strictly speaking the plugin can be loaded on a 3.7 system now, but I saw some exceptions trying to play around with it that are not 3.7 incompatibility issues. I can finish this PR and, if you want, create new ones for the remaining issues and test coverage. Might take some time as I'm rather busy at the moment, but eventually we'll get there.

[peteeckel]

The only thing bugging me is with the Application model, since we doesn't have a intermediary model we can't add models to the Application without going to the ApplicationEdit view, and that king reduces the usability of the model, and the Chield View tables call thes Model Edit/Delete outside the Application Context

I'm afraid I didn't dive into the inner workings of the plugin deep enough to understand what the problem is ... if I can somehow figure that out maybe I can find some solutions, but currently I'm completely in the dark.

[renatoalmeidaoliveira]

@peteeckel fixed the last things I had in mind, thank you very much for the help, merging this PR and publishing the package :D. If you have any other issue or contribution please let me know.

[renatoalmeidaoliveira]

Fixes #12 and #10