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

feat: Validate swagger definitions #5694

Merged
merged 70 commits into from
Jan 13, 2023
Merged

feat: Validate swagger definitions #5694

merged 70 commits into from
Jan 13, 2023

Conversation

mtojek
Copy link
Member

@mtojek mtojek commented Jan 12, 2023

Fixes: #3522

This PR concludes the autogenerating docs initiative with a basic Swagger validator and custom rules, that will let us keep the better quality of docs. The validator uses go/ast to parse comments and Go structure fields. It covers OSS and Enterprise chi handlers.

Checks:

  1. Ensure consistency between @Summary and @ID.
  2. Verify @Success and/or @Failure.
  3. Verify presence of required annotations.
  4. Make sure that Go comment (above the function) is before swagger's @...
  5. Ensure that path parameters are defined.
  6. Ensure that @Security tag is present.
  7. Verify @Accept annotation.
  8. Verify @Produce annotation.
  9. Ensure unique swagger routes - no copy/paste mistakes.
  10. Make sure that uuid.UUID and time.Time fields are formatted.

To reviewer:

  • There are many files in this PR, mainly due to minor fixes spotted while implementing the validator. I suggest starting the peer review with coderdtest/swaggerparser.go.

@mtojek mtojek self-assigned this Jan 12, 2023
@mtojek mtojek changed the title Validate feat: Validate swagger definitions Jan 12, 2023
@mtojek mtojek mentioned this pull request Jan 12, 2023
29 tasks
@mtojek mtojek requested review from a team, sreya, mafredri and coadler and removed request for a team and sreya January 12, 2023 17:11
@mtojek mtojek marked this pull request as ready for review January 12, 2023 17:12
Copy link
Member

@johnstcn johnstcn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't gone through the generated docs with a fine-toothed comb, but I just had a couple of questions/observations about the Swagger parser.

coderd/coderdtest/swaggerparser.go Outdated Show resolved Hide resolved
Copy link
Member

@johnstcn johnstcn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pretty neat 👍

@mtojek mtojek merged commit deebfcb into coder:main Jan 13, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jan 13, 2023
@Emyrk
Copy link
Member

Emyrk commented Jan 13, 2023

Love how easily the go parser was to get comment groups.

I would suggest we should comment what each assertion does. Some are obvious, but it would be helpful to have some english.

Do you intend to parse the go code at all? Like enforce all routes have a swagger defintion? I missed the Chi walker at first. Neat!

Do you intend to dig into the functions and seeing if the return type is called via httapi.Write? Curious if you think we should do any more analysis on the types, via the SDK or the route functions.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Autogenerated docs for REST API
3 participants