Skip to content
Permalink
Browse files

[JENKINS-21486] Fix plugin dependencies resolution

* Check that dependencies are enabled. A disabled optional dependency
  will not prevent a plugin from loading.
* Check versions of dependencies declared by a plugin before loading it.
  If any dependency (even optional) is older than what is required,
  then the plugin isn't loaded.

This should prevent use cases where a plugin is loaded but one of its
dependencies is too old so that :
* its @extension annotated classes cannot be loaded, causing the full
  Jenkins to blow up with crapload of exceptions which are tedious to
  investigate to understand the root cause.
* NoSuchMethodError and the likes at runtime even though boot has
  completed.

Version check (for setups where version list is manually crafted but yet
works) can be disabled by starting Jenkins with

-Dhudson.PluginWrapper.dependenciesVersionCheck.enabled=true

Minor fixes done while implementing this change :
* Fix version parsing in PluginWrapper.Dependency
* Dynamic plugin load didn't check for disabled flag
  • Loading branch information...
Vlatombe committed Mar 25, 2016
1 parent 86e14dd commit 43ddbdfc1cd651e0430289766637a11717494e88
@@ -844,7 +844,8 @@ public void dynamicLoad(File arc, boolean removeExisting) throws IOException, In
// so existing plugins can't be depending on this newly deployed one.

plugins.add(p);
activePlugins.add(p);
if (p.isActive())
activePlugins.add(p);
synchronized (((UberClassLoader) uberClassLoader).loaded) {
((UberClassLoader) uberClassLoader).loaded.clear();
}
@@ -88,6 +88,12 @@
*/
@ExportedBean
public class PluginWrapper implements Comparable<PluginWrapper>, ModelObject {
/**
* A plugin won't be loaded unless his declared dependencies are present and match the required minimal version.
* This can be set to false to disable the version check (legacy behaviour)
*/
private static final boolean ENABLE_PLUGIN_DEPENDENCIES_VERSION_CHECK = Boolean.parseBoolean(System.getProperty(PluginWrapper.class.getName()+"." + "dependenciesVersionCheck.enabled", "true"));

/**
* {@link PluginManager} to which this belongs to.
*/
@@ -229,7 +235,7 @@ public Dependency(String s) {

@Override
public String toString() {
return shortName + " (" + version + ")";
return shortName + " (" + version + ") " + (optional ? "optional" : "");
}
}

@@ -394,6 +400,21 @@ private String getVersionOf(Manifest manifest) {
return "???";
}

/**
* Returns the required Jenkins core version of this plugin.
* @return the required Jenkins core version of this plugin.
* @since XXX
*/
@Exported
public @CheckForNull String getRequiredCoreVersion() {
String v = manifest.getMainAttributes().getValue("Jenkins-Version");
if (v!= null) return v;

v = manifest.getMainAttributes().getValue("Hudson-Version");
if (v!= null) return v;
return null;
}

/**
* Returns the version number of this plugin
*/
@@ -524,19 +545,71 @@ public boolean hasLicensesXml() {
* thrown if one or several mandatory dependencies doesn't exists.
*/
/*package*/ void resolvePluginDependencies() throws IOException {
List<Dependency> missingDependencies = new ArrayList<>();
if (ENABLE_PLUGIN_DEPENDENCIES_VERSION_CHECK) {
String requiredCoreVersion = getRequiredCoreVersion();
if (requiredCoreVersion == null) {
LOGGER.warning(shortName + " doesn't declare required core version.");
} else {
if (Jenkins.getVersion().isOlderThan(new VersionNumber(requiredCoreVersion))) {
throw new IOException(shortName + " requires a more recent core version (" + requiredCoreVersion + ") than the current (" + Jenkins.getVersion() + ").");
}
}
}
List<String> missingDependencies = new ArrayList<String>();
List<String> obsoleteDependencies = new ArrayList<String>();
List<String> disabledDependencies = new ArrayList<String>();
// make sure dependencies exist
for (Dependency d : dependencies) {
if (parent.getPlugin(d.shortName) == null)
missingDependencies.add(d);
}
if (!missingDependencies.isEmpty())
throw new MissingDependencyException(this.shortName, missingDependencies);
PluginWrapper dependency = parent.getPlugin(d.shortName);
if (dependency == null) {
missingDependencies.add(d.toString());
} else {
if (dependency.isActive()) {
if (ENABLE_PLUGIN_DEPENDENCIES_VERSION_CHECK && dependency.getVersionNumber().isOlderThan(new VersionNumber(d.version))) {
obsoleteDependencies.add(dependency.getShortName() + "(" + dependency.getVersion() + " < " + d.version + ")");
}
} else {
disabledDependencies.add(d.toString());
}

}
}
// add the optional dependencies that exists
for (Dependency d : optionalDependencies) {
if (parent.getPlugin(d.shortName) != null)
dependencies.add(d);
PluginWrapper dependency = parent.getPlugin(d.shortName);
if (dependency != null && dependency.isActive()) {
if (ENABLE_PLUGIN_DEPENDENCIES_VERSION_CHECK && dependency.getVersionNumber().isOlderThan(new VersionNumber(d.version))) {
obsoleteDependencies.add(dependency.getShortName() + "(" + dependency.getVersion() + " < " + d.version + ")");
} else {
dependencies.add(d);
}
}
}
StringBuilder messageBuilder = new StringBuilder();
if (!missingDependencies.isEmpty()) {
boolean plural = missingDependencies.size() > 1;
messageBuilder.append(plural ? "Dependencies " : "Dependency ")
.append(Util.join(missingDependencies, ", "))
.append(" ").append(plural ? "don't" : "doesn't")
.append(" exist. ");
}
if (!disabledDependencies.isEmpty()) {
boolean plural = disabledDependencies.size() > 1;
messageBuilder.append(plural ? "Dependencies " : "Dependency ")
.append(Util.join(missingDependencies, ", "))
.append(" ").append(plural ? "are" : "is")
.append(" disabled. ");
}
if (!obsoleteDependencies.isEmpty()) {
boolean plural = obsoleteDependencies.size() > 1;
messageBuilder.append(plural ? "Dependencies " : "Dependency ")
.append(Util.join(obsoleteDependencies, ", "))
.append(" ").append(plural ? "are" : "is")
.append(" older than required.");
}
String message = messageBuilder.toString();
if (!message.isEmpty()) {
throw new IOException(message);
}
}

@@ -47,6 +47,7 @@
import org.apache.commons.io.FileUtils;
import org.apache.tools.ant.filters.StringInputStream;
import static org.junit.Assert.*;

import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
@@ -339,6 +340,75 @@ private String callDependerValue() throws Exception {
assertTrue(r.jenkins.getExtensionList("org.jenkinsci.plugins.dependencytest.dependee.DependeeExtensionPoint").isEmpty());
}

@Issue("JENKINS-21486")
@Test public void installPluginWithObsoleteDependencyFails() throws Exception {
// Load dependee 0.0.1.
{
dynamicLoad("dependee.hpi");
}

// Load mandatory-depender 0.0.2, depending on dependee 0.0.2
try {
dynamicLoad("mandatory-depender-0.0.2.hpi");
fail("Should not have worked");
} catch (IOException e) {
// Expected
}
}

@Issue("JENKINS-21486")
@Test public void installPluginWithDisabledOptionalDependencySucceeds() throws Exception {
// Load dependee 0.0.2.
{
dynamicLoadAndDisable("dependee-0.0.2.hpi");
}

// Load depender 0.0.2, depending optionally on dependee 0.0.2
{
dynamicLoad("depender-0.0.2.hpi");
}

// dependee is not loaded so we cannot list any extension for it.
try {
r.jenkins.getExtensionList("org.jenkinsci.plugins.dependencytest.dependee.DependeeExtensionPoint");
fail();
} catch( ClassNotFoundException _ ){
}
}

@Issue("JENKINS-21486")
@Test public void installPluginWithDisabledDependencyFails() throws Exception {
// Load dependee 0.0.2.
{
dynamicLoadAndDisable("dependee-0.0.2.hpi");
}

// Load mandatory-depender 0.0.2, depending on dependee 0.0.2
try {
dynamicLoad("mandatory-depender-0.0.2.hpi");
fail("Should not have worked");
} catch (IOException e) {
// Expected
}
}


@Issue("JENKINS-21486")
@Test public void installPluginWithObsoleteOptionalDependencyFails() throws Exception {
// Load dependee 0.0.1.
{
dynamicLoad("dependee.hpi");
}

// Load depender 0.0.2, depending optionally on dependee 0.0.2
try {
dynamicLoad("depender-0.0.2.hpi");
fail("Should not have worked");
} catch (IOException e) {
// Expected
}
}

@Issue("JENKINS-12753")
@WithPlugin("tasks.jpi")
@Test public void dynamicLoadRestartRequiredException() throws Exception {
@@ -378,6 +448,10 @@ private void dynamicLoad(String plugin) throws IOException, InterruptedException
PluginManagerUtil.dynamicLoad(plugin, r.jenkins);
}

private void dynamicLoadAndDisable(String plugin) throws IOException, InterruptedException, RestartRequiredException {
PluginManagerUtil.dynamicLoad(plugin, r.jenkins, true);
}

@Test public void uploadDependencyResolution() throws Exception {
PersistedList<UpdateSite> sites = r.jenkins.getUpdateCenter().getSites();
sites.clear();
@@ -48,9 +48,16 @@ public void before() throws Throwable {
}

public static void dynamicLoad(String plugin, Jenkins jenkins) throws IOException, InterruptedException, RestartRequiredException {
dynamicLoad(plugin, jenkins, false);
}

public static void dynamicLoad(String plugin, Jenkins jenkins, boolean disable) throws IOException, InterruptedException, RestartRequiredException {
URL src = PluginManagerTest.class.getClassLoader().getResource("plugins/" + plugin);
File dest = new File(jenkins.getRootDir(), "plugins/" + plugin);
FileUtils.copyURLToFile(src, dest);
if (disable) {
new File(dest.getPath() + ".disabled").createNewFile();
}
jenkins.pluginManager.dynamicLoad(dest);
}
}
@@ -0,0 +1,26 @@
package hudson;

import org.junit.Test;

import static org.junit.Assert.assertEquals;

public class PluginWrapperTest {

@Test
public void dependencyTest() {
String version = "plugin:0.0.2";
PluginWrapper.Dependency dependency = new PluginWrapper.Dependency(version);
assertEquals("plugin", dependency.shortName);
assertEquals("0.0.2", dependency.version);
assertEquals(false, dependency.optional);
}

@Test
public void optionalDependencyTest() {
String version = "plugin:0.0.2;resolution:=optional";
PluginWrapper.Dependency dependency = new PluginWrapper.Dependency(version);
assertEquals("plugin", dependency.shortName);
assertEquals("0.0.2", dependency.version);
assertEquals(true, dependency.optional);
}
}
Binary file not shown.
Binary file not shown.
Binary file not shown.

0 comments on commit 43ddbdf

Please sign in to comment.
You can’t perform that action at this time.