Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cache well-known responses to avoid making too much calls to the IdP #251

Merged
merged 1 commit into from
Apr 17, 2024

Conversation

nacx
Copy link
Collaborator

@nacx nacx commented Apr 17, 2024

While working on #250, I saw that authservice was calling many times the well-known endpoint for a single OIDC session. This is because we call it when initializing the OIDC handlers, and that is done per request to the Envoy ext-authz filter, and a single OIDC session may involve several requests, given the redirects, making several unnecessary calls to the IdP.

This PR caches the seen well-known requests, and returns them from the cache for subsequent requests. The well-known endpoints are usually stable and it is very rare that they change, so it should be fine to just cache them by default.

Copy link
Collaborator

@sergicastro sergicastro left a comment

Choose a reason for hiding this comment

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

Nice

@istio-testing
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nacx, sergicastro

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@istio-testing istio-testing merged commit 1df3e7e into istio-ecosystem:main Apr 17, 2024
12 checks passed
@nacx nacx deleted the discovery-cache branch April 17, 2024 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants