From 916655dbd3dfd49fd38ba58b918deb0c909046eb Mon Sep 17 00:00:00 2001 From: Chris Roberts Date: Mon, 8 Jun 2020 17:15:32 -0700 Subject: [PATCH] Fix trigger matching on `:all` special value Updates the type to do a proper comparison when checking for the `:all` special value as well as applied ignores. Fixes #11599 --- lib/vagrant/plugin/v2/trigger.rb | 5 +- plugins/kernel_v2/config/vm_trigger.rb | 6 +- .../kernel_v2/config/vm_trigger_test.rb | 2 +- test/unit/vagrant/plugin/v2/trigger_test.rb | 61 ++++++++++++++++--- 4 files changed, 60 insertions(+), 14 deletions(-) diff --git a/lib/vagrant/plugin/v2/trigger.rb b/lib/vagrant/plugin/v2/trigger.rb index 0a6919d2c..fda810d11 100644 --- a/lib/vagrant/plugin/v2/trigger.rb +++ b/lib/vagrant/plugin/v2/trigger.rb @@ -72,13 +72,13 @@ module Vagrant if stage == :before triggers = config.before_triggers.select do |t| - (t.command == :all && !t.ignore.include?(name)) || + (t.command.respond_to?(:to_sym) && t.command.to_sym == :all && !t.ignore.include?(name.to_sym)) || (type == :hook && matched_hook?(t.command, name)) || nameify(t.command) == name end elsif stage == :after triggers = config.after_triggers.select do |t| - (t.command == :all && !t.ignore.include?(name)) || + (t.command.respond_to?(:to_sym) && t.command.to_sym == :all && !t.ignore.include?(name.to_sym)) || (type == :hook && matched_hook?(t.command, name)) || nameify(t.command) == name end @@ -89,6 +89,7 @@ module Vagrant type: type, guest_name: guest end + filter_triggers(triggers, guest, type) end diff --git a/plugins/kernel_v2/config/vm_trigger.rb b/plugins/kernel_v2/config/vm_trigger.rb index 182ea3d8a..18f9e3d65 100644 --- a/plugins/kernel_v2/config/vm_trigger.rb +++ b/plugins/kernel_v2/config/vm_trigger.rb @@ -114,7 +114,11 @@ module VagrantPlugins # Internal options @id = SecureRandom.uuid - @command = command.to_s + if command.respond_to?(:to_sym) + @command = command.to_sym + else + @command = command + end @ruby_block = UNSET_VALUE @logger.debug("Trigger defined for: #{command}") diff --git a/test/unit/plugins/kernel_v2/config/vm_trigger_test.rb b/test/unit/plugins/kernel_v2/config/vm_trigger_test.rb index 6fbba226f..6735e219f 100644 --- a/test/unit/plugins/kernel_v2/config/vm_trigger_test.rb +++ b/test/unit/plugins/kernel_v2/config/vm_trigger_test.rb @@ -53,7 +53,7 @@ describe VagrantPlugins::Kernel_V2::VagrantConfigTrigger do it "sets a command" do subject.finalize! - expect(subject.command).to eq(command.to_s) + expect(subject.command).to eq(command) end it "uses default error behavior" do diff --git a/test/unit/vagrant/plugin/v2/trigger_test.rb b/test/unit/vagrant/plugin/v2/trigger_test.rb index 6a4fff094..42b4d541e 100644 --- a/test/unit/vagrant/plugin/v2/trigger_test.rb +++ b/test/unit/vagrant/plugin/v2/trigger_test.rb @@ -23,19 +23,19 @@ describe Vagrant::Plugin::V2::Trigger do ui: ui, } } - let(:triggers) { VagrantPlugins::Kernel_V2::TriggerConfig.new } + let(:triggers) { + @triggers ||= VagrantPlugins::Kernel_V2::TriggerConfig.new.tap do |triggers| + triggers.before(:up, hash_block) + triggers.before(:destroy, hash_block) + triggers.before(:halt, hash_block_two) + triggers.after(:up, hash_block) + triggers.after(:destroy, hash_block) + triggers.finalize! + end + } let(:hash_block) { {info: "hi", run: {inline: "echo 'hi'"}} } let(:hash_block_two) { {warn: "WARNING!!", run_remote: {inline: "echo 'hi'"}} } - before do - triggers.before(:up, hash_block) - triggers.before(:destroy, hash_block) - triggers.before(:halt, hash_block_two) - triggers.after(:up, hash_block) - triggers.after(:destroy, hash_block) - triggers.finalize! - end - let(:subject) { described_class.new(env, triggers, machine, ui) } describe "#fire" do @@ -87,6 +87,47 @@ describe Vagrant::Plugin::V2::Trigger do subject.find(:halt, :before, "guest", :action) end + context "with :all special value" do + let(:triggers) { VagrantPlugins::Kernel_V2::TriggerConfig.new } + let(:ignores) { [] } + + before do + triggers.before(:all, hash_block.merge(ignore: ignores)) + triggers.after(:all, hash_block.merge(ignore: ignores)) + triggers.finalize! + end + + [:destroy, :halt, :provision, :reload, :resume, :suspend, :up].each do |supported_action| + it "should locate trigger when before #{supported_action} action is requested" do + expect(subject.find(supported_action, :before, "guest", :action)).not_to be_empty + end + + it "should locate trigger when after #{supported_action} action is requested" do + expect(subject.find(supported_action, :after, "guest", :action)).not_to be_empty + end + end + + context "with ignores" do + let(:ignores) { [:halt, :up] } + + it "should not locate trigger when before command is ignored" do + expect(subject.find(:up, :before, "guest", :action)).to be_empty + end + + it "should not locate trigger when after command is ignored" do + expect(subject.find(:halt, :after, "guest", :action)).to be_empty + end + + it "should locate trigger when before command is not ignored" do + expect(subject.find(:provision, :before, "guest", :action)).not_to be_empty + end + + it "should locate trigger when after command is not ignored" do + expect(subject.find(:provision, :after, "guest", :action)).not_to be_empty + end + end + end + context "with hook type" do before do triggers.before(:environment_load, hash_block.merge(type: :hook))