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

[bitnami/keycloak] Added aliases for environment variables from Keycloak documentation #73141

Closed
wants to merge 1 commit into from

Conversation

Fydon
Copy link

@Fydon Fydon commented Oct 7, 2024

Description of the change

Currently some of the environment variable mentioned in the Keycloak documentation have aliases in the scripts, but others do not. I followed the changes from #72481 to build this pull request.

Benefits

Transferring an existing setup from the Keycloak containers to the Bitnami setup takes less effort, although there are differences.

Possible drawbacks

It may make the setup differences less obvious, but then all aliases should be removed.

Applicable issues

Currently none.

Additional information

Keycloak 26 has been released. When the new release is created there may be new environment variables that can have aliases.

Signed-off-by: Trevor Dearham <trevor.dearham@gmail.com>
@carrodher carrodher added verify Execute verification workflow for these changes in-progress labels Oct 9, 2024
@github-actions github-actions bot removed the triage Triage is needed label Oct 9, 2024
@github-actions github-actions bot removed the request for review from carrodher October 9, 2024 17:30
@github-actions github-actions bot requested a review from migruiz4 October 9, 2024 17:30
@migruiz4
Copy link
Member

Hi @Fydon,

Thank you very much for your contribution!

Nevertheless, I don't think we can merge this feature.

First, I would like to clarify the difference between KEYCLOAK_ and KC_ environment variables.

The KEYCLOAK_ env variables correspond to variables used by the Bitnami initialization logic (libkeycloak.sh), while the KC_ env variables correspond to variables directly used by the Keycloak binary.

For context, many of the KEYCLOAK_ were implemented before the KC_ variables existed, so that is why there are duplicities between some of them.

Our goal now is to start removing the logic behind for some of the KEYCLOAK_ variables and use the KC_ instead while keeping the KEYCLOAK_ env var as an alias for backward-compatibility purposes.

The reason why I can not accept this PR is because it performs the opposite we are looking for. It aliases KC_ env variables to KEYCLOAK_ variables, which may cause issues due to the keycloak binary receiving configuration duplicated (from settings configured by the init logic and the KC_ env variable).

I'm sorry for the inconvenience.

@Fydon Fydon closed this Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keycloak solved verify Execute verification workflow for these changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants