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

Auto-generated API documentation #7874

Closed
stgraber opened this issue Sep 16, 2020 · 10 comments
Closed

Auto-generated API documentation #7874

stgraber opened this issue Sep 16, 2020 · 10 comments
Assignees
Labels
Documentation Documentation needs updating Easy Good for new contributors Feature New feature, not a bug
Milestone

Comments

@stgraber
Copy link
Contributor

Over the years, we've certainly hit the limits of doc/rest-api.md as a suitable way to describe our API and its valid input/output.
Advanced users know to go look at shared/api to find everything that's possible for every endpoint but this is far from ideal.

I'd like us to switch over to auto-generated API documentation.
Looking around, I've found https://github.com/swaggo/swag which seems to fit the bill and in theory should just work with our existing functions and structs.

For this issue, we should pick a somewhat standard REST API endpoint, let's say /1.0/instances/NAME/exec and annotate it so that everything that's supported by that endpoint is covered. We can then review the resulting API documentation and if we find it suitable, can keep going with the rest of the endpoints.

If this works out, we'd ultimately generate doc/rest-api.md from the swagger spec and use it to render a nice interactive doc for our API.

@stgraber stgraber added Feature New feature, not a bug Easy Good for new contributors Documentation Documentation needs updating labels Sep 16, 2020
@stgraber stgraber added this to the soon milestone Sep 16, 2020
@WilliamGrossKC
Copy link

Could I be assigned this issue?

@stgraber
Copy link
Contributor Author

yep, done!

@stgraber
Copy link
Contributor Author

@WilliamGrossKC Do you still intend to work on this or should we un-assign so someone else can have a go at it?

@WilliamGrossKC
Copy link

WilliamGrossKC commented Jan 19, 2021 via email

@stgraber
Copy link
Contributor Author

Sorry for your loss, I'll unassign this from you.

@MatzeS
Copy link

MatzeS commented Feb 4, 2021

Two points:

First, based on some initial experimentation I feel that using swaggo requires a lot of refactoring.
For example, swaggo only generates OpenAPI information for exported struct fields. See here.
Therefore, for example

type syncResponse struct {
    success  bool
    ...
}

has to be refactored to:

type syncResponse struct {
    Success  bool
    ...
}

This is not difficult or challenging, but I guess this touches a lot of code. Is this suitable and ok with you?

Second, my intention is to use the swagger/OpenAPI description to code-generate a corresponding API client.
Therefore, I basically want the API to be 100% perfectly reflected in the OpenAPI specification.

I have manually written an OpenAPI spec and observed that the LXD API is incompatible with OpenAPI on some endpoints resp. in some design aspects.

For example:
POST /1.0/storage-pools/<name>/volumes accepts various different JSON objects with slightly deviating schema.
The schema changes based on the content of the type-property (migration, copy, ...).
AFAIK this is not compatible with JSON schema or at least very, very complicated and therefore cannot be covered by API client code-generators.

This problem shows at various points, for example also when receiving responses of variable schema.
Therefore, a receiver cannot fully parse the response without examining the values. Again, generally not compatible with code generation.

Is there any chance to resolve this?
Changing the endpoints is obviously not possible due to backwards compatibility.
Creating a 2.0 API is a lot of work and I assume not worth it. (But please share your insight, if you think this should be done.)

A reasonable solution might be to add additional (redundant) endpoints with suitably defined request/response structures (and maybe deprecating old ones).

Let's discuss.

@stgraber
Copy link
Contributor Author

On our side, the main goal right now is to replace doc/rest-api.md with something that's generated from our code so we never forget to document an endpoint. We also would like a much better API explorer on our website so using swagger/OpenAPI makes sense there.

I've been looking at some options today, most swaggo and go-swagger but also some of the OpenAPI 3.0 options.
Basically swaggo has the nicest syntax but falls apart pretty quickly when using nested structs and external packages, the OpenAPI 3.0 options are nowhere near production ready at this point, so that currently leaves me with go-swagger which I'll be poking more at tomorrow.

I'm currently just trying to have the YAML and then web rendering of GET / and GET /1.0 to look perfect to me, once those behave, I'll probably try my hands at the much more complex /1.0/instances (supports GET, POST and PUT with a variety of input for those) see if go-swagger behaves with that one too. Assuming all looks good at that point, I'll apply the same pattern to the rest of our API endpoints.

As for your question, I'm currently not intending to make any API changes or additions with this, purely document what's there.
We also don't intend to consume the generated metadata for purposes other than documentation generation, so it's not really intended to magically get auto-generated clients. I'm not even sure how you'd get such a client to automatically know what to do with background operations as that's not really something we can specify in the API.

@stgraber stgraber self-assigned this Feb 17, 2021
@stgraber stgraber modified the milestones: soon, lxd-4.12 Feb 17, 2021
@stgraber
Copy link
Contributor Author

Current preview: https://dl.stgraber.org/swag-lxd/

@tomponline
Copy link
Member

@stgraber looks good!

@stgraber
Copy link
Contributor Author

stgraber commented Mar 2, 2021

  • api10
  • api10Cmd
  • clusterCmd
  • clusterNodeCmd
  • clusterNodesCmd
  • projectCmd
  • projectsCmd
  • certificateCmd
  • certificatesCmd
  • eventsCmd
  • imageAliasCmd
  • imageAliasesCmd
  • imageCmd
  • imageExportCmd
  • imageRefreshCmd
  • imagesCmd
  • imageSecretCmd
  • instanceLogCmd
  • instanceLogsCmd
  • instanceBackupCmd
  • instanceBackupExportCmd
  • instanceBackupsCmd
  • instanceCmd
  • instanceConsoleCmd
  • instanceExecCmd
  • instanceFileCmd
  • instanceMetadataCmd
  • instanceMetadataTemplatesCmd
  • instancesCmd
  • instanceSnapshotCmd
  • instanceSnapshotsCmd
  • instanceStateCmd
  • networkACLCmd
  • networkACLsCmd
  • networkCmd
  • networkLeasesCmd
  • networksCmd
  • networkStateCmd
  • operationCmd
  • operationsCmd
  • operationWait
  • operationWebsocket
  • profileCmd
  • profilesCmd
  • api10ResourcesCmd
  • storagePoolResourcesCmd
  • storagePoolCmd
  • storagePoolsCmd
  • storagePoolVolumeTypeCustomBackupCmd
  • storagePoolVolumeTypeCustomBackupExportCmd
  • storagePoolVolumeTypeCustomBackupsCmd
  • storagePoolVolumesCmd
  • storagePoolVolumesTypeCmd
  • storagePoolVolumeTypeCmd
  • storagePoolVolumeSnapshotsTypeCmd
  • storagePoolVolumeSnapshotTypeCmd
  • storagePoolVolumeTypeStateCmd

@stgraber stgraber modified the milestones: lxd-4.12, lxd-4.13 Mar 4, 2021
@stgraber stgraber modified the milestones: lxd-4.13, lxd-4.14 Apr 9, 2021
@stgraber stgraber modified the milestones: lxd-4.14, lxd-4.15 May 7, 2021
stgraber added a commit that referenced this issue Jun 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Documentation needs updating Easy Good for new contributors Feature New feature, not a bug
Projects
None yet
Development

No branches or pull requests

4 participants