Add non-halting exception support to resources (#2235)

* Add non-halting exception support to resources

This adds two `Inspec::Exceptions` that can be used within resources to
either skip or fail a test without halting execution.

Signed-off-by: Jerry Aldrich <jerryaldrichiii@gmail.com>

docs/ruby_usage.md

@@ -114,7 +114,6 @@ methods available, etc. For the above example:
 [3] pry> ls perl_out
 Inspec::Plugins::Resource#methods: inspect
 Inspec::Resources::Cmd#methods: command  exist?  exit_status  result  stderr  stdout  to_s
-Inspec::Plugins::ResourceCommon#methods: resource_skipped  skip_resource
 Inspec::Resource::Registry::Command#methods: inspec
 instance variables: @__backend_runner__  @__resource_name__  @command  @result
 [4] pry> perl_out.stdout.partition('@INC:').last.strip.split("\n")

lib/inspec/exceptions.rb

@@ -5,6 +5,8 @@ module Inspec
   module Exceptions
     class AttributesFileDoesNotExist < ArgumentError; end
     class AttributesFileNotReadable < ArgumentError; end
+    class ResourceFailed < StandardError; end
+    class ResourceSkipped < StandardError; end
     class SecretsBackendNotFound < ArgumentError; end
   end
 end

lib/inspec/plugins/resource.rb

@@ -37,14 +37,26 @@ module Inspec
       Inspec::Resource.registry
     end
 
-    def __register(name, obj)
+    def __register(name, obj) # rubocop:disable Metrics/MethodLength
       cl = Class.new(obj) do
+        attr_reader :resource_exception_message
+
         def initialize(backend, name, *args)
+          @resource_skipped = false
+          @resource_failed = false
+
           # attach the backend to this instance
           @__backend_runner__ = backend
           @__resource_name__ = name
+
           # call the resource initializer
-          super(*args)
+          begin
+            super(*args)
+          rescue Inspec::Exceptions::ResourceSkipped => e
+            skip_resource(e.message)
+          rescue Inspec::Exceptions::ResourceFailed => e
+            fail_resource(e.message)
+          end
         end
 
         def self.desc(description = nil)
@@ -57,12 +69,29 @@ module Inspec
           @example = example
         end
 
-        def resource_skipped
-          @resource_skipped if defined?(@resource_skipped)
+        def skip_resource(message)
+          @resource_skipped = true
+          @resource_exception_message = message
         end
 
-        def skip_resource(message)
-          @resource_skipped = message
+        def resource_skipped?
+          @resource_skipped
+        end
+
+        def resource_skipped
+          warn('[DEPRECATION] Use `resource_exception_message` for the resource skipped message. This method will be removed in InSpec 2.0.')
+          # Returning `nil` here to match previous behavior
+          return nil if @resource_skipped == false
+          @resource_exception_message
+        end
+
+        def fail_resource(message)
+          @resource_failed = true
+          @resource_exception_message = message
+        end
+
+        def resource_failed?
+          @resource_failed
         end
 
         def inspec

lib/inspec/runner.rb

@@ -231,35 +231,18 @@ module Inspec
     def get_check_example(method_name, arg, block)
       opts = block_source_info(block)
 
-      if !arg.empty? &&
-         arg[0].respond_to?(:resource_skipped) &&
-         !arg[0].resource_skipped.nil?
-        return @test_collector.example_group(*arg, opts) do
-          it arg[0].resource_skipped
-        end
-      else
-        # add the resource
-        case method_name
-        when 'describe'
-          return @test_collector.example_group(*arg, opts, &block)
-        when 'expect'
-          return block.example_group
-        when 'describe.one'
-          tests = arg.map do |x|
-            @test_collector.example_group(x[1][0], block_source_info(x[2]), &x[2])
-          end
-          return nil if tests.empty?
-          ok_tests = tests.find_all(&:run)
-          # return all tests if none succeeds; we will just report full failure
-          return tests if ok_tests.empty?
-          # otherwise return all working tests
-          return ok_tests
-        else
-          raise "A rule was registered with #{method_name.inspect}, "\
-               "which isn't understood and cannot be processed."
-        end
+      return nil if arg.empty?
+
+      if arg[0].respond_to?(:resource_skipped?) && arg[0].resource_skipped?
+        return rspec_skipped_block(arg, opts, arg[0].resource_exception_message)
       end
-      nil
+
+      if arg[0].respond_to?(:resource_failed?) && arg[0].resource_failed?
+        return rspec_failed_block(arg, opts, arg[0].resource_exception_message)
+      end
+
+      # If neither skipped nor failed then add the resource
+      add_resource(method_name, arg, opts, block)
     end
 
     def register_rule(rule)
@@ -288,5 +271,46 @@ module Inspec
 
       true
     end
+
+    def rspec_skipped_block(arg, opts, message)
+      @test_collector.example_group(*arg, opts) do
+        # Send custom `it` block to RSpec
+        it message
+      end
+    end
+
+    def rspec_failed_block(arg, opts, message)
+      @test_collector.example_group(*arg, opts) do
+        # Send custom `it` block to RSpec
+        it '' do
+          # Raising here to fail the test and get proper formatting
+          raise Inspec::Exceptions::ResourceFailed, message
+        end
+      end
+    end
+
+    def add_resource(method_name, arg, opts, block)
+      case method_name
+      when 'describe'
+        @test_collector.example_group(*arg, opts, &block)
+      when 'expect'
+        block.example_group
+      when 'describe.one'
+        tests = arg.map do |x|
+          @test_collector.example_group(x[1][0], block_source_info(x[2]), &x[2])
+        end
+        return nil if tests.empty?
+
+        successful_tests = tests.find_all(&:run)
+
+        # Return all tests if none succeeds; we will just report full failure
+        return tests if successful_tests.empty?
+
+        successful_tests
+      else
+        raise "A rule was registered with #{method_name.inspect}," \
+              "which isn't understood and cannot be processed."
+      end
+    end
   end
 end

lib/resources/packages.rb

@@ -63,7 +63,7 @@ module Inspec::Resources
     end
 
     def filtered_packages
-      warn "The packages resource is not yet supported on OS #{inspec.os.name}" if resource_skipped
+      warn "The packages resource is not yet supported on OS #{inspec.os.name}" if resource_skipped?
       @list
     end
   end

test/unit/mock/profiles/profile-with-resource-exceptions/README.md

@@ -0,0 +1,3 @@
+# Profile With Resource Exceptions
+
+This profile is intended to test resource exception handling.

test/unit/mock/profiles/profile-with-resource-exceptions/controls/exception_resource.rb

@@ -0,0 +1,31 @@
+# encoding: utf-8
+
+describe exception_resource_test('should raise ResourceSkipped', :skip_me) do
+  its('value') { should eq 'does not matter' }
+end
+
+describe exception_resource_test('should raise ResourceFailed', :fail_me) do
+  its('value') { should eq 'does not matter' }
+end
+
+describe exception_resource_test('should pass') do
+  its('value') { should eq 'should pass' }
+end
+
+describe exception_resource_test('fail inside matcher') do
+  its('inside_matcher') { should eq 'does not matter' }
+end
+
+describe exception_resource_test('skip inside matcher') do
+  its('inside_matcher') { should eq 'does not matter' }
+end
+
+control 'should-work-within-control' do
+  describe exception_resource_test('should skip', :skip_me) do
+    its('value') { should eq 'does not matter' }
+  end
+
+  describe exception_resource_test('should fail', :fail_me) do
+    its('value') { should eq 'does not matter' }
+  end
+end

test/unit/mock/profiles/profile-with-resource-exceptions/inspec.yml

@@ -0,0 +1,8 @@
+name: profile-with-resource-exceptions
+title: Profile With Exceptions
+maintainer: The Authors
+copyright: The Authors
+copyright_email: you@example.com
+license: Apache-2.0
+summary: Used to test Resource exceptions
+version: 0.1.0

test/unit/mock/profiles/profile-with-resource-exceptions/libraries/example_resource.rb

@@ -0,0 +1,57 @@
+# encoding: utf-8
+
+class ExceptionResourceTest < Inspec.resource(1)
+  name 'exception_resource_test'
+
+  desc '
+    Used to test resource exceptions.
+  '
+
+  example "
+    # Should execute always and pass
+    describe exception_resource_test('foo') do
+      its('value') { should eq 'foo' }
+    end
+
+    # Should execute always and fail
+    describe exception_resource_test('foo') do
+      its('value') { should eq 'bar' }
+    end
+
+    # Should raise Inspec::Exceptions::SkipResource but not halt run
+    # Example: Command not found
+    describe exception_resource_test('foo', :skip_me) do
+      its('value') { should eq 'foo' }
+    end
+
+    # Should raise Inspec::Exceptions::FailResource but not halt run
+    # Example: Command failed
+    describe exception_resource_test('foo', :fail_me) do
+      its('value') { should eq 'foo' }
+    end
+  "
+
+  attr_reader :value
+
+  def initialize(value, qualifier = nil)
+    @value = value
+    @inside_matcher = inside_matcher
+    case qualifier
+    when :skip_me
+      raise Inspec::Exceptions::ResourceSkipped, 'Skipping because reasons'
+    when :fail_me
+      raise Inspec::Exceptions::ResourceFailed, 'Failing because reasons'
+    end
+  end
+
+  def inside_matcher
+    case @value
+    when 'fail inside matcher'
+      raise Inspec::Exceptions::ResourceFailed, 'Failing inside matcher'
+    when 'skip inside matcher'
+      raise Inspec::Exceptions::ResourceSkipped, 'Skipping inside matcher'
+    else
+      'inside_matcher'
+    end
+  end
+end

test/unit/profiles/profile_context_test.rb

@@ -139,13 +139,17 @@ describe Inspec::ProfileContext do
       it 'alters controls when positive' do
         profile.load(if_false + control)
         get_checks.length.must_equal 1
-        get_checks[0][1][0].resource_skipped.must_equal 'Skipped control due to only_if condition.'
+        get_checks[0][1][0].resource_skipped?.must_equal true
+        get_checks[0][1][0].resource_exception_message.must_equal 'Skipped control due to only_if condition.'
+        get_checks[0][1][0].resource_failed?.must_equal false
       end
 
       it 'alters non-controls when positive' do
         profile.load(if_false + describe)
         get_checks.length.must_equal 1
-        get_checks[0][1][0].resource_skipped.must_equal 'Skipped control due to only_if condition.'
+        get_checks[0][1][0].resource_skipped?.must_equal true
+        get_checks[0][1][0].resource_exception_message.must_equal 'Skipped control due to only_if condition.'
+        get_checks[0][1][0].resource_failed?.must_equal false
       end
 
       it 'doesnt alter controls when negative' do
@@ -163,13 +167,17 @@ describe Inspec::ProfileContext do
       it 'doesnt overwrite falsy only_ifs' do
         profile.load(if_false + if_true + control)
         get_checks.length.must_equal 1
-        get_checks[0][1][0].resource_skipped.must_equal 'Skipped control due to only_if condition.'
+        get_checks[0][1][0].resource_skipped?.must_equal true
+        get_checks[0][1][0].resource_exception_message.must_equal 'Skipped control due to only_if condition.'
+        get_checks[0][1][0].resource_failed?.must_equal false
       end
 
       it 'doesnt overwrite falsy only_ifs' do
         profile.load(if_true + if_false + control)
         get_checks.length.must_equal 1
-        get_checks[0][1][0].resource_skipped.must_equal 'Skipped control due to only_if condition.'
+        get_checks[0][1][0].resource_skipped?.must_equal true
+        get_checks[0][1][0].resource_exception_message.must_equal 'Skipped control due to only_if condition.'
+        get_checks[0][1][0].resource_failed?.must_equal false
       end
 
       it 'doesnt extend into other control files' do
@@ -324,7 +332,9 @@ describe Inspec::ProfileContext do
       it 'skips with only_if == false' do
         profile.load(format(context_format, 'only_if { false }'))
         get_checks.length.must_equal 1
-        get_checks[0][1][0].resource_skipped.must_equal 'Skipped control due to only_if condition.'
+        get_checks[0][1][0].resource_skipped?.must_equal true
+        get_checks[0][1][0].resource_exception_message.must_equal 'Skipped control due to only_if condition.'
+        get_checks[0][1][0].resource_failed?.must_equal false
       end
 
       it 'does nothing with only_if == false' do
@@ -335,13 +345,17 @@ describe Inspec::ProfileContext do
       it 'doesnt overwrite falsy only_ifs' do
         profile.load(format(context_format, "only_if { false }\nonly_if { true }"))
         get_checks.length.must_equal 1
-        get_checks[0][1][0].resource_skipped.must_equal 'Skipped control due to only_if condition.'
+        get_checks[0][1][0].resource_skipped?.must_equal true
+        get_checks[0][1][0].resource_exception_message.must_equal 'Skipped control due to only_if condition.'
+        get_checks[0][1][0].resource_failed?.must_equal false
       end
 
       it 'doesnt overwrite falsy only_ifs' do
         profile.load(format(context_format, "only_if { true }\nonly_if { false }"))
         get_checks.length.must_equal 1
-        get_checks[0][1][0].resource_skipped.must_equal 'Skipped control due to only_if condition.'
+        get_checks[0][1][0].resource_skipped?.must_equal true
+        get_checks[0][1][0].resource_exception_message.must_equal 'Skipped control due to only_if condition.'
+        get_checks[0][1][0].resource_failed?.must_equal false
       end
     end
   end

test/unit/profiles/profile_resource_exceptions_test.rb

@@ -0,0 +1,78 @@
+# encoding: utf-8
+
+# author: Jerry Aldrich
+
+require 'helper'
+require 'inspec/profile_context'
+
+describe 'resource exception' do
+  let(:profile) do
+    profile = MockLoader.load_profile('profile-with-resource-exceptions')
+    profile.load_libraries
+    profile.collect_tests
+    profile
+  end
+
+  let(:checks) do
+    checks = []
+    profile.runner_context.rules.values.each do |rule|
+      checks.push(Inspec::Rule.prepare_checks(rule))
+    end
+    checks
+  end
+
+  describe 'within initialize' do
+    it 'skips resource when `Inspec::Exceptions::ResourceSkipped` is raised' do
+      checks[0][0][1][0].resource_skipped?.must_equal true
+      checks[0][0][1][0].resource_exception_message.must_equal 'Skipping because reasons'
+      checks[0][0][1][0].resource_failed?.must_equal false
+    end
+
+    it 'fails resource when `Inspec::Exceptions::ResourceFailed` is raised' do
+      checks[1][0][1][0].resource_failed?.must_equal true
+      checks[1][0][1][0].resource_exception_message.must_equal 'Failing because reasons'
+      checks[1][0][1][0].resource_skipped?.must_equal false
+    end
+
+    it 'does not affect other tests' do
+      checks[2][0][1][0].resource_skipped?.must_equal false
+      checks[2][0][1][0].resource_failed?.must_equal false
+      checks[2][0][1][0].resource_exception_message.must_be_nil
+    end
+  end
+
+  describe 'within a matcher' do
+    it 'skips resource when `Inspec::Exceptions::ResourceSkipped` is raised' do
+      checks[4][0][1][0].resource_skipped?.must_equal true
+      checks[4][0][1][0].resource_exception_message.must_equal 'Skipping inside matcher'
+      checks[4][0][1][0].resource_failed?.must_equal false
+    end
+
+    it 'fails resource when `Inspec::Exceptions::ResourceFailed` is raised' do
+      checks[3][0][1][0].resource_failed?.must_equal true
+      checks[3][0][1][0].resource_exception_message.must_equal 'Failing inside matcher'
+      checks[3][0][1][0].resource_skipped?.must_equal false
+    end
+  end
+
+  describe 'within a control' do
+    it 'skips resource when `Inspec::Exceptions::ResourceSkipped` is raised' do
+      checks[5][0][1][0].resource_skipped?.must_equal true
+      checks[5][0][1][0].resource_exception_message.must_equal 'Skipping because reasons'
+      checks[5][0][1][0].resource_failed?.must_equal false
+    end
+
+    it 'fails resource when `Inspec::Exceptions::ResourceFailed` is raised' do
+      checks[5][1][1][0].resource_failed?.must_equal true
+      checks[5][1][1][0].resource_exception_message.must_equal 'Failing because reasons'
+      checks[5][1][1][0].resource_skipped?.must_equal false
+    end
+  end
+
+  describe 'when using deprecated `resource_skip` method' do
+    it 'warns the user' do
+      _, err = capture_io { checks[0][0][1][0].resource_skipped }
+      err.must_match(/DEPRECATION/)
+    end
+  end
+end

test/unit/resources/json_test.rb

@@ -37,7 +37,7 @@ describe 'Inspec::Resources::JSON' do
     let (:resource) { load_resource('json', 'nonexistent.json') }
 
     it 'produces an error' do
-      _(resource.resource_skipped).must_equal 'Can\'t find file "nonexistent.json"'
+      _(resource.resource_exception_message).must_equal 'Can\'t find file "nonexistent.json"'
     end
   end
 end

test/unit/resources/nginx_conf_test.rb

@@ -97,7 +97,7 @@ describe 'Inspec::Resources::NginxConf' do
   it 'skips the resource if it cannot parse the config' do
     resource = MockLoader.new(:ubuntu1404).load_resource('nginx_conf', '/etc/nginx/failed.conf')
     _(resource.params).must_equal({})
-    _(resource.instance_variable_get(:@resource_skipped)).must_equal "Cannot parse NginX config in /etc/nginx/failed.conf."
+    _(resource.resource_exception_message).must_equal "Cannot parse NginX config in /etc/nginx/failed.conf."
   end
 
   describe '#http' do

test/unit/resources/package_test.rb

@@ -78,7 +78,7 @@ describe 'Inspec::Resources::Package' do
         'curl',
         rpm_dbpath: '/var/lib/rpmdb_does_not_exist',
       )
-      _(resource.resource_skipped).wont_equal nil
+      _(resource.resource_skipped?).must_equal true
     end
   end
 

test/unit/resources/packages_test.rb

@@ -65,7 +65,7 @@ describe 'Inspec::Resources::Packages' do
 
   it 'skips on non debian platforms' do
     resource = MockLoader.new(:hpux).load_resource('packages', 'bash')
-    _(resource.resource_skipped).must_equal 'The packages resource is not yet supported on OS hpux'
+    _(resource.resource_exception_message).must_equal 'The packages resource is not yet supported on OS hpux'
   end
 
   it 'fails if the packages name is not a string or regexp' do