netbox-community / netbox

The premier source of truth powering network automation. Open source under Apache 2. Try NetBox Cloud free: https://netboxlabs.com/free-netbox-cloud/
http://netboxlabs.com/oss/netbox/
Apache License 2.0
15.91k stars 2.55k forks source link

Swagger API has incorrect documentation for /api/circuits/provider-accounts/ and api/circuits/provider-networks #16765

Closed aaroneg closed 2 months ago

aaroneg commented 3 months ago

Deployment Type

Self-hosted

NetBox Version

v4.0.6

Python Version

3.10

Steps to Reproduce

View the swagger API docs located at https://$hostname/api/schema/swagger-ui/#/circuits/circuits_provider_accounts_create and you will see that the schema for ProviderAccountRequest does not mention any need to specify which provider the account is mapped to, but this field is in fact required.

Expected Behavior

The documentation should show all fields, especially required fields.

Observed Behavior

The required field 'provider' is missing from the documentation.

aaroneg commented 3 months ago

Looks like it also affects /api/circuits/provider-networks/

jeffgdotorg commented 3 months ago

Thank you for reporting this problem in NetBox. I was able to reproduce it in a fresh installation of v4.0.6. I'm moving your issue along to needs owner status.

We're generally aware that there are gaps in our OpenAPI coverage, but currently lack the team capacity to prioritize an effort to address these kinds of problems. If you or another developer with the requisite skills and capacity would like to work it through to a PR, please say so and a maintainer will assign the issue to you.

aaroneg commented 3 months ago

I'm not much of a python programmer but if you can whip up a short explainer and it's simple enough i'm willing to give it a shot.

On Mon, Jul 1, 2024 at 3:42 PM Jeff Gehlbach @.***> wrote:

Thank you for reporting this problem in NetBox. I was able to reproduce it in a fresh installation of v4.0.6. I'm moving your issue along to needs owner status.

We're generally aware that there are gaps in our OpenAPI coverage, but currently lack the team capacity to prioritize an effort to address these kinds of problems. If you or another developer with the requisite skills and capacity would like to work it through to a PR, please say so and a maintainer will assign the issue to you.

— Reply to this email directly, view it on GitHub https://github.com/netbox-community/netbox/issues/16765#issuecomment-2200983510, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKU7P7NO2UO7WNK5OMI54DZKG5LBAVCNFSM6AAAAABKDOPYWWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMBQHE4DGNJRGA . You are receiving this because you authored the thread.Message ID: @.***>

jeffgdotorg commented 3 months ago

I'm not much of a python programmer but if you can whip up a short explainer and it's simple enough i'm willing to give it a shot.

I want your expectations to be realistic – I can't guarantee approval of a PR, but if you're up for a learning exercise, go for it. Addressing these gaps really needs to be done as part of a holistic effort, using a consistent approach that has yet to be fleshed out.

arthanson commented 2 months ago

This looks potentially like a display issue with spectacular, the field is required in the yaml:

    ProviderAccount:
      type: object
      description: Adds support for custom fields and tags.
      properties:
        id:
          type: integer
          readOnly: true
        url:
          type: string
          format: uri
          readOnly: true
        display:
          type: string
          readOnly: true
        provider:
          $ref: '#/components/schemas/Provider'
        name:
          type: string
          default: ''
          maxLength: 100
        account:
          type: string
          title: Account ID
          maxLength: 100
        description:
          type: string
          maxLength: 200
        comments:
          type: string
        tags:
          type: array
          items:
            $ref: '#/components/schemas/NestedTag'
        custom_fields:
          type: object
          additionalProperties: {}
        created:
          type: string
          format: date-time
          readOnly: true
          nullable: true
        last_updated:
          type: string
          format: date-time
          readOnly: true
          nullable: true
      required:
      - account
      - created
      - display
      - id
      - last_updated
      - provider
      - url
arthanson commented 2 months ago

What's happening is the nested=True serializer is getting instantiated first in CircuitSerializer, it looks like Spectacular is caching the fields from this call and using that.

arthanson commented 2 months ago

closing as duplicate of #16670