Merge pull request #1371 from chef/robb/untar-with-mixlib-archive

Fix fieri's handling of cookbook tarballs with non-dir/file entries

src/fieri/Gemfile.lock

@@ -13,6 +13,7 @@ PATH
     fieri (0.0.1)
       dotenv-rails
       foodcritic
+      mixlib-archive (~> 0.2)
       rails (~> 4.1.14.2)
       sidekiq
 
@@ -53,33 +54,36 @@ GEM
     connection_pool (2.2.0)
     crack (0.4.3)
       safe_yaml (~> 1.0.0)
-    cucumber-core (1.4.0)
-      gherkin (~> 3.2.0)
+    cucumber-core (1.5.0)
+      gherkin (~> 4.0)
     diff-lcs (1.2.5)
     dotenv (2.1.1)
     dotenv-rails (2.1.1)
       dotenv (= 2.1.1)
       railties (>= 4.0, < 5.1)
     erubis (2.7.0)
-    foodcritic (6.2.0)
+    foodcritic (7.0.1)
       cucumber-core (>= 1.3)
       erubis
       nokogiri (>= 1.5, < 2.0)
       rake
       rufus-lru (~> 1.0)
       treetop (~> 1.4)
       yajl-ruby (~> 1.1)
-    gherkin (3.2.0)
+    gherkin (4.0.0)
     hashdiff (0.3.0)
     i18n (0.7.0)
     json (1.8.3)
     mail (2.6.4)
       mime-types (>= 1.16, < 4)
-    mime-types (3.0)
+    mime-types (3.1)
       mime-types-data (~> 3.2015)
-    mime-types-data (3.2016.0221)
+    mime-types-data (3.2016.0521)
     mini_portile2 (2.1.0)
     minitest (5.8.4)
+    mixlib-archive (0.2.0)
+      mixlib-log
+    mixlib-log (1.6.0)
     nokogiri (1.6.8)
       mini_portile2 (~> 2.1.0)
       pkg-config (~> 1.1.7)
@@ -89,6 +93,8 @@ GEM
     polyglot (0.3.5)
     powerpack (0.1.1)
     rack (1.5.5)
+    rack-protection (1.5.3)
+      rack
     rack-test (0.6.3)
       rack (>= 1.0)
     rails (4.1.14.2)
@@ -135,11 +141,16 @@ GEM
     ruby-progressbar (1.8.1)
     rufus-lru (1.1.0)
     safe_yaml (1.0.4)
-    sidekiq (4.1.2)
+    sidekiq (4.1.4)
       concurrent-ruby (~> 1.0)
       connection_pool (~> 2.2, >= 2.2.0)
       redis (~> 3.2, >= 3.2.1)
-    sprockets (3.6.0)
+      sinatra (>= 1.4.7)
+    sinatra (1.4.7)
+      rack (~> 1.5)
+      rack-protection (~> 1.4)
+      tilt (>= 1.3, < 3)
+    sprockets (3.6.3)
       concurrent-ruby (~> 1.0)
       rack (> 1, < 3)
     sprockets-rails (2.3.3)
@@ -149,6 +160,7 @@ GEM
     sqlite3 (1.3.11)
     thor (0.19.1)
     thread_safe (0.3.5)
+    tilt (2.0.5)
     treetop (1.6.5)
       polyglot (~> 0.3)
     tzinfo (1.2.2)

src/fieri/app/models/cookbook_artifact.rb

@@ -1,12 +1,13 @@
 require 'open-uri'
 require 'rubygems/package'
 require 'foodcritic'
+require 'mixlib/archive'
 
 class CookbookArtifact
   #
   # Accessors
   #
-  attr_accessor :url, :archive, :directory, :job_id, :work_dir
+  attr_accessor :url, :job_id, :work_dir
 
   #
   # Initializes a +CookbookArtifact+ downloading and unarchiving the
@@ -19,8 +20,15 @@ def initialize(url, jid)
     @url = url
     @job_id = jid || 'nojobid'
     @work_dir = File.join(Dir.tmpdir, job_id)
-    @archive = download
-    @directory = unarchive
+  end
+
+  #
+  # downloads and untars a cookbook to the work_dir
+  #
+  def prep
+    downloaded_tarball = download
+    tar = Mixlib::Archive.new(downloaded_tarball.path)
+    tar.extract(work_dir, perms: false, ignore: /^\.$/)
   end
 
   #
@@ -30,7 +38,9 @@ def initialize(url, jid)
   # @return [String] the would be command line out from FoodCritic
   #
   def criticize
-    args = [directory, "-f #{ENV['FIERI_FOODCRITIC_FAIL_TAGS']}"]
+    prep
+
+    args = [work_dir, "-f #{ENV['FIERI_FOODCRITIC_FAIL_TAGS']}"]
     ENV['FIERI_FOODCRITIC_TAGS'].split.each do |tag|
       args.push("-t #{tag}")
     end if ENV['FIERI_FOODCRITIC_TAGS']
@@ -64,32 +74,4 @@ def download
       saved_file
     end
   end
-
-  #
-  # Unarchives an artifact into the tmp directory. The unarchived artifact
-  # will be deleted when the +CookbookArtifact+ is garbage collected.
-  #
-  # @return [String] the directory where the unarchived artifact lives.
-  #
-  def unarchive
-    Gem::Package::TarReader.new(Zlib::GzipReader.open(archive.path)) do |tar|
-      root = File.join(work_dir, tar.first.header.name.split('/')[0])
-      tar.rewind
-
-      tar.each do |entry|
-        next unless entry.file?
-
-        destination_file = File.join(work_dir, entry.header.name)
-        destination_dir = File.dirname(destination_file)
-
-        FileUtils.mkdir_p destination_dir unless File.directory?(destination_dir)
-
-        file = File.open(destination_file, 'wb+')
-        file << entry.read
-        file.close
-      end
-
-      return root
-    end
-  end
 end

src/fieri/fieri.gemspec

@@ -20,4 +20,5 @@ Gem::Specification.new do |s|
   s.add_dependency 'sidekiq'
   s.add_dependency 'dotenv-rails'
   s.add_dependency 'foodcritic'
+  s.add_dependency 'mixlib-archive', '~> 0.2'
 end

src/fieri/spec/models/cookbook_artifact_spec.rb

@@ -17,17 +17,21 @@
         expect(artifact.url).to eq('http://example.com/apache.tar.gz')
       end
 
-      it 'assigns #archive' do
-        expect(artifact.archive).to be_a(File)
+      it 'assigns #work_dir' do
+        expect(artifact.work_dir).to eq(File.join(Dir.tmpdir, 'somejobid'))
       end
+    end
+
+    describe '#prep' do
+      it 'prepares a unique directory for the job' do
+        artifact.prep
 
-      it 'assigns #directory' do
-        expect(artifact.directory).to eq(File.join(artifact.work_dir, 'apache2'))
+        expect(Dir).to exist(artifact.work_dir)
       end
     end
 
     describe '#criticize' do
-      it 'it returns the feedback and status from the FoodCritic run' do
+      it 'returns the feedback and status from the FoodCritic run' do
         feedback, status = artifact.criticize
 
         assert_match(/FC023/, feedback)
@@ -37,8 +41,10 @@
 
     describe '#clean' do
       it 'deletes the artifacts unarchived directory' do
+        artifact.prep
+
         artifact.cleanup
-        assert !Dir.exist?(artifact.work_dir)
+        expect(Dir).not_to exist(artifact.work_dir)
       end
     end
   end

src/fieri/spec/models/foodcritic_worker_spec.rb

@@ -36,23 +36,4 @@
       req.body =~ /FC023/
     end
   end
-
-  it 'creates a unique directory for each job to work within' do
-    Sidekiq::Testing.inline! do
-      job_id_1 = FoodcriticWorker.perform_async(
-        'cookbook_artifact_url' => 'http://example.com/apache.tar.gz',
-        'cookbook_name' => 'apache2',
-        'cookbook_version' => '1.2.0'
-      )
-
-      job_id_2 = FoodcriticWorker.perform_async(
-        'cookbook_artifact_url' => 'http://example.com/apache.tar.gz',
-        'cookbook_name' => 'apache2',
-        'cookbook_version' => '1.2.0'
-      )
-
-      assert Dir.exist?(File.join(Dir.tmpdir, job_id_1))
-      assert Dir.exist?(File.join(Dir.tmpdir, job_id_2))
-    end
-  end
 end

src/supermarket/Gemfile.lock

@@ -13,6 +13,7 @@ PATH
     fieri (0.0.1)
       dotenv-rails
       foodcritic
+      mixlib-archive (~> 0.2)
       rails (~> 4.1.14.2)
       sidekiq
 
@@ -143,8 +144,8 @@ GEM
       safe_yaml (~> 1.0.0)
     css_parser (1.3.5)
       addressable
-    cucumber-core (1.4.0)
-      gherkin (~> 3.2.0)
+    cucumber-core (1.5.0)
+      gherkin (~> 4.0)
     database_cleaner (1.3.0)
     debugger-linecache (1.2.0)
     descendants_tracker (0.0.4)
@@ -169,7 +170,7 @@ GEM
     ffi-yajl (1.4.0)
       ffi (~> 1.5)
       libyajl2 (~> 1.2)
-    foodcritic (6.2.0)
+    foodcritic (7.0.1)
       cucumber-core (>= 1.3)
       erubis
       nokogiri (>= 1.5, < 2.0)
@@ -182,7 +183,7 @@ GEM
       thor (~> 0.19.1)
     formatador (0.2.5)
     fssm (0.2.10)
-    gherkin (3.2.0)
+    gherkin (4.0.0)
     guard (2.13.0)
       formatador (>= 0.2.4)
       listen (>= 2.7, <= 4.0)
@@ -248,6 +249,8 @@ GEM
     mimemagic (0.3.0)
     mini_portile2 (2.1.0)
     minitest (5.8.4)
+    mixlib-archive (0.2.0)
+      mixlib-log
     mixlib-authentication (1.3.0)
       mixlib-log
     mixlib-cli (1.5.0)