Keycloak upgrade #5
Conversation
🔧 (Codeowners): update code owners list ⬆️ (Dockerfile): upgrade Keycloak base image and dependencies 📝 (README.md): add TODO section for build instructions 🔧 (docker-compose.yml): add docker-compose configuration for Keycloak ✨ (jars): add new jar files for postgres-socket-factory and jgroups-google
roosnic1
requested review from
devnoname120 and
RudolfSchreier
as code owners
| KC_DB_USERNAME: keycloak | ||
| KC_DB_PASSWORD: keycloak |
There was a problem hiding this comment.
ditto: Is there a safety mechanism so that we don't accidentally use these weak credentials in production?
| FROM quay.io/phasetwo/phasetwo-keycloak:25.0.5 AS base | ||
| #FROM quay.io/keycloak/keycloak:25.0.5 AS base |
There was a problem hiding this comment.
What is the difference between these two?
There was a problem hiding this comment.
@devnoname120 the first one is the image with phasetwo plugin, the second one is pure keycloak
There was a problem hiding this comment.
Ok in this case I suggest to remove the commented out code:
| FROM quay.io/phasetwo/phasetwo-keycloak:25.0.5 AS base | |
| #FROM quay.io/keycloak/keycloak:25.0.5 AS base | |
| FROM quay.io/phasetwo/phasetwo-keycloak:25.0.5 AS base |
| KEYCLOAK_ADMIN: admin | ||
| KEYCLOAK_ADMIN_PASSWORD: password |
There was a problem hiding this comment.
Is there a safety mechanism so that we don't accidentally use these weak credentials in production?
There was a problem hiding this comment.
I agree with @swisstxtsokol (If I understood the comment right, please LMK!):
- adding an
example.env - referencing a
env_file: .envhere - having the developer rename
example.envto.envto get started
instead would force the developer to think about it at least once and avoid inadvertent mistakes.
(As free bonus, it would also reduce duplication of the DB username/password)
| POSTGRES_USER: keycloak | ||
| POSTGRES_PASSWORD: keycloak |
There was a problem hiding this comment.
ditto: Is there a safety mechanism so that we don't accidentally use these weak credentials in production?
| KC_DB_URL: jdbc:postgresql://db:5432/keycloak | ||
| KC_DB_USERNAME: keycloak | ||
| KC_DB_PASSWORD: keycloak | ||
| KC_HOSTNAME: localhost |
There was a problem hiding this comment.
Should it be the url of our keycloak instance?
| dockerfile: Dockerfile | ||
| environment: | ||
| KEYCLOAK_ADMIN: admin | ||
| KEYCLOAK_ADMIN_PASSWORD: password |
There was a problem hiding this comment.
Maybe move the .env?
| @@ -1 +1,6 @@ | |||
| # keycloak-with-socket-factory | |||
|
|
|||
| # TODO: | |||
There was a problem hiding this comment.
Are there stories to do these TODOs?
I am always a fan of putting a Jira story id on TODOs so it is clear if is more of a nice-to-have style TODO or an actual lets-do-this-next kind of TODO.
| @@ -0,0 +1,38 @@ | |||
| version: '3.8' | |||
There was a problem hiding this comment.
The change itself was a bit contentious, but version in docker-compose.yml files is now obsolete.
I would not go back through all of them to remove it, but if you are adding a new one, I think it would be better to leave it out.
(Totally optional change though 😆)
| KEYCLOAK_ADMIN: admin | ||
| KEYCLOAK_ADMIN_PASSWORD: password |
There was a problem hiding this comment.
I agree with @swisstxtsokol (If I understood the comment right, please LMK!):
- adding an
example.env - referencing a
env_file: .envhere - having the developer rename
example.envto.envto get started
instead would force the developer to think about it at least once and avoid inadvertent mistakes.
(As free bonus, it would also reduce duplication of the DB username/password)
This PR upgrades keycloak to version 25.0.5 and adds a plugin from phasetwo. It also adds a docker-compose for local testing.