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

fix: add backwards compatible support for mac_address field #62

Merged
merged 2 commits into from
Feb 13, 2025

Conversation

ltucker
Copy link

@ltucker ltucker commented Feb 12, 2025

This fix adds backwards compatible support for specifying interface mac addresses as mac_address in the diode apply changeset api for dcim.interface and virtualization.vminterface. In netbox 4.2 there is really a list of mac addresses associated with an interface and one can be specified as the primary. Until this is supported in diode, the mac_address field specified when applying a change set is treated as the specification of the primary mac address for the object being created or updated.

caveats

This does not disassociate any mac addresses ... it just ensures that the specified mac address is present and promotes it to the primary mac address. So a changing mac address would leave a set of old mac addresses associated until multiple associations are supported directly. Alternatively we could make the specified mac address the only associated address? Not sure which is better.

etc

This pull request introduces a new method to handle backward compatibility for MAC addresses in interfaces and updates the relevant tests to ensure this functionality works correctly. The most important changes include adding the _handle_interface_mac_address_compat method, updating the post method to use this new method, and adding new test cases to verify the backward compatibility.

Backward Compatibility for MAC Addresses:

Updates to the post Method:

  • netbox_diode_plugin/api/views.py: Updated the post method to call _handle_interface_mac_address_compat for dcim.interface and virtualization.vminterface object types.

Test Enhancements:

Copy link

github-actions bot commented Feb 13, 2025

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
1346 1323 98% 0% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
netbox_diode_plugin/api/views.py 93% 🟢
netbox_diode_plugin/tests/test_api_apply_change_set.py 100% 🟢
TOTAL 97% 🟢

updated for commit: aecb728 by action🐍

Copy link
Member

@mfiedorowicz mfiedorowicz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙌

@ltucker ltucker merged commit 04fdc42 into fix-object-state-retrieval Feb 13, 2025
2 checks passed
@ltucker ltucker deleted the fix_mac_address_42 branch February 13, 2025 16:04
mfiedorowicz added a commit that referenced this pull request Feb 14, 2025
* fix: retrieve object states using concrete models

Search backend/CachedValue is global and doesn't seem to be reliable with branching

Signed-off-by: Michal Fiedorowicz <[email protected]>

* tidy up

Signed-off-by: Michal Fiedorowicz <[email protected]>

* chore: bump netbox min version and netbox-docker

Signed-off-by: Michal Fiedorowicz <[email protected]>

* chore: bump netbox min version

Signed-off-by: Michal Fiedorowicz <[email protected]>

* fix tests

Signed-off-by: Michal Fiedorowicz <[email protected]>

* use clean_fields() and tidy up

Signed-off-by: Michal Fiedorowicz <[email protected]>

* fix: add backwards compatible support for mac_address field (#62)

* fix: add backwards compatible support for mac_address field

* feat: extract and serialize site from scope

Signed-off-by: Michal Fiedorowicz <[email protected]>

* feat: apply change set - handle scope site

Signed-off-by: Michal Fiedorowicz <[email protected]>

---------

Signed-off-by: Michal Fiedorowicz <[email protected]>
Co-authored-by: Luke Tucker <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants