Break out profile vendor activities into separate class

Per PR feedback, `Inspec::ProfileVendor` is created to centralize
the logic and data of vendoring profile dependencies. The `BaseCLI`
class and the `Habitat::Profile` class have been modified to use it

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

lib/bundles/inspec-habitat/profile.rb

@@ -1,6 +1,7 @@
 # encoding: utf-8
 # author: Adam Leff
 
+require 'inspec/profile_vendor'
 require 'mixlib/shellout'
 require 'toml'
 
@@ -83,19 +84,10 @@ module Habitat
     def create_profile_object
       @profile = Inspec::Profile.for_target(
         path,
-        cache: Inspec::Cache.new(cache_path.to_s),
         backend: Inspec::Backend.create(target: 'mock://'),
       )
     end
 
-    def cache_path
-      File.join(path, 'vendor')
-    end
-
-    def inspec_lockfile
-      File.join(path, 'inspec.lock')
-    end
-
     def verify_profile
       Habitat::Log.info('Checking to see if the profile is valid...')
 
@@ -107,13 +99,15 @@ module Habitat
     end
 
     def vendor_profile_dependencies
-      if File.exist?(inspec_lockfile) && Dir.exist?(cache_path)
+      profile_vendor = Inspec::ProfileVendor.new(path)
+      if profile_vendor.lockfile.exist? && profile_vendor.cache_path.exist?
         Habitat::Log.info("Profile's dependencies are already vendored, skipping vendor process.")
       else
         Habitat::Log.info("Vendoring the profile's dependencies...")
-        FileUtils.rm_rf(cache_path)
-        File.delete(inspec_lockfile) if File.exist?(inspec_lockfile)
-        File.write(inspec_lockfile, profile.generate_lockfile.to_yaml)
+        profile_vendor.vendor!
+
+        Habitat::Log.info('Ensuring all vendored content has read permissions...')
+        profile_vendor.make_readable
 
         # refresh the profile object since the profile now has new files
         create_profile_object

lib/fetchers/url.rb

@@ -155,7 +155,6 @@ module Fetchers
       final_path = "#{path}#{@archive_type}"
       FileUtils.mkdir_p(File.dirname(final_path))
       FileUtils.mv(temp_archive_path, final_path)
-      FileUtils.chmod(0644, final_path)
       Inspec::Log.debug("Fetched archive moved to: #{final_path}")
       @temp_archive_path = nil
       final_path

lib/inspec/base_cli.rb

@@ -4,6 +4,7 @@
 
 require 'thor'
 require 'inspec/log'
+require 'inspec/profile_vendor'
 
 module Inspec
   class BaseCLI < Thor # rubocop:disable Metrics/ClassLength
@@ -147,30 +148,16 @@ module Inspec
     end
 
     def vendor_deps(path, opts)
-      path.nil? ? path = Pathname.new(Dir.pwd) : path = Pathname.new(path)
-      cache_path = path.join('vendor')
-      inspec_lock = path.join('inspec.lock')
+      profile_path = path || Dir.pwd
+      profile_vendor = Inspec::ProfileVendor.new(profile_path)
 
-      if (cache_path.exist? || inspec_lock.exist?) && !opts[:overwrite]
+      if (profile_vendor.cache_path.exist? || profile_vendor.lockfile.exist?) && !opts[:overwrite]
         puts 'Profile is already vendored. Use --overwrite.'
         return false
       end
 
-      # remove existing
-      FileUtils.rm_rf(cache_path) if cache_path.exist?
-      File.delete(inspec_lock) if inspec_lock.exist?
-
-      puts "Vendor dependencies of #{path} into #{cache_path}"
-      opts[:logger] = Logger.new(STDOUT)
-      opts[:logger].level = get_log_level(opts.log_level)
-      opts[:cache] = Inspec::Cache.new(cache_path.to_s)
-      opts[:backend] = Inspec::Backend.create(target: 'mock://')
-      configure_logger(opts)
-
-      # vendor dependencies and generate lockfile
-      profile = Inspec::Profile.for_target(path.to_s, opts)
-      lockfile = profile.generate_lockfile
-      File.write(inspec_lock, lockfile.to_yaml)
+      profile_vendor.vendor!
+      puts "Profile dependencies successfully vendored to #{profile_vendor.cache_path}"
     rescue StandardError => e
       pretty_handle_exception(e)
     end

lib/inspec/profile_vendor.rb

@@ -0,0 +1,66 @@
+# encoding: utf-8
+# author: Adam Leff
+
+require 'inspec/profile'
+
+module Inspec
+  class ProfileVendor
+    attr_reader :profile_path
+
+    def initialize(path)
+      @profile_path = Pathname.new(path)
+    end
+
+    def vendor!
+      vendor_dependencies
+    end
+
+    # The URL fetcher uses a Tempfile to retrieve the vendored
+    # profile, which creates a file that is only readable by
+    # the current user. In most circumstances, this is likely OK.
+    # However, in environments like a Habitat package, these files
+    # need to be readable by all users or the Habitat Supervisor
+    # may not be able to start InSpec correctly.
+    #
+    # This method makes sure all vendored files are mode 644 for this
+    # use case. This method is not called by default - the caller
+    # vendoring the profile must make the decision as to whether this
+    # is necessary.
+    def make_readable
+      Dir.glob("#{cache_path}/**/*") do |e|
+        FileUtils.chmod(0644, e) if File.file?(e)
+      end
+    end
+
+    def cache_path
+      profile_path.join('vendor')
+    end
+
+    def lockfile
+      profile_path.join('inspec.lock')
+    end
+
+    private
+
+    def profile
+      @profile ||= Inspec::Profile.for_target(profile_path.to_s, profile_opts)
+    end
+
+    def profile_opts
+      {
+        cache: Inspec::Cache.new(cache_path.to_s),
+        backend: Inspec::Backend.create(target: 'mock://'),
+      }
+    end
+
+    def vendor_dependencies
+      delete_vendored_data
+      File.write(lockfile, profile.generate_lockfile.to_yaml)
+    end
+
+    def delete_vendored_data
+      FileUtils.rm_rf(cache_path) if cache_path.exist?
+      File.delete(lockfile) if lockfile.exist?
+    end
+  end
+end

test/unit/bundles/inspect-habitat/profile_test.rb

@@ -34,37 +34,35 @@ describe Habitat::Profile do
   end
 
   describe '#vendor_profile_dependencies' do
-    let(:cache)    { '/path/to/cache' }
-    let(:lockfile) { '/path/to/lock' }
-    let(:profile) do
-      profile = mock
-      profile.stubs(:generate_lockfile).returns(foo: 'bar')
-      profile
+    let(:profile_vendor) do
+      profile_vendor = mock
+      profile_vendor.stubs(:lockfile).returns(lockfile)
+      profile_vendor.stubs(:cache_path).returns(cache_path)
+      profile_vendor
     end
+    let(:lockfile) { mock }
+    let(:cache_path) { mock }
 
     before do
-      subject.expects(:cache_path).at_least_once.returns(cache)
-      subject.expects(:inspec_lockfile).at_least_once.returns(lockfile)
+      Inspec::ProfileVendor.expects(:new).returns(profile_vendor)
     end
 
     describe 'when lockfile exists and cache dir exists' do
-      it 'does not generate the lockfile' do
-        File.expects(:exist?).with(lockfile).returns(true)
-        Dir.expects(:exist?).with(cache).returns(true)
-        subject.expects(:profile).returns(profile).never
-        profile.expects(:generate_lockfile).never
+      it 'does not vendor the dependencies' do
+        lockfile.expects(:exist?).returns(true)
+        cache_path.expects(:exist?).returns(true)
+        profile_vendor.expects(:vendor!).never
+        profile_vendor.expects(:make_readable).never
         subject.send(:vendor_profile_dependencies)
       end
     end
 
     describe 'when the lockfile exists but the cache dir does not' do
-      it 'deletes the lockfile, generates the lockfile, and refreshes the profile object' do
-        File.expects(:exist?).with(lockfile).returns(true).at_least_once
-        Dir.expects(:exist?).with(cache).returns(false).at_least_once
-        FileUtils.expects(:rm_rf).with(cache)
-        File.expects(:delete).with(lockfile)
-        subject.expects(:profile).returns(profile)
-        File.expects(:write).with(lockfile, "---\n:foo: bar\n")
+      it 'vendors the dependencies and refreshes the profile object' do
+        lockfile.expects(:exist?).returns(true)
+        cache_path.expects(:exist?).returns(false)
+        profile_vendor.expects(:vendor!)
+        profile_vendor.expects(:make_readable)
         subject.expects(:create_profile_object)
 
         subject.send(:vendor_profile_dependencies)
@@ -72,11 +70,10 @@ describe Habitat::Profile do
     end
 
     describe 'when the lockfile does not exist' do
-      it 'generates the lockfile, and refreshes the profile object' do
-        File.expects(:exist?).with(lockfile).returns(false).at_least_once
-        FileUtils.expects(:rm_rf).with(cache)
-        subject.expects(:profile).returns(profile)
-        File.expects(:write).with(lockfile, "---\n:foo: bar\n")
+      it 'vendors the dependencies and refreshes the profile object' do
+        lockfile.expects(:exist?).returns(false)
+        profile_vendor.expects(:vendor!)
+        profile_vendor.expects(:make_readable)
         subject.expects(:create_profile_object)
 
         subject.send(:vendor_profile_dependencies)