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

OpenAPI schema is incorrect for the ipam_prefixes_available-prefixes_create endpoint #11729

Closed
Gaardsholt opened this issue Feb 10, 2023 · 7 comments
Labels
pending closure Requires immediate attention to avoid being closed for inactivity severity: low Does not significantly disrupt application functionality, or a workaround is available status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation topic: OpenAPI type: bug A confirmed report of unexpected behavior in the application

Comments

@Gaardsholt
Copy link

Gaardsholt commented Feb 10, 2023

NetBox version

v3.4.5-dev

Python version

3.9

Steps to Reproduce

  1. Open the API docs at /api/docs/
  2. Find the /ipam/prefixes/{id}/available-prefixes/endpoint
  3. See that it says that you can only specify the prefix_length in the body

Expected Behavior

That I can use all the fields set on a prefix.

I have successfully tested with the following fields:

{
  "prefix_length": 0,
  "site": 0,
  "role": 0,
  "tenant": 0,
  "vrf": 0,
  "vlan": 0,
  "status": "container",
  "is_pool": true,
  "mark_utilized": true,
  "description": "string",
  "comments": "string"
}

Observed Behavior

That the API docs shows all the available fields that I can set.

I have been playing around with it locally, so I will be happy to do a pull request for it.

@Gaardsholt Gaardsholt added the type: bug A confirmed report of unexpected behavior in the application label Feb 10, 2023
@arthanson
Copy link
Collaborator

This looks like it is still an issue for OpenAPI 3, but blocked till #9608 gets in.

@arthanson arthanson added the status: blocked Another issue or external requirement is preventing implementation label Mar 22, 2023
@arthanson arthanson self-assigned this Mar 22, 2023
@arthanson
Copy link
Collaborator

@Gaardsholt I don't think this is actually a bug. prefix_length is the only valid field to pass for available_prefixes. By default DRF (which is used to create the API endpoints) by default doesn't error on extra fields passed in (see: https://stackoverflow.com/questions/22178266/django-rest-framework-raise-error-when-extra-fields-are-present-on-post). You could open a feature request to enable this (but should be done on all APIs, not just this one) if this is a problem. Closing for now.

@Gaardsholt
Copy link
Author

@arthanson I guess it shouldn't error on the extra fields?
Because when I set those extra fields, they are applied, so it has to be a bug in the API schema, right?

@kkthxbye-code
Copy link
Contributor

@arthanson

prefix_length is the only valid field to pass for available_prefixes.

As Gaardsholt said this is incorrect. available_prefixes accepts and persists all prefix fields as far as I can tell.

Simple payload as this shows it:

https://demo.netbox.dev/api/ipam/prefixes/98/available-prefixes/

{
  "prefix_length": 30,
  "description": "test"
}

image

@jeremystretch
Copy link
Member

jeremystretch commented Apr 14, 2023

I think this was my mistake. I told @arthanson that PrefixLengthSerializer only accepts a prefix_length field (which is technically true, hence the bug), but it also passes other data into the Prefix instance. Ultimately the serializer itself really should be rebuilt more cleanly.

@jeremystretch jeremystretch reopened this Apr 14, 2023
@jeremystretch jeremystretch added status: accepted This issue has been accepted for implementation and removed status: blocked Another issue or external requirement is preventing implementation labels Apr 14, 2023
@arthanson arthanson added topic: OpenAPI status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation labels Apr 24, 2023
@arthanson arthanson removed their assignment Apr 24, 2023
@jeremystretch jeremystretch removed the status: accepted This issue has been accepted for implementation label May 3, 2023
@jeremystretch jeremystretch added the severity: low Does not significantly disrupt application functionality, or a workaround is available label Jun 23, 2023
@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. NetBox is governed by a small group of core maintainers which means not all opened issues may receive direct feedback. Do not attempt to circumvent this process by "bumping" the issue; doing so will result in its immediate closure and you may be barred from participating in any future discussions. Please see our contributing guide.

@github-actions github-actions bot added the pending closure Requires immediate attention to avoid being closed for inactivity label Sep 22, 2023
@MinDBreaK
Copy link

It seems that since the 3.6 release, the documentation has been updated for this API, but now it is missing the prefix_length from the documentation (Which now break codegen for the API). Should a new ticket be opened or we can use this one even if the bug is not exactly the same (but same place, same type of bug) ?

@jeremystretch jeremystretch closed this as not planned Won't fix, can't repro, duplicate, stale Nov 29, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pending closure Requires immediate attention to avoid being closed for inactivity severity: low Does not significantly disrupt application functionality, or a workaround is available status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation topic: OpenAPI type: bug A confirmed report of unexpected behavior in the application
Projects
None yet
Development

No branches or pull requests

5 participants