From d08c68ecf3d23418fac0184e13a04f96569153f9 Mon Sep 17 00:00:00 2001
From: Chris Roberts <croberts@hashicorp.com>
Date: Wed, 11 Mar 2020 18:40:45 -0700
Subject: [PATCH 1/9] Adjust how trigger actions are inserted into the stack

This adjusts how triggers are implemented during a normal run. Any
defined triggers which are applicable are located and injected into
the run stack as the stack is built, including hook type triggers.

Support is included for dynamic hook lookup.

The data type used when defining triggers has also been relaxed to
support symbols, strings, or constants.
---
 lib/vagrant/action/builder.rb                 | 172 +++++++++++++---
 lib/vagrant/action/builtin/after_trigger.rb   |   6 +-
 lib/vagrant/action/builtin/before_trigger.rb  |   6 +-
 lib/vagrant/action/hook.rb                    |  22 +-
 lib/vagrant/action/runner.rb                  |  36 +---
 lib/vagrant/action/warden.rb                  |  51 +----
 lib/vagrant/cli.rb                            |   6 +-
 lib/vagrant/machine.rb                        |   5 +-
 lib/vagrant/plugin/v2/manager.rb              |   6 +-
 lib/vagrant/plugin/v2/trigger.rb              |  90 +++++---
 lib/vagrant/util.rb                           |   1 +
 plugins/kernel_v2/config/vm_trigger.rb        |   2 +-
 templates/locales/en.yml                      |   5 +-
 .../kernel_v2/config/vm_trigger_test.rb       |   2 +-
 test/unit/vagrant/action/builder_test.rb      | 192 +++++++++++++++++-
 test/unit/vagrant/action/warden_test.rb       | 131 ------------
 test/unit/vagrant/cli_test.rb                 |  11 +-
 test/unit/vagrant/environment_test.rb         |   2 -
 test/unit/vagrant/plugin/v2/trigger_test.rb   | 116 +++++++++--
 19 files changed, 551 insertions(+), 311 deletions(-)

diff --git a/lib/vagrant/action/builder.rb b/lib/vagrant/action/builder.rb
index 5adc635f8..00c5cf696 100644
--- a/lib/vagrant/action/builder.rb
+++ b/lib/vagrant/action/builder.rb
@@ -122,6 +122,7 @@ module Vagrant
       # @return [Integer]
       def index(object)
         stack.each_with_index do |item, i|
+          return i if item == object
           return i if item[0] == object
           return i if item[0].respond_to?(:name) && item[0].name == object
         end
@@ -132,42 +133,153 @@ module Vagrant
       # Converts the builder stack to a runnable action sequence.
       #
       # @param [Hash] env The action environment hash
-      # @return [Object] A callable object
+      # @return [Warden] A callable object
       def to_app(env)
-        app_stack = nil
+        # Start with a duplicate of ourself which can
+        # be modified
+        builder = self.dup
 
-        # If we have action hooks, then we apply them
-        if env[:action_hooks]
-          builder = self.dup
+        # Apply all dynamic modifications of the stack. This
+        # will generate dynamic hooks for all actions within
+        # the stack, load any triggers for action classes, and
+        # apply them to the builder's stack
+        builder.apply_dynamic_updates(env)
 
-          # These are the options to pass into hook application.
-          options = {}
-
-          # If we already ran through once and did append/prepends,
-          # then don't do it again.
-          if env[:action_hooks_already_ran]
-            options[:no_prepend_or_append] = true
-          end
-
-          # Specify that we already ran, so in the future we don't repeat
-          # the prepend/append hooks.
-          env[:action_hooks_already_ran] = true
-
-          # Apply all the hooks to the new builder instance
-          env[:action_hooks].each do |hook|
-            hook.apply(builder, options)
-          end
-
-          # The stack is now the result of the new builder
-          app_stack = builder.stack.dup
-        end
-
-        # If we don't have a stack then default to using our own
-        app_stack ||= stack.dup
+        # Now that the stack is fully expanded, apply any
+        # action hooks that may be defined so they are on
+        # the outermost locations of the stack
+        builder.apply_action_name(env)
 
         # Wrap the middleware stack with the Warden to provide a consistent
         # and predictable behavior upon exceptions.
-        Warden.new(app_stack, env)
+        Warden.new(builder.stack.dup, env)
+      end
+
+      # Find any action hooks or triggers which have been defined
+      # for items within the stack. Update the stack with any
+      # hooks or triggers found.
+      #
+      # @param [Hash] env Call environment
+      # @return [Builder] self
+      def apply_dynamic_updates(env)
+        if Vagrant::Util::Experimental.feature_enabled?("typed_triggers")
+          triggers = env[:triggers]
+        end
+
+        # Use a Hook as a convenient interface for injecting
+        # any applicable trigger actions within the stack
+        machine_name = env[:machine].name if env[:machine]
+
+        # Iterate over all items in the stack and apply new items
+        # into the hook as they are found. Must be sure to dup the
+        # stack here since we are modifying the stack in the loop.
+        stack.dup.each do |item|
+          hook = Hook.new
+
+          action = item.first
+          next if action.is_a?(Proc)
+
+          # Start with adding any action triggers that may be defined
+          if triggers && !triggers.find(action, :before, machine_name, :action).empty?
+            hook.prepend(Vagrant::Action::Builtin::BeforeTriggerAction,
+              action.name, triggers)
+          end
+
+          if triggers && !triggers.find(action, :after, machine_name, :action).empty?
+            hook.append(Vagrant::Action::Builtin::AfterTriggerAction,
+              action.name, triggers)
+          end
+
+          # Next look for any hook triggers that may be defined against
+          # the dynamically generated action class hooks
+          if triggers && !triggers.find(action, :before, machine_name, :hook).empty?
+            hook.prepend(Vagrant::Action::Builtin::BeforeTriggerAction,
+              action.name, triggers, :hook)
+          end
+
+          if triggers && !triggers.find(action, :after, machine_name, :hook).empty?
+            hook.append(Vagrant::Action::Builtin::AfterTriggerAction,
+              action.name, triggers, :hook)
+          end
+
+          # Finally load any registered hooks for dynamically generated
+          # action class based hooks
+          Vagrant.plugin("2").manager.find_action_hooks(action).each do |hook_proc|
+            hook_proc.call(hook)
+          end
+
+          hook.apply(self, root: item)
+        end
+
+        # Apply the hook to ourself to update the stack
+        self
+      end
+
+      # If action hooks have not already been set, this method
+      # will perform three tasks:
+      #   1. Load any hook triggers defined for the action_name
+      #   2. Load any action_hooks defined from plugins
+      #   3. Load any action triggers based on machine action called (not action classes)
+      #
+      # @param [Hash] env Call environment
+      # @return [Builder]
+      def apply_action_name(env)
+        return self if !env[:action_name]
+        hook = Hook.new
+        machine_name = env[:machine].name if env[:machine]
+
+        # Start with loading any hook triggers if applicable
+        if Vagrant::Util::Experimental.feature_enabled?("typed_triggers") && env[:triggers]
+          if !env[:triggers].find(env[:action_name], :before, machine_name, :hook).empty?
+            hook.prepend(Vagrant::Action::Builtin::BeforeTriggerAction,
+              env[:action_name].to_sym, env[:triggers])
+          end
+          if !env[:triggers].find(env[:action_name], :after, machine_name, :hook).empty?
+            hook.append(Vagrant::Action::Builtin::AfterTriggerAction,
+              env[:action_name].to_sym, env[:triggers])
+          end
+        end
+
+        # Next we load up all the action hooks that plugins may
+        # have defined
+        action_hooks = Vagrant.plugin("2").manager.action_hooks(env[:action_name])
+        action_hooks.each do |hook_proc|
+          hook_proc.call(hook)
+        end
+
+        # Finally load any action triggers defined
+        if env[:triggers]
+          if !env[:triggers].find(env[:raw_action_name], :before, machine_name, :action)
+            hook.prepend(Vagrant::Action::Builtin::BeforeTriggerAction,
+              env[:raw_action_name].to_sym, env[:triggers])
+          end
+          if !env[:triggers].find(env[:raw_action_name], :after, machine_name, :action)
+            hook.append(Vagrant::Action::Builtin::AfterTriggerAction,
+              env[:raw_action_name].to_sym, env[:triggers])
+          end
+        end
+
+        # If the hooks are empty, then there was nothing to apply and
+        # we can just send ourself back
+        return self if hook.empty?
+
+        # These are the options to pass into hook application.
+        options = {}
+
+        # If we already ran through once and did append/prepends,
+        # then don't do it again.
+        if env[:action_hooks_already_ran]
+          options[:no_prepend_or_append] = true
+        end
+
+        # Specify that we already ran, so in the future we don't repeat
+        # the prepend/append hooks.
+        env[:action_hooks_already_ran] = true
+
+        # Apply all the hooks to the new builder instance
+        hook.apply(self, options)
+
+        self
       end
     end
   end
diff --git a/lib/vagrant/action/builtin/after_trigger.rb b/lib/vagrant/action/builtin/after_trigger.rb
index 106d10df1..46a2d3d91 100644
--- a/lib/vagrant/action/builtin/after_trigger.rb
+++ b/lib/vagrant/action/builtin/after_trigger.rb
@@ -9,18 +9,20 @@ module Vagrant
       class AfterTriggerAction
         # @param [Symbol] action_name - The action class name to fire trigger on
         # @param [Vagrant::Plugin::V2::Triger] triggers - trigger object
-        def initialize(app, env, action_name, triggers)
+        def initialize(app, env, action_name, triggers, type=:action)
           @app         = app
           @env         = env
           @triggers    = triggers
           @action_name = action_name
+          @type        = type
         end
 
         def call(env)
           machine = env[:machine]
           machine_name = machine.name if machine
 
-          @triggers.fire_triggers(@action_name, :after, machine_name, :action) if Vagrant::Util::Experimental.feature_enabled?("typed_triggers");
+          @triggers.fire(@action_name, :after, machine_name, @type) if
+            Vagrant::Util::Experimental.feature_enabled?("typed_triggers");
 
           # Carry on
           @app.call(env)
diff --git a/lib/vagrant/action/builtin/before_trigger.rb b/lib/vagrant/action/builtin/before_trigger.rb
index fad2e5799..74a6f8754 100644
--- a/lib/vagrant/action/builtin/before_trigger.rb
+++ b/lib/vagrant/action/builtin/before_trigger.rb
@@ -6,18 +6,20 @@ module Vagrant
       class BeforeTriggerAction
         # @param [Symbol] action_name - The action class name to fire trigger on
         # @param [Vagrant::Plugin::V2::Triger] triggers - trigger object
-        def initialize(app, env, action_name, triggers)
+        def initialize(app, env, action_name, triggers, type=:action)
           @app         = app
           @env         = env
           @triggers    = triggers
           @action_name = action_name
+          @type        = type
         end
 
         def call(env)
           machine = env[:machine]
           machine_name = machine.name if machine
 
-          @triggers.fire_triggers(@action_name, :before, machine_name, :action) if Vagrant::Util::Experimental.feature_enabled?("typed_triggers");
+          @triggers.fire(@action_name, :before, machine_name, @type) if
+            Vagrant::Util::Experimental.feature_enabled?("typed_triggers");
 
           # Carry on
           @app.call(env)
diff --git a/lib/vagrant/action/hook.rb b/lib/vagrant/action/hook.rb
index 21881e8ba..3ab308085 100644
--- a/lib/vagrant/action/hook.rb
+++ b/lib/vagrant/action/hook.rb
@@ -65,6 +65,14 @@ module Vagrant
         @prepend_hooks << [new, args, block]
       end
 
+      # @return [Boolean]
+      def empty?
+        before_hooks.empty? &&
+          after_hooks.empty? &&
+          prepend_hooks.empty? &&
+          append_hooks.empty?
+      end
+
       # This applies the given hook to a builder. This should not be
       # called directly.
       #
@@ -75,12 +83,22 @@ module Vagrant
         if !options[:no_prepend_or_append]
           # Prepends first
           @prepend_hooks.each do |klass, args, block|
-            builder.insert(0, klass, *args, &block)
+            if options[:root]
+              idx = builder.index(options[:root])
+            else
+              idx = 0
+            end
+            builder.insert(idx, klass, *args, &block)
           end
 
           # Appends
           @append_hooks.each do |klass, args, block|
-            builder.use(klass, *args, &block)
+            if options[:root]
+              idx = builder.index(options[:root])
+              builder.insert(idx + 1, klass, *args, &block)
+            else
+              builder.use(klass, *args, &block)
+            end
           end
         end
 
diff --git a/lib/vagrant/action/runner.rb b/lib/vagrant/action/runner.rb
index f125617b1..d9b791a66 100644
--- a/lib/vagrant/action/runner.rb
+++ b/lib/vagrant/action/runner.rb
@@ -34,30 +34,18 @@ module Vagrant
         environment.merge!(@lazy_globals.call) if @lazy_globals
         environment.merge!(options || {})
 
-        if Vagrant::Util::Experimental.feature_enabled?("typed_triggers")
-          # NOTE: Triggers are initialized later in the Action::Runer because of
-          # how `@lazy_globals` are evaluated. Rather than trying to guess where
-          # the `env` is coming from, we can wait until they're merged into a single
-          # hash above.
-          env = environment[:env]
-          machine = environment[:machine]
-          machine_name = machine.name if machine
+        # NOTE: Triggers are initialized later in the Action::Runer because of
+        # how `@lazy_globals` are evaluated. Rather than trying to guess where
+        # the `env` is coming from, we can wait until they're merged into a single
+        # hash above.
+        env = environment[:env]
+        machine = environment[:machine]
+        machine_name = machine.name if machine
 
+        if env
           ui = Vagrant::UI::Prefixed.new(env.ui, "vagrant")
-          triggers = Vagrant::Plugin::V2::Trigger.new(env, env.vagrantfile.config.trigger, machine, ui)
-        end
-
-        # Setup the action hooks
-        hooks = Vagrant.plugin("2").manager.action_hooks(environment[:action_name])
-        if !hooks.empty?
-          @logger.info("Preparing hooks for middleware sequence...")
-          environment[:action_hooks] = hooks.map do |hook_proc|
-            Hook.new.tap do |h|
-              hook_proc.call(h)
-            end
-          end
-
-          @logger.info("#{environment[:action_hooks].length} hooks defined.")
+          environment[:triggers] = Vagrant::Plugin::V2::Trigger.
+            new(env, env.vagrantfile.config.trigger, machine, ui)
         end
 
         # Run the action chain in a busy block, marking the environment as
@@ -95,14 +83,10 @@ module Vagrant
 
         action_name = environment[:action_name]
 
-        triggers.fire_triggers(action_name, :before, machine_name, :hook) if Vagrant::Util::Experimental.feature_enabled?("typed_triggers")
-
         # We place a process lock around every action that is called
         @logger.info("Running action: #{environment[:action_name]} #{callable_id}")
         Util::Busy.busy(int_callback) { callable.call(environment) }
 
-        triggers.fire_triggers(action_name, :after, machine_name, :hook) if Vagrant::Util::Experimental.feature_enabled?("typed_triggers")
-
         # Return the environment in case there are things in there that
         # the caller wants to use.
         environment
diff --git a/lib/vagrant/action/warden.rb b/lib/vagrant/action/warden.rb
index b50d219f1..32cb6abfd 100644
--- a/lib/vagrant/action/warden.rb
+++ b/lib/vagrant/action/warden.rb
@@ -47,40 +47,9 @@ module Vagrant
           raise Errors::VagrantInterrupt if env[:interrupted]
           action = @actions.shift
           @logger.info("Calling IN action: #{action}")
-
-          actions = [action]
-
-          # If this is an action class, set name to check for any defined hooks
-          if !action.is_a?(Proc)
-            hook_name = action.class.name
-            @logger.debug("Performing hooks lookup on #{hook_name} (Action: #{action} - class: #{action.class})")
-            hooks = Vagrant.plugin("2").manager.find_action_hooks(hook_name)
-
-            if !hooks.empty?
-              @logger.debug("Found #{hooks.count} hooks defined for #{hook_name} (Action: #{action})")
-              # Create a new builder for the current action so we can
-              # properly apply the hooks with the correct ordering
-              b = Builder.new
-              b.stack << action
-
-              hooks.each do |hook_proc|
-                hook = Hook.new.tap { |h| hook_proc.call(h) }
-                hook.apply(b)
-              end
-
-              actions = b.stack.map { |a| finalize_action(a, @env) }
-            end
-          end
-
-          @logger.info("Calling BEGIN action: #{action}")
-          actions.each do |local_action|
-            @logger.info("Calling IN action: #{local_action} (root: #{action})")
-            @stack.unshift(local_action).first.call(env)
-            raise Errors::VagrantInterrupt if env[:interrupted]
-            @logger.info("Calling OUT action: #{local_action} (root: #{action})")
-          end
-
-          @logger.info("Calling COMPLETE action: #{action}")
+          @stack.unshift(action).first.call(env)
+          raise Errors::VagrantInterrupt if env[:interrupted]
+          @logger.info("Calling OUT action: #{action}")
         rescue SystemExit, NoMemoryError
           # This means that an "exit" or "abort" was called, or we have run out
           # of memory. In these cases, we just exit immediately.
@@ -132,19 +101,7 @@ module Vagrant
         args ||= []
 
         if klass.is_a?(Class)
-          # A action klass which is to be instantiated with the
-          # app, env, and any arguments given
-
-          # We wrap the action class in two Trigger method calls so that
-          # action triggers can fire before and after each given action in the stack.
-          klass_name = klass.name
-          [Vagrant::Action::Builtin::BeforeTriggerAction.new(self, env,
-                                                             klass_name,
-                                                             @triggers),
-           klass.new(self, env, *args, &block),
-           Vagrant::Action::Builtin::AfterTriggerAction.new(self, env,
-                                                            klass_name,
-                                                            @triggers)]
+          klass.new(self, env, *args, &block)
         elsif klass.respond_to?(:call)
           # Make it a lambda which calls the item then forwards
           # up the chain
diff --git a/lib/vagrant/cli.rb b/lib/vagrant/cli.rb
index 43bdafe41..9421c9c2b 100644
--- a/lib/vagrant/cli.rb
+++ b/lib/vagrant/cli.rb
@@ -62,9 +62,11 @@ module Vagrant
       # Initialize and execute the command class, returning the exit status.
       result = 0
       begin
-        @triggers.fire_triggers(@sub_command.to_sym, :before, nil, :command) if Vagrant::Util::Experimental.feature_enabled?("typed_triggers")
+        @triggers.fire(@sub_command, :before, nil, :command) if
+          Vagrant::Util::Experimental.feature_enabled?("typed_triggers")
         result = command_class.new(@sub_args, @env).execute
-        @triggers.fire_triggers(@sub_command.to_sym, :after, nil, :command) if Vagrant::Util::Experimental.feature_enabled?("typed_triggers")
+        @triggers.fire(@sub_command, :after, nil, :command) if
+          Vagrant::Util::Experimental.feature_enabled?("typed_triggers")
       rescue Interrupt
         @env.ui.info(I18n.t("vagrant.cli_interrupt"))
         result = 1
diff --git a/lib/vagrant/machine.rb b/lib/vagrant/machine.rb
index 0b1ca1b65..a593b0fff 100644
--- a/lib/vagrant/machine.rb
+++ b/lib/vagrant/machine.rb
@@ -160,8 +160,6 @@ module Vagrant
     #   as extra data set on the environment hash for the middleware
     #   runner.
     def action(name, opts=nil)
-      @triggers.fire_triggers(name, :before, @name.to_s, :action)
-
       @logger.info("Calling action: #{name} on provider #{@provider}")
 
       opts ||= {}
@@ -210,8 +208,6 @@ module Vagrant
         ui.machine("action", name.to_s, "end")
         action_result
       end
-
-      @triggers.fire_triggers(name, :after, @name.to_s, :action)
       # preserve returning environment after machine action runs
       return return_env
     rescue Errors::EnvironmentLockedError
@@ -230,6 +226,7 @@ module Vagrant
     def action_raw(name, callable, extra_env=nil)
       # Run the action with the action runner on the environment
       env = {
+        raw_action_name: name,
         action_name: "machine_action_#{name}".to_sym,
         machine: self,
         machine_action: name,
diff --git a/lib/vagrant/plugin/v2/manager.rb b/lib/vagrant/plugin/v2/manager.rb
index ce21be1f4..bba37ae04 100644
--- a/lib/vagrant/plugin/v2/manager.rb
+++ b/lib/vagrant/plugin/v2/manager.rb
@@ -64,7 +64,11 @@ module Vagrant
         # @param [Class, String] key Base key for generation
         # @return [Array<String>] all valid keys
         def generate_hook_keys(key)
-          key = key.is_a?(Class) ? key.name : key.to_s
+          if key.is_a?(Class)
+            key = key.name.to_s
+          else
+            key = key.to_s
+          end
           parts = key.split("::")
           [].tap do |keys|
             until parts.empty?
diff --git a/lib/vagrant/plugin/v2/trigger.rb b/lib/vagrant/plugin/v2/trigger.rb
index 7ab4d73d8..abee675df 100644
--- a/lib/vagrant/plugin/v2/trigger.rb
+++ b/lib/vagrant/plugin/v2/trigger.rb
@@ -30,58 +30,97 @@ module Vagrant
           @logger = Log4r::Logger.new("vagrant::trigger::#{self.class.to_s.downcase}")
         end
 
-        # Fires all triggers, if any are defined for the action and guest. Returns early
+        # Fires all triggers, if any are defined for the named type and guest. Returns early
         # and logs a warning if the community plugin `vagrant-triggers` is installed
         #
-        # @param [Symbol] action Vagrant command to fire trigger on
+        # @param [Symbol] name Name of `type` thing to fire trigger on
         # @param [Symbol] stage :before or :after
-        # @param [String] guest_name The guest that invoked firing the triggers
-        def fire_triggers(action, stage, guest_name, type)
+        # @param [String] guest The guest that invoked firing the triggers
+        # @param [Symbol] type Type of trigger to fire
+        def fire(name, stage, guest, type)
           if community_plugin_detected?
             @logger.warn("Community plugin `vagrant-triggers detected, so core triggers will not fire")
             return
           end
 
-          if !action
-            @logger.warn("Action given is nil, no triggers will fire")
-            return
-          else
-            action = action.to_sym
-          end
+          return @logger.warn("Name given is nil, no triggers will fire") if !name
+          return @logger.warn("Name given cannot be symbolized, no triggers will fire") if
+            !name.respond_to?(:to_sym)
+
+          name = name.to_sym
 
           # get all triggers matching action
-          triggers = []
+          triggers = find(name, stage, guest, type)
+
+          if !triggers.empty?
+            @logger.info("Firing trigger for #{type} #{name} on guest #{guest}")
+            @ui.info(I18n.t("vagrant.trigger.start", type: type, stage: stage, name: name))
+            execute(triggers)
+          end
+        end
+
+        # Find all triggers defined for the named type and guest.
+        #
+        # @param [Symbol] name Name of `type` thing to fire trigger on
+        # @param [Symbol] stage :before or :after
+        # @param [String] guest The guest that invoked firing the triggers
+        # @param [Symbol] type Type of trigger to fire
+        # @return [Array]
+        def find(name, stage, guest, type)
+          triggers = nil
+          name = nameify(name)
+
           if stage == :before
             triggers = config.before_triggers.select do |t|
-              t.command == action || (t.command == :all && !t.ignore.include?(action))
+              (t.command == :all && !t.ignore.include?(name)) ||
+                (type == :hook && matched_hook?(t.command, name)) ||
+                nameify(t.command) == name
             end
           elsif stage == :after
             triggers = config.after_triggers.select do |t|
-              t.command == action || (t.command == :all && !t.ignore.include?(action))
+              (t.command == :all && !t.ignore.include?(name)) ||
+                (type == :hook && matched_hook?(t.command, name)) ||
+                nameify(t.command) == name
             end
           else
             raise Errors::TriggersNoStageGiven,
-              action: action,
+              name: name,
               stage: stage,
-              guest_name: guest_name
-          end
-
-          triggers = filter_triggers(triggers, guest_name, type)
-
-          if !triggers.empty?
-            @logger.info("Firing trigger for action #{action} on guest #{guest_name}")
-            @ui.info(I18n.t("vagrant.trigger.start", type: type, stage: stage, action: action))
-            fire(triggers, guest_name)
+              type: type,
+              guest_name: guest
           end
+          filter_triggers(triggers, guest, type)
         end
 
         protected
 
+        # Convert object into name
+        #
+        # @param [Object, Class] object Object to name
+        # @return [String]
+        def nameify(object)
+          if object.is_a?(Class)
+            object.name.to_s
+          else
+            object.to_s
+          end
+        end
 
         #-------------------------------------------------------------------
         # Internal methods, don't call these.
         #-------------------------------------------------------------------
 
+        # Generate all valid lookup keys for given action key
+        #
+        # @param [Class, String] key Base key for generation
+        # @return [Array<String>] all valid keys
+        def matched_hook?(key, subject)
+          subject = nameify(subject)
+          Vagrant.plugin("2").manager.generate_hook_keys(key).any? do |k|
+            k == subject
+          end
+        end
+
         # Looks up if the community plugin `vagrant-triggers` is installed
         # and also caches the result
         #
@@ -133,12 +172,11 @@ module Vagrant
           return triggers
         end
 
-        # Fires off all triggers in the given array
+        # Execute all triggers in the given array
         #
         # @param [Array] triggers An array of triggers to be fired
-        def fire(triggers, guest_name)
+        def execute(triggers)
           # ensure on_error is respected by exiting or continuing
-
           triggers.each do |trigger|
             @logger.debug("Running trigger #{trigger.id}...")
 
diff --git a/lib/vagrant/util.rb b/lib/vagrant/util.rb
index 32babe50d..f8be2baf0 100644
--- a/lib/vagrant/util.rb
+++ b/lib/vagrant/util.rb
@@ -7,6 +7,7 @@ module Vagrant
     autoload :CredentialScrubber,        'vagrant/util/credential_scrubber'
     autoload :DeepMerge,                 'vagrant/util/deep_merge'
     autoload :Env,                       'vagrant/util/env'
+    autoload :Experimental,              'vagrant/util/experimental'
     autoload :HashWithIndifferentAccess, 'vagrant/util/hash_with_indifferent_access'
     autoload :GuestInspection,           'vagrant/util/guest_inspection'
     autoload :LoggingFormatter,          'vagrant/util/logging_formatter'
diff --git a/plugins/kernel_v2/config/vm_trigger.rb b/plugins/kernel_v2/config/vm_trigger.rb
index 2595eea49..3bd2b0f19 100644
--- a/plugins/kernel_v2/config/vm_trigger.rb
+++ b/plugins/kernel_v2/config/vm_trigger.rb
@@ -114,7 +114,7 @@ module VagrantPlugins
 
         # Internal options
         @id = SecureRandom.uuid
-        @command = command.to_sym
+        @command = command.to_s
         @ruby_block = UNSET_VALUE
 
         @logger.debug("Trigger defined for: #{command}")
diff --git a/templates/locales/en.yml b/templates/locales/en.yml
index 743be4761..949d16993 100644
--- a/templates/locales/en.yml
+++ b/templates/locales/en.yml
@@ -301,7 +301,7 @@ en:
         Vagrant has been configured to abort. Vagrant will terminate
         after remaining running actions have completed...
       start: |-
-        Running %{type} triggers %{stage} %{action} ...
+        Running %{type} triggers %{stage} %{name} ...
       fire_with_name: |-
         Running trigger: %{name}...
       fire: |-
@@ -1529,7 +1529,8 @@ en:
       triggers_no_stage_given: |-
         The incorrect stage was given to the trigger plugin:
         Guest: %{guest_name}
-        Action: %{action}
+        Name:  %{name}
+        Type:  %{type}
         Stage: %{stage}
 
         This is an internal error that should be reported as a bug.
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 6735e219f..6fbba226f 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)
+      expect(subject.command).to eq(command.to_s)
     end
 
     it "uses default error behavior" do
diff --git a/test/unit/vagrant/action/builder_test.rb b/test/unit/vagrant/action/builder_test.rb
index 94f9d846a..8ebbe5b04 100644
--- a/test/unit/vagrant/action/builder_test.rb
+++ b/test/unit/vagrant/action/builder_test.rb
@@ -237,13 +237,19 @@ describe Vagrant::Action::Builder do
   end
 
   describe "action hooks" do
-    it "applies them properly" do
-      hook = double("hook")
-      allow(hook).to receive(:apply) do |builder|
-        builder.use appender_proc(2)
-      end
+    let(:hook) { double("hook") }
+    let(:manager) { Vagrant.plugin("2").manager }
 
-      data[:action_hooks] = [hook]
+    before do
+      allow(manager).to receive(:action_hooks).and_return([])
+    end
+
+    it "applies them properly" do
+      hook_proc = proc{ |h| h.append(appender_proc(2)) }
+      expect(manager).to receive(:action_hooks).with(:test_action).
+        and_return([hook_proc])
+
+      data[:action_name] = :test_action
 
       subject.use appender_proc(1)
       subject.call(data)
@@ -253,11 +259,16 @@ describe Vagrant::Action::Builder do
     end
 
     it "applies without prepend/append if it has already" do
-      hook = double("hook")
-      expect(hook).to receive(:apply).with(anything, { no_prepend_or_append: true }).once
+      hook_proc = proc{ |h| h.append(appender_proc(2)) }
+      expect(manager).to receive(:action_hooks).with(:test_action).
+        and_return([hook_proc])
 
-      data[:action_hooks] = [hook]
-      data[:action_hooks_already_ran] = true
+      data[:action_name] = :test_action
+
+      subject.use appender_proc(1)
+      subject.call(data.merge(action_hooks_already_ran: true))
+
+      expect(data[:data]).to eq([1])
       subject.call(data)
     end
   end
@@ -295,4 +306,165 @@ describe Vagrant::Action::Builder do
         "1_in", "2_in", "3_in", "3_out", "2_out", "1_out"])
     end
   end
+
+  describe "dynamic action hooks" do
+    class ActionOne
+      def initialize(app, env)
+        @app = app
+      end
+
+      def call(env)
+        env[:data] << 1 if env[:data]
+        @app.call(env)
+      end
+
+      def recover(env)
+        env[:recover] << 1
+      end
+    end
+
+    class ActionTwo
+      def initialize(app, env)
+        @app = app
+      end
+
+      def call(env)
+        env[:data] << 2 if env[:data]
+        @app.call(env)
+      end
+
+      def recover(env)
+        env[:recover] << 2
+      end
+    end
+
+    let(:data) { {data: []} }
+    let(:hook_action_name) { :action_two }
+
+    let(:plugin) do
+      h_name = hook_action_name
+      @plugin ||= Class.new(Vagrant.plugin("2")) do
+        name "Test Plugin"
+        action_hook(:before_test, h_name) do |hook|
+          hook.prepend(proc{ |env| env[:data] << :first })
+        end
+      end
+    end
+
+    before { plugin }
+
+    after do
+      Vagrant.plugin("2").manager.unregister(@plugin) if @plugin
+      @plugin = nil
+    end
+
+    it "should call hook before running action" do
+      instance = described_class.build(ActionTwo)
+      instance.call(data)
+      expect(data[:data].first).to eq(:first)
+      expect(data[:data].last).to eq(2)
+    end
+
+    context "when hook is appending to action" do
+      let(:plugin) do
+        @plugin ||= Class.new(Vagrant.plugin("2")) do
+          name "Test Plugin"
+          action_hook(:before_test, :action_two) do |hook|
+            hook.append(proc{ |env| env[:data] << :first })
+          end
+        end
+      end
+
+      it "should call hook after action when action is nested" do
+        instance = described_class.build(ActionTwo).use(described_class.build(ActionOne))
+        instance.call(data)
+        expect(data[:data][0]).to eq(2)
+        expect(data[:data][1]).to eq(:first)
+        expect(data[:data][2]).to eq(1)
+      end
+    end
+
+    context "when hook uses class name" do
+      let(:hook_action_name) { "ActionTwo" }
+
+      it "should execute the hook" do
+        instance = described_class.build(ActionTwo)
+        instance.call(data)
+        expect(data[:data]).to include(:first)
+      end
+    end
+
+    context "when action includes a namespace" do
+      module Vagrant
+        module Test
+          class ActionTest
+            def initialize(app, env)
+              @app = app
+            end
+
+            def call(env)
+              env[:data] << :test if env[:data]
+              @app.call(env)
+            end
+          end
+        end
+      end
+
+      let(:instance) { described_class.build(Vagrant::Test::ActionTest) }
+
+      context "when hook uses short snake case name" do
+        let(:hook_action_name) { :action_test }
+
+        it "should execute the hook" do
+          instance.call(data)
+          expect(data[:data]).to include(:first)
+        end
+      end
+
+      context "when hook uses partial snake case name" do
+        let(:hook_action_name) { :test_action_test }
+
+        it "should execute the hook" do
+          instance.call(data)
+          expect(data[:data]).to include(:first)
+        end
+      end
+
+      context "when hook uses full snake case name" do
+        let(:hook_action_name) { :vagrant_test_action_test }
+
+        it "should execute the hook" do
+          instance.call(data)
+          expect(data[:data]).to include(:first)
+        end
+      end
+
+      context "when hook uses short class name" do
+        let(:hook_action_name) { "ActionTest" }
+
+        it "should execute the hook" do
+          instance.call(data)
+          expect(data[:data]).to include(:first)
+        end
+      end
+
+      context "when hook uses partial namespace class name" do
+        let(:hook_action_name) { "Test::ActionTest" }
+
+        it "should execute the hook" do
+          instance.call(data)
+          expect(data[:data]).to include(:first)
+        end
+      end
+
+      context "when hook uses full namespace class name" do
+        let(:hook_action_name) { "Vagrant::Test::ActionTest" }
+
+        it "should execute the hook" do
+          instance.call(data)
+          expect(data[:data]).to include(:first)
+        end
+      end
+    end
+  end
 end
diff --git a/test/unit/vagrant/action/warden_test.rb b/test/unit/vagrant/action/warden_test.rb
index 439dead01..6cb649f4f 100644
--- a/test/unit/vagrant/action/warden_test.rb
+++ b/test/unit/vagrant/action/warden_test.rb
@@ -103,135 +103,4 @@ describe Vagrant::Action::Warden do
     # The recover should not have been called
     expect(data.key?(:recover)).not_to be
   end
-
-  describe "dynamic action hooks" do
-    let(:data) { {data: []} }
-    let(:hook_action_name) { :action_two }
-
-    let(:plugin) do
-      h_name = hook_action_name
-      @plugin ||= Class.new(Vagrant.plugin("2")) do
-        name "Test Plugin"
-        action_hook(:before_test, h_name) do |hook|
-          hook.prepend(proc{ |env| env[:data] << :first })
-        end
-      end
-    end
-
-    before { plugin }
-
-    after do
-      Vagrant.plugin("2").manager.unregister(@plugin) if @plugin
-      @plugin = nil
-    end
-
-    it "should call hook before running action" do
-      instance = described_class.new([ActionTwo], data)
-      instance.call(data)
-      expect(data[:data].first).to eq(:first)
-      expect(data[:data].last).to eq(2)
-    end
-
-    context "when hook is appending to action" do
-      let(:plugin) do
-        @plugin ||= Class.new(Vagrant.plugin("2")) do
-          name "Test Plugin"
-          action_hook(:before_test, :action_two) do |hook|
-            hook.append(proc{ |env| env[:data] << :first })
-          end
-        end
-      end
-
-      it "should call hook after action when action is nested" do
-        instance = described_class.new([described_class.new([ActionTwo], data), ActionOne], data)
-        instance.call(data)
-        expect(data[:data][0]).to eq(2)
-        expect(data[:data][1]).to eq(:first)
-        expect(data[:data][2]).to eq(1)
-      end
-    end
-
-    context "when hook uses class name" do
-      let(:hook_action_name) { "ActionTwo" }
-
-      it "should execute the hook" do
-        instance = described_class.new([ActionTwo], data)
-        instance.call(data)
-        expect(data[:data]).to include(:first)
-      end
-    end
-
-    context "when action includes a namespace" do
-      module Vagrant
-        module Test
-          class ActionTest
-            def initialize(app, env)
-              @app = app
-            end
-
-            def call(env)
-              env[:data] << :test if env[:data]
-              @app.call(env)
-            end
-          end
-        end
-      end
-
-      let(:instance) { described_class.new([Vagrant::Test::ActionTest], data) }
-
-      context "when hook uses short snake case name" do
-        let(:hook_action_name) { :action_test }
-
-        it "should execute the hook" do
-          instance.call(data)
-          expect(data[:data]).to include(:first)
-        end
-      end
-
-      context "when hook uses partial snake case name" do
-        let(:hook_action_name) { :test_action_test }
-
-        it "should execute the hook" do
-          instance.call(data)
-          expect(data[:data]).to include(:first)
-        end
-      end
-
-      context "when hook uses full snake case name" do
-        let(:hook_action_name) { :vagrant_test_action_test }
-
-        it "should execute the hook" do
-          instance.call(data)
-          expect(data[:data]).to include(:first)
-        end
-      end
-
-      context "when hook uses short class name" do
-        let(:hook_action_name) { "ActionTest" }
-
-        it "should execute the hook" do
-          instance.call(data)
-          expect(data[:data]).to include(:first)
-        end
-      end
-
-      context "when hook uses partial namespace class name" do
-        let(:hook_action_name) { "Test::ActionTest" }
-
-        it "should execute the hook" do
-          instance.call(data)
-          expect(data[:data]).to include(:first)
-        end
-      end
-
-      context "when hook uses full namespace class name" do
-        let(:hook_action_name) { "Vagrant::Test::ActionTest" }
-
-        it "should execute the hook" do
-          instance.call(data)
-          expect(data[:data]).to include(:first)
-        end
-      end
-    end
-  end
 end
diff --git a/test/unit/vagrant/cli_test.rb b/test/unit/vagrant/cli_test.rb
index db7e7e9bd..f0d31a58f 100644
--- a/test/unit/vagrant/cli_test.rb
+++ b/test/unit/vagrant/cli_test.rb
@@ -59,8 +59,9 @@ describe Vagrant::CLI do
 
     it "fires triggers, if enabled" do
       allow(Vagrant::Util::Experimental).to receive(:feature_enabled?).
-        with("typed_triggers").and_return("true")
-      allow(triggers).to receive(:fire_triggers)
+        with("typed_triggers").and_return(true)
+      allow(triggers).to receive(:fire)
+      allow(triggers).to receive(:find).and_return([double("trigger-result")])
 
       commands[:destroy] = [command_lambda("destroy", 42), {}]
 
@@ -68,7 +69,7 @@ describe Vagrant::CLI do
 
       subject = described_class.new(["destroy"], env)
 
-      expect(triggers).to receive(:fire_triggers).twice
+      expect(triggers).to receive(:fire).twice
 
       expect(subject).not_to receive(:help)
       expect(subject.execute).to eql(42)
@@ -76,13 +77,13 @@ describe Vagrant::CLI do
 
     it "does not fire triggers if disabled" do
       allow(Vagrant::Util::Experimental).to receive(:feature_enabled?).
-        with("typed_triggers").and_return("false")
+        with("typed_triggers").and_return(false)
 
       commands[:destroy] = [command_lambda("destroy", 42), {}]
 
       subject = described_class.new(["destroy"], env)
 
-      expect(triggers).not_to receive(:fire_triggers)
+      expect(triggers).not_to receive(:fire)
 
       expect(subject).not_to receive(:help)
       expect(subject.execute).to eql(42)
diff --git a/test/unit/vagrant/environment_test.rb b/test/unit/vagrant/environment_test.rb
index 834dd418d..3694560d6 100644
--- a/test/unit/vagrant/environment_test.rb
+++ b/test/unit/vagrant/environment_test.rb
@@ -987,7 +987,6 @@ VF
         Dir.chdir(temp_dir) do
           instance = described_class.new(local_data_path: "foo")
           expect(instance.local_data_path).to eq(instance.cwd.join("foo"))
-          expect(File.exist?(instance.local_data_path)).to be(false)
         end
       end
     end
@@ -1108,7 +1107,6 @@ VF
         end
 
         expect { instance }.to_not raise_error
-        expect(Pathname.new(local_data_path)).to_not be_exist
       end
 
       it "should upgrade all active VMs" do
diff --git a/test/unit/vagrant/plugin/v2/trigger_test.rb b/test/unit/vagrant/plugin/v2/trigger_test.rb
index 370b4b5db..6a4fff094 100644
--- a/test/unit/vagrant/plugin/v2/trigger_test.rb
+++ b/test/unit/vagrant/plugin/v2/trigger_test.rb
@@ -36,31 +36,99 @@ describe Vagrant::Plugin::V2::Trigger do
     triggers.finalize!
   end
 
-
   let(:subject) { described_class.new(env, triggers, machine, ui) }
 
-  context "#fire_triggers" do
-    it "raises an error if an inproper stage is given" do
-      expect{ subject.fire_triggers(:up, :not_real, "guest", :action) }.
+  describe "#fire" do
+    it "raises an error if an improper stage is given" do
+      expect{ subject.fire(:up, :not_real, "guest", :action) }.
        to raise_error(Vagrant::Errors::TriggersNoStageGiven)
     end
 
     it "does not fire triggers if community plugin is detected" do
       allow(subject).to receive(:community_plugin_detected?).and_return(true)
 
-      expect(subject).not_to receive(:fire)
-      subject.fire_triggers(:up, :before, "guest", :action)
+      expect(subject).not_to receive(:execute)
+      subject.fire(:up, :before, "guest", :action)
     end
 
     it "does fire triggers if community plugin is not detected" do
       allow(subject).to receive(:community_plugin_detected?).and_return(false)
 
-      expect(subject).to receive(:fire)
-      subject.fire_triggers(:up, :before, "guest", :action)
+      expect(subject).to receive(:execute)
+      subject.fire(:up, :before, "guest", :action)
     end
   end
 
-  context "#filter_triggers" do
+  describe "#find" do
+    it "raises an error if an improper stage is given" do
+      expect { subject.find(:up, :not_real, "guest", :action) }.
+        to raise_error(Vagrant::Errors::TriggersNoStageGiven)
+    end
+
+    it "returns empty array when no triggers are found" do
+      expect(subject.find(:halt, :after, "guest", :action)).to be_empty
+    end
+
+    it "returns items in array when triggers are found" do
+      expect(subject.find(:halt, :before, "guest", :action)).not_to be_empty
+    end
+
+    it "returns the execpted number of items in the array when triggers are found" do
+      expect(subject.find(:halt, :before, "guest", :action).count).to eq(1)
+    end
+
+    it "filters all found triggers" do
+      expect(subject).to receive(:filter_triggers)
+      subject.find(:halt, :before, "guest", :action)
+    end
+
+    it "should not attempt to match hook name with non-hook type" do
+      expect(subject).not_to receive(:matched_hook?)
+      subject.find(:halt, :before, "guest", :action)
+    end
+
+    context "with hook type" do
+      before do
+        triggers.before(:environment_load, hash_block.merge(type: :hook))
+        triggers.before(Vagrant::Action::Builtin::SyncedFolders, hash_block.merge(type: :hook))
+        triggers.finalize!
+      end
+
+      it "returns empty array when no triggers are found" do
+        expect(subject.find(:environment_unload, :before, "guest", :hook)).to be_empty
+      end
+
+      it "returns items in array when triggers are found" do
+        expect(subject.find(:environment_load, :before, "guest", :hook).size).to eq(1)
+      end
+
+      it "should locate hook trigger using class constant" do
+        expect(subject.find(Vagrant::Action::Builtin::SyncedFolders, :before, "guest", :hook)).
+          not_to be_empty
+      end
+
+      it "should locate hook trigger using string" do
+        expect(subject.find("environment_load", :before, "guest", :hook)).not_to be_empty
+      end
+
+      it "should locate hook trigger using full converted name" do
+        expect(subject.find(:vagrant_action_builtin_synced_folders, :before, "guest", :hook)).
+          not_to be_empty
+      end
+
+      it "should locate hook trigger using partial suffix converted name" do
+        expect(subject.find(:builtin_synced_folders, :before, "guest", :hook)).
+          not_to be_empty
+      end
+
+      it "should not locate hook trigger using partial prefix converted name" do
+        expect(subject.find(:vagrant_action, :before, "guest", :hook)).
+          to be_empty
+      end
+    end
+  end
+
+  describe "#filter_triggers" do
     it "returns all triggers if no constraints" do
       before_triggers = triggers.before_triggers
       filtered_triggers = subject.send(:filter_triggers, before_triggers, "guest", :action)
@@ -101,17 +169,17 @@ describe Vagrant::Plugin::V2::Trigger do
     end
   end
 
-  context "#fire" do
+  describe "#execute" do
     it "calls the corresponding trigger methods if options set" do
       expect(subject).to receive(:info).twice
       expect(subject).to receive(:warn).once
       expect(subject).to receive(:run).twice
       expect(subject).to receive(:run_remote).once
-      subject.send(:fire, triggers.before_triggers, "guest")
+      subject.send(:execute, triggers.before_triggers)
     end
   end
 
-  context "#info" do
+  describe "#info" do
     let(:message) { "Printing some info" }
 
     it "prints messages at INFO" do
@@ -125,7 +193,7 @@ describe Vagrant::Plugin::V2::Trigger do
     end
   end
 
-  context "#warn" do
+  describe "#warn" do
     let(:message) { "Printing some warnings" }
 
     it "prints messages at WARN" do
@@ -139,7 +207,7 @@ describe Vagrant::Plugin::V2::Trigger do
     end
   end
 
