Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-36088

Use NIO rather than JNR whenever possible

    XMLWordPrintable

    Details

    • Similar Issues:

      Description

      A user reports that Pipeline stash does not preserve the executable bit on 64-bit AIX, apparently due to a missing JNR port. Probably the same applies to any use of TarArchiver.

      Since we now assume Java 7, we can use java.nio.file calls. In particular, making IOUtils.mode and FilePath._chmod use PosixFilePermission rather than JNR would be appropriate.

        Attachments

          Issue Links

            Activity

            Hide
            oleg_nenashev Oleg Nenashev added a comment -

            The fix has been integrated towards 2.93. I doubt it is backportable, but feel free to add the label if you feel it's reasonable

            Show
            oleg_nenashev Oleg Nenashev added a comment - The fix has been integrated towards 2.93. I doubt it is backportable, but feel free to add the label if you feel it's reasonable
            Hide
            typz Francois Ferrand added a comment -

            This actually breaks the docker-commons plugin: when it tries to 'materialize' the credentials, it relies on a chmod() call to create a folder (seems this is the behavior of native API), which now leads to an exception:

            15:38:55 java.nio.file.NoSuchFileException: /var/lib/jenkins/.docker/7d36d1a6-4917-4dc8-97b6-2babbb2ae3d7
            15:38:55 at sun.nio.fs.UnixException.translateToIOException(UnixException.java:86)
            15:38:55 at sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:102)
            15:38:55 at sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:107)
            15:38:55 at sun.nio.fs.UnixFileAttributeViews$Posix.setMode(UnixFileAttributeViews.java:238)
            15:38:55 at sun.nio.fs.UnixFileAttributeViews$Posix.setPermissions(UnixFileAttributeViews.java:260) 15:38:55 at java.nio.file.Files.setPosixFilePermissions(Files.java:2045)
            15:38:55 at hudson.FilePath._chmod(FilePath.java:1618) 15:38:55 at hudson.FilePath.access$1500(FilePath.java:197)
            15:38:55 at hudson.FilePath$29.invoke(FilePath.java:1600)
            15:38:55 at hudson.FilePath$29.invoke(FilePath.java:1597)
            15:38:55 at hudson.FilePath.act(FilePath.java:998)
            15:38:55 at hudson.FilePath.act(FilePath.java:976)
            15:38:55 at hudson.FilePath.chmod(FilePath.java:1597)
            15:38:55 at org.jenkinsci.plugins.docker.commons.impl.ServerKeyMaterialFactory.materialize(ServerKeyMaterialFactory.java:86)
            15:38:55 at org.jenkinsci.plugins.docker.commons.impl.CompositeKeyMaterialFactory.materialize(CompositeKeyMaterialFactory.java:69)
            15:38:55 at org.jenkinsci.plugins.docker.commons.impl.CompositeKeyMaterialFactory.materialize(CompositeKeyMaterialFactory.java:69)
            15:38:55 at com.cloudbees.dockerpublish.DockerBuilder$Perform.executeCmd(DockerBuilder.java:459)
            15:38:55 at com.cloudbees.dockerpublish.DockerBuilder$Perform.executeCmd(DockerBuilder.java:431)
            15:38:55 at com.cloudbees.dockerpublish.DockerBuilder$Perform.buildAndTag(DockerBuilder.java:373)
            15:38:55 at com.cloudbees.dockerpublish.DockerBuilder$Perform.exec(DockerBuilder.java:311)
            15:38:55 at com.cloudbees.dockerpublish.DockerBuilder$Perform.access$100(DockerBuilder.java:291)
            15:38:55 at com.cloudbees.dockerpublish.DockerBuilder.perform(DockerBuilder.java:262)
            15:38:55 at hudson.tasks.BuildStepMonitor$1.perform(BuildStepMonitor.java:20)
            15:38:55 at hudson.model.AbstractBuild$AbstractBuildExecution.perform(AbstractBuild.java:744)
            15:38:55 at hudson.model.Build$BuildExecution.build(Build.java:206)
            15:38:55 at hudson.model.Build$BuildExecution.doRun(Build.java:163)
            15:38:55 at hudson.model.AbstractBuild$AbstractBuildExecution.run(AbstractBuild.java:504)
            15:38:55 at hudson.model.Run.execute(Run.java:1727)
            15:38:55 at hudson.model.FreeStyleBuild.run(FreeStyleBuild.java:43)
            15:38:55 at hudson.model.ResourceController.execute(ResourceController.java:97)
            15:38:55 at hudson.model.Executor.run(Executor.java:429)
            15:38:55 Build step 'Docker Build and Publish' marked build as failure

             

            Show
            typz Francois Ferrand added a comment - This actually breaks the docker-commons plugin: when it tries to 'materialize' the credentials, it relies on a chmod() call to create a folder (seems this is the behavior of native API), which now leads to an exception: 15:38:55 java.nio.file.NoSuchFileException: /var/lib/jenkins/.docker/7d36d1a6-4917-4dc8-97b6-2babbb2ae3d7 15:38:55 at sun.nio.fs.UnixException.translateToIOException(UnixException.java:86) 15:38:55 at sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:102) 15:38:55 at sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:107) 15:38:55 at sun.nio.fs.UnixFileAttributeViews$Posix.setMode(UnixFileAttributeViews.java:238) 15:38:55 at sun.nio.fs.UnixFileAttributeViews$Posix.setPermissions(UnixFileAttributeViews.java:260) 15:38:55 at java.nio.file.Files.setPosixFilePermissions(Files.java:2045) 15:38:55 at hudson.FilePath._chmod(FilePath.java:1618) 15:38:55 at hudson.FilePath.access$1500(FilePath.java:197) 15:38:55 at hudson.FilePath$29.invoke(FilePath.java:1600) 15:38:55 at hudson.FilePath$29.invoke(FilePath.java:1597) 15:38:55 at hudson.FilePath.act(FilePath.java:998) 15:38:55 at hudson.FilePath.act(FilePath.java:976) 15:38:55 at hudson.FilePath.chmod(FilePath.java:1597) 15:38:55 at org.jenkinsci.plugins.docker.commons.impl.ServerKeyMaterialFactory.materialize(ServerKeyMaterialFactory.java:86) 15:38:55 at org.jenkinsci.plugins.docker.commons.impl.CompositeKeyMaterialFactory.materialize(CompositeKeyMaterialFactory.java:69) 15:38:55 at org.jenkinsci.plugins.docker.commons.impl.CompositeKeyMaterialFactory.materialize(CompositeKeyMaterialFactory.java:69) 15:38:55 at com.cloudbees.dockerpublish.DockerBuilder$Perform.executeCmd(DockerBuilder.java:459) 15:38:55 at com.cloudbees.dockerpublish.DockerBuilder$Perform.executeCmd(DockerBuilder.java:431) 15:38:55 at com.cloudbees.dockerpublish.DockerBuilder$Perform.buildAndTag(DockerBuilder.java:373) 15:38:55 at com.cloudbees.dockerpublish.DockerBuilder$Perform.exec(DockerBuilder.java:311) 15:38:55 at com.cloudbees.dockerpublish.DockerBuilder$Perform.access$100(DockerBuilder.java:291) 15:38:55 at com.cloudbees.dockerpublish.DockerBuilder.perform(DockerBuilder.java:262) 15:38:55 at hudson.tasks.BuildStepMonitor$1.perform(BuildStepMonitor.java:20) 15:38:55 at hudson.model.AbstractBuild$AbstractBuildExecution.perform(AbstractBuild.java:744) 15:38:55 at hudson.model.Build$BuildExecution.build(Build.java:206) 15:38:55 at hudson.model.Build$BuildExecution.doRun(Build.java:163) 15:38:55 at hudson.model.AbstractBuild$AbstractBuildExecution.run(AbstractBuild.java:504) 15:38:55 at hudson.model.Run.execute(Run.java:1727) 15:38:55 at hudson.model.FreeStyleBuild.run(FreeStyleBuild.java:43) 15:38:55 at hudson.model.ResourceController.execute(ResourceController.java:97) 15:38:55 at hudson.model.Executor.run(Executor.java:429) 15:38:55 Build step 'Docker Build and Publish' marked build as failure  
            Hide
            dnusbaum Devin Nusbaum added a comment -

            Francois Ferrand Thanks for reporting the bug. My guess is that previously the call to `FilePath#chmod` failed, but the return value was not checked and so no exception was thrown, and it went unnoticed because ServerKeyMaterialFactory#copyInto calls FilePath#write which created parent directories if they don't already exist.

            I will test to see what happens when the native chmod is used on a non-existent file or directory. If it fails silently before this change, I will file a PR against docker-commons to create the file before changing permissions. If it succeeds, I will change the new code to create the file or directory first if necessary.

            Show
            dnusbaum Devin Nusbaum added a comment - Francois Ferrand Thanks for reporting the bug. My guess is that previously the call to `FilePath#chmod` failed, but the return value was not checked and so no exception was thrown, and it went unnoticed because ServerKeyMaterialFactory#copyInto calls FilePath#write which created parent directories if they don't already exist. I will test to see what happens when the native chmod is used on a non-existent file or directory. If it fails silently before this change, I will file a PR against docker-commons to create the file before changing permissions. If it succeeds, I will change the new code to create the file or directory first if necessary.
            Hide
            dnusbaum Devin Nusbaum added a comment -

            The following test passes against master, which implies that before PR #3135, FilePath#chmod on a non-existent file or directory would fail silently:

                @Test
                public void chmodFileDoesNotExist() throws Exception {
                    assumeFalse(Functions.isWindows());
                    Util.NATIVE_CHMOD_MODE = true; // Use native chmod
                    File f = File.createTempFile("chmod-test", ".tmp");
                    f.delete();
                    FilePath fp = new FilePath(f);
                    fp.chmod(0666);
                    assertFalse(f.exists());
                }
            

            Changing Util.NATIVE_CHMOD_MODE to false (to use NIO) causes the test to throw NoSuchFileException as noted by Francois Ferrand. The native implementation of IOUtils#mode threw an exception if the file did not exist, so it seems only the behavior of FilePath#chmod has changed.

            I searched through the jenkinsci org for chmod to see if any other uses were likely to break after this change.
            Almost all uses call one of mkdirs, touch, exists, write, or createTempFile/Dir immediately before chmod which should prevent them from seeing a NoSuchFileException.There are a few uses such as android-emulator-plugin and copy-data-to-workspace-plugin that could hit new exceptions if files are deleted while the directories are being traversed.

            I think there is value in throwing NoSuchFileException to catch legitimate bugs as in docker-commons, and it doesn't appear that there will be widespread issues with the exception being thrown in plugins where it wouldn't have been before, so I will file a PR against docker-commons to create the directory before calling chmod. If anyone feels strongly that that we should preserve the old behavior I am also happy to catch NoSuchFileException in FilePath#chmod.

            Show
            dnusbaum Devin Nusbaum added a comment - The following test passes against master, which implies that before PR #3135, FilePath#chmod on a non-existent file or directory would fail silently: @Test public void chmodFileDoesNotExist() throws Exception { assumeFalse(Functions.isWindows()); Util.NATIVE_CHMOD_MODE = true ; // Use native chmod File f = File.createTempFile( "chmod-test" , ".tmp" ); f.delete(); FilePath fp = new FilePath(f); fp.chmod(0666); assertFalse(f.exists()); } Changing Util.NATIVE_CHMOD_MODE to false (to use NIO) causes the test to throw NoSuchFileException as noted by Francois Ferrand . The native implementation of IOUtils#mode threw an exception if the file did not exist, so it seems only the behavior of FilePath#chmod has changed. I searched through the jenkinsci org for chmod to see if any other uses were likely to break after this change. Almost all uses call one of mkdirs , touch , exists , write , or createTempFile/Dir immediately before chmod which should prevent them from seeing a NoSuchFileException .There are a few uses such as android-emulator-plugin and  copy-data-to-workspace-plugin that could hit new exceptions if files are deleted while the directories are being traversed. I think there is value in throwing NoSuchFileException to catch legitimate bugs as in docker-commons, and it doesn't appear that there will be widespread issues with the exception being thrown in plugins where it wouldn't have been before, so I will file a PR against docker-commons to create the directory before calling chmod. If anyone feels strongly that that we should preserve the old behavior I am also happy to catch NoSuchFileException in FilePath#chmod .
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Jesse Glick
            Path:
            src/main/java/org/jenkinsci/plugins/docker/commons/impl/ServerKeyMaterialFactory.java
            src/test/java/org/jenkinsci/plugins/docker/commons/credentials/DockerServerEndpointTest.java
            http://jenkins-ci.org/commit/docker-commons-plugin/c871b5e85fc96df8f70ef65aa96d2d73edf16615
            Log:
            Merge pull request #62 from dwnusbaum/JENKINS-36088-exposed-bug

            Create directory before calling chmod

            Compare: https://github.com/jenkinsci/docker-commons-plugin/compare/c0d9723d787a...c871b5e85fc9

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Jesse Glick Path: src/main/java/org/jenkinsci/plugins/docker/commons/impl/ServerKeyMaterialFactory.java src/test/java/org/jenkinsci/plugins/docker/commons/credentials/DockerServerEndpointTest.java http://jenkins-ci.org/commit/docker-commons-plugin/c871b5e85fc96df8f70ef65aa96d2d73edf16615 Log: Merge pull request #62 from dwnusbaum/ JENKINS-36088 -exposed-bug Create directory before calling chmod Compare: https://github.com/jenkinsci/docker-commons-plugin/compare/c0d9723d787a...c871b5e85fc9

              People

              • Assignee:
                Unassigned
                Reporter:
                jglick Jesse Glick
              • Votes:
                1 Vote for this issue
                Watchers:
                8 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: