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

[JENKINS-69487] prevent uninstalled detached plugins get reinstalled #8634

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion core/src/main/java/hudson/ClassicPluginStrategy.java
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,12 @@
if (paths.isEmpty()) {
LOGGER.info("No classpaths found for plugin " + archive.getName());
}

File uninstalledFile = new File(archive.getPath() + ".uninstalled");
if (uninstalledFile.exists()) {
if (!uninstalledFile.delete()) {

Check warning on line 218 in core/src/main/java/hudson/ClassicPluginStrategy.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 218 is only partially covered, one branch is missing
LOGGER.warning("Failed to delete plugin uninstall file " + uninstalledFile);

Check warning on line 219 in core/src/main/java/hudson/ClassicPluginStrategy.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 219 is not covered by tests
}
}
// compute dependencies
List<PluginWrapper.Dependency> dependencies = new ArrayList<>();
List<PluginWrapper.Dependency> optionalDependencies = new ArrayList<>();
Expand Down
68 changes: 68 additions & 0 deletions core/src/main/java/hudson/PluginManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import static java.util.logging.Level.WARNING;
import static java.util.stream.Collectors.toList;

import com.google.common.annotations.VisibleForTesting;
import edu.umd.cs.findbugs.annotations.CheckForNull;
import edu.umd.cs.findbugs.annotations.NonNull;
import edu.umd.cs.findbugs.annotations.Nullable;
Expand Down Expand Up @@ -214,11 +215,18 @@
*/
/* private final */ static int CHECK_UPDATE_ATTEMPTS;

/**
* List of detached plugins that should not be installed as implied dependency.
*/
@VisibleForTesting
/* private */ static final List<String> IGNORE_DETACHED = new ArrayList<>();

static {
try {
// Secure initialization
CHECK_UPDATE_SLEEP_TIME_MILLIS = SystemProperties.getInteger(PluginManager.class.getName() + ".checkUpdateSleepTimeMillis", 1000);
CHECK_UPDATE_ATTEMPTS = SystemProperties.getInteger(PluginManager.class.getName() + ".checkUpdateAttempts", 1);
IGNORE_DETACHED.addAll(List.of(SystemProperties.getString(PluginManager.class.getName() + ".ignoreDetached", "").split(",")));
} catch (RuntimeException e) {
LOGGER.warning(String.format("There was an error initializing the PluginManager. Exception: %s", e));
} finally {
Expand Down Expand Up @@ -615,6 +623,23 @@
}});
}

@Restricted(NoExternalUse.class)
public boolean isImpliedToBeIgnored(String shortName, boolean register) {
if (new File(rootDir, shortName + ".jpi.uninstalled").isFile() || IGNORE_DETACHED.contains(shortName)) {
if (Jenkins.get().getInitLevel() == COMPLETED && register) {
LOGGER.log(INFO, () -> "not considering loading a detached dependency " + shortName + " as it is marked as uninstalled.");
DetachedPluginIgnoredMonitor monitor = AdministrativeMonitor.all().get(DetachedPluginIgnoredMonitor.class);
if (monitor != null) {

Check warning on line 632 in core/src/main/java/hudson/PluginManager.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 632 is only partially covered, one branch is missing
monitor.setActive(true, shortName);
}
} else {
LOGGER.log(FINE, () -> "not considering loading a detached dependency " + shortName + " as it is marked as uninstalled.");
}
return true;
}
return false;
}

void considerDetachedPlugin(String shortName, String source) {
if (new File(rootDir, shortName + ".jpi").isFile() ||
new File(rootDir, shortName + ".hpi").isFile() ||
Expand All @@ -623,6 +648,9 @@
LOGGER.fine(() -> "not considering loading a detached dependency " + shortName + " as it is already on disk");
return;
}
if (isImpliedToBeIgnored(shortName, true)) {
return;
}
LOGGER.fine(() -> "considering loading a detached dependency " + shortName);
for (String loadedFile : loadPluginsFromWar(getDetachedLocation(), (dir, name) -> normalisePluginName(name).equals(shortName))) {
String loaded = normalisePluginName(loadedFile);
Expand Down Expand Up @@ -2598,6 +2626,46 @@
}
}

@Restricted(NoExternalUse.class)
@Symbol("detachedPluginIgnored")
@Extension
public static final class DetachedPluginIgnoredMonitor extends AdministrativeMonitor {

private boolean active;
private Set<String> ignored = Collections.synchronizedSet(new HashSet<>());

@Override
public String getDisplayName() {
return Messages.PluginManager_DetachedPluginIgnoredMonitor_DisplayName();
}

@Override
public boolean isActivated() {
return active;
}

public void setActive(boolean active, String pluginName) {
this.active = active;
ignored.add(pluginName);
}

public Set<String> getIgnored() {
return Collections.unmodifiableSet(ignored);
}

@POST
public HttpResponse doAct(@QueryParameter String reset) throws IOException {
Jenkins.get().checkPermission(Jenkins.ADMINISTER);
if (reset != null) {
active = false;
ignored.clear();
} else {
disable(true);
}
return HttpResponses.redirectViaContextPath("/manage");

Check warning on line 2665 in core/src/main/java/hudson/PluginManager.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines

Lines 2653-2665 are not covered by tests
}
}

@Restricted(DoNotUse.class)
public String unscientific(double d) {
return String.format(Locale.US, "%15.4f", d);
Expand Down
9 changes: 8 additions & 1 deletion core/src/main/java/hudson/PluginWrapper.java
Original file line number Diff line number Diff line change
Expand Up @@ -1351,10 +1351,17 @@
@RequirePOST
public HttpResponse doDoUninstall() throws IOException {
Jenkins jenkins = Jenkins.get();

jenkins.checkPermission(Jenkins.ADMINISTER);
Files.deleteIfExists(Util.fileToPath(archive));
Files.deleteIfExists(Util.fileToPath(disableFile));
if (isDetached()) {
File uninstallFile = new File(archive.getAbsolutePath() + ".uninstalled");
try (OutputStream os = Files.newOutputStream(uninstallFile.toPath())) {
// creates an empty file
} catch (InvalidPathException e) {

Check warning on line 1361 in core/src/main/java/hudson/PluginWrapper.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 1361 is only partially covered, one branch is missing
throw new IOException(e);

Check warning on line 1362 in core/src/main/java/hudson/PluginWrapper.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered line

Line 1362 is not covered by tests
}
}

// Redo who depends on who.
jenkins.getPluginManager().resolveDependentPlugins();
Expand Down
4 changes: 3 additions & 1 deletion core/src/main/java/hudson/model/UpdateSite.java
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,9 @@ public final class Data {
if (!implicitDeps.isEmpty()) {
for (PluginWrapper.Dependency dep : implicitDeps) {
if (!p.dependencies.containsKey(dep.shortName)) {
p.dependencies.put(dep.shortName, dep.version);
if (!Jenkins.get().getPluginManager().isImpliedToBeIgnored(dep.shortName, false)) {
p.dependencies.put(dep.shortName, dep.version);
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/resources/hudson/Messages.properties
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ PluginManager.UpdateSiteError=Error checking update sites for {0} attempt(s). La
PluginManager.UpdateSiteChangeLogLevel=Change the log level of {0} logger to WARNING or below to see more information and the error message of every attempt
PluginManager.UnexpectedException=Unexpected exception going through the retrying process of checking update servers


PluginManager.compatWarning=\
Warning: The new version of this plugin is marked as incompatible with the installed version. \
This is usually the case because its behavior or APIs changed, or because it uses a different settings format than the installed version. \
Expand Down Expand Up @@ -102,6 +101,7 @@ PluginManager.emptyUpdateSiteUrl=\

PluginManager.connectionFailed=\
Unable to connect to the URL.
PluginManager.DetachedPluginIgnoredMonitor.DisplayName=Detached Plugin Ignored Monitor

AboutJenkins.DisplayName=About Jenkins
AboutJenkins.Description=See the version and license information.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:l="/lib/layout" xmlns:f="/lib/form" xmlns:i="jelly:fmt" xmlns:st="jelly:stapler">
${%blurb}
</j:jelly>
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
blurb = When a plugin is installed that has an implicit dependency which is ignored, this message is shown.
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<!--
The MIT License

Copyright (c) 2012, Dominik Bartholdi

Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
in the Software without restriction, including without limitation the rights
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
copies of the Software, and to permit persons to whom the Software is
furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in
all copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
THE SOFTWARE.
-->

<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form">
<div class="alert alert-warning">
<form method="post" action="${rootURL}/${it.url}/act" name="${it.id}">
<f:submit name="reset" value="${%Reset}"/>
<f:submit name="no" value="${%Dismiss}"/>
</form>
<dl>
<dt>${%blurb}</dt>
<j:forEach var="d" items="${it.ignored}">
<dd>${d}</dd>
</j:forEach>
</dl>
</div>
</j:jelly>
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
blurb=The following implied plugins have been ignored
10 changes: 9 additions & 1 deletion core/src/main/resources/hudson/PluginManager/_table.js
Original file line number Diff line number Diff line change
Expand Up @@ -484,9 +484,17 @@ window.addEventListener("load", function () {
uninstallButton.addEventListener("click", () => {
const title = uninstallButton.dataset.message;
const href = uninstallButton.dataset.href;
let message = i18n("uninstall-description");
const pluginRow = uninstallButton.closest(
"TR.possibly-has-implied-dependents",
);

if (pluginRow != null) {
message += "\n" + i18n("uninstall-detached-warning");
}

const options = {
message: i18n("uninstall-description"),
message: message,
type: "destructive",
};

Expand Down
4 changes: 3 additions & 1 deletion core/src/main/resources/hudson/PluginManager/installed.jelly
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,9 @@ THE SOFTWARE.
data-detached-disable="${%detached-disable}"
data-detached-uninstall="${%detached-uninstall}"
data-detached-possible-dependents="${%detached-possible-dependents}"
data-uninstall-description="${%uninstall-description}" />
data-uninstall-description="${%uninstall-description}"
data-uninstall-detached-warning="${%uninstall-detached-warning}"
/>

<j:choose>
<j:when test="${noPlugins}">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,8 @@ adoptThisPlugin=\
Visit our <a href="https://www.jenkins.io/doc/developer/plugin-governance/adopt-a-plugin/" rel="noopener noreferrer" target="_blank">Adopt a Plugin</a> initiative for more information.
reportIssue=Report an issue with this plugin
uninstall-title=Are you sure you want to uninstall {0}?
uninstall-description=This will remove the plugin binary from your $JENKINS_HOME, but it will leave the configuration files of the plugin untouched
uninstall-description=This will remove the plugin binary from your $JENKINS_HOME, but it will leave the configuration files of the plugin untouched.
uninstall-detached-warning=A marker file will be created to prevent that it is reinstalled again due to an implied dependency. If another plugin \
has a required dependency, it will be installed again. \
In case a problem occurs after installing another plugin that implicitly requires this plugin, it should be \
explicitly installed, which will remove the marker file.
14 changes: 14 additions & 0 deletions test/src/test/java/hudson/PluginManagerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -853,6 +853,20 @@ public void verifyUploadedPluginFromURLPermission() throws Exception {
assertEquals(EnumSet.of(OWNER_EXECUTE, OWNER_READ, OWNER_WRITE), filesPermission[0]);
}

@Issue("JENKINS-69487")
@Test public void impliedIsIgnoredWithSystemProperty() throws Exception {
PluginManager.IGNORE_DETACHED.add("javax-mail-api");
HtmlPage page = r.createWebClient().goTo("pluginManager/advanced");
HtmlForm f = page.getFormByName("uploadPlugin");
File dir = tmp.newFolder();
File plugin = new File(dir, "htmlpublisher.jpi");
FileUtils.copyURLToFile(getClass().getClassLoader().getResource("plugins/htmlpublisher.jpi"), plugin);
f.getInputByName("name").setValue(plugin.getAbsolutePath());
r.submit(f);
PluginWrapper pw = r.jenkins.pluginManager.getPlugin("javax-mail-api");
assertNull(pw);
}

static class AlertHandlerImpl implements AlertHandler {
List<String> messages = Collections.synchronizedList(new ArrayList<>());

Expand Down
43 changes: 43 additions & 0 deletions test/src/test/java/jenkins/install/LoadDetachedPluginsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.not;
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.ClassicPluginStrategy;
Expand All @@ -41,6 +43,7 @@
import hudson.PluginManagerUtil;
import hudson.PluginWrapper;
import hudson.util.VersionNumber;
import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;
Expand All @@ -53,18 +56,21 @@
import org.junit.Rule;
import org.junit.Test;
import org.junit.experimental.categories.Category;
import org.junit.rules.TemporaryFolder;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;
import org.jvnet.hudson.test.LoggerRule;
import org.jvnet.hudson.test.RestartableJenkinsRule;
import org.jvnet.hudson.test.SmokeTest;
import org.jvnet.hudson.test.recipes.LocalData;
import org.jvnet.hudson.test.recipes.WithPlugin;

@Category(SmokeTest.class)
public class LoadDetachedPluginsTest {

@Rule public RestartableJenkinsRule rr = PluginManagerUtil.newRestartableJenkinsRule();
@Rule public LoggerRule logging = new LoggerRule();
@Rule public TemporaryFolder tmp = new TemporaryFolder();

@Issue("JENKINS-48365")
@Test
Expand Down Expand Up @@ -163,6 +169,43 @@ public void nonstandardFilenames() {
});
}

@Issue("JENKINS-69487")
@WithPlugin("htmlpublisher.jpi")
@Test public void createUninstallStatusAfterUninstallDetached() throws Exception {
rr.then(r -> {
PluginWrapper pw = r.jenkins.pluginManager.getPlugin("javax-mail-api");
assertNotNull(pw);

pw.doDoUninstall();

File uninstalledMarker = new File(r.jenkins.getRootDir(), "plugins/javax-mail-api.jpi.uninstalled");
assertTrue(uninstalledMarker.exists()); // `.uninstalled` file should be created after uninstall of detached
});
rr.then(r -> {
PluginWrapper pw = r.jenkins.pluginManager.getPlugin("javax-mail-api");
assertNull(pw);

pw = r.jenkins.pluginManager.getPlugin("htmlpublisher");
assertNotNull(pw);

pw.doDoUninstall();

File uninstalledMarker = new File(r.jenkins.getRootDir(), "plugins/htmlpublisher.jpi.uninstalled");
assertFalse(uninstalledMarker.exists()); // `.uninstalled` file should not be created after uninstall of regular
});
}

@Issue("JENKINS-69487")
@Test public void uninstalledDetachedIsLoadedOnExplicit() throws Exception {
rr.then(r -> {
File uninstalledMarker = new File(r.jenkins.getRootDir(), "plugins/javax-activation-api.jpi.uninstalled");
uninstalledMarker.createNewFile();
PluginManagerUtil.dynamicLoad("javax-activation-api.jpi", r.jenkins);

assertFalse(uninstalledMarker.exists()); // `.uninstalled` file should be deleted after explicit install of detached
});
}

private List<PluginWrapper> getInstalledDetachedPlugins(JenkinsRule r, List<DetachedPlugin> detachedPlugins) {
PluginManager pluginManager = r.jenkins.getPluginManager();
List<PluginWrapper> installedPlugins = new ArrayList<>();
Expand Down
Binary file not shown.
Loading