Merge pull request #6045 from coderanger/segment-oops

Fix for Chef 13 upload reversion.

3 files changed, 211 insertions, 3 deletions

diff --git a/lib/chef/cookbook/manifest_v0.rb b/lib/chef/cookbook/manifest_v0.rb
index 3e50d86071..fd2d62a6d4 100644
--- a/lib/chef/cookbook/manifest_v0.rb
+++ b/lib/chef/cookbook/manifest_v0.rb
@@ -27,13 +27,14 @@ class Chef
       COOKBOOK_SEGMENTS = %w{ resources providers recipes definitions libraries attributes files templates root_files }
 
       def self.from_hash(hash)
-        response = Mash.new
+        response = Mash.new(hash)
         response[:all_files] = COOKBOOK_SEGMENTS.inject([]) do |memo, segment|
           next memo if hash[segment].nil? || hash[segment].empty?
           hash[segment].each do |file|
             file["name"] = "#{segment}/#{file["name"]}" unless segment == "root_files"
             memo << file
           end
+          response.delete(segment)
           memo
         end
         response
@@ -44,7 +45,7 @@ class Chef
         result.delete("all_files")
 
         files = manifest.by_parent_directory
-        files.keys.inject(result) do |memo, parent|
+        files.keys.each_with_object(result) do |parent, memo|
           if COOKBOOK_SEGMENTS.include?(parent)
             memo[parent] ||= []
             files[parent].each do |file|
@@ -53,7 +54,11 @@ class Chef
               memo[parent] << file
             end
           end
-          memo
+        end
+        # Ensure all segments are set to [] if they don't exist.
+        # See https://github.com/chef/chef/issues/6044
+        COOKBOOK_SEGMENTS.each do |segment|
+          result[segment] ||= []
         end
 
         result.merge({ "frozen?" => manifest.frozen_version?, "chef_type" => "cookbook_version" })
diff --git a/spec/unit/cookbook/manifest_v0_spec.rb b/spec/unit/cookbook/manifest_v0_spec.rb
new file mode 100644
index 0000000000..0f5cfbe7a4
--- /dev/null
+++ b/spec/unit/cookbook/manifest_v0_spec.rb
@@ -0,0 +1,133 @@
+#
+# Copyright:: Copyright 2017, Chef Software Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+require "spec_helper"
+require "chef/cookbook_manifest"
+require "chef/digester"
+require "pathname"
+
+describe Chef::Cookbook::ManifestV0 do
+  let(:version) { "1.2.3" }
+
+  let(:identifier) { "9e10455ce2b4a4e29424b7064b1d67a1a25c9d3b" }
+
+  let(:metadata) do
+    Chef::Cookbook::Metadata.new.tap do |m|
+      m.version(version)
+    end
+  end
+
+  let(:cookbook_root) { "/tmp/blah" }
+
+  let(:cookbook_version) do
+    Chef::CookbookVersion.new("tatft", cookbook_root).tap do |c|
+      c.metadata = metadata
+      c.identifier = identifier
+    end
+  end
+
+  let(:cookbook_manifest) { Chef::CookbookManifest.new(cookbook_version) }
+
+  let(:cookbook_root) { File.join(CHEF_SPEC_DATA, "cb_version_cookbooks", "tatft") }
+
+  let(:all_files) { Dir[File.join(cookbook_root, "**", "**")].reject { |f| File.directory? f } }
+
+  let(:expected_hash) do
+    {
+      "attributes" => [{ "name" => "default.rb", "path" => "attributes/default.rb", "checksum" => "a88697db56181498a8828d5531271ad9", "specificity" => "default" }],
+      "chef_type" => "cookbook_version",
+      "cookbook_name" => "tatft",
+      "definitions" => [{ "name" => "runit_service.rb", "path" => "definitions/runit_service.rb", "checksum" => "c40cf9b4c6eb15a8e49e31602f701161", "specificity" => "default" }],
+      "files" => [{ "name" => "giant_blob.tgz", "path" => "files/default/giant_blob.tgz", "checksum" => "5b4b194bb80938bb18da7af5c823cb1b", "specificity" => "default" }],
+      "frozen?" => false,
+      "libraries" => [{ "name" => "ownage.rb", "path" => "libraries/ownage.rb", "checksum" => "4686edd9968909034692e09e058d90d9", "specificity" => "default" }],
+      "name" => "tatft-1.2.3",
+      "providers" => [{ "name" => "lwp.rb", "path" => "providers/lwp.rb", "checksum" => "bc189d68f77bb054d1070aeff7669557", "specificity" => "default" }],
+      "recipes" => [{ "name" => "default.rb", "path" => "recipes/default.rb", "checksum" => "09bc749f00c68717d288de9c8d7c644f", "specificity" => "default" }],
+      "resources" => [{ "name" => "lwr.rb", "path" => "resources/lwr.rb", "checksum" => "609c40d3d3f269e7edf230277a240ef5", "specificity" => "default" }],
+      "root_files" => [{ "name" => "README.rdoc", "path" => "README.rdoc", "checksum" => "cd7be9a1b9b1f33e3bcd9c3f4bc8dde5", "specificity" => "default" }],
+      "templates" => [{ "name" => "configuration.erb", "path" => "templates/default/configuration.erb", "checksum" => "d41d8cd98f00b204e9800998ecf8427e", "specificity" => "default" }],
+      "version" => "1.2.3",
+    }
+  end
+
+  describe "#from_hash" do
+    let(:source_hash) do
+      {
+      "attributes" => [{ "name" => "default.rb", "path" => "attributes/default.rb", "checksum" => "a88697db56181498a8828d5531271ad9", "specificity" => "default" }],
+      "recipes" => [{ "name" => "default.rb", "path" => "recipes/default.rb", "checksum" => "09bc749f00c68717d288de9c8d7c644f", "specificity" => "default" }],
+      "root_files" => [{ "name" => "README.rdoc", "path" => "README.rdoc", "checksum" => "cd7be9a1b9b1f33e3bcd9c3f4bc8dde5", "specificity" => "default" }],
+      "name" => "tatft-1.2.3",
+      "version" => "1.2.3",
+      }
+    end
+
+    it "preserves the version" do
+      result = described_class.from_hash(source_hash)
+      expect(result["version"]).to eq "1.2.3"
+    end
+
+    it "creates an all_files key and populates it" do
+      result = described_class.from_hash(source_hash)
+      expect(result[:all_files].map { |f| f["name"] }).to match_array %w{ recipes/default.rb attributes/default.rb README.rdoc }
+    end
+
+    it "deletes unwanted segment types" do
+      result = described_class.from_hash(source_hash)
+      expect(result["attributes"]).to be_nil
+    end
+
+    it "preserves frozeness" do
+      source_hash["frozen?"] = true
+      result = described_class.from_hash(source_hash)
+      expect(result["frozen?"]).to be true
+    end
+  end
+
+  describe "#to_hash" do
+    it "accepts a cookbook manifest" do
+      result = described_class.to_hash(cookbook_manifest)
+      expect(result).to be_a(Hash)
+    end
+
+    it "preserves frozeness" do
+      cookbook_version.freeze_version
+      expect(described_class.to_hash(cookbook_manifest)["frozen?"]).to be true
+    end
+  end
+
+  context "ensures that all segments exist" do
+    Chef::Cookbook::ManifestV0::COOKBOOK_SEGMENTS.each do |segment|
+      it "with #{segment}" do
+        result = described_class.to_hash(cookbook_manifest)
+        expect(result[segment]).to be_empty
+      end
+    end
+  end
+
+  context "when given a cookbook with some files" do
+    before do
+      cookbook_version.all_files = all_files
+    end
+
+    Chef::Cookbook::ManifestV0::COOKBOOK_SEGMENTS.each do |segment|
+      it "places the files for #{segment} correctly" do
+        result = described_class.to_hash(cookbook_manifest)
+        expect(result[segment]).to eq(expected_hash[segment])
+      end
+    end
+  end
+end
diff --git a/spec/unit/cookbook/manifest_v2_spec.rb b/spec/unit/cookbook/manifest_v2_spec.rb
new file mode 100644
index 0000000000..23df950f4a
--- /dev/null
+++ b/spec/unit/cookbook/manifest_v2_spec.rb
@@ -0,0 +1,70 @@
+#
+# Copyright:: Copyright 2017, Chef Software Inc.
+# License:: Apache License, Version 2.0
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+require "spec_helper"
+require "chef/cookbook_manifest"
+require "chef/digester"
+require "pathname"
+
+describe Chef::Cookbook::ManifestV2 do
+  let(:version) { "1.2.3" }
+
+  let(:identifier) { "9e10455ce2b4a4e29424b7064b1d67a1a25c9d3b" }
+
+  let(:metadata) do
+    Chef::Cookbook::Metadata.new.tap do |m|
+      m.version(version)
+    end
+  end
+
+  let(:cookbook_root) { "/tmp/blah" }
+
+  let(:cookbook_version) do
+    Chef::CookbookVersion.new("tatft", cookbook_root).tap do |c|
+      c.metadata = metadata
+      c.identifier = identifier
+    end
+  end
+
+  let(:cookbook_manifest) { Chef::CookbookManifest.new(cookbook_version) }
+
+  let(:cookbook_root) { File.join(CHEF_SPEC_DATA, "cb_version_cookbooks", "tatft") }
+
+  let(:all_files) { Dir[File.join(cookbook_root, "**", "**")].reject { |f| File.directory? f } }
+
+  describe "#to_hash" do
+    it "accepts a cookbook manifest" do
+      result = described_class.to_hash(cookbook_manifest)
+      expect(result).to be_a(Hash)
+    end
+
+    it "preserves frozeness" do
+      cookbook_version.freeze_version
+      expect(described_class.to_hash(cookbook_manifest)["frozen?"]).to be true
+    end
+  end
+
+  context "when given a cookbook with some files" do
+    before do
+      cookbook_version.all_files = all_files
+    end
+
+    it "populates all_files correctly" do
+      result = described_class.to_hash(cookbook_manifest)
+      expect(result["all_files"][0]).not_to include(:full_path)
+    end
+  end
+end