starred/netbox
1
0
Fork 0
mirror of https://github.com/netbox-community/netbox.git synced 2026-01-11 21:10:29 +01:00
Code Issues 273 Packages Projects Releases 10 Wiki Activity

Fix WritableAvailableIP related API documentation #7958

New Issue
Closed
opened 2025-12-29 20:30:29 +01:00 by adam · 6 comments
adam commented 2025-12-29 20:30:29 +01:00
Owner
Copy Link

Originally created by @agowa on GitHub (Apr 28, 2023).

Change Type

Correction

Area

Integrations/API

Proposed Changes

The documentation of this API object has two issues:

  1. No attributes for WritableAvailableIP are documented. From the swagger docs it looks like it doesn't have any. Until I discovered a comment in #11578 I thought it's a stub intended for future use by the API.
  2. For multiple API requests the datastructure for it is documented incorrectly. It is documented as "{}" instead of "[{}]", this in turn also causes the returned datastructure to be different from what is documented.

I discovered this issue because of POST /api/ipam/ip-ranges/{id}/available-ips/, where the documentation says it needs an empty object as body.
image
image
Such a request:

curl -X 'POST' \
  'http://localhost:8000/api/ipam/ip-ranges/4/available-ips/' \
  -H 'accept: application/json' \
  -H 'Content-Type: application/json' \
  -H 'X-CSRFToken: *****' \
  -d '{}'

Does not match the documented return type of "List of IPAddress" but instead it returns just IPAddress (I.E. if request is object, response is object. If request is list the response is a list as well).

I think the request is documented incorrectly here, as passing a list of WritableAvailableIP also works and results in exactly the documented return type. It also is way more logical and meaningful.
Assuming passing a single object of WritableAvailableIP was never intended to begin with, and to NOT cause any breaking changes, I'd suggest to fix this issue by changing the datatype within the request to "List of WritableAvailableIP". That way we don't have to care about this strange and hard to handle different return types.

Tl;Dr:

  • Document all possible attributes for WritableAvailableIP (the request-body of e.g. POST /api/ipam/ip-ranges/{id}/available-ips/)
  • Change the documentation of the requests data structure from WritableAvailableIP to [WritableAvailableIP] (but keep current API behavior, but make the current [passing a single object instead of a list with objects] undocumented and thereby "just works by coincidence, don't rely on it and use the correct one").
  • Check all other usages of WritableAvailableIP for a similar error and also fix that.
Originally created by @agowa on GitHub (Apr 28, 2023). ### Change Type Correction ### Area Integrations/API ### Proposed Changes The documentation of this API object has two issues: 1. No attributes for WritableAvailableIP are documented. From the swagger docs it looks like it doesn't have any. Until I discovered a comment in #11578 I thought it's a stub intended for future use by the API. 2. For multiple API requests the datastructure for it is documented incorrectly. It is documented as "{}" instead of "[{}]", this in turn also causes the returned datastructure to be different from what is documented. I discovered this issue because of `POST /api/ipam/ip-ranges/{id}/available-ips/`, where the documentation says it needs an empty object as body. ![image](https://user-images.githubusercontent.com/2544867/235118400-4db233a4-76cc-4df5-8a32-8ee0af04bc20.png) ![image](https://user-images.githubusercontent.com/2544867/235120537-e1d0f3e3-575e-4cc8-a44d-2ad61564e3ad.png) Such a request: ```sh curl -X 'POST' \ 'http://localhost:8000/api/ipam/ip-ranges/4/available-ips/' \ -H 'accept: application/json' \ -H 'Content-Type: application/json' \ -H 'X-CSRFToken: *****' \ -d '{}' ``` Does not match the documented return type of "List of IPAddress" but instead it returns just IPAddress (I.E. if request is object, response is object. If request is list the response is a list as well). I think the request is documented incorrectly here, as passing a list of WritableAvailableIP also works and results in exactly the documented return type. It also is way more logical and meaningful. Assuming passing a single object of WritableAvailableIP was never intended to begin with, and to NOT cause any breaking changes, I'd suggest to fix this issue by changing the datatype within the request to "List of WritableAvailableIP". That way we don't have to care about this strange and hard to handle different return types. Tl;Dr: - [ ] Document all possible attributes for WritableAvailableIP (the request-body of e.g. `POST /api/ipam/ip-ranges/{id}/available-ips/`) - [ ] Change the documentation of the requests data structure from `WritableAvailableIP` to `[WritableAvailableIP]` (but keep current API behavior, but make the current [passing a single object instead of a list with objects] undocumented and thereby "just works by coincidence, don't rely on it and use the correct one"). - [ ] Check all other usages of WritableAvailableIP for a similar error and also fix that.
adam added the status: duplicatetype: documentation labels 2025-12-29 20:30:29 +01:00
adam closed this issue 2025-12-29 20:30:29 +01:00
adam commented 2025-12-29 20:30:30 +01:00
Author
Owner
Copy Link

@kkthxbye-code commented on GitHub (Apr 28, 2023):

