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

Implement linking of IP Adresses to newly created device. #73

Merged
merged 17 commits into from
Sep 5, 2024

Conversation

ByteOtter
Copy link
Member

@ByteOtter ByteOtter commented Apr 23, 2024

What does this PR change?

Currently we can only create devices without the IP address fields being set.

This PR aims to fix that. For this, after device creation, we need to create an Interface for the device, create an IP address object and then PATCH the device with the ID of its primary IP address linked to one of its devices.

As references serve these threads:

netbox-community/netbox#8746

netbox-community/netbox#8837

Tick the applicable box:

  • Add new feature
  • Security changes
  • Tests
  • Documentation changed

  • General Maintenance

Links

Fixes: #69

  • DONE

Documentation

  • No documentation needed

  • DONE

@ByteOtter ByteOtter self-assigned this Apr 23, 2024
@github-actions github-actions bot added rust configuration This issue is linked to the application configuration publisher translator labels Apr 23, 2024
@ByteOtter
Copy link
Member Author

ByteOtter commented Apr 25, 2024

Currently we do not get a valid JSON response body when the creation is successful. We need to investigate if this issue is on our end or on theirs.
A possible workaround could look like this:

  1. Create interface only checking for valid errors and ignoring JSON parsing errors.
  2. Get list of all Interfaces from NetBox
  3. Search list for the interface we just created. Best guess would be the MAC-Address?

Problems with this workaround:

  • Will not work for prototype hardware without a valid MAC-Address
  • MAC-Address and name fields are not necessarily unique or even present as they are optional

Edit: The problem seems to be with us as running it manually:

curl -X POST https://demo.netbox.dev/api/dcim/interfaces/ \
-H "Authorization: Token $TOKEN" \
-H "Content-Type: application/json" \
-d '{"name": "Nazara-Test", "device": 112, "type": "1000base-t"}' \
-o ~/Documents/test.txt

Returns what appears to be a valid JSON:

{
    "id":2494,
    "url":"https://demo.netbox.dev/api/dcim/interfaces/2494/",
    "display":"Nazara0",
    "device":{
        "id":112,"url":"https://demo.netbox.dev/api/dcim/devices/112/",
        "display":"CISCO",
        "name":"CISCO"
        },
    "vdcs":[],
    "module":null,
    "name":"Nazara0",
    "label":"",
    "type":
        {
        "value":"1000base-t",
        "label":"1000BASE-T (1GE)"
        },
    "enabled":true,
    "parent":null,
    "bridge":null,
    "lag":null,
    "mtu":null,
    "mac_address":null,
    "speed":null,
    "duplex":null,
    "wwn":null,
    "mgmt_only":false,
    "description":"",
    "mode":null,
    "rf_role":null,
    "rf_channel":null,
    "poe_mode":null,
    "poe_type":null,
    "rf_channel_frequency":null,
    "rf_channel_width":null,
    "tx_power":null,
    "untagged_vlan":null,
    "tagged_vlans":[],
    "mark_connected":false,
    "cable":null,
    "cable_end":"",
    "wireless_link":null,
    "link_peers":[],
    "link_peers_type":null,
    "wireless_lans":[],
    "vrf":null,
    "l2vpn_termination":null,
    "connected_endpoints":null,
    "connected_endpoints_type":null,
    "connected_endpoints_reachable":null,
    "tags":[],
    "custom_fields":{},
    "created":"2024-04-25T11:55:29.998975Z",
    "last_updated":"2024-04-25T11:55:29.999015Z",
    "count_ipaddresses":0,
    "count_fhrp_groups":0,
    "_occupied":false
}

This does not make sense though as we use serde to handle responses and other endpoints work flawlessly.

Edit Edit:

It looks like l2vpn_termination is null. This is fine for creation but cannot be parsed as the struct serde tries to make out of the response (DcimInterfacesCreateResponse) said that it is not an Option.

according to netbox's code this should be nullable. Meaning their spec is wrong...

l2vpn_termination = NestedL2VPNTerminationSerializer(read_only=True, allow_null=True)

@ByteOtter ByteOtter force-pushed the dev/link-ip-adresses branch 2 times, most recently from 29600fa to 2dd45d8 Compare April 25, 2024 14:09
@ByteOtter ByteOtter force-pushed the dev/link-ip-adresses branch from d17e7de to f4ff496 Compare May 2, 2024 11:30
@ByteOtter ByteOtter force-pushed the dev/link-ip-adresses branch from 0d6e9be to e65a530 Compare May 27, 2024 08:08
@ByteOtter ByteOtter force-pushed the dev/link-ip-adresses branch from 104b47a to 719c54e Compare June 27, 2024 06:35
@ByteOtter ByteOtter force-pushed the dev/link-ip-adresses branch from 0ede402 to 216cb16 Compare July 16, 2024 16:28
@ByteOtter ByteOtter force-pushed the dev/link-ip-adresses branch from e60b1ad to 216cb16 Compare July 24, 2024 21:14
@ByteOtter ByteOtter force-pushed the dev/link-ip-adresses branch from efa3958 to 89c5b1e Compare August 16, 2024 13:56
@ByteOtter
Copy link
Member Author

After further investigation, it definitely looks like this is a problem on NetBox's end.
Looking at the fields link_peers_type and connected_endpoints in the Interface struct, we can see that the schema requires them to be a String and a Vec. Which gets translated correctly in thanix_client.
However, the application returns both of these values as null, which does not correspond with the schema.

This leads to several conclusions:

  1. This could a problem that can re-occur in more fields than the ones we encountered here.
  2. A temporary workaround is required to allow v0.1.0 to be working with NetBox v3.6.x as per the decision made in [TASK] Port API calls to NetBox v4.x #74.
  3. An automated integration test suite is required to catch these errors correctly.

Backend bug hunting must also be conducted as we cannot be sure this problem is fixed with NetBox v4.x.
I had opened an issue with NetBox about a possible bug on this end (netbox-community/netbox#16301) they asserted that this is not an issue with them. We need to collect further information before we can proceed to report a bug again.

@ByteOtter ByteOtter force-pushed the dev/link-ip-adresses branch from 751b299 to df2053e Compare September 4, 2024 11:17
Copy link
Contributor

@marv7000 marv7000 left a comment

Choose a reason for hiding this comment

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

LGTM

@ByteOtter ByteOtter marked this pull request as ready for review September 5, 2024 11:44
Copy link
Collaborator

@SchoolGuy SchoolGuy left a comment

Choose a reason for hiding this comment

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

LGTM. The fine details can be fixed later. :)

@ByteOtter ByteOtter merged commit a972201 into The-Nazara-Project:main Sep 5, 2024
4 checks passed
@ByteOtter ByteOtter deleted the dev/link-ip-adresses branch September 5, 2024 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[TASK:] Automatically link IP Adresses to device in NetBox.
3 participants