-
Notifications
You must be signed in to change notification settings - Fork 170
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
Some fixes / improvements for Keycloak integration #310
Conversation
apicast/src/configuration_loader.lua
Outdated
@@ -128,7 +128,11 @@ end | |||
function boot.init_worker(configuration) | |||
local interval = ttl() or 0 | |||
|
|||
configuration.keycloak = keycloak.load_configuration() | |||
local keycloak_config = _M.run_external_command("keycloak") |
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.
We should move this to the .init
function. init_worker
is executed for each worker, but init
just once.
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.
Yep, in fact it's already in the .init
. I guess should be just removed from here, as Keycloak configuration is not something that will change with automatic reload. Right?
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.
And it is already loaded. Init_worker is executed right after. So it should be removed.
And of course, if this was broken there was no test so this should be covered by a new test. |
…INT environment variable is set
(fixes the issue with initial client options for ssl verification)
910d007
to
10870e7
Compare
This PR fixes the issue with getting the configuration by Keycloak when running with
APICAST_CONFIGURATION_LOADER=boot
.