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

feat: add support for setting keycloak redirect URIs for lagoon UI #683

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rocketeerbkw
Copy link
Member

Support was added to set valid redirect URIs for the lagoon UI client in uselagoon/lagoon#3640. This PR adds this setting to the charts. It defaults to the UI ingress, but can be overridden by keycloakLagoonUIClientRedirectUris.

@shreddedbacon
Copy link
Member

This is sort of a breaking change. I'm fine with it, but it has to be called out in the release notes

@shreddedbacon shreddedbacon added this to the core-v2.21.0 milestone Sep 9, 2024
@rocketeerbkw
Copy link
Member Author

rocketeerbkw commented Sep 9, 2024

I'm not sure this is the best option. If there are more client settings that we have to accommodate later, maybe it's better to have no default and do something like:

keycloak:
  clientSettings:
    lagoonUI:
      redirectUris: 

@shreddedbacon
Copy link
Member

I'm not sure this is the best option. If there are more client settings that we have to accommodate later, maybe it's better to have no default and do something like:

keycloak:
  clientSettings:
    lagoonUI:
      redirectUris: 

There is prior art for this sort of structure too https://github.com/uselagoon/lagoon-charts/blob/main/charts/lagoon-core/values.yaml#L235-L243

We'd obviously need to only handle clients that exist already or are created by Lagoon so that lagoonUi can be transformed to lagoon-ui in keycloak though.

@shreddedbacon shreddedbacon removed this from the core-v2.21.0 milestone Sep 9, 2024
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.

2 participants