engine-api icon indicating copy to clipboard operation
engine-api copied to clipboard

Swagger specs

Open ahmetb opened this issue 9 years ago • 41 comments

It came up a lot in the Docker Remote API rework discussions: https://github.com/docker/docker/issues/15188 https://github.com/docker/docker/issues/5893 I also think swagger is a great way to describe APIs.

If there's a way maybe we can declare types/* in swagger specs and generate source files using swagger code generators, that would be very preferable as other language SDKs can take advantage of the swagger specs as well.

Just leaving this here.

ahmetb avatar Feb 04 '16 09:02 ahmetb

Also: https://github.com/docker-php/docker-php/blob/1.21/docker-swagger.json

I agree it would be good to either generate the go types from the spec, or generate the spec from the go code. Both can work, and we seem to have both sources already.

I think engine-api is missing a big part of the interface specification (the path, query params, headers, and response codes). Using swagger would force us to include all of those (which I think is good).

dnephin avatar Feb 04 '16 18:02 dnephin

Good idea to get it standardized format.

Does swagger have something to handle the stream hijacking that is done for attach, etc?

MHBauer avatar Feb 05 '16 21:02 MHBauer

Well, swagger supports plugins/extensions. If there is no way of specifying hijacking, docker can specify hijack mode and code generators would implement it generically.

More generally, people don't use hijack/attach very often in the client libraries. I tend to believe it's not much of a common task. They often do container/image managament as expected.

ahmetb avatar Feb 05 '16 21:02 ahmetb

Does swagger have something to handle the stream hijacking that is done for attach, etc?

I've been thinking about this. I think the way to support it would be to use produces: ['application/x-json-stream'] and the "response type" would be the format of each item in the stream. Clients would need to either support that mime-type or allow for plugins to support it.

dnephin avatar Feb 05 '16 22:02 dnephin

I agree it would be good to either generate the go types from the spec, or generate the spec from the go code. Both can work, and we seem to have both sources already.

we already have the types, let's generate the spec from them. That's what kubernetes does, bth. We might be able to reuse their scripts.

calavera avatar Feb 08 '16 23:02 calavera

fyi: http://goswagger.io/generate/spec/

casualjim avatar Feb 17 '16 17:02 casualjim

Could also use this spec for auto testing, where the example part of the path can be the acceptance test.

joelwurtz avatar Feb 17 '16 17:02 joelwurtz

Hey guys, just wanted to chime in. I'm Loc with VMware, working on project VIC. I'm not sure how far you guys have gotten through your swagger efforts, but we've been working on our own version for VIC. @casualjim and I have been working closely to build a docker API server based on swagger. Our swagger spec is based on the docker-php one, but it's been cleaned up a lot. That original spec didn't really work for us. We've also been fixing and adding features to go-swagger to support the docker swagger spec.

Our original plan was to hand over the spec to you guys, but it appears you guys are already looking at it. If you guys are interested in this spec, it might accelerate your work. There still quite a bit of work on both the spec and go-swagger to make it fully compliant with the docker api. We should talk. We're literally right around the corner from you guys.

sflxn avatar Mar 09 '16 22:03 sflxn

I promise to show up if you have a meeting in SF.

MHBauer avatar Mar 09 '16 22:03 MHBauer

Our swagger spec is based on the docker-php one, but it's been cleaned up a lot. That original spec didn't really work for us. We've also been fixing and adding features to go-swagger to support the docker swagger spec.

I'm really interested for knowing why it wasn't working for you ? :)

joelwurtz avatar Mar 09 '16 22:03 joelwurtz

We're using go-swagger for our generator. Ideally, it should be the same as using swagger, but what I found was there were a lot of mistakes in the original docker-php spec. We got a cleaned up version of the spec from @casualjim, then through trial and error, we cleaned up the spec again to make it work. Our desire was to make sure the server generated with the spec would work flawlessly with both CURL and the docker CLI. That original spec (at least the version I got originally) won't work. It's missing a lot of mime-types, incorrect definitions, missing definitions, etc. Also, @casualjim has had to add features to go-swagger to make the server compliant with docker. We've spent quite a bit of time debugging HTTP headers. In case you're wondering, go-swagger generates golang code. That's why we're using it instead of regular swagger.

sflxn avatar Mar 09 '16 22:03 sflxn

