Merge pull request #2642 from opscode/btm/site_install_json

knife cookbook site install json support w/tests

3 files changed, 194 insertions, 124 deletions

diff --git a/lib/chef/exceptions.rb b/lib/chef/exceptions.rb
index bf33cb9b52..f5d91c24a6 100644
--- a/lib/chef/exceptions.rb
+++ b/lib/chef/exceptions.rb
@@ -150,6 +150,11 @@ class Chef
     class IllegalVersionConstraint < NotImplementedError; end
 
     class MetadataNotValid < StandardError; end
+    class MetadataNotFound < StandardError
+      def initialize
+        super "No metadata.rb or metadata.json!"
+      end
+    end
 
     # File operation attempted but no permissions to perform it
     class InsufficientPermissions < RuntimeError; end
diff --git a/lib/chef/knife/cookbook_site_install.rb b/lib/chef/knife/cookbook_site_install.rb
index 913d171b3c..edf8dd14f0 100644
--- a/lib/chef/knife/cookbook_site_install.rb
+++ b/lib/chef/knife/cookbook_site_install.rb
@@ -17,11 +17,11 @@
 #
 
 require 'chef/knife'
+require 'chef/exceptions'
 require 'shellwords'
 
 class Chef
   class Knife
-
     class CookbookSiteInstall < Knife
 
       deps do
@@ -107,11 +107,8 @@ class Chef
           end
         end
 
-
         unless config[:no_deps]
-          md = Chef::Cookbook::Metadata.new
-          md.from_file(File.join(@install_path, @cookbook_name, "metadata.rb"))
-          md.dependencies.each do |cookbook, version_list|
+          preferred_metadata.dependencies.each do |cookbook, version_list|
             # Doesn't do versions.. yet
             nv = self.class.new
             nv.config = config
@@ -144,6 +141,7 @@ class Chef
 
       def extract_cookbook(upstream_file, version)
         ui.info("Uncompressing #{@cookbook_name} version #{version}.")
+        # FIXME: Detect if we have the bad tar from git on Windows: https://github.com/opscode/chef/issues/1753
         shell_out!("tar zxvf #{convert_path upstream_file}", :cwd => @install_path)
       end
 
@@ -153,11 +151,37 @@ class Chef
       end
 
       def convert_path(upstream_file)
-      	if ENV['MSYSTEM'] == 'MINGW32'
-      	  return upstream_file.sub(/^([[:alpha:]]):/, '/\1')
-	      else
-      	  return Shellwords.escape upstream_file
-      	end
+        # converts a Windows path (C:\foo) to a mingw path (/c/foo)
+        if ENV['MSYSTEM'] == 'MINGW32'
+          return upstream_file.sub(/^([[:alpha:]]):/, '/\1')
+        else
+          return Shellwords.escape upstream_file
+        end
+      end
+
+      # Get the preferred metadata path on disk. Chef prefers the metadata.rb
+      # over the metadata.json.
+      #
+      # @raise if there is no metadata in the cookbook
+      #
+      # @return [Chef::Cookbook::Metadata]
+      def preferred_metadata
+        md = Chef::Cookbook::Metadata.new
+
+        rb   = File.join(@install_path, @cookbook_name, "metadata.rb")
+        if File.exist?(rb)
+          md.from_file(rb)
+          return md
+        end
+
+        json = File.join(@install_path, @cookbook_name, "metadata.json")
+        if File.exist?(json)
+          json = IO.read(json)
+          md.from_json(json)
+          return md
+        end
+
+        raise Chef::Exceptions::MetadataNotFound
       end
     end
   end
diff --git a/spec/unit/knife/cookbook_site_install_spec.rb b/spec/unit/knife/cookbook_site_install_spec.rb
index c4bd8f67d2..b3eef32b39 100644
--- a/spec/unit/knife/cookbook_site_install_spec.rb
+++ b/spec/unit/knife/cookbook_site_install_spec.rb
@@ -19,132 +19,173 @@
 require File.expand_path(File.join(File.dirname(__FILE__), "..", "..", "spec_helper"))
 
 describe Chef::Knife::CookbookSiteInstall do
+  let(:knife) { Chef::Knife::CookbookSiteInstall.new }
+  let(:stdout) { StringIO.new }
+  let(:stderr) { StringIO.new }
+  let(:downloader) { Hash.new }
+  let(:repo) { double(:sanity_check => true, :reset_to_default_state => true,
+                      :prepare_to_import => true, :finalize_updates_to => true,
+                      :merge_updates_from => true) }
+  let(:install_path) { if Chef::Platform.windows?
+                        'C:/tmp/chef'
+                      else
+                        '/var/tmp/chef'
+                      end }
+
   before(:each) do
     require 'chef/knife/core/cookbook_scm_repo'
-    @stdout = StringIO.new
-    @knife = Chef::Knife::CookbookSiteInstall.new
-    allow(@knife.ui).to receive(:stdout).and_return(@stdout)
-    @knife.config = {}
-    if Chef::Platform.windows?
-      @install_path = 'C:/tmp/chef'
-    else
-      @install_path = '/var/tmp/chef'
-    end
-    @knife.config[:cookbook_path] = [ @install_path ]
-
-    @stdout = StringIO.new
-    @stderr = StringIO.new
-    allow(@knife).to receive(:stderr).and_return(@stdout)
-    allow(@knife).to receive(:stdout).and_return(@stdout)
-
-    #Assume all external commands would have succeed. :(
+
+    allow(knife.ui).to receive(:stdout).and_return(stdout)
+    knife.config = {}
+    knife.config[:cookbook_path] = [ install_path ]
+
+    allow(knife).to receive(:stderr).and_return(stderr)
+    allow(knife).to receive(:stdout).and_return(stdout)
+
+    # Assume all external commands would have succeed. :(
     allow(File).to receive(:unlink)
     allow(File).to receive(:rmtree)
-    allow(@knife).to receive(:shell_out!).and_return(true)
-
-    #CookbookSiteDownload Stup
-    @downloader = {}
-    allow(@knife).to receive(:download_cookbook_to).and_return(@downloader)
-    allow(@downloader).to receive(:version) do
-      if @knife.name_args.size == 2
-        @knife.name_args[1]
+    allow(knife).to receive(:shell_out!).and_return(true)
+
+    # CookbookSiteDownload Stup
+    allow(knife).to receive(:download_cookbook_to).and_return(downloader)
+    allow(downloader).to receive(:version) do
+      if knife.name_args.size == 2
+        knife.name_args[1]
       else
         "0.3.0"
       end
     end
 
-    #Stubs for CookbookSCMRepo
-    @repo = double(:sanity_check => true, :reset_to_default_state => true,
-                 :prepare_to_import => true, :finalize_updates_to => true,
-                 :merge_updates_from => true)
-    allow(Chef::Knife::CookbookSCMRepo).to receive(:new).and_return(@repo)
+    # Stubs for CookbookSCMRepo
+    allow(Chef::Knife::CookbookSCMRepo).to receive(:new).and_return(repo)
   end
 
-
   describe "run" do
-    it "should return an error if a cookbook name is not provided" do
-      @knife.name_args = []
-      expect(@knife.ui).to receive(:error).with("Please specify a cookbook to download and install.")
-      expect { @knife.run }.to raise_error(SystemExit)
-    end
-
-    it "should return an error if more than two arguments are given" do
-      @knife.name_args = ["foo", "bar", "baz"]
-      expect(@knife.ui).to receive(:error).with("Installing multiple cookbooks at once is not supported.")
-      expect { @knife.run }.to raise_error(SystemExit)
-    end
-
-    it "should return an error if the second argument is not a version" do
-      @knife.name_args = ["getting-started", "1pass"]
-      expect(@knife.ui).to receive(:error).with("Installing multiple cookbooks at once is not supported.")
-      expect { @knife.run }.to raise_error(SystemExit)
-    end
-
-    it "should return an error if the second argument is a four-digit version" do
-      @knife.name_args = ["getting-started", "0.0.0.1"]
-      expect(@knife.ui).to receive(:error).with("Installing multiple cookbooks at once is not supported.")
-      expect { @knife.run }.to raise_error(SystemExit)
-    end
-
-    it "should return an error if the second argument is a one-digit version" do
-      @knife.name_args = ["getting-started", "1"]
-      expect(@knife.ui).to receive(:error).with("Installing multiple cookbooks at once is not supported.")
-      expect { @knife.run }.to raise_error(SystemExit)
-    end
-
-    it "should install the specified version if second argument is a three-digit version" do
-      @knife.name_args = ["getting-started", "0.1.0"]
-      @knife.config[:no_deps] = true
-      upstream_file = File.join(@install_path, "getting-started.tar.gz")
-      expect(@knife).to receive(:download_cookbook_to).with(upstream_file)
-      expect(@knife).to receive(:extract_cookbook).with(upstream_file, "0.1.0")
-      expect(@knife).to receive(:clear_existing_files).with(File.join(@install_path, "getting-started"))
-      expect(@repo).to receive(:merge_updates_from).with("getting-started", "0.1.0")
-      @knife.run
-    end
-
-    it "should install the specified version if second argument is a two-digit version" do
-      @knife.name_args = ["getting-started", "0.1"]
-      @knife.config[:no_deps] = true
-      upstream_file = File.join(@install_path, "getting-started.tar.gz")
-      expect(@knife).to receive(:download_cookbook_to).with(upstream_file)
-      expect(@knife).to receive(:extract_cookbook).with(upstream_file, "0.1")
-      expect(@knife).to receive(:clear_existing_files).with(File.join(@install_path, "getting-started"))
-      expect(@repo).to receive(:merge_updates_from).with("getting-started", "0.1")
-      @knife.run
-    end
-
-    it "should install the latest version if only a cookbook name is given" do
-      @knife.name_args = ["getting-started"]
-      @knife.config[:no_deps] = true
-      upstream_file = File.join(@install_path, "getting-started.tar.gz")
-      expect(@knife).to receive(:download_cookbook_to).with(upstream_file)
-      expect(@knife).to receive(:extract_cookbook).with(upstream_file, "0.3.0")
-      expect(@knife).to receive(:clear_existing_files).with(File.join(@install_path, "getting-started"))
-      expect(@repo).to receive(:merge_updates_from).with("getting-started", "0.3.0")
-      @knife.run
-    end
-
-    it "should not create/reset git branches if use_current_branch is set" do
-      @knife.name_args = ["getting-started"]
-      @knife.config[:use_current_branch] = true
-      @knife.config[:no_deps] = true
-      upstream_file = File.join(@install_path, "getting-started.tar.gz")
-      expect(@repo).not_to receive(:prepare_to_import)
-      expect(@repo).not_to receive(:reset_to_default_state)
-      @knife.run
-    end
-
-    it "should not raise an error if cookbook_path is a string" do
-      @knife.config[:cookbook_path] = @install_path
-      @knife.config[:no_deps] = true
-      @knife.name_args = ["getting-started"]
-      upstream_file = File.join(@install_path, "getting-started.tar.gz")
-      expect(@knife).to receive(:download_cookbook_to).with(upstream_file)
-      expect(@knife).to receive(:extract_cookbook).with(upstream_file, "0.3.0")
-      expect(@knife).to receive(:clear_existing_files).with(File.join(@install_path, "getting-started"))
-      expect(@repo).to receive(:merge_updates_from).with("getting-started", "0.3.0")
-      expect { @knife.run }.not_to raise_error
+    it "raises an error if a cookbook name is not provided" do
+      knife.name_args = []
+      expect(knife.ui).to receive(:error).with("Please specify a cookbook to download and install.")
+      expect { knife.run }.to raise_error(SystemExit)
+    end
+
+    it "raises an error if more than two arguments are given" do
+      knife.name_args = ["foo", "bar", "baz"]
+      expect(knife.ui).to receive(:error).with("Installing multiple cookbooks at once is not supported.")
+      expect { knife.run }.to raise_error(SystemExit)
+    end
+
+    it "raises an error if the second argument is not a version" do
+      knife.name_args = ["getting-started", "1pass"]
+      expect(knife.ui).to receive(:error).with("Installing multiple cookbooks at once is not supported.")
+      expect { knife.run }.to raise_error(SystemExit)
+    end
+
+    it "raises an error if the second argument is a four-digit version" do
+      knife.name_args = ["getting-started", "0.0.0.1"]
+      expect(knife.ui).to receive(:error).with("Installing multiple cookbooks at once is not supported.")
+      expect { knife.run }.to raise_error(SystemExit)
+    end
+
+    it "raises an error if the second argument is a one-digit version" do
+      knife.name_args = ["getting-started", "1"]
+      expect(knife.ui).to receive(:error).with("Installing multiple cookbooks at once is not supported.")
+      expect { knife.run }.to raise_error(SystemExit)
+    end
+
+    it "installs the specified version if second argument is a three-digit version" do
+      knife.name_args = ["getting-started", "0.1.0"]
+      knife.config[:no_deps] = true
+      upstream_file = File.join(install_path, "getting-started.tar.gz")
+      expect(knife).to receive(:download_cookbook_to).with(upstream_file)
+      expect(knife).to receive(:extract_cookbook).with(upstream_file, "0.1.0")
+      expect(knife).to receive(:clear_existing_files).with(File.join(install_path, "getting-started"))
+      expect(repo).to receive(:merge_updates_from).with("getting-started", "0.1.0")
+      knife.run
+    end
+
+    it "installs the specified version if second argument is a two-digit version" do
+      knife.name_args = ["getting-started", "0.1"]
+      knife.config[:no_deps] = true
+      upstream_file = File.join(install_path, "getting-started.tar.gz")
+      expect(knife).to receive(:download_cookbook_to).with(upstream_file)
+      expect(knife).to receive(:extract_cookbook).with(upstream_file, "0.1")
+      expect(knife).to receive(:clear_existing_files).with(File.join(install_path, "getting-started"))
+      expect(repo).to receive(:merge_updates_from).with("getting-started", "0.1")
+      knife.run
+    end
+
+    it "installs the latest version if only a cookbook name is given" do
+      knife.name_args = ["getting-started"]
+      knife.config[:no_deps] = true
+      upstream_file = File.join(install_path, "getting-started.tar.gz")
+      expect(knife).to receive(:download_cookbook_to).with(upstream_file)
+      expect(knife).to receive(:extract_cookbook).with(upstream_file, "0.3.0")
+      expect(knife).to receive(:clear_existing_files).with(File.join(install_path, "getting-started"))
+      expect(repo).to receive(:merge_updates_from).with("getting-started", "0.3.0")
+      knife.run
+    end
+
+    it "does not create/reset git branches if use_current_branch is set" do
+      knife.name_args = ["getting-started"]
+      knife.config[:use_current_branch] = true
+      knife.config[:no_deps] = true
+      upstream_file = File.join(install_path, "getting-started.tar.gz")
+      expect(repo).not_to receive(:prepare_to_import)
+      expect(repo).not_to receive(:reset_to_default_state)
+      knife.run
     end
