Add Swagger UI capability when running in dev mode + OpenAPI YAML/JSON #39834
Conversation
* Add new ClientRepresentation Co-authored-by: Peter Zaoral <pzaoral@redhat.com> Co-authored-by: Václav Muzikář <vmuzikar@redhat.com> Signed-off-by: Martin Bartoš <mabartos@redhat.com> * Add APIs Signed-off-by: Martin Bartoš <mabartos@redhat.com> * Add ApiModelMapper SPI Signed-off-by: Martin Bartoš <mabartos@redhat.com> * Add MapStruct as default ApiModelMapper Signed-off-by: Martin Bartoš <mabartos@redhat.com> * Add default APIs implementations Signed-off-by: Martin Bartoš <mabartos@redhat.com> * Provide Service SPI and ClientService Signed-off-by: Martin Bartoš <mabartos@redhat.com> * Add default Keycloak services and Client service Signed-off-by: Martin Bartoš <mabartos@redhat.com> * Add ModelMapper to shared modules Signed-off-by: Martin Bartoš <mabartos@redhat.com> * Implement Client service, add ServiceException class Signed-off-by: Martin Bartoš <mabartos@redhat.com> * Use ClientService in Client REST API Signed-off-by: Martin Bartoš <mabartos@redhat.com> * Update rest/admin-api/src/main/java/org/keycloak/admin/api/client/ClientsApi.java Co-authored-by: Václav Muzikář <vaclav@muzikari.cz> Signed-off-by: Martin Bartoš <mabartos@redhat.com> * Fix ModelMapperSpi Signed-off-by: Martin Bartoš <mabartos@redhat.com> * Use /admin/api/v2 as a root path Signed-off-by: Martin Bartoš <mabartos@redhat.com> * Support latest API version by default Signed-off-by: Martin Bartoš <mabartos@redhat.com> * Rename path param to comply with API spec Signed-off-by: Martin Bartoš <mabartos@redhat.com> --------- Signed-off-by: Martin Bartoš <mabartos@redhat.com> Co-authored-by: Peter Zaoral <pzaoral@redhat.com> Co-authored-by: Václav Muzikář <vmuzikar@redhat.com> Co-authored-by: Václav Muzikář <vaclav@muzikari.cz>
Signed-off-by: Robin Meese <39960884+robson90@users.noreply.github.com>
|
@robson90 Thanks for the PR. I think it's a good start. Couple of thoughts for possible follow-ups:
|
|
@vmuzikar I fully agree with ur points. For the config toggle option, you want to handle it the same way as the /health & /metrics endpoints ? |
Yes, pretty much. Not sure about the naming though – maybe CC @keycloak/cloud-native |
9d1a0bd to
2bb4747
Compare
|
Just want to clarify - do you see these as completely coupled and only for dev mode, or should exposing the openapi endpoints be allowed in non-dev as well?
That seems like an alternative to making this only for dev. Under what circumstance would you expect someone to run the swagger ui in a non-dev environment? Just to double check - what if any would be differences between this "raw" openapi spec and the one that is included in our docs? For example are the paths in raw spec still relative? If so, won't that be a problem if the hostname-admin is used or we try to serve the raw spec off the management interface?
Depending up the answer from the above, wouldn't the old stuff still be valid correct?
openapi-enabled seems good. The notion of having experimental options has mostly be replaced by features, but it's not clear if we need to hide this behind a feature flag as well. If so we also need to be careful about how a feature is versioned. That is we don't want to end up at some point with something like feature openapi:v2 as that would be confusing with the openapi versioning. |
For me personally I would allow it in non-dev. It would simplify the process sending requests to the instance.
I don't know if the paths are still relevant. I will write a test for the hostname-admin feature flag. |
I was thinking about it more, and I believe we might need to split Swagger from the OpenAPI spec as such. The reason is exactly this ^^. IMHO Swagger should be tied to dev mode only, it's basically a web app, i.e. additional attack surface, and usually this would be used by devs to play around the API. OpenAPI spec on the other hand might be used for generating clients from a live server, i.e. it might be available in prod as well. The question then is whether it really makes sense to expose on the management interface – but probably yes, it's rather something you'd consume internally.
That's a good point. I think in the Swagger UI they can't be relative, can they?
My point was to keep v1 OpenAPI as is, not serve it in the new interface etc. |
|
|
||
| public class OpenApiSwaggerOptions { | ||
|
|
||
| public static final Option<Boolean> OPENAPI_SWAGGER_ENABLED = new OptionBuilder<>("openapi-enabled", Boolean.class) |
There was a problem hiding this comment.
@robson90 You will need to use a mapper to map this option to a Quarkus option, see e.g.:
Signed-off-by: Robin Meese <39960884+robson90@users.noreply.github.com>
Signed-off-by: Robin Meese <39960884+robson90@users.noreply.github.com>
aaf4f8f to
4b0043b
Compare
How to start:
mvn install -T 4 -DskipTests && mvn clean quarkus:dev -Dquarkus.args="start-dev" -pl quarkus/server-T 4is only for maven to use 4 Threads.Open a Browser and go to:
http://localhost:8080/swagger-uiHere you can see that the @operation capability is working:
Here is an example where I try to fetch all Clients in the master realm via the UI:
Please leave some feedback, for what could be improved or what you would like to see.
One open point for me is sorting the requests by version number in Path, so "V2" and "V3" is in its own group. Then Group inside those two groups per resource in alphabetic order. The request method per Resource for e.g. Clients, should always be in the same order.
For example: