From a21bdc4a04cdb93e659facb5e95a5b8698ac5905 Mon Sep 17 00:00:00 2001 From: Adam Leff Date: Fri, 5 May 2017 09:04:02 -0400 Subject: [PATCH] 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 --- lib/inspec.rb | 1 + lib/inspec/base_cli.rb | 2 +- lib/inspec/exceptions.rb | 8 ++++ lib/inspec/runner.rb | 23 +++++++---- lib/inspec/secrets/yaml.rb | 5 +++ test/helper.rb | 1 + test/unit/runner_test.rb | 84 ++++++++++++++++++++++++++++++++++++++ 7 files changed, 114 insertions(+), 10 deletions(-) create mode 100644 lib/inspec/exceptions.rb create mode 100644 test/unit/runner_test.rb diff --git a/lib/inspec.rb b/lib/inspec.rb index 3a5982b67..5f8cfa527 100644 --- a/lib/inspec.rb +++ b/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' diff --git a/lib/inspec/base_cli.rb b/lib/inspec/base_cli.rb index f4589d2ad..ea795a902 100644 --- a/lib/inspec/base_cli.rb +++ b/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 diff --git a/lib/inspec/exceptions.rb b/lib/inspec/exceptions.rb new file mode 100644 index 000000000..9fb42665f --- /dev/null +++ b/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 diff --git a/lib/inspec/runner.rb b/lib/inspec/runner.rb index 733125ac3..d70e38cdf 100644 --- a/lib/inspec/runner.rb +++ b/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 diff --git a/lib/inspec/secrets/yaml.rb b/lib/inspec/secrets/yaml.rb index 17053abd7..064ac0d6c 100644 --- a/lib/inspec/secrets/yaml.rb +++ b/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 diff --git a/test/helper.rb b/test/helper.rb index 7aba8aa15..a8958114a 100644 --- a/test/helper.rb +++ b/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' diff --git a/test/unit/runner_test.rb b/test/unit/runner_test.rb new file mode 100644 index 000000000..3ec5f656c --- /dev/null +++ b/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