[Admin API v2] Skeleton prototype #39322

mabartos · 2025-04-29T08:24:35Z

Used services to handle business logic - f.e. ClientService
Services can be overridden via SPI
Used MapStruct for mapping Model->Representation entities

Try it out

Get clients

curl http://localhost:8080/admin/api/realms/master/clients

or

curl http://localhost:8080/admin/api/v2/realms/master/clients

Get client

curl http://localhost:8080/admin/api/realms/master/clients/account

or

curl http://localhost:8080/admin/api/v2/realms/master/clients/account

If you want to retrieve the full representation, add the query param runtime=true

curl http://localhost:8080/admin/api/v2/realms/master/clients/account?runtime=true

Create client

curl -X POST http://localhost:8080/admin/api/realms/master/clients \
  -H "Content-Type: application/json" \
  -d '{"clientId":"myClient2"}'

or

curl -X POST http://localhost:8080/admin/api/v2/realms/master/clients \
  -H "Content-Type: application/json" \
  -d '{"clientId":"myClient2"}'

For the second execution, it should return 409. For PUT, it will update the client (not implemented yet).

Questions

Do we want to create some middle service layer to take care of the business logic and authz? And on the REST level, we could only do HTTP/REST stuff and mapping Model->Representation. Given that we could provide the SPI for each part of the API, I don't think it's worth to include additional abstraction layer. YES

shawkins · 2025-04-29T12:49:57Z

Looks good @mabartos, here's some initial thoughts.

Possibility to create a whole custom API

By API you are referring to the entire Admin API Rest layer correct? Basically what is contributed by the AdminApiSpi and related classes?

Are you splitting it out of concern the default impl brings in dependencies that devs shouldn't otherwise be using?

If not, I would advocate for using a single module for the default impl and the spis, rather than splitting it, as it seems likely that anything a dev would want to do here would simply extend the existing logic (given the rigidity of the rest interface), rather than completely rewrite it.

Are there some thoughts about what kind of customizations users would want to do at this layer? I added some musing about field mix-in below - it would be good to expand on how this abstraction point could be used.

Used MapStruct for mapping Model->Representation entities

That seems good, especially if it is already productized. Since it is based on code generation there isn't a worry about the apis getting out-of-sync.

Another thought is that Representations as records are concise, but may be too limiting down the road - some logic has ended up in the current Representation classes, like the DeviceRepresentation.setBrowser method. They are also not extensible making them a hard contract for the API Spi. That may be taking away some of the extensiblity of this layer. For example if you wanted a straight-forward way to mixin some fields on a representation in your custom api. I know this is not trivial - this would also require the base class to support something similar to what is done in fabric8 with additionalProperties on all Kubernetes resources, so that you could preserve that on deserialization, and of course that would need to be accounted for in the openapi metadata via something like preserve unknown.

An alternative (which I know some would be reluctant to consider, but if we're already bringing in MapStruct...) is the lombok Data annotation - that gives you the concise record like definition, but is an actual class that you can further refine.

Do we want to create some middle service layer to take care of the business logic and authz?

More than likely yes - that is the layer which would be most easily extensible by something like the graphql community project.

And on the REST level, we could only do HTTP/REST stuff and mapping Model->Representation. Given that we could provide the SPI for each part of the API, I don't think it's worth to include additional abstraction layer.

Alternatively if we cannot come up with compelling scenarios where the entire rest spis are extended / replaced, then we should focus more on what the business layer definition as the point of extensibility.

keycloak-github-bot

Unreported flaky test detected, please review

keycloak-github-bot · 2025-04-29T12:51:25Z

Unreported flaky test detected

If the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR.

org.keycloak.testsuite.webauthn.account.WebAuthnSigningInTest#categoriesTest

Keycloak CI - WebAuthn IT (firefox)

java.lang.AssertionError: 

Expected: is <2>
     but: was <0>
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
...

Report flaky test

org.keycloak.testsuite.broker.KcOidcBrokerTest#loginWithExistingUserWithBruteForceEnabled

Keycloak CI - Java Distribution IT (windows-latest - temurin - 24)

org.openqa.selenium.NoSuchElementException: 
Unable to locate element
For documentation on this error, please visit: https://www.selenium.dev/documentation/webdriver/troubleshooting/errors#no-such-element-exception
Build info: version: '4.28.1', revision: '73f5ad48a2'
System info: os.name: 'Windows Server 2022', os.arch: 'amd64', os.version: '10.0', java.version: '24.0.1'
...

Report flaky test

mabartos · 2025-04-29T13:53:55Z

@shawkins Thanks for the review! I'll try to answer incrementally.

By API you are referring to the entire Admin API Rest layer correct? Basically what is contributed by the AdminApiSpi and related classes?

Yes.

Are you splitting it out of concern the default impl brings in dependencies that devs shouldn't otherwise be using?
---- If not, I would advocate for using a single module for the default impl and the spis, rather than splitting it, as it seems likely that anything a dev would want to do here would simply extend the existing logic (given the rigidity of the rest interface), rather than completely rewrite it.
---- Are there some thoughts about what kind of customizations users would want to do at this layer? I added some musing about field mix-in below - it would be good to expand on how this abstraction point could be used.

Yes, it will probably make more sense to merge the default impl with the API contracts. I wanted to have them as separate units to have a clean code and "only-contract" module to have it easily used without these additional deps (but there will not be so many more perhaps, so it's ok).

Are there some thoughts about what kind of customizations users would want to do at this layer?

The intention behind this is that devs might provide some additional endpoints outside of the Admin API v2 contract. There are some possibilities, but this might be clean to just use SPI for ClientsAPI for instance.

.....

mabartos · 2025-04-29T15:53:57Z

That seems good, especially if it is already productized. Since it is based on code generation there isn't a worry about the apis getting out-of-sync.

Yes, from what I checked, it's productized on our side. I think it's gonna be better approach than using the RepresentationToModel <--> ModelToRepresentation classes.

Another thought is that Representations as records are concise, but may be too limiting down the road - some logic has ended up in the current Representation classes, like the DeviceRepresentation.setBrowser method. They are also not extensible making them a hard contract for the API Spi. That may be taking away some of the extensiblity of this layer. For example if you wanted a straight-forward way to mixin some fields on a representation in your custom API.

Yes, in terms of extendability, a normal class seems a better direction to go.

An alternative (which I know some would be reluctant to consider, but if we're already bringing in MapStruct...) is the lombok Data annotation - that gives you the concise record like definition, but is an actual class that you can further refine.

I'd say it's some sort of standard to use Lombok in Java apps as the maturity of it is high and there are multiple resources available. I would not probably resist so much. Yes, as you mentioned - if we're bringing in MapStruct ;)
EDIT: as I checked, the productization on our side is not very frequent.

More than likely yes - that is the layer which would be most easily extensible by something like the graphql community project.

Make sense.

Alternatively if we cannot come up with compelling scenarios where the entire rest spis are extended / replaced, then we should focus more on what the business layer definition as the point of extensibility.

Yes, we can discuss.

I'll try to update the skeleton based on the feedback and how I think it might be good to have it :)

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>

rest/admin-api/src/main/java/org/keycloak/admin/api/client/ClientsApi.java

shawkins · 2025-04-29T19:45:51Z

I'll try to update the skeleton based on the feedback and how I think it might be good to have it :)

Just wanted to muse more on versioning.

With this pr you can contribute different implemenations of v2, which we'll need to come up with more use-cases for.

At an even coarser level there's Admin API v1 vs v2, with additional revisions to follow in the future. We may want to go back and similarly encapsulate the v1 implementation so that users via profile features can run 1 or more of our admin apis of their choosing.

Versioning at the resource level is something we've only touched on a little in the meetings. We talked about it in terms of the "default problem" and a little in comparison to how kubernetes handles things. So it's worth touching on here - from a specific version of the admin api, do we want to support multiple versions of a given resource? That is could you submit and even retrieve Client v1 (for example the ClientRepresentation shown so far in this pr) and Client v2 (a ClientRepresentation with different fields and / or defaults)? If the answer is uncategorically yes, that will greatly complicate things.

More pragmatically we may want to be able to account for this like how we allow our CRDs to change over time - we will allow changes that are schema compatible, but we may still want to capture in the representation what version is being used (to determine what defaults are applicable for example) and if the object model can potentially be used in a way that spans multiple versions, we may need to add something like the additionalProperties handling mentioned in the previous comment.

mabartos · 2025-04-30T10:15:24Z

@shawkins

With this pr you can contribute different implemenations of v2, which we'll need to come up with more use-cases for.

It will probably not be necessary to introduce it as we already have the AdminRealmResourceProvider as users might potentially add their resources/endpoints that way. I'll remove the abstraction.

Versioning at the resource level is something we've only touched on a little in the meetings. We talked about it in terms of the "default problem" and a little in comparison to how kubernetes handles things. So it's worth touching on here - from a specific version of the admin api, do we want to support multiple versions of a given resource? That is could you submit and even retrieve Client v1 (for example the ClientRepresentation shown so far in this pr) and Client v2 (a ClientRepresentation with different fields and / or defaults)? If the answer is uncategorically yes, that will greatly complicate things.
More pragmatically we may want to be able to account for this like how we allow our CRDs to change over time - we will allow changes that are schema compatible, but we may still want to capture in the representation what version is being used (to determine what defaults are applicable for example) and if the object model can potentially be used in a way that spans multiple versions, we may need to add something like the additionalProperties handling mentioned in the previous comment.

Yes, versioning of these resources should be a good way on how to build a good API - if we are able to store some changelog/set of defaults for versions. Not sure how difficult would it be to manage field changes over different versions. It is possible out of scope of this PR as even the code introduced here can be changed with no difficulties.

And agree that we need some mechanism to store some metadata (or the additionalProperties as you mentioned. In that case, we'd need to probably update even the Model interfaces to contain some metadata - as some Model interfaces already contains Map of attributes, but might be separated as they can represent some bigger business value + not all models have the map.

Signed-off-by: Martin Bartoš <mabartos@redhat.com>

vmuzikar

Nice work @mabartos! It's a good start.

rest/admin-api/src/main/java/org/keycloak/admin/api/client/ClientsApi.java

vmuzikar · 2025-04-30T16:02:29Z

core/src/main/java/org/keycloak/representations/admin/v2/ClientRepresentation.java

Just a side note. Based on a comment from Stian we might in the end keep the structure solid, unsplitted. But that's for a follow up.

vmuzikar · 2025-04-30T16:04:45Z

pom.xml

+            <dependency>
+                <groupId>org.mapstruct</groupId>
+                <artifactId>mapstruct</artifactId>
+                <version>${org.mapstruct.version}</version>
+            </dependency>


Not sure about productization but I assume it's build time only processor.

From what I've checked, it is fully productized.

vmuzikar · 2025-04-30T16:06:32Z

Ad versioning. It's out of scope of this PR, IMHO. We'll tackle it soon as a follow-up.

rest/admin-api/src/main/java/org/keycloak/admin/api/AdminRootV2.java

…entsApi.java Co-authored-by: Václav Muzikář <vaclav@muzikari.cz> Signed-off-by: Martin Bartoš <mabartos@redhat.com>

Signed-off-by: Martin Bartoš <mabartos@redhat.com>

mabartos · 2025-05-05T12:39:16Z

Latest changes

Root path is /admin/api
API version can be specified (/admin/api/v2)
If API version is not specified, the latest API version is used (/admin/api)

Signed-off-by: Martin Bartoš <mabartos@redhat.com>

* 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>

mabartos added the team/admin-api-wg Admin API v2 Working Group label Apr 29, 2025

mabartos force-pushed the admin-api-v2-skeleton branch 4 times, most recently from d84fce8 to 100131b Compare April 29, 2025 09:33

mabartos marked this pull request as ready for review April 29, 2025 09:41

mabartos requested review from a team as code owners April 29, 2025 09:41

keycloak-github-bot bot added the team/cloud-native label Apr 29, 2025

mabartos force-pushed the admin-api-v2-skeleton branch 2 times, most recently from 6ee7a49 to dfe6650 Compare April 29, 2025 12:05

keycloak-github-bot bot added the flaky-test label Apr 29, 2025

keycloak-github-bot bot reviewed Apr 29, 2025

View reviewed changes

mabartos marked this pull request as draft April 29, 2025 13:33

mabartos force-pushed the admin-api-v2-skeleton branch from dfe6650 to e87db8b Compare April 29, 2025 17:43

Add new ClientRepresentation

bc1ed83

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>

mabartos force-pushed the admin-api-v2-skeleton branch 2 times, most recently from 1b3db76 to 92fa00d Compare April 29, 2025 17:55

mabartos commented Apr 29, 2025

View reviewed changes

rest/admin-api/src/main/java/org/keycloak/admin/api/client/ClientsApi.java Show resolved Hide resolved

mabartos added 5 commits April 30, 2025 12:37

Add APIs

c730b89

Signed-off-by: Martin Bartoš <mabartos@redhat.com>

Add ApiModelMapper SPI

e08cd24

Signed-off-by: Martin Bartoš <mabartos@redhat.com>

Add MapStruct as default ApiModelMapper

82e3464

Signed-off-by: Martin Bartoš <mabartos@redhat.com>

Add default APIs implementations

e083766

Signed-off-by: Martin Bartoš <mabartos@redhat.com>

Provide Service SPI and ClientService

1edf09d

Signed-off-by: Martin Bartoš <mabartos@redhat.com>

mabartos added 4 commits April 30, 2025 12:42

Add default Keycloak services and Client service

bfbe728

Signed-off-by: Martin Bartoš <mabartos@redhat.com>

Add ModelMapper to shared modules

b9c63ce

Signed-off-by: Martin Bartoš <mabartos@redhat.com>

Implement Client service, add ServiceException class

2321b38

Signed-off-by: Martin Bartoš <mabartos@redhat.com>

Use ClientService in Client REST API

307c4d7

Signed-off-by: Martin Bartoš <mabartos@redhat.com>

mabartos force-pushed the admin-api-v2-skeleton branch from 92fa00d to 307c4d7 Compare April 30, 2025 10:43

vmuzikar approved these changes Apr 30, 2025

View reviewed changes

thomasdarimont reviewed May 5, 2025

View reviewed changes

rest/admin-api/src/main/java/org/keycloak/admin/api/AdminRootV2.java Show resolved Hide resolved

mabartos and others added 4 commits May 5, 2025 14:16

Update rest/admin-api/src/main/java/org/keycloak/admin/api/client/Cli…

d6fe20b

…entsApi.java Co-authored-by: Václav Muzikář <vaclav@muzikari.cz> Signed-off-by: Martin Bartoš <mabartos@redhat.com>

Fix ModelMapperSpi

135e91d

Signed-off-by: Martin Bartoš <mabartos@redhat.com>

Use /admin/api/v2 as a root path

4a5aa03

Signed-off-by: Martin Bartoš <mabartos@redhat.com>

Support latest API version by default

04966b2

Signed-off-by: Martin Bartoš <mabartos@redhat.com>

mabartos marked this pull request as ready for review May 5, 2025 12:37

Rename path param to comply with API spec

ff830f2

Signed-off-by: Martin Bartoš <mabartos@redhat.com>

vmuzikar approved these changes May 6, 2025

View reviewed changes

vmuzikar merged commit 364c48a into keycloak:feature/admin-api-v2 May 6, 2025
74 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Admin API v2] Skeleton prototype #39322

[Admin API v2] Skeleton prototype #39322

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

[Admin API v2] Skeleton prototype #39322

[Admin API v2] Skeleton prototype #39322

Uh oh!

Conversation

Uh oh!

Try it out

Get clients

Get client

Create client

Questions

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Unreported flaky test detected

org.keycloak.testsuite.webauthn.account.WebAuthnSigningInTest#categoriesTest

org.keycloak.testsuite.broker.KcOidcBrokerTest#loginWithExistingUserWithBruteForceEnabled

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Latest changes

Uh oh!

Uh oh!

Uh oh!