diff --git a/lib/pluginmanager/bundler/logstash_uninstall.rb b/lib/pluginmanager/bundler/logstash_uninstall.rb index 11cfbefbc..3e32d9ece 100644 --- a/lib/pluginmanager/bundler/logstash_uninstall.rb +++ b/lib/pluginmanager/bundler/logstash_uninstall.rb @@ -34,54 +34,50 @@ module Bundler @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, {}) + def uninstall!(gems_to_remove) + gems_to_remove = Array(gems_to_remove) - 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 + 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 + 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) } + + builder = Dsl.new + 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, {}) + + # 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 + + return true + end end - def remove_gem(gem_name) - builder = Dsl.new - file = File.new(gemfile_path, "r+") - - gemfile = LogStash::Gemfile.new(file).load - gemfile.remove(gem_name) - builder.eval_gemfile(::File.join(::File.dirname(gemfile_path), "gemfile to changes"), gemfile.generate) - - definition = builder.to_definition(lockfile_path, {}) - definition.lock(lockfile_path) - gemfile.save - - LogStash::PluginManager.ui.info("Successfully removed #{gem_name}") - ensure - file.close if file - 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 @@ Failed to remove \"#{gem_name}\" because the following plugins or libraries depe 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 diff --git a/lib/pluginmanager/remove.rb b/lib/pluginmanager/remove.rb index e6c5d33d4..36a8592f3 100644 --- a/lib/pluginmanager/remove.rb +++ b/lib/pluginmanager/remove.rb @@ -20,48 +20,51 @@ require "pluginmanager/x_pack_interceptor.rb" 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 + + # 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("The plugin `#{plugin}` is provided by '#{integration_plugin.name}' so it can't be removed individually") + end + + 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 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 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 - - 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) - - local_gem = gemfile.locally_installed_gems.detect { |local_gem| local_gem.name == plugin } - signal_error(not_installed_message) unless local_gem - 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 diff --git a/qa/integration/fixtures/plugins/generate-gems.sh b/qa/integration/fixtures/plugins/generate-gems.sh new file mode 100755 index 000000000..2871367b2 --- /dev/null +++ b/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 \ No newline at end of file diff --git a/qa/integration/fixtures/plugins/logstash-filter-four_depends_on_one_and_three.gemspec b/qa/integration/fixtures/plugins/logstash-filter-four_depends_on_one_and_three.gemspec new file mode 100644 index 000000000..a2b36dfb7 --- /dev/null +++ b/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 diff --git a/qa/integration/fixtures/plugins/logstash-filter-one_no_dependencies.gemspec b/qa/integration/fixtures/plugins/logstash-filter-one_no_dependencies.gemspec new file mode 100644 index 000000000..edc854a29 --- /dev/null +++ b/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 diff --git a/qa/integration/fixtures/plugins/logstash-filter-three_no_dependencies.gemspec b/qa/integration/fixtures/plugins/logstash-filter-three_no_dependencies.gemspec new file mode 100644 index 000000000..edc854a29 --- /dev/null +++ b/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 diff --git a/qa/integration/fixtures/plugins/logstash-filter-two_depends_on_one.gemspec b/qa/integration/fixtures/plugins/logstash-filter-two_depends_on_one.gemspec new file mode 100644 index 000000000..e0c724ede --- /dev/null +++ b/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 diff --git a/qa/integration/fixtures/plugins/logstash-filter-zero_no_dependencies.gemspec b/qa/integration/fixtures/plugins/logstash-filter-zero_no_dependencies.gemspec new file mode 100644 index 000000000..edc854a29 --- /dev/null +++ b/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 diff --git a/qa/integration/services/logstash_service.rb b/qa/integration/services/logstash_service.rb index 1a915ca33..b4881807a 100644 --- a/qa/integration/services/logstash_service.rb +++ b/qa/integration/services/logstash_service.rb @@ -20,6 +20,7 @@ require_relative "monitoring_api" require "childprocess" require "bundler" require "socket" +require "shellwords" require "tempfile" require 'yaml' @@ -337,8 +338,9 @@ class LogstashService < 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,12 +353,16 @@ class LogstashService < Service 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) @@ -364,7 +370,7 @@ class LogstashService < Service 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 diff --git a/qa/integration/specs/cli/remove_spec.rb b/qa/integration/specs/cli/remove_spec.rb index 91aba8cf7..efc64f1be 100644 --- a/qa/integration/specs/cli/remove_spec.rb +++ b/qa/integration/specs/cli/remove_spec.rb @@ -22,7 +22,7 @@ require_relative '../../framework/helpers' require "logstash/devutils/rspec/spec_helper" describe "CLI > logstash-plugin remove" do - before(:all) do + before(:each) do @fixture = Fixture.new(__FILE__) @logstash_plugin = @fixture.get_service("logstash").plugin_cli end @@ -109,5 +109,131 @@ describe "CLI > logstash-plugin remove" do expect(presence_check.stderr_and_stdout).to match(/logstash-codec-json/) end end + + context "multiple plugins" do + + let(:setup_plugin_list) do + fail("spec must override `setup_plugin_list`") + end + + before(:each) do + if setup_plugin_list.any? + search_dir = File.expand_path(File.join(__dir__, "..", "..", "fixtures", "plugins")) + plugin_paths = [] + + aggregate_failures('setup: resolve plugin paths') do + setup_plugin_list.each do |requested_plugin| + found = Dir.glob(File.join(search_dir, "#{requested_plugin}-*.gem")) + expect(found).to have_attributes(:size => 1), lambda { "expected exactly one `#{requested_plugin}` in `#{search_dir}`, got #{found.inspect}" } + plugin_paths << found.first + end + end + + aggregate_failures('setup: installing plugins') do + puts "installing plugins #{plugin_paths.inspect}" + outcome = @logstash_plugin.install(*plugin_paths) + + expect(outcome.exit_code).to eq(0) + expect(outcome.stderr_and_stdout).to match(/Installation successful/) + end + end + end + + context "when a remaining plugin has a dependency on a removed plugin" do + let(:setup_plugin_list) do + %w( + logstash-filter-zero_no_dependencies + logstash-filter-one_no_dependencies + logstash-filter-two_depends_on_one + logstash-filter-three_no_dependencies + logstash-filter-four_depends_on_one_and_three + ) + end + it "errors helpfully without removing any of the plugins" do + execute = @logstash_plugin.remove("logstash-filter-three_no_dependencies", "logstash-filter-zero_no_dependencies") + + expect(execute.exit_code).to eq(1) + expect(execute.stderr_and_stdout).to include('Failed to remove "logstash-filter-three_no_dependencies"') + expect(execute.stderr_and_stdout).to include("* logstash-filter-four_depends_on_one_and_three") # one of the dependency + expect(execute.stderr_and_stdout).to include("No plugins were removed.") + + aggregate_failures("list plugins") do + presence_check = @logstash_plugin.list + expect(presence_check.exit_code).to eq(0) + expect(presence_check.stderr_and_stdout).to include('logstash-filter-three_no_dependencies') + expect(presence_check.stderr_and_stdout).to include('logstash-filter-zero_no_dependencies') + end + end + end + context "when multiple remaining plugins have a dependency on a removed plugin" do + let(:setup_plugin_list) do + %w( + logstash-filter-zero_no_dependencies + logstash-filter-one_no_dependencies + logstash-filter-two_depends_on_one + logstash-filter-three_no_dependencies + logstash-filter-four_depends_on_one_and_three + ) + end + it "errors helpfully without removing any of the plugins" do + execute = @logstash_plugin.remove("logstash-filter-one_no_dependencies", "logstash-filter-zero_no_dependencies") + + expect(execute.exit_code).to eq(1) + expect(execute.stderr_and_stdout).to include('Failed to remove "logstash-filter-one_no_dependencies"') + expect(execute.stderr_and_stdout).to include("* logstash-filter-four_depends_on_one_and_three") # one of the dependency + expect(execute.stderr_and_stdout).to include("* logstash-filter-two_depends_on_one") # one of the dependency + expect(execute.stderr_and_stdout).to include("No plugins were removed.") + + aggregate_failures("list plugins") do + presence_check = @logstash_plugin.list + expect(presence_check.exit_code).to eq(0) + expect(presence_check.stderr_and_stdout).to include('logstash-filter-one_no_dependencies') + expect(presence_check.stderr_and_stdout).to include('logstash-filter-zero_no_dependencies') + end + end + end + context "when removing plugins and all plugins that depend on them" do + let(:setup_plugin_list) do + %w( + logstash-filter-zero_no_dependencies + logstash-filter-one_no_dependencies + logstash-filter-two_depends_on_one + logstash-filter-three_no_dependencies + logstash-filter-four_depends_on_one_and_three + ) + end + it "removes the plugins" do + plugins_to_remove = %w( + logstash-filter-one_no_dependencies + logstash-filter-two_depends_on_one + logstash-filter-three_no_dependencies + logstash-filter-four_depends_on_one_and_three + ).shuffle #random order + execute = @logstash_plugin.remove(*plugins_to_remove) + + aggregate_failures("removal action") do + expect(execute).to have_attributes(:exit_code => 0, :stderr_and_stdout => include("Success")) + plugins_to_remove.each do |gem_name| + expect(execute.stderr_and_stdout).to include("Successfully removed #{gem_name}") + end + end + + aggregate_failures("list plugins") do + presence_check = @logstash_plugin.list + expect(presence_check.exit_code).to eq(0) + aggregate_failures("removed plugins") do + plugins_to_remove.each do |expected_removed_plugin| + expect(presence_check.stderr_and_stdout).to_not include(expected_removed_plugin) + end + end + aggregate_failures("non-removed plugins") do + (setup_plugin_list - plugins_to_remove).each do |expected_remaining_plugin| + expect(presence_check.stderr_and_stdout).to include(expected_remaining_plugin) + end + end + end + end + end + end end end diff --git a/rakelib/plugin.rake b/rakelib/plugin.rake index ffd7a1cbf..a41c8fa1e 100644 --- a/rakelib/plugin.rake +++ b/rakelib/plugin.rake @@ -24,9 +24,9 @@ namespace "plugin" do LogStash::PluginManager::Main.run("bin/logstash-plugin", ["install"] + args) end - def remove_plugin(plugin) + def remove_plugin(plugin, *more_plugins) require_relative "../lib/pluginmanager/main" - LogStash::PluginManager::Main.run("bin/logstash-plugin", ["remove", plugin]) + LogStash::PluginManager::Main.run("bin/logstash-plugin", ["remove", plugin] + more_plugins) end task "install-base" => "bootstrap" do