netbox-community / go-netbox

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

Update client to support Netbox > 2.5 #41

Closed mraerino closed 4 years ago

mraerino commented 5 years ago

@davcamer @mdlayher Are you still maintaining this module?

There are several month-old pull requests for this repo that would improve its usefulness a lot. Is there a chance somebody can step up as a maintainer?

mdlayher commented 5 years ago

I no longer work at DigitalOcean and have no plans to maintain this project myself.

jeremystretch commented 5 years ago

If someone wants to volunteer to take over maintainership, I'm happy to move this repo over to https://github.com/netbox-community.

davcamer commented 5 years ago

I thought I had shared this in another issue, but it was in response to an email, so that doesn't produce any discussion. Here's what a said to a similar query over email, on November 30th:

I am uncertain how to maintain go-netbox. The code that is there now was nearly all generated from OpenAPI definitions, as explained in the last part of the README: https://github.com/digitalocean/go-netbox#regenerating-the-client

Since it is generated code, fixes directly to the code will be removed the next time the client is generated. If there are mismatches between the generated code and Netbox's API, there is probably a problem in the OpenAPI json generated by Netbox. Those can probably be fixed by a change to Netbox, but they require knowledge of Netbox, Python, Django and the OpenAPI plugin used there.

A colleague I work with daily looked at making some updates to go-netbox through this route last week. He ended up giving up because there are significant changes to make across the whole of Netbox's OpenAPI annotations to describe this change in Netbox 2.4.0:

API writes now return a nested representation of related objects (rather than only a numeric ID)

I would like to deal with the PRs by understanding what needs to change in Netbox itself to have the same effect, and then re-generate the client. But, I have not had the need to do it until now, and the task has grown to the point that it doesn't seem worth it at the moment. Sadly. :(

If this were maintained by consistently regenerating using the OpenAPI definitions, this repo could probably be entirely automated using a Travis CI build triggered by changes to Netbox. But, we know from trying to do so ourselves, that the OpenAPI definitions are not sufficient to get an entirely compatible client.

Does it the make sense to patch here on top of the generated code? I know there are strategies to maintain a patch set on a branch and rebase against a "master" that has been generated.

jeremystretch commented 5 years ago

On the NetBox side, we're working on replacing Swagger with something better suited to introspection of the API scheme we employ (see #2665). I'm going to try to get this done for v2.6 but we're still in the exploratory phase at this point.

davcamer commented 5 years ago

If there were an updated, upstream OpenAPI definition I'd be happy to regenerate this code, and look at setting up an automated, or at least low friction, way of regenerating it again in the future.

I can't make the commitment to respond to and curate patches in the way that is needed to support a client that is not generated.

mraerino commented 5 years ago

I see. Thanks for all the info!

I can imagine several steps that can be taken independently of where the Netbox docs generation is heading:

Sadly I don't really have any clients right now that will pay me for heading this effort. If I ever get to write more network automation on top of Netbox, I'd eventually get involved in these steps. I hope I do.

If there is anyone out there who'd like to collaborate on the things I mentioned, I'd be happy to talk!

awlx commented 5 years ago

I am trying to generate a patched swagger.json but it's utterly broken atm :/.

It seems the only way to get a compatible client is to reverse engineer the API by hand since the Swagger output differs much from the reality.

For example in cables it says only a string is returned ... in reality it is a nested object.

termination_a | stringtitle: Termination breadOnly: true

       "termination_a" : {
            "url" : "http://netbox.dev/api/dcim/interfaces/709/",
            "connection_status" : {
               "label" : "Planned",
               "value" : false
            }, 
            "id" : 709,
            "device" : {
               "name" : "e-f1-007",
               "display_name" : "e-f1-007",
               "url" : "http://netbox.dev/api/dcim/devices/64/",
               "id" : 64
            }, 
            "name" : "eth3",
            "cable" : 40
         },
mraerino commented 5 years ago

@jeremystretch will you accept PRs in Netbox for improving the current swagger implementation? Or would you rather focus on the new approach you mentioned?

Right know I'm thinking it might be more productive to help Netbox output the correct swagger.json

mraerino commented 5 years ago

Something I just yet realized: The change proposed in https://github.com/digitalocean/netbox/issues/2665 will remove all response schemas from the swagger docs.

This means that there will be no way to generate a Go API client out of the new docs since the structs for response bodies will not be present. Basically the whole point of an API client is to not write that typing yourself.

@awlx Are you happy to explore what is needed to maintain an independent swagger file for this client? This would basically mean setting up a test suite to verify it against a Netbox instance.

Even now with response schemas in place it is really difficult for any tooling to generate a good swagger spec through static analysis since there are a lot of cases where the response schema depends on the data. Those cases are not even fully supported by the OpenAPI spec.

So later we might also need to provide some kind of helpers to unmarshal these polymorphic relations correctly.

awlx commented 5 years ago

@awlx Are you happy to explore what is needed to maintain an independent swagger file for this client? This would basically mean setting up a test suite to verify it against a Netbox instance.

Yep, I am in. Let's find a way to do this and keep it maintainable.

jeremystretch commented 5 years ago

@jeremystretch will you accept PRs in Netbox for improving the current swagger implementation? Or would you rather focus on the new approach you mentioned?

Right now I think we're still trying to figure out the best way forward. It might just take some concerted effort to get our implementation of drf-yasg to the right place.

Something I just yet realized: The change proposed in digitalocean/netbox#2665 will remove all response schemas from the swagger docs.

I still want to keep the generated OpenAPI doc. DRF can generate this natively, but its capabilities are still pretty limited compared to drf-yasg.

kobayashi commented 5 years ago

Hi @jeremystretch I hope to keep maintaining this by community include me. Is it possible to move this repo to netbox-community.

valentin2105 commented 5 years ago

Looking forward an updated version of this library. It would be great.

Thanks !

mraerino commented 5 years ago

Important related change in netbox 2.6 (currently in beta): https://github.com/digitalocean/netbox/issues/2968 We need to consider if this makes it easier to generate a Go client.

There is also the pattern of providing so-called plumbing (generated) and porcelain (hand-written, on top of plumbing) api clients to improve on generated openapi clients. Netlify is doing this: https://github.com/netlify/open-api#go-client

smutel commented 4 years ago

Hello,

What is the last update on this topic ?

I would like to implement a terraform provider for Netbox and a netbox lib in go will be appeciate. We use currently Netbox 2.5 ...

Thanks.

mraerino commented 4 years ago

we could start another attempt. but I guess we'd really want to have this repository in netbox-community together with a maintainer...

smutel commented 4 years ago

Agree

smutel commented 4 years ago

Waiting this, anyone can help me on this topic https://www.digitalocean.com/community/questions/example-to-create-a-tenant-with-https-github-com-digitalocean-go-netbox ?

Thanks.

awlx commented 4 years ago

Actually I asked some months ago on how to get this merged: https://github.com/awlx/go-netbox/tree/netbox_v2.6

And if go-netbox will be moved to netbox-community. No clear answer yet :/.

If there is nobody who wants to maintain it, I will (try to) do it.

mraerino commented 4 years ago

@jeremystretch can you move this to netbox community? I think @awlx and I could share maintainership.

jeremystretch commented 4 years ago

I no longer work at DigitalOcean and have no authority/ability to relocate a repository owned by their GitHub organization. We're happy to have this over at netbox-community, but someone within DigitalOcean needs to initiate the migration.

davcamer commented 4 years ago

I can move it over, I just need permissions to create repositories in the https://github.com/netbox-community org. Not sure who the best person to grant that is? I only need the permissions temporarily.

jeremystretch commented 4 years ago

@davcamer I'm trying to remember what we did when we moved the netbox repo over (which went very smoothly). Let me take a look.

jeremystretch commented 4 years ago

Looks like it needs to be initiated from the current repo under Settings > Transfer Ownership:

github_transfer_repo

Once that's done, John or I can accept the transfer request on the community side. GitHub will automatically redirect traffic from the old location to the new one.

davcamer commented 4 years ago

@jeremystretch Yes, that's how I initiated it, but I get this error:

Screen Shot 2019-11-20 at 11 00 08 AM

This is also documented here: https://help.github.com/en/github/administering-a-repository/transferring-a-repository#about-repository-transfers

I wasn't sure if it would apply in an Org to Org transfer, but apparently it does.

jeremystretch commented 4 years ago

@davcamer I've invited you as a member. I may still need to grant you permission once you accept the invitation.

davcamer commented 4 years ago

Invite accepted. Please grant me the permission.

jeremystretch commented 4 years ago

@davcamer try now please

mdlayher commented 4 years ago

Thanks for making this happen all!

davcamer commented 4 years ago

Here we are in netbox-community 😄

awlx commented 4 years ago

Awesome!

kobayashi commented 4 years ago

Hi folks, I am joining this repository as a collaborator. I work with netbox as a mainteiner also. As my first task, I have created slack channel #go-netbox on NetworkToCode

@davcamer, @chrigl, Do you have any restriction to keep up-to-date this repo?

@mraerino, @awlx, I know you guys have lots of works. If you are still interested in, I will discuss maintainer works in slack #go-netbox channel.

awlx commented 4 years ago

With https://github.com/netbox-community/go-netbox/pull/47 merged now we can move on to cleanup stuff a bit :).

smutel commented 4 years ago

Hi folks, I am joining this repository as a collaborator. I work with netbox as a mainteiner also. As my first task, I have created slack channel #go-netbox on NetworkToCode

@davcamer, @chrigl, Do you have any restriction to keep up-to-date this repo?

@mraerino, @awlx, I know you guys have lots of works. If you are still interested in, I will discuss maintainer works in slack #go-netbox channel.

It seems that slack.networktocode.com is down this morning :weary: I have just a question about errors ...

Currently I use the error returned by go-netbox in terraform and it looks like unknown error (status 400): {resp:0xc000122480} Any way to have an explanation of the message instead ?

awlx commented 4 years ago

Slack seems up to me.

The error 400 means bad request you should look into the netbox webserver to see what happened.

smutel commented 4 years ago

I don't speak about slack but slack.networktocode.com, the page is unreachable so I am not able to register.

Yeah ... but I would like to be able to display human message on the client side without going to the netbox server. I hope there is more than only the http code in the answer, no ?

kobayashi commented 4 years ago

Hello @smutel Can you try https://networktocode.herokuapp.com/ to join slack? I will close this issue later because no more reason to keep as open.

kobayashi commented 4 years ago

I would like to close this issue. We are working this repo to support newer version of netbox.