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.

Sign up for GitHub

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

Stop cleaning the plugin directory by default + Add an opt-in flag for cleaning the target plugin directory + Restore the --list command #239

Merged
merged 11 commits into from
Dec 9, 2020
Merged

Stop cleaning the plugin directory by default + Add an opt-in flag for cleaning the target plugin directory + Restore the --list command #239

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
6bcc47d
Stop cleaning the plugin directory by default, add an opt-in flag for…
oleg-nenashev Dec 8, 2020
d6bdb3e
Ensure that the plugins get actually downloaded
oleg-nenashev Dec 8, 2020
d06b926
Update plugin download logic to use a temporary directory and to hand…
oleg-nenashev Dec 8, 2020
7643ab2
Move plugin files, do not just copy
oleg-nenashev Dec 8, 2020
add4faa
Apply suggestions from code review
oleg-nenashev Dec 8, 2020
921ec69
Fix test imports
oleg-nenashev Dec 8, 2020
18272ca
Revert to the old method make Mockito happy
oleg-nenashev Dec 8, 2020
4c9cdd3
Update Plugin Installation Manager, add smoke test for downloads and …
oleg-nenashev Dec 8, 2020
9fe8735
Mockito test is replaced bz the integration test
oleg-nenashev Dec 8, 2020
13e5200
Fix Checkstyle
oleg-nenashev Dec 9, 2020
cbcdbba
Apply suggestions from code review
oleg-nenashev Dec 9, 2020
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
10 changes: 10 additions & 0 deletions plugin-management-cli/src/main/java/io/jenkins/tools/pluginmanager/cli/CliOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ class CliOptions {
handler = FileOptionHandler.class)
private File pluginDir;

@Option(name = "--clean-download-directory",
usage = "If sets, cleans the plugin download directory before plugin installation. " +
"Otherwise the tool performs plugin download and reports compatibility issues, if any.")
private boolean cleanPluginDir;

