Migration to OpenAPI 3

[v0ctor]

I'd like to open a discussion about migrating go-netbox to a generation library compatible with OpenAPI 3, because the Netbox API is now specified with this version of the standard (since Netbox 3.5).

We currently use go-swagger to build the project from Netbox' OpenAPI specification. However, this library isn't and won't be compatible with OpenAPI 3 (see this issue).

This is therefore a necessary change for the project to remain up to date. However, it won't be backward compatible with previous versions.

For now, my proposal is to evaluate openapi-generator. I created a PR to test this option. There are other options, such as oapi-codegen, but I didn't evaluate it nor tested it.

As this is a major change, opening the discussion to the community ensures better decision-making.

Useful links

• PR: replace "go-swagger" by "openapi-generator"
• Netbox discussion to test the new API specification
• Proposals to use oapi-codegen: 1 and 2

[jqueuniet]

Major API breakages can't be helped given the nature of code generation, and from cursory reading Netbox did not try to keep the new spec compatible anyway (which is all for the better as it lifts some severe limitations with the OpenAPI 2 spec). I guess the new client should be published as a github.com/netbox-community/go-netbox/v3-oapi3 module or something like that to avoid breaking current code using this project.

I used a Nautobot client generated with openapi-generator for a year or so before we migrated to Netbox. After a few minor patches, it worked rather well, with less limitations than the current OpenAPI 2 generated clients for Netbox. The most obvious change is the ability to filter with multiple values for the same key, which wasn't allowed with the previous spec. Especially useful when filtering on tags. My usecase is mostly VM management and their network resources though, I haven't delved much in devices and other endpoints.

I do remember the Nautobot available-ips/available-prefixes OpenAPI 3 spec was broken and unusable without manual patches. It likely won't come as a surprise as its OpenAPI 2 spec was already broken. As this endpoint is somewhat tricky to specify it likely warrants some attention to avoid generating broken code. There is an open Netbox issue on this subject.

I'm currently not in a position to test the PR right now as we purposefully stayed on Netbox 3.4 for the time being, but I'll have a look at spinning up a test instance on 3.5 when I can.

Not a lot of experience with oapi-codegen, I tried it briefly, but didn't like the fact it stuffed everything in a single file which made any kind of code reading painful given the size of said file with a specification as large as Netbox (I think it was somewhere in the ballpark of 100k lines). I remember Goland complaining about the file size too. I guess it works for code embedding, not so well for a full fledged module. While looking for generators, I also found another one from contiamo, haven't really tried it out much either, I published a sample generation if you want to take a look.

[fbreckle]

I maintain fbreckle/go-netbox as a fork from this repo to support e-breuninger/terraform-provider-netbox.

With the openapi3 changes, I realized that this whole thing is growing over my head. I have only so much time for open source work and I rather spend that working on features in the provider than maintaining a go library (or in this case, completely replace it, basically). So I put my go-netbox project in "manual mode" (see here if interested).

So what I'm saying is: I look forward to using the "official" netbox-community/go-netbox eventually and get rid of my fork.

During my tests, in addition ot openapi-generator and oapi-codegen (its ~270k lines with all tags), I also found swagger-codegen, which, despite the name, seems to support openapi3. That one did not run out of the box, though.

@jqueuniet how would one then instanciate a client in https://github.com/jqueuniet/netbox-go-contiamo ? Am I missing something or is there code missing?

[jqueuniet]

I never really tried it, this "project" was just to check if the generator worked. Looking at it more closely, I can only find server code, nothing for client usage indeed. The server needs some boilerplate to work properly too, see here : https://github.com/contiamo/openapi-generator-go#the-router-generator

So, probably useless for this project, forget I said anything about it 😅

[jqueuniet]

Since I started testing the current PR, here is some Go client code to instantiate a client, in case anyone needs it.

func NetboxClient() (apiClient *netbox.APIClient, err error) {
	configuration := netbox.NewConfiguration()
	configuration.Host = viper.GetString("netbox.host")
	configuration.UserAgent = fmt.Sprintf("test-go-netbox-client/0.1")
	configuration.Scheme = viper.GetString("netbox.scheme")
	configuration.AddDefaultHeader(
		"Authorization",
		fmt.Sprintf("Token %s", viper.GetString("netbox.token")))
	apiClient = netbox.NewAPIClient(configuration)
	return
}

The configuration object exposes an HTTPClient for anything more advanced

If you would rather use per-call authentication rather than a static header:

func ContextWithNetboxAuth(ctx context.Context) context.Context {
	authValues := map[string]netbox.APIKey{
		"tokenAuth": {
			Prefix: "Token",
			Key:    viper.GetString("netbox.token"),
		},
	}
	return context.WithValue(ctx, netbox.ContextAPIKeys, authValues)
}

The API uses a chained method design with finishers (any *Execute method).

// Retrieves a single VRF by ID
vrf, resp, err := client.IpamApi.
	IpamVrfsRetrieve(ctx, id).
	Execute()

// Filtered list of virtualization clusters
paginatedClusterList, resp, err := client.VirtualizationApi.
	VirtualizationClustersList(ctx).
	SiteId([]*int32{&site.Id}).
	Status([]string{"active"}).
	Tag([]string{
		fmt.Sprintf("vnet_%s", vnet),
		fmt.Sprintf("env_%s", env),
		fmt.Sprintf("criticality_%s", criticality),
	}).
	Execute()

// Create an IP address
ipInput := netbox.WritableIPAddressRequest{
	Address: address,
}
ipInput.SetTenant(tenant.Id)
ipInput.SetStatus("active")
if vrf != nil {
	ipInput.SetVrf(vrf.Id)
}

ipAddress, resp, err := client.
	IpamApi.
	IpamIpAddressesCreate(ctx).
	WritableIPAddressRequest(ipInput).
	Execute()

// Create an IP address using available-ips endpoint
availableIPRequest := netbox.IPAddressRequest{}
availableIPRequest.SetStatus("active")
availableIPRequest.SetTenant(netbox.NestedTenantRequest{
	Name: tenant.Name,
	Slug: tenant.Slug,
})
if vrf != nil {
	availableIPRequest.SetVrf(netbox.NestedVRFRequest{
		Name: vrf.Name,
	})
}

ipAlloc, resp, err := client.IpamApi.
	IpamPrefixesAvailableIpsCreate(ctx, prefix.Id).
	IPAddressRequest([]netbox.IPAddressRequest{availableIPRequest}).
	Execute()

// Update a virtual machine status
updates := netbox.NewPatchedWritableVirtualMachineWithConfigContextRequest()
updates.SetStatus(status)

_, resp, err := client.
	VirtualizationApi.
	VirtualizationVirtualMachinesPartialUpdate(ctx, vmID).
	PatchedWritableVirtualMachineWithConfigContextRequest(*updates).
	Execute()

// Delete a virtual machine
resp, err := client.
	VirtualizationApi.
	VirtualizationVirtualMachinesDestroy(ctx, vmID).
	Execute()

[jqueuniet]

I started taking a look at the oapi-codegen proposal and re-implementing code using it. The basic workflow of building a client, and executing requests seems fine. Types are generated for enums, which wasn't handled by openapi-generator (looks like it's a 4-year old issue).

However, I'm much less convinced by the way response and request objects are generated. There's pointers absolutely everywhere, no kind of getter or setter to help deal with the tediousness when writing requests, and it gets downright dangerous reading responses. Any kind of traversal requires nil checks everywhere or risking a panic.

On a probably less important note, oapi-codegen stuffed every type definition in a single 2.67MB source file which is obnoxious to handle when you need to read the code to understand something, my IDE disables inspections by default opening it. The client code is even worse in this regard and clocks in at a whooping 7.1MB file size.

To demonstrate a bit, here is how I need to write an IP address request using the prefix available-ips endpoint:

status := netbox.IPAddressRequestStatusActive
dnsName := viper.GetString("hfjdkhfkjdhf")
ipaddrreq := netbox.IPAddressRequest{
	Status: &status,
	Tenant: &netbox.NestedTenantRequest{
		Name: tenant.Name,
		Slug: tenant.Slug,
	},
	DnsName: &dnsName,
}
if vrf != nil {
	ipaddrreq.Vrf = &netbox.NestedVRFRequest{Name: vrf.Name}
}

I can't refer to the API enum values without reallocating them to variables as they're constants and constants don't have an address. Hardcoded values have the same issue, but one could argue it's for the better as an incentive to avoid them. More annoyingly, you can't reference function return values, like demonstrated with the viper call.

availableIPRequest := netbox.IPAddressRequest{}
availableIPRequest.SetStatus("active")
availableIPRequest.SetDnsName(viper.GetString("hfjdkhfkjdhf"))
availableIPRequest.SetTenant(netbox.NestedTenantRequest{
	Name: tenant.Name,
	Slug: tenant.Slug,
})
if vrf != nil {
	availableIPRequest.SetVrf(netbox.NestedVRFRequest{
		Name: vrf.Name,
	})
}

openapi-generator also uses pointers for optional fields, but provides setters to hide the referencing. A bit more verbose, less tedious I think, but I guess that's arguable.

Now, for response parsing, let's say I want to access a nested field like an IP VRF ID. With oapi-codegen, it would work like this.

var vrfID int
// Unsafe
vrfID = *addr.Vrf.Id

// Safe
if addr != nil && addr.Vrf != nil && addr.Vrf.Id != nil {
	vrfID = *addr.Vrf.Id
}

Compared to openapi-generator using getters with full nil safety. nil address and VRF are handled, and the Id field is not a pointer.

var vrfID int32
vrfID = addr.GetVrf().Id

Code of the generated getter for reference.

func (o *IPAddress) GetVrf() NestedVRF {
	if o == nil || IsNil(o.Vrf.Get()) {
		var ret NestedVRF
		return ret
	}
	return *o.Vrf.Get()
}

[tagur87]

I got some time to work on this, and it seems that ENUMS are now supported well with a flag in the latest version of openapi-generator-cli. I was able to successfully generate all the code by adding these two flags to the cli command under generate-code.sh

    -p enumClassPrefix=true \
    --inline-schema-options RESOLVE_INLINE_ENUMS=true

This generates the enums in the go representation of enums (constants) with a prefix of the model, preventing naming collisions.

The only issue I am now facing is that there are a couple types in the latest spec that have a null enum value. Which the generator doesn't know how to deal with. For example:

// List of ConsolePortRequest_speed
const (
	CONSOLEPORTREQUESTSPEED__1200   ConsolePortRequestSpeed = 1200
	CONSOLEPORTREQUESTSPEED__2400   ConsolePortRequestSpeed = 2400
	CONSOLEPORTREQUESTSPEED__4800   ConsolePortRequestSpeed = 4800
	CONSOLEPORTREQUESTSPEED__9600   ConsolePortRequestSpeed = 9600
	CONSOLEPORTREQUESTSPEED__19200  ConsolePortRequestSpeed = 19200
	CONSOLEPORTREQUESTSPEED__38400  ConsolePortRequestSpeed = 38400
	CONSOLEPORTREQUESTSPEED__57600  ConsolePortRequestSpeed = 57600
	CONSOLEPORTREQUESTSPEED__115200 ConsolePortRequestSpeed = 115200
	CONSOLEPORTREQUESTSPEED__null   ConsolePortRequestSpeed = null
)

// All allowed values of ConsolePortRequestSpeed enum
var AllowedConsolePortRequestSpeedEnumValues = []ConsolePortRequestSpeed{
	1200,
	2400,
	4800,
	9600,
	19200,
	38400,
	57600,
	115200,
	null,
}

Null is not a valid go type, and I am not sure the best way to resolve this for smooth generation of the client.

Any ideas @v0ctor ?

[tagur87]

I found that if you just remove the - null value from the OpenAPI spec, then it fixes the issue in the code. Here what I got after generating: https://github.com/tagur87/go-netbox

[tagur87]

I personally like the layout and structure of openapi-generator over oapi-codegen. It splits the code into different files based on model, which makes it easier to debug and understand.

oapi-codegen also doesn't seem to handle the enums as well, in addition to the comment here: #156 (comment) about pointers.

