-
Notifications
You must be signed in to change notification settings - Fork 302
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
fix(keycloak): add realm imports #565
fix(keycloak): add realm imports #565
Conversation
if you can make it backwards compatible (ie not break tests) then i dont see why we wouldnt merge |
My bad! I have fixed a bug I introduced, that prevented to wait for the first user to be added. Additionally I made the cmd configurable, which I think make sense, depending on the use caes. However, I cannot get the tests running locally, neither on this branch nor on main, so I have a hard time verify whether it works consitently now. |
so it seems straightforward enough, so lgtm! also idk if you accounted for this but the command can be a list or a string. since backwards (or anything beyond basic) compatibility is really not a top concern for community modules, im going to merge this, but feel free to follow up before we do a release. not sure how soon that will be since im pretty backlogged and i dont think we have anything to release that is super critical (happy to be corrected about this). |
Backward compatibility will not be an issue, as there was no way to set the docker command in the previous version - it was automatically overwritten to _DEFAULT_DEV_COMMAND by the start() function. So I guess it's good enough for now. Thanks a lot for the thourough review and time! |
🤖 I have created a release *beep* *boop* --- ## [4.4.1](testcontainers-v4.4.0...testcontainers-v4.4.1) (2024-05-14) ### Bug Fixes * Add memcached container ([#322](#322)) ([690b9b4](690b9b4)) * Add selenium video support [#6](#6) ([#364](#364)) ([3c8006c](3c8006c)) * **core:** add empty _configure to DockerContainer ([#556](#556)) ([08916c8](08916c8)) * **core:** remove version from compose tests ([#571](#571)) ([38946d4](38946d4)) * **keycloak:** add realm imports ([#565](#565)) ([f761b98](f761b98)) * **mysql:** Add seed support in MySQL ([#552](#552)) ([396079a](396079a)) * url quote passwords ([#549](#549)) ([6c5d227](6c5d227)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
With the option to import keycloak realms, introduced with #565, the[_configure()](https://github.com/testcontainers/testcontainers-python/blob/78b6f0ecb15e8cba687eb4588c5ce19ca32208bc/modules/keycloak/testcontainers/keycloak/__init__.py#L57) method is called twice. Once it is called in the [start()](https://github.com/testcontainers/testcontainers-python/blob/78b6f0ecb15e8cba687eb4588c5ce19ca32208bc/modules/keycloak/testcontainers/keycloak/__init__.py#L83) method of keycloak itself and then it is called a second time in the [start()](https://github.com/testcontainers/testcontainers-python/blob/78b6f0ecb15e8cba687eb4588c5ce19ca32208bc/core/testcontainers/core/container.py#L90) method of DockerContainer. This wasn't an issue so far. But if a realm shall be imported (self.has_realm_import in keycloak is True), then every time the string " --import-realm" is added to the start command in the _configure() method. The keycloak container won't start if "--import-realm" is specified multiple times. This is probably the easiest solution to solve the issue. If wished, I can also work on a more robust solution, e.g. by storing the start command in a list and checking that "--import-realm" is only added once. Co-authored-by: Sebastian Scholz <sebastian.scholz@cemocom.de>
This PR adds an option to start a keycloak container with importing one or more realms. This mirrors a feature present in the java keycloak testcontainer.