-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
[bitnami/keycloak]: add url env vars to fix admin ingress access #25386
[bitnami/keycloak]: add url env vars to fix admin ingress access #25386
Conversation
ddc7de3
to
92a38c8
Compare
Thanks for your contribution, due to some changes in our test suite it is needed to rebase from main and the update the PR, could you please do that? Thanks beforehand and sorry for the inconvenience. |
92a38c8
to
763b079
Compare
Sure, no worries! |
Hi @MrFreezeex, Thanks for creating this PR. Could you please take a look at my suggestions? I think the URL is not generated properly. |
Replace KC_HOSTNAME_ADMIN by KC_HOSTNAME_ADMIN_URL to include the scheme (http or https) and add KC_HOSTNAME_URL for the frontend to function correctly. These variable allow the front to function correctly if we have the tls option enabled. Before this commit the front was always attempting to load some resources over http while it may be exposed in https. Signed-off-by: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@ledger.fr>
Signed-off-by: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@ledger.fr>
e246ba1
to
36eaa7f
Compare
Woops indeed sorry for that and thanks for catching it! I just fixed it |
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.
LGTM! Thanks for your contribution.
…nami#25386) * [bitnami/keycloak]: add url env vars to fix admin ingress access Replace KC_HOSTNAME_ADMIN by KC_HOSTNAME_ADMIN_URL to include the scheme (http or https) and add KC_HOSTNAME_URL for the frontend to function correctly. These variable allow the front to function correctly if we have the tls option enabled. Before this commit the front was always attempting to load some resources over http while it may be exposed in https. Signed-off-by: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@ledger.fr> * [bitnami/keycloak]: fix license header to fix the CI Signed-off-by: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@ledger.fr> --------- Signed-off-by: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@ledger.fr> Signed-off-by: Matheus Goncalves <matheus.goncalves@bexio.com>
…nami#25386) * [bitnami/keycloak]: add url env vars to fix admin ingress access Replace KC_HOSTNAME_ADMIN by KC_HOSTNAME_ADMIN_URL to include the scheme (http or https) and add KC_HOSTNAME_URL for the frontend to function correctly. These variable allow the front to function correctly if we have the tls option enabled. Before this commit the front was always attempting to load some resources over http while it may be exposed in https. Signed-off-by: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@ledger.fr> * [bitnami/keycloak]: fix license header to fix the CI Signed-off-by: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@ledger.fr> --------- Signed-off-by: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@ledger.fr> Signed-off-by: Matheus Goncalves <matheus.goncalves@bexio.com>
…nami#25386) * [bitnami/keycloak]: add url env vars to fix admin ingress access Replace KC_HOSTNAME_ADMIN by KC_HOSTNAME_ADMIN_URL to include the scheme (http or https) and add KC_HOSTNAME_URL for the frontend to function correctly. These variable allow the front to function correctly if we have the tls option enabled. Before this commit the front was always attempting to load some resources over http while it may be exposed in https. Signed-off-by: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@ledger.fr> * [bitnami/keycloak]: fix license header to fix the CI Signed-off-by: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@ledger.fr> --------- Signed-off-by: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@ledger.fr> Signed-off-by: Matheus Gonçalves <mathmgoncalves@gmail.com>
…nami#25386) * [bitnami/keycloak]: add url env vars to fix admin ingress access Replace KC_HOSTNAME_ADMIN by KC_HOSTNAME_ADMIN_URL to include the scheme (http or https) and add KC_HOSTNAME_URL for the frontend to function correctly. These variable allow the front to function correctly if we have the tls option enabled. Before this commit the front was always attempting to load some resources over http while it may be exposed in https. Signed-off-by: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@ledger.fr> * [bitnami/keycloak]: fix license header to fix the CI Signed-off-by: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@ledger.fr> --------- Signed-off-by: Arthur Outhenin-Chalandre <arthur.outhenin-chalandre@ledger.fr> Signed-off-by: Matheus Gonçalves <mathmgoncalves@gmail.com>
Description of the change
Replace KC_HOSTNAME_ADMIN by KC_HOSTNAME_ADMIN_URL to include the scheme (http or https) and add KC_HOSTNAME_URL for the frontend to function correctly. These variable allow the front to function correctly if we have the tls option enabled. Before this commit the front was always attempting to load some resources over http while it may be exposed in https.
Benefits
Keycloak admin ingress works if tls/https is configured
Possible drawbacks
If some users defined
KC_HOSTNAME
orKC_HOSTNAME_URL
in extraEnv it could prevent keycloak to start, I reduced the risk by adding this variable only if the ingress is not enabled thoughApplicable issues
Additional information
Checklist
Chart.yaml
according to semver. This is not necessary when the changes only affect README.md files.README.md
using readme-generator-for-helm