@Option(name = "--plugins", aliases = {"-p"}, usage = "List of plugins to install, separated by a space",
handler = StringArrayOptionHandler.class)
private String[] plugins = new String[0];
Expand Down Expand Up @@ -144,6 +149,7 @@ Config setup() {
return Config.builder()
.withPlugins(getPlugins())
.withPluginDir(getPluginDir())
.withCleanPluginsDir(isCleanPluginDir())
.withJenkinsUc(getUpdateCenter())
.withJenkinsUcExperimental(getExperimentalUpdateCenter())
.withJenkinsIncrementalsRepoMirror(getIncrementalsMirror())
Expand Down Expand Up @@ -214,6 +220,10 @@ private File getPluginDir() {
return new File(Settings.DEFAULT_PLUGIN_DIR_LOCATION);
}

public boolean isCleanPluginDir() {
return cleanPluginDir;
}

@CheckForNull
private VersionNumber getJenkinsVersion() {
if (jenkinsVersion != null) {
Expand Down
6 changes: 0 additions & 6 deletions plugin-management-cli/src/main/java/io/jenkins/tools/pluginmanager/cli/Main.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,12 +67,6 @@ public static void main(String[] args) throws IOException {
return;
}

if (cfg.isShowPluginsToBeDownloaded()) {
System.out.println("The --list flag is currently unsafe and is temporarily disabled, " +
"see https://github.com/jenkinsci/plugin-installation-manager-tool/issues/173");
return;
}

pm.start();
} catch (Exception e) {
if (options.isVerbose()) {
Expand Down
14 changes: 14 additions & 0 deletions plugin-management-library/src/main/java/io/jenkins/tools/pluginmanager/config/Config.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
*/
public class Config {
private File pluginDir;
private boolean cleanPluginDir;
private boolean showWarnings;
private boolean showAllWarnings;
private boolean showAvailableUpdates;
Expand Down Expand Up @@ -49,6 +50,7 @@ public class Config {

private Config(
File pluginDir,
boolean cleanPluginDir,
boolean showWarnings,
boolean showAllWarnings,
boolean showAvailableUpdates,
Expand All @@ -67,6 +69,7 @@ private Config(
boolean skipFailedPlugins,
OutputFormat outputFormat) {
this.pluginDir = pluginDir;
this.cleanPluginDir = cleanPluginDir;
this.showWarnings = showWarnings;
this.showAllWarnings = showAllWarnings;
this.showAvailableUpdates = showAvailableUpdates;
Expand All @@ -90,6 +93,10 @@ public File getPluginDir() {
return pluginDir;
}

public boolean isCleanPluginDir() {
return cleanPluginDir;
}

public boolean isShowWarnings() {
return showWarnings;
}
Expand Down Expand Up @@ -165,6 +172,7 @@ public static Builder builder() {

public static class Builder {
private File pluginDir;
private boolean cleanPluginDir;
private boolean showWarnings;
private boolean showAllWarnings;
private boolean showAvailableUpdates;
Expand All @@ -191,6 +199,11 @@ public Builder withPluginDir(File pluginDir) {
return this;
}

public Builder withCleanPluginsDir(boolean cleanPluginDir) {
this.cleanPluginDir = cleanPluginDir;
return this;
}

public Builder withShowWarnings(boolean showWarnings) {
this.showWarnings = showWarnings;
return this;
Expand Down Expand Up @@ -284,6 +297,7 @@ public Builder withOutputFormat(OutputFormat outputFormat) {
public Config build() {
return new Config(
pluginDir,
cleanPluginDir,
showWarnings,
showAllWarnings,
showAvailableUpdates,
Expand Down
136 changes: 83 additions & 53 deletions ...n-management-library/src/main/java/io/jenkins/tools/pluginmanager/impl/PluginManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,9 @@
import java.net.URISyntaxException;
import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.nio.file.FileSystem;
import java.nio.file.FileSystems;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.PathMatcher;
import java.nio.file.Paths;
import java.nio.file.*;
import java.security.MessageDigest;
import java.util.ArrayList;
import java.util.Base64;
import java.util.Collections;
import java.util.Deque;
import java.util.HashMap;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.*;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.ForkJoinPool;
import java.util.jar.Attributes;
Expand All @@ -41,6 +28,8 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;
import javax.annotation.CheckForNull;

import io.jenkins.tools.pluginmanager.util.ManifestTools;
import org.apache.commons.codec.digest.DigestUtils;
import org.apache.commons.io.FileUtils;
import org.apache.commons.io.FilenameUtils;
Expand Down Expand Up @@ -68,7 +57,10 @@
public class PluginManager {
private static final VersionNumber LATEST = new VersionNumber("latest");
private List<Plugin> failedPlugins;
private File refDir;
/**
* Directory where the plugins will be downloaded
*/
private File pluginDir;
private String jenkinsUcLatest;
private @CheckForNull VersionNumber jenkinsVersion;
private @CheckForNull File jenkinsWarFile;
Expand Down Expand Up @@ -96,7 +88,7 @@ public class PluginManager {
@SuppressFBWarnings(value = "PATH_TRAVERSAL_IN", justification = "we want the user to be able to specify a path")
public PluginManager(Config cfg) {
this.cfg = cfg;
refDir = cfg.getPluginDir();
pluginDir = cfg.getPluginDir();
jenkinsVersion = cfg.getJenkinsVersion();
final String warArg = cfg.getJenkinsWar();
jenkinsWarFile = warArg != null ? new File(warArg) : null;
Expand Down Expand Up @@ -129,19 +121,20 @@ public void start() {
* @since TODO
*/
public void start(boolean downloadUc) {
if (refDir.exists()) {
if (cfg.isCleanPluginDir() && pluginDir.exists()) {
oleg-nenashev marked this conversation as resolved.
Show resolved Hide resolved
try {
File[] toBeDeleted = refDir.listFiles();
logVerbose("Cleaning up the target plugin directory: " + pluginDir);
File[] toBeDeleted = pluginDir.listFiles();
if (toBeDeleted != null) {
for (File deletableFile : toBeDeleted) {
FileUtils.forceDelete(deletableFile);
}
}
} catch (IOException e) {
throw new UncheckedIOException("Unable to delete: " + refDir.getAbsolutePath(), e);
throw new UncheckedIOException("Unable to delete: " + pluginDir.getAbsolutePath(), e);
}
}
createRefDir();
createPluginDir(cfg.isCleanPluginDir());

if (useLatestSpecified && useLatestAll) {
throw new PluginDependencyStrategyException("Only one plugin dependency version strategy can be selected " +
Expand Down Expand Up @@ -171,9 +164,19 @@ public void start(boolean downloadUc) {
System.out.println("Done");
}

void createRefDir() {
void createPluginDir(boolean failIfExists) {
if (pluginDir.exists()) {
if (failIfExists) {
throw new DirectoryCreationException("The plugin directory already exists: " + pluginDir);
} else {
if (!pluginDir.isDirectory()) {
throw new DirectoryCreationException("The plugin directory path is not a directory: " + pluginDir);
}
return;
}
}
try {
Files.createDirectories(refDir.toPath());
Files.createDirectories(pluginDir.toPath());
} catch (IOException e) {
throw new DirectoryCreationException("Unable to create plugin directory", e);
}
Expand Down Expand Up @@ -206,6 +209,8 @@ public List<Plugin> findPluginsToDownload(Map<String, Plugin> requestedPlugins)
installedPluginVersions.get(pluginName).getVersion();
}
if (installedVersion == null) {
logVerbose(String.format(
"Will install new plugin %s %s", pluginName, plugin.getVersion()));
pluginsToDownload.add(plugin);
} else if (installedVersion.isOlderThan(plugin.getVersion())) {
logVerbose(String.format(
Expand Down Expand Up @@ -449,15 +454,24 @@ public void checkVersionCompatibility(VersionNumber jenkinsVersion, List<Plugin>
}

/**
* Downloads a list of plugins
* Downloads a list of plugins.
* Plugins will be downloaded to a temporary directory, and then copied over to the final destination.
*
* @param plugins list of plugins to download
*/
public void downloadPlugins(List<Plugin> plugins) {
final File downloadsTmpDir;
try {
downloadsTmpDir = Files.createTempDirectory("plugin-installation-manager-downloads").toFile();
} catch (IOException ex) {
throw new DownloadPluginException("Cannot create a temporary directory for downloads", ex);
}

// Download to a temporary dir
ForkJoinPool ioThreadPool = new ForkJoinPool(64);
try {
ioThreadPool.submit(() -> plugins.parallelStream().forEach(plugin -> {
boolean successfulDownload = downloadPlugin(plugin, null);
boolean successfulDownload = downloadPlugin(plugin, new File(downloadsTmpDir, plugin.getArchiveFileName()));
if (skipFailedPlugins) {
System.out.println(
"SKIP: Unable to download " + plugin.getName());
Expand All @@ -474,6 +488,35 @@ public void downloadPlugins(List<Plugin> plugins) {
e.printStackTrace();
}
}

// Filter out failed plugins
final List<Plugin> failedPlugins = getFailedPlugins();
if (!skipFailedPlugins && failedPlugins.size() > 0) {
throw new DownloadPluginException("Some plugin downloads failed: " +
oleg-nenashev marked this conversation as resolved.
Show resolved Hide resolved
failedPlugins.stream().map(Plugin::getName).collect(Collectors.joining(",")));
}
Set<String> failedPluginNames = new HashSet<>(failedPlugins.size());
failedPlugins.forEach(plugin -> failedPluginNames.add(plugin.getName()));

// Copy files over to the destination directory
for (Plugin plugin : plugins) {
String archiveName = plugin.getArchiveFileName();
File downloadedPlugin = new File(downloadsTmpDir, archiveName);
try {
if (failedPluginNames.contains(plugin.getName())) {
System.out.println("Will skip the failed plugin download: " + plugin.getName());
Files.deleteIfExists(downloadedPlugin.toPath());
oleg-nenashev marked this conversation as resolved.
Show resolved Hide resolved
}
// We do not double-check overrides here, because findPluginsToDownload() has already done it
Files.move(downloadedPlugin.toPath(), new File(pluginDir, archiveName).toPath(), StandardCopyOption.REPLACE_EXISTING);
} catch (IOException ex) {
if (skipFailedPlugins) {
System.out.println("SKIP: Unable to move" + plugin.getName() + " to the plugin directory");
} else {
throw new DownloadPluginException("Unable to move" + plugin.getName() + " to the plugin directory", ex);
}
}
}
}

/**
Expand Down Expand Up @@ -705,6 +748,7 @@ public VersionNumber getLatestPluginVersion(String pluginName) {
* @return list of dependencies that were parsed from the plugin's manifest file
*/
public List<Plugin> resolveDependenciesFromManifest(Plugin plugin) {
// TODO(oleg_nenashev): refactor to use ManifestTools. This logic not only resolves dependencies, but also modifies the plugin's metadata
List<Plugin> dependentPlugins = new ArrayList<>();
try {
File tempFile = Files.createTempFile(FilenameUtils.getName(plugin.getName()), ".jpi").toFile();
Expand Down Expand Up @@ -909,11 +953,11 @@ private Map<String, Plugin> resolveRecursiveDependencies(Plugin plugin, @CheckFo
* resolved after the plugin is downloaded.
*
* @param plugin to download
* @param location location to download plugin to. If location is set to null, will download to the plugin folder
* @param location location to download plugin to. If location is set to {@code null}, will download to the plugin folder
* otherwise will download to the temporary location specified.
* @return boolean signifying if plugin was successful
*/
public boolean downloadPlugin(Plugin plugin, File location) {
public boolean downloadPlugin(Plugin plugin, @CheckForNull File location) {
String pluginName = plugin.getName();
VersionNumber pluginVersion = plugin.getVersion();
// location will be populated if downloading a plugin to a temp file to determine dependencies
Expand Down Expand Up @@ -995,7 +1039,7 @@ public String getPluginDownloadUrl(Plugin plugin) {
* be null
* @return true if download is successful, false otherwise
*/
public boolean downloadToFile(String urlString, Plugin plugin, File fileLocation) {
public boolean downloadToFile(String urlString, Plugin plugin, @CheckForNull File fileLocation) {
return downloadToFile(urlString, plugin, fileLocation, DEFAULT_MAX_RETRIES);
}

Expand All @@ -1011,10 +1055,10 @@ public boolean downloadToFile(String urlString, Plugin plugin, File fileLocation
* @return true if download is successful, false otherwise
*/
@SuppressFBWarnings({"RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE", "PATH_TRAVERSAL_IN"})
public boolean downloadToFile(String urlString, Plugin plugin, File fileLocation, int maxRetries) {
public boolean downloadToFile(String urlString, Plugin plugin, @CheckForNull File fileLocation, int maxRetries) {
File pluginFile;
if (fileLocation == null) {
pluginFile = new File(refDir, plugin.getArchiveFileName());
pluginFile = new File(pluginDir, plugin.getArchiveFileName());
System.out.println("\nDownloading plugin " + plugin.getName() + " from url: " + urlString);
} else {
pluginFile = fileLocation;
Expand Down Expand Up @@ -1126,28 +1170,6 @@ private byte[] calculateChecksum(File pluginFile) {
}
}

/**
* Given a jar file and a key to retrieve from the jar's MANIFEST.MF file, confirms that the file is a jar returns
* the value matching the key
*
* @param file jar file to get manifest from
* @param key key matching value to retrieve
* @return value matching the key in the jar file
*/
public String getAttributeFromManifest(File file, String key) {
try (JarFile jarFile = new JarFile(file)) {
Manifest manifest = jarFile.getManifest();
Attributes attributes = manifest.getMainAttributes();
return attributes.getValue(key);
} catch (IOException e) {
System.out.println("Unable to open " + file);
if (key.equals("Plugin-Dependencies")) {
throw new DownloadPluginException("Unable to determine plugin dependencies", e);
}
}
return null;
}

/**
* Gets Jenkins version using one of the available methods.
* @return Jenkins version or {@code null} if it cannot be determined
Expand Down Expand Up @@ -1199,6 +1221,14 @@ public String getPluginVersion(File file) {
return version;
}

/**
* @deprecated Use {@link ManifestTools#getAttributeFromManifest(File, String)}
*/
@Deprecated
public String getAttributeFromManifest(File file, String key) {
return getAttributeFromManifest(file, key);
}

/**
* Finds all the plugins and their versions currently in the plugin directory specified in the Config class
*
Expand All @@ -1209,7 +1239,7 @@ public Map<String, Plugin> installedPlugins() {
FileFilter fileFilter = new WildcardFileFilter("*.jpi");

// Only lists files in same directory, does not list files recursively
File[] files = refDir.listFiles(fileFilter);
File[] files = pluginDir.listFiles(fileFilter);

if (files != null) {
for (File file : files) {
Expand Down
Loading