terraform-provider-keycloak icon indicating copy to clipboard operation
terraform-provider-keycloak copied to clipboard

`keycloak_openid_audience_protocol_mapper`: Unpaginated request to `/admin/realms/${realm}/clients`

Open sybereal opened this issue 9 months ago • 0 comments

~~For reasons I do not understand, even after a cursory glance over the source code,~~ the keycloak_openid_audience_protocol_mapper resource issues a request to /admin/realms/${realm}/clients when creating.

Notably, this request is not parameterized in any way, i.e. no pagination. This means the provider tries to load the entire client list at once, which can be prohibitively expensive and result in timeouts, especially when the Keycloak server has been restarted shortly before and the clients are not yet cached.

For context, we create a number of other Keycloak resources as well, specifically the following:

  • keycloak_generic_protocol_mapper
  • keycloak_openid_client
  • keycloak_openid_user_client_role_protocol_mapper
  • keycloak_role

But this issue occurs only with keycloak_openid_audience_protocol_mapper specifically.

Example:

keycloak_openid_audience_protocol_mapper.audience: Creation complete after 32s [id=00000000-0000-0000-0000-000000000000]

Corresponding access log line (method, path, status, duration in ms):

GET /admin/realms/Portal/clients 200 31883

EDIT: After digging around the source code some more, I found the reason for the request.

https://github.com/mrparkers/terraform-provider-keycloak/blob/3f6b75b79ada48eddb41de6055f57a357d9b691c/keycloak/openid_audience_protocol_mapper.go#L127

Given the context of that line, I also understand now why the request is made.

However, performance-wise, I believe GetGenericClientByClientId() instead of listGenericClients() would be preferrable. At least, I don't see a reason not to use it. I'll try preparing a patch for that.

sybereal avatar May 13 '24 13:05 sybereal