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

Ability to disable keepalive #307

Merged
merged 4 commits into from
Feb 11, 2020
Merged

Conversation

3bit-techs
Copy link

Hello everyone,
In one of our customers, we use this (amazing) library to behave like a lightweight and fast oidc reverse proxy (deployed as a k8s sidecar along with SPA applications).
To be able to proxy page requests and also ajax requests, we configured the proxy to have two default actions (for non authenticated requests):

  • For ajax requests we deny every request that has failed to authenticate
  • For page requests we don't specify an action (nil), redirecting to the OP if the requests it's not already logged in

However, after some time we started to notice intermittent failures with the ajax requests. The request failed but the session was valid (so it wasn't because HTTP session expiration, SSO session revoked, etc.).

On the openresty logs we found this error:

2020/02/06 18:32:57 [error] 8#8: *11162 [lua] openidc.lua:1406: authenticate(): lost access token:accessing token endpoint (https://sso.acme.com/auth/realms/realm/protocol/openid-connect/token) failed: closed, client: x.x.x.x, server: frontend.project.svc, request: "GET / HTTP/1.1", host: "frontend.acme.com", referrer: "https://frontend.acme.com/api"

After some research we figured out that the lua-resty-openidc uses a library called lua-resty-http to make calls to the OP.

This library (lua-resty-http) by default enables keepalive on the connections so it can create a connection pool to cache and reuse them later. This is necessary for a lot of good reasons (mostly performance related), but for some cases this behaviour can generate this issues, because the connection eventually will be closed and in my case this intermittent errors caused unnecessary api retries and extra traffic.

The idea of this PR is to make it possible for the user to enable or disable keepalive on the lua-resty-http library (just like the ssl_verify option).

The default behaviour would be to keep the keepalive enabled, to avoid causing problems with other users.

Regards,

@bodewig
Copy link
Collaborator

bodewig commented Feb 8, 2020

Looks good to me.

You may want to add something to the changelog and add yourself to the authors file.

@3bit-techs
Copy link
Author

Looks good to me.

You may want to add something to the changelog and add yourself to the authors file.

Done!

Thank you @bodewig .

@bodewig bodewig merged commit a0d2707 into zmartzone:master Feb 11, 2020
@bodewig
Copy link
Collaborator

bodewig commented Feb 11, 2020

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants