Create an OpenAPI/Swagger specification

wedi commented 4 years ago

I'd like to suggest to create/generate an OpenAPI/Swagger specification for the API. This would allow to use great and widely adopted tools like Postman to expore it. In addition, the – already great – documentation could become interactive and improve even further to make us nerds happy.

There is a generator for the Django REST Framework (https://django-rest-swagger.readthedocs.io/) that might help but I have not a clue about Django, unfortunately.

peterthomassen commented 4 years ago

That's actually pretty straightforward! https://www.django-rest-framework.org/api-guide/schemas/

I'll take a look when I find some time.

rugk commented 3 years ago

An Swagger/OpenAPI spec would IMHO complement the existing great doc, indeed.

Seeing there were some commits mentioning and claiming to solve this issue, I wonder what the current status is here? The issue is still open and I also could not find the YML/Swagger Editor preview anywhere?

peterthomassen commented 3 years ago

We use Django REST framework for our API, which comes with integrated support for generating an OpenAPI specification. However, at the time when I attempted this (the other commits), there was some issue with the automated generation that I was not able to solve at that point.

It's possible that the situation has improved. I will take a look at it next week.

nils-wisiol commented 3 years ago

related: #100

peterthomassen commented 3 years ago

@rugk I made another attempt with DRF's schema functionality, and manged to produce some results with Swagger and with ReDoc. (The schema files themselves are linked in the HTML sources, which are pretty small.)

However, there are several issues:

Some functionality is available under several URLs for convenience; for example, rrsets/@/ and rrsets/.../ is equivalent. The schema lists them separately (okay, why not), but also assigns them the same anchors so you can't properly "jump" to them by clicking on the corresponding link in the index: It will always jump to the first variant, because the name is ambiguous. That's pretty confusing for a reader.
Some endpoints have broken escaping, e.g. (look for {subname}\.\.\. which should be {subname}...)
~Some objects have hidden fields. One example is the content (solution) of a captcha. The schema generator seems to be looking at the models behind the endpoints, and not at the field visibility configuration of each endpoint (even though that's parseable). Such fields are misleading to the user, as they can be neither read nor written, and they should be hidden from readers.~ Captcha.contentwas included because settings.DEBUG was True.
Similarly, we sometimes include an extra field when returning an object; most notably the plaintext value of a fresh token. As this field does not correspond to a database column, it's not considered a true model field, and it's missing from the schema. That's not how it should be (again, it can be inferred from the endpoint configuration).
For RRset objects, the schema thinks that the records field is a JSON array (list) of objects, while it is really an array of strings. From a model perspective, that's correct, but we flatten the objects during serialization to improve UX. Inference from the endpoint configuration is not as easy, and it's not clear to me how to make the schema generator do the right thing.
A similar example are variables associated with confirmation links, most of which are encrypted in the verification link itself. The schema gives the impression you could pass these fields in a request body.
Lastly, time duration fields (token validity fields) produce broken output, apparently because they can't be represented in YAML/JSON (which is the schema format). I had to remove those lines from the output to make things render.

So, it seems to me that the schema generation functionality is not yet mature enough to cover these cases. Unfortunately, I don't have the time to maintain the schema manually (it's enough work to maintain the regular docs). We'd either have to a) wait for the generator to mature, b) proactively work on the generator (which I don't feel competent to do), or c) have someone else maintain a schema manually. I'm a bit at a loss at this point.

nils-wisiol commented 3 years ago

Voting to close this. Anyone who wants to work on this: feel free to reopen and/or submit a PR. Contributions are always appreciated

rugk commented 3 years ago

Could you at least report busg for the issues you've found to that upstream project? I totally understand that you cannot maintain both things, but if they don’t know about the bug, they also won’t be able to fix it.

Some things I’ve noticed:

The DELETE /api/v1/auth/tokens/{id}/ has a strange return value, only 204
The whole authentication does not seem to be documented. Swagger/OpenAPI offers “security schemas” for this (see their doc), which would be shown as a button in Swagger UI.
if you wanted to group the things in the Swagger UI, you would have to use tags. Don’t know whether your thing supports it.

In any case, thanks a lot for trying! :slightly_smiling_face:

wedi commented 3 years ago

It's a pitty because having SDKs in a selection of of languages would be very convenient but I totally agree that it's neither feasible nor sensible to manually maintain two different versions of the same documentation.

Unfortunately, Django is not a framework I am familiar with, so I have no arguments to vote against closing it, other than that it would serve as a reminder to reevaluate the topic from time to time.

peterthomassen commented 3 years ago

Could you at least report busg for the issues you've found to that upstream project? I totally understand that you cannot maintain both things, but if they don’t know about the bug, they also won’t be able to fix it.

Yep, I'm going to do that.

Some things I’ve noticed:

The DELETE /api/v1/auth/tokens/{id}/ has a strange return value, only 204

What's strange about that? (We implement the idempotence principle, i.e. 204 tells you that after the request returns, you can be sure that the object does not exist (regardless of whether it did before). If you want to check existence, use HEAD or GET.)

The whole authentication does not seem to be documented. Swagger/OpenAPI offers “security schemas” for this (see their doc), which would be shown as a button in Swagger UI.

if you wanted to group the things in the Swagger UI, you would have to use tags. Don’t know whether your thing supports it.

Good points! We'll look into that once the other issues (or at least most of them) are resolved.

Unfortunately, Django is not a framework I am familiar with, so I have no arguments to vote against closing it, other than that it would serve as a reminder to reevaluate the topic from time to time.

I think keeping it open is a good suggestion.

peterthomassen commented 3 years ago

I found some issues at the DRF repository, and opened a few new ones. Related:

https://github.com/encode/django-rest-framework/issues/8041 https://github.com/encode/django-rest-framework/issues/7255 https://github.com/encode/django-rest-framework/discussions/7929#discussioncomment-658681 https://github.com/encode/django-rest-framework/issues/8062 https://github.com/encode/django-rest-framework/issues/8063 https://github.com/encode/django-rest-framework/issues/8065

This does not cover all problematic aspects identified above, but the most important ones. Once there are solutions for them, we can revisit what to do with the remaining ones.

peterthomassen commented 2 months ago

It looks like Django REST Framework won't address the above blocking issues. However, there's another app that specializes in OpenAPI schema generation for DRF projects.

Putting it out here in case someone wants to take a stab at it: https://drf-spectacular.readthedocs.io/en/latest/

desec-io / desec-stack

Create an OpenAPI/Swagger specification #359