JENKINS-18401: Don't fail on files outside $JENKINS_HOME

This plugin can only put things under $JENKINS_HOME into SCM. If workspaces are located elsewhere, it failed with an exception. Newly, we just ignore such files. In all likelihood, users won't want to sync workspace files anyway. The same goes for the builds directory. People who really might want to put stuff from there into SCM can try symlinking to their desired workspace/build roots from $JENKINS_HOME. Restoring from SCM may, however, then produce unexpected results. Should fix JENKINS-18401 and related issues such as JENKINS-13593 and JENKINS-19984. Includes two tests for buildPathRelativeToHudsonRoot(), and use org.junit.Assert.assertNotNull etc.instead of the hamcrest matchers.
jenkinsci · Jul 1, 2015 · dec7428 · dec7428
1 parent 652c6d2
commit dec7428
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 23 deletions.
diff --git a/src/main/java/hudson/plugins/scm_sync_configuration/JenkinsFilesHelper.java b/src/main/java/hudson/plugins/scm_sync_configuration/JenkinsFilesHelper.java
@@ -9,7 +9,7 @@ public class JenkinsFilesHelper {
 	public static String buildPathRelativeToHudsonRoot(File file){
 		File hudsonRoot = Hudson.getInstance().getRootDir();
 		if(!file.getAbsolutePath().startsWith(hudsonRoot.getAbsolutePath())){
-			throw new IllegalArgumentException("Err ! File ["+file.getAbsolutePath()+"] seems not to reside in ["+hudsonRoot.getAbsolutePath()+"] !");
+			return null;
 		}
 		String truncatedPath = file.getAbsolutePath().substring(hudsonRoot.getAbsolutePath().length()+1); // "+1" because we don't need ending file separator
 		return truncatedPath.replaceAll("\\\\", "/"); 

diff --git a/...lugins/scm_sync_configuration/strategies/model/JobOrFolderConfigurationEntityMatcher.java b/...lugins/scm_sync_configuration/strategies/model/JobOrFolderConfigurationEntityMatcher.java
@@ -39,7 +39,8 @@ public boolean matches(Saveable saveable, File file) {
 				// Deleted.
 				file = ((AbstractItem) saveable).getConfigFile().getFile();
 			}
-			return CONFIGS_TO_MATCH.matcher(JenkinsFilesHelper.buildPathRelativeToHudsonRoot(file)).matches();
+			String jenkinsRelativePath = JenkinsFilesHelper.buildPathRelativeToHudsonRoot(file);
+			return jenkinsRelativePath != null && CONFIGS_TO_MATCH.matcher(jenkinsRelativePath).matches();				
 		}
 		return false;
 	}

diff --git a/...in/java/hudson/plugins/scm_sync_configuration/strategies/model/PatternsEntityMatcher.java b/...in/java/hudson/plugins/scm_sync_configuration/strategies/model/PatternsEntityMatcher.java
@@ -27,17 +27,19 @@ public boolean matches(Saveable saveable, File file) {
 			return false;
 		}
 		String pathRelativeToRoot = JenkinsFilesHelper.buildPathRelativeToHudsonRoot(file);
-		// Guard our own SCM workspace and the war directory. User-defined includes might inadvertently include those if they start with * or **!
-		if (pathRelativeToRoot.equals(SCM_WORKING_DIRECTORY) || pathRelativeToRoot.startsWith(SCM_WORKING_DIRECTORY + '/')) {
-			return false;
-		} else if (pathRelativeToRoot.equals(WAR_DIRECTORY) || pathRelativeToRoot.startsWith(WAR_DIRECTORY + '/')) {
-			return false;
-		}
-        AntPathMatcher matcher = new AntPathMatcher();
-        for (String pattern : includesPatterns) {
-            if (matcher.match(pattern, pathRelativeToRoot)) {
-                return true;
-            }
+		if (pathRelativeToRoot != null) {
+			// Guard our own SCM workspace and the war directory. User-defined includes might inadvertently include those if they start with * or **!
+			if (pathRelativeToRoot.equals(SCM_WORKING_DIRECTORY) || pathRelativeToRoot.startsWith(SCM_WORKING_DIRECTORY + '/')) {
+				return false;
+			} else if (pathRelativeToRoot.equals(WAR_DIRECTORY) || pathRelativeToRoot.startsWith(WAR_DIRECTORY + '/')) {
+				return false;
+			}
+	        AntPathMatcher matcher = new AntPathMatcher();
+	        for (String pattern : includesPatterns) {
+	            if (matcher.match(pattern, pathRelativeToRoot)) {
+	                return true;
+	            }
+			}
 		}
 		return false;
 	}

