Skip to content

Commit b31ed6a

Browse files
authored
Plugins: Add plugin cli specific exit codes (#23599)
We currently use POSIX exit codes in all of our CLIs. However, posix only suggests these exit codes are standard across tools. It does not prescribe particular uses for codes outside of that range. This commit adds 2 exit codes specific to plugin installation to make distinguishing an incorrectly built plugin and a plugin already existing easier. closes #15295
1 parent 111e703 commit b31ed6a

File tree

2 files changed

+31
-18
lines changed

2 files changed

+31
-18
lines changed

core/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java

Lines changed: 30 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,13 @@ class InstallPluginCommand extends EnvironmentAwareCommand {
105105

106106
private static final String PROPERTY_STAGING_ID = "es.plugins.staging";
107107

108+
// exit codes for install
109+
/** A plugin with the same name is already installed. */
110+
static final int PLUGIN_EXISTS = 1;
111+
/** The plugin zip is not properly structured. */
112+
static final int PLUGIN_MALFORMED = 2;
113+
114+
108115
/** The builtin modules, which are plugins, but cannot be installed or removed. */
109116
static final Set<String> MODULES;
110117
static {
@@ -333,7 +340,8 @@ private Path downloadZipAndChecksum(Terminal terminal, String urlString, Path tm
333340
byte[] zipbytes = Files.readAllBytes(zip);
334341
String gotChecksum = MessageDigests.toHexString(MessageDigests.sha1().digest(zipbytes));
335342
if (expectedChecksum.equals(gotChecksum) == false) {
336-
throw new UserException(ExitCodes.IO_ERROR, "SHA1 mismatch, expected " + expectedChecksum + " but got " + gotChecksum);
343+
throw new UserException(ExitCodes.IO_ERROR,
344+
"SHA1 mismatch, expected " + expectedChecksum + " but got " + gotChecksum);
337345
}
338346

339347
return zip;
@@ -357,12 +365,14 @@ private Path unzip(Path zip, Path pluginsDir) throws IOException, UserException
357365
hasEsDir = true;
358366
Path targetFile = target.resolve(entry.getName().substring("elasticsearch/".length()));
359367

360-
// Using the entry name as a path can result in an entry outside of the plugin dir, either if the
361-
// name starts with the root of the filesystem, or it is a relative entry like ../whatever.
362-
// This check attempts to identify both cases by first normalizing the path (which removes foo/..)
363-
// and ensuring the normalized entry is still rooted with the target plugin directory.
368+
// Using the entry name as a path can result in an entry outside of the plugin dir,
369+
// either if the name starts with the root of the filesystem, or it is a relative
370+
// entry like ../whatever. This check attempts to identify both cases by first
371+
// normalizing the path (which removes foo/..) and ensuring the normalized entry
372+
// is still rooted with the target plugin directory.
364373
if (targetFile.normalize().startsWith(target) == false) {
365-
throw new IOException("Zip contains entry name '" + entry.getName() + "' resolving outside of plugin directory");
374+
throw new UserException(PLUGIN_MALFORMED, "Zip contains entry name '" +
375+
entry.getName() + "' resolving outside of plugin directory");
366376
}
367377

368378
// be on the safe side: do not rely on that directories are always extracted
@@ -384,7 +394,8 @@ private Path unzip(Path zip, Path pluginsDir) throws IOException, UserException
384394
Files.delete(zip);
385395
if (hasEsDir == false) {
386396
IOUtils.rm(target);
387-
throw new UserException(ExitCodes.DATA_ERROR, "`elasticsearch` directory is missing in the plugin zip");
397+
throw new UserException(PLUGIN_MALFORMED,
398+
"`elasticsearch` directory is missing in the plugin zip");
388399
}
389400
return target;
390401
}
@@ -424,19 +435,20 @@ private PluginInfo verify(Terminal terminal, Path pluginRoot, boolean isBatch, E
424435
if (Files.exists(destination)) {
425436
final String message = String.format(
426437
Locale.ROOT,
427-
"plugin directory [%s] already exists; if you need to update the plugin, uninstall it first using command 'remove %s'",
438+
"plugin directory [%s] already exists; if you need to update the plugin, " +
439+
"uninstall it first using command 'remove %s'",
428440
destination.toAbsolutePath(),
429441
info.getName());
430-
throw new UserException(ExitCodes.CONFIG, message);
442+
throw new UserException(PLUGIN_EXISTS, message);
431443
}
432444

433445
terminal.println(VERBOSE, info.toString());
434446

435447
// don't let user install plugin as a module...
436448
// they might be unavoidably in maven central and are packaged up the same way)
437449
if (MODULES.contains(info.getName())) {
438-
throw new UserException(
439-
ExitCodes.USAGE, "plugin '" + info.getName() + "' cannot be installed like this, it is a system module");
450+
throw new UserException(ExitCodes.USAGE, "plugin '" + info.getName() +
451+
"' cannot be installed like this, it is a system module");
440452
}
441453

442454
// check for jar hell before any copying
@@ -534,17 +546,16 @@ public FileVisitResult visitFile(Path pluginFile, BasicFileAttributes attrs) thr
534546
/** Copies the files from {@code tmpBinDir} into {@code destBinDir}, along with permissions from dest dirs parent. */
535547
private void installBin(PluginInfo info, Path tmpBinDir, Path destBinDir) throws Exception {
536548
if (Files.isDirectory(tmpBinDir) == false) {
537-
throw new UserException(ExitCodes.IO_ERROR, "bin in plugin " + info.getName() + " is not a directory");
549+
throw new UserException(PLUGIN_MALFORMED, "bin in plugin " + info.getName() + " is not a directory");
538550
}
539551
Files.createDirectory(destBinDir);
540552
setFileAttributes(destBinDir, BIN_DIR_PERMS);
541553

542554
try (DirectoryStream<Path> stream = Files.newDirectoryStream(tmpBinDir)) {
543555
for (Path srcFile : stream) {
544556
if (Files.isDirectory(srcFile)) {
545-
throw new UserException(
546-
ExitCodes.DATA_ERROR,
547-
"Directories not allowed in bin dir for plugin " + info.getName() + ", found " + srcFile.getFileName());
557+
throw new UserException(PLUGIN_MALFORMED, "Directories not allowed in bin dir " +
558+
"for plugin " + info.getName() + ", found " + srcFile.getFileName());
548559
}
549560

550561
Path destFile = destBinDir.resolve(tmpBinDir.relativize(srcFile));
@@ -561,7 +572,8 @@ private void installBin(PluginInfo info, Path tmpBinDir, Path destBinDir) throws
561572
*/
562573
private void installConfig(PluginInfo info, Path tmpConfigDir, Path destConfigDir) throws Exception {
563574
if (Files.isDirectory(tmpConfigDir) == false) {
564-
throw new UserException(ExitCodes.IO_ERROR, "config in plugin " + info.getName() + " is not a directory");
575+
throw new UserException(PLUGIN_MALFORMED,
576+
"config in plugin " + info.getName() + " is not a directory");
565577
}
566578

567579
Files.createDirectories(destConfigDir);
@@ -577,7 +589,8 @@ private void installConfig(PluginInfo info, Path tmpConfigDir, Path destConfigDi
577589
try (DirectoryStream<Path> stream = Files.newDirectoryStream(tmpConfigDir)) {
578590
for (Path srcFile : stream) {
579591
if (Files.isDirectory(srcFile)) {
580-
throw new UserException(ExitCodes.DATA_ERROR, "Directories not allowed in config dir for plugin " + info.getName());
592+
throw new UserException(PLUGIN_MALFORMED,
593+
"Directories not allowed in config dir for plugin " + info.getName());
581594
}
582595

583596
Path destFile = destConfigDir.resolve(tmpConfigDir.relativize(srcFile));

qa/evil-tests/src/test/java/org/elasticsearch/plugins/InstallPluginCommandTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -595,7 +595,7 @@ public void testZipRelativeOutsideEntryName() throws Exception {
595595
stream.putNextEntry(new ZipEntry("elasticsearch/../blah"));
596596
}
597597
String pluginZip = zip.toUri().toURL().toString();
598-
IOException e = expectThrows(IOException.class, () -> installPlugin(pluginZip, env.v1()));
598+
UserException e = expectThrows(UserException.class, () -> installPlugin(pluginZip, env.v1()));
599599
assertTrue(e.getMessage(), e.getMessage().contains("resolving outside of plugin directory"));
600600
}
601601

0 commit comments

Comments
 (0)