Ok, i'm the one who write the swagger spec for docker-php, and i'm really interested to have a shared spec that works for multiple projects (this will, IMO, decrease maintenance for everyone).

Don't know if that's ok with you to have a shared spec (and don't know if it's the correct topic to speak about that) feel free to open an issue on https://github.com/docker-php/docker-php if you are interested.

joelwurtz avatar Mar 09 '16 22:03 joelwurtz

I agree on a shared spec. Our original intent was to get this working and hand it over to Docker and see if they would be interested in maintaining it since they control their APIs. BTW, I want to emphasize that work must also be done on the generator to support functionality required by the docker API server. There's no way to know what this work entails until you run the swagger generated server, run curl request and docker CLI commands and look at the headers.

sflxn avatar Mar 09 '16 23:03 sflxn

btw, there's another reason we want to give Docker this spec. We're very interested in this separation of the engine-api server work. It appears both we and Docker are doing similar things but starting in different points. We're trying to build a compliant api server and you guys already have this. We started out with swagger and you guys want to move in this direction. There are some gotchas using swagger. You might have to rethink how the swagger server hooks into the rest of docker, specifically your backend interfaces. We could probably work together on this.

sflxn avatar Mar 09 '16 23:03 sflxn

Our original plan was to hand over the spec to you guys, but it appears you guys are already looking at it. If you guys are interested in this spec, it might accelerate your work. There still quite a bit of work on both the spec and go-swagger to make it fully compliant with the docker api. We should talk. We're literally right around the corner from you guys.

@joelwurtz, @sflxn please, do not hesitate to open pull request with a spec, completed or not. All contributions are very welcome. I'm pretty sure, nobody else is working on it. That's a much much better starting point than people waiting around :smile_cat:

calavera avatar Mar 09 '16 23:03 calavera

@joelwurtz some of the issues I fixed through sending a PR. The cases are missing parameters, consumes, produces and so on.

One of the funnier ones was: there were some response status codes which are 4O4 instead of 404 so the letter O instead of the number 0.

casualjim avatar Mar 10 '16 18:03 casualjim

+1

fehguy avatar Mar 18 '16 21:03 fehguy

@joelwurtz Would you have any qualms if I do a PR today in this repo? Once that happens, I can do a PR on your repo. I sense you want docker-php to be the source of sharing, but I believe keeping the spec in this repo is good place for people to come and collaborate on it. Eventually, this spec and any other that are shared should probably be submitted to swaggerhub.

I want to comment about how this spec should be used. For the time being, it's a good source to generate API clients. Generating a server will be much more involved. The docker API server have some custom behaviors that are different from the default behavior of a generated swagger server. @casualjim and I have worked on both the spec and go-swagger to make it come as close as we can, but there are still differences. If there is a desire to move to a swagger based server implementation, I recommend a staged approach. First, refactor data structures passed into the backend methods, adding swagger annotation in the process. The refactoring of these arguments need to be disentagled from docker specific internal structures. An even better approach would be to generate swagger models from the spec and refactor the backends to use those data structures. Down the road, it will become easier to migrate over to a swagger based server.

sflxn avatar Mar 22 '16 16:03 sflxn

@joelwurtz Would you have any qualms if I do a PR today in this repo? Once that happens, I can do a PR on your repo. I sense you want docker-php to be the source of sharing, but I believe keeping the spec in this repo is good place for people to come and collaborate on it. Eventually, this spec and any other that are shared should probably be submitted to swaggerhub.

Go ahead and no, i don't want docker-php to be the source of sharing on the contrary i would prefer to have a dependency on this repo to reduce my maintenance cost.

I want to comment about how this spec should be used. For the time being, it's a good source to generate API clients. Generating a server will be much more involved.

I think there is 2 approach :

  1. Start from the swagger spec and generate everything (client and server skeleton)
  2. Start from server code and generate the swagger spec, which can be used to generate client api

IMO, the second approach is the best regarding the current status on docker (no bc break), and in this view proposing a swagger spec to this repo is more for testing, so this will ensure that the generation of the spec is correct about the api vision.

joelwurtz avatar Mar 23 '16 16:03 joelwurtz

Joel, I agree that annotation is a great way to approach when there is an existing code base. There is some value using a starting spec to generate models and using those swagger generated data structures as a guide for updating the existing code.

