From 8e72ae5bd65d85c2f37805806a66c9bbf425697c Mon Sep 17 00:00:00 2001
From: Brian Cain <bcain@hashicorp.com>
Date: Tue, 13 Mar 2018 14:35:24 -0700
Subject: [PATCH] Add basic validation for trigger config objects

---
 plugins/kernel_v2/config/trigger.rb    | 18 +++----
 plugins/kernel_v2/config/vm_trigger.rb | 68 ++++++++++++++++++++++++--
 templates/locales/en.yml               | 21 ++++++++
 3 files changed, 93 insertions(+), 14 deletions(-)

diff --git a/plugins/kernel_v2/config/trigger.rb b/plugins/kernel_v2/config/trigger.rb
index 3a6049917..b486aaf55 100644
--- a/plugins/kernel_v2/config/trigger.rb
+++ b/plugins/kernel_v2/config/trigger.rb
@@ -84,27 +84,25 @@ module VagrantPlugins
       def finalize!
         # read through configured settings blocks and set their values
         # and then set up action hooks here?
-        if !@_before_triggers.empty?
-          binding.pry
-        end
+        #if !@_before_triggers.empty?
+        #  binding.pry
+        #end
       end
 
       # Validate Trigger settings
       def validate(machine)
-        if !@_before_triggers.empty?
-          binding.pry
-        end
-
         errors = _detected_errors
         @_before_triggers.each do |bt|
-          errors << bt.validate(machine)
+          error = bt.validate(machine)
+          errors.concat error if !error.empty?
         end
 
         @_after_triggers.each do |at|
-          errors << at.validate(machine)
+          error = at.validate(machine)
+          errors.concat error if !error.empty?
         end
 
-        {"triggers" => errors}
+        {"trigger" => errors}
       end
 
       # The String representation of this Trigger.
diff --git a/plugins/kernel_v2/config/vm_trigger.rb b/plugins/kernel_v2/config/vm_trigger.rb
index 7afedb080..03c070fc8 100644
--- a/plugins/kernel_v2/config/vm_trigger.rb
+++ b/plugins/kernel_v2/config/vm_trigger.rb
@@ -90,16 +90,76 @@ module VagrantPlugins
         @warn = nil if @warn == UNSET_VALUE
         @on_error = DEFAULT_ON_ERROR if @on_error == UNSET_VALUE
         @ignore = nil if @ignore == UNSET_VALUE
-        @only_on = nil if @only_on == UNSET_VALUE
         @run = nil if @run == UNSET_VALUE
         @run_remote = nil if @run_remote == UNSET_VALUE
+        @only_on = nil if @only_on == UNSET_VALUE
+
+        # these values are expected to always be an Array internally,
+        # but can be set as a single String or Symbol
+        if @only_on.is_a?(String)
+          @logger.debug("Updating @only_on variable to be an Array")
+          @only_on = Array(@only_on)
+        end
+
+        if @ignore.is_a?(String)
+          @logger.debug("Updating @ignore variable to be an Array and Symbol")
+          @ignore = Array(@ignore.to_sym)
+        elsif @ignore.is_a?(Symbol)
+          @logger.debug("Updating @ignore variable to be an Array")
+          @ignore = Array(@ignore)
+        end
       end
 
       def validate(machine)
-        binding.pry
         errors = _detected_errors
-        # Validate that each config option has the right values and is the right type
-        {"triggers" => errors}
+
+        if !@run.nil?
+          if !@run.is_a?(Hash)
+            # Run must be a hash
+            errors << I18n.t("vagrant.config.triggers.run.bad_type", cmd: @command)
+          end
+
+          if !@run.key?(:inline) && !@run.key?(:file)
+            errors << I18n.t("vagrant.config.triggers.run.missing_context", cmd: @command)
+          end
+        end
+
+        if !@run_remote.nil?
+          if !@run_remote.is_a?(Hash)
+            errors << I18n.t("vagrant.config.triggers.run_remote.bad_type", cmd: @command)
+          end
+        end
+
+        if !@name.nil? && !@name.is_a?(String)
+          errors << I18n.t("vagrant.config.triggers.name_bad_type", cmd: @command)
+        end
+
+        if !@info.nil? && !@info.is_a?(String)
+          errors << I18n.t("vagrant.config.triggers.info_bad_type", cmd: @command)
+        end
+
+        if !@warn.nil? && !@warn.is_a?(String)
+          errors << I18n.t("vagrant.config.triggers.warn_bad_type", cmd: @command)
+        end
+
+        if @on_error != :halt
+          if @on_error != :continue
+            # must be :halt or :continue
+            errors << I18n.t("vagrant.config.triggers.on_error_bad_type", cmd: @command)
+          end
+        end
+
+        # @ignore validations?
+        # @only_on validations?
+
+        errors
+      end
+
+      # The String representation of this Trigger.
+      #
+      # @return [String]
+      def to_s
+        "Trigger Config"
       end
     end
   end
diff --git a/templates/locales/en.yml b/templates/locales/en.yml
index c0897632b..e663515f0 100644
--- a/templates/locales/en.yml
+++ b/templates/locales/en.yml
@@ -1677,6 +1677,27 @@ en:
         paranoid_deprecated: |-
           The key `paranoid` is deprecated. Please use `verify_host_key`. Supported
           values are exactly the same, only the name of the option has changed.
+      triggers:
+        name_bad_type: |-
+          Invalid type set for `name` on trigger for command '%{cmd}'. `name` should be a String.
+        on_error_bad_type: |-
+          Invalid type set for `on_error` on trigger for command '%{cmd}'. `on_error` can
+          only be `:halt` (default) or `:continue`.
+        run:
+          bad_type: |-
+            Invalid type set for `run` on trigger for command '%{cmd}'. `run` must be a
+            Hash with either key `:inline` or `:file`
+          missing_context: |-
+            Missing key for trigger config setting `run` for command '%{cmd}'. `run` is expected
+            to be given either an `:inline` or `:file` setting.
+        run_remote:
+          bad_type: |-
+            Invalid type set for `run_remote` on trigger for command '%{cmd}'. `run` must be a
+            Hash with either key `:inline` or `:file`
+          missing_context: |-
+            Missing key for trigger config setting `run` for command '%{cmd}'. `run_remote` is expected
+            to be given either an `:inline` or `:file` setting.
+
       vm:
         bad_version: |-
           Invalid box version constraints: %{version}