search

dasniko / testcontainers-keycloak

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

issue #17 - allow jboss user to delete the deployment marker file #18

Closed lmsurpre closed 3 years ago

lmsurpre commented 3 years ago

By mounting its parent directory instead of just the specific file, then marking it writeable for others.

I also upgraded it to Keycloak 12.0.4, but I can easilly revert that if you want.

thomasdarimont commented 3 years ago

Hello @lmsurpre

thanks for your PR. I just gave it a quick try and noticed that this fails on Fedora with Podman. If I set the fileSystemBind to the .deploy file explicitly, it works on plain Docker and podman.

withFileSystemBind(deploymentTriggerFile.getAbsolutePath(), deploymentLocation + "/" + deploymentTriggerFileName, BindMode.READ_WRITE);

Could you give it a try with:

    private void createDeploymentTriggerFileForWildfly(String deploymentLocation, String uniqueExtensionNameForExtensionClassFolder) {

        String deploymentTriggerFileName = uniqueExtensionNameForExtensionClassFolder + ".dodeploy";
        try {
            File tmpdir = Files.createTempDirectory("kc-tc-deploy").toFile();

            // Refactor once test-containers support mounting a string as file
            // see https://github.com/testcontainers/testcontainers-java/issues/3814
            File deploymentTriggerFile = Files.createFile(tmpdir.toPath().resolve(deploymentTriggerFileName)).toFile();
            // make the file writeable by anyone, so that the non-root jboss user can remove it
            boolean ignored = deploymentTriggerFile.setWritable(true, false);
            deploymentTriggerFile.deleteOnExit();
            Files.write(deploymentTriggerFile.toPath(), "true".getBytes(StandardCharsets.UTF_8));

            withFileSystemBind(deploymentTriggerFile.getAbsolutePath(), deploymentLocation + "/" + deploymentTriggerFileName, BindMode.READ_WRITE);
        } catch (IOException e) {
            throw new RuntimeException("Could not create extensions deployment trigger file", e);
        }
    }
thomasdarimont commented 3 years ago

@lmsurpre I also think it makes sense keep the keycloak version upgrade in a separate commit.

lmsurpre commented 3 years ago

Could you give it a try with:

I tried it, but that doesn't seem to work with docker on mac osx. I still see the same behavior reported in the issue. It cannot delete a file that is a direct mount point.

15:15:52,647 WARN [org.jboss.as.server.deployment.scanner] (DeploymentScanner-threads - 2) WFLYDS0002: Cannot remove extraneous deployment marker file /opt/jboss/keycloak/standalone/deployments/-495401560-extensions.jar.dodeploy

lmsurpre commented 3 years ago

I just gave it a quick try and noticed that this fails on Fedora with Podman.

Mind sharing the error?

lmsurpre commented 3 years ago

New idea: lets just add the *.dodeploy file to the extensionClassFolder that is already bindMounted at this location. I'll update the PR to use this approach and maybe you could confirm it works on Fedora with Podman?

thomasdarimont commented 3 years ago

The line 

withFileSystemBind(tmpdir.getAbsolutePath(), deploymentLocation, BindMode.READ_WRITE);

produces the following mounts:

        "Mounts": [
            {
                "Type": "bind",
                "Source": "/home/tom/dev/repos/gh/thomasdarimont/keycloak-dev/testcontainers-keycloak/target/test-classes",
                "Destination": "/opt/jboss/keycloak/standalone/deployments/-1944174167-extensions.jar",
                "Mode": "rw,Z",
                "RW": true,
                "Propagation": "rprivate"
            },
            {
                "Type": "bind",
                "Source": "/tmp/kc-tc-deploy15891177025880316696",
                "Destination": "/opt/jboss/keycloak/standalone/deployments",
                "Mode": "rw",
                "RW": true,
                "Propagation": "rprivate"
            }
        ],

and results in the following exception during the test run:

Tests run: 2, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 29.039 sec <<< FAILURE!
shouldDeployExtension(dasniko.testcontainers.keycloak.KeycloakContainerExtensionTest)  Time elapsed: 16.277 sec  <<< FAILURE!
java.lang.AssertionError: 
Expected: not null
     but: was null
    at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
    at org.junit.Assert.assertThat(Assert.java:956)
    at org.junit.Assert.assertThat(Assert.java:923)
    at dasniko.testcontainers.keycloak.KeycloakContainerExtensionTest.shouldDeployExtension(KeycloakContainerExtensionTest.java:67)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:566)
    at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
    at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
    at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
    at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
    at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
    at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
    at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
    at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
    at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
    at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
    at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
    at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
    at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:252)
    at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:141)
    at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:112)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:566)
    at org.apache.maven.surefire.util.ReflectionUtils.invokeMethodWithArray(ReflectionUtils.java:189)
    at org.apache.maven.surefire.booter.ProviderFactory$ProviderProxy.invoke(ProviderFactory.java:165)
    at org.apache.maven.surefire.booter.ProviderFactory.invokeProvider(ProviderFactory.java:85)
    at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:115)
    at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:75)

This indicates that the extensions were not deployed. It seems that Keycloak cannot read the deployments folder:

bash-4.4$ cd standalone/deployments/
bash-4.4$ ls
ls: cannot open directory '.': Permission denied

If I look into the container as root user, I see this:

[root@5a83b4c0a881 standalone]# ls -al
total 52
drwxrwx--x. 1 jboss root 4096 Mar  1 08:20 .
drwxrwxr-x. 1 jboss root 4096 Mar  1 06:59 ..
drwxrwxr-x. 1 jboss root 4096 Apr 20 15:49 configuration
drwxrwxr-x. 1 jboss root 4096 Apr 20 15:49 data
drwx------. 3 jboss 1000   80 Apr 20 15:49 deployments
drwxrwxr-x. 3 jboss root 4096 Mar  1 06:59 lib
drwxrwxr-x. 1 jboss root 4096 Apr 20 15:49 log
drwxrwx--x. 1 jboss root 4096 Apr 20 15:49 tmp

If I use the line:

withFileSystemBind(deploymentTriggerFile.getAbsolutePath(), deploymentLocation + "/" + deploymentTriggerFileName, BindMode.READ_WRITE);

I get the following mounts:

        "Mounts": [
            {
                "Type": "bind",
                "Source": "/home/tom/dev/repos/gh/thomasdarimont/keycloak-dev/testcontainers-keycloak/target/test-classes",
                "Destination": "/opt/jboss/keycloak/standalone/deployments/-1944174167-extensions.jar",
                "Mode": "rw,Z",
                "RW": true,
                "Propagation": "rprivate"
            },
            {
                "Type": "bind",
                "Source": "/tmp/kc-tc-deploy14363211496663638826/-1944174167-extensions.jar.dodeploy",
                "Destination": "/opt/jboss/keycloak/standalone/deployments/-1944174167-extensions.jar.dodeploy",
                "Mode": "rw",
                "RW": true,
                "Propagation": "rprivate"
            }
        ],

Within the docker container it looks like this:

bash-4.4$ pwd
/opt/jboss/keycloak/standalone/deployments
bash-4.4$ ls -Afl
total 40
drwxrwxr-x. 1 jboss root 4096 Apr 20 15:56 .
-rw-rw-r--. 1 jboss root 8888 Mar  1 06:59 README.txt
drwxrwx--x. 1 jboss root 4096 Mar  1 08:20 ..
drwxr-xr-x. 4 jboss 1000 4096 Apr 20 15:55 -1944174167-extensions.jar
-rw-r--r--. 1 jboss root   26 Apr 20 15:55 -1944174167-extensions.jar.deployed
-rw-rw-rw-. 1 jboss 1000    4 Apr 20 15:55 -1944174167-extensions.jar.dodeploy

This is on fedora linux release 33

$ docker version
Client:
 Version:           19.03.13
 API version:       1.40
 Go version:        go1.15.1
 Git commit:        4484c46
 Built:             Fri Oct  2 19:31:30 2020
 OS/Arch:           linux/amd64
 Experimental:      false

Server:
 Engine:
  Version:          19.03.13
  API version:      1.40 (minimum version 1.12)
  Go version:       go1.15.1
  Git commit:       4484c46
  Built:            Fri Oct  2 00:00:00 2020
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          1.4.4
  GitCommit:        05f951a3781f4f2c1911b05e61c160e9c30eaa8e
 runc:
  Version:          1.0.0-rc93
  GitCommit:        12644e614e25b05da6fd08a38ffa0cfe1903fdec
 docker-init:
  Version:          0.18.0
  GitCommit:
lmsurpre commented 3 years ago

thanks @thomasdarimont I tried this other approach and I thought it was wroking but its not...I didn't realize the classes mount was one level too low. any ideas for something that will work on both our systems? :-)

thomasdarimont commented 3 years ago

Thanks for trying :) I'm looking into it...

thomasdarimont commented 3 years ago

@lmsurpre could you try this:

Files.write(deploymentTriggerFile.toPath(), "true".getBytes(StandardCharsets.UTF_8));
Files.setPosixFilePermissions(deploymentTriggerFile.toPath(), PosixFilePermissions.fromString("rw-rw-rw-"));
withFileSystemBind(deploymentTriggerFile.getAbsolutePath(), deploymentLocation + "/" + deploymentTriggerFileName, BindMode.READ_WRITE);
thomasdarimont commented 3 years ago

@lmsurpre I have now a mac to tests things on, could you create a minimal reproducer test case for this issue?

lmsurpre commented 3 years ago

Same result. Its not a file permission thing. Its a bind mount thing.

bash-4.4$ ls -l
total 20
drwxr-xr-x 5 root  root  160 Apr 20 15:47 -495401560-extensions.jar
-rw-r--r-- 1 jboss root   25 Apr 20 15:47 -495401560-extensions.jar.deployed
-rw-rw-rw- 1 root  root    4 Apr 20 16:14 -495401560-extensions.jar.dodeploy
-rw-rw-r-- 1 jboss root 8888 Dec 18 09:23 README.txt

bash-4.4$ rm ./-495401560-extensions.jar.dodeploy
rm: cannot remove './-495401560-extensions.jar.dodeploy': Device or resource busy

What if we just use withCopyFileToContainer() instead of setting up a mount just for this one marker file?

thomasdarimont commented 3 years ago

What if we just use withCopyFileToContainer() This can only be used after the container was created and is running.

lmsurpre commented 3 years ago

I just tested it and this is working fine for me.

lmsurpre commented 3 years ago

This can only be used after the container was created and is running.

It runs after all these other withCopyFileToContainer invocations, so I think it should be fine. image

thomasdarimont commented 3 years ago

I just gave this a try on OSX and it didn't work.

thomasdarimont commented 3 years ago

@lmsurpre could you give this a spin: https://github.com/thomasdarimont/testcontainers-keycloak/tree/issue-17-test

lmsurpre commented 3 years ago

Ugh, I accidentally broke it while cleaning it up (I didn't need the second argument to deploymentTriggerFiles any more). I amended my commit and repushed.

Your branch also looks good to me...will confirm it works (but it should since its basically the same approach).

thomasdarimont commented 3 years ago

I think I found a reliable and robust approach. @lmsurpre could you take another look at my branch and see if that works for you? https://github.com/thomasdarimont/testcontainers-keycloak/tree/issue-17-test

thomasdarimont commented 3 years ago

@lmsurpre I polishied the fix a bit and created a new PR with a proper fix and a test for container reuse support: https://github.com/dasniko/testcontainers-keycloak/pull/19

I also moved the version upgrade into a dedicated PR: https://github.com/dasniko/testcontainers-keycloak/pull/21

Does this work for you?

lmsurpre commented 3 years ago

I confirmed that your branch is working for me as well.

The one thing I noticed, and I can't really pin-point it, is I don't seem to get this nice useful message any more:

INFO org.testcontainers.containers.wait.strategy.HttpWaitStrategy - /frosty_shannon: Waiting for 120 seconds for URL: http://localhost:55166/auth

Was that removed on purpose or just collateral damage from the changes?

thomasdarimont commented 3 years ago

@lmsurpre good catch! It seems that waiting for the extension deployment replaced the HttpWaitStrategy. I just updated the PR to combine existing WaitStrategies if present.

thomasdarimont commented 3 years ago

btw. thanks for the good virtual pairing session :) That helped a lot!

dasniko commented 3 years ago

Closing this PR in favor of #19