Just my 2c but would love to see if we can get some movement on this as having updated support for the latest versions of netbox through a generated client would be the first step in modernizing some of our other tooling.

Thanks!

[v0ctor]

I agree with everything that has been said here.

I think "openapi-generator" is less elegant in the way the code is generated (i.e., the process seems to me more complex and, therefore, the repo is "dirtier"), but the resulting library is better than "oapi-codegen" for the following reasons:

• The API exposed by the library is very similar to the current one, so adapting the code is more convenient.
• The library is higher level when working with responses. I say this because in "oapi-codegen, objects returned by a response are inside a "JSONXXX" attribute, being "XXX" the HTTP status code returned. I think it is very important that the library does not expose the transport underneath because, precisely, its role is to abstract communications with the Netbox API. This problem was discussed in the "oapi-codegen" repo (here), but no conclusion has been reached.
• It uses method chaining, making it very easy to use and keeping the code clean and readable.
• Nullable types make working with models very convenient.
• The generated code is split into several files, so that it does not cause problems with IDEs.

Regarding "ogen", I have not been able to generate the code...

For these reasons, I think we are now ready to choose an option, and I think it should be "openapi-generator". I have updated the PR to the most recent versions of the generation library and the Netbox API specification. In addition, I have taken into account the errors that you have been reporting, and I have applied the corrections to the generation process.

I was going to release an alpha version right away, but I wanted to do some testing first, and I'm having some problems. I am using demo.netbox.dev for testing. When I try to list the devices, I get the following error:

no value given for required property id

I am looking into it. I drop the code in case someone wants to try it:

package main

import (
	"context"
	"log"
	"math"

	"github.com/netbox-community/go-netbox/v3"
)

func main() {
	ctx := context.Background()

	client := netbox.NewAPIClientFor("https://demo.netbox.dev", "<token>")

	res, _, err := client.DcimAPI.
		DcimDevicesList(ctx).
		Status([]string{"active"}).
		Limit(math.MaxInt32).
		Execute()

	if err != nil {
		log.Fatal(err)
	}

	log.Printf("%v", res)
}

Do not forget to install the right version of the library before running the code:

go get github.com/netbox-community/go-netbox/v3@87af8b4e02afed3aedbec92e24a5bd6c570a35ac

[zyxep]

Very nice :) can't wait to try to dig into it.

Do I read it correctly that the error handling in this codegen is going to be way better?
because the error handling in the version for 3.4.6 is really bad, my CLI I have made "crashes" if the API doesn't respond or fails and it's just "annoying" to make pretty.

[tagur87]

Thanks @v0ctor for the nice summary. I agree with your statements. It all looks good. Only suggestion I had was looking to put the generated code in a separate folder called api, netbox, generated, or something similar. this would keep the generated code separate from any other code that is used. Thoughts?

I was hitting the listing problem you were, but seems like you found a nice fix in this commit: 6912321 👍🏻

Nice Work1

[v0ctor]

@zyxep Error handling is very idiomatic now. At least I haven't had any problems with the tests I've done.

@tagur87 I completely agree that the repository looks much cleaner with the generated code in a subdirectory, but I finally decided to leave it in the root because there is no way not to expose this when consuming the library.

At the root, it is consumed as follows:

import "github.com/netbox-community/go-netbox/v3"

Whereas in a subdirectory it would look like this:

import "github.com/netbox-community/go-netbox/v3/api"

Although the repository looks worse, it is more idiomatic this way. I have followed the example of other generated projects, such as the Stripe library for Go.

Thank you all very much!

[v0ctor]

A new alpha version that supports the latest Netbox version has been released. 🥳

Errors are likely to be encountered when testing it, but they can be easily fixed by patching the OpenAPI spec. See the readme for more info on this.

I preferred to release an alpha version to evaluate this, because at @brutalsys we make use of very few Netbox API endpoints and, although the use cases I have tested work fine, I don't want this change to turn into a disaster.

Please feel free to test it and to provide feedback!

[v0ctor]

By the way, I'm sorry this has taken so long, but I haven't been able to spend as much time on it as I'd had liked.

[smutel]

Someone to help me on this : #166