plugins: improve remove command to support multiple plugins

lib/pluginmanager/bundler/logstash_uninstall.rb

@@ -34,54 +34,50 @@ def initialize(gemfile_path, lockfile_path)
       @lockfile_path = lockfile_path
     end
 
-    # To be uninstalled the candidate gems need to be standalone.
-    def dependants_gems(gem_name)
-      builder = Dsl.new
-      builder.eval_gemfile(::File.join(::File.dirname(gemfile_path), "original gemfile"), File.read(gemfile_path))
-      definition = builder.to_definition(lockfile_path, {})
-
-      definition.specs
-        .select { |spec| spec.dependencies.collect(&:name).include?(gem_name) }
-        .collect(&:name).sort.uniq
-    end
-
-    def uninstall!(gem_name)
-      unfreeze_gemfile do
-        dependencies_from = dependants_gems(gem_name)
-
-        if dependencies_from.size > 0
-          display_cant_remove_message(gem_name, dependencies_from)
-          false
-        else
-          remove_gem(gem_name)
-          true
+    def uninstall!(gems_to_remove)
+      gems_to_remove = Array(gems_to_remove)
+
+      unsatisfied_dependency_mapping = Dsl.evaluate(gemfile_path, lockfile_path, {}).specs.each_with_object({}) do |spec, memo|
+        next if gems_to_remove.include?(spec.name)
+        deps = spec.runtime_dependencies.collect(&:name)
+        deps.intersection(gems_to_remove).each do |missing_dependency|
+          memo[missing_dependency] ||= []
+          memo[missing_dependency] << spec.name
         end
       end
-    end
+      if unsatisfied_dependency_mapping.any?
+        unsatisfied_dependency_mapping.each { |gem_to_remove, gems_depending_on_removed| display_cant_remove_message(gem_to_remove, gems_depending_on_removed) }
+        LogStash::PluginManager.ui.info("No plugins were removed.")
+        return false
+      end
+
+      with_mutable_gemfile do |gemfile|
+        gems_to_remove.each { |gem_name| gemfile.remove(gem_name) }
 
-    def remove_gem(gem_name)
-      builder = Dsl.new
-      file = File.new(gemfile_path, "r+")
+        builder = Dsl.new
+        builder.eval_gemfile(::File.join(::File.dirname(gemfile_path), "gemfile to changes"), gemfile.generate)
 
-      gemfile = LogStash::Gemfile.new(file).load
-      gemfile.remove(gem_name)
-      builder.eval_gemfile(::File.join(::File.dirname(gemfile_path), "gemfile to changes"), gemfile.generate)
+        # build a definition, providing an intentionally-empty "unlock" mapping
+        # to ensure that all gem versions remain locked
+        definition = builder.to_definition(lockfile_path, {})
 
-      definition = builder.to_definition(lockfile_path, {})
-      definition.lock(lockfile_path)
-      gemfile.save
+        # lock the definition and save our modified gemfile
+        definition.lock(lockfile_path)
+        gemfile.save
+
+        gems_to_remove.each do |gem_name|
+          LogStash::PluginManager.ui.info("Successfully removed #{gem_name}")
+        end
 
-      LogStash::PluginManager.ui.info("Successfully removed #{gem_name}")
-    ensure
-      file.close if file
+        return true
+      end
     end
 
     def display_cant_remove_message(gem_name, dependencies_from)
-        message = <<-eos
-Failed to remove \"#{gem_name}\" because the following plugins or libraries depend on it:
-
-* #{dependencies_from.join("\n* ")}
-        eos
+        message = <<~EOS
+          Failed to remove \"#{gem_name}\" because the following plugins or libraries depend on it:
+          * #{dependencies_from.join("\n* ")}
+        EOS
         LogStash::PluginManager.ui.info(message)
     end
 
@@ -93,10 +89,22 @@ def unfreeze_gemfile
       end
     end
 
-    def self.uninstall!(gem_name, options = { :gemfile => LogStash::Environment::GEMFILE, :lockfile => LogStash::Environment::LOCKFILE })
-      gemfile_path = options[:gemfile]
-      lockfile_path = options[:lockfile]
-      LogstashUninstall.new(gemfile_path, lockfile_path).uninstall!(gem_name)
+    ##
+    # Yields a mutable gemfile backed by an open, writable file handle.
+    # It is the responsibility of the caller to send `LogStash::Gemfile#save` to persist the result.
+    # @yieldparam [LogStash::Gemfile]
+    def with_mutable_gemfile
+      unfreeze_gemfile do
+        File.open(gemfile_path, 'r+') do |file|
+          yield LogStash::Gemfile.new(file).load
+        end
+      end
+    end
+
+    def self.uninstall!(gem_names, options={})
+      gemfile_path = options[:gemfile] || LogStash::Environment::GEMFILE
+      lockfile_path = options[:lockfile] || LogStash::Environment::LOCKFILE
+      LogstashUninstall.new(gemfile_path, lockfile_path).uninstall!(Array(gem_names))
     end
   end
 end

lib/pluginmanager/remove.rb

@@ -20,48 +20,51 @@
 require "pluginmanager/command"
 
 class LogStash::PluginManager::Remove < LogStash::PluginManager::Command
-  parameter "PLUGIN", "plugin name"
+  parameter "PLUGIN ...", "plugin name(s)"
 
   def execute
+    signal_error("One or more plugins must be specified") if plugin_list.empty?
     signal_error("File #{LogStash::Environment::GEMFILE_PATH} does not exist or is not writable, aborting") unless File.writable?(LogStash::Environment::GEMFILE_PATH)
 
     LogStash::Bundler.prepare({:without => [:build, :development]})
 
-    if LogStash::PluginManager::ALIASES.has_key?(plugin)
-      unless LogStash::PluginManager.installed_plugin?(plugin, gemfile)
-        aliased_plugin = LogStash::PluginManager::ALIASES[plugin]
-        puts "Cannot remove the alias #{plugin}, which is an alias for #{aliased_plugin}; if you wish to remove it, you must remove the aliased plugin instead."
-        return
+    plugin_list.each do |plugin|
+      if LogStash::PluginManager::ALIASES.has_key?(plugin)
+        unless LogStash::PluginManager.installed_plugin?(plugin, gemfile)
+          aliased_plugin = LogStash::PluginManager::ALIASES[plugin]
+          puts "Cannot remove the alias #{plugin}, which is an alias for #{aliased_plugin}; if you wish to remove it, you must remove the aliased plugin instead."
+          return
+        end
       end
-    end
 
-    # If a user is attempting to uninstall X-Pack, present helpful output to guide
-    # them toward the OSS-only distribution of Logstash
-    LogStash::PluginManager::XPackInterceptor::Remove.intercept!(plugin)
+      # If a user is attempting to uninstall X-Pack, present helpful output to guide
+      # them toward the OSS-only distribution of Logstash
+      LogStash::PluginManager::XPackInterceptor::Remove.intercept!(plugin)
 
-    # if the plugin is provided by an integration plugin. abort.
-    if integration_plugin = LogStash::PluginManager.which_integration_plugin_provides(plugin, gemfile)
-      signal_error("This plugin is already provided by '#{integration_plugin.name}' so it can't be removed individually")
-    end
+      # if the plugin is provided by an integration plugin. abort.
+      if integration_plugin = LogStash::PluginManager.which_integration_plugin_provides(plugin, gemfile)
+        signal_error("The plugin `#{plugin}` is provided by '#{integration_plugin.name}' so it can't be removed individually")
+      end
 
-    not_installed_message = "This plugin has not been previously installed"
-    plugin_gem_spec = LogStash::PluginManager.find_plugins_gem_specs(plugin).any?
-    if plugin_gem_spec
-      # make sure this is an installed plugin and present in Gemfile.
-      # it is not possible to uninstall a dependency not listed in the Gemfile, for example a dependent codec
-      signal_error(not_installed_message) unless LogStash::PluginManager.installed_plugin?(plugin, gemfile)
-    else
-      # locally installed plugins are not recoginized by ::Gem::Specification
-      # we may ::Bundler.setup to reload but it resets all dependencies so we get error message for future bundler operations
-      # error message: `Bundler::GemNotFound: Could not find rubocop-1.60.2... in locally installed gems`
-      # instead we make sure Gemfile has a definition and ::Gem::Specification recognizes local gem
-      signal_error(not_installed_message) unless !!gemfile.find(plugin)
+      not_installed_message = "The plugin `#{plugin}` has not been previously installed"
+      plugin_gem_spec = LogStash::PluginManager.find_plugins_gem_specs(plugin).any?
+      if plugin_gem_spec
+        # make sure this is an installed plugin and present in Gemfile.
+        # it is not possible to uninstall a dependency not listed in the Gemfile, for example a dependent codec
+        signal_error(not_installed_message) unless LogStash::PluginManager.installed_plugin?(plugin, gemfile)
+      else
+        # locally installed plugins are not recoginized by ::Gem::Specification
+        # we may ::Bundler.setup to reload but it resets all dependencies so we get error message for future bundler operations
+        # error message: `Bundler::GemNotFound: Could not find rubocop-1.60.2... in locally installed gems`
+        # instead we make sure Gemfile has a definition and ::Gem::Specification recognizes local gem
+        signal_error(not_installed_message) unless !!gemfile.find(plugin)
 
-      local_gem = gemfile.locally_installed_gems.detect { |local_gem| local_gem.name == plugin }
-      signal_error(not_installed_message) unless local_gem
+        local_gem = gemfile.locally_installed_gems.detect { |local_gem| local_gem.name == plugin }
+        signal_error(not_installed_message) unless local_gem
+      end
     end
 
-    exit(1) unless ::Bundler::LogstashUninstall.uninstall!(plugin)
+    exit(1) unless ::Bundler::LogstashUninstall.uninstall!(plugin_list)
     LogStash::Bundler.genericize_platform
     remove_unused_locally_installed_gems!
   rescue => exception

qa/integration/fixtures/plugins/generate-gems.sh

@@ -0,0 +1,4 @@
+#!/usr/bin/env sh
+
+cd "$( dirname "$0" )"
+find . -name '*.gemspec' | xargs -n1 gem build

qa/integration/fixtures/plugins/logstash-filter-four_depends_on_one_and_three.gemspec

@@ -0,0 +1,26 @@
+Gem::Specification.new do |s|
+  s.name            = File.basename(__FILE__, ".gemspec")
+  s.version         = '0.1.1'
+  s.licenses        = ['Apache-2.0']
+  s.summary         = "A dummy plugin with two plugin dependencies"
+  s.description     = "This plugin is only used in the acceptance test"
+  s.authors         = ["Elasticsearch"]
+  s.email           = 'info@elasticsearch.com'
+  s.homepage        = "http://www.elasticsearch.org/guide/en/logstash/current/index.html"
+  s.require_paths = ["lib"]
+
+  # Files
+  s.files = [__FILE__]
+
+  # Tests
+  s.test_files = []
+
+  # Special flag to let us know this is actually a logstash plugin
+  s.metadata = { "logstash_plugin" => "true", "logstash_group" => "filter" }
+
+  # Gem dependencies
+  s.add_runtime_dependency "logstash-core-plugin-api", ">= 1.60", "<= 2.99"
+
+  s.add_runtime_dependency "logstash-filter-one_no_dependencies", "~> 0.1"
+  s.add_runtime_dependency "logstash-filter-three_no_dependencies", "~> 0.1"
+end

qa/integration/fixtures/plugins/logstash-filter-one_no_dependencies.gemspec

@@ -0,0 +1,23 @@
+Gem::Specification.new do |s|
+  s.name            = File.basename(__FILE__, ".gemspec")
+  s.version         = '0.1.1'
+  s.licenses        = ['Apache-2.0']
+  s.summary         = "A dummy plugin with no dependencies"
+  s.description     = "This plugin is only used in the acceptance test"
+  s.authors         = ["Elasticsearch"]
+  s.email           = 'info@elasticsearch.com'
+  s.homepage        = "http://www.elasticsearch.org/guide/en/logstash/current/index.html"
+  s.require_paths = ["lib"]
+
+  # Files
+  s.files = [__FILE__]
+
+  # Tests
+  s.test_files = []
+
+  # Special flag to let us know this is actually a logstash plugin
+  s.metadata = { "logstash_plugin" => "true", "logstash_group" => "filter" }
+
+  # Gem dependencies
+  s.add_runtime_dependency "logstash-core-plugin-api", ">= 1.60", "<= 2.99"
+end

qa/integration/fixtures/plugins/logstash-filter-three_no_dependencies.gemspec

@@ -0,0 +1,23 @@
+Gem::Specification.new do |s|
+  s.name            = File.basename(__FILE__, ".gemspec")
+  s.version         = '0.1.1'
+  s.licenses        = ['Apache-2.0']
+  s.summary         = "A dummy plugin with no dependencies"
+  s.description     = "This plugin is only used in the acceptance test"
+  s.authors         = ["Elasticsearch"]
+  s.email           = 'info@elasticsearch.com'
+  s.homepage        = "http://www.elasticsearch.org/guide/en/logstash/current/index.html"
+  s.require_paths = ["lib"]
+
+  # Files
+  s.files = [__FILE__]
+
+  # Tests
+  s.test_files = []
+
+  # Special flag to let us know this is actually a logstash plugin
+  s.metadata = { "logstash_plugin" => "true", "logstash_group" => "filter" }
+
+  # Gem dependencies
+  s.add_runtime_dependency "logstash-core-plugin-api", ">= 1.60", "<= 2.99"
+end

