Skip to content

Commit e73db34

Browse files
committed
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 1bcbac9 commit e73db34

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
@@ -104,6 +104,13 @@ class InstallPluginCommand extends EnvironmentAwareCommand {
104104

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

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

336344
return zip;
@@ -354,12 +362,14 @@ private Path unzip(Path zip, Path pluginsDir) throws IOException, UserException
354362
hasEsDir = true;
355363
Path targetFile = target.resolve(entry.getName().substring("elasticsearch/".length()));
356364

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

365375
// be on the safe side: do not rely on that directories are always extracted
@@ -381,7 +391,8 @@ private Path unzip(Path zip, Path pluginsDir) throws IOException, UserException
381391
Files.delete(zip);
382392
if (hasEsDir == false) {
383393
IOUtils.rm(target);
384-
throw new UserException(ExitCodes.DATA_ERROR, "`elasticsearch` directory is missing in the plugin zip");
394+
throw new UserException(PLUGIN_MALFORMED,
395+
"`elasticsearch` directory is missing in the plugin zip");
385396
}
386397
return target;
387398
}
@@ -421,19 +432,20 @@ private PluginInfo verify(Terminal terminal, Path pluginRoot, boolean isBatch, E
421432
if (Files.exists(destination)) {
422433
final String message = String.format(
423434
Locale.ROOT,
424-
"plugin directory [%s] already exists; if you need to update the plugin, uninstall it first using command 'remove %s'",
435+
"plugin directory [%s] already exists; if you need to update the plugin, " +
436+
"uninstall it first using command 'remove %s'",
425437
destination.toAbsolutePath(),
426438
info.getName());
427-
throw new UserException(ExitCodes.CONFIG, message);
439+
throw new UserException(PLUGIN_EXISTS, message);
428440
}
429441

430442
terminal.println(VERBOSE, info.toString());
431443

432444
// don't let user install plugin as a module...
433445
// they might be unavoidably in maven central and are packaged up the same way)
434446
if (MODULES.contains(info.getName())) {
435-
throw new UserException(
436-
ExitCodes.USAGE, "plugin '" + info.getName() + "' cannot be installed like this, it is a system module");
447+
throw new UserException(ExitCodes.USAGE, "plugin '" + info.getName() +
448+
"' cannot be installed like this, it is a system module");
437449
}
438450

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

539551
try (DirectoryStream<Path> stream = Files.newDirectoryStream(tmpBinDir)) {
540552
for (Path srcFile : stream) {
541553
if (Files.isDirectory(srcFile)) {
542-
throw new UserException(
543-
ExitCodes.DATA_ERROR,
544-
"Directories not allowed in bin dir for plugin " + info.getName() + ", found " + srcFile.getFileName());
554+
throw new UserException(PLUGIN_MALFORMED, "Directories not allowed in bin dir " +
555+
"for plugin " + info.getName() + ", found " + srcFile.getFileName());
545556
}
546557

547558
Path destFile = destBinDir.resolve(tmpBinDir.relativize(srcFile));
@@ -558,7 +569,8 @@ private void installBin(PluginInfo info, Path tmpBinDir, Path destBinDir) throws
558569
*/
559570
private void installConfig(PluginInfo info, Path tmpConfigDir, Path destConfigDir) throws Exception {
560571
if (Files.isDirectory(tmpConfigDir) == false) {
561-
throw new UserException(ExitCodes.IO_ERROR, "config in plugin " + info.getName() + " is not a directory");
572+
throw new UserException(PLUGIN_MALFORMED,
573+
"config in plugin " + info.getName() + " is not a directory");
562574
}
563575

564576
Files.createDirectories(destConfigDir);
@@ -574,7 +586,8 @@ private void installConfig(PluginInfo info, Path tmpConfigDir, Path destConfigDi
574586
try (DirectoryStream<Path> stream = Files.newDirectoryStream(tmpConfigDir)) {
575587
for (Path srcFile : stream) {
576588
if (Files.isDirectory(srcFile)) {
577-
throw new UserException(ExitCodes.DATA_ERROR, "Directories not allowed in config dir for plugin " + info.getName());
589+
throw new UserException(PLUGIN_MALFORMED,
590+
"Directories not allowed in config dir for plugin " + info.getName());
578591
}
579592

580593
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
@@ -596,7 +596,7 @@ public void testZipRelativeOutsideEntryName() throws Exception {
596596
stream.putNextEntry(new ZipEntry("elasticsearch/../blah"));
597597
}
598598
String pluginZip = zip.toUri().toURL().toString();
599-
IOException e = expectThrows(IOException.class, () -> installPlugin(pluginZip, env.v1()));
599+
UserException e = expectThrows(UserException.class, () -> installPlugin(pluginZip, env.v1()));
600600
assertTrue(e.getMessage(), e.getMessage().contains("resolving outside of plugin directory"));
601601
}
602602

0 commit comments

Comments
 (0)