+
+    it "does not raise an error if cookbook_path is a string" do
+      knife.config[:cookbook_path] = install_path
+      knife.config[:no_deps] = true
+      knife.name_args = ["getting-started"]
+      upstream_file = File.join(install_path, "getting-started.tar.gz")
+      expect(knife).to receive(:download_cookbook_to).with(upstream_file)
+      expect(knife).to receive(:extract_cookbook).with(upstream_file, "0.3.0")
+      expect(knife).to receive(:clear_existing_files).with(File.join(install_path, "getting-started"))
+      expect(repo).to receive(:merge_updates_from).with("getting-started", "0.3.0")
+      expect { knife.run }.not_to raise_error
+    end
+  end # end of run
+
+  let(:metadata) { Chef::Cookbook::Metadata.new }
+  let(:rb_metadata_path) { File.join(install_path, "post-punk-kitchen", "metadata.rb") }
+  let(:json_metadata_path) { File.join(install_path, "post-punk-kitchen", "metadata.json") }
+
+  describe "preferred_metadata" do
+    before do
+      allow(Chef::Cookbook::Metadata).to receive(:new).and_return(metadata)
+      allow(File).to receive(:exist?).and_return(false)
+      knife.instance_variable_set(:@cookbook_name, "post-punk-kitchen")
+      knife.instance_variable_set(:@install_path, install_path)
+    end
+
+    it "returns a populated Metadata object if metadata.rb exists" do
+      allow(File).to receive(:exist?).with(rb_metadata_path).and_return(true)
+      expect(metadata).to receive(:from_file).with(rb_metadata_path)
+      knife.preferred_metadata
+    end
+
+    it "returns a populated Metadata object if metadata.json exists" do
+      allow(File).to receive(:exist?).with(json_metadata_path).and_return(true)
+      #expect(IO).to receive(:read).with(json_metadata_path)
+      allow(IO).to receive(:read)
+      expect(metadata).to receive(:from_json)
+      knife.preferred_metadata
+    end
+
+    it "prefers metadata.rb over metadata.json" do
+      allow(File).to receive(:exist?).with(rb_metadata_path).and_return(true)
+      allow(File).to receive(:exist?).with(json_metadata_path).and_return(true)
+      allow(IO).to receive(:read)
+      expect(metadata).to receive(:from_file).with(rb_metadata_path)
+      expect(metadata).not_to receive(:from_json)
+      knife.preferred_metadata
+    end
+
+    it "rasies an error if it finds no metadata file" do
+      expect { knife.preferred_metadata }.to raise_error(Chef::Exceptions::MetadataNotFound)
+    end
+
   end
 end