From f760c54cba969e588374f9806426291e85af616d Mon Sep 17 00:00:00 2001 From: Clinton Wolfe Date: Fri, 11 Jan 2019 20:10:48 -0800 Subject: [PATCH 1/6] Move plugin activate() method into Activator class Signed-off-by: Clinton Wolfe --- lib/inspec/plugin/v2/activator.rb | 21 +++++++++++++++++++++ lib/inspec/plugin/v2/loader.rb | 2 +- lib/inspec/plugin/v2/registry.rb | 15 --------------- test/unit/plugin/v2/loader_test.rb | 3 +-- 4 files changed, 23 insertions(+), 18 deletions(-) diff --git a/lib/inspec/plugin/v2/activator.rb b/lib/inspec/plugin/v2/activator.rb index cdc380dc2..0feb11234 100644 --- a/lib/inspec/plugin/v2/activator.rb +++ b/lib/inspec/plugin/v2/activator.rb @@ -17,5 +17,26 @@ module Inspec::Plugin::V2 return self[:'activated?'] if new_value.nil? self[:'activated?'] = new_value end + + # Load a plugin, but if an error is encountered, store it and continue + def activate + return if activated? + # rubocop: disable Lint/RescueException + begin + impl_class = self[:activation_proc].call + self[:'activated?'] = true + self[:implementation_class] = impl_class + rescue Exception => ex + exception = ex + Inspec::Log.error "Could not activate #{self[:plugin_type]} hook named '#{self[:activator_name]}' for plugin #{self[:plugin_name]}" + end + # rubocop: enable Lint/RescueException + end + + # Load a plugin, but if an error is encountered, re-throw it + def activate! + activate + raise exception if exception + end end end diff --git a/lib/inspec/plugin/v2/loader.rb b/lib/inspec/plugin/v2/loader.rb index 334727c7b..4008e4ddf 100644 --- a/lib/inspec/plugin/v2/loader.rb +++ b/lib/inspec/plugin/v2/loader.rb @@ -102,7 +102,7 @@ module Inspec::Plugin::V2 # OK, activate. if activate_me - registry.activate(:cli_command, act.activator_name) + act.activate act.implementation_class.register_with_thor end end diff --git a/lib/inspec/plugin/v2/registry.rb b/lib/inspec/plugin/v2/registry.rb index f9fd30a60..0bf07970d 100644 --- a/lib/inspec/plugin/v2/registry.rb +++ b/lib/inspec/plugin/v2/registry.rb @@ -67,21 +67,6 @@ module Inspec::Plugin::V2 end end - def activate(plugin_type, hook_name) - activator = find_activators(plugin_type: plugin_type, activator_name: hook_name).first - # We want to capture literally any possible exception here, since we are storing them. - # rubocop: disable Lint/RescueException - begin - impl_class = activator.activation_proc.call - activator.activated?(true) - activator.implementation_class = impl_class - rescue Exception => ex - activator.exception = ex - Inspec::Log.error "Could not activate #{activator.plugin_type} hook named '#{activator.activator_name}' for plugin #{plugin_name}" - end - # rubocop: enable Lint/RescueException - end - def register(name, status) if known_plugin? name Inspec::Log.debug "PluginLoader: refusing to re-register plugin '#{name}': an existing plugin with that name was loaded via #{registry[name].installation_type}-loading from #{registry[name].entry_point}" diff --git a/test/unit/plugin/v2/loader_test.rb b/test/unit/plugin/v2/loader_test.rb index 84a30757d..17ae1fece 100644 --- a/test/unit/plugin/v2/loader_test.rb +++ b/test/unit/plugin/v2/loader_test.rb @@ -187,7 +187,6 @@ class PluginLoaderTests < MiniTest::Test # Management methods for activation assert_respond_to status, :activators, 'A plugin status should respond to `activators`' assert_respond_to registry, :find_activators, 'Registry should respond to `find_activators`' - assert_respond_to registry, :activate, 'Registry should respond to `activate`' # Finding an Activator assert_kind_of Array, status.activators, 'status should have an array for activators' @@ -205,7 +204,7 @@ class PluginLoaderTests < MiniTest::Test assert_nil activator.implementation_class, 'Test activator should not know implementation class prior to activation' refute InspecPlugins::MeaningOfLife.const_defined?(:MockPlugin), 'impl_class should not be defined prior to activation' - registry.activate(:mock_plugin_type, :'meaning-of-life-the-universe-and-everything') + activator.activate # Activation postconditions assert activator.activated?, 'Test activator should be activated after activate' From 1010142139ed662221a8d9536e028e864c90bb0b Mon Sep 17 00:00:00 2001 From: Clinton Wolfe Date: Fri, 11 Jan 2019 20:13:40 -0800 Subject: [PATCH 2/6] Convenience method for the common case of finding exactly one plugin activator Signed-off-by: Clinton Wolfe --- lib/inspec/plugin/v2/registry.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/lib/inspec/plugin/v2/registry.rb b/lib/inspec/plugin/v2/registry.rb index 0bf07970d..8caa7a562 100644 --- a/lib/inspec/plugin/v2/registry.rb +++ b/lib/inspec/plugin/v2/registry.rb @@ -67,6 +67,17 @@ module Inspec::Plugin::V2 end end + # Convenience method for when you expect exactly one + def find_activator(filters = {}) + matched_plugins = find_activators(filters) + if matched_plugins.count > 1 + raise Inspec::Plugin::V2::LoadError.new("Plugin hooks search returned multiple results for filter #{filters.inspect} - use more filters, or use find_activators (plural)") + elsif matched_plugins.empty? + raise Inspec::Plugin::V2::LoadError.new("Plugin hooks search returned zero results for filter #{filters.inspect}") + end + matched_plugins.first + end + def register(name, status) if known_plugin? name Inspec::Log.debug "PluginLoader: refusing to re-register plugin '#{name}': an existing plugin with that name was loaded via #{registry[name].installation_type}-loading from #{registry[name].entry_point}" From 0805e9bed34eed3a5a9e8c8bd3c5fb322f754e90 Mon Sep 17 00:00:00 2001 From: Clinton Wolfe Date: Fri, 11 Jan 2019 20:23:30 -0800 Subject: [PATCH 3/6] linting Signed-off-by: Clinton Wolfe --- lib/inspec/plugin/v2/activator.rb | 2 +- lib/inspec/plugin/v2/registry.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/inspec/plugin/v2/activator.rb b/lib/inspec/plugin/v2/activator.rb index 0feb11234..8e6a7b5ca 100644 --- a/lib/inspec/plugin/v2/activator.rb +++ b/lib/inspec/plugin/v2/activator.rb @@ -27,7 +27,7 @@ module Inspec::Plugin::V2 self[:'activated?'] = true self[:implementation_class] = impl_class rescue Exception => ex - exception = ex + self[:exception] = ex Inspec::Log.error "Could not activate #{self[:plugin_type]} hook named '#{self[:activator_name]}' for plugin #{self[:plugin_name]}" end # rubocop: enable Lint/RescueException diff --git a/lib/inspec/plugin/v2/registry.rb b/lib/inspec/plugin/v2/registry.rb index 8caa7a562..a52097eba 100644 --- a/lib/inspec/plugin/v2/registry.rb +++ b/lib/inspec/plugin/v2/registry.rb @@ -71,9 +71,9 @@ module Inspec::Plugin::V2 def find_activator(filters = {}) matched_plugins = find_activators(filters) if matched_plugins.count > 1 - raise Inspec::Plugin::V2::LoadError.new("Plugin hooks search returned multiple results for filter #{filters.inspect} - use more filters, or use find_activators (plural)") + raise Inspec::Plugin::V2::LoadError, "Plugin hooks search returned multiple results for filter #{filters.inspect} - use more filters, or use find_activators (plural)" elsif matched_plugins.empty? - raise Inspec::Plugin::V2::LoadError.new("Plugin hooks search returned zero results for filter #{filters.inspect}") + raise Inspec::Plugin::V2::LoadError, "Plugin hooks search returned zero results for filter #{filters.inspect}" end matched_plugins.first end From 927b2690cf32265b07bb900a8768ce9826e14fa4 Mon Sep 17 00:00:00 2001 From: Clinton Wolfe Date: Sun, 27 Jan 2019 20:48:20 -0500 Subject: [PATCH 4/6] Update activate() calls used by DSL plugin type Signed-off-by: Clinton Wolfe --- lib/inspec/control_eval_context.rb | 2 +- lib/inspec/dsl.rb | 2 +- lib/inspec/plugin/v1/plugin_types/resource.rb | 2 +- lib/inspec/rspec_extensions.rb | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/inspec/control_eval_context.rb b/lib/inspec/control_eval_context.rb index 51a0ecbbc..b635d183c 100644 --- a/lib/inspec/control_eval_context.rb +++ b/lib/inspec/control_eval_context.rb @@ -40,7 +40,7 @@ module Inspec if hook # OK, load the hook if it hasn't been already. We'll then know a module, # which we can then inject into the context - registry.activate(:control_dsl, method_name) unless hook.activated? + hook.activate unless hook.activated? # Inject the module's methods into the context. # implementation_class is the field name, but this is actually a module. self.class.include(hook.implementation_class) diff --git a/lib/inspec/dsl.rb b/lib/inspec/dsl.rb index 6a07c6f9e..8520383f0 100644 --- a/lib/inspec/dsl.rb +++ b/lib/inspec/dsl.rb @@ -38,7 +38,7 @@ module Inspec::DSL if hook # OK, load the hook if it hasn't been already. We'll then know a module, # which we can then inject into the context - registry.activate(:outer_profile_dsl, method_name) unless hook.activated? + hook.activate unless hook.activated? # Inject the module's methods into the context # implementation_class is the field name, but this is actually a module. self.class.include(hook.implementation_class) diff --git a/lib/inspec/plugin/v1/plugin_types/resource.rb b/lib/inspec/plugin/v1/plugin_types/resource.rb index f2520a96e..078d415b8 100644 --- a/lib/inspec/plugin/v1/plugin_types/resource.rb +++ b/lib/inspec/plugin/v1/plugin_types/resource.rb @@ -52,7 +52,7 @@ module Inspec if hook # OK, load the hook if it hasn't been already. We'll then know a module, # which we can then inject into the resource - registry.activate(:resource_dsl, method_name) unless hook.activated? + hook.activate unless hook.activated? # Inject the module's methods into the resource as class methods. # implementation_class is the field name, but this is actually a module. extend(hook.implementation_class) diff --git a/lib/inspec/rspec_extensions.rb b/lib/inspec/rspec_extensions.rb index 2f66ebc56..8d477e784 100644 --- a/lib/inspec/rspec_extensions.rb +++ b/lib/inspec/rspec_extensions.rb @@ -17,7 +17,7 @@ module Inspec if hook # OK, load the hook if it hasn't been already. We'll then know a module, # which we can then inject into the context - registry.activate(:describe_dsl, method_name) unless hook.activated? + hook.activate unless hook.activated? # Inject the module's methods into the example group contexts. # implementation_class is the field name, but this is actually a module. @@ -46,7 +46,7 @@ module Inspec if hook # OK, load the hook if it hasn't been already. We'll then know a module, # which we can then inject into the context - registry.activate(:test_dsl, method_name) unless hook.activated? + hook.activate unless hook.activated? # Inject the module's methods into the example group contexts. # implementation_class is the field name, but this is actually a module. From 91ed6e3256afc49be4742d319ecead2f77bdd08d Mon Sep 17 00:00:00 2001 From: Clinton Wolfe Date: Sun, 27 Jan 2019 23:37:32 -0500 Subject: [PATCH 5/6] Remove quotes from symbol representation of activated? Signed-off-by: Clinton Wolfe --- lib/inspec/plugin/v2/activator.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/inspec/plugin/v2/activator.rb b/lib/inspec/plugin/v2/activator.rb index 8e6a7b5ca..129fda716 100644 --- a/lib/inspec/plugin/v2/activator.rb +++ b/lib/inspec/plugin/v2/activator.rb @@ -3,7 +3,7 @@ module Inspec::Plugin::V2 :plugin_name, :plugin_type, :activator_name, - :'activated?', + :activated?, :exception, :activation_proc, :implementation_class, @@ -14,8 +14,8 @@ module Inspec::Plugin::V2 end def activated?(new_value = nil) - return self[:'activated?'] if new_value.nil? - self[:'activated?'] = new_value + return self[:activated?] if new_value.nil? + self[:activated?] = new_value end # Load a plugin, but if an error is encountered, store it and continue @@ -24,7 +24,7 @@ module Inspec::Plugin::V2 # rubocop: disable Lint/RescueException begin impl_class = self[:activation_proc].call - self[:'activated?'] = true + self[:activated?] = true self[:implementation_class] = impl_class rescue Exception => ex self[:exception] = ex From 591317e659e33cd894ae7ad5b658ffe79982165c Mon Sep 17 00:00:00 2001 From: Clinton Wolfe Date: Wed, 6 Feb 2019 12:22:51 -0500 Subject: [PATCH 6/6] Remove spurious 'unless activated?' on most activation calls Signed-off-by: Clinton Wolfe --- lib/inspec/control_eval_context.rb | 3 ++- lib/inspec/dsl.rb | 2 +- lib/inspec/plugin/v1/plugin_types/resource.rb | 2 +- lib/inspec/rspec_extensions.rb | 4 ++-- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/inspec/control_eval_context.rb b/lib/inspec/control_eval_context.rb index b635d183c..dc2b994fc 100644 --- a/lib/inspec/control_eval_context.rb +++ b/lib/inspec/control_eval_context.rb @@ -40,7 +40,8 @@ module Inspec if hook # OK, load the hook if it hasn't been already. We'll then know a module, # which we can then inject into the context - hook.activate unless hook.activated? + hook.activate + # Inject the module's methods into the context. # implementation_class is the field name, but this is actually a module. self.class.include(hook.implementation_class) diff --git a/lib/inspec/dsl.rb b/lib/inspec/dsl.rb index 8520383f0..4c59054d0 100644 --- a/lib/inspec/dsl.rb +++ b/lib/inspec/dsl.rb @@ -38,7 +38,7 @@ module Inspec::DSL if hook # OK, load the hook if it hasn't been already. We'll then know a module, # which we can then inject into the context - hook.activate unless hook.activated? + hook.activate # Inject the module's methods into the context # implementation_class is the field name, but this is actually a module. self.class.include(hook.implementation_class) diff --git a/lib/inspec/plugin/v1/plugin_types/resource.rb b/lib/inspec/plugin/v1/plugin_types/resource.rb index 078d415b8..24002443e 100644 --- a/lib/inspec/plugin/v1/plugin_types/resource.rb +++ b/lib/inspec/plugin/v1/plugin_types/resource.rb @@ -52,7 +52,7 @@ module Inspec if hook # OK, load the hook if it hasn't been already. We'll then know a module, # which we can then inject into the resource - hook.activate unless hook.activated? + hook.activate # Inject the module's methods into the resource as class methods. # implementation_class is the field name, but this is actually a module. extend(hook.implementation_class) diff --git a/lib/inspec/rspec_extensions.rb b/lib/inspec/rspec_extensions.rb index 8d477e784..7977280f9 100644 --- a/lib/inspec/rspec_extensions.rb +++ b/lib/inspec/rspec_extensions.rb @@ -17,7 +17,7 @@ module Inspec if hook # OK, load the hook if it hasn't been already. We'll then know a module, # which we can then inject into the context - hook.activate unless hook.activated? + hook.activate # Inject the module's methods into the example group contexts. # implementation_class is the field name, but this is actually a module. @@ -46,7 +46,7 @@ module Inspec if hook # OK, load the hook if it hasn't been already. We'll then know a module, # which we can then inject into the context - hook.activate unless hook.activated? + hook.activate # Inject the module's methods into the example group contexts. # implementation_class is the field name, but this is actually a module.