(GH-1350) Allow entire plugin_hooks to be set with a reference

Previously, setting `plugin_hooks` to a `_plugin` reference would cause
a failure because we would merge the default `plugin_hooks` with the
reference before evaluating it. That caused the reference to receive
arguments it didn't expect. We now evaluate the configured
`plugin_hooks` key and then merge it over top of the defaults afterward.

Author: nicklewis

lib/bolt/config.rb

@@ -71,7 +71,7 @@ def initialize(boltdir, config_data, overrides = {})
       @save_rerun = true
       @puppetfile_config = {}
       @plugins = {}
-      @plugin_hooks = { 'puppet_library' => { 'plugin' => 'puppet_agent', 'stop_service' => true } }
+      @plugin_hooks = {}
 
       # add an entry for the default console logger
       @log = { 'console' => {} }
@@ -167,7 +167,7 @@ def update_from_file(data)
       @save_rerun = data['save-rerun'] if data.key?('save-rerun')
 
       @plugins = data['plugins'] if data.key?('plugins')
-      @plugin_hooks.merge!(data['plugin_hooks']) if data.key?('plugin_hooks')
+      @plugin_hooks = data['plugin_hooks'] if data.key?('plugin_hooks')
 
       %w[concurrency format puppetdb color].each do |key|
         send("#{key}=", data[key]) if data.key?(key)

lib/bolt/inventory.rb

@@ -234,7 +234,7 @@ def update_target(target)
       set_facts(target.name, data['facts']) unless @target_facts[target.name]
       data['features']&.each { |feature| set_feature(target, feature) } unless @target_features[target.name]
       unless @target_plugin_hooks[target.name]
-        set_plugin_hooks(target.name, (@plugins&.default_plugin_hooks || {}).merge(data['plugin_hooks'] || {}))
+        set_plugin_hooks(target.name, (@plugins&.plugin_hooks || {}).merge(data['plugin_hooks'] || {}))
       end
 
       # Use Config object to ensure config section is treated consistently with config file

lib/bolt/inventory/target.rb

@@ -78,7 +78,7 @@ def set_feature(feature, value = true)
       def plugin_hooks
         # Merge plugin_hooks from the config file with any defined by the group
         # or assigned dynamically to the target
-        @inventory.plugins.default_plugin_hooks.merge(group_cache['plugin_hooks']).merge(@plugin_hooks)
+        @inventory.plugins.plugin_hooks.merge(group_cache['plugin_hooks']).merge(@plugin_hooks)
       end
 
       def set_config(key_or_key_path, value)

lib/bolt/plugin.rb

@@ -132,16 +132,17 @@ def self.setup(config, pal, pdb_client, analytics)
         plugins.by_name(plugin)
       end
 
-      plugins.default_plugin_hooks = plugins.resolve_references(config.plugin_hooks)
+      plugins.plugin_hooks.merge!(plugins.resolve_references(config.plugin_hooks))
 
       plugins
     end
 
     RUBY_PLUGINS = %w[install_agent task pkcs7 prompt].freeze
     BUILTIN_PLUGINS = %w[task terraform pkcs7 prompt vault aws_inventory puppetdb azure_inventory].freeze
+    DEFAULT_PLUGIN_HOOKS = { 'puppet_library' => { 'plugin' => 'puppet_agent', 'stop_service' => true } }.freeze
 
     attr_reader :pal, :plugin_context
-    attr_accessor :default_plugin_hooks
+    attr_accessor :plugin_hooks
 
     private_class_method :new
 
@@ -154,7 +155,7 @@ def initialize(config, pal, analytics)
       @unknown = Set.new
       @resolution_stack = []
       @unresolved_plugin_configs = config.plugins.dup
-      @default_plugin_hooks = {}
+      @plugin_hooks = DEFAULT_PLUGIN_HOOKS.dup
     end
 
     def modules

spec/bolt/plugin_spec.rb

@@ -70,7 +70,29 @@ def identity(value)
           'param' => identity('foobar')
         }
       }
-      expect(plugins.default_plugin_hooks['puppet_library']).to eq('plugin' => 'my_hook', 'param' => 'foobar')
+      expect(plugins.plugin_hooks['puppet_library']).to eq('plugin' => 'my_hook', 'param' => 'foobar')
+    end
+
+    it 'allows the whole plugin_hooks value to be set with a reference' do
+      hooks = {
+        'another_hook' => {
+          'plugin' => 'my_hook',
+          'param' => identity('foobar')
+        }
+      }
+
+      config_data['plugin_hooks'] = identity(hooks)
+
+      expect(plugins.plugin_hooks).to eq(
+        'another_hook' => {
+          'plugin' => 'my_hook',
+          'param' => 'foobar'
+        },
+        'puppet_library' => {
+          'plugin' => 'puppet_agent',
+          'stop_service' => true
+        }
+      )
     end
   end
 end