Add branching support and tests

[arthanson]

Fixes merge failures when NetBox DNS is used alongside the NetBox Branching plugin (creating zones, or deleting objects with DNS records in a branch and merging, previously failed on both iterative and squash strategies).

The main change is just telling branching to include the M2M models - this is done through a branching resolver.

There were a couple other places where the clean or save were doing some extra fixup of records that was causing issues with merge/revert. For now put in a check with if dns_branch_replay_active(): this could probably be refactored and/or cleaned up as haven't run into that with netbox or other plugins.

Note: Also add some misc test fixes unrelated to this PR

Root cause

1. M2M through models weren't branch-aware. Branching routes a model to the active branch's schema only if it inherits ChangeLoggingMixin. The auto-generated through models behind Zone.nameservers, View.prefixes, ZoneTemplate.nameservers/record_templates and DNSSECPolicy.key_templates are plain models.Model subclasses, so their rows were written to main instead of the branch.

2. Reconciliation side effects ran during replay. DNS regenerates managed SOA/NS/PTR records and SOA serials via save()/delete()/m2m_changed. During a merge/sync/revert, Branching replays the recorded changes - which already include those managed records - so the side effects recreated them, colliding with the replayed copies (e.g. "there is already an active SOA record for name @").

Changes

• netbox_dns/branching.py (new): a branching resolver that marks DNS models and their M2M through models branchable, and a thread-local replay guard.
• __init__.py: registers the resolver and wires the replay guard to Branching's pre/post merge/sync/revert signals in ready(). Fully guarded by try/except ImportError - no effect when Branching isn't installed.
• zone.py, record.py, nameserver.py, signals/dnssec.py: save/delete/clean/clean_fields and the M2M receivers skip their reconciliation while a replay is active, letting the changelog reproduce the branch state exactly. No-ops in normal operation.
• Tests: netbox_dns/tests/branching/ - a TransactionTestCase harness plus 12 merge/revert tests run under both merge strategies (including a comprehensive one-of-every-model round trip, all M2M relations, a sync test, and resolver unit tests). Adds configuration_branching.py and a test-netbox-branching CI job.

Notes

• The integration is optional: DNS behaves identically without Branching installed.
• The CI job pins netboxlabs-netbox-branching>=1.0.4 (the version exposing register_branching_resolver).

[arthanson]

@peteeckel this looks like it fixes the branching issues and adds some test suites for branching. The actual core changes are fairly straight-forward (see my comments in the PR description). The changes to the save and clean should probably be looked at in the future and refactored out. I also needed to cleanup some issues with the test suite, unrelated to this change.

Should definitely give it a good once-over as I'm not an expert with this codebase, also I did spot checks and tried to make the test suite cover everything - but probably worth some extra spot checks with branching if you know of any specific problem areas.

[peteeckel]

Hi @arthanson, first of all thank you very much for the effort you put into this. Your results are really pushing the issue forward, and I will see how far I get building upon this foundation.

I still see the save()/clean()/delete() issue as one of the remaining points. For example, when you change something in a zone, calling save() results in the SOA record for the zone being updated as well. This should of course happen in the same branch in which the zone change was done, so I will need to pass on the using value to the subsequent save() calls. Correct? If so, this seems like something I can handle quite mechanically.

Once that's done, I'm planning to release an 1.6 beta release with branching support and see how it turns out in the field. I'm really glad this finally seems to be resolved!

Thanks for fixing the tests as well. I'll backport your changes to 1.5.10 before going any further,

[arthanson]

@peteeckel that sounds good. For the save()/clean()/delete() issues I didn't dig down too much as I'd have to be a lot more familiar with the codebase. I was curious as we haven't run into an issue like this in NetBox or in any other plugin - I don't think you are doing anything really wrong there, but it can probably be refactored so these aren't an issue - I'm thinking in the direction of signals possibly, but again haven't dug down into it and the current shim looks like it works, although it is a bit of a hack.

re: calling save() results in the SOA record for the zone being updated as well... so I will need to pass on the using value ... - I don't think so. If you are in a branch SOA should be a branch aware model so the code should just auto save in the branch under the covers, you shouldn't have to worry in the plugin code where it is routed, this should be handle automatically. Then when you merge that SOA record is already created in the branch and will get merged, so it doesn't need to be auto updated in the save() code again. If I'm understanding what you are asking correctly.

The whole thing with the shim in save()/clean() was because during merge it was saving the base item then other items were getting created/updated in the save() but that had already previously been done when they were originally created (and those records are getting merged as well) so it was basically happening twice which was causing issues.

Also one thing I forgot to add above, currently the routine _m2m_through_models iterates through M2M models in dns, in branching we just use an INCLUDE_MODELS list and update that, I made it dynamic here to catch future changes but you might want switch it to a simple INCLUDE_MODELS as the code is simpler and more explicit.

[peteeckel]

Hi @arthanson, I just created a new 1.6 beta release branch and merged your PR into that. Is it OK if I close this one? I think it makes more sense to continue work on the 1.6 branch and merge that one when we're sufficiently confident it will work in production.

[peteeckel]

Hi Art, the changes look OK to me - I'll run manual tests over the next couple of weeks when I find the time, and see if additional tests make sense.