From e65bf33302613d59551e4e40a130ddbc6a6b4298 Mon Sep 17 00:00:00 2001 From: Ry Biesemeyer Date: Tue, 4 Mar 2025 10:59:35 -0800 Subject: [PATCH 1/2] Pluginmanager clean after mutate (#17203) * pluginmanager: always clean after mutate * pluginmanager: don't skip updating plugins installed with --version * pr feedback (cherry picked from commit 8c969138073b98de7a99daf3c62527e5758711f9) --- lib/pluginmanager/command.rb | 25 +++++ lib/pluginmanager/gemfile.rb | 4 +- lib/pluginmanager/install.rb | 1 + lib/pluginmanager/remove.rb | 1 + lib/pluginmanager/update.rb | 4 +- qa/integration/fixtures/update_spec.yml | 3 + qa/integration/services/logstash_service.rb | 50 ++++++++- qa/integration/specs/cli/install_spec.rb | 102 +++++++++++++++++- .../specs/cli/pluginmanager_spec_helper.rb | 59 ++++++++++ qa/integration/specs/cli/remove_spec.rb | 43 +++++++- qa/integration/specs/cli/update_spec.rb | 65 +++++++++++ 11 files changed, 345 insertions(+), 12 deletions(-) create mode 100644 qa/integration/fixtures/update_spec.yml create mode 100644 qa/integration/specs/cli/pluginmanager_spec_helper.rb create mode 100644 qa/integration/specs/cli/update_spec.rb diff --git a/lib/pluginmanager/command.rb b/lib/pluginmanager/command.rb index 8fd700db564..5603dc383a1 100644 --- a/lib/pluginmanager/command.rb +++ b/lib/pluginmanager/command.rb @@ -47,6 +47,31 @@ def remove_unused_locally_installed_gems! end end + def remove_orphan_dependencies! + locked_gem_names = ::Bundler::LockfileParser.new(File.read(LogStash::Environment::LOCKFILE)).specs.map(&:full_name).to_set + orphan_gem_specs = ::Gem::Specification.each + .reject(&:stubbed?) # skipped stubbed (uninstalled) gems + .reject(&:default_gem?) # don't touch jruby-included default gems + .reject{ |spec| locked_gem_names.include?(spec.full_name) } + .sort + + inactive_plugins, orphaned_dependencies = orphan_gem_specs.partition { |spec| LogStash::PluginManager.logstash_plugin_gem_spec?(spec) } + + # uninstall plugins first, to limit damage should one fail to uninstall + inactive_plugins.each { |plugin| uninstall_gem!("inactive plugin", plugin) } + orphaned_dependencies.each { |dep| uninstall_gem!("orphaned dependency", dep) } + end + + def uninstall_gem!(desc, spec) + require "rubygems/uninstaller" + Gem::DefaultUserInteraction.use_ui(debug? ? Gem::DefaultUserInteraction.ui : Gem::SilentUI.new) do + Gem::Uninstaller.new(spec.name, version: spec.version, force: true, executables: true).uninstall + end + puts "cleaned #{desc} #{spec.name} (#{spec.version})" + rescue Gem::InstallError => e + report_exception("Failed to uninstall `#{spec.full_name}`", e) + end + def relative_path(path) require "pathname" ::Pathname.new(path).relative_path_from(::Pathname.new(LogStash::Environment::LOGSTASH_HOME)).to_s diff --git a/lib/pluginmanager/gemfile.rb b/lib/pluginmanager/gemfile.rb index 6e658b762d8..7e295a9fcf1 100644 --- a/lib/pluginmanager/gemfile.rb +++ b/lib/pluginmanager/gemfile.rb @@ -133,8 +133,8 @@ def add_gem(_gem) # update existing or add new def update_gem(_gem) if old = find_gem(_gem.name) - # always overwrite requirements if specified - old.requirements = _gem.requirements unless no_constrains?(_gem.requirements) + # always overwrite requirements + old.requirements = _gem.requirements # but merge options old.options = old.options.merge(_gem.options) else diff --git a/lib/pluginmanager/install.rb b/lib/pluginmanager/install.rb index 085b6430214..e1a3513987d 100644 --- a/lib/pluginmanager/install.rb +++ b/lib/pluginmanager/install.rb @@ -79,6 +79,7 @@ def execute install_gems_list!(gems) remove_unused_locally_installed_gems! remove_unused_integration_overlaps! + remove_orphan_dependencies! end private diff --git a/lib/pluginmanager/remove.rb b/lib/pluginmanager/remove.rb index 36a8592f326..48d832abcd3 100644 --- a/lib/pluginmanager/remove.rb +++ b/lib/pluginmanager/remove.rb @@ -67,6 +67,7 @@ def execute exit(1) unless ::Bundler::LogstashUninstall.uninstall!(plugin_list) LogStash::Bundler.genericize_platform remove_unused_locally_installed_gems! + remove_orphan_dependencies! rescue => exception report_exception("Operation aborted, cannot remove plugin", exception) end diff --git a/lib/pluginmanager/update.rb b/lib/pluginmanager/update.rb index 76130bfa584..91306bc1e85 100644 --- a/lib/pluginmanager/update.rb +++ b/lib/pluginmanager/update.rb @@ -95,10 +95,8 @@ def update_gems! output << LogStash::Bundler.genericize_platform unless output.nil? end - # We currently dont removed unused gems from the logstash installation - # see: https://github.com/elastic/logstash/issues/6339 - # output = LogStash::Bundler.invoke!(:clean => true) display_updated_plugins(previous_gem_specs_map) + remove_orphan_dependencies! rescue => exception gemfile.restore! report_exception("Updated Aborted", exception) diff --git a/qa/integration/fixtures/update_spec.yml b/qa/integration/fixtures/update_spec.yml new file mode 100644 index 00000000000..cbfc784af81 --- /dev/null +++ b/qa/integration/fixtures/update_spec.yml @@ -0,0 +1,3 @@ +--- +services: + - logstash diff --git a/qa/integration/services/logstash_service.rb b/qa/integration/services/logstash_service.rb index a42bb95a967..d8750571e2b 100644 --- a/qa/integration/services/logstash_service.rb +++ b/qa/integration/services/logstash_service.rb @@ -308,7 +308,7 @@ def run_cmd(cmd_args, change_dir = true, environment = {}) if ENV.key?("BUILD_JAVA_HOME") && !process.environment.key?("LS_JAVA_HOME") process.environment["LS_JAVA_HOME"] = ENV["BUILD_JAVA_HOME"] end - process.io.stdout = process.io.stderr = out + process.io.stdout = process.io.stderr = SynchronizedDelegate.new(out) Bundler.with_unbundled_env do if change_dir @@ -329,6 +329,31 @@ def run(*args) run_cmd [@logstash_bin, *args] end + ## + # A `SynchronizedDelegate` wraps any object and ensures that exactly one + # calling thread is invoking methods on it at a time. This is useful for our + # clumsy setting of process io STDOUT and STDERR to the same IO object, which + # can cause interleaved writes. + class SynchronizedDelegate + def initialize(obj) + require "monitor" + @mon = Monitor.new + @obj = obj + end + + def respond_to_missing?(method_name, include_private = false) + @obj.respond_to?(method_name, include_private) || super + end + + def method_missing(method_name, *args, &block) + return super unless @obj.respond_to?(method_name) + + @mon.synchronize do + @obj.public_send(method_name, *args, &block) + end + end + end + class PluginCli LOGSTASH_PLUGIN = File.join("bin", "logstash-plugin") @@ -362,9 +387,26 @@ def list(*plugins, verbose: false) run(command) end - def install(plugin_name, *additional_plugins) - plugin_list = ([plugin_name]+additional_plugins).flatten - run("install #{Shellwords.shelljoin(plugin_list)}") + def install(plugin_name, *additional_plugins, version: nil, verify: true, preserve: false, local: false) + args = [] + args << "--no-verify" unless verify + args << "--preserve" if preserve + args << "--local" if local + args << "--version" << version unless version.nil? + args.concat(([plugin_name]+additional_plugins).flatten) + + run("install #{Shellwords.shelljoin(args)}") + end + + def update(*plugin_list, level: nil, local: nil, verify: nil, conservative: nil) + args = [] + args << (verify ? "--verify" : "--no-verify") unless verify.nil? + args << "--level" << "#{level}" unless level.nil? + args << "--local" if local + args << (conservative ? "--conservative" : "--no-conservative") unless conservative.nil? + args.concat(plugin_list) + + run("update #{Shellwords.shelljoin(args)}") end def run(command) diff --git a/qa/integration/specs/cli/install_spec.rb b/qa/integration/specs/cli/install_spec.rb index 89a97973c6e..c3b55f926e3 100644 --- a/qa/integration/specs/cli/install_spec.rb +++ b/qa/integration/specs/cli/install_spec.rb @@ -19,6 +19,7 @@ require_relative "../../framework/settings" require_relative "../../services/logstash_service" require_relative "../../framework/helpers" +require_relative "pluginmanager_spec_helper" require "logstash/devutils/rspec/spec_helper" require "stud/temporary" require "fileutils" @@ -29,16 +30,21 @@ def gem_in_lock_file?(pattern, lock_file) content.match(pattern) end +def plugin_filename_re(name, version) + %Q(\b#{Regexp.escape name}-#{Regexp.escape version}(-java)?\b) +end + # Bundler can mess up installation successful output: https://github.com/elastic/logstash/issues/15801 INSTALL_SUCCESS_RE = /IB?nstall successful/ INSTALLATION_SUCCESS_RE = /IB?nstallation successful/ +INSTALLATION_ABORTED_RE = /Installation aborted/ + describe "CLI > logstash-plugin install" do - before(:all) do + before(:each) do @fixture = Fixture.new(__FILE__) @logstash = @fixture.get_service("logstash") @logstash_plugin = @logstash.plugin_cli - @pack_directory = File.expand_path(File.join(File.dirname(__FILE__), "..", "..", "fixtures", "logstash-dummy-pack")) end shared_examples "install from a pack" do @@ -46,6 +52,10 @@ def gem_in_lock_file?(pattern, lock_file) let(:install_command) { "bin/logstash-plugin install" } let(:change_dir) { true } + before(:all) do + @pack_directory = File.expand_path(File.join(File.dirname(__FILE__), "..", "..", "fixtures", "logstash-dummy-pack")) + end + # When you are on anything by linux we won't disable the internet with seccomp if RbConfig::CONFIG["host_os"] == "linux" context "without internet connection (linux seccomp wrapper)" do @@ -152,4 +162,92 @@ def gem_in_lock_file?(pattern, lock_file) end end end + + context "rubygems hosted plugin" do + include_context "pluginmanager validation helpers" + shared_examples("overwriting existing") do + before(:each) do + aggregate_failures("precheck") do + expect("#{plugin_name}-#{existing_plugin_version}").to_not be_installed_gem + expect("#{plugin_name}-#{specified_plugin_version}").to_not be_installed_gem + end + aggregate_failures("setup") do + execute = @logstash_plugin.install(plugin_name, version: existing_plugin_version) + + expect(execute.stderr_and_stdout).to match(INSTALLATION_SUCCESS_RE) + expect(execute.exit_code).to eq(0) + + expect("#{plugin_name}-#{existing_plugin_version}").to be_installed_gem + expect("#{plugin_name}-#{specified_plugin_version}").to_not be_installed_gem + end + end + it "installs the specified version and removes the pre-existing one" do + execute = @logstash_plugin.install(plugin_name, version: specified_plugin_version) + + aggregate_failures("command execution") do + expect(execute.stderr_and_stdout).to match(INSTALLATION_SUCCESS_RE) + expect(execute.exit_code).to eq(0) + end + + installed = @logstash_plugin.list(plugin_name, verbose: true) + expect(installed.stderr_and_stdout).to match(/#{Regexp.escape plugin_name} [(]#{Regexp.escape(specified_plugin_version)}[)]/) + + expect("#{plugin_name}-#{existing_plugin_version}").to_not be_installed_gem + expect("#{plugin_name}-#{specified_plugin_version}").to be_installed_gem + end + end + + context "when installing over an older version" do + let(:plugin_name) { "logstash-filter-qatest" } + let(:existing_plugin_version) { "0.1.0" } + let(:specified_plugin_version) { "0.1.1" } + + include_examples "overwriting existing" + end + + context "when installing over a newer version" do + let(:plugin_name) { "logstash-filter-qatest" } + let(:existing_plugin_version) { "0.1.0" } + let(:specified_plugin_version) { "0.1.1" } + + include_examples "overwriting existing" + end + + context "installing plugin that isn't present" do + it "installs the plugin" do + aggregate_failures("prevalidation") do + expect("logstash-filter-qatest").to_not be_installed_gem + end + + execute = @logstash_plugin.install("logstash-filter-qatest") + + expect(execute.stderr_and_stdout).to match(INSTALLATION_SUCCESS_RE) + expect(execute.exit_code).to eq(0) + + installed = @logstash_plugin.list("logstash-filter-qatest") + expect(installed.stderr_and_stdout).to match(/logstash-filter-qatest/) + expect(installed.exit_code).to eq(0) + + expect(gem_in_lock_file?(/logstash-filter-qatest/, @logstash.lock_file)).to be_truthy + + expect("logstash-filter-qatest").to be_installed_gem + end + end + context "installing plugin that doesn't exist on rubygems" do + it "doesn't install anything" do + execute = @logstash_plugin.install("logstash-filter-404-no-exist") + + expect(execute.stderr_and_stdout).to match(INSTALLATION_ABORTED_RE) + expect(execute.exit_code).to eq(1) + end + end + context "installing gem that isn't a plugin" do + it "doesn't install anything" do + execute = @logstash_plugin.install("dummy_gem") + + expect(execute.stderr_and_stdout).to match(INSTALLATION_ABORTED_RE) + expect(execute.exit_code).to eq(1) + end + end + end end diff --git a/qa/integration/specs/cli/pluginmanager_spec_helper.rb b/qa/integration/specs/cli/pluginmanager_spec_helper.rb new file mode 100644 index 00000000000..6270f7933c1 --- /dev/null +++ b/qa/integration/specs/cli/pluginmanager_spec_helper.rb @@ -0,0 +1,59 @@ +require 'pathname' + +shared_context "pluginmanager validation helpers" do + + matcher :be_installed_gem do + match do |actual| + common(actual) + @gemspec_present && @gem_installed + end + + match_when_negated do |actual| + common(actual) + !@gemspec_present && !@gem_installed + end + + define_method :common do |actual| + version_suffix = /-[0-9.]+(-java)?$/ + filename_matcher = actual.match?(version_suffix) ? actual : /^#{Regexp.escape(actual)}#{version_suffix}/ + + @gems = (logstash_gemdir / "gems").glob("*-*") + @gemspecs = (logstash_gemdir / "specifications").glob("*-*.gemspec") + + @gem_installed = @gems.find { |gem| gem.basename.to_s.match?(filename_matcher) } + @gemspec_present = @gemspecs.find { |gemspec| gemspec.basename(".gemspec").to_s.match?(filename_matcher) } + end + + failure_message do |actual| + reasons = [] + reasons << "the gem dir could not be found (#{@gems})" unless @gem_installed + reasons << "the gemspec could not be found (#{@gemspecs})" unless @gemspec_present + + "expected that #{actual} would be installed, but #{reasons.join(' and ')}" + end + failure_message_when_negated do |actual| + reasons = [] + reasons << "the gem dir is present (#{@gem_installed})" if @gem_installed + reasons << "the gemspec is present (#{@gemspec_present})" if @gemspec_present + + "expected that #{actual} would not be installed, but #{reasons.join(' and ')}" + end + end + + def logstash_home + return super() if defined?(super) + return @logstash.logstash_home if @logstash + fail("no @logstash, so we can't get logstash_home") + end + + def logstash_gemdir + pathname_base = (Pathname.new(logstash_home) / "vendor" / "bundle" / "jruby") + candidate_dirs = pathname_base.glob("[0-9]*") + case candidate_dirs.size + when 0 then fail("no version dir found in #{pathname_base}") + when 1 then candidate_dirs.first + else + fail("multiple version dirs found in #{pathname_base} (#{candidate_dirs.map(&:basename)}") + end + end +end \ No newline at end of file diff --git a/qa/integration/specs/cli/remove_spec.rb b/qa/integration/specs/cli/remove_spec.rb index b3d914f6565..bf168aafc4f 100644 --- a/qa/integration/specs/cli/remove_spec.rb +++ b/qa/integration/specs/cli/remove_spec.rb @@ -19,12 +19,17 @@ require_relative '../../framework/settings' require_relative '../../services/logstash_service' require_relative '../../framework/helpers' +require_relative "pluginmanager_spec_helper" require "logstash/devutils/rspec/spec_helper" describe "CLI > logstash-plugin remove" do + + include_context "pluginmanager validation helpers" + before(:each) do @fixture = Fixture.new(__FILE__) - @logstash_plugin = @fixture.get_service("logstash").plugin_cli + @logstash = @fixture.get_service("logstash") + @logstash_plugin = @logstash.plugin_cli end if RbConfig::CONFIG["host_os"] == "linux" @@ -55,6 +60,8 @@ presence_check = @logstash_plugin.list(test_plugin) expect(presence_check.exit_code).to eq(1) expect(presence_check.stderr_and_stdout).to match(/ERROR: No plugins found/) + + expect("logstash-filter-qatest").to_not be_installed_gem end end @@ -92,6 +99,38 @@ presence_check = @logstash_plugin.list(test_plugin) expect(presence_check.exit_code).to eq(1) expect(presence_check.stderr_and_stdout).to match(/ERROR: No plugins found/) + + expect("logstash-filter-qatest").to_not be_installed_gem + end + end + + context "plugins with unshared dependencies" do + let(:plugin_to_remove) { } + + it "successfully removes the plugin and its unshared dependencies" do + execute = @logstash_plugin.remove("logstash-integration-aws") + + expect(execute.exit_code).to eq(0) + expect(execute.stderr_and_stdout).to match(/Successfully removed logstash-integration-aws/) + + expect("logstash-integration-aws").to_not be_installed_gem + + # known unshared dependencies, including transitive dependencies + aggregate_failures("known unshared dependencies") do + expect("aws-sdk-core").to_not be_installed_gem + expect("aws-sdk-s3").to_not be_installed_gem + expect("aws-sdk-kms").to_not be_installed_gem + expect("aws-sdk-cloudfront").to_not be_installed_gem + expect("aws-sdk-cloudwatch").to_not be_installed_gem + expect("aws-eventstream").to_not be_installed_gem + expect("aws-partitions").to_not be_installed_gem + end + + # known shared dependencies + aggregate_failures("known shared dependencies") do + expect("concurrent-ruby").to be_installed_gem + expect("logstash-codec-json").to be_installed_gem + end end end @@ -108,6 +147,8 @@ expect(presence_check.exit_code).to eq(0) expect(presence_check.stderr_and_stdout).to match(/logstash-codec-json/) + + expect("logstash-codec-json").to be_installed_gem end end diff --git a/qa/integration/specs/cli/update_spec.rb b/qa/integration/specs/cli/update_spec.rb new file mode 100644 index 00000000000..56ad75ec791 --- /dev/null +++ b/qa/integration/specs/cli/update_spec.rb @@ -0,0 +1,65 @@ +# Licensed to Elasticsearch B.V. under one or more contributor +# license agreements. See the NOTICE file distributed with +# this work for additional information regarding copyright +# ownership. Elasticsearch B.V. licenses this file to you under +# the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +require_relative "../../framework/fixture" +require_relative "../../framework/settings" +require_relative "../../services/logstash_service" +require_relative "../../framework/helpers" +require_relative "pluginmanager_spec_helper" +require "logstash/devutils/rspec/spec_helper" + +describe "CLI > logstash-plugin update" do + + include_context "pluginmanager validation helpers" + + before(:each) do + @fixture = Fixture.new(__FILE__) + @logstash = @fixture.get_service("logstash") + @logstash_plugin = @logstash.plugin_cli + end + + context "upgrading a plugin" do + before(:each) do + aggregate_failures("precheck") do + expect("logstash-filter-qatest").to_not be_installed_gem + end + aggregate_failures("setup") do + execute = @logstash_plugin.install("logstash-filter-qatest", version: "0.1.0") + + expect(execute.stderr_and_stdout).to match(/Installation successful/) + expect(execute.exit_code).to eq(0) + + expect("logstash-filter-qatest-0.1.0").to be_installed_gem + end + end + it "upgrades the plugin and cleans the old one" do + execute = @logstash_plugin.update("logstash-filter-qatest") + + aggregate_failures("command execution") do + expect(execute.stderr_and_stdout).to include("Updated logstash-filter-qatest 0.1.0 to 0.1.1") + expect(execute.exit_code).to eq(0) + end + + installed = @logstash_plugin.list("logstash-filter-qatest", verbose: true) + expect(execute.exit_code).to eq(0) + expect(installed.stderr_and_stdout).to include("logstash-filter-qatest") + + expect("logstash-filter-qatest-0.1.1").to be_installed_gem + expect("logstash-filter-qatest-0.1.0").to_not be_installed_gem + end + end +end \ No newline at end of file From e5e67a782bc15efdc3ee7a642252757de77faa38 Mon Sep 17 00:00:00 2001 From: Ry Biesemeyer Date: Wed, 5 Mar 2025 20:38:59 -0800 Subject: [PATCH 2/2] Pluginmanager install preserve (#17267) * tests: integration tests for pluginmanager install --preserve * fix regression where pluginmanager's install --preserve flag didn't --- lib/pluginmanager/install.rb | 4 +- qa/integration/specs/cli/install_spec.rb | 69 +++++++++++++++++-- .../specs/cli/pluginmanager_spec_helper.rb | 33 +++++++++ 3 files changed, 98 insertions(+), 8 deletions(-) diff --git a/lib/pluginmanager/install.rb b/lib/pluginmanager/install.rb index e1a3513987d..d0392f76ba9 100644 --- a/lib/pluginmanager/install.rb +++ b/lib/pluginmanager/install.rb @@ -214,7 +214,9 @@ def install_gems_list!(install_list) plugin_gem = gemfile.find(plugin) if preserve? puts("Preserving Gemfile gem options for plugin #{plugin}") if plugin_gem && !plugin_gem.options.empty? - gemfile.update(plugin, version, options) + # if the plugin exists and no version was specified, keep the existing requirements + requirements = (plugin_gem && version.nil? ? plugin_gem.requirements : [version]).compact + gemfile.update(plugin, *requirements, options) else gemfile.overwrite(plugin, version, options) end diff --git a/qa/integration/specs/cli/install_spec.rb b/qa/integration/specs/cli/install_spec.rb index c3b55f926e3..0463fc57d5f 100644 --- a/qa/integration/specs/cli/install_spec.rb +++ b/qa/integration/specs/cli/install_spec.rb @@ -165,11 +165,11 @@ def plugin_filename_re(name, version) context "rubygems hosted plugin" do include_context "pluginmanager validation helpers" - shared_examples("overwriting existing") do + shared_context("install over existing") do before(:each) do aggregate_failures("precheck") do expect("#{plugin_name}-#{existing_plugin_version}").to_not be_installed_gem - expect("#{plugin_name}-#{specified_plugin_version}").to_not be_installed_gem + expect("#{plugin_name}").to_not be_installed_gem end aggregate_failures("setup") do execute = @logstash_plugin.install(plugin_name, version: existing_plugin_version) @@ -178,9 +178,12 @@ def plugin_filename_re(name, version) expect(execute.exit_code).to eq(0) expect("#{plugin_name}-#{existing_plugin_version}").to be_installed_gem - expect("#{plugin_name}-#{specified_plugin_version}").to_not be_installed_gem + expect(plugin_name).to be_in_gemfile.with_requirements(existing_plugin_version) end end + end + shared_examples("overwriting existing with explicit version") do + include_context "install over existing" it "installs the specified version and removes the pre-existing one" do execute = @logstash_plugin.install(plugin_name, version: specified_plugin_version) @@ -197,20 +200,72 @@ def plugin_filename_re(name, version) end end - context "when installing over an older version" do + context "when installing over an older version using --version" do let(:plugin_name) { "logstash-filter-qatest" } let(:existing_plugin_version) { "0.1.0" } let(:specified_plugin_version) { "0.1.1" } - include_examples "overwriting existing" + include_examples "overwriting existing with explicit version" end - context "when installing over a newer version" do + context "when installing over a newer version using --version" do let(:plugin_name) { "logstash-filter-qatest" } let(:existing_plugin_version) { "0.1.0" } let(:specified_plugin_version) { "0.1.1" } - include_examples "overwriting existing" + include_examples "overwriting existing with explicit version" + end + + context "when installing over existing without --version" do + let(:plugin_name) { "logstash-filter-qatest" } + let(:existing_plugin_version) { "0.1.0" } + + include_context "install over existing" + + context "with --preserve" do + it "succeeds without changing the requirements in the Gemfile" do + execute = @logstash_plugin.install(plugin_name, preserve: true) + + aggregate_failures("command execution") do + expect(execute.stderr_and_stdout).to match(INSTALLATION_SUCCESS_RE) + expect(execute.exit_code).to eq(0) + end + + installed = @logstash_plugin.list(verbose: true) + expect(installed.stderr_and_stdout).to match(/#{Regexp.escape plugin_name}/) + + # we want to ensure that the act of installing an already-installed plugin + # does not change its requirements in the gemfile, and leaves the previously-installed + # version in-tact. + expect(plugin_name).to be_in_gemfile.with_requirements(existing_plugin_version) + expect("#{plugin_name}-#{existing_plugin_version}").to be_installed_gem + end + end + + context "without --preserve" do + # this spec is OBSERVED behaviour, which I believe to be undesirable. + it "succeeds and removes the version requirement from the Gemfile" do + execute = @logstash_plugin.install(plugin_name) + + aggregate_failures("command execution") do + expect(execute.stderr_and_stdout).to match(INSTALLATION_SUCCESS_RE) + expect(execute.exit_code).to eq(0) + end + + installed = @logstash_plugin.list(plugin_name, verbose: true) + expect(installed.stderr_and_stdout).to match(/#{Regexp.escape plugin_name}/) + + # This is the potentially-undesirable surprising behaviour, specified here + # as a means of documentation, not a promise of future behavior. + expect(plugin_name).to be_in_gemfile.without_requirements + + # we expect _a_ version of the plugin to be installed, but cannot be opinionated + # about which version was installed because bundler won't necessarily re-resolve + # the dependency graph to get us an upgrade since the no-requirements dependency + # is still met (but it MAY do so if also installing plugins that are not present). + expect("#{plugin_name}").to be_installed_gem + end + end end context "installing plugin that isn't present" do diff --git a/qa/integration/specs/cli/pluginmanager_spec_helper.rb b/qa/integration/specs/cli/pluginmanager_spec_helper.rb index 6270f7933c1..4a1b9998256 100644 --- a/qa/integration/specs/cli/pluginmanager_spec_helper.rb +++ b/qa/integration/specs/cli/pluginmanager_spec_helper.rb @@ -1,4 +1,5 @@ require 'pathname' +require 'bundler/definition' shared_context "pluginmanager validation helpers" do @@ -40,12 +41,44 @@ end end + matcher :be_in_gemfile do + match do |actual| + common(actual) + @dep && (@version_requirements.nil? || @version_requirements == @dep.requirement) + end + define_method :common do |actual| + @definition = Bundler::Definition.build(logstash_gemfile, logstash_gemfile_lock, false) + @dep = @definition.dependencies.find { |s| s.name == actual } + end + chain :with_requirements do |version_requirements| + @version_requirements = Gem::Requirement.create(version_requirements) + end + chain :without_requirements do + @version_requirements = Gem::Requirement.default + end + failure_message do |actual| + if @dep.nil? + "expected the gem to be in the gemspec, but it wasn't (#{@definition.dependencies.map(&:name)})" + else + "expected the `#{actual}` gem to have requirements `#{@version_requirements}`, but they were `#{@dep.requirement}`" + end + end + end + def logstash_home return super() if defined?(super) return @logstash.logstash_home if @logstash fail("no @logstash, so we can't get logstash_home") end + def logstash_gemfile + Pathname.new(logstash_home) / "Gemfile" + end + + def logstash_gemfile_lock + Pathname.new(logstash_home) / "Gemfile.lock" + end + def logstash_gemdir pathname_base = (Pathname.new(logstash_home) / "vendor" / "bundle" / "jruby") candidate_dirs = pathname_base.glob("[0-9]*")