vapor-ware / netbox-virtual-circuit-plugin

A plugin for NetBox that supports Virtual Circuit management
GNU General Public License v3.0
57 stars 19 forks source link

Fix swagger missing id #35

Closed hoanhan101 closed 4 years ago

hoanhan101 commented 4 years ago

Follow up from #34, we realize that the missing id is the unique identifier of Virtual-Circuit-to-VLAN (VCVLAN) connections. The previous implementation overrode id to return the nested VLAN objects and hence broke the Swagger specs. This PR fixes the issue by reserving that id key and use vlan key for querying VLAN objects instead.

This PR also removes the depth level so that only VLAN id and VCVLAN connection id will be returned - for a couple of reasons:

So now, the response instead of looking like this:

{
  "count": 1,
  "next": null,
  "previous": null,
  "results": [
    {
      "vcid": 101,
      "name": "curl",
      "status": "pending-configuration",
      "context": "",
      "vlans": [
        {
          "vlan": {
            "id": 1,
            "created": "2020-06-02",
            "last_updated": "2020-06-02T19:08:07.817046Z",
            "vid": 1,
            "name": "foo",
            "status": "active",
            "description": "",
            "site": null,
            "group": null,
            "tenant": null,
            "role": null
          }
        },
        {
          "vlan": {
            "id": 2,
            "created": "2020-06-02",
            "last_updated": "2020-06-02T19:08:12.453937Z",
            "vid": 2,
            "name": "bar",
            "status": "active",
            "description": "",
            "site": null,
            "group": null,
            "tenant": null,
            "role": null
          }
        }
      ]
    }
  ]
}

will look like this:

{
  "count": 1,
  "next": null,
  "previous": null,
  "results": [
    {
      "vcid": 101,
      "name": "curl",
      "status": "pending-configuration",
      "context": "",
      "vlans": [
        {
          "id": 1,
          "vlan": 1
        },
        {
          "id": 2,
          "vlan": 2
        }
      ]
    }
  ]
}

The POST body for creating a Virtual Circuit with VLANs data instead of looking like this:

➜  ~ curl -X POST -H "Authorization: Token REDACTED" -H "Content-Type: application/json" -H "Accept: application/json; indent=4" http://localhost:8000/api/plugins/virtual-circuit/virtual-circuits/ --data '{"vcid": 101, "name": "curl", "vlans": [{"id": 1}, {"id": 2}]}'

will look like this:

➜  ~ curl -X POST -H "Authorization: Token REDACTED" -H "Content-Type: application/json" -H "Accept: application/json; indent=4" http://localhost:8000/api/plugins/virtual-circuit/virtual-circuits/ --data '{"vcid": 101, "name": "curl", "vlans": [{"vlan": 1}, {"vlan": 2}]}'

where vlan is used instead of reserved id key.

jinaloo7 commented 4 years ago

I don't think this response is appropriate specially the way Network-api is going to consume.From yesterday's talk during Marco's office hours we require organization(tenant), interface, device info.If this way response will be provided through virtual_circuit api response then it will increase overhead while consuming.

hoanhan101 commented 4 years ago

@jinaloo7 @marcoceppi I think A Virtual Circuit should only know about its associated VLANs but nothing more. Trying to do many things at one and include organization/tenant, interface, device info in the response is outside of scope for this implementation. Also, this above information is only meaningful to Vapor internals and can be handled internally. Since the goal of this plugin to provide a clean, single-purpose, pluggable interface after all.