Skip to content

Commit 5cdd831

Browse files
authored
Remove silent batch mode from install plugin (elastic#29359)
Today we have a silent batch mode in the install plugin command when standard input is closed or there is no tty. It appears that historically this was useful when running tests where we want to accept plugin permissions without having to acknowledge them. Now that we have an explicit batch mode flag, this use-case is removed. The motivation for removing this now is that there is another place where silent batch mode arises and that is when a user attempts to install a plugin inside a Docker container without keeping standard input open and attaching a tty. In this case, the install plugin command will treat the situation as a silent batch mode and therefore the user will never have the chance to acknowledge the additional permissions required by a plugin. This commit removes this silent batch mode in favor of using the --batch flag when running tests and requiring the user to take explicit action to acknowledge the additional permissions (either by leaving standard input open and attaching a tty, or by passing the --batch flags themselves). Note that with this change the user will now see a null pointer exception when they try to install a plugin in a Docker container without keeping standard input open and attaching a tty. This will be addressed in an immediate follow-up, but because the implications of that change are larger, they should be handled separately from this one.
1 parent 8fdca6a commit 5cdd831

File tree

4 files changed

+6
-6
lines changed

4 files changed

+6
-6
lines changed

buildSrc/src/main/groovy/org/elasticsearch/gradle/test/ClusterFormationTasks.groovy

+1-1
Original file line numberDiff line numberDiff line change
@@ -494,7 +494,7 @@ class ClusterFormationTasks {
494494
* the short name requiring the path to already exist.
495495
*/
496496
final Object esPluginUtil = "${-> node.binPath().resolve('elasticsearch-plugin').toString()}"
497-
final Object[] args = [esPluginUtil, 'install', file]
497+
final Object[] args = [esPluginUtil, 'install', '--batch', file]
498498
return configureExecTask(name, project, setup, node, args)
499499
}
500500

distribution/tools/plugin-cli/src/main/java/org/elasticsearch/plugins/InstallPluginCommand.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ protected void printAdditionalHelp(Terminal terminal) {
208208
@Override
209209
protected void execute(Terminal terminal, OptionSet options, Environment env) throws Exception {
210210
String pluginId = arguments.value(options);
211-
boolean isBatch = options.has(batchOption) || System.console() == null;
211+
final boolean isBatch = options.has(batchOption);
212212
execute(terminal, pluginId, isBatch, env);
213213
}
214214

qa/vagrant/src/test/resources/packaging/tests/module_and_plugin_test_cases.bash

+2-2
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,7 @@ fi
416416

417417
@test "[$GROUP] install a sample plugin with different logging modes and check output" {
418418
local relativePath=${1:-$(readlink -m custom-settings-*.zip)}
419-
sudo -E -u $ESPLUGIN_COMMAND_USER "$ESHOME/bin/elasticsearch-plugin" install "file://$relativePath" > /tmp/plugin-cli-output
419+
sudo -E -u $ESPLUGIN_COMMAND_USER "$ESHOME/bin/elasticsearch-plugin" install --batch "file://$relativePath" > /tmp/plugin-cli-output
420420
# exclude progress line
421421
local loglines=$(cat /tmp/plugin-cli-output | grep -v "^[[:cntrl:]]" | wc -l)
422422
[ "$loglines" -eq "2" ] || {
@@ -427,7 +427,7 @@ fi
427427
remove_plugin_example
428428

429429
local relativePath=${1:-$(readlink -m custom-settings-*.zip)}
430-
sudo -E -u $ESPLUGIN_COMMAND_USER ES_JAVA_OPTS="-Des.logger.level=DEBUG" "$ESHOME/bin/elasticsearch-plugin" install "file://$relativePath" > /tmp/plugin-cli-output
430+
sudo -E -u $ESPLUGIN_COMMAND_USER ES_JAVA_OPTS="-Des.logger.level=DEBUG" "$ESHOME/bin/elasticsearch-plugin" install --batch "file://$relativePath" > /tmp/plugin-cli-output
431431
local loglines=$(cat /tmp/plugin-cli-output | grep -v "^[[:cntrl:]]" | wc -l)
432432
[ "$loglines" -gt "2" ] || {
433433
echo "Expected more than 2 lines excluding progress bar but the output had $loglines lines and was:"

qa/vagrant/src/test/resources/packaging/utils/plugins.bash

+2-2
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,9 @@ install_plugin() {
4747
fi
4848

4949
if [ -z "$umask" ]; then
50-
sudo -E -u $ESPLUGIN_COMMAND_USER "$ESHOME/bin/elasticsearch-plugin" install -batch "file://$path"
50+
sudo -E -u $ESPLUGIN_COMMAND_USER "$ESHOME/bin/elasticsearch-plugin" install --batch "file://$path"
5151
else
52-
sudo -E -u $ESPLUGIN_COMMAND_USER bash -c "umask $umask && \"$ESHOME/bin/elasticsearch-plugin\" install -batch \"file://$path\""
52+
sudo -E -u $ESPLUGIN_COMMAND_USER bash -c "umask $umask && \"$ESHOME/bin/elasticsearch-plugin\" install --batch \"file://$path\""
5353
fi
5454

5555
#check we did not accidentially create a log file as root as /usr/share/elasticsearch

0 commit comments

Comments
 (0)