dasniko / testcontainers-keycloak

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

Add option to add 3rd party libraries to classpath #58

Closed dasniko closed 2 years ago

dasniko commented 2 years ago

When testing extensions with .withExtensionClassesFrom() it might become necessary to add 3rd party libs as dependencies to the classpath, as the extensions rely on these libraries. Currently that's not possible.

A method like .withExtensionLibsFrom() might be a good solution.

(this issue is deviated from discussion #57)

piantino commented 2 years ago

Hi @dasniko,

Can I take this issue? I need this right now e I would like to contribute to your project too.

dasniko commented 2 years ago

Hi @piantino, if you like, you can send a PR for this, I didn't start yet.

But I think we should name it .withProviderLibsFrom() as the method for custom provider code is called .withProviderClassesFrom().

Looking forward for your suggestion.

dasniko commented 2 years ago

WDYT about something like .withProviderLibsFromMavenRepository("groupId:artifactId:version") (or .withMavenDependency(...))? We can discuss about the naming, I just wanted to proof the idea of referencing libs/dependencies from (local) maven repo an copying these into the containers provider directory.

piantino commented 2 years ago

For my necessity, the absolute path is enough.

Bellow a sample after my change (is working):

keycloak = new KeycloakContainer();
keycloak.withProviderLibsFrom("./target/my-provider-jar-with-dependencies.jar");

I will research how to look for local maven dependencies, but if the project was not a maven?

dasniko commented 2 years ago

I will research how to look for local maven dependencies, but if the project was not a maven?

We should start with a Maven version. It's still the most used dependency management tool. If it works and there are people needing other approaches, I still can think about it.

Having a way to include deps from a repo is way more flexible than just using paths. When I have a dependency from a maven repo in my project, I dont't want to copy the lib into my project, just for the test cases. Additionally, the path on my machine may be different (if it's not part of the project directory) to other machines.

dasniko commented 2 years ago

This might be a friend (and I already use other ShrinkWrap libs in the project): https://github.com/shrinkwrap/resolver

dasniko commented 2 years ago

The more I think about this, the more I would go back to the first approach to just add a list of files/paths to the /providers folder in the container. Everything else seems to re-implement another opinionated API (Shrinkwrap resolver). As already mentioned in #60, I'd do the Maven deps part with an extra documentation (e.g. wiki page).

This approach would be flexible enough for any usecase. If one needs some local libs, it's possible, same with Maven deps resolved via another way.

Let's use a method with a signature like this: .withProviderLibs(List<File> files) Or would List<Path> be better as attribute?

Can you please update your PR #60 accordingly? Sorry for the overhead with Shrinkwrap resolver. You can skip the part with the docs/readme and leave it untouched (resp. revert your current changes), I'll add an appropriate documentation afterwards. Thanks.

piantino commented 2 years ago

I agree that accepting files is a better approach because this library will not simplify the use of containers, wherein most cases are just copying files from the host to the image/container. I didn't know Shrinkwrap, but is very simple to use, so, I agree that providing samples and documentation is very nice. Also, this has support to Gradle, may be other in the future. Finally, is easier to implement and test if the file was copied. Take the sample of a custom template, today this library doesn't support it, but most simply are just copied to the container and calls the build action, Why complicate that!

benbeu commented 2 years ago

Thanks for this amazing feature! We also just encountered this problem and would be thankful for a release. Could you tell me when are you planning to release this in 2.0.0? Are you also planning to backport this for older versions? If not, may I contribute? Thanks in advance!

dasniko commented 2 years ago

@benbeu This will be released with 2.1.0 hopefully soon. I won't implement this in older versions, as older versions would mean v1.x which is Wildfly based and deprecated, so I won't publish new features for this branch. See this as a chance to upgrade your Keycloak environment to the new Quarkus based version. You'll have to do it anyhow latest in a few months.

benbeu commented 2 years ago

Thanks for the quick response! You are probably right and we will discuss this. Have a great day!