dasniko / testcontainers-keycloak

A Testcontainer implementation for Keycloak IAM & SSO.
Apache License 2.0
328 stars 51 forks source link

Continuous testing issue #64

Closed gaetancollaud closed 2 years ago

gaetancollaud commented 2 years ago

Hi,

Nice project. I'm using it with Quarkus and it works great except for continuous testing. Not sure what Quarkus is doing behind the scene with the classpath, but this line causes issues: https://github.com/dasniko/testcontainers-keycloak/blob/main/src/main/java/dasniko/testcontainers/keycloak/KeycloakContainer.java#L185

I replaced it with:

new ObjectMapper().readValue(Thread.currentThread().getContextClassLoader().getResource(importFile), RealmRepresentation.class)

And it seems to work, but then I have another issue. I work on the first execution, but starting from the second one I got something really weird:

Caused by: java.lang.RuntimeException: Unable to start Quarkus test resource ch.koalasense.resource.KeycloakResource@4e20583b
    at io.quarkus.test.common.TestResourceManager$TestResourceEntryRunnable.run(TestResourceManager.java:457)
    at java.base/java.util.concurrent.CompletableFuture$AsyncRun.run(CompletableFuture.java:1804)
    ... 3 more
Caused by: org.testcontainers.containers.ContainerLaunchException: Container startup failed
    at org.testcontainers.containers.GenericContainer.doStart(GenericContainer.java:336)
    at org.testcontainers.containers.GenericContainer.start(GenericContainer.java:317)
    at ch.koalasense.resource.KeycloakResource.start(KeycloakResource.java:21)
    at io.quarkus.test.common.TestResourceManager$TestResourceEntryRunnable.run(TestResourceManager.java:452)
    ... 4 more
Caused by: org.rnorth.ducttape.RetryCountExceededException: Retry limit hit with exception
    at org.rnorth.ducttape.unreliables.Unreliables.retryUntilSuccess(Unreliables.java:88)
    at org.testcontainers.containers.GenericContainer.doStart(GenericContainer.java:329)
    ... 7 more
Caused by: org.testcontainers.containers.ContainerLaunchException: Could not create/start container
    at org.testcontainers.containers.GenericContainer.tryStart(GenericContainer.java:525)
    at org.testcontainers.containers.GenericContainer.lambda$doStart$0(GenericContainer.java:331)
    at org.rnorth.ducttape.unreliables.Unreliables.retryUntilSuccess(Unreliables.java:81)
    ... 8 more
Caused by: java.lang.RuntimeException: RESTEASY003940: Unable to instantiate MessageBodyReader
    at org.jboss.resteasy.core.providerfactory.CommonProviders.processProviderContracts(CommonProviders.java:93)
    at org.jboss.resteasy.core.providerfactory.ClientHelper.processProviderContracts(ClientHelper.java:104)
    at org.jboss.resteasy.core.providerfactory.ResteasyProviderFactoryImpl.processProviderContracts(ResteasyProviderFactoryImpl.java:841)
    at org.jboss.resteasy.core.providerfactory.ResteasyProviderFactoryImpl.registerProvider(ResteasyProviderFactoryImpl.java:829)
    at org.jboss.resteasy.core.providerfactory.ResteasyProviderFactoryImpl.register(ResteasyProviderFactoryImpl.java:1506)
    at org.jboss.resteasy.core.providerfactory.ResteasyProviderFactoryImpl.register(ResteasyProviderFactoryImpl.java:105)
    at org.jboss.resteasy.client.jaxrs.internal.ResteasyClientBuilderImpl.register(ResteasyClientBuilderImpl.java:479)
    at org.jboss.resteasy.client.jaxrs.internal.ResteasyClientBuilderImpl.register(ResteasyClientBuilderImpl.java:47)
    at org.keycloak.admin.client.Keycloak.newRestEasyClient(Keycloak.java:71)
    at org.keycloak.admin.client.Keycloak.getInstance(Keycloak.java:78)
    at org.keycloak.admin.client.Keycloak.getInstance(Keycloak.java:94)
    at dasniko.testcontainers.keycloak.KeycloakContainer.getKeycloakAdminClient(KeycloakContainer.java:323)
    at dasniko.testcontainers.keycloak.KeycloakContainer.containerIsStarted(KeycloakContainer.java:180)
    at org.testcontainers.containers.GenericContainer.containerIsStarted(GenericContainer.java:688)
    at org.testcontainers.containers.GenericContainer.tryStart(GenericContainer.java:504)
    ... 10 more
Caused by: java.lang.NullPointerException: Cannot invoke "io.quarkus.arc.ArcContainer.instanceSupplier(java.lang.Class, java.lang.annotation.Annotation[])" because the return value of "io.quarkus.arc.Arc.container()" is null
    at io.quarkus.resteasy.common.runtime.QuarkusConstructorInjector.construct(QuarkusConstructorInjector.java:35)
    at org.jboss.resteasy.core.providerfactory.Utils.createProviderInstance(Utils.java:102)
    at org.jboss.resteasy.core.providerfactory.CommonProviders.processProviderContracts(CommonProviders.java:87)
    ... 24 more

It seems like the Arc container is null and not initialized for some reason.

I didn't see any mention of continuous testing in this repo nor in your presentation video. I'm wondering if you were able to make it work ?

dasniko commented 2 years ago

Thanks for reporting. Unfortunately, I don't have any experience yet with Quarkus continious testing, hence I don't know what's going on there and why the container startup fails. So if you or anybody else can help here, I'd appreciate your work!

gaetancollaud commented 2 years ago

I will try to see if someone can help me on the Quarkus Zulip chat.

In the meantime, would you be open to a pull request that uses:

Thread.currentThread().getContextClassLoader().getResource(importFile)

instead of

this.getClass().getResource(importFile)

?

This would already resolve the first part of the problem. I can work on it this week.

dasniko commented 2 years ago

As long as this change doesn't break other behavior, I'm ok with it. Just create the PR and I'll check it.

And for continous testing support in Quarkus: Can you provide a minimum working (or here: failing) example, on which we can reproduce the behavior and check which changes have which impacts? Maybe as a small repo and some guidance, how to enforce failure behavior. I can't promise, but I'll try to have a look into it.

dasniko commented 2 years ago

@gaetancollaud Any progress so far? (no hurry, just asking)

gaetancollaud commented 2 years ago

@gaetancollaud Any progress so far? (no hurry, just asking)

Sorry I was busy with other projects. I will put a reminder to myself to work on it tonight or tomorrow night

gaetancollaud commented 2 years ago

@dasniko Here is a reproducer: https://github.com/gaetancollaud/reproducer-keycloak-testcontainer

gaetancollaud commented 2 years ago

If you checkout my pr #68, do a mvn clean install and then use the version 2.1.2-SNAPSHOT in the reproducer you will be able to reproduce the stacktrace above.

If you're ok with #68 please merge it and do a release. I will then adapt the reproducer and create an issue at https://github.com/quarkusio/quarkus (if you don't think it's related to this repository).

dasniko commented 2 years ago

I think this issue will - at least partly - also dissappear, when the auto-import is back again in Keycloak. The exception is happening directly after container startup, when the Testcontainer tries to import the realm. This is currently achieved using the Keycloak admin client, this client uses a RestEasy client.

When executing a test in Quarkus, Quarkus seems to close something RestEasy client related, so that the container is not able to instantiate a new admin client. Don't know why this happens exactly and if/how to get around this.

However, I found a - partly - working scenario. Parly for the testcontainer... So, create the testcontainer with .withReuse(true) and additonally don't stop the container in the stop() method. This will yield in having a running container still after the tests have finished. The next time the test runs, it can reuse the already existing and running container. As the container is reused, it won't try to import the realm again.

So far, so good... But now the creation of the admin client in your KeycloakResource is failing with the similar exception. So, good for me, it's not my code ;) Just joking...

In your concrete scenario, you could simply add the client secret into the realm.json file and don't create it programmatically. Thus, you don't need to create the admin client to access the container. You can simply put the client-id and client-secret from the realm.json into your test application.yml and should be good to go. Your KeycloakResource simply has to return a map with one entry only, the url. With these changes I can re-run the continous tests in your reproducer, but the test is failing anyway... ;)

gaetancollaud commented 2 years ago

When executing a test in Quarkus, Quarkus seems to close something RestEasy client related, so that the container is not able to instantiate a new admin client. Don't know why this happens exactly and if/how to get around this.

It's true that we can see this in the console:

2022-03-24 20:07:31,724 WARN  [org.jbo.res.cli.jax.i18n] (Finalizer) RESTEASY004687: Closing a class org.jboss.resteasy.client.jaxrs.engines.ApacheHttpClient43Engine instance for you. Please close clients yourself.
2022-03-24 20:07:31,937 WARN  [org.jbo.res.cli.jax.i18n] (Finalizer) RESTEASY004687: Closing a class org.jboss.resteasy.client.jaxrs.engines.ApacheHttpClient43Engine instance for you. Please close clients yourself.

I will investigate

gaetancollaud commented 2 years ago

ok, so one of the client comes from the test container. I created #69 to close it properly. Now I need a way to properly close the client in my reproducer. But apparently, beans that are AutoCloseable are not closed. :disappointed: Which is disappointing...

I will continue to investigate this more.

gaetancollaud commented 2 years ago

@dasniko could you have a look at #69 so I can upgrade my reproducer and progress on this ?

dasniko commented 2 years ago

Have it merged, you can use it with Jitpack and a snapshot of the main-branch. I'd rather wait for KC18 to publish a new release.

dasniko commented 2 years ago

With KC 18 and the default realm-import feature, the custom admin-client code was removed. It's currently in the main branch, not yet released. Release will follow the next few days.