qa/integration/fixtures/plugins/logstash-filter-two_depends_on_one.gemspec

@@ -0,0 +1,25 @@
+Gem::Specification.new do |s|
+  s.name            = File.basename(__FILE__, ".gemspec")
+  s.version         = '0.1.1'
+  s.licenses        = ['Apache-2.0']
+  s.summary         = "A dummy plugin with one plugin dependency"
+  s.description     = "This plugin is only used in the acceptance test"
+  s.authors         = ["Elasticsearch"]
+  s.email           = 'info@elasticsearch.com'
+  s.homepage        = "http://www.elasticsearch.org/guide/en/logstash/current/index.html"
+  s.require_paths = ["lib"]
+
+  # Files
+  s.files = [__FILE__]
+
+  # Tests
+  s.test_files = []
+
+  # Special flag to let us know this is actually a logstash plugin
+  s.metadata = { "logstash_plugin" => "true", "logstash_group" => "filter" }
+
+  # Gem dependencies
+  s.add_runtime_dependency "logstash-core-plugin-api", ">= 1.60", "<= 2.99"
+
+  s.add_runtime_dependency "logstash-filter-one_no_dependencies", "~> 0.1"
+end

qa/integration/fixtures/plugins/logstash-filter-zero_no_dependencies.gemspec

@@ -0,0 +1,23 @@
+Gem::Specification.new do |s|
+  s.name            = File.basename(__FILE__, ".gemspec")
+  s.version         = '0.1.1'
+  s.licenses        = ['Apache-2.0']
+  s.summary         = "A dummy plugin with no dependencies"
+  s.description     = "This plugin is only used in the acceptance test"
+  s.authors         = ["Elasticsearch"]
+  s.email           = 'info@elasticsearch.com'
+  s.homepage        = "http://www.elasticsearch.org/guide/en/logstash/current/index.html"
+  s.require_paths = ["lib"]
+
+  # Files
+  s.files = [__FILE__]
+
+  # Tests
+  s.test_files = []
+
+  # Special flag to let us know this is actually a logstash plugin
+  s.metadata = { "logstash_plugin" => "true", "logstash_group" => "filter" }
+
+  # Gem dependencies
+  s.add_runtime_dependency "logstash-core-plugin-api", ">= 1.60", "<= 2.99"
+end

qa/integration/services/logstash_service.rb

@@ -20,6 +20,7 @@
 require "childprocess"
 require "bundler"
 require "socket"
+require "shellwords"
 require "tempfile"
 require 'yaml'
 
@@ -337,8 +338,9 @@ def initialize(logstash_service)
       @logstash_plugin = File.join(@logstash.logstash_home, LOGSTASH_PLUGIN)
     end
 
-    def remove(plugin_name)
-      run("remove #{plugin_name}")
+    def remove(plugin_name, *additional_plugins)
+      plugin_list = Shellwords.shelljoin([plugin_name]+additional_plugins)
+      run("remove #{plugin_list}")
     end
 
     def prepare_offline_pack(plugins, output_zip = nil)
@@ -351,20 +353,24 @@ def prepare_offline_pack(plugins, output_zip = nil)
       end
     end
 
-    def list(plugin_name, verbose = false)
-      run("list #{plugin_name} #{verbose ? "--verbose" : ""}")
+    def list(*plugins, verbose: false)
+      command = "list"
+      command << " --verbose" if verbose
+      command << " #{Shellwords.shelljoin(plugins)}" if plugins.any?
+      run(command)
     end
 
-    def install(plugin_name)
-      run("install #{plugin_name}")
+    def install(plugin_name, *additional_plugins)
+      plugin_list = ([plugin_name]+additional_plugins).flatten
+      run("install #{Shellwords.shelljoin(plugin_list)}")
     end
 
     def run(command)
       run_raw("#{logstash_plugin} #{command}")
     end
 
     def run_raw(cmd, change_dir = true, environment = {})
-      @logstash.run_cmd(cmd.split(' '), change_dir, environment)
+      @logstash.run_cmd(Shellwords.shellsplit(cmd), change_dir, environment)
     end
   end
 end