diff --git a/src/test/java/hudson/plugins/scm_sync_configuration/basic/ScmSyncConfigurationBasicTest.java b/src/test/java/hudson/plugins/scm_sync_configuration/basic/ScmSyncConfigurationBasicTest.java
@@ -1,16 +1,19 @@
 package hudson.plugins.scm_sync_configuration.basic;
 
-import static org.hamcrest.CoreMatchers.equalTo;
-import static org.hamcrest.CoreMatchers.is;
-import static org.hamcrest.CoreMatchers.not;
-import static org.hamcrest.CoreMatchers.notNullValue;
-import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 import hudson.model.Hudson;
+import hudson.plugins.scm_sync_configuration.JenkinsFilesHelper;
 import hudson.plugins.scm_sync_configuration.util.ScmSyncConfigurationBaseTest;
 import hudson.plugins.test.utils.scms.ScmUnderTestSubversion;
 
 import java.io.File;
 
+import jenkins.model.Jenkins;
+
 import org.junit.Test;
 
 public class ScmSyncConfigurationBasicTest extends ScmSyncConfigurationBaseTest {
@@ -22,16 +25,38 @@ public ScmSyncConfigurationBasicTest() {
 	@Test
 	public void shouldRetrieveMockedHudsonInstanceCorrectly() throws Throwable {
 		Hudson hudsonInstance = Hudson.getInstance();
-		assertThat(hudsonInstance, is(notNullValue()));
-		assertThat(hudsonInstance.toString().split("@")[0], is(not(equalTo("hudson.model.Hudson"))));
+		assertNotNull("Jenkins instance must not be null", hudsonInstance);
+		assertFalse("Expected a mocked Jenkins instance", "hudson.model.Hudson".equals(hudsonInstance.toString().split("@")[0]));
 	}
 
 	@Test
 	public void shouldVerifyIfHudsonRootDirectoryExists() throws Throwable {
-
 		Hudson hudsonInstance = Hudson.getInstance();
 		File hudsonRootDir = hudsonInstance.getRootDir();
-		assertThat(hudsonRootDir, is(not(equalTo(null))));
-		assertThat(hudsonRootDir.exists(), is(true));
+		assertNotNull("Jenkins instance must not be null", hudsonRootDir);
+		assertTrue("$JENKINS_HOME must be an existing directory", hudsonRootDir.isDirectory());
+	}
+
+	@Test
+	public void testPathesOutsideJenkisRoot () throws Exception {
+		Jenkins jenkins = Jenkins.getInstance();
+		File rootDirectory = jenkins.getRootDir().getAbsoluteFile();
+		File parentDirectory = rootDirectory.getParentFile();
+		assertNull("File outside $JENKINS_HOME should return null", JenkinsFilesHelper.buildPathRelativeToHudsonRoot(parentDirectory));
+		assertNull("File outside $JENKINS_HOME should return null", JenkinsFilesHelper.buildPathRelativeToHudsonRoot(new File(parentDirectory, "foo.txt")));
+	}
+
+	@Test
+	public void testPathesInsideJenkisRoot () throws Exception {
+		Jenkins jenkins = Jenkins.getInstance();
+		File rootDirectory = jenkins.getRootDir().getAbsoluteFile();
+		File pathUnderTest = new File(rootDirectory, "config.xml");
+		String result = JenkinsFilesHelper.buildPathRelativeToHudsonRoot(pathUnderTest);
+		assertNotNull("File inside $JENKINS_HOME must not return null path", result);
+		assertEquals("Path " + pathUnderTest + " should resolve properly", result, "config.xml");
+		pathUnderTest = new File(new File (rootDirectory, "someDir"), "foo.txt");
+		result = JenkinsFilesHelper.buildPathRelativeToHudsonRoot(pathUnderTest);
+		assertNotNull("File inside $JENKINS_HOME must not return null path", result);
+		assertEquals("Path " + pathUnderTest + " should resolve properly", result, "someDir/foo.txt");
 	}
 }