Skip to content

Commit 77c0fba

Browse files
committed
Remove silent batch mode from install plugin (#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 e9050d3 commit 77c0fba

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
@@ -498,7 +498,7 @@ class ClusterFormationTasks {
498498
* the short name requiring the path to already exist.
499499
*/
500500
final Object esPluginUtil = "${-> node.binPath().resolve('elasticsearch-plugin').toString()}"
501-
final Object[] args = [esPluginUtil, 'install', file]
501+
final Object[] args = [esPluginUtil, 'install', '--batch', file]
502502
return configureExecTask(name, project, setup, node, args)
503503
}
504504

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
@@ -408,7 +408,7 @@ fi
408408

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

421421
local relativePath=${1:-$(readlink -m custom-settings-*.zip)}
422-
sudo -E -u $ESPLUGIN_COMMAND_USER ES_JAVA_OPTS="-Des.logger.level=DEBUG" "$ESHOME/bin/elasticsearch-plugin" install "file://$relativePath" > /tmp/plugin-cli-output
422+
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
423423
local loglines=$(cat /tmp/plugin-cli-output | grep -v "^[[:cntrl:]]" | wc -l)
424424
[ "$loglines" -gt "2" ] || {
425425
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)