Merge pull request #413 from chef/sr/fix-408

Use target helpers in Inspec::Profile#from_file

lib/inspec/metadata.rb

@@ -146,6 +146,8 @@ module Inspec
     end
 
     def self.from_ref(ref, contents, profile_id, logger = nil)
+      # NOTE there doesn't have to exist an actual file, it may come from an
+      # archive (i.e., contents)
       case File.basename(ref)
       when 'inspec.yml'
         from_yaml(ref, contents, profile_id, logger)

lib/inspec/profile.rb

@@ -19,29 +19,33 @@ module Inspec
     attr_reader :path
     attr_reader :metadata
 
+    # rubocop:disable Metrics/AbcSize
     def initialize(options = nil)
       @options = options || {}
-
-      @params = {}
       @logger = options[:logger] || Logger.new(nil)
-
       @path = @options[:path]
-      fail 'Cannot read an empty path.' if @path.nil? || @path.empty?
-      fail "Cannot find directory #{@path}" unless File.directory?(@path)
+      @profile_id = options[:id]
 
-      @metadata = read_metadata
-      @params = @metadata.params
-      # use the id from parameter, name or fallback to nil
-      @profile_id = options[:id] || params[:name] || nil
-      @params[:name] = @profile_id
-
-      @params[:rules] = rules = {}
       @runner = Runner.new(
         id: @profile_id,
         backend: :mock,
         test_collector: @options.delete(:test_collector),
       )
-      @runner.add_tests([@path], @options)
+
+      # we're checking a profile, we don't care if it runs on the host machine
+      @options[:ignore_supports] = true
+      tests, libs, metadata = @runner.add_tests([@path], @options)
+      @content = tests + libs + metadata
+
+      # NB if you want to check more than one profile, use one
+      # Inspec::Profile#from_file per profile
+      m = metadata.first
+      @metadata = Metadata.from_ref(m[:ref], m[:content], @profile_id, @logger)
+      @params = @metadata.params
+      @profile_id ||= params[:name]
+      @params[:name] = @profile_id
+      @params[:rules] = rules = {}
+
       @runner.rules.each do |id, rule|
         file = rule.instance_variable_get(:@__file)
         rules[file] ||= {}
@@ -98,15 +102,13 @@ module Inspec
 
       @logger.info "Checking profile in #{@path}"
 
-      if Pathname.new(path).join('metadata.rb').exist?
+      @logger.info 'Metadata OK.' if @metadata.valid?
+
+      if @content.any? { |h| h[:type] == :metadata && h[:ref] =~ /metadata\.rb$/ }
         warn.call('The use of `metadata.rb` is deprecated. Use `inspec.yml`.')
       end
 
-      @logger.info 'Metadata OK.' if @metadata.valid?
-
-      # check if the profile is using the old test directory instead of the
-      # new controls directory
-      if Pathname.new(path).join('test').exist? && !Pathname.new(path).join('controls').exist?
+      if @content.any? { |h| h[:type] == :test && h[:ref] =~ %r{test/[^/]+$} }
         warn.call('Profile uses deprecated `test` directory, rename it to `controls`.')
       end
 
@@ -195,16 +197,5 @@ module Inspec
       @logger.info 'Finished archive generation.'
       true
     end
-
-    private
-
-    def read_metadata
-      mpath = Pathname.new(path).join('inspec.yml')
-
-      # fallback to metadata.rb if inspec.yml does not exist
-      # TODO deprecated, will be removed in InSpec 1.0
-      mpath = File.join(@path, 'metadata.rb') if !mpath.exist?
-      Metadata.from_file(mpath, @profile_id, @logger)
-    end
   end
 end

lib/inspec/runner.rb

@@ -66,6 +66,7 @@ module Inspec
 
       tests = items.find_all { |i| i[:type] == :test }
       libs = items.find_all { |i| i[:type] == :library }
+      meta = items.find_all { |i| i[:type] == :metadata }
 
       # Ensure each test directory exists on the $LOAD_PATH. This
       # will ensure traditional RSpec-isms like `require 'spec_helper'`
@@ -82,6 +83,8 @@ module Inspec
       tests.flatten.each do |test|
         add_content(test, libs)
       end
+
+      [tests, libs, meta]
     end
 
     def create_context

lib/inspec/targets/archive.rb

@@ -23,17 +23,21 @@ module Inspec::Targets
         fail "Don't know how to handle folder #{target}"
       end
 
-      # get all test file contents
-      raw_files = helper.get_filenames(files)
-      tests = content(target, raw_files, rootdir, base_folder: target)
+      res = {
+        test:     collect(helper, files, :get_filenames),
+        library:  collect(helper, files, :get_libraries),
+        metadata: collect(helper, files, :get_metadata),
+      }.map { |as, list|
+        content(target, list, rootdir, base_folder: target, as: as)
+      }
 
-      libs = []
-      if helper.respond_to? :get_libraries
-        raw_libs = helper.get_libraries(files)
-        libs = content(target, raw_libs, rootdir, base_folder: target, as: :library)
+      res.flatten
     end
 
-      libs + tests
+    # FIXME(sr) dedup inspec/targets/folder
+    def collect(helper, files, getter)
+      return [] unless helper.respond_to? getter
+      helper.method(getter).call(files)
     end
   end
 end

lib/inspec/targets/tar.rb

@@ -39,9 +39,10 @@ module Inspec::Targets
           elsif entry.file?
             if files.include?(entry.full_name.gsub(rootdir, ''))
               h = {
-                content: entry.read,
+                # NB if some file is empty, return empty-string, not nil
+                content: entry.read || '',
                 type: opts[:as] || :test,
-                # ref: File.join(input, entry.name),
+                ref: entry.full_name,
               }
               content.push(h)
             end

lib/inspec/targets/zip.rb

@@ -18,9 +18,10 @@ module Inspec::Targets
         while (entry = io.get_next_entry)
           next if !files.include?(entry.name.gsub(rootdir, ''))
           h = {
-            content: io.read,
+            # NB if some file is empty, return empty-string, not nil
+            content: io.read || '',
             type: opts[:as] || :test,
-            # ref: File.join(input, entry.name),
+            ref: entry.name,
           }
           content.push(h)
         end

test/unit/profile_test.rb

@@ -6,6 +6,7 @@ require 'helper'
 require 'inspec/profile_context'
 require 'inspec/runner'
 require 'inspec/runner_mock'
+require 'fileutils'
 
 describe Inspec::Profile do
   let(:logger) { Minitest::Mock.new }
@@ -16,6 +17,20 @@ describe Inspec::Profile do
     Inspec::Profile.from_path("#{home}/mock/profiles/#{name}", opts)
   end
 
+  def load_profile_tgz(name, opts = {})
+    path = "#{home}/mock/profiles/#{name}"
+    `tar zcvf #{path}.tgz #{path}`
+    load_profile("#{name}.tgz", opts)
+    FileUtils.rm("#{path}.tgz")
+  end
+
+  def load_profile_zip(name, opts = {})
+    path = "#{home}/mock/profiles/#{name}"
+    `zip #{path}.zip #{path}`
+    load_profile("#{name}.zip", opts)
+    FileUtils.rm("#{path}.zip")
+  end
+
   describe 'with an empty profile' do
     let(:profile) { load_profile('empty-metadata') }
 
@@ -76,13 +91,14 @@ describe Inspec::Profile do
       let(:profile_id) { 'empty-metadata' }
 
       it 'prints loads of warnings' do
+        inspec_yml = "#{home}/mock/profiles/#{profile_id}/inspec.yml"
         logger.expect :info, nil, ["Checking profile in #{home}/mock/profiles/#{profile_id}"]
-        logger.expect :error, nil, ['Missing profile name in inspec.yml']
-        logger.expect :error, nil, ['Missing profile version in inspec.yml']
-        logger.expect :warn, nil, ['Missing profile title in inspec.yml']
-        logger.expect :warn, nil, ['Missing profile summary in inspec.yml']
-        logger.expect :warn, nil, ['Missing profile maintainer in inspec.yml']
-        logger.expect :warn, nil, ['Missing profile copyright in inspec.yml']
+        logger.expect :error, nil, ["Missing profile name in #{inspec_yml}"]
+        logger.expect :error, nil, ["Missing profile version in #{inspec_yml}"]
+        logger.expect :warn, nil, ["Missing profile title in #{inspec_yml}"]
+        logger.expect :warn, nil, ["Missing profile summary in #{inspec_yml}"]
+        logger.expect :warn, nil, ["Missing profile maintainer in #{inspec_yml}"]
+        logger.expect :warn, nil, ["Missing profile copyright in #{inspec_yml}"]
         logger.expect :warn, nil, ['No controls or tests were defined.']
 
         load_profile(profile_id, {logger: logger}).check
@@ -94,14 +110,15 @@ describe Inspec::Profile do
       let(:profile_id) { 'legacy-empty-metadata' }
 
       it 'prints loads of warnings' do
+        metadata_rb = "#{home}/mock/profiles/#{profile_id}/metadata.rb"
         logger.expect :info, nil, ["Checking profile in #{home}/mock/profiles/#{profile_id}"]
+        logger.expect :error, nil, ["Missing profile name in #{metadata_rb}"]
+        logger.expect :error, nil, ["Missing profile version in #{metadata_rb}"]
+        logger.expect :warn, nil, ["Missing profile title in #{metadata_rb}"]
+        logger.expect :warn, nil, ["Missing profile summary in #{metadata_rb}"]
+        logger.expect :warn, nil, ["Missing profile maintainer in #{metadata_rb}"]
+        logger.expect :warn, nil, ["Missing profile copyright in #{metadata_rb}"]
         logger.expect :warn, nil, ['The use of `metadata.rb` is deprecated. Use `inspec.yml`.']
-        logger.expect :error, nil, ['Missing profile name in metadata.rb']
-        logger.expect :error, nil, ['Missing profile version in metadata.rb']
-        logger.expect :warn, nil, ['Missing profile title in metadata.rb']
-        logger.expect :warn, nil, ['Missing profile summary in metadata.rb']
-        logger.expect :warn, nil, ['Missing profile maintainer in metadata.rb']
-        logger.expect :warn, nil, ['Missing profile copyright in metadata.rb']
         logger.expect :warn, nil, ['No controls or tests were defined.']
 
         load_profile(profile_id, {logger: logger}).check
@@ -131,7 +148,8 @@ describe Inspec::Profile do
         logger.expect :info, nil, ["Checking profile in #{home}/mock/profiles/#{profile_id}"]
         logger.expect :warn, nil, ['The use of `metadata.rb` is deprecated. Use `inspec.yml`.']
         logger.expect :info, nil, ['Metadata OK.']
-        logger.expect :warn, nil, ["Profile uses deprecated `test` directory, rename it to `controls`."]
+        # NB we only look at content that is loaded, i.e., there're no empty directories anymore
+        # logger.expect :warn, nil, ["Profile uses deprecated `test` directory, rename it to `controls`."]
         logger.expect :warn, nil, ['No controls or tests were defined.']
 
         profile.check
@@ -153,7 +171,39 @@ describe Inspec::Profile do
         logger.expect :debug, nil, ["Verify all rules in  #{home}/mock/profiles/#{profile_id}/controls/filesystem_spec.rb"]
         logger.expect :info, nil, ['Rule definitions OK.']
 
-        load_profile(profile_id, {logger: logger, ignore_supports: true}).check
+        load_profile(profile_id, {logger: logger}).check
+        logger.verify
+      end
+    end
+
+    describe 'a complete metadata profile with controls in a tarball' do
+      let(:profile_id) { 'complete-profile' }
+      let(:profile) { load_profile_tgz(profile_id, {logger: logger}) }
+
+      it 'prints ok messages and counts the rules' do
+        logger.expect :info, nil, ["Checking profile in #{home}/mock/profiles/#{profile_id}"]
+        logger.expect :info, nil, ['Metadata OK.']
+        logger.expect :info, nil, ['Found 1 rules.']
+        logger.expect :debug, nil, ["Verify all rules in  #{home}/mock/profiles/#{profile_id}/controls/filesystem_spec.rb"]
+        logger.expect :info, nil, ['Rule definitions OK.']
+
+        load_profile(profile_id, {logger: logger}).check
+        logger.verify
+      end
+    end
+
+    describe 'a complete metadata profile with controls in zipfile' do
+      let(:profile_id) { 'complete-profile' }
+      let(:profile) { load_profile_zip(profile_id, {logger: logger}) }
+
+      it 'prints ok messages and counts the rules' do
+        logger.expect :info, nil, ["Checking profile in #{home}/mock/profiles/#{profile_id}"]
+        logger.expect :info, nil, ['Metadata OK.']
+        logger.expect :info, nil, ['Found 1 rules.']
+        logger.expect :debug, nil, ["Verify all rules in  #{home}/mock/profiles/#{profile_id}/controls/filesystem_spec.rb"]
+        logger.expect :info, nil, ['Rule definitions OK.']
+
+        load_profile(profile_id, {logger: logger}).check
         logger.verify
       end
     end