search

jhipster / generator-jhipster

JHipster is a development platform to quickly generate, develop, & deploy modern web applications & microservice architectures.
https://www.jhipster.tech
Apache License 2.0
21.27k stars 4k forks source link

Upgrade keycloak to 18.0.0 #18441

Closed yelhouti closed 2 years ago

yelhouti commented 2 years ago

Please make sure the below checklist is followed for Pull Requests.

When you are still working on the PR, consider converting it to Draft (below reviewers) and adding skip-ci label, you can still see CI build result at your branch.

yelhouti commented 2 years ago

fixes: #17850

yelhouti commented 2 years ago

@DanielFran @vishal423 can anyone give access to the cypress dashboard so I can see why some tests are sporadic. Ouath2/keycloak seems to work only half the time :s

vishal423 commented 2 years ago

@yelhouti, I suggest trying failed workflows on local env to reproduce issues

yelhouti commented 2 years ago

@vishal423 I could not reproduce locally, also they fail sporadically in the CI. This is why some ouath2 test pass and other fails. Is there a way to access cypress dashboard ?

yelhouti commented 2 years ago

are you saying there is a recent change in docker ports mapping behaviour? If not, then, your configuration is incorrect. The previous configuration used to work and was thoroughly tested in local and CI (with docker compose).

@vishal423 no what I am saying is the way keycloak starts changed (no way to have a port offset of 1000) https://github.com/jhipster/generator-jhipster/pull/18441/files#diff-1bfaf6e31fe218cd507027450c04913b93f510df3f63b4a2ea84163c6423e9fbL339

But we still want to offset ports to have docker and keycloak not fight for port 8080 on localhost so we map keycloak:8080 to localhost:9080

https://github.com/jhipster/generator-jhipster/pull/18441/files#diff-8eef712ac6e760fcb9b7b54a8cb6d5579608b146d14323efa75a30bb1397f82bR107

Also please give others the benefit of the doubt :)

vishal423 commented 2 years ago

are you saying there is a recent change in docker ports mapping behaviour? If not, then, your configuration is incorrect. The previous configuration used to work and was thoroughly tested in local and CI (with docker compose).

@vishal423 no what I am saying is the way keycloak starts changed (no way to have a port offset of 1000) https://github.com/jhipster/generator-jhipster/pull/18441/files#diff-1bfaf6e31fe218cd507027450c04913b93f510df3f63b4a2ea84163c6423e9fbL339

But we still want to offset ports to have docker and keycloak not fight for port 8080 on localhost so we map keycloak:8080 to localhost:9080

https://github.com/jhipster/generator-jhipster/pull/18441/files#diff-8eef712ac6e760fcb9b7b54a8cb6d5579608b146d14323efa75a30bb1397f82bR107

Also please give others the benefit of the doubt :)

Got it. You are trying to access within the docker network. Failure tests seems related to micro-frontend and micro-services. Can you try those flows locally?

yelhouti commented 2 years ago

I tried ms-angular locally which one would you recommend.

Also --monolith and --worksoace doesn't seem to work correctly maybe I am doing something wrong

I use scripts 11 and 12 the same way they are used on the workflow

mraible commented 2 years ago

Can your please fix conflicts @yelhouti?

yelhouti commented 2 years ago

I spent the last few hours understanding the issue with my PR and it is as follows:

Since keycloak doesn't support port offset anymore we are "forced" to change the port mapping at the docker level. The issue then becomes as follows: Since our code uses the same issuer-uri to:

  1. talk directly to the IDP (verify tokens...)
  2. redirect the user for login If we set it to:
  3. keycloak:8080 the browser is unable to talk to it because it can't resolve keycloak correctly (and pointing to localhost as I suspect we do now doesn't solve the problem) as mentioned in docker-compose.yaml: # For Keycloak to work, you need to add '127.0.0.1 keycloak' to your hosts file (which is cheating and I don't know how github actions was configured to do that) EDIT: which is done in 03-system.sh
  4. localhost:9080 the gateway is not able to talk to localhost correctly because it is inside the container.

Possible solution: The gateway should know 2 urls: 1 for internal use (to verify token, retrieve oidc config in Security Configuration...) and one to return to the frontend or redirect the frontend to. There are other solutions I could think of but they are much more complicated, change the used authentication flow, or don't work in some edge cases...

What the you all think we should do about this ? @mraible @vishal423

echo "127.0.0.1 keycloak" | sudo tee -a /etc/hosts

mraible commented 2 years ago

Can we convince Keycloak to add the port offset back in? Is there an issue to track its removal?

On Tue, May 17, 2022 at 07:40 yelhouti @.***> wrote:

I spent the last few hours understanding the issue with my PR and it is as follows:

Since keycloak doesn't support port offset anymore we are "forced" to change the port mapping at the docker level. The issue then becomes as follows: Since our code uses the same issuer-uri to:

  1. talk directly to the IDP (verify tokens...)
  2. redirect the user for login If we set it to:
  3. keycloak:8080 the browser is unable to talk to it because it can't resolve keycloak correctly (and pointing to localhost as I suspect we do now doesn't solve the problem) as mentioned in docker-compose.yaml: # For Keycloak to work, you need to add '127.0.0.1 keycloak' to your hosts file (which is cheating and I don't know how github actions was configured to do that
  4. localhost:9080 the gateway is not able to talk to localhost correctly because it is inside the container.

Possible solution: The gateway should know 2 urls: 1 for internal use (to verify token, retrieve oidc config in Security Configuration...) and one to return to the frontend or redirect the frontend to. There are other solutions I could think of but they are much more complicated, change the used authentication flow, or don't work in some edge cases...

What the you all think we should do about this ? @mraible https://github.com/mraible @vishal423 https://github.com/vishal423

— Reply to this email directly, view it on GitHub https://github.com/jhipster/generator-jhipster/pull/18441#issuecomment-1128524089, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAELZDD23Q5SVBOER3IUM3VKNEM7ANCNFSM5UBGLBVQ . You are receiving this because you were mentioned.Message ID: @.***>

yelhouti commented 2 years ago

@mraible I don't think they should, the only reason our code was working is because we where using a "hack". I think its our code that needs fixing... Maybe there is a way to retrieve the IP of the keycloak container and make keycloak resolve to that instead of 127.0.0.1 which would be a correct way of doing this.

yelhouti commented 2 years ago

@mraible 03-system.sh is the one pointing keycloak to 127.0.0.1 we could fix the ip of keycloak an change 03-system.sh. Or we could run a similar script once keycloak has started and the ip is read using docker container inspect | grep ...

mraible commented 2 years ago

I agree the current requirement to edit your hosts file isn't ideal. However, it's only required when running everything with Docker or in CI. I've yet to figure out a better solution but would love to find one!

yelhouti commented 2 years ago

As I mentioned, this can be achieved by specifying a different URL for server to server communication than the one used to redirect the user to... But for now what d you think I should do ?

mshima commented 2 years ago

Making the keycloak host dynamic will be a pain for development.

Add --http-port 9080 to entrypoint. Tried locally seem to work:

17 14:25:33,330 INFO  [io.quarkus] (main) Keycloak 18.0.0 on JVM (powered by Quarkus 2.7.5.Final) started in 13.276s. Listening on: http://0.0.0.0:9080
yelhouti commented 2 years ago

@mshima this is a hack, I would rather configure a network in app.yaml and set the IP in the compose file to a fixed one, and tell people to add it to /etc*hosts. Also this is not needed for development since in development the app is not started using docker-compose so using localhost:9080 works perfectly.

yelhouti commented 2 years ago

Also. the only raison I went against using a network is to avoid conflicts between project. (but I can use it if people prefer it over dynamically setting /etc/hosts. Which can be done using: 

sudo docker container inspect docker-compose_keycloak_1 | jq -r '.[0].NetworkSettings.Networks."docker-compose_default".IPAddress' | echo "`cat -` keycloak" | sudo tee -a /etc/hosts
yelhouti commented 2 years ago

Finally !!! could anyone merge this please ? @mshima @mraible

mshima commented 2 years ago

Also this is not needed for development since in development the app is not started using docker-compose so using localhost:9080 works perfectly.

You are only considering ./mvnw. It's possible to develop frontends:

docker-compose -f src/main/docker/app.yml up
npm start
sudo docker container inspect docker-compose_keycloak_1 | jq -r '.[0].NetworkSettings.Networks."docker-compose_default".IPAddress' | echo "`cat -` keycloak" | sudo tee -a /etc/hosts

I really don't think this is less hacky than the http port change. Works on macOS? What about windows?

Before merging you need to update:

IMO you are trying to fix something that's not broken.

mshima commented 2 years ago

This new behavior will affect microservices/microfrontends. So we should wait for @mraible review.

yelhouti commented 2 years ago

You are only considering ./mvnw. It's possible to develop frontends:

It won't be an issue if you are developing frontends as long as the backend is started without docker. Also for frontends 127.0.0.1 still works thanks to the port redirect

I really don't think this is less hacky than the http port change. Works on macOS? What about windows?

Yes it works on all platforms, you just need to do the equivalent of 127.0.0.1 keycloak by replacing the IP to the keycloak container's IP. And no this not hacky, this is exactly how you talk to your containers from the host.

mshima commented 2 years ago

MacOS:

sudo docker container inspect docker-compose_keycloak_1
[]
Error: No such container: docker-compose_keycloak_1

It's started as docker-compose-keycloak-1

sudo docker container inspect docker-compose_keycloak_1 | jq -r '.[0].NetworkSettings.Networks."docker-compose_default".IPAddress' | echo "`cat -` keycloak" | sudo tee -a /etc/hosts
zsh: command not found: jq
Error: No such container: docker-compose_keycloak_1
yelhouti commented 2 years ago

@mshima here you go:

documentation https://www.jhipster.tech/docker-compose/#7

https://github.com/jhipster/jhipster.github.io/compare/main...yelhouti:patch-3

ionic blueprint https://github.com/jhipster/generator-jhipster-ionic/blob/b9c96386b28c459722732e4c50d2dd08052994f9/.github/workflows/ionic.yml#L107-L110.

https://github.com/jhipster/generator-jhipster-ionic/pull/691

svelte blueprint https://github.com/jhipster/generator-jhipster-svelte/blob/d1a88ebd581e7f346791ea0041d2b598635e9c90/.github/workflows/applications.yml#L418

https://github.com/jhipster/generator-jhipster-svelte/pull/1168

native blueprint https://github.com/jhipster/generator-jhipster-native/blob/054dddd3a097b72062d2555cd01a234cda147ebf/.github/workflows/jdl.yml#L83

https://github.com/jhipster/generator-jhipster-native/pull/50

Check every official blueprint

How do I know the others, also it is a one line change, it could be easily fixed by the blueprint maintainer using a copy paste.

yelhouti commented 2 years ago

It's started as docker-compose-keycloak-1 Seems like macos uses a different naming convention, shouldn't be a problem though right ?

vishal423 commented 2 years ago

I was going over Keycloak migration guide and seems they have some changes on logout endpoint. @yelhouti, can you confirm the application logout works correctly with this PR?

Also, can you confirm no realm configurations update available in this version as well (last time you confirmed for v17, I guess)

yelhouti commented 2 years ago

I was going over Keycloak migration guide and seems they have some changes on logout endpoint. @yelhouti, can you confirm the application logout works correctly with this PR?

No I can't :D thank for pointing this out fixing it. EDIT: Fixed, thank you very much :)

Also, can you confirm no realm configurations update available in this version as well (last time you confirmed for v17, I guess)

Yes I am pretty sure I have checked :)

yelhouti commented 2 years ago

@mraible waiting for your review

mraible commented 2 years ago

I'll give it a try in a few hours. I'm traveling this week in Iceland.

yelhouti commented 2 years ago

@mraible please no rush, this can wait for next week. Enjoy your vacation !

mraible commented 2 years ago

@yelhouti I'm not on vacation. This is work. ;0)

I tested it with jhipster jdl reactive-ms on my M1 since that's all I have with me.

It works when running all the subsystems with Docker and the gateway + microservices with Gradle. However, when you build Docker images and try to run everything with docker compose up, it fails.

It tries to redirect to http://keycloak:8080/, which doesn't exist. Keycloak is running at 9080, but both the JHipster Registry and the gateway app try to redirect to 8080.

yelhouti commented 2 years ago

@mraible You need to change your host to point to the docker ip instead of 127.0.0.1 a commad for that is available in the docs PR

mraible commented 2 years ago

@yelhouti Do you have a link to the docs PR or is it part of this one? If it's part of this one, can you tell me the file to look for changes in?

mshima commented 2 years ago

I would prefer to keep the keycloak static. Having to change the keycloak ip from time to time is bad.

yelhouti commented 2 years ago

@mraible sorry I am on my phone here you go

sudo docker container inspect docker-compose_keycloak_1 | jq -r '.[0].NetworkSettings.Networks."docker-compose_default".IPAddress' | echo "`cat -` keycloak" | sudo tee -a /etc/hosts
mraible commented 2 years ago

IMO, this isn't a great developer experience because I can't memorize it. Nevertheless, I tried it:

sudo docker container inspect docker-compose_keycloak_1 | jq -r '.[0].NetworkSettings.Networks."docker-compose_default".IPAddress' | echo "cat - keycloak" | sudo tee -a /etc/hosts

It results in:

Error: No such container: docker-compose_keycloak_1

If I do a docker ps, it shows:

CONTAINER ID   IMAGE                               COMMAND                  CREATED          STATUS          PORTS                                            NAMES
fd9bfc31307b   blog                                "bash -c /entrypoint…"   15 minutes ago   Up 15 minutes   8081/tcp                                         docker-compose-blog-1
4dd3e779b620   store                               "bash -c /entrypoint…"   15 minutes ago   Up 15 minutes   8082/tcp                                         docker-compose-store-1
bc4acd679f4b   neo4j:4.4.6                         "tini -g -- /startup…"   15 minutes ago   Up 15 minutes   7473-7474/tcp, 7687/tcp                          docker-compose-blog-neo4j-1
b2d167b3ce8a   mongo:4.4.13                        "docker-entrypoint.s…"   15 minutes ago   Up 15 minutes   27017/tcp                                        docker-compose-store-mongodb-1
2ee88ba89092   jhipster/jhipster-registry:v7.3.0   "bash -c /entrypoint…"   15 minutes ago   Up 15 minutes   0.0.0.0:8761->8761/tcp                           docker-compose-jhipster-registry-1
7c4ad6afc410   gateway                             "bash -c /entrypoint…"   15 minutes ago   Up 15 minutes   0.0.0.0:8080->8080/tcp                           docker-compose-gateway-1
7247589108de   quay.io/keycloak/keycloak:18.0.0    "/opt/keycloak/bin/k…"   15 minutes ago   Up 15 minutes   0.0.0.0:9080->8080/tcp, 0.0.0.0:9443->8443/tcp   docker-compose-keycloak-1
58437475eacd   postgres:14.2                       "docker-entrypoint.s…"   15 minutes ago   Up 15 minutes   5432/tcp                                         docker-compose-gateway-postgresql-1
yelhouti commented 2 years ago

@mshima we can do that we would have to use the same network in keycloak.yml and app.yml which not great. We should also be aware of conflicts between multiple projects... This is more flexible. But whatever you guys decide

yelhouti commented 2 years ago

For macOS it seems to be -1 not _1

mraible commented 2 years ago 
$ sudo docker container inspect docker-compose_keycloak-1 | jq -r '.[0].NetworkSettings.Networks."docker-compose_default".IPAddress' | echo "cat - keycloak" | sudo tee -a /etc/hosts
Password:
cat - keycloak
Error: No such container: docker-compose_keycloak-1
vishal423 commented 2 years ago

@mraible, You can check exact name under Names column of docker ps output. From last pasted output, it seems to be docker-compose-keycloak-1

mraible commented 2 years ago

This command does execute:

sudo docker container inspect docker-compose-keycloak-1 | jq -r '.[0].NetworkSettings.Networks."docker-compose_default".IPAddress' | echo "cat - keycloak" | sudo tee -a /etc/hosts

However, if I look in my /etc/hosts/ file, I see:

cat - keycloak
cat - keycloak
cat - keycloak

This doesn't seem to be that easy. ;)

yelhouti commented 2 years ago

It seems like cat - doesn't work the same in MacOS. Just docker run the first part to read the IP and set it manually in the file and it should work :)

mraible commented 2 years ago

@yelhouti Can you please explain why when I run Keycloak for the gateway (with docker-compose -f src/main/docker/keycloak.yml up -d), it can run on 9080. However, when I run with Docker Compose (from the docker-compose folder), it needs to run on 8080?

yelhouti commented 2 years ago

@yelhouti Can you please explain why when I run Keycloak for the gateway (with docker-compose -f src/main/docker/keycloak.yml up -d), it can run on 9080. However, when I run with Docker Compose (from the docker-compose folder), it needs to run on 8080?

@mraible Sure ! I understand why you think that they run in different ports but in reality they don't. Here is what is happening: docker run apps in a private network with it's own port management, dns resolution... In that network the keycloak container has a dns entry keycloak and an open port where the app runs on 8080. All pods inside docker (in the same network, the default one when nothing is specified) can talk to keycloak using the "url" keycloak:8080. However your browser is running directly in the host, not inside docker and therefor can not talk to keycloak, unless the port is exposed. We do that by specifying 127.0.0.1:9080:8080 ie redirect all traffic coming to 127.0.0.1:9080 to this container's (keycloak) 8080 and now keycloak can be reached on 127.0.0.1:9080 .

Why is using keycloak.yml and docker-compose.yml different. When using keycloak.yml everything expect keycloak is running on the host, and keycloak is exposed for all component on 127.0.0.1:9080 So everything work great. However, when using docker-compose.yml everything expect the browser is running inside the private network and they can talk to it using: keycloak:8080. But the browser doesn't know how to resolve keycloak so it fails. To fix the browser, we just need to tell our machine where to resolve keycloak and everything should work great. How we so that is just by changing /etc/hosts <PRIAVTE_KEYCLOAK_IP> keycloak and we can reach it in the private network ip and port 8080. Of course we can still reach it in this case too on 127.0.0.1:9080 if it is exposed, or we can just choose not to expose it in this case (since everything is inside the private network and we told the host/browser how to resolve keycloak in that network).

Finally keycloak:9080 should never work unless you still have the old 127.0.0.1 keycloak in your /etc/hosts file.

mraible commented 2 years ago

Can we make it so docker-compose.yml uses port 9080 like keycloak.yml does?

That way, your way (grab the IP from Docker and add it to the hosts file) and the current way both work.

yelhouti commented 2 years ago

No it's not possible, because the app needs to talk to keycloak directly inside the private network. Going through the host is in theory possible but requires more tweaking than just modifying /etc/hosts like I do

mraible commented 2 years ago

Going through the host is in theory possible but requires more tweaking than just modifying /etc/hosts like I do

I'm just saying the current way is pretty much the same in that it makes the end user modify /etc/hosts. The new way in this PR just makes you calculate it every time instead of making it static.

yelhouti commented 2 years ago

@mraible I can make it static just not 127.0.0.1 let me do that

yelhouti commented 2 years ago

@mraible I fixed it to 172.31.0.200 and updated the docs accordingly https://github.com/jhipster/jhipster.github.io/compare/main...yelhouti:patch-3

I ll fix PRs on other repos once this is merged to the tests to pass

mraible commented 2 years ago

I tried adding this IP to my /etc/hosts file. Then, I tried to run the following from the gateway:

docker-compose -f src/main/docker/keycloak.yml up -d

It fails with:

Error response from daemon: Invalid address 172.31.0.200: It does not belong to any of this network's subnets

FWIW, I'm on an airplane right now and my IP is 10.178.94.218.

yelhouti commented 2 years ago

@mraible It this you issue is related to how networks in docker work/cached. can please try to docker-compose down before up ?