-  context "#run" do
+  describe "#run" do
     let(:trigger_run) { VagrantPlugins::Kernel_V2::TriggerConfig.new }
     let(:shell_block) { {info: "hi", run: {inline: "echo 'hi'", env: {"KEY"=>"VALUE"}}} }
     let(:shell_block_exit_codes) {
@@ -305,7 +373,7 @@ describe Vagrant::Plugin::V2::Trigger do
     end
   end
 
-  context "#run_remote" do
+  describe "#run_remote" do
     let (:trigger_run) { VagrantPlugins::Kernel_V2::TriggerConfig.new }
     let (:shell_block) { {info: "hi", run_remote: {inline: "echo 'hi'", env: {"KEY"=>"VALUE"}}} }
     let (:path_block) { {warn: "bye",
@@ -412,7 +480,7 @@ describe Vagrant::Plugin::V2::Trigger do
     end
   end
 
-  context "#trigger_abort" do
+  describe "#trigger_abort" do
     it "system exits when called" do
       allow(Process).to receive(:exit!).and_return(true)
       output = ""
@@ -456,7 +524,7 @@ describe Vagrant::Plugin::V2::Trigger do
     end
   end
 
-  context "#ruby" do
+  describe "#ruby" do
     let(:trigger_run) { VagrantPlugins::Kernel_V2::TriggerConfig.new }
     let(:block) { proc{var = 1+1} }
     let(:ruby_trigger) { {info: "hi", ruby: block} }
@@ -471,4 +539,18 @@ describe Vagrant::Plugin::V2::Trigger do
       subject.send(:execute_ruby, block)
     end
   end
+
+  describe "#nameify" do
+    it "should return empty string when object" do
+      expect(subject.send(:nameify, "")).to eq("")
+    end
+
+    it "should return name of class" do
+      expect(subject.send(:nameify, String)).to eq("String")
+    end
+
+    it "should return empty string when class has no name" do
+      expect(subject.send(:nameify, Class.new)).to eq("")
+    end
+  end
 end

From 8fad4865bbb8312fa0b42dbe95b1af3160b54ff7 Mon Sep 17 00:00:00 2001
From: Chris Roberts <croberts@hashicorp.com>
Date: Mon, 23 Mar 2020 17:06:29 -0700
Subject: [PATCH 2/9] Update before trigger action to be generic trigger action

---
 lib/vagrant/action/builtin/before_trigger.rb | 26 ++++++++++++--------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/lib/vagrant/action/builtin/before_trigger.rb b/lib/vagrant/action/builtin/before_trigger.rb
index 74a6f8754..8a79bc47c 100644
--- a/lib/vagrant/action/builtin/before_trigger.rb
+++ b/lib/vagrant/action/builtin/before_trigger.rb
@@ -1,26 +1,32 @@
 module Vagrant
   module Action
     module Builtin
-      # This class is intended to be used by the Action::Warden class for executing
-      # action triggers before any given action.
-      class BeforeTriggerAction
-        # @param [Symbol] action_name - The action class name to fire trigger on
-        # @param [Vagrant::Plugin::V2::Triger] triggers - trigger object
-        def initialize(app, env, action_name, triggers, type=:action)
+      # This class is used within the Builder class for injecting triggers into
+      # different parts of the call stack.
+      class Trigger
+        # @param [Class, String, Symbol] name Name of trigger to fire
+        # @param [Vagrant::Plugin::V2::Triger] triggers Trigger object
+        # @param [Symbol] timing When trigger should fire (:before/:after)
+        # @param [Symbol] type Type of trigger
+        def initialize(app, env, name, triggers, timing, type=:action)
           @app         = app
           @env         = env
           @triggers    = triggers
-          @action_name = action_name
+          @name        = name
+          @timing      = timing
           @type        = type
+
+          if ![:before, :after].include?(at)
+            raise ArgumentError,
+              "Invalid value provided for `timing` (allowed: :before or :after)"
+          end
         end
 
         def call(env)
           machine = env[:machine]
           machine_name = machine.name if machine
 
-          @triggers.fire(@action_name, :before, machine_name, @type) if
-            Vagrant::Util::Experimental.feature_enabled?("typed_triggers");
-
+          @triggers.fire(@name, @timing, machine_name, @type)
           # Carry on
           @app.call(env)
         end

From 5f6ac0a6e4a6e09b65d83bf414aef70445eee563 Mon Sep 17 00:00:00 2001
From: Chris Roberts <croberts@hashicorp.com>
Date: Mon, 23 Mar 2020 17:08:44 -0700
Subject: [PATCH 3/9] Update trigger action files

---
 lib/vagrant/action/builtin/after_trigger.rb   | 33 -------------------
 .../builtin/{before_trigger.rb => trigger.rb} |  0
 2 files changed, 33 deletions(-)
 delete mode 100644 lib/vagrant/action/builtin/after_trigger.rb
 rename lib/vagrant/action/builtin/{before_trigger.rb => trigger.rb} (100%)

diff --git a/lib/vagrant/action/builtin/after_trigger.rb b/lib/vagrant/action/builtin/after_trigger.rb
deleted file mode 100644
index 46a2d3d91..000000000
--- a/lib/vagrant/action/builtin/after_trigger.rb
+++ /dev/null
@@ -1,33 +0,0 @@
-module Vagrant
-  module Action
-    module Builtin
-      # This class is intended to be used by the Action::Warden class for executing
-      # action triggers before any given action.
-      #
-      # @param [Symbol] action_name - name to fire trigger on
-      # @param [Vagrant::Plugin::V2::Triger] triggers - trigger object
-      class AfterTriggerAction
-        # @param [Symbol] action_name - The action class name to fire trigger on
-        # @param [Vagrant::Plugin::V2::Triger] triggers - trigger object
-        def initialize(app, env, action_name, triggers, type=:action)
-          @app         = app
-          @env         = env
-          @triggers    = triggers
-          @action_name = action_name
-          @type        = type
-        end
-
-        def call(env)
-          machine = env[:machine]
-          machine_name = machine.name if machine
-
-          @triggers.fire(@action_name, :after, machine_name, @type) if
-            Vagrant::Util::Experimental.feature_enabled?("typed_triggers");
-
-          # Carry on
-          @app.call(env)
-        end
-      end
-    end
-  end
-end
diff --git a/lib/vagrant/action/builtin/before_trigger.rb b/lib/vagrant/action/builtin/trigger.rb
similarity index 100%
rename from lib/vagrant/action/builtin/before_trigger.rb
rename to lib/vagrant/action/builtin/trigger.rb

From 505715500dc9d93547dd6c88549eb34829f6e298 Mon Sep 17 00:00:00 2001
From: Chris Roberts <croberts@hashicorp.com>
Date: Mon, 23 Mar 2020 17:09:46 -0700
Subject: [PATCH 4/9] Include autoloading for trigger constant

---
 lib/vagrant/action.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/vagrant/action.rb b/lib/vagrant/action.rb
index a646109c2..68a1c62ef 100644
--- a/lib/vagrant/action.rb
+++ b/lib/vagrant/action.rb
@@ -35,6 +35,7 @@ module Vagrant
       autoload :SSHRun,  "vagrant/action/builtin/ssh_run"
       autoload :SyncedFolders, "vagrant/action/builtin/synced_folders"
       autoload :SyncedFolderCleanup, "vagrant/action/builtin/synced_folder_cleanup"
+      autoload :Trigger, "vagrant/action/builtin/trigger"
       autoload :WaitForCommunicator, "vagrant/action/builtin/wait_for_communicator"
     end
 

From 4b6536885255bcfb3d753e5e14854b9ee34296c7 Mon Sep 17 00:00:00 2001
From: Chris Roberts <croberts@hashicorp.com>
Date: Mon, 23 Mar 2020 17:10:17 -0700
Subject: [PATCH 5/9] Update builder to use generic trigger action

---
 lib/vagrant/action/builder.rb | 36 +++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/lib/vagrant/action/builder.rb b/lib/vagrant/action/builder.rb
index 00c5cf696..bafae1f56 100644
--- a/lib/vagrant/action/builder.rb
+++ b/lib/vagrant/action/builder.rb
@@ -181,25 +181,25 @@ module Vagrant
 
           # Start with adding any action triggers that may be defined
           if triggers && !triggers.find(action, :before, machine_name, :action).empty?
-            hook.prepend(Vagrant::Action::Builtin::BeforeTriggerAction,
-              action.name, triggers)
+            hook.prepend(Vagrant::Action::Builtin::Trigger,
+              action.name, triggers, :before, :action)
           end
 
           if triggers && !triggers.find(action, :after, machine_name, :action).empty?
-            hook.append(Vagrant::Action::Builtin::AfterTriggerAction,
-              action.name, triggers)
+            hook.append(Vagrant::Action::Builtin::Trigger,
+              action.name, triggers, :after, :action)
           end
 
           # Next look for any hook triggers that may be defined against
           # the dynamically generated action class hooks
           if triggers && !triggers.find(action, :before, machine_name, :hook).empty?
-            hook.prepend(Vagrant::Action::Builtin::BeforeTriggerAction,
-              action.name, triggers, :hook)
+            hook.prepend(Vagrant::Action::Builtin::Trigger,
+              action.name, triggers, :before, :hook)
           end
 
           if triggers && !triggers.find(action, :after, machine_name, :hook).empty?
-            hook.append(Vagrant::Action::Builtin::AfterTriggerAction,
-              action.name, triggers, :hook)
+            hook.append(Vagrant::Action::Builtin::Trigger,
+              action.name, triggers, :after, :hook)
           end
 
           # Finally load any registered hooks for dynamically generated
@@ -231,12 +231,12 @@ module Vagrant
         # Start with loading any hook triggers if applicable
         if Vagrant::Util::Experimental.feature_enabled?("typed_triggers") && env[:triggers]
           if !env[:triggers].find(env[:action_name], :before, machine_name, :hook).empty?
-            hook.prepend(Vagrant::Action::Builtin::BeforeTriggerAction,
-              env[:action_name].to_sym, env[:triggers])
+            hook.prepend(Vagrant::Action::Builtin::Trigger,
+              env[:action_name], :env[:triggers], :before, :hook)
           end
           if !env[:triggers].find(env[:action_name], :after, machine_name, :hook).empty?
-            hook.append(Vagrant::Action::Builtin::AfterTriggerAction,
-              env[:action_name].to_sym, env[:triggers])
+            hook.append(Vagrant::Action::Builtin::Trigger,
+              env[:action_name], env[:triggers], :after, :hook)
           end
         end
 
@@ -249,13 +249,13 @@ module Vagrant
 
         # Finally load any action triggers defined
         if env[:triggers]
-          if !env[:triggers].find(env[:raw_action_name], :before, machine_name, :action)
-            hook.prepend(Vagrant::Action::Builtin::BeforeTriggerAction,
-              env[:raw_action_name].to_sym, env[:triggers])
+          if !env[:triggers].find(env[:raw_action_name], :before, machine_name, :action).empty?
+            hook.prepend(Vagrant::Action::Builtin::Trigger,
+              env[:raw_action_name], env[:triggers], :before, :action)
           end
-          if !env[:triggers].find(env[:raw_action_name], :after, machine_name, :action)
-            hook.append(Vagrant::Action::Builtin::AfterTriggerAction,
-              env[:raw_action_name].to_sym, env[:triggers])
+          if !env[:triggers].find(env[:raw_action_name], :after, machine_name, :action).empty?
+            hook.append(Vagrant::Action::Builtin::Trigger,
+              env[:raw_action_name], env[:triggers], :after, :action)
           end
         end
 

From 2e0e772897dd6ef7b7e2e9290bf79d0495aaa036 Mon Sep 17 00:00:00 2001
From: Chris Roberts <croberts@hashicorp.com>
Date: Tue, 24 Mar 2020 14:32:34 -0700
Subject: [PATCH 6/9] Add test coverage on trigger action

---
 lib/vagrant/action/builtin/trigger.rb         |  2 +-
 .../vagrant/action/builtin/trigger_test.rb    | 63 +++++++++++++++++++
 2 files changed, 64 insertions(+), 1 deletion(-)
 create mode 100644 test/unit/vagrant/action/builtin/trigger_test.rb

diff --git a/lib/vagrant/action/builtin/trigger.rb b/lib/vagrant/action/builtin/trigger.rb
index 8a79bc47c..0591ea7b4 100644
--- a/lib/vagrant/action/builtin/trigger.rb
+++ b/lib/vagrant/action/builtin/trigger.rb
@@ -16,7 +16,7 @@ module Vagrant
           @timing      = timing
           @type        = type
 
-          if ![:before, :after].include?(at)
+          if ![:before, :after].include?(timing)
             raise ArgumentError,
               "Invalid value provided for `timing` (allowed: :before or :after)"
           end
diff --git a/test/unit/vagrant/action/builtin/trigger_test.rb b/test/unit/vagrant/action/builtin/trigger_test.rb
new file mode 100644
index 000000000..a389c944c
--- /dev/null
+++ b/test/unit/vagrant/action/builtin/trigger_test.rb
@@ -0,0 +1,63 @@
+require File.expand_path("../../../../base", __FILE__)
+
+describe Vagrant::Action::Builtin::Trigger do
+  let(:app) { lambda { |env| } }
+  let(:env) { {machine: machine} }
+  let(:machine) { nil }
+  let(:triggers) { double("triggers") }
+  let(:name) { "trigger-name" }
+  let(:timing) { :before }
+  let(:type) { :action }
+
+  let(:subject) { described_class.
+      new(app, env, name, triggers, timing, type) }
+
+  before do
+    allow(triggers).to receive(:fire)
+    allow(app).to receive(:call)
+  end
+
+
+  it "should properly create a new instance" do
+    expect(subject).to be_a(described_class)
+  end
+
+  it "should fire triggers" do
+    expect(triggers).to receive(:fire)
+    subject.call(env)
+  end
+
+  it "should fire triggers without machine name" do
+    expect(triggers).to receive(:fire).with(name, timing, nil, type)
+    subject.call(env)
+  end
+
+  context "when machine is provided" do
+    let(:machine) { double("machine", name: "machine-name") }
+
+    it "should include machine name when firing triggers" do
+      expect(triggers).to receive(:fire).with(name, timing, "machine-name", type)
+      subject.call(env)
+    end
+  end
+
+  context "when timing is :before" do
+    it "should not error" do
+      expect { subject }.not_to raise_error
+    end
+  end
+
+  context "when timing is :after" do
+    it "should not error" do
+      expect { subject }.not_to raise_error
+    end
+  end
+
+  context "when timing is not :before or :after" do
+    let(:timing) { :unknown }
+
+    it "should raise error" do
+      expect { subject }.to raise_error(ArgumentError)
+    end
+  end
+end

From bcbbc825e04b0bcaaac84ffab08f11d228655bfa Mon Sep 17 00:00:00 2001
From: Chris Roberts <croberts@hashicorp.com>
Date: Tue, 24 Mar 2020 17:15:41 -0700
Subject: [PATCH 7/9] Add test coverage on builder

---
 lib/vagrant/action/builder.rb            |   2 +-
 lib/vagrant/action/warden.rb             |   2 -
 lib/vagrant/plugin/v2/plugin.rb          |   1 +
 test/unit/vagrant/action/builder_test.rb | 289 +++++++++++++++++++++++
 4 files changed, 291 insertions(+), 3 deletions(-)

diff --git a/lib/vagrant/action/builder.rb b/lib/vagrant/action/builder.rb
index bafae1f56..13fbbbd87 100644
--- a/lib/vagrant/action/builder.rb
+++ b/lib/vagrant/action/builder.rb
@@ -232,7 +232,7 @@ module Vagrant
         if Vagrant::Util::Experimental.feature_enabled?("typed_triggers") && env[:triggers]
           if !env[:triggers].find(env[:action_name], :before, machine_name, :hook).empty?
             hook.prepend(Vagrant::Action::Builtin::Trigger,
-              env[:action_name], :env[:triggers], :before, :hook)
+              env[:action_name], env[:triggers], :before, :hook)
           end
           if !env[:triggers].find(env[:action_name], :after, machine_name, :hook).empty?
             hook.append(Vagrant::Action::Builtin::Trigger,
diff --git a/lib/vagrant/action/warden.rb b/lib/vagrant/action/warden.rb
index 32cb6abfd..828e317fc 100644
--- a/lib/vagrant/action/warden.rb
+++ b/lib/vagrant/action/warden.rb
@@ -1,7 +1,5 @@
 require "log4r"
 require 'vagrant/util/experimental'
-require 'vagrant/action/builtin/before_trigger'
-require 'vagrant/action/builtin/after_trigger'
 
 module Vagrant
   module Action
diff --git a/lib/vagrant/plugin/v2/plugin.rb b/lib/vagrant/plugin/v2/plugin.rb
index cab042437..9bc166268 100644
--- a/lib/vagrant/plugin/v2/plugin.rb
+++ b/lib/vagrant/plugin/v2/plugin.rb
@@ -71,6 +71,7 @@ module Vagrant
         # @return [Array] List of the hooks for the given action.
         def self.action_hook(name, hook_name=nil, &block)
           # The name is currently not used but we want it for the future.
+          hook_name = hook_name.to_s if hook_name
 
           hook_name ||= ALL_ACTIONS
           components.action_hooks[hook_name.to_sym] << block
diff --git a/test/unit/vagrant/action/builder_test.rb b/test/unit/vagrant/action/builder_test.rb
index 8ebbe5b04..dfe220074 100644
--- a/test/unit/vagrant/action/builder_test.rb
+++ b/test/unit/vagrant/action/builder_test.rb
@@ -467,4 +467,293 @@ describe Vagrant::Action::Builder do
       end
     end
   end
+
+  describe "#apply_dynamic_updates" do
+    let(:env) { {triggers: triggers, machine: machine} }
+    let(:machine) { nil }
+    let(:triggers) { nil }
+
+    let(:subject) do
+      @subject ||= described_class.new.tap do |b|
+        b.use Vagrant::Action::Builtin::EnvSet
+        b.use Vagrant::Action::Builtin::Confirm
+      end
+    end
+
+    after { @subject = nil }
+
+    it "should not modify the builder stack by default" do
+      s1 = subject.stack.dup
+      subject.apply_dynamic_updates(env)
+      s2 = subject.stack.dup
+      expect(s1).to eq(s2)
+    end
+
+    context "when an action hooks is defined" do
+      let(:plugin) do
+        @plugin ||= Class.new(Vagrant.plugin("2")) do
+          name "Test Plugin"
+          action_hook(:before_action, Vagrant::Action::Builtin::Confirm) do |hook|
+            hook.prepend(Vagrant::Action::Builtin::Call)
+          end
+        end
+      end
+
+      before { plugin }
+
+      after do
+        Vagrant.plugin("2").manager.unregister(@plugin) if @plugin
+        @plugin = nil
+      end
+
+      it "should modify the builder stack" do
+        s1 = subject.stack.dup
+        subject.apply_dynamic_updates(env)
+        s2 = subject.stack.dup
+        expect(s1).not_to eq(s2)
+      end
+
+      it "should add new action to the middle of the call stack" do
+        subject.apply_dynamic_updates(env)
+        expect(subject.stack[1].first).to eq(Vagrant::Action::Builtin::Call)
+      end
+    end
+
+    context "when triggers are enabled" do
+      let(:triggers) { double("triggers") }
+
+      before do
+        allow(Vagrant::Util::Experimental).to receive(:feature_enabled?).
+          with("typed_triggers").and_return(true)
+        allow(triggers).to receive(:find).and_return([])
+      end
+
+      it "should not modify the builder stack by default" do
+        s1 = subject.stack.dup
+        subject.apply_dynamic_updates(env)
+        s2 = subject.stack.dup
+        expect(s1).to eq(s2)
+      end
+
+      context "when triggers are found" do
+        let(:action) { Vagrant::Action::Builtin::EnvSet }
+
+        before { expect(triggers).to receive(:find).
+            with(action, timing, nil, type).and_return([true]) }
+
+        context "for action type" do
+          let(:type) { :action }
+
+          context "for before timing" do
+            let(:timing) { :before }
+
+            it "should add trigger action to start of stack" do
+              subject.apply_dynamic_updates(env)
+              expect(subject.stack[0].first).to eq(Vagrant::Action::Builtin::Trigger)
+            end
+
+            it "should have timing and type arguments" do
+              subject.apply_dynamic_updates(env)
+              args = subject.stack[0][1]
+              expect(args).to include(type)
+              expect(args).to include(timing)
+              expect(args).to include(action.to_s)
+            end
+          end
+
+          context "for after timing" do
+            let(:timing) { :after }
+
+            it "should add trigger action to middle of stack" do
+              subject.apply_dynamic_updates(env)
+              expect(subject.stack[1].first).to eq(Vagrant::Action::Builtin::Trigger)
+            end
+
+            it "should have timing and type arguments" do
+              subject.apply_dynamic_updates(env)
+              args = subject.stack[1][1]
+              expect(args).to include(type)
+              expect(args).to include(timing)
+              expect(args).to include(action.to_s)
+            end
+          end
+        end
+
+        context "for hook type" do
+          let(:type) { :hook }
+
+          context "for before timing" do
+            let(:timing) { :before }
+
+            it "should add trigger action to start of stack" do
+              subject.apply_dynamic_updates(env)
+              expect(subject.stack[0].first).to eq(Vagrant::Action::Builtin::Trigger)
+            end
+
+            it "should have timing and type arguments" do
+              subject.apply_dynamic_updates(env)
+              args = subject.stack[0][1]
+              expect(args).to include(type)
+              expect(args).to include(timing)
+              expect(args).to include(action.to_s)
+            end
+          end
+
+          context "for after timing" do
+            let(:timing) { :after }
+
+            it "should add trigger action to middle of stack" do
+              subject.apply_dynamic_updates(env)
+              expect(subject.stack[1].first).to eq(Vagrant::Action::Builtin::Trigger)
+            end
+
+            it "should have timing and type arguments" do
+              subject.apply_dynamic_updates(env)
+              args = subject.stack[1][1]
+              expect(args).to include(type)
+              expect(args).to include(timing)
+              expect(args).to include(action.to_s)
+            end
+          end
+        end
+      end
+    end
+  end
+
+  describe "#apply_action_name" do
+    let(:env) { {triggers: triggers, machine: machine, action_name: action_name, raw_action_name: raw_action_name} }
+    let(:raw_action_name) { :up }
+    let(:action_name) { "machine_#{raw_action_name}".to_sym }
+    let(:machine) { nil }
+    let(:triggers) { double("triggers") }
+
+    let(:subject) do
+      @subject ||= described_class.new.tap do |b|
+        b.use Vagrant::Action::Builtin::EnvSet
+        b.use Vagrant::Action::Builtin::Confirm
+      end
+    end
+
+    before { allow(triggers).to receive(:find).and_return([]) }
+    after { @subject = nil }
+
+    it "should mark action hooks applied within env" do
+      subject.apply_action_name(env)
+      expect(env[:action_hooks_already_ran]).to be_truthy
+    end
+
+    context "when a plugin has added an action hook" do
+      let(:plugin) do
+        @plugin ||= Class.new(Vagrant.plugin("2")) do
+          name "Test Plugin"
+          action_hook(:before_action, :machine_up) do |hook|
+            hook.prepend(Vagrant::Action::Builtin::Call)
+          end
+        end
+      end
+
+      before { plugin }
+
+      after do
+        Vagrant.plugin("2").manager.unregister(@plugin) if @plugin
+        @plugin = nil
+      end
+
+      it "should add new action to the call stack" do
+        subject.apply_action_name(env)
+        expect(subject.stack[0].first).to eq(Vagrant::Action::Builtin::Call)
+      end
+
+      it "should only add new action to the call stack once" do
+        subject.apply_action_name(env)
+        subject.apply_action_name(env)
+        expect(subject.stack[0].first).to eq(Vagrant::Action::Builtin::Call)
+        expect(subject.stack[1].first).not_to eq(Vagrant::Action::Builtin::Call)
+      end
+    end
+
+    context "when trigger has been defined for raw action" do
+      before do
+        expect(triggers).to receive(:find).with(raw_action_name, timing, nil, :action).
+          and_return([true])
+      end
+
+      context "when timing is before" do
+        let(:timing) { :before }
+
+        it "should add a trigger action to the start of the stack" do
+          subject.apply_action_name(env)
+          expect(subject.stack[0].first).to eq(Vagrant::Action::Builtin::Trigger)
+        end
+
+        it "should include arguments to the trigger action" do
+          subject.apply_action_name(env)
+          args = subject.stack[0][1]
+          expect(args).to include(raw_action_name)
+          expect(args).to include(timing)
+          expect(args).to include(:action)
+        end
+      end
+
+      context "when timing is after" do
+        let(:timing) { :after }
+
+        it "should add a trigger action to the end of the stack" do
+          subject.apply_action_name(env)
+          expect(subject.stack.last.first).to eq(Vagrant::Action::Builtin::Trigger)
+        end
+
+        it "should include arguments to the trigger action" do
+          subject.apply_action_name(env)
+          args = subject.stack.last[1]
+          expect(args).to include(raw_action_name)
+          expect(args).to include(timing)
+          expect(args).to include(:action)
+        end
+      end
+    end
+
+    context "when trigger has been defined for hook" do
+      before do
+        allow(Vagrant::Util::Experimental).to receive(:feature_enabled?).
+          with("typed_triggers").and_return(true)
+        expect(triggers).to receive(:find).with(action_name, timing, nil, :hook).
+          and_return([true])
+      end
+
+      context "when timing is before" do
+        let(:timing) { :before }
+
+        it "should add a trigger action to the start of the stack" do
+          subject.apply_action_name(env)
+          expect(subject.stack[0].first).to eq(Vagrant::Action::Builtin::Trigger)
+        end
+
+        it "should include arguments to the trigger action" do
+          subject.apply_action_name(env)
+          args = subject.stack[0][1]
+          expect(args).to include(action_name)
+          expect(args).to include(timing)
+          expect(args).to include(:hook)
+        end
+      end
+
+      context "when timing is after" do
+        let(:timing) { :after }
+
+        it "should add a trigger action to the end of the stack" do
+          subject.apply_action_name(env)
+          expect(subject.stack.last.first).to eq(Vagrant::Action::Builtin::Trigger)
+        end
+
+        it "should include arguments to the trigger action" do
+          subject.apply_action_name(env)
+          args = subject.stack.last[1]
+          expect(args).to include(action_name)
+          expect(args).to include(timing)
+          expect(args).to include(:hook)
+        end
+      end
+    end
+  end
 end

From 217f2530db99137d7951dfc95124dba82d30c0e3 Mon Sep 17 00:00:00 2001
From: Chris Roberts <croberts@hashicorp.com>
Date: Thu, 26 Mar 2020 17:20:55 -0700
Subject: [PATCH 8/9] Use machine specific triggers instance when machine is
 available in runner

---
 lib/vagrant/action/runner.rb            |  5 +--
 lib/vagrant/machine.rb                  |  5 +++
 lib/vagrant/plugin/v2/trigger.rb        |  2 +-
 plugins/kernel_v2/config/vm_trigger.rb  |  5 +--
 test/unit/vagrant/action/runner_test.rb | 45 +++++++++++++++++++++++++
 5 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/lib/vagrant/action/runner.rb b/lib/vagrant/action/runner.rb
index d9b791a66..dd36588e6 100644
--- a/lib/vagrant/action/runner.rb
+++ b/lib/vagrant/action/runner.rb
@@ -40,11 +40,12 @@ module Vagrant
         # hash above.
         env = environment[:env]
         machine = environment[:machine]
-        machine_name = machine.name if machine
+
+        environment[:triggers] = machine.triggers if machine
 
         if env
           ui = Vagrant::UI::Prefixed.new(env.ui, "vagrant")
-          environment[:triggers] = Vagrant::Plugin::V2::Trigger.
+          environment[:triggers] ||= Vagrant::Plugin::V2::Trigger.
             new(env, env.vagrantfile.config.trigger, machine, ui)
         end
 
diff --git a/lib/vagrant/machine.rb b/lib/vagrant/machine.rb
index a593b0fff..f782a60f9 100644
--- a/lib/vagrant/machine.rb
+++ b/lib/vagrant/machine.rb
@@ -62,6 +62,11 @@ module Vagrant
     # @return [Hash]
     attr_reader :provider_options
 
+    # The triggers with machine specific configuration applied
+    #
+    # @return [Vagrant::Plugin::V2::Trigger]
+    attr_reader :triggers
+
     # The UI for outputting in the scope of this machine.
     #
     # @return [UI]
diff --git a/lib/vagrant/plugin/v2/trigger.rb b/lib/vagrant/plugin/v2/trigger.rb
index abee675df..0a6919d2c 100644
--- a/lib/vagrant/plugin/v2/trigger.rb
+++ b/lib/vagrant/plugin/v2/trigger.rb
@@ -36,7 +36,7 @@ module Vagrant
         # @param [Symbol] name Name of `type` thing to fire trigger on
         # @param [Symbol] stage :before or :after
         # @param [String] guest The guest that invoked firing the triggers
-        # @param [Symbol] type Type of trigger to fire
+        # @param [Symbol] type Type of trigger to fire (:action, :hook, :command)
         def fire(name, stage, guest, type)
           if community_plugin_detected?
             @logger.warn("Community plugin `vagrant-triggers detected, so core triggers will not fire")
diff --git a/plugins/kernel_v2/config/vm_trigger.rb b/plugins/kernel_v2/config/vm_trigger.rb
index 3bd2b0f19..182ea3d8a 100644
--- a/plugins/kernel_v2/config/vm_trigger.rb
+++ b/plugins/kernel_v2/config/vm_trigger.rb
@@ -214,10 +214,7 @@ module VagrantPlugins
         end
 
         if @type == :command || !@type
-          commands = []
-          Vagrant.plugin("2").manager.commands.each do |key,data|
-            commands.push(key)
-          end
+          commands = Vagrant.plugin("2").manager.commands.keys.map(&:to_s)
 
           if !commands.include?(@command) && @command != :all
             machine.ui.warn(I18n.t("vagrant.config.triggers.bad_command_warning",
diff --git a/test/unit/vagrant/action/runner_test.rb b/test/unit/vagrant/action/runner_test.rb
index 7ffc1d14e..44db5536f 100644
--- a/test/unit/vagrant/action/runner_test.rb
+++ b/test/unit/vagrant/action/runner_test.rb
@@ -96,4 +96,49 @@ describe Vagrant::Action::Runner do
     instance.run(callable)
     expect(result).to eq("bar")
   end
+
+  describe "triggers" do
+    let(:environment) { double("environment", ui: nil) }
+    let(:machine) { double("machine", triggers: machine_triggers, name: "") }
+    let(:env_triggers) { double("env_triggers", find: []) }
+    let(:machine_triggers) { double("machine_triggers", find: []) }
+
+    before do
+      allow(environment).to receive_message_chain(:vagrantfile, :config, :trigger)
+      allow(Vagrant::Plugin::V2::Trigger).to receive(:new).
+        and_return(env_triggers)
+    end
+
+    context "when only environment is provided" do
+      let(:instance) { described_class.new(action_name: "test", env: environment) }
+
+      it "should use environment to build new trigger" do
+        expect(environment).to receive_message_chain(:vagrantfile, :config, :trigger)
+        instance.run(lambda{|_|})
+      end
+
+      it "should pass environment based triggers to callable" do
+        callable = lambda { |env| expect(env[:triggers]).to eq(env_triggers) }
+        instance.run(callable)
+      end
+    end
+
+    context "when only machine is provided" do
+      let(:instance) { described_class.new(action_name: "test", machine: machine) }
+
+      it "should pass machine based triggers to callable" do
+        callable = lambda { |env| expect(env[:triggers]).to eq(machine_triggers) }
+        instance.run(callable)
+      end
+    end
+
+    context "when both environment and machine is provided" do
+      let(:instance) { described_class.new(action_name: "test", machine: machine, env: environment) }
+
+      it "should pass machine based triggers to callable" do
+        callable = lambda { |env| expect(env[:triggers]).to eq(machine_triggers) }
+        instance.run(callable)
+      end
+    end
+  end
 end

From 5d70cc3bf2a3c782d2b64293aa4a7b7b40407cc6 Mon Sep 17 00:00:00 2001
From: Chris Roberts <croberts@hashicorp.com>
Date: Fri, 3 Apr 2020 15:47:00 -0700
Subject: [PATCH 9/9] Retain original trigger behavior

These updates allow the after trigger to behave the same as the
original with regards to the execution location of the trigger
within the execution stack.
---
 lib/vagrant/action.rb                         |  1 +
 lib/vagrant/action/builder.rb                 | 13 ++++++++--
 lib/vagrant/action/builtin/delayed.rb         | 26 +++++++++++++++++++
 test/unit/vagrant/action/builder_test.rb      |  6 +++--
 .../vagrant/action/builtin/delayed_test.rb    | 22 ++++++++++++++++
 5 files changed, 64 insertions(+), 4 deletions(-)
 create mode 100644 lib/vagrant/action/builtin/delayed.rb
 create mode 100644 test/unit/vagrant/action/builtin/delayed_test.rb

diff --git a/lib/vagrant/action.rb b/lib/vagrant/action.rb
index 68a1c62ef..31a910c04 100644
--- a/lib/vagrant/action.rb
+++ b/lib/vagrant/action.rb
@@ -16,6 +16,7 @@ module Vagrant
       autoload :CleanupDisks, "vagrant/action/builtin/cleanup_disks"
       autoload :Confirm, "vagrant/action/builtin/confirm"
       autoload :ConfigValidate, "vagrant/action/builtin/config_validate"
+      autoload :Delayed, "vagrant/action/builtin/delayed"
       autoload :DestroyConfirm, "vagrant/action/builtin/destroy_confirm"
       autoload :Disk, "vagrant/action/builtin/disk"
       autoload :EnvSet,  "vagrant/action/builtin/env_set"
diff --git a/lib/vagrant/action/builder.rb b/lib/vagrant/action/builder.rb
index 13fbbbd87..356e3dace 100644
--- a/lib/vagrant/action/builder.rb
+++ b/lib/vagrant/action/builder.rb
@@ -247,15 +247,24 @@ module Vagrant
           hook_proc.call(hook)
         end
 
-        # Finally load any action triggers defined
+        # Finally load any action triggers defined. The action triggers
+        # are the originally implemented trigger style. They run before
+        # and after specific provider actions (like :up, :halt, etc) and
+        # are different from true action triggers
         if env[:triggers]
           if !env[:triggers].find(env[:raw_action_name], :before, machine_name, :action).empty?
             hook.prepend(Vagrant::Action::Builtin::Trigger,
               env[:raw_action_name], env[:triggers], :before, :action)
           end
           if !env[:triggers].find(env[:raw_action_name], :after, machine_name, :action).empty?
-            hook.append(Vagrant::Action::Builtin::Trigger,
+            # NOTE: These after triggers need to be delayed before running to
+            #       allow the rest of the call stack to complete before being
+            #       run. The delayed action is prepended to the stack (not appended)
+            #       to ensure it is called first, which results in it properly
+            #       waiting for everything to finish before itself completing.
+            builder = self.class.build(Vagrant::Action::Builtin::Trigger,
               env[:raw_action_name], env[:triggers], :after, :action)
+            hook.prepend(Vagrant::Action::Builtin::Delayed, builder)
           end
         end
 
diff --git a/lib/vagrant/action/builtin/delayed.rb b/lib/vagrant/action/builtin/delayed.rb
new file mode 100644
index 000000000..d91d1eebe
--- /dev/null
+++ b/lib/vagrant/action/builtin/delayed.rb
@@ -0,0 +1,26 @@
+module Vagrant
+  module Action
+    module Builtin
+      # This class is used to delay execution until the end of
+      # a configured stack
+      class Delayed
+        # @param [Object] callable The object to call (must respond to #call)
+        def initialize(app, env, callable)
+          if !callable.respond_to?(:call)
+            raise TypeError, "Callable argument is expected to respond to `#call`"
+          end
+          @app         = app
+          @env         = env
+          @callable    = callable
+        end
+
+        def call(env)
+          # Allow the rest of the call stack to execute
+          @app.call(env)
+          # Now call our delayed stack
+          @callable.call(env)
+        end
+      end
+    end
+  end
+end
diff --git a/test/unit/vagrant/action/builder_test.rb b/test/unit/vagrant/action/builder_test.rb
index dfe220074..93bf6ec2d 100644
--- a/test/unit/vagrant/action/builder_test.rb
+++ b/test/unit/vagrant/action/builder_test.rb
@@ -700,12 +700,14 @@ describe Vagrant::Action::Builder do
 
         it "should add a trigger action to the end of the stack" do
           subject.apply_action_name(env)
-          expect(subject.stack.last.first).to eq(Vagrant::Action::Builtin::Trigger)
+          expect(subject.stack.first.first).to eq(Vagrant::Action::Builtin::Delayed)
         end
 
         it "should include arguments to the trigger action" do
           subject.apply_action_name(env)
-          args = subject.stack.last[1]
+          builder = subject.stack.first[1]&.first
+          expect(builder).not_to be_nil
+          args = builder.stack.first[1]
           expect(args).to include(raw_action_name)
           expect(args).to include(timing)
           expect(args).to include(:action)
diff --git a/test/unit/vagrant/action/builtin/delayed_test.rb b/test/unit/vagrant/action/builtin/delayed_test.rb
new file mode 100644
index 000000000..766062c0b
--- /dev/null
+++ b/test/unit/vagrant/action/builtin/delayed_test.rb
@@ -0,0 +1,22 @@
+require File.expand_path("../../../../base", __FILE__)
+
+describe Vagrant::Action::Builtin::Delayed do
+  let(:app) { lambda {|*_|} }
+  let(:env) { {} }
+
+  it "should raise error when callable does not provide #call" do
+    expect { described_class.new(app, env, true) }.
+      to raise_error(TypeError)
+  end
+
+  it "should delay executing action to end of stack" do
+    result = []
+    one = proc{ |*_| result << :one }
+    two = proc{ |*_| result << :two }
+    builder = Vagrant::Action::Builder.build(described_class, two)
+    builder.use(one)
+    builder.call(env)
+    expect(result.first).to eq(:one)
+    expect(result.last).to eq(:two)
+  end
+end