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

Fix API Swagger specs, add OpenAPI v3 #4541

Merged
merged 41 commits into from
Jun 11, 2021

Conversation

valentijnscholten
Copy link
Member

@valentijnscholten valentijnscholten commented May 21, 2021

The API is very important, but currently we get feedback that:

  • The SwaggerUI is incomplete/incorrect with regards to field types, parameter types, etc.
  • Client generation either fails or generates incorrect code due to the schema not being correct;

This PR should help to fix that:

Swagger (OpenAPIv2) fixes:

  • Fix all issues reported by test_swagger_schema;
  • Fix notes() endpoints to have correct spec and consistent response type (instead of empty list when there are no notes);
  • Fix accept_risk endpoints to have correct spec;
  • Fix testdata for authorized_users in products, and for JIRA_Instance in test data and sample data;
  • Support password as writeOnly field;
  • Fix AppAnalysis and Finding_Template Tag serialization;
  • Fix Finding request_response serialization specs;
  • Fix response status codes to comply with RFC (no content = 204, created = 201)
  • Rename get_duplicate_status to get_duplicate_cluster
  • Remove left-over / incorrectly used TagList class
  • Incorrect schema for reverse ManyToMany relationships (Incorrect prefetch schema for reverse ManyToMany fields #4618)

Some stuff is hard to fix in Swagger/OpenAPIv2. For example the NumberInFilter (comma seperated list of integers) is not correctly specced in the Swagger (v2) schema. It's better to migrate to OpenAPI v3 instead of having to fix all this inside django-filters itself.

The OpenAPI v3 schema generator built in to DRF doesn't work it seems, so I chose the most popular one: drf-spectacular (https://github.com/tfranzel/drf-spectacular)

  • Add OpenAPI v3 spec using drf-spectacular (experimental);
  • Port all @swagger-auto-schema decorators to @extend-schema
  • Port schema validation code from test_swagger_schema.py to test_rest_framework.py;
  • Add check for warnings in schema to start of unit tests;
  • I implemented the prefetch parameter / response mechanism using a drf-spectacular post processor. This doesn't feel as nice as the composable schema implemented for swagger in the past. On the other hand my implementation is a lot simpler and less bound to which library we are using for the schema generation. Open to suggestions / improvements in follow up PRs.

Future:

@valentijnscholten valentijnscholten changed the title WIP Openapi3 WIP Fix API Swagger specs, add experimental Openapi3 spec May 21, 2021
@github-actions
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions
Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

@valentijnscholten valentijnscholten marked this pull request as draft May 30, 2021 19:43
@valentijnscholten
Copy link
Member Author

back to draft as I saw a problem

@alles-klar
Copy link
Contributor

Good work @valentijnscholten!

  • you added OpenAPI v3 (which is awesome!) with a different URL. Should we document and present it as our v3 API?
    I mean, OpenAPI and Swagger are very different. it will confuse users if we are considering the both our "v2" API

The API is still more or less the same, just upgrading from openapiv2 to v3 doesn't mean we have a v3 API. I would stick to v2 as API Version and just remove the openapiv2 documentation in the future.

@valentijnscholten
Copy link
Member Author

valentijnscholten commented May 31, 2021

It now looks like this:
image

After 1 or 2 months we could remove or hide the "old" swagger docs.

@alles-klar
Copy link
Contributor

It now looks like this:
image

After 1 or 2 months we could remove or hide the "old" swagger docs.

We should rename Swagger docs to OpenAPI2 Docs. See https://swagger.io/blog/api-strategy/difference-between-swagger-and-openapi/

@github-actions
Copy link
Contributor

github-actions bot commented Jun 6, 2021

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 6, 2021

Conflicts have been resolved. A maintainer will review the pull request shortly.

@valentijnscholten valentijnscholten marked this pull request as ready for review June 6, 2021 19:24
@valentijnscholten
Copy link
Member Author

We should rename Swagger docs to OpenAPI2 Docs. See https://swagger.io/blog/api-strategy/difference-between-swagger-and-openapi/

Done.

Also fixed the last open issue around selecting the correct serializer for the prefetch fields, so ready for review @alles-klar

@madchap
Copy link
Contributor

madchap commented Jun 9, 2021

It seems that the oa3 does not log the user in automatically, is it an oversight or just something that may no longer be possible?

@valentijnscholten
Copy link
Member Author

It seems that the oa3 does not log the user in automatically, is it an oversight or just something that may no longer be possible?

What do you mean by "log in automatically"? When I browse to the swagger docs of OA3 I can execute requests without having to login or provide tokens.

@StefanFl
Copy link
Contributor

@valentijnscholten Is it intentional, that there are no prefetches for one-to-one relationships anymore, for both OpenAPI V2 and V3? They are very useful, for example when you want to get the name of a role for a product member.

@valentijnscholten
Copy link
Member Author

valentijnscholten commented Jun 12, 2021

@valentijnscholten Is it intentional, that there are no prefetches for one-to-one relationships anymore, for both OpenAPI V2 and V3? They are very useful, for example when you want to get the name of a role for a product member.

No, it's not. I noticed they weren't there in the OpenAPI v2 so didn't think I needed to make them work in OpenAPI v3 and thought maybe it was intended as we also have the related_fields mechanism. But it looks like my PR somehow made them disappear in v2 already. Without this PR they are there. Will take a look.

kiblik pushed a commit to kiblik/django-DefectDojo that referenced this pull request Jun 13, 2021
* APIv2: Add OpenAPI v3 schema (draft)

* Swagger UI schema link default to json

* swagger: fix schema engagement notes read

* disable doc expansion

* swagger: fix schema engagement notes read

* fix risk acceptance swagger spec

* fix risk acceptance swagger spec

* adjust unit tests to new response code

* add notes and risk acceptance to spectacular

* Fix product tests

* support password field as writeonly

* fix AppAnalysis Tag serialization

* fix Finding_Template Tag serialization

* fix finding request response serialization specs

* fix risak acceptance spec spectacular

* fix RequestResponse Seriliazer

* fix risk acceptance swagger spec

* more openapi3 schema check work

* fix note types path

* fix enums and warnings

* convert swagger decorators to openapi3

* convert swagger decorators to openapi3

* backport check for unexpected fields

* add openapi v3 schema validation to unit tests

* add openapi v3 schema validation to unit tests

* finetune api docs

* fix related fields schema

* fix related fields schema

* implement prefetch mixin support

* improve schema types user groups

* cleanup

* cleanup

* cleanup

* improive menu labels

* use correct serializer for preftch response schema

* fix reverse manytomany
valentijnscholten added a commit that referenced this pull request Jun 26, 2021
* APIv2: Add missing methods + tests

* Cleanup

* Skip Notes

* Bump google-auth from 1.30.2 to 1.31.0 (#4645)

Bumps [google-auth](https://github.com/googleapis/google-auth-library-python) from 1.30.2 to 1.31.0.
- [Release notes](https://github.com/googleapis/google-auth-library-python/releases)
- [Changelog](https://github.com/googleapis/google-auth-library-python/blob/master/CHANGELOG.md)
- [Commits](googleapis/google-auth-library-python@v1.30.2...v1.31.0)

---
updated-dependencies:
- dependency-name: google-auth
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump sqlalchemy from 1.4.17 to 1.4.18 (#4644)

Bumps [sqlalchemy](https://github.com/sqlalchemy/sqlalchemy) from 1.4.17 to 1.4.18.
- [Release notes](https://github.com/sqlalchemy/sqlalchemy/releases)
- [Changelog](https://github.com/sqlalchemy/sqlalchemy/blob/master/CHANGES)
- [Commits](https://github.com/sqlalchemy/sqlalchemy/commits)

---
updated-dependencies:
- dependency-name: sqlalchemy
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Bump python-gitlab from 2.7.1 to 2.8.0 (#4647)

Bumps [python-gitlab](https://github.com/python-gitlab/python-gitlab) from 2.7.1 to 2.8.0.
- [Release notes](https://github.com/python-gitlab/python-gitlab/releases)
- [Changelog](https://github.com/python-gitlab/python-gitlab/blob/master/CHANGELOG.md)
- [Commits](python-gitlab/python-gitlab@v2.7.1...v2.8.0)

---
updated-dependencies:
- dependency-name: python-gitlab
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* chore(deps): update dependency postcss from 8.3.0 to v8.3.2 (docs/package.json) (#4639)

Co-authored-by: Renovate Bot <[email protected]>

* Authorization V2: Global roles (#4520)

* proof of concept

* use Dojo_User instead of User

* merge db migrations

* rename db migration after rebase

* initial commit

* before tests

* fixes after unit and integration tests

* fixes after code review

* documentation

* rename db migration after rebase

* global role for group after manual test

* amended unit test

* rename db migration after rebase

* adjust unit test case after rebase

* validation for global role in api

* model class for global role

* flake8

* api for global roles

* changes for role_model

* after code review

* after 2nd code review

* Add multiple members at once (#4625)

* Fix API Swagger specs, add OpenAPI v3 (#4541)

* APIv2: Add OpenAPI v3 schema (draft)

* Swagger UI schema link default to json

* swagger: fix schema engagement notes read

* disable doc expansion

* swagger: fix schema engagement notes read

* fix risk acceptance swagger spec

* fix risk acceptance swagger spec

* adjust unit tests to new response code

* add notes and risk acceptance to spectacular

* Fix product tests

* support password field as writeonly

* fix AppAnalysis Tag serialization

* fix Finding_Template Tag serialization

* fix finding request response serialization specs

* fix risak acceptance spec spectacular

* fix RequestResponse Seriliazer

* fix risk acceptance swagger spec

* more openapi3 schema check work

* fix note types path

* fix enums and warnings

* convert swagger decorators to openapi3

* convert swagger decorators to openapi3

* backport check for unexpected fields

* add openapi v3 schema validation to unit tests

* add openapi v3 schema validation to unit tests

* finetune api docs

* fix related fields schema

* fix related fields schema

* implement prefetch mixin support

* improve schema types user groups

* cleanup

* cleanup

* cleanup

* improive menu labels

* use correct serializer for preftch response schema

* fix reverse manytomany

* Migrate to OpenAPI3

* rerun scans

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Renovate Bot <[email protected]>
Co-authored-by: Stefan Fleckenstein <[email protected]>
Co-authored-by: valentijnscholten <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants