json resource (et. al.): allow inspec check to succeed when using command (#2317)

* json resource (et. al.): allow inspec check to succeed when using command

When using the `json` resource (or any of the resources that subclass
JsonConfig), `inspec check` would fail if the content was supplied with
the `command` option. This is because the `command` resource is mocked
and an empty string would be returned for `stdout`. That content would
be blindly passed to the `parse` method would which raise an exception
and cause `inspec check` to fail.

This change refactors JsonConfig to be a bit cleaner and use some helper
methods. Additionally, we use the new Exceptions to properly raise errors
which are naturally caught by Inspec::Profile, etc.

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

* Make `resource_base_name` method private

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

lib/resources/csv.rb

@@ -34,6 +34,8 @@ module Inspec::Resources
 
       # convert to hash
       csv.to_a.map(&:to_hash)
+    rescue => e
+      raise Inspec::Exceptions::ResourceFailed, "Unable to parse CSV: #{e.message}"
     end
 
     # override the value method from JsonConfig
@@ -45,8 +47,12 @@ module Inspec::Resources
       @params.map { |x| x[key.first.to_s] }.compact
     end
 
-    def to_s
-      "Csv #{@path}"
+    private
+
+    # used by JsonConfig to build up a full to_s method
+    # based on whether a file path, content, or command was supplied.
+    def resource_base_name
+      'CSV'
     end
   end
 end

lib/resources/ini.rb

@@ -18,8 +18,12 @@ module Inspec::Resources
       SimpleConfig.new(content).params
     end
 
-    def to_s
-      "INI #{@path}"
+    private
+
+    # used by JsonConfig to build up a full to_s method
+    # based on whether a file path, content, or command was supplied.
+    def resource_base_name
+      'INI'
     end
   end
 end

lib/resources/json.rb

@@ -26,45 +26,11 @@ module Inspec::Resources
     include ObjectTraverser
 
     # make params readable
-    attr_reader :params
+    attr_reader :params, :raw_content
 
     def initialize(opts)
-      @opts = opts
-      if opts.is_a?(Hash)
-        if opts.key?(:content)
-          @file_content = opts[:content]
-        elsif opts.key?(:command)
-          @command = inspec.command(opts[:command])
-          @file_content = @command.stdout
-        end
-      else
-        @path = opts
-        @file = inspec.file(@opts)
-        @file_content = @file.content
-
-        # check if file is available
-        if !@file.file?
-          skip_resource "Can't find file \"#{@path}\""
-          return @params = {}
-        end
-
-        # check if file is readable
-        if @file_content.nil? && !@file.empty?
-          skip_resource "Can't read file \"#{@path}\""
-          return @params = {}
-        end
-      end
-
-      @params = parse(@file_content)
-    end
-
-    def parse(content)
-      require 'json'
-      JSON.parse(content)
-    end
-
-    def value(key)
-      extract_value(key, @params)
+      @raw_content = load_raw_content(opts)
+      @params = parse(@raw_content)
     end
 
     # Shorthand to retrieve a parameter name via `#its`.
@@ -79,12 +45,65 @@ module Inspec::Resources
       value(keys)
     end
 
+    def value(key)
+      # uses ObjectTraverser.extract_value to walk the hash looking for the key,
+      # which may be an Array of keys for a nested Hash.
+      extract_value(key, params)
+    end
+
     def to_s
-      if @opts.is_a?(Hash) && @opts.key?(:content)
-        'Json content'
-      else
-        "Json #{@path}"
+      "#{resource_base_name} #{@resource_name_supplement || 'content'}"
+    end
+
+    private
+
+    def parse(content)
+      require 'json'
+      JSON.parse(content)
+    rescue => e
+      raise Inspec::Exceptions::ResourceFailed, "Unable to parse JSON: #{e.message}"
+    end
+
+    def load_raw_content(opts)
+      # if the opts isn't a hash, we assume it's a path to a file
+      unless opts.is_a?(Hash)
+        @resource_name_supplement = opts
+        return load_raw_from_file(opts)
       end
+
+      if opts.key?(:command)
+        @resource_name_supplement = "from command: #{opts[:command]}"
+        load_raw_from_command(opts[:command])
+      elsif opts.key?(:content)
+        opts[:content]
+      else
+        raise Inspec::Exceptions::ResourceFailed, 'No JSON content; must specify a file, command, or raw JSON content'
+      end
+    end
+
+    def load_raw_from_file(path)
+      file = inspec.file(path)
+
+      # these are currently ResourceSkipped to maintain consistency with the resource
+      # pre-refactor (which used skip_resource). These should likely be changed to
+      # ResourceFailed during a major version bump.
+      raise Inspec::Exceptions::ResourceSkipped, "No such file: #{path}" unless file.file?
+      raise Inspec::Exceptions::ResourceSkipped, "File #{path} is empty or is not readable by current user" if file.content.nil? || file.content.empty?
+
+      file.content
+    end
+
+    def load_raw_from_command(command)
+      command_output = inspec.command(command).stdout
+      raise Inspec::Exceptions::ResourceSkipped, "No output from command: #{command}" if command_output.nil? || command_output.empty?
+
+      command_output
+    end
+
+    # for resources the subclass JsonConfig, this allows specification of the resource
+    # base name in each subclass so we can build a good to_s method
+    def resource_base_name
+      'JSON'
     end
   end
 end

lib/resources/toml.rb

@@ -17,10 +17,16 @@ module Inspec::Resources
 
     def parse(content)
       Tomlrb.parse(content)
+    rescue => e
+      raise Inspec::Exceptions::ResourceFailed, "Unable to parse TOML: #{e.message}"
     end
 
-    def to_s
-      "TOML #{@path}"
+    private
+
+    # used by JsonConfig to build up a full to_s method
+    # based on whether a file path, content, or command was supplied.
+    def resource_base_name
+      'TOML'
     end
   end
 end

lib/resources/xml.rb

@@ -14,14 +14,20 @@ module Inspec::Resources
     def parse(content)
       require 'rexml/document'
       REXML::Document.new(content)
+    rescue => e
+      raise Inspec::Exceptions::ResourceFailed, "Unable to parse XML: #{e.message}"
     end
 
     def value(key)
       REXML::XPath.each(@params, key.first.to_s).map(&:text)
     end
 
-    def to_s
-      "XML #{@path}"
+    private
+
+    # used by JsonConfig to build up a full to_s method
+    # based on whether a file path, content, or command was supplied.
+    def resource_base_name
+      'XML'
     end
   end
 end

lib/resources/yaml.rb

@@ -30,10 +30,16 @@ module Inspec::Resources
     # override file load and parse hash from yaml
     def parse(content)
       YAML.load(content)
+    rescue => e
+      raise Inspec::Exceptions::ResourceFailed, "Unable to parse YAML: #{e.message}"
     end
 
-    def to_s
-      "YAML #{@path}"
+    private
+
+    # used by JsonConfig to build up a full to_s method
+    # based on whether a file path, content, or command was supplied.
+    def resource_base_name
+      'YAML'
     end
   end
 end

test/unit/resources/json_test.rb

@@ -37,7 +37,69 @@ describe 'Inspec::Resources::JSON' do
     let (:resource) { load_resource('json', 'nonexistent.json') }
 
     it 'produces an error' do
-      _(resource.resource_exception_message).must_equal 'Can\'t find file "nonexistent.json"'
+      _(resource.resource_exception_message).must_equal 'No such file: nonexistent.json'
+    end
+  end
+
+  describe '#load_raw_from_file' do
+    let(:path)     { '/path/to/file.txt' }
+    let(:resource) { Inspec::Resources::JsonConfig.allocate }
+    let(:inspec)   { mock }
+    let(:file)     { mock }
+
+    before do
+      resource.stubs(:inspec).returns(inspec)
+      inspec.expects(:file).with(path).returns(file)
+    end
+
+    it 'raises an exception when the file does not exist' do
+      file.expects(:file?).returns(false)
+      proc { resource.send(:load_raw_from_file, path) }.must_raise Inspec::Exceptions::ResourceSkipped
+    end
+
+    it 'raises an exception if the file content is nil' do
+      file.expects(:file?).returns(true)
+      file.expects(:content).returns(nil)
+      proc { resource.send(:load_raw_from_file, path) }.must_raise Inspec::Exceptions::ResourceSkipped
+    end
+
+    it 'raises an exception if the file content is empty' do
+      file.expects(:file?).returns(true)
+      file.expects(:content).at_least_once.returns('')
+      proc { resource.send(:load_raw_from_file, path) }.must_raise Inspec::Exceptions::ResourceSkipped
+    end
+
+    it 'returns the file content' do
+      file.expects(:file?).returns(true)
+      file.expects(:content).at_least_once.returns('json goes here')
+      resource.send(:load_raw_from_file, path).must_equal 'json goes here'
+    end
+  end
+
+  describe '#load_raw_from_file' do
+    let(:cmd_str)  { 'curl localhost' }
+    let(:resource) { Inspec::Resources::JsonConfig.allocate }
+    let(:inspec)   { mock }
+    let(:command)  { mock }
+
+    before do
+      resource.stubs(:inspec).returns(inspec)
+      inspec.expects(:command).with(cmd_str).returns(command)
+    end
+
+    it 'raises an exception if command stdout is nil' do
+      command.expects(:stdout).returns(nil)
+      proc { resource.send(:load_raw_from_command, cmd_str) }.must_raise Inspec::Exceptions::ResourceSkipped
+    end
+
+    it 'raises an exception if command stdout is empty' do
+      command.expects(:stdout).returns('')
+      proc { resource.send(:load_raw_from_command, cmd_str) }.must_raise Inspec::Exceptions::ResourceSkipped
+    end
+
+    it 'returns the command output' do
+      command.expects(:stdout).returns('json goes here')
+      resource.send(:load_raw_from_command, cmd_str).must_equal 'json goes here'
     end
   end
 end