For example, the current server routes can easily be swapped out, in favor of a swagger based server. The swagger generated server is really nothing more than a REST server front end. The data structure generated (models) from the swagger spec can then be passed into the backends or they can be massaged into the existing data structures passed into the backends.

sflxn avatar Mar 23 '16 17:03 sflxn

@casualjim can chime in here re: go-swagger

fehguy avatar Mar 23 '16 17:03 fehguy

the least risky path is to use the documentation comments. Docker has a very interesting use of http and stretches the swagger definition already seriously. Interesting use is around the event streams and blob uploads.

Later on perhaps it can use the generated server but the main thing is that it gets a swagger spec that is generated from the actual code. That is where the most value is at this stage imo. It's also a fairly risk-free approach.

casualjim avatar Mar 23 '16 19:03 casualjim

I agree with @casualjim, annotation is less risky right now. There are still more work that needs to go into go-swagger to be able to handle all the unique cases that the docker server currently handles. We can work on beefing up go-swagger on our end to get it there, but it would be tough to switch over the rest handling to swagger on the server side right now.

sflxn avatar Mar 23 '16 21:03 sflxn

I have been experimenting with adding Swagger annotation to the source code:

https://github.com/docker/docker/compare/master...bfirsh:generate-swagger-specification https://github.com/docker/engine-api/compare/master...bfirsh:add-swagger-annotations

This will allow us to automatically generate the sort of thing in #168 and, helpfully, also forces to add loads of documentation to the API.

I'm running into a few cases where Swagger can't generate the correct docs from the objects we have, so there's going to be a bit of refactoring needed to make that work. Refactoring for the better though - it forces us to have clean objects for representing API requests and responses, etc.

There are also a few things that go-swagger can't really represent properly (e.g. JSON responses for success, plain text for error). Perhaps we should actually fix these problems in the API rather than try and shoehorn Swagger into the problems. ;)

bfirsh avatar May 03 '16 01:05 bfirsh

We have had complaints about the plain text error responses being unexpected.

justincormack avatar May 03 '16 08:05 justincormack

Any progress on this? From what I understand this would allow us to generate libraries for other languages using codegen.

CooCooCaCha avatar Jun 20 '16 19:06 CooCooCaCha

Just updated our swagger schema with last API Changes : https://github.com/docker-php/docker-php/blob/1.24/docker-swagger.json There is also support for all swarm API endpoints.

And IMO, this is really needed, i can't say the number of times where i have see errors on the actual documentation (fields not displayed, wrong field names, updates not reported, a lot of missing informations). I know i can do pull requests to fix all of this, but i'm afraid this will continue to have errors as the api evolves due to the current way of working.

Having an auto generated documentation will really help to also generate a synchronized and better documentation than the current one (and allow code generation, but for me this is not the primary goal)

Given this, @bfirsh is there any way to help ? I'm not a go expert, but i can take time to update my knowledge and advance this, unless this is not a wanted feature ?

joelwurtz avatar Aug 08 '16 18:08 joelwurtz

This is definitely wanted and I'm currently cranking through the whole API adding go-swagger definitions. See my branches above for work-in-progress as of a few days ago. Unfortunately I'm running into lots of go-swagger issues, but the author is pretty responsive and helpful.

Once I've added a first pass of definitions, I'd really appreciate some help checking it over to make sure it is accurate. Once we've got that in, then we can generate some API documentation. 😄

BTW – thanks for your work on the manually created version. I've been using it as a reference for the auto-generated one to make sure it is correct!

bfirsh avatar Aug 08 '16 18:08 bfirsh

Current work in progress, automatically generated from the branches above:

https://gist.githubusercontent.com/bfirsh/936b39514515e833df1457bd445d624f/raw/b3ab78318eb0abcbcc833b1bd53840be64c89822/swagger.json

You can plug it into http://petstore.swagger.io/ and click "explore" to get a GUI. Lots of stuff missing/broken still, but hopefully gives you an idea of where it's heading.

bfirsh avatar Aug 10 '16 18:08 bfirsh

Anybody have any good ideas on how to test this? I was thinking some kind of sanity test to generate it and check some expected stuff is in it, but it would be quite nice to check it's actually correct somehow.

I can imagine a fully fledged integration suite which actually tries to generate a client out of the swagger.json, then tests that against a running version of the code it was generated it from. Sounds a bit ambitious/slow for a first pass though...

bfirsh avatar Aug 10 '16 18:08 bfirsh