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

Update swagger API specification #1782

Merged

Conversation

nitram509
Copy link
Contributor

@nitram509 nitram509 commented May 29, 2023

Summary

#292

This PR drops the outdated former swagger.yaml/json and introduced automatic API document generation from Go code.
The generated code is also used to generate documentation/markdown for the community page,
as well as enable the Woodpecker server to serve a Swagger Web UI for manual tinkering.

I did opt-in for gin-swagger, a middleware for the Gin framework, to ease implementation and have a sophisticated output.
This middleware only produces Swagger v2 specs. AFAIK the newer OpenApi 3x tooling is not yet that mature,
so I guess that's fine for now.

Motivation

I was not able to find up-to-date API documentation, for my Woodpecker integration project.
There was some outdated swagger.json in the source repo, but that's very much outdated.

Having proper API documentation tremendously eases the integration and use of Woodpecker for all admins and developers.
Also, I was surprised and found a new un-documented feature (keyword: ccmenu) which is great and underpins, how important documentation is.

Implemenation notes

  • former swagger.json files removed
  • former // swagger godocs removed
  • introduced new dependency gin-swagger, which uses godoc annotations on top of Gin Handler functions.
  • reworked Makefile to automatically generate Go code for the server
  • introduce new dependency go-swagger, to generate Markdown for documentation purposes
  • add a Swagger Web UI, incl. capabilities for manual API exploration
  • consider relative root paths in the implementation
  • write documentation for all exposed API endpoints
  • incl. API docs in the community website (auto-generated)
  • provide developer documentation, for the Woodpecker authors
  • no other existing logic/code was intentionally changed

Areas of uncertainty

This is my first contribution to Woodpecker and I tried my best to align with your conventions.
That said, I found myself uncertain about these things and would be glad about getting feedback.

  • what does POST /user/token do? -- the API summary I left blank, I couldn't grasp the details of the function
  • what's the difference between the endpoints: /pipelines vs. /queue/info -- the API summary does not yet well enough explain the difference
  • /stream endpoints are not documented since it's already marked "todo/deprecated"
  • model GetQueueInfo -> InfoT can't be used, swag tool complains -- I left a TODO and will create a bug-issue at the swag project
  • How do the versioned_docs work? Are my markdown files placed correctly?

Visual examples

The API list is quite long, he're two screenshots.
Screenshot 2023-05-29 at 01 49 53
Screenshot 2023-05-29 at 01 50 13

Drop former swagger code/references, because it was not working
Makefile Show resolved Hide resolved
@qwerty287 qwerty287 added server enhancement improve existing features labels May 29, 2023
cmd/server/swagger.go Outdated Show resolved Hide resolved
cmd/server/woodpecker_docs_gen.go Outdated Show resolved Hide resolved
cmd/server/woodpecker_docs_gen.go Outdated Show resolved Hide resolved
cmd/server/woodpecker_docs_gen.go Outdated Show resolved Hide resolved
cmd/server/woodpecker_docs_gen.go Outdated Show resolved Hide resolved
docs/docs/92-development/08-swagger.md Outdated Show resolved Hide resolved
Makefile Show resolved Hide resolved
server/router/router.go Outdated Show resolved Hide resolved
@woodpecker-bot
Copy link
Collaborator

woodpecker-bot commented May 29, 2023

Deployment of preview was successful: https://woodpecker-ci-woodpecker-pr-1782.surge.sh

@qwerty287
Copy link
Contributor

Here you go: https://woodpecker-ci-woodpecker-pr-1782.surge.sh/docs/next/usage/rest-api

@qwerty287

This comment was marked as outdated.

server/api/badge.go Outdated Show resolved Hide resolved
server/api/user.go Outdated Show resolved Hide resolved
server/api/hook.go Outdated Show resolved Hide resolved
server/api/hook.go Outdated Show resolved Hide resolved
@6543
Copy link
Member

6543 commented Jun 1, 2023

if you run make generate you get uncommited diffs ... we should check and enforce to get it commited into git history via ci

-> either track it or remove it from git completly ... I'm for the first one

@nitram509
Copy link
Contributor Author

if you run make generate you get uncommited diffs ... we should check and enforce to get it commited into git history via ci

-> either track it or remove it from git completly ... I'm for the first one

Hi @6543 I'm sorry but I'm not fully getting what you mean.
Following your advice, may I ask what should I technically do?

@6543
Copy link
Member

6543 commented Jun 1, 2023

I checked out this branch and did run make generate witch changed git tracked files ... this check should be done via ci

@nitram509
Copy link
Contributor Author

I checked out this branch and did run make generate witch changed git tracked files ... this check should be done via ci

Now I got it, thanks for pointing out.
Even though the code generation is stable = actually you wouldn't have noticed,
BUT I had to adapt/remove server/api/file go, because of another PR happening (avoiding merge conflict).

That said, I anticipate, excluding the generated docs.go file from the git will prevent accidental inconsistencies in code, like you faced. Simply because I forgot to re-generate+commit the new swagger after deleting the file from the PR.

Thanks for the feedback, will remove it with my next commit.

@6543
Copy link
Member

6543 commented Jun 3, 2023

@nitram509 I'll push some patches that will explain it better :)

@anbraten
Copy link
Member

anbraten commented Jun 3, 2023

This is how I've added the redoc page: 4fba29c

@6543
Copy link
Member

6543 commented Jun 3, 2023

well beside all the experiments and some code comments i did added

@nitram509 what I ment by "add a check": 52fdc38

@6543
Copy link
Member

6543 commented Jun 3, 2023

so I expect the pipeline to fail now and then I'll commit a updated version and it should pass

@6543
Copy link
Member

6543 commented Jun 3, 2023

@nitram509 thanks for the work you put into it :)

@6543 6543 merged commit 1417763 into woodpecker-ci:master Jun 3, 2023
6543 added a commit that referenced this pull request Jun 4, 2023
@nitram509 nitram509 deleted the feature/swagger-ui-and-auto-generation branch June 5, 2023 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement improve existing features server
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants