Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid Apache Commons IO utility methods when writing config files to JENKINS_HOME to fix AccessDeniedException in some cases #436

Merged
merged 1 commit into from
Sep 11, 2024

Conversation

dwnusbaum
Copy link
Member

(I can file a Jira ticket if needed)

I recently investigated an issue reported by a user where Jenkins was working totally normally, but when they tried to change their security realm to SAML, they were unable to save the change. The issue seemed to be that they had JENKINS_HOME hard linked to an NFS mount point like /my-mount, and this broke the logic in the Apache Common IO utility methods that check for the existence of the file and create parent directories if needed. Here is the relevant part of the stack trace:

java.nio.file.AccessDeniedException: /my-mount
	at java.base/sun.nio.fs.UnixException.translateToIOException(UnixException.java:90)
	at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:111)
	at java.base/sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:116)
	at java.base/sun.nio.fs.UnixFileSystemProvider.createDirectory(UnixFileSystemProvider.java:389)
	at java.base/java.nio.file.Files.createDirectory(Files.java:690)
	at java.base/java.nio.file.Files.createAndCheckIsDirectory(Files.java:797)
	at java.base/java.nio.file.Files.createDirectories(Files.java:783)
	at org.apache.commons.io.file.PathUtils.createParentDirectories(PathUtils.java:401)
	at org.apache.commons.io.file.PathUtils.newOutputStream(PathUtils.java:1212)
	at org.apache.commons.io.file.PathUtils.newOutputStream(PathUtils.java:1207)
	at org.apache.commons.io.FileUtils.newOutputStream(FileUtils.java:2476)
	at org.apache.commons.io.FileUtils.writeStringToFile(FileUtils.java:3509)
	at org.apache.commons.io.FileUtils.writeStringToFile(FileUtils.java:3494)
	at org.jenkinsci.plugins.saml.IdpMetadataConfiguration.createIdPMetadataFile(IdpMetadataConfiguration.java:119)

Interactive testing in the script console showed that the FileUtils.writeStringToFile method really was uniquely causing the issue (I also checked whether the use of getAbsolutePath in the config file path getter methods was a problem), and Files.write worked fine for the same files.

In this plugin, we always use these methods to write files that are direct children of JENKINS_HOME, so there is no need to
check for and create parent directories since we can assume that JENKINS_HOME exists, so this PR just switches to using Files.write in all of the relevant cases.

I did not find a way to reproduce the same AccessDeniesException reported by the user in a test to be able to create a regression test. The logic seems to be covered in at least basic ways by the existing tests though.

Submitter checklist

  • JIRA issue is well described
  • Appropriate autotests or explanation to why this change has no tests

@kuisathaverat
Copy link

kuisathaverat commented Sep 11, 2024

Poor performance NFS (<150MB/s) will always be an issue if you mount the JENKINS_HOME there. I guess that java.nio performs better than commons IO, so the problem is less frequent, but it will still fail.

@kuisathaverat kuisathaverat merged commit 9f1c332 into jenkinsci:main Sep 11, 2024
15 checks passed
@kuisathaverat
Copy link

released at https://github.com/jenkinsci/saml-plugin/releases/tag/4.487.v9f1c3328f1c0

@dwnusbaum dwnusbaum deleted the avoid-commons-io branch September 11, 2024 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants