netbox-community / go-netbox

The official Go API client for Netbox IPAM and DCIM service.
Other
197 stars 150 forks source link

Call to AvailablePrefixes crashes #131

Closed darkweaver87 closed 8 months ago

darkweaver87 commented 2 years ago

Hello,

When calling IpamPrefixesAvailablePrefixesCreate with a /16 cidr block, this client library fails with the following error:

json: cannot unmarshal object into Go value of type []*models.Prefix

As the API behavior looks correct to your colleagues (cf. https://github.com/netbox-community/netbox/issues/9471), I'll let you discuss internally on how to handle it.

Thanks.

Rémi

darkweaver87 commented 2 years ago

On my side I've done this ugly thing so far:

package netboxhack

// https://github.com/netbox-community/go-netbox/blob/master/netbox/client/ipam/ipam_client.go

import (
    "errors"
    "fmt"
    "io"

    oprt "github.com/go-openapi/runtime"
    "github.com/go-openapi/strfmt"
    "github.com/netbox-community/go-netbox/netbox/client/ipam"
    "github.com/netbox-community/go-netbox/netbox/models"
)

// IpamPrefixesAvailablePrefixesCreate create a prefix on an available slot.
func (c *Client) IpamPrefixesAvailablePrefixesCreate(params *ipam.IpamPrefixesAvailablePrefixesCreateParams, authInfo oprt.ClientAuthInfoWriter, opts ...ipam.ClientOption) (*ipam.IpamPrefixesAvailablePrefixesCreateCreated, error) {
    // TODO: Validate the params before sending
    if params == nil {
        params = ipam.NewIpamPrefixesAvailablePrefixesCreateParams()
    }
    op := &oprt.ClientOperation{
        ID:                 "ipam_prefixes_available-prefixes_create",
        Method:             "POST",
        PathPattern:        "/ipam/prefixes/{id}/available-prefixes/",
        ProducesMediaTypes: []string{"application/json"},
        ConsumesMediaTypes: []string{"application/json"},
        Schemes:            []string{"https"},
        Params:             params,
        Reader:             &IpamPrefixesAvailablePrefixesCreateReader{formats: c.formats},
        AuthInfo:           authInfo,
        Context:            params.Context,
        Client:             params.HTTPClient,
    }
    for _, opt := range opts {
        opt(op)
    }

    result, err := c.runtimeClient.Submit(op)
    if err != nil {
        return nil, err
    }
    success, ok := result.(*IpamPrefixesAvailablePrefixesCreateCreatedError)
    if ok {
        return &ipam.IpamPrefixesAvailablePrefixesCreateCreated{
            Payload: success.Payload,
        }, nil
    }
    // unexpected success response
    // safeguard: normally, absent a default response, unknown success responses return an error above: so this is a codegen issue
    msg := fmt.Sprintf("unexpected success response for ipam_prefixes_available-prefixes_create: API contract not enforced by server. Client expected to get an error, but got: %T", result)
    panic(msg)
}

// IpamPrefixesPartialUpdate updates a prefix.
func (c *Client) IpamPrefixesPartialUpdate(params *ipam.IpamPrefixesPartialUpdateParams, authInfo oprt.ClientAuthInfoWriter, opts ...ipam.ClientOption) (*ipam.IpamPrefixesPartialUpdateOK, error) {
    return c.netboxClient.Ipam.IpamPrefixesPartialUpdate(params, authInfo, opts...)
}

// IpamPrefixesDelete deletes a prefix.
func (c *Client) IpamPrefixesDelete(params *ipam.IpamPrefixesDeleteParams, authInfo oprt.ClientAuthInfoWriter, opts ...ipam.ClientOption) (*ipam.IpamPrefixesDeleteNoContent, error) {
    return c.netboxClient.Ipam.IpamPrefixesDelete(params, authInfo, opts...)
}

// IpamPrefixesAvailablePrefixesCreateReader is a Reader for the IpamPrefixesAvailablePrefixesCreate structure.
type IpamPrefixesAvailablePrefixesCreateReader struct {
    formats strfmt.Registry
}

// ReadResponse reads a server response into the received o.
func (o *IpamPrefixesAvailablePrefixesCreateReader) ReadResponse(response oprt.ClientResponse, consumer oprt.Consumer) (interface{}, error) {
    switch response.Code() {
    case 201:
        result := NewIpamPrefixesAvailablePrefixesCreateCreated()
        if err := result.readResponse(response, consumer, o.formats); err != nil {
            return nil, err
        }
        return result, nil
    default:
        return nil, oprt.NewAPIError("response status code does not match any response statuses defined for this endpoint in the swagger spec", response, response.Code())
    }
}

// NewIpamPrefixesAvailablePrefixesCreateCreated creates a IpamPrefixesAvailablePrefixesCreateCreatedError with default headers values.
func NewIpamPrefixesAvailablePrefixesCreateCreated() *IpamPrefixesAvailablePrefixesCreateCreatedError {
    return &IpamPrefixesAvailablePrefixesCreateCreatedError{}
}

/* IpamPrefixesAvailablePrefixesCreateCreatedError describes a response with status code 201, with default header values.

IpamPrefixesAvailablePrefixesCreateCreatedError ipam prefixes available prefixes create created.
*/
type IpamPrefixesAvailablePrefixesCreateCreatedError struct {
    Payload []*models.Prefix
}

func (o *IpamPrefixesAvailablePrefixesCreateCreatedError) Error() string {
    return fmt.Sprintf("[POST /ipam/prefixes/{id}/available-prefixes/][%d] ipamPrefixesAvailablePrefixesCreateCreated  %+v", 201, o.Payload)
}

func (o *IpamPrefixesAvailablePrefixesCreateCreatedError) readResponse(response oprt.ClientResponse, consumer oprt.Consumer, formats strfmt.Registry) error {
    var payload models.Prefix

    // response payload
    if err := consumer.Consume(response.Body(), &payload); err != nil && !errors.Is(err, io.EOF) {
        return err
    }

    o.Payload = append(o.Payload, &payload)

    return nil
}
v0ctor commented 1 year ago

The cause is that POST /ipam/prefixes/{id}/available-prefixes/ may return []*models.Prefix (201 status) or object (default status) and Swagger does not handle it properly.

We have to migrate this project from go-swagger to openapi-generator for Netbox 3.5. Maybe this will fix the issue.

I leave the issue open until further notice.

Atoms commented 1 year ago

It is the same with call to AvailableIPs, I have no clue how to workaround in go this properly, as go is strict and I cannot add fields to the payload which are not in spec as suggested in https://github.com/netbox-community/netbox/issues/11578#issuecomment-1425702907

As I don't plan to request more than one IP address I'm adjusting the module after go mod tidy, and this is my workaround currently :(

amhn commented 1 year ago

This is actually a bug in the schema definition in netbox. Once you provide a fixed schema swagger will happily generate the right code.

The problem is that the schema defines a single object as input and a list of objects as output. But if you POST a single object, you receive a single object. If you POST a list, you will receive a list.

See also https://github.com/netbox-community/netbox/issues/10311

jqueuniet commented 1 year ago

The response is one part of the problem, but there is also an issue with the request spec. Right now you can't really request more than one IP at once as it should be asked with [WritableAvailableIP, WritableAvailableIP, ...], but the request spec is only written for WritableAvailableIP, a request for a single IP.

      "post": {
        "operationId": "ipam_prefixes_available-ips_create",
        "description": "",
        "parameters": [
          {
            "name": "data",
            "in": "body",
            "required": true,
            "schema": {
              "$ref": "#/definitions/WritableAvailableIP"
            }
          }
        ],
        "responses": {
          "201": {
            "description": "",
            "schema": {
              "type": "array",
              "items": {
                "$ref": "#/definitions/IPAddress"
              }
            }
          }
        }

To sum it up, the spec is currently written for asking a single IP, but expect a response with multiple IP, and neither use-case (single or multiple) can currently work with the generated Go code.

At the very least, either the response or the request spec should be modified to follow one use-case end-to-end, until a solution is found to specify the full range of possibilities exposed by this endpoint. Probably the single one as the multiple can more easily be worked around by the client if the single works.

gboor commented 1 year ago

This issue is also present on the available-ips endpoint of ip-ranges. It's not just for prefixes.

v0ctor commented 8 months ago

A new alpha version has been released with a different software to generate the library, so hopefully this bug has been resolved.

Please feel free to test it and to provide feedback.

darkweaver87 commented 8 months ago

Thanks for the work. Unfortunately since then, I don't use netbox anymore but I'm pretty sure I was not the only one to face this issue, so some might reopen it if needed ;-)

jqueuniet commented 8 months ago

Haven't had the time to upgrade to the alpha yet, but earlier commits from the OpenAPI PR did solve this issue for me.