Isn't this just a duplicate of the issue you linked? https://github.com/netbox-community/netbox/issues/11578

Is there a reason this could not just be folded in under that issue?

@kkthxbye-code commented on GitHub (Apr 28, 2023): Isn't this just a duplicate of the issue you linked? https://github.com/netbox-community/netbox/issues/11578 Is there a reason this could not just be folded in under that issue?
adam commented 2025-12-29 20:30:30 +01:00
Author
Owner
Copy Link

@agowa commented on GitHub (Apr 28, 2023):

No, from what I've seen #11578 is only about /ipam/prefixes/{id}/available-ips/ and maybe I've misunderstood that ticket, but I thought that one was suggesting some behavioral change to that specific API endpoint. Whereas this one is a) about a different API endpoint and b) only a documentation change without changing any code functionality.

@agowa commented on GitHub (Apr 28, 2023): No, from what I've seen #11578 is only about `/ipam/prefixes/{id}/available-ips/` and maybe I've misunderstood that ticket, but I thought that one was suggesting some behavioral change to that specific API endpoint. Whereas this one is a) about a different API endpoint and b) only a documentation change without changing any code functionality.
adam commented 2025-12-29 20:30:30 +01:00
Author
Owner
Copy Link

@kkthxbye-code commented on GitHub (Apr 28, 2023):

Assuming passing a single object of WritableAvailableIP was never intended to begin with

I don't think that's a correct assumption. The API takes either a single object or a list of objects and returns either a single object or a list of objects depending on the input. It was made that way intentionally, at least as far as I recall.

I'd suggest to fix this issue by changing the datatype within the request to "List of WritableAvailableIP". That way we don't have to care about this strange and hard to handle different return types.

This will just break the other case where it returns a single object, I don't think this is fixable without changing the code.

Again, I think this is the exact same issue as discussed in #11578 - so not sure what new information is brought to light here, other than the fact that it also applies to ip-ranges, which could be added as a comment. Both cases would likely be fixed at the same time anyway, tracking it in two different issues seems confusing.

@kkthxbye-code commented on GitHub (Apr 28, 2023): > Assuming passing a single object of WritableAvailableIP was never intended to begin with I don't think that's a correct assumption. The API takes either a single object or a list of objects and returns either a single object or a list of objects depending on the input. It was made that way intentionally, at least as far as I recall. > I'd suggest to fix this issue by changing the datatype within the request to "List of WritableAvailableIP". That way we don't have to care about this strange and hard to handle different return types. This will just break the other case where it returns a single object, I don't think this is fixable without changing the code. Again, I think this is the exact same issue as discussed in #11578 - so not sure what new information is brought to light here, other than the fact that it also applies to ip-ranges, which could be added as a comment. Both cases would likely be fixed at the same time anyway, tracking it in two different issues seems confusing.
adam commented 2025-12-29 20:30:30 +01:00
Author
Owner
Copy Link

@jeremystretch commented on GitHub (Apr 28, 2023):

It was made that way intentionally, at least as far as I recall.

This is correct. Both an individual object as well as a list of objects is supported, in the interest of client convenience. There are comments to this effect in the code as well:

# Initialize the serializer with a list or a single object depending on what was requested
@jeremystretch commented on GitHub (Apr 28, 2023): > It was made that way intentionally, at least as far as I recall. This is correct. Both an individual object as well as a list of objects is supported, in the interest of client convenience. There are comments to this effect in the code as well: ``` # Initialize the serializer with a list or a single object depending on what was requested ```
adam commented 2025-12-29 20:30:31 +01:00
Author
Owner
Copy Link

@agowa commented on GitHub (Apr 28, 2023):

Ok, then it is intentional. But it is clearly an error to mix both versions.

The current documentation is request single object and response list.

@agowa commented on GitHub (Apr 28, 2023): Ok, then it is intentional. But it is clearly an error to mix both versions. The current documentation is request single object and response list.
adam commented 2025-12-29 20:30:31 +01:00
Author
Owner
Copy Link

@kkthxbye-code commented on GitHub (May 1, 2023):

Duplicate of https://github.com/netbox-community/netbox/issues/11578

@kkthxbye-code commented on GitHub (May 1, 2023): Duplicate of https://github.com/netbox-community/netbox/issues/11578
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
beta
breaking change
complexity: high
complexity: low
complexity: medium
needs milestone
netbox
pending closure
plugin candidate
pull-request
Mirrored from GitHub Pull Request
 severity: high
severity: low
severity: medium
status: accepted
status: backlog
status: blocked
status: duplicate
status: needs owner
status: needs triage
status: revisions needed
status: under review
topic: GraphQL
topic: Internationalization
topic: OpenAPI
topic: UI/UX
topic: cabling
topic: event rules
topic: htmx navigation
topic: industrialization
topic: migrations
topic: plugins
topic: scripts
topic: templating
topic: testing
type: bug
type: deprecation
type: documentation
type: feature
type: housekeeping
type: translation
No Label status: duplicate type: documentation
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
adam
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/netbox#7958
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?