keycloak / terraform-provider-keycloak

Terraform provider for Keycloak
https://registry.terraform.io/providers/mrparkers/keycloak/latest/docs
Apache License 2.0
650 stars 317 forks source link

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

Open sybereal opened 6 months ago

sybereal commented 6 months ago

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:

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.