Why can't keycloak-admin-client-reactive use a custom ClientRequestFilter?

[mickroll]

We are in the process of switching from an OpenAPI based Keycloak access to using keycloak-admin-client-reactive.

With our previous approach we could add custom ClientRequestFilter (and ClientResponseFilter) by registering them either on a specific Api interface using @org.eclipse.microprofile.rest.client.annotation.RegisterProvider(ExampleClientRequestFilter.class) or globally by annotating the filter class with @javax.ws.rs.ext.Provider.

But the rest client that is internally used by keycloak-admin-client-reactive does neither pick up any global filter that is annotated by @Provider nor respect a @RegisterProvider annotation on an extension interface that we stuff into org.keycloak.admin.client.Keycloak.proxy(Class<KeycloakAdminExtensionApi>, URI).

I think that is because io.quarkus.keycloak.admin.client.reactive.runtime.ResteasyReactiveClientProvider has its own logic to build a jakarta.ws.rs.client.Client, instead of using something like io.quarkus.rest.client.reactive.runtime.RestClientBuilderImpl. The latter has extensive logic in its build method for extending the client while building it.

I am a bit puzzled now. Why does ..keycloak..ResteasyReactiveClientProvider implement its own logic for creating a rest client instead of reusing the existing logic of io.quarkus.rest.client.reactive?

Especially having a way to add a ClientRequestFilter/ClientResponseFilter to the used rest client would be very useful to our use case.

[quarkus-bot[bot]]

/cc @pedroigor (keycloak), @sberyozkin (keycloak)

[sberyozkin]

Hi @mickroll Sorry for a delay, as far as I recall it was tried originally but it was causing side-effects for Keycloak Admin API calls, example, Jackson provider would be picked up. etc. Also CC @michalvavrik

[mickroll]

No worries @michalvavrik, thanks for your extensive answer.

To me, having a complete custom ResteasyClientProvider doesn't feel right. It's too deep down the rabbit hole i think.

On the other hand, the proposed ResteasyReactiveClientProviderCustomizer#customize(ClientBuilderImpl) seems really nice, as there is already org.jboss.resteasy.reactive.client.impl.ClientBuilderImpl.clientLogger(ClientLogger) which should perfectly fit our use case. Would this ResteasyReactiveClientProviderCustomizer be called for both keycloak-client and 'normal' reactive clients? This would be a nice way to have a customizable ClientLogger for all reactive clients.

Thinking a bit further, instead of exposing the impl class ClientBuilderImpl, which seems a bit weird as it is a somewhat internal class, how about the following: ClientBuilderImpl could, instead of initializing its clientLogger with new DefaultClientLogger(), use an existing ClientLogger bean and just fall back to DefaultClientLogger there. That would feel more "quarkus style" to me: write a custom bean, otherwise a default one is used. And keycloak client logger config would (should) work similar as for reactive rest clients.
This would need some adjustments in io.quarkus.keycloak.admin.client.reactive.runtime.ResteasyReactiveClientProvider to mimic the behaviour of io.quarkus.rest.client.reactive.runtime.RestClientBuilderImpl.build(Class<T>) regarding handling of the io.quarkus.restclient.config.RestClientLoggingConfig, which in turn would have to be used in org.keycloak.admin.client.Config.

All-in-all i propose a keycloak client, whose configurability (via config properties or beans) is more similar to the 'normal' reactive rest clients. (Minus the problems with custom Jackson handling ;-) )

WDYT?

[michalvavrik]

Your idea is better @mickroll, I didn't like allowing customization of this particular client anyway (it feels like something that should be provided by KC adapter, not us, as they know how their own endpoints). This should be RESTEasy Reactive feature then, would you like to go ahead and create PR? I can do it now, but it is not time sensitive, you can take your time...

@geoand we would like to have RR to pick up org.jboss.resteasy.reactive.client.api.ClientLogger bean if available and replace org.jboss.resteasy.reactive.client.logging.DefaultClientLogger. WDYT? Same will be done by io.quarkus.keycloak.admin.client.reactive.runtime.ResteasyReactiveClientProvider.

[mickroll]

Thanks @michalvavrik,

having the custom ClientLogger picked up is only part of the solution: it still needs to be configured. And this has to happen in io.quarkus.keycloak.admin.client.reactive.runtime.ResteasyReactiveClientProvider as only this knows where to get the configuration from: eg. org.keycloak.admin.client.Config.

I'll try to create a PR for the ClientLogger bean pickup.

[geoand]

@michalvavrik sure, that makes sense to me

[mickroll]

I added a PR: #33868