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

Internal improvements to the IPAM coupling feature #102

Merged
merged 1 commit into from
Dec 21, 2023

Conversation

peteeckel
Copy link
Owner

@peteeckel peteeckel commented Nov 20, 2023

This PR fixes a couple of quirks around the experimental IPAM coupling feature.

fixes #105
fixes #67

  • IPAM coupling did not work on model layer, so when a new IP address was created i.e. via IPAddress.create() no DNS record would be created
  • IPAM coupling tests were incomplete. Object level permissions weren't tested at all (see below), and some operations such as deleting an IP address without the permission to delete address records neither
  • Object permissions were not enforced. The main reason was that the code designed to validate them ran on model level, where no permissions are enforced at all
  • Deleting an IP address with an associated DNS record without the (model level) permission to delete DNS records resulted in an internal server error (500) instead of an error message

The necessary changes resulted in a major redesign of the underlying middleware and the addition of custom signal handlers for requests on IPAddress views. As a result, the code should be more stable and somewhat more concise than before, and better perpared for upcoming developments in the IPAM coupling feature.

@peteeckel peteeckel marked this pull request as draft December 5, 2023 10:40
netbox_dns/signals/ipam_coupling.py Dismissed Show dismissed Hide dismissed
@peteeckel peteeckel changed the title Internal improvenments to the IPAM coupling feature Internal improvements to the IPAM coupling feature Dec 13, 2023
@peteeckel peteeckel force-pushed the fix/ipam-coupling-model branch 2 times, most recently from 7f48258 to 9c7dd3e Compare December 15, 2023 21:01
* Removed the middleware code as it turned out to be too limited for the purpose. More
  specifically, it is not used for model-level changes to objects, so it doesn't work
  with Custom Scripts etc. (see issue #105)
* All checks and operations are now done in signal handlers for ipam.IPAddress that are
  part of NetBox DNS. Future releases (dropping support for NetBox < 3.7) could be
  using CUSTOM_VALIDATORS and PROTECTION_RULES instead.
* Testing was extended to cover model-level actions and API level actions to IPAddress
  objects.
* Improved DNS record permission checks.

Open issue:

* Although actions with missing permissions to delete DNS records work properly in the
  API and the GUI, the respective tests have to be skipped as they fail in the context
  of unit testing. Potential reasons include a bug in the test framework. This does not
  have any functional implications and just affects tests for the deletion of IPAddress
  objects with missing NetBox DNS (object) permissions.
@peteeckel peteeckel marked this pull request as ready for review December 21, 2023 23:19
@peteeckel peteeckel merged commit ef3fb7b into main Dec 21, 2023
14 checks passed
@peteeckel peteeckel deleted the fix/ipam-coupling-model branch December 21, 2023 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using IPAM-DNS coupling from a custom script IPAM-DNS coupling
1 participant