Performance of JWKS view

@jbpasquier recently completed a performance test and found that the call to the JWKS URI (/jwks/) took 103 seconds to complete. Looking at the view code, it loops over all RSAKey objects in the database, renders and returns the public key for each. The code itself is largely the same as the code forked from django-oidc-provider

Looking at community.startinblox and paris.happy-dev, both have around 55-60 keys in the database. Reading the docs I believe that these have been created using the python manage.py creatersakey command. I imagine that we don't need all of these and they're being created during the auto-deployment process? @plup

changed title from Performance investigation to Performance of JWKS view

added 30m of time spent at 2021-06-08

We had this discussion many times with @jbpasquier. I think from his point of view, the most keys we have the better.

But I always had a problem with generating keys at each deployment so at one point I limited this behavior. I ask Ansible to create a specific file when the key creation is made then if the file exists the step is skipped: https://git.startinblox.com/infra/platform/blob/master/roles/sibserver/tasks/configure_packages.yml#L8

So in a nutshell: no it doesn't come from the autodeployment anymore, but it might be a n artefact from its early moments.

Should I remove the keys everywhere and regenerate one ?

Should I remove the keys everywhere and regenerate one ?

@jbpasquier I think it needs you to answer this?

It's not an issue to remove all keys and regenerate them once.

Documentation told that more keys = more security and to always have at least two keys.

My theory is that having 60 keys is why the endpoint is slow but I haven't confirmed that in a test, just had a quick look during 30mins

@calummackervoy @plup any idea who could investigate that ? Maybe Martin if we ask gently ?

@balessan I think I should be able to next week but this week I have the MR to finish for the subscription proxy

Who is financing the issue?

oh sorry @plup I see that you might be working on this already?

@calummackervoy it's linked to the global performance epic so it's investment, SIB is paying for that :-)

@calummackervoy I'm off until the 1st of July. So I won't be able to do any progress on this subject until then. I don't think I can wait that mcuh time. Go for it if you have the opportunity.

I started my investigation. I'm obviously trying to reproduce the issue.

I don't see the same results:

$ time curl -I https://api.community.startinblox.com/jwks
HTTP/1.1 200 OK
Content-Type: application/json
Access-Control-Allow-Origin: *
Access-Control-Allow-Headers: authorization, Content-Type, if-match, accept, sentry-trace, DPoP
Vary: Accept-Encoding, Cookie
X-Frame-Options: DENY
Content-Length: 24925
Via: 1.1 alproxy
Date: Thu, 17 Jun 2021 21:12:39 GMT


real	0m3.276s
user	0m0.015s
sys	0m0.009s

Which seems to mean that it depends of external conditions. But all know how complicated it can be if e can't reliably reproduce the bug...

Distribution graph also indicates 3s in the most common case. I guess I still can try to optimize that but we are not talking about the same gain we started this issue with.

You have to be logged in and send a valid user DPoP & Authorization header to reach the pain point.

Might be replicable in a unit test? Not being able to use DPoP authentication in Postman or curl is a pain!

mentioned in issue management/core-team#2

added 15m of time spent at 2021-06-23

assigned to @calummackervoy and unassigned @plup

I quickly tried reducing 24 keys to 1 key in test2.startinblox.com and the performance improvement during login was noticeable (apologies for the lack of metrics!)

I did this via the admin panel, @jbpasquier is this the quickest way to roll it out to production?

Also note that the jwks/ endpoint is called here while verifying the DPoP access token: https://git.startinblox.com/djangoldp-packages/djangoldp-account/blob/master/djangoldp_account/auth/backends.py#L83. Removing keys will mean that some users have to log in again, they will receive a 401 response

is this the quickest way to roll it out to production?

Definitely not, it would require to create an admin account on every productions (55, as of platform's master). I know that @plup made some script to auto-run some commands everywhere. Do you have a table cleanup on your toolkit, @plup ?
@calummackervoy Does it work if we remove all of them, then re-create only one?

Does it work if we remove all of them, then re-create only one?

@jbpasquier yeah that should be fine

All right so I'll truncate the approriate table and then restart the command creation for all instances.

@jbpasquier @calummackervoy @balessan It's ready and tested on lab. Do you want me to deploy on all staging ? When should I do the prods ? It takes the time to deploy the server on all instances.

$ ansible -i inventory/lab/ all -m ansible.builtin.file -a "dest=~/startinblox/sibserver/.rsakeyid state=absent"
$ ansible-playbook -i inventory/lab/ maintain.yml -t drop_table -e table=oidc_provider_rsakey
$ ansible-playbook -i inventory/lab/ deploy.yml -t server

If we are sure yes you can proceed I guess.

Deploying per env:

• dev
• stg
• prd

Ok it's done. Unauthenticated requests are twice faster:

$ time curl -I https://api.community.startinblox.com/jwks
HTTP/1.1 200 OK
Content-Type: application/json
Access-Control-Allow-Origin: *
Access-Control-Allow-Headers: authorization, Content-Type, if-match, accept, sentry-trace, DPoP
Vary: Accept-Encoding, Cookie
X-Frame-Options: DENY
Content-Length: 463
Via: 1.1 alproxy
Date: Mon, 05 Jul 2021 15:52:09 GMT


real	0m1.542s
user	0m0.007s
sys	0m0.020

added 25m of time spent at 2021-06-29

assigned to @plup and unassigned @calummackervoy

closed

For legacy, the proper action script now:

$ ansible-playbook maintain.yml -t drop_table -e table="oidc_provider_rsakey,oidc_provider_client,oidc_provider_code,oidc_provider_token"
$ ansible-playbook maintain.yml -t rsakey