Merge pull request #5080 from chef/tm/fix_tarball_extract

Use Mixlib::Archive to extract tarballs

7 files changed, 26 insertions, 28 deletions

diff --git a/Gemfile.lock b/Gemfile.lock
index b880bb57ee..131186a456 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -26,6 +26,7 @@ PATH
       ffi-yajl (~> 2.2)
       highline (~> 1.6, >= 1.6.9)
       iniparse (~> 1.4)
+      mixlib-archive (>= 0.2.0)
       mixlib-authentication (~> 1.4)
       mixlib-cli (~> 1.4)
       mixlib-log (~> 1.3)
@@ -36,9 +37,9 @@ PATH
       ohai (>= 8.6.0.alpha.1, < 9)
       plist (~> 3.2)
       proxifier (~> 1.0)
-      rspec-core (~> 3.4)
-      rspec-expectations (~> 3.4)
-      rspec-mocks (~> 3.4)
+      rspec-core (~> 3.4.0)
+      rspec-expectations (~> 3.4.0)
+      rspec-mocks (~> 3.4.0)
       rspec_junit_formatter (~> 0.2.0)
       serverspec (~> 2.7)
       specinfra (~> 2.10)
@@ -54,6 +55,7 @@ PATH
       ffi-yajl (~> 2.2)
       highline (~> 1.6, >= 1.6.9)
       iniparse (~> 1.4)
+      mixlib-archive (>= 0.2.0)
       mixlib-authentication (~> 1.4)
       mixlib-cli (~> 1.4)
       mixlib-log (~> 1.3)
@@ -64,9 +66,9 @@ PATH
       ohai (>= 8.6.0.alpha.1, < 9)
       plist (~> 3.2)
       proxifier (~> 1.0)
-      rspec-core (~> 3.4)
-      rspec-expectations (~> 3.4)
-      rspec-mocks (~> 3.4)
+      rspec-core (~> 3.4.0)
+      rspec-expectations (~> 3.4.0)
+      rspec-mocks (~> 3.4.0)
       rspec_junit_formatter (~> 0.2.0)
       serverspec (~> 2.7)
       specinfra (~> 2.10)
@@ -218,6 +220,8 @@ GEM
       mime-types-data (~> 3.2015)
     mime-types-data (3.2016.0521)
     mini_portile2 (2.1.0)
+    mixlib-archive (0.2.0)
+      mixlib-log
     mixlib-authentication (1.4.1)
       mixlib-log
     mixlib-cli (1.6.0)
diff --git a/chef.gemspec b/chef.gemspec
index b88c899d5c..b65332bb91 100644
--- a/chef.gemspec
+++ b/chef.gemspec
@@ -21,6 +21,7 @@ Gem::Specification.new do |s|
   s.add_dependency "mixlib-log", "~> 1.3"
   s.add_dependency "mixlib-authentication", "~> 1.4"
   s.add_dependency "mixlib-shellout", "~> 2.0"
+  s.add_dependency "mixlib-archive", ">= 0.2.0"
   s.add_dependency "ohai", ">= 8.6.0.alpha.1", "< 9"
 
   s.add_dependency "ffi-yajl", "~> 2.2"
@@ -37,7 +38,7 @@ Gem::Specification.new do |s|
   s.add_dependency "iniparse", "~> 1.4"
 
   # Audit mode requires these, so they are non-developmental dependencies now
-  %w{rspec-core rspec-expectations rspec-mocks}.each { |gem| s.add_dependency gem, "~> 3.4" }
+  %w{rspec-core rspec-expectations rspec-mocks}.each { |gem| s.add_dependency gem, "~> 3.4.0" }
   s.add_dependency "rspec_junit_formatter", "~> 0.2.0"
   s.add_dependency "serverspec", "~> 2.7"
   s.add_dependency "specinfra", "~> 2.10"
diff --git a/lib/chef/application/client.rb b/lib/chef/application/client.rb
index 77c86ad559..2a2f70c3a1 100644
--- a/lib/chef/application/client.rb
+++ b/lib/chef/application/client.rb
@@ -27,6 +27,7 @@ require "chef/handler/error_report"
 require "chef/workstation_config_loader"
 require "chef/mixin/shell_out"
 require "chef-config/mixin/dot_d"
+require "mixlib/archive"
 
 class Chef::Application::Client < Chef::Application
   include Chef::Mixin::ShellOut
@@ -334,8 +335,7 @@ class Chef::Application::Client < Chef::Application
         FileUtils.mkdir_p(Chef::Config.chef_repo_path)
         tarball_path = File.join(Chef::Config.chef_repo_path, "recipes.tgz")
         fetch_recipe_tarball(Chef::Config[:recipe_url], tarball_path)
-        result = shell_out!("tar zxvf #{tarball_path} -C #{Chef::Config.chef_repo_path}")
-        Chef::Log.debug "#{result.stdout}"
+        Mixlib::Archive.new(tarball_path).extract(Chef::Config.chef_repo_path, perms: false, ignore: /^\.$/)
       end
     end
 
diff --git a/lib/chef/application/solo.rb b/lib/chef/application/solo.rb
index d2516926c4..a7c4038f4c 100644
--- a/lib/chef/application/solo.rb
+++ b/lib/chef/application/solo.rb
@@ -29,6 +29,7 @@ require "fileutils"
 require "chef/mixin/shell_out"
 require "pathname"
 require "chef-config/mixin/dot_d"
+require "mixlib/archive"
 
 class Chef::Application::Solo < Chef::Application
   include Chef::Mixin::ShellOut
@@ -273,8 +274,7 @@ class Chef::Application::Solo < Chef::Application
       FileUtils.mkdir_p(recipes_path)
       tarball_path = File.join(recipes_path, "recipes.tgz")
       fetch_recipe_tarball(Chef::Config[:recipe_url], tarball_path)
-      result = shell_out!("tar zxvf #{tarball_path} -C #{recipes_path}")
-      Chef::Log.debug "#{result.stdout}"
+      Mixlib::Archive.new(tarball_path).extract(Chef::Config.chef_repo_path, perms: false, ignore: /^\.$/)
     end
 
     # json_attribs shuld be fetched after recipe_url tarball is unpacked.
diff --git a/lib/chef/knife/cookbook_site_install.rb b/lib/chef/knife/cookbook_site_install.rb
index 9a1ac93821..43d015dcc4 100644
--- a/lib/chef/knife/cookbook_site_install.rb
+++ b/lib/chef/knife/cookbook_site_install.rb
@@ -19,6 +19,7 @@
 require "chef/knife"
 require "chef/exceptions"
 require "shellwords"
+require "mixlib/archive"
 
 class Chef
   class Knife
@@ -149,17 +150,7 @@ class Chef
 
       def extract_cookbook(upstream_file, version)
         ui.info("Uncompressing #{@cookbook_name} version #{version}.")
-        extract_command = "tar zxvf \"#{convert_path upstream_file}\""
-        if Chef::Platform.windows?
-          tar_version = shell_out("tar --version").stdout.tr("\n", " ")
-          if tar_version =~ /GNU tar/
-            Chef::Log.debug("GNU tar detected, adding --force-local")
-            extract_command << " --force-local"
-          else
-            Chef::Log.debug("non-GNU tar detected, not adding --force-local")
-          end
-        end
-        shell_out!(extract_command, :cwd => @install_path)
+        Mixlib::Archive.new(convert_path(upstream_file)).extract(@install_path, perms: false)
       end
 
       def clear_existing_files(cookbook_path)
diff --git a/spec/unit/application/solo_spec.rb b/spec/unit/application/solo_spec.rb
index b1931414cc..686ae745d8 100644
--- a/spec/unit/application/solo_spec.rb
+++ b/spec/unit/application/solo_spec.rb
@@ -120,9 +120,10 @@ Enable chef-client interval runs by setting `:client_fork = true` in your config
         expect(app).to receive(:open).with("http://junglist.gen.nz/recipes.tgz").and_yield(tarfile)
         expect(File).to receive(:open).with("#{Dir.tmpdir}/chef-solo/recipes.tgz", "wb").and_yield(target_file)
 
-        shellout = instance_double("Mixlib::ShellOut", run_command: nil, error!: nil, stdout: "")
+        archive = double(Mixlib::Archive)
 
-        expect(app).to receive(:shell_out!).with("tar zxvf #{Dir.tmpdir}/chef-solo/recipes.tgz -C #{Dir.tmpdir}/chef-solo").and_return(shellout)
+        expect(Mixlib::Archive).to receive(:new).with("#{Dir.tmpdir}/chef-solo/recipes.tgz").and_return(archive)
+        expect(archive).to receive(:extract).with("#{Dir.tmpdir}/chef-solo", { perms: false, ignore: /^\.$/ })
         app.reconfigure
         expect(target_file.string).to eq("remote_tarball_content")
       end
@@ -136,11 +137,10 @@ Enable chef-client interval runs by setting `:client_fork = true` in your config
         Chef::Config[:cookbook_path] = "#{Dir.tmpdir}/chef-solo/cookbooks"
         expect(FileUtils).to receive(:mkdir_p).with("#{Dir.tmpdir}/chef-solo").and_return(true)
 
-        allow(Chef::Mixin::Command).to receive(:run_command).and_return(true)
+        archive = double(Mixlib::Archive)
 
-        shellout = instance_double("Mixlib::ShellOut", run_command: nil, error!: nil, stdout: "")
-
-        expect(app).to receive(:shell_out!).with("tar zxvf #{Dir.tmpdir}/chef-solo/recipes.tgz -C #{Dir.tmpdir}/chef-solo").and_return(shellout)
+        expect(Mixlib::Archive).to receive(:new).with("#{Dir.tmpdir}/chef-solo/recipes.tgz").and_return(archive)
+        expect(archive).to receive(:extract).with("#{Dir.tmpdir}/chef-solo", { perms: false, ignore: /^\.$/ })
         expect(app).to receive(:fetch_recipe_tarball).ordered
         expect(Chef::ConfigFetcher).to receive(:new).ordered.and_return(config_fetcher)
         app.reconfigure
diff --git a/spec/unit/knife/cookbook_site_install_spec.rb b/spec/unit/knife/cookbook_site_install_spec.rb
index d60443d779..1549245ea3 100644
--- a/spec/unit/knife/cookbook_site_install_spec.rb
+++ b/spec/unit/knife/cookbook_site_install_spec.rb
@@ -23,6 +23,7 @@ describe Chef::Knife::CookbookSiteInstall do
   let(:stdout) { StringIO.new }
   let(:stderr) { StringIO.new }
   let(:downloader) { Hash.new }
+  let(:archive) { double(Mixlib::Archive, extract: true) }
   let(:repo) { double(:sanity_check => true, :reset_to_default_state => true,
                       :prepare_to_import => true, :finalize_updates_to => true,
                       :merge_updates_from => true) }
@@ -48,6 +49,7 @@ describe Chef::Knife::CookbookSiteInstall do
     allow(File).to receive(:unlink)
     allow(File).to receive(:rmtree)
     allow(knife).to receive(:shell_out!).and_return(true)
+    allow(Mixlib::Archive).to receive(:new).and_return(archive)
 
     # CookbookSiteDownload Stup
     allow(knife).to receive(:download_cookbook_to).and_return(downloader)