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
Make auth-webhook async #156
Conversation
return ack(req) | ||
except Exception: | ||
app.logger.exception('Failed to get secrets') | ||
return nak(req) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
❤️ nice "do not pass go" if we can't get secrets.
This converts auth-webhook from a synchronous Flask app managed by gunicorn to an asynchronous aiohttp app managed by gunicorn. This allows it to process any number of concurrent requests even if requests end up blocked or timing out on a configured external auth endpoint (keystone, custom addr, etc). Fixes: [lp:1927145][] [lp:1927145]: https://bugs.launchpad.net/charm-kubernetes-master/+bug/1927145
Co-authored-by: Kevin W Monroe <kevin.monroe@canonical.com>
Co-authored-by: Kevin W Monroe <kevin.monroe@canonical.com>
Co-authored-by: Kevin W Monroe <kevin.monroe@canonical.com>
4816cb4
to
08dfce2
Compare
Manual test run results: |
Cache the secrets in memory and refresh them in a background task, rather than hitting the API server again on every request. Also fixes the subprocess / kubectl calls not actually being async.
Ready for review. For the life of me, I can't see why the lint is still failing on GH. Manual test run results: Edit: And here's the auth-webhook.log as well, for good measure. |
78581b9
to
e29acd5
Compare
Apparently, I needed to recreate the tox env to update dependencies.
e29acd5
to
7c4357e
Compare
Apparently, the lint errors were due to a slightly out-of-date tox environment, and were entirely for doc-string formattings "issues" pretty much completely unrelated to this change. That is... annoying. |
Updated manual integration test with latest commits. |
Updated manual integration test run with latest changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Thanks
* Make auth-webhook async This converts auth-webhook from a synchronous Flask app managed by gunicorn to an asynchronous aiohttp app managed by gunicorn. This allows it to process any number of concurrent requests even if requests end up blocked or timing out on a configured external auth endpoint (keystone, custom addr, etc). Fixes: [lp:1927145][] [lp:1927145]: https://bugs.launchpad.net/charm-kubernetes-master/+bug/1927145 * Fix verify_ssl flag after failed cert Co-authored-by: Kevin W Monroe <kevin.monroe@canonical.com> * Fix rendering of extra auth URLs when they are None Co-authored-by: Kevin W Monroe <kevin.monroe@canonical.com> * Fix api_ver to be rendered as template var, not dynamic route Co-authored-by: Kevin W Monroe <kevin.monroe@canonical.com> * Cache secrets in memory Cache the secrets in memory and refresh them in a background task, rather than hitting the API server again on every request. Also fixes the subprocess / kubectl calls not actually being async. * Improve error handling around request and config file parsing * Add test for auth load / slow custom endpoint * Fix lint errors Apparently, I needed to recreate the tox env to update dependencies. * Add retry for reading kube config to account for race between charm and webhook * Drop Flask and Werkzeug, and pin aiohttp Co-authored-by: Kevin W Monroe <kevin.monroe@canonical.com>
Backported to stable in 00e66e6 |
* Make auth-webhook async This converts auth-webhook from a synchronous Flask app managed by gunicorn to an asynchronous aiohttp app managed by gunicorn. This allows it to process any number of concurrent requests even if requests end up blocked or timing out on a configured external auth endpoint (keystone, custom addr, etc). Fixes: [lp:1927145][] [lp:1927145]: https://bugs.launchpad.net/charm-kubernetes-master/+bug/1927145 * Fix verify_ssl flag after failed cert Co-authored-by: Kevin W Monroe <kevin.monroe@canonical.com> * Fix rendering of extra auth URLs when they are None Co-authored-by: Kevin W Monroe <kevin.monroe@canonical.com> * Fix api_ver to be rendered as template var, not dynamic route Co-authored-by: Kevin W Monroe <kevin.monroe@canonical.com> * Cache secrets in memory Cache the secrets in memory and refresh them in a background task, rather than hitting the API server again on every request. Also fixes the subprocess / kubectl calls not actually being async. * Improve error handling around request and config file parsing * Add test for auth load / slow custom endpoint * Fix lint errors Apparently, I needed to recreate the tox env to update dependencies. * Add retry for reading kube config to account for race between charm and webhook * Drop Flask and Werkzeug, and pin aiohttp Co-authored-by: Kevin W Monroe <kevin.monroe@canonical.com>
This converts auth-webhook from a synchronous Flask app managed by gunicorn to an asynchronous aiohttp app managed by gunicorn. This allows it to process any number of concurrent requests even if requests end up blocked or timing out on a configured external auth endpoint (keystone, custom addr, etc).
Fixes: lp:1927145