8000 [Admin v2] Querying draft by vmuzikar · Pull Request #41333 · keycloak/keycloak · GitHub
[go: up one dir, main page]
Skip to content

[Admin v2] Querying draft - #41333

Closed
vmuzikar wants to merge 5 commits into
keycloak:feature/admin-api-v2from
vmuzikar:querying
Closed

[Admin v2] Querying draft#41333
vmuzikar wants to merge 5 commits into
keycloak:feature/admin-api-v2from
vmuzikar:querying

Conversation

@vmuzikar
@vmuzikar vmuzikar commented Jul 22, 2025 •
edited
Loading
Copy link
Copy Markdown
Contributor

This PR is meant as a discussion with concrete example (hence not opening it just as a GH Discussion) of how querying could work, it is not meant as a final result (even just for the prototype).

The design choices made here are made with simplicity in mind, i.e. it does not itroduce arbitrary existing formats or libraries. This allows us to have full control over what expressions we support and what we don't support, while keeping the implementation relatively simple.

It captures two parts:

  • Query syntax, incl. the parsing.
  • Resource filtering based on the parsed query.

Query syntax

Uses a custom syntax inspired by GitHub APIs. We're making a few assumptions here to keep it simple which implies a few limitations.

  • Simple expression: field:value
  • Combined expression: field1:value1 field2:value2
    • Treated as AND.
  • Quoting: field:"value with spaces and :colons"
    • This allows spaces and colons in the expressions.
    • Escaping quotes is not supported, i.e. expressions can't contain quotes.
  • Nested fields: field.subfield:value
  • Lists and maps: field:[item1, item2] or field:[key1:val1, key2:val2]
    • List syntax can be used to check presence of a key in a map too. E.g. to lookup resources that contain an attribute named phone, we could do something like attrs:[phone].
    • Lists and maps are treated as "contains", not "equals". I.e. the expression means "I'm looking for resources that include these elements in this collection field".
    • Nested fields are not supported.
    • Quotes (i.e. spaces and colons in the expressions) are not supported.
    • Commas in the expressions are not supported.
    • No suppo 8000 rt for nested collections.

Additional limitations:

  • Expressions implicitly support only AND, i.e. more complex expressions with OR are not supported.
  • NOT is not supported at the moment, but could be relatively easily added.
  • Everything is compared for equality as a String.

Alternatives

SCIM query syntax is an obvious candidate though there are a few considerations to make around that.

  • Full SCIM support as such is out of scope of the Admin v2, that’s a completely different effort.
  • When we add full support for SCIM, it will be different endpoints from Admin APIs.
  • SCIM is primarily meant to be used for provisioning users and groups.
  • So if we adopted SCIM for querying in Admin v2, it would be just for the syntax.
    • It might be confusing to the users as we'd support just the syntax but not the schema, endpoints etc.
    • The main benefit of that would be a familiar syntax for users, and also that we could potentially leverage some library for parsing the queries. And we could also share the logic when we adopt SCIM.
    • The downside is the complexity. And if we stripped it down just to some basic operators, it wouldn’t brought too many benefits and users would naturally expect we will add more if it's too similar to the SCIM query syntax.
    • But problem is also which library we could use. There’s not that many, one of the most popular ones seem to be pingidentity/scim2 but it seems to have a single maintainer, only a few forks and it’s not very active. I would hesitate to adopt it in KC.

Resource filtering

  • Works in general for all fields.
    • We might want to introduce an exclusion mechanism (e.g. using annotations) for fields we do not want to be searchable.
  • Done at runtime on the result set fetched from the DB.
    • This doesn't help the server performance, but network and clients would benefit from it.
    • Can be easily extended to do optimized SQL queries before the runtime filtering to help the server side performance.
  • Currently uses reflections.
    • Very poor performance.
    • We should consider some alternative approach.
      • Maybe filtering somewhere at the Jakson level when serializing? It would not help if we needed to do some additional processing of the results before serializing.
      • Or static class generation at build time to hardcode the filtering for all fields (which we know at the build time) of all supported representations?

mabartos and others added 5 commits July 22, 2025 13:18
@mabartos @Pepo48 @vmuzikar
[Admin API v2] Skeleton prototype (keycloak#39322)
e44402e 
* 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>
@shawkins @mabartos
refinements related to service methods, serdes (keycloak#40110)
4b5cf70 
and adding a default to representation

Signed-off-by: Steve Hawkins <shawkins@redhat.com>
@shawkins @mabartos
shows what patching logic should look like (keycloak#40450)
5dad958 
* shows what patching logic should look like

also shows basic field validation

Signed-off-by: Steve Hawkins <shawkins@redhat.com>

* updates based upon review comments

Signed-off-by: Steve Hawkins <shawkins@redhat.com>

---------

Signed-off-by: Steve Hawkins <shawkins@redhat.com>
8000
@shawkins @mabartos
implementing json patch (keycloak#40904)
aadbf3f 
* implementing json patch

Signed-off-by: Steve Hawkins <shawkins@redhat.com>

* adding zjsonpatch to the dist and adding basic tests

Signed-off-by: Steve Hawkins <shawkins@redhat.com>

---------

Signed-off-by: Steve Hawkins <shawkins@redhat.com>
@vmuzikar
[Admin v2] Querying draft
658ad3d 
Signed-off-by: Václav Muzikář <vmuzikar@redhat.com>
@vmuzikar vmuzikar added the team/admin-api-wg Admin API v2 Working Group label Jul 22, 2025
keycloak-github-bot[bot]
keycloak-github-bot Bot reviewed Jul 22, 2025
View reviewed changes
@keycloak-github-bot keycloak-github-bot Bot left a comment
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unreported flaky test detected, please review

@keycloak-github-bot
keycloak-github-bot Bot commented Jul 22, 2025
Copy link
Copy Markdown

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.cluster.GroupInvalidationClusterTest#crudWithFailover

Keycloak CI - Clustering IT

java.lang.RuntimeException: java.lang.IllegalStateException: Keycloak unexpectedly died :(
	at org.keycloak.testsuite.arquillian.containers.KeycloakQuarkusServerDeployableContainer.start(KeycloakQuarkusServerDeployableContainer.java:71)
	at org.jboss.arquillian.container.impl.ContainerImpl.start(ContainerImpl.java:185)
	at org.jboss.arquillian.container.impl.client.container.ContainerLifecycleController$8.perform(ContainerLifecycleController.java:137)
	at org.jboss.arquillian.container.impl.client.container.ContainerLifecycleController$8.perform(ContainerLifecycleController.java:133)
...

Report flaky test

@shawkins
shawkins commented Jul 22, 2025
Copy link
Copy Markdown
Contributor

We might want to introduce an exclusion mechanism (e.g. using annotations) for fields we do not want to be searchable.

What fields would you not want to be searchable?

Can be easily extended to do optimized SQL queries before the runtime filtering to help the server side performance.

In situations where logical operations take place after filtering you have to be careful. Partial push-down of predicates means you can't also pushdown limit / offset, or grouping should that ever need to be supported. You could also consider having formal parameters for things that are indexed, rather than looking for them in the q parameter.

Maybe filtering somewhere at the Jakson level when serializing?

There is subtlety here. Reflection / toString may not capture the actual Jackson mapping. And it flattens the type model - everything is a string, which may be problematic for numerical values - especially floating point numbers or if we introduce greater than / less than.

You could entertain simply supporting json path predicates on top of the result rather than a custom query syntax. Of course json path could be used for projection as well.

@stianst
stianst commented Jul 23, 2025
Copy link
Copy Markdown
Contributor

Quite related to search is sort, do we have some discussion around that?

@stianst
stianst commented Jul 23, 2025
8000 Copy link
Copy Markdown
Contributor

I would like to see some research on different approaches in existing APIs (I mostly like GH search syntax, but..), as well as looking for if there is some existing libraries that can be useful rather than inventing/implementing our own.

I'd also like to see some more concrete use-cases/examples; for example find me all clients that have at least one web-origin set, or all clients that do use a non-https redirect-uri. Expand the examples also to users, as that would have different use-cases.

@stianst
stianst commented Jul 23, 2025
Copy link
Copy Markdown
Contributor

I don't like commenting/discussing this through a PR (as this PR could be closed and then discussions would be somewhat lost), would be better to use a discussion.

@mabartos
mabartos force-pushed the feature/admin-api-v2 branch from 266bb90 to d207105 Compare August 5, 2025 14:49
@mabartos
mabartos force-pushed the feature/admin-api-v2 branch from 02245d9 to 17e8407 Compare September 5, 2025 09:08
@vmuzikar vmuzikar closed this May 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@keycloak-github-bot keycloak-github-bot[bot] keycloak-github-bot[bot] left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

flaky-test team/admin-api-wg Admin API v2 Working Group

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vmuzikar @shawkins @stianst @mabartos
0