DevServices for Keycloak should be able to discover the container started by KeycloakTestResourceLifecycleManager

sberyozkin commented 3 years ago

Description

KeycloakTestResourceLifecycleManager starts a Keycloak container but it is not discovered when Dev Services for Keycloak launches its own container.

Implementation ideas

Update Dev Services for Keycloak to discover the container running with a matching label and KeycloakTestResourceLifecycleManager to register the container it creates with a label

CC @sebastienblanc

quarkus-bot[bot] commented 3 years ago

/cc @pedroigor, @stuartwdouglas

sberyozkin commented 3 years ago

I've edited it too early but I won't re-editing again. It can work both ways:

if I run a dev mode test in main (ex, as part of quarkus-oidc/deployment tests) then it is KeycloakTestResourceLifecycleManager which is launched first, followed by DevServices for Keycloak.
if I start a quickstart with quarkus:dev and press r to run the tests I have DevServices for Keycloak starting first followed by KeycloakTestResourceLifecycleManager

stuartwdouglas commented 3 years ago

I don't think we should do this, why would you want to use KeycloakTestResourceLifecycleManager when you already have DevServices, you should just use DevServices directly, and KeycloakTestResourceLifecycleManager should likely be deprecated.

In addition you don't really want your tests connecting to the same service as dev mode, as they may have different config.

sberyozkin commented 3 years ago

@stuartwdouglas

I don't think we should do this, why would you want to use KeycloakTestResourceLifecycleManager when you already have DevServices, you should just use DevServices directly, and KeycloakTestResourceLifecycleManager should likely be deprecated. In addition you don't really want your tests connecting to the same service as dev mode, as they may have different config.

Have you thought you may not be right in your assumption ? Why have you closed without even allowing for the discussion to flow ?

sberyozkin commented 3 years ago

@stuartwdouglas

KeycloakTestResourceLifecycleManager should likely be deprecated.

It is meant to support running the normal tests against a live Keycloak instance, when no quarkus:dev is used. So when a user has invested in this feature it is impossible to optimally combine it with DevServices being up as @sebastienblanc has discovered. After quarkus:dev and pressing r 3 containers are up, dev mode, test mode (r related) and the one launched by KeycloakTestResourceLifecycleManager.

Yes, I can start writing the tests without KeycloakTestResourceLifecycleManager when I start with quarkus;dev.

First problem here is how to acquire the test tokens in the test code - KeycloakTestResourceLifecycleManager knows how to do it (in my draft PR). OK, now I've somehow written these tests. But what is next for these tests without quarkus:dev ?

I've just tried simply doing mvn clean install in quarkus-quickstarts/security-openid-connect-quickstart but first I removed the existing test factory there starting its own container and no test container is started and the tests fail.

What am I missing ?

Thanks

stuartwdouglas commented 3 years ago

@stuartwdouglas

KeycloakTestResourceLifecycleManager should likely be deprecated.

It is meant to support running the normal tests against a live Keycloak instance, when no quarkus:dev is used. So when a user has invested in this feature it is impossible to optimally combine it with DevServices being up as @sebastienblanc has discovered. After quarkus:dev and pressing r 3 containers are up, dev mode, test mode (r related) and the one launched by KeycloakTestResourceLifecycleManager.

DevServices does not require quarkus:dev, it also works for running normal tests.

Yes, I can start writing the tests without KeycloakTestResourceLifecycleManager when I start with quarkus;dev.

First problem here is how to acquire the test tokens in the test code - KeycloakTestResourceLifecycleManager knows how to do it (in my draft PR). OK, now I've somehow written these tests. But what is next for these tests without quarkus:dev ?

I've just tried simply doing mvn clean install in quarkus-quickstarts/security-openid-connect-quickstart but first I removed the existing test factory there starting its own container and no test container is started and the tests fail.

https://github.com/quarkusio/quarkus-quickstarts/pull/925 shows how to use DevServices in the test.

What am I missing ?

Thanks

sberyozkin commented 3 years ago

@stuartwdouglas Thanks for that PR, I thought that did not work in the native mode (injecting in the test code) but may be it has changed now. So does that mean that all tests in main run for the moment with 2 containers - most of them have test factories launching KC (and many tests are much more involved in how they set up Keycloak - compared to how DevServices does it - and in some cases it is not even possible to capture things in the custom json) I'll have a look tomorrow, too tired now. Cheers !

stuartwdouglas commented 3 years ago

Good, point, that won't work in native.

Every other DevService does not really need to be accessed from the test code, so it is not really an issue, but it is for keycloak. We really need some way to inject these properties in native mode. I will come up with something.

stuartwdouglas commented 3 years ago

I have updated the PR to work in native, although its not super nice. We should provide a nice utility for the keycloak tests to easily get the token.

sberyozkin commented 3 years ago

Good evening @stuartwdouglas, OK, the new day starts and things are seen differently for me. Yesterday evening I logged in as I was about to open a PR and was looking for the issue number so got a bit of a surprise seeing it no longer existed and typed a lot of nonsense feeling sorry for myself as I never miss an opportunity to embarrass myself :-) - but it is really a poor understanding on my part of what %prod is was the reason I spent a good part of the day yesterday on pursuing some new solution - I simply had no idea I could use this mechanism for having Dev Services activating the container for the tests because %prod%quarkus.oidc.auth-server-url would only be configured when starting the application in the normal/prod mode. But now I know, so trying to create that different solution was worth it just for it.

I'll work on the docs.

Your PR is nice, not a problem for the moment in native mode one needs to access the devservices property. I guess I can try to have it wired in a test resource factory. Also note, for web-app Keycloak applications the HtmlUnit tests would work totally fine, I'll follow up with another PR to quickstarts to show it. The problem is only for testing the services acquiring the tokens.

As far as KeycloakTestResourceLifecycleManager is concerned - right now I'm not 100% sure it should be gone (starting from deprecating it now). When it is used as doc-ed Dev Services are disabled (auth-server-url is configured), and the users can have custom factories starting KC the way it may not be possible to do with Dev Services. Ideally such factories would coordinate with Dev Services (for ex as suggested in the draft PR - the token acquisition works in native) but I'm not worried about not having it working now.

I'll update that PR to get it focused on the label discovery only. thanks

quarkusio / quarkus

DevServices for Keycloak should be able to discover the container started by KeycloakTestResourceLifecycleManager #19608

Description

Implementation ideas