diff --git a/core/src/main/java/hudson/PluginManager.java b/core/src/main/java/hudson/PluginManager.java index 92416858e700..910d560a8bfd 100644 --- a/core/src/main/java/hudson/PluginManager.java +++ b/core/src/main/java/hudson/PluginManager.java @@ -505,7 +505,7 @@ public void run(Reactor session) throws Exception { // schedule execution of loading plugins for (final PluginWrapper p : activePlugins.toArray(new PluginWrapper[activePlugins.size()])) { - g.followedBy().notFatal().attains(PLUGINS_PREPARED).add("Loading plugin " + p.getShortName(), new Executable() { + g.followedBy().notFatal().attains(PLUGINS_PREPARED).add(String.format("Loading plugin %s v%s (%s)", p.getLongName(), p.getVersion(), p.getShortName()), new Executable() { public void run(Reactor session) throws Exception { try { p.resolvePluginDependencies(); @@ -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(); } @@ -1867,14 +1868,14 @@ public static final class PluginCycleDependenciesMonitor extends AdministrativeM private transient volatile boolean isActive = false; - private transient volatile List pluginsWithCycle; + private transient volatile List pluginsWithCycle; public boolean isActivated() { if(pluginsWithCycle == null){ - pluginsWithCycle = new ArrayList(); + pluginsWithCycle = new ArrayList<>(); for (PluginWrapper p : Jenkins.getInstance().getPluginManager().getPlugins()) { if(p.hasCycleDependency()){ - pluginsWithCycle.add(p.getShortName()); + pluginsWithCycle.add(p); isActive = true; } } @@ -1882,7 +1883,7 @@ public boolean isActivated() { return isActive; } - public List getPluginsWithCycle() { + public List getPluginsWithCycle() { return pluginsWithCycle; } } diff --git a/core/src/main/java/hudson/PluginWrapper.java b/core/src/main/java/hudson/PluginWrapper.java index f61b96e7cf20..b97e977ede35 100644 --- a/core/src/main/java/hudson/PluginWrapper.java +++ b/core/src/main/java/hudson/PluginWrapper.java @@ -26,43 +26,51 @@ import com.google.common.collect.ImmutableSet; import hudson.PluginManager.PluginInstanceStore; +import hudson.model.AdministrativeMonitor; import hudson.model.Api; import hudson.model.ModelObject; -import jenkins.MissingDependencyException; import jenkins.YesNoMaybe; import jenkins.model.Jenkins; import hudson.model.UpdateCenter; import hudson.model.UpdateSite; import hudson.util.VersionNumber; +import org.jvnet.localizer.ResourceBundleHolder; +import org.kohsuke.stapler.HttpResponse; +import org.kohsuke.stapler.HttpResponses; +import org.kohsuke.stapler.StaplerRequest; +import org.kohsuke.stapler.StaplerResponse; +import org.kohsuke.stapler.export.Exported; +import org.kohsuke.stapler.export.ExportedBean; +import org.kohsuke.stapler.interceptor.RequirePOST; +import org.apache.commons.lang.StringUtils; +import org.apache.commons.logging.LogFactory; +import javax.annotation.CheckForNull; +import javax.annotation.Nonnull; +import java.io.Closeable; import java.io.File; import java.io.FileOutputStream; import java.io.IOException; import java.io.OutputStream; -import java.io.Closeable; import java.net.URL; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collection; import java.util.Collections; +import java.util.Enumeration; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Iterator; import java.util.List; +import java.util.Map; import java.util.Set; +import java.util.jar.JarFile; import java.util.jar.Manifest; +import java.util.logging.Level; import java.util.logging.Logger; + import static java.util.logging.Level.WARNING; import static org.apache.commons.io.FilenameUtils.getBaseName; -import org.apache.commons.lang.StringUtils; -import org.apache.commons.logging.LogFactory; -import org.kohsuke.stapler.HttpResponse; -import org.kohsuke.stapler.HttpResponses; -import org.kohsuke.stapler.export.Exported; -import org.kohsuke.stapler.export.ExportedBean; -import org.kohsuke.stapler.interceptor.RequirePOST; - -import java.util.Enumeration; -import java.util.jar.JarFile; -import java.util.logging.Level; -import javax.annotation.CheckForNull; -import javax.annotation.Nonnull; /** * Represents a Jenkins plug-in and associated control information @@ -88,6 +96,12 @@ */ @ExportedBean public class PluginWrapper implements Comparable, 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. */ @@ -142,6 +156,12 @@ public class PluginWrapper implements Comparable, ModelObject { private final List dependencies; private final List optionalDependencies; + public List getDependencyErrors() { + return Collections.unmodifiableList(dependencyErrors); + } + + private final transient List dependencyErrors = new ArrayList<>(); + /** * Is this plugin bundled in jenkins.war? */ @@ -229,7 +249,7 @@ public Dependency(String s) { @Override public String toString() { - return shortName + " (" + version + ")"; + return shortName + " (" + version + ")" + (optional ? " optional" : ""); } } @@ -394,6 +414,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,20 +559,71 @@ public boolean hasLicensesXml() { * thrown if one or several mandatory dependencies doesn't exists. */ /*package*/ void resolvePluginDependencies() throws IOException { - List 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 { + VersionNumber actualVersion = Jenkins.getVersion(); + if (actualVersion.isOlderThan(new VersionNumber(requiredCoreVersion))) { + dependencyErrors.add(Messages.PluginWrapper_obsoleteCore(Jenkins.getVersion().toString(), requiredCoreVersion)); + } + } + } // 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) { + PluginWrapper failedDependency = NOTICE.getPlugin(d.shortName); + if (failedDependency != null) { + dependencyErrors.add(Messages.PluginWrapper_failed_to_load_dependency(failedDependency.getLongName(), d.version)); + break; + } else { + dependencyErrors.add(Messages.PluginWrapper_missing(d.shortName, d.version)); + } + } else { + if (dependency.isActive()) { + if (isDependencyObsolete(d, dependency)) { + dependencyErrors.add(Messages.PluginWrapper_obsolete(dependency.getLongName(), dependency.getVersion(), d.version)); + } + } else { + if (isDependencyObsolete(d, dependency)) { + dependencyErrors.add(Messages.PluginWrapper_disabledAndObsolete(dependency.getLongName(), dependency.getVersion(), d.version)); + } else { + dependencyErrors.add(Messages.PluginWrapper_disabled(dependency.getLongName())); + } + } + } + } // 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 (isDependencyObsolete(d, dependency)) { + dependencyErrors.add(Messages.PluginWrapper_obsolete(dependency.getLongName(), dependency.getVersion(), d.version)); + } else { + dependencies.add(d); + } + } } + if (!dependencyErrors.isEmpty()) { + NOTICE.addPlugin(this); + StringBuilder messageBuilder = new StringBuilder(); + messageBuilder.append(Messages.PluginWrapper_failed_to_load_plugin(getLongName(), getVersion())).append(System.lineSeparator()); + for (Iterator iterator = dependencyErrors.iterator(); iterator.hasNext(); ) { + String dependencyError = iterator.next(); + messageBuilder.append(" - ").append(dependencyError); + if (iterator.hasNext()) { + messageBuilder.append(System.lineSeparator()); + } + } + throw new IOException(messageBuilder.toString()); + } + } + + private boolean isDependencyObsolete(Dependency d, PluginWrapper dependency) { + return ENABLE_PLUGIN_DEPENDENCIES_VERSION_CHECK && dependency.getVersionNumber().isOlderThan(new VersionNumber(d.version)); } /** @@ -645,6 +731,46 @@ public boolean isPinningForcingOldVersion() { return false; } + @Extension + public final static PluginWrapperAdministrativeMonitor NOTICE = new PluginWrapperAdministrativeMonitor(); + + /** + * Administrative Monitor for failed plugins + */ + public static final class PluginWrapperAdministrativeMonitor extends AdministrativeMonitor { + private final Map plugins = new HashMap<>(); + + void addPlugin(PluginWrapper plugin) { + plugins.put(plugin.shortName, plugin); + } + + public boolean isActivated() { + return !plugins.isEmpty(); + } + + public Collection getPlugins() { + return plugins.values(); + } + + public PluginWrapper getPlugin(String shortName) { + return plugins.get(shortName); + } + + /** + * Depending on whether the user said "dismiss" or "correct", send him to the right place. + */ + public void doAct(StaplerRequest req, StaplerResponse rsp) throws IOException { + if(req.hasParameter("correct")) { + rsp.sendRedirect(req.getContextPath()+"/pluginManager"); + + } + } + + public static PluginWrapperAdministrativeMonitor get() { + return AdministrativeMonitor.all().get(PluginWrapperAdministrativeMonitor.class); + } + } + // // // Action methods diff --git a/core/src/main/resources/hudson/Messages.properties b/core/src/main/resources/hudson/Messages.properties index 038ecc0cd630..ec7f6002b40a 100644 --- a/core/src/main/resources/hudson/Messages.properties +++ b/core/src/main/resources/hudson/Messages.properties @@ -73,4 +73,11 @@ ProxyConfiguration.Success=Success Functions.NoExceptionDetails=No Exception details +PluginWrapper.missing={0} v{1} is missing. To fix, install v{1} or later. +PluginWrapper.failed_to_load_plugin={0} v{1} failed to load. +PluginWrapper.failed_to_load_dependency={0} v{1} failed to load. Fix this plugin first. +PluginWrapper.disabledAndObsolete={0} v{1} is disabled and older than required. To fix, install v{2} or later and enable it. +PluginWrapper.disabled={0} is disabled. To fix, enable it. +PluginWrapper.obsolete={0} v{1} is older than required. To fix, install v{2} or later. +PluginWrapper.obsoleteCore=You must update Jenkins from v{0} to v{1} or later to run this plugin. TcpSlaveAgentListener.PingAgentProtocol.displayName=Ping protocol diff --git a/core/src/main/resources/hudson/PluginManager/PluginCycleDependenciesMonitor/message.jelly b/core/src/main/resources/hudson/PluginManager/PluginCycleDependenciesMonitor/message.jelly index 3a7110fdca57..1c80b2d66a3c 100644 --- a/core/src/main/resources/hudson/PluginManager/PluginCycleDependenciesMonitor/message.jelly +++ b/core/src/main/resources/hudson/PluginManager/PluginCycleDependenciesMonitor/message.jelly @@ -28,7 +28,7 @@ THE SOFTWARE. ${%PluginCycles}
    -
    • +
diff --git a/core/src/main/resources/hudson/PluginWrapper/PluginWrapperAdministrativeMonitor/message.jelly b/core/src/main/resources/hudson/PluginWrapper/PluginWrapperAdministrativeMonitor/message.jelly new file mode 100644 index 000000000000..54fe07266086 --- /dev/null +++ b/core/src/main/resources/hudson/PluginWrapper/PluginWrapperAdministrativeMonitor/message.jelly @@ -0,0 +1,22 @@ + + +
+
+
+ +
+
+ There are dependency errors loading some plugins: +
    + +
  • ${plugin.longName} v${plugin.version} +
      + +
    • ${d}
      • +       +
    +
    • +     +
+
+ diff --git a/test/src/test/java/hudson/PluginManagerTest.java b/test/src/test/java/hudson/PluginManagerTest.java index 42847950cdfb..e7bb69de1bdd 100644 --- a/test/src/test/java/hudson/PluginManagerTest.java +++ b/test/src/test/java/hudson/PluginManagerTest.java @@ -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 sites = r.jenkins.getUpdateCenter().getSites(); sites.clear(); diff --git a/test/src/test/java/hudson/PluginManagerUtil.java b/test/src/test/java/hudson/PluginManagerUtil.java index e67b9b31f9f3..2263622b4c44 100644 --- a/test/src/test/java/hudson/PluginManagerUtil.java +++ b/test/src/test/java/hudson/PluginManagerUtil.java @@ -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); } } diff --git a/test/src/test/java/hudson/PluginWrapperTest.java b/test/src/test/java/hudson/PluginWrapperTest.java new file mode 100644 index 000000000000..7529c5319495 --- /dev/null +++ b/test/src/test/java/hudson/PluginWrapperTest.java @@ -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); + } +} diff --git a/test/src/test/java/hudson/model/UsageStatisticsTest.java b/test/src/test/java/hudson/model/UsageStatisticsTest.java index 63b6a14279de..a02ee18d7844 100644 --- a/test/src/test/java/hudson/model/UsageStatisticsTest.java +++ b/test/src/test/java/hudson/model/UsageStatisticsTest.java @@ -105,7 +105,9 @@ public void roundtrip() throws Exception { List plugins = sortPlugins((List) o.get("plugins")); Set detached = new TreeSet<>(); for (ClassicPluginStrategy.DetachedPlugin p: ClassicPluginStrategy.getDetachedPlugins()) { - detached.add(p.getShortName()); + if (p.getSplitWhen().isOlderThan(Jenkins.getVersion())) { + detached.add(p.getShortName()); + } } Set keys = new TreeSet<>(); keys.add("name"); diff --git a/test/src/test/resources/plugins/dependee-0.0.2.hpi b/test/src/test/resources/plugins/dependee-0.0.2.hpi new file mode 100644 index 000000000000..79525b08464f Binary files /dev/null and b/test/src/test/resources/plugins/dependee-0.0.2.hpi differ diff --git a/test/src/test/resources/plugins/depender-0.0.2.hpi b/test/src/test/resources/plugins/depender-0.0.2.hpi new file mode 100644 index 000000000000..34826d3f7707 Binary files /dev/null and b/test/src/test/resources/plugins/depender-0.0.2.hpi differ diff --git a/test/src/test/resources/plugins/mandatory-depender-0.0.2.hpi b/test/src/test/resources/plugins/mandatory-depender-0.0.2.hpi new file mode 100644 index 000000000000..dedc65a62f99 Binary files /dev/null and b/test/src/test/resources/plugins/mandatory-depender-0.0.2.hpi differ