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

Replace "go-swagger" by "openapi-generator" #157

Merged
merged 15 commits into from
Jan 18, 2024
Merged

Conversation

v0ctor
Copy link
Collaborator

@v0ctor v0ctor commented Aug 17, 2023

Since version 3.5, Netbox specifies its API with OpenAPI 3. Thus, it is no longer possible to keep this project up to date, as it is generated using go-swagger, a library that doesn't support OpenAPI 3 and has no intention to do so.

This PR replaces go-swagger by openapi-generator and rebuilds the library for the latest version of Netbox (3.5.8). This change is completely incompatible with previous versions of the library.

Note that this is a work in progress. I still didn't test the changes.

Any comments, suggestions or corrections are welcome! Furthermore, everyone is invited to participate in the OpenAPI 3 migration discussion.

@v0ctor v0ctor mentioned this pull request Aug 17, 2023
@jqueuniet
Copy link

I've started testing this PR out and I'm running into troubles. There is some kind of reflection issue with filters on any list endpoint.

Misbehaving code samples:

vrflist, resp, err := client.IpamApi.
	IpamVrfsList(ctx).
	Name([]string{name}).
	Execute()

platformlist, resp, err := client.
	DcimApi.
	DcimPlatformsList(ctx).
	Slug([]string{slug}).
	Execute()

clspaglist, 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()

Resulting queries:

GET /api/ipam/vrfs/?name=reflect.Value+value HTTP/1.1

GET /api/dcim/platforms/?slug=reflect.Value+value HTTP/1.1

GET /api/virtualization/clusters/?site_id=reflect.Value+value&status=reflect.Value+value&tag=reflect.Value+value&tag=reflect.Value+value&tag=reflect.Value+value HTTP/1.1

No issue with retrieving objects by ID.

@jqueuniet
Copy link

Finished testing with my main workflows and other than this listing problem I did not run into any other issue. Everything seems to work fine for object creation, update and deletion

@mytlogos
Copy link

For the listing problem see OpenAPITools/openapi-generator#14798
It should be already fixed on the openapi-master

@jqueuniet
Copy link

Thanks for the notice. Looking at the openapi-generator patch, it looks like the fix landed in 7.0.0, I can see the new .Interface() calls if I regenerate the client using this version.

@v0ctor v0ctor force-pushed the feature/openapi-generator branch from 7ce11a9 to c159071 Compare September 24, 2023 14:20
@v0ctor v0ctor force-pushed the feature/openapi-generator branch from c159071 to 94875e4 Compare September 24, 2023 14:21
@v0ctor
Copy link
Collaborator Author

v0ctor commented Sep 24, 2023

Thank you very much to all for the feedback!

I've updated the PR to openapi-generator 7.0.1 and Netbox 3.6.2, and I've built the library with these changes.

I've not been able to test it yet, but seeing that openapi-generator 7+ is giving good results, it might be a good idea to release an alpha version.

@jqueuniet
Copy link

Just wanted to chime in to say I updated my working branch and now everything works fine.

@tagur87
Copy link

tagur87 commented Jan 4, 2024

Only issue I've found so far is this: #156 (reply in thread) when enabling Enums. But other than that with the small workaround I found, it seems to work great!

@v0ctor v0ctor force-pushed the feature/openapi-generator branch 2 times, most recently from 9381825 to 37a98ae Compare January 9, 2024 15:54
@v0ctor v0ctor force-pushed the feature/openapi-generator branch from 37a98ae to 87af8b4 Compare January 9, 2024 16:12
@v0ctor
Copy link
Collaborator Author

v0ctor commented Jan 10, 2024

It seems that this is finally the most viable option. I am doing some tests before releasing the first alpha version. See the comment in the discussion.

@v0ctor v0ctor force-pushed the feature/openapi-generator branch from ec77beb to 6912321 Compare January 10, 2024 14:30
@v0ctor v0ctor force-pushed the feature/openapi-generator branch from 6912321 to 97dbb8f Compare January 18, 2024 16:29
@v0ctor v0ctor force-pushed the feature/openapi-generator branch from 8773ca9 to b34d5dc Compare January 18, 2024 16:46
@v0ctor
Copy link
Collaborator Author

v0ctor commented Jan 18, 2024

Finally, after five months, this PR is ready to be merged! 🥳

@v0ctor v0ctor merged commit 239313c into master Jan 18, 2024
2 checks passed
@v0ctor v0ctor changed the title [WIP] Replace "go-swagger" by "openapi-generator" Replace "go-swagger" by "openapi-generator" Jan 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants