Make `name` a required attribute in metadata

14 files changed, 89 insertions, 10 deletions

diff --git a/lib/chef/cookbook/cookbook_version_loader.rb b/lib/chef/cookbook/cookbook_version_loader.rb
index 02bc4fb0db..6c85cdef54 100644
--- a/lib/chef/cookbook/cookbook_version_loader.rb
+++ b/lib/chef/cookbook/cookbook_version_loader.rb
@@ -147,12 +147,6 @@ class Chef
 
         @metadata = Chef::Cookbook::Metadata.new
 
-        # Compatibility if metadata is missing the name attribute:
-        # TODO: Metadata should distinguish between inferred name and actual.
-        if @metadata.name.nil?
-          @metadata.name(@inferred_cookbook_name)
-        end
-
         metadata_filenames.each do |metadata_file|
           case metadata_file
           when /\.rb$/
@@ -166,7 +160,6 @@ class Chef
           end
         end
 
-
         @metadata
 
         # Rescue errors so that users can upload cookbooks via `knife cookbook
@@ -181,6 +174,14 @@ class Chef
 
       def raise_metadata_error!
         raise @metadata_error unless @metadata_error.nil?
+        # Metadata won't be valid if the cookbook is empty. If the cookbook is
+        # actually empty, a metadata error here would be misleading, so don't
+        # raise it (if called by #load!, a different error is raised).
+        if !empty? && !metadata.valid?
+          message = "Cookbook loaded at path(s) [#{@cookbook_paths.join(', ')}] has invalid metadata: #{metadata.errors.join('; ')}"
+          raise Exceptions::MetadataNotValid, message
+        end
+        false
       end
 
       def empty?
diff --git a/lib/chef/cookbook/metadata.rb b/lib/chef/cookbook/metadata.rb
index c1fe20e748..3964354d50 100644
--- a/lib/chef/cookbook/metadata.rb
+++ b/lib/chef/cookbook/metadata.rb
@@ -18,6 +18,7 @@
 # limitations under the License.
 #
 
+require 'chef/exceptions'
 require 'chef/mash'
 require 'chef/mixin/from_file'
 require 'chef/mixin/params_validate'
diff --git a/lib/chef/exceptions.rb b/lib/chef/exceptions.rb
index bd6d887884..194c758f37 100644
--- a/lib/chef/exceptions.rb
+++ b/lib/chef/exceptions.rb
@@ -132,6 +132,8 @@ class Chef
     # Version constraints are not allowed in chef-solo
     class IllegalVersionConstraint < NotImplementedError; end
 
+    class MetadataNotValid < StandardError; end
+
     # File operation attempted but no permissions to perform it
     class InsufficientPermissions < RuntimeError; end
 
diff --git a/spec/data/cookbooks/angrybash/metadata.rb b/spec/data/cookbooks/angrybash/metadata.rb
new file mode 100644
index 0000000000..720b8085be
--- /dev/null
+++ b/spec/data/cookbooks/angrybash/metadata.rb
@@ -0,0 +1,2 @@
+name "angrybash"
+version "1.0.0"
diff --git a/spec/data/cookbooks/apache2/metadata.rb b/spec/data/cookbooks/apache2/metadata.rb
new file mode 100644
index 0000000000..1273d20d79
--- /dev/null
+++ b/spec/data/cookbooks/apache2/metadata.rb
@@ -0,0 +1,2 @@
+name "apache2"
+version "0.0.1"
diff --git a/spec/data/cookbooks/borken/metadata.rb b/spec/data/cookbooks/borken/metadata.rb
new file mode 100644
index 0000000000..58700b2d8e
--- /dev/null
+++ b/spec/data/cookbooks/borken/metadata.rb
@@ -0,0 +1,2 @@
+name "borken"
+version "1.0.0"
diff --git a/spec/data/cookbooks/ignorken/metadata.rb b/spec/data/cookbooks/ignorken/metadata.rb
new file mode 100644
index 0000000000..f91d92de32
--- /dev/null
+++ b/spec/data/cookbooks/ignorken/metadata.rb
@@ -0,0 +1,2 @@
+name "ignorken"
+version "1.0.0"
diff --git a/spec/data/cookbooks/java/metadata.rb b/spec/data/cookbooks/java/metadata.rb
new file mode 100644
index 0000000000..7c5585304c
--- /dev/null
+++ b/spec/data/cookbooks/java/metadata.rb
@@ -0,0 +1,2 @@
+name "java"
+version "0.0.1"
diff --git a/spec/data/cookbooks/preseed/metadata.rb b/spec/data/cookbooks/preseed/metadata.rb
new file mode 100644
index 0000000000..a48deec08b
--- /dev/null
+++ b/spec/data/cookbooks/preseed/metadata.rb
@@ -0,0 +1,2 @@
+name "preseed"
+version "1.0.0"
diff --git a/spec/data/incomplete-metadata-chef-repo/incomplete-metadata/README.md b/spec/data/incomplete-metadata-chef-repo/incomplete-metadata/README.md
new file mode 100644
index 0000000000..3ac2a4f90c
--- /dev/null
+++ b/spec/data/incomplete-metadata-chef-repo/incomplete-metadata/README.md
@@ -0,0 +1,4 @@
+# incomplete-metadata
+
+TODO: Enter the cookbook description here.
+
diff --git a/spec/data/incomplete-metadata-chef-repo/incomplete-metadata/metadata.rb b/spec/data/incomplete-metadata-chef-repo/incomplete-metadata/metadata.rb
new file mode 100644
index 0000000000..50284be3dd
--- /dev/null
+++ b/spec/data/incomplete-metadata-chef-repo/incomplete-metadata/metadata.rb
@@ -0,0 +1,13 @@
+# ## INCOMPLETE METADATA ##
+# This cookbook is invalid b/c it does not set the `name' of the cookbook.
+
+# Commented out for illustrative purposes
+# name             'incomplete-metadata'
+
+maintainer       ''
+maintainer_email ''
+license          ''
+description      'Installs/Configures incomplete-metadata'
+long_description 'Installs/Configures incomplete-metadata'
+version          '0.1.0'
+
diff --git a/spec/data/incomplete-metadata-chef-repo/incomplete-metadata/recipes/default.rb b/spec/data/incomplete-metadata-chef-repo/incomplete-metadata/recipes/default.rb
new file mode 100644
index 0000000000..65ae049ce8
--- /dev/null
+++ b/spec/data/incomplete-metadata-chef-repo/incomplete-metadata/recipes/default.rb
@@ -0,0 +1,8 @@
+#
+# Cookbook Name:: incomplete-metadata
+# Recipe:: default
+#
+# Copyright (C) 2014 
+#
+# 
+#
diff --git a/spec/unit/cookbook/cookbook_version_loader_spec.rb b/spec/unit/cookbook/cookbook_version_loader_spec.rb
index c8292de762..ad10f24573 100644
--- a/spec/unit/cookbook/cookbook_version_loader_spec.rb
+++ b/spec/unit/cookbook/cookbook_version_loader_spec.rb
@@ -108,7 +108,7 @@ describe Chef::Cookbook::CookbookVersionLoader do
 
     end
 
-    context "when a cookbook has an invalid metadata file [CHEF-2923]" do
+    context "when a cookbook has a metadata file with a ruby error [CHEF-2923]" do
 
       let(:cookbook_path) { File.join(CHEF_SPEC_DATA, "invalid-metadata-chef-repo/invalid-metadata") }
 
@@ -136,6 +136,43 @@ describe Chef::Cookbook::CookbookVersionLoader do
 
     end
 
+    context "when a cookbook has a metadata file with invalid metadata" do
+
+      let(:cookbook_path) { File.join(CHEF_SPEC_DATA, "incomplete-metadata-chef-repo/incomplete-metadata") }
+
+      let(:error_message) do
+        "Cookbook loaded at path(s) [#{cookbook_path}] has invalid metadata: The `name' attribute is required in cookbook metadata"
+      end
+
+      it "raises an error when loading with #load!" do
+        expect { cookbook_loader.load! }.to raise_error(Chef::Exceptions::MetadataNotValid, error_message)
+      end
+
+      it "raises an error when called with #load" do
+        expect { cookbook_loader.load }.to raise_error(Chef::Exceptions::MetadataNotValid, error_message)
+      end
+
+      it "uses the inferred cookbook name [CHEF-2923]" do
+        # This behavior is intended to support the CHEF-2923 feature where
+        # invalid metadata doesn't prevent you from uploading other cookbooks.
+        #
+        # The metadata is the definitive source of the cookbook name, but if
+        # the metadata is incomplete/invalid, we can't read the name from it.
+        #
+        # The CookbookLoader stores CookbookVersionLoaders in a Hash with
+        # cookbook names as the keys, and finds the loader in this Hash to call
+        # #load on it when the user runs a command like `knife cookbook upload specific-cookbook`
+        #
+        # Most of the time this will work, but if the user tries to upload a
+        # specific cookbook by name, has customized that cookbook's name (so it
+        # doesn't match the inferred name), and that metadata file has a syntax
+        # error, we might report a "cookbook not found" error instead of the
+        # metadata syntax error that is the actual cause.
+        expect(cookbook_loader.cookbook_name).to eq(:"incomplete-metadata")
+      end
+
+    end
+
   end
 
 end
diff --git a/spec/unit/cookbook/syntax_check_spec.rb b/spec/unit/cookbook/syntax_check_spec.rb
index 7f7a914115..b83bffe1c9 100644
--- a/spec/unit/cookbook/syntax_check_spec.rb
+++ b/spec/unit/cookbook/syntax_check_spec.rb
@@ -147,7 +147,8 @@ describe Chef::Cookbook::SyntaxCheck do
 
         it "does not remove the invalid file from the list of untested files" do
           syntax_check.untested_ruby_files.should include(File.join(cookbook_path, 'recipes', 'default.rb'))
-          lambda { syntax_check.validate_ruby_files }.should_not change(syntax_check, :untested_ruby_files)
+          syntax_check.validate_ruby_files
+          syntax_check.untested_ruby_files.should include(File.join(cookbook_path, 'recipes', 'default.rb'))
         end
 
         it "indicates that a template file has a syntax error" do
@@ -166,7 +167,7 @@ describe Chef::Cookbook::SyntaxCheck do
           cookbook_path = File.join(CHEF_SPEC_DATA, 'cookbooks', 'ignorken')
           Chef::Config[:cookbook_path] = File.dirname(cookbook_path)
           syntax_check.cookbook_path.replace(cookbook_path)
-          @ruby_files = [File.join(cookbook_path, 'recipes/default.rb')]
+          @ruby_files = [File.join(cookbook_path, 'metadata.rb'), File.join(cookbook_path, 'recipes/default.rb')]
         end
 
         it "shows that ignored ruby files do not require a syntax check" do