Handle parse errors for attrs/secrets

Inspired by #1640, this change cleans up the logic used when
reading in secrets files, provides clearer warnings when the
secrets files can't be parsed, and adds tests for those methods.

Signed-off-by: Adam Leff <adam@leff.co>

lib/inspec.rb

@@ -8,6 +8,7 @@ libdir = File.dirname(__FILE__)
 $LOAD_PATH.unshift(libdir) unless $LOAD_PATH.include?(libdir)
 
 require 'inspec/version'
+require 'inspec/exceptions'
 require 'inspec/profile'
 require 'inspec/rspec_json_formatter'
 require 'inspec/rule'

lib/inspec/base_cli.rb

@@ -81,7 +81,7 @@ module Inspec
       runner = Inspec::Runner.new(o)
       targets.each { |target| runner.add_target(target) }
       exit runner.run
-    rescue RuntimeError, Train::UserError => e
+    rescue ArgumentError, RuntimeError, Train::UserError => e
       $stderr.puts e.message
       exit 1
     end

lib/inspec/exceptions.rb

@@ -0,0 +1,8 @@
+# encoding: utf-8
+# copyright: 2017, Chef Software Inc.
+
+module Inspec
+  module Exceptions
+    class SecretsBackendNotFound < ArgumentError; end
+  end
+end

lib/inspec/runner.rb

@@ -119,18 +119,23 @@ module Inspec
 
     # determine all attributes before the execution, fetch data from secrets backend
     def load_attributes(options)
-      attributes = {}
-      # read endpoints for secrets eg. yml file
+      options[:attributes] ||= {}
+
       secrets_targets = options[:attrs]
-      unless secrets_targets.nil?
-        secrets_targets.each do |target|
-          secrets = Inspec::SecretsBackend.resolve(target)
-          # merge hash values
-          attributes = attributes.merge(secrets.attributes) unless secrets.nil? || secrets.attributes.nil?
+      return options[:attributes] if secrets_targets.nil?
+
+      secrets_targets.each do |target|
+        secrets = Inspec::SecretsBackend.resolve(target)
+        if secrets.nil?
+          raise Inspec::Exceptions::SecretsBackendNotFound,
+                "Unable to find a parser for attributes file #{target}. " \
+                'Check to make sure the file exists and has the appropriate extension.'
         end
+
+        next if secrets.attributes.nil?
+        options[:attributes].merge!(secrets.attributes)
       end
-      options[:attributes] = options[:attributes] || {}
-      options[:attributes] = options[:attributes].merge(attributes)
+
       options[:attributes]
     end
 

lib/inspec/secrets/yaml.rb

@@ -18,6 +18,11 @@ module Secrets
     # array of yaml file paths
     def initialize(target)
       @attributes = ::YAML.load_file(target)
+
+      if @attributes == false || !@attributes.is_a?(Hash)
+        Inspec::Log.warn("#{self.class} unable to parse #{target}: invalid YAML or contents is not a Hash")
+        @attributes = nil
+      end
     rescue => e
       raise "Error reading Inspec attributes: #{e}"
     end

test/helper.rb

@@ -21,6 +21,7 @@ require 'zip'
 
 require 'inspec/base_cli'
 require 'inspec/version'
+require 'inspec/exceptions'
 require 'inspec/fetcher'
 require 'inspec/source_reader'
 require 'inspec/resource'

test/unit/runner_test.rb

@@ -0,0 +1,84 @@
+# encoding: utf-8
+# copyright: 2017, Chef Software Inc.
+
+require 'helper'
+
+describe Inspec::Runner do
+  describe '#load_attributes' do
+    let(:runner) { Inspec::Runner.new }
+
+    describe 'when no attrs are specified' do
+      it 'returns an empty hash' do
+        options = {}
+        runner.load_attributes(options).must_equal({})
+      end
+    end
+
+    describe 'when an attr is provided and does not resolve' do
+      it 'raises an exception' do
+        options = { attrs: ['nope.jpg'] }
+        Inspec::SecretsBackend.expects(:resolve).with('nope.jpg').returns(nil)
+        proc { runner.load_attributes(options) }.must_raise Inspec::Exceptions::SecretsBackendNotFound
+      end
+    end
+
+    describe 'when an attr is provided and has no attributes' do
+      it 'returns an empty hash' do
+        secrets = mock
+        secrets.stubs(:attributes).returns(nil)
+        options = { attrs: ['empty.yaml'] }
+        Inspec::SecretsBackend.expects(:resolve).with('empty.yaml').returns(secrets)
+        runner.load_attributes(options).must_equal({})
+      end
+    end
+
+    describe 'when an attr is provided and has attributes' do
+      it 'returns a hash containing the attributes' do
+        options = { attrs: ['file1.yaml'] }
+        attributes = { foo: 'bar' }
+        secrets = mock
+        secrets.stubs(:attributes).returns(attributes)
+        Inspec::SecretsBackend.expects(:resolve).with('file1.yaml').returns(secrets)
+        runner.load_attributes(options).must_equal(attributes)
+      end
+    end
+
+    describe 'when multiple attrs are provided and one fails' do
+      it 'raises an exception' do
+        options = { attrs: ['file1.yaml', 'file2.yaml'] }
+        secrets = mock
+        secrets.stubs(:attributes).returns(nil)
+        Inspec::SecretsBackend.expects(:resolve).with('file1.yaml').returns(secrets)
+        Inspec::SecretsBackend.expects(:resolve).with('file2.yaml').returns(nil)
+        proc { runner.load_attributes(options) }.must_raise Inspec::Exceptions::SecretsBackendNotFound
+      end
+    end
+
+    describe 'when multiple attrs are provided and one has no attributes' do
+      it 'returns a hash containing the attributes from the valid files' do
+        options = { attrs: ['file1.yaml', 'file2.yaml'] }
+        attributes = { foo: 'bar' }
+        secrets1 = mock
+        secrets1.stubs(:attributes).returns(nil)
+        secrets2 = mock
+        secrets2.stubs(:attributes).returns(attributes)
+        Inspec::SecretsBackend.expects(:resolve).with('file1.yaml').returns(secrets1)
+        Inspec::SecretsBackend.expects(:resolve).with('file2.yaml').returns(secrets2)
+        runner.load_attributes(options).must_equal(attributes)
+      end
+    end
+
+    describe 'when multiple attrs are provided and all have attributes' do
+      it 'returns a hash containing all the attributes' do
+        options = { attrs: ['file1.yaml', 'file2.yaml'] }
+        secrets1 = mock
+        secrets1.stubs(:attributes).returns({ key1: 'value1' })
+        secrets2 = mock
+        secrets2.stubs(:attributes).returns({ key2: 'value2' })
+        Inspec::SecretsBackend.expects(:resolve).with('file1.yaml').returns(secrets1)
+        Inspec::SecretsBackend.expects(:resolve).with('file2.yaml').returns(secrets2)
+        runner.load_attributes(options).must_equal({ key1: 'value1', key2: 'value2' })
+      end
+    end
+  end
+end