diff options
6 files changed, 106 insertions, 10 deletions
diff --git a/lib/chef/cookbook/cookbook_version_loader.rb b/lib/chef/cookbook/cookbook_version_loader.rb index 745a5279ef..02bc4fb0db 100644 --- a/lib/chef/cookbook/cookbook_version_loader.rb +++ b/lib/chef/cookbook/cookbook_version_loader.rb @@ -29,6 +29,8 @@ class Chef # The cookbook's name as inferred from its directory. attr_reader :inferred_cookbook_name + attr_reader :metadata_error + def initialize(path, chefignore=nil) @cookbook_path = File.expand_path( path ) # cookbook_path from which this was loaded # We keep a list of all cookbook paths that have been merged in @@ -53,8 +55,11 @@ class Chef } @metadata_filenames = [] + @metadata_error = nil end + # Load the cookbook. Raises an error if the cookbook_path given to the + # constructor doesn't point to a valid cookbook. def load! file_paths_map = load @@ -64,7 +69,15 @@ class Chef file_paths_map end + # Load the cookbook. Does not raise an error if given a non-cookbook + # directory as the cookbook_path. This behavior is provided for + # compatibility, it is recommended to use #load! instead. def load + metadata # force lazy evaluation to occur + + # re-raise any exception that occurred when reading the metadata + raise_metadata_error! + load_as(:attribute_filenames, 'attributes', '*.rb') load_as(:definition_filenames, 'definitions', '*.rb') load_as(:recipe_filenames, 'recipes', '*.rb') @@ -134,6 +147,12 @@ 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$/ @@ -147,13 +166,21 @@ class Chef end end - # Compatibility if metadata is missing the name attribute: - # TODO: probably should live elsewhere. - if @metadata.name.nil? - @metadata.name(@inferred_cookbook_name) - end @metadata + + # Rescue errors so that users can upload cookbooks via `knife cookbook + # upload` even if some cookbooks in their chef-repo have errors in + # their metadata. We only rescue StandardError because you have to be + # doing something *really* terrible to raise an exception that inherits + # directly from Exception in your metadata.rb file. + rescue StandardError => e + @metadata_error = e + @metadata + end + + def raise_metadata_error! + raise @metadata_error unless @metadata_error.nil? end def empty? diff --git a/spec/data/invalid-metadata-chef-repo/invalid-metadata/README.md b/spec/data/invalid-metadata-chef-repo/invalid-metadata/README.md new file mode 100644 index 0000000000..a41867f17c --- /dev/null +++ b/spec/data/invalid-metadata-chef-repo/invalid-metadata/README.md @@ -0,0 +1,4 @@ +# invalid-metadata + +TODO: Enter the cookbook description here. + diff --git a/spec/data/invalid-metadata-chef-repo/invalid-metadata/metadata.rb b/spec/data/invalid-metadata-chef-repo/invalid-metadata/metadata.rb new file mode 100644 index 0000000000..f548ad1dd8 --- /dev/null +++ b/spec/data/invalid-metadata-chef-repo/invalid-metadata/metadata.rb @@ -0,0 +1,10 @@ +raise "THIS METADATA HAS A BUG" + +name 'invalid-metadata' +maintainer '' +maintainer_email '' +license '' +description 'Installs/Configures invalid-metadata' +long_description 'Installs/Configures invalid-metadata' +version '0.1.0' + diff --git a/spec/data/invalid-metadata-chef-repo/invalid-metadata/recipes/default.rb b/spec/data/invalid-metadata-chef-repo/invalid-metadata/recipes/default.rb new file mode 100644 index 0000000000..1411b9e39f --- /dev/null +++ b/spec/data/invalid-metadata-chef-repo/invalid-metadata/recipes/default.rb @@ -0,0 +1,8 @@ +# +# Cookbook Name:: invalid-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 9528eea198..c8292de762 100644 --- a/spec/unit/cookbook/cookbook_version_loader_spec.rb +++ b/spec/unit/cookbook/cookbook_version_loader_spec.rb @@ -110,16 +110,28 @@ describe Chef::Cookbook::CookbookVersionLoader do context "when a cookbook has an invalid metadata file [CHEF-2923]" do + let(:cookbook_path) { File.join(CHEF_SPEC_DATA, "invalid-metadata-chef-repo/invalid-metadata") } + it "raises an error when loading with #load!" do - pending + expect { cookbook_loader.load! }.to raise_error("THIS METADATA HAS A BUG") end - it "skips the cookbook when called with #load" do - pending + it "raises an error when called with #load" do + expect { cookbook_loader.load }.to raise_error("THIS METADATA HAS A BUG") + end + + it "doesn't raise an error when determining the cookbook name" do + expect { cookbook_loader.cookbook_name }.to_not raise_error + end + + it "doesn't raise an error when metadata is first generated" do + expect { cookbook_loader.metadata }.to_not raise_error end it "sets an error flag containing error information" do - pending + cookbook_loader.metadata + expect(cookbook_loader.metadata_error).to be_a(StandardError) + expect(cookbook_loader.metadata_error.message).to eq("THIS METADATA HAS A BUG") end end diff --git a/spec/unit/cookbook_loader_spec.rb b/spec/unit/cookbook_loader_spec.rb index 8c1a0886c2..d5d585b8e1 100644 --- a/spec/unit/cookbook_loader_spec.rb +++ b/spec/unit/cookbook_loader_spec.rb @@ -167,10 +167,26 @@ describe Chef::CookbookLoader do cookbook_loader.metadata[:openldap].should be_a_kind_of(Chef::Cookbook::Metadata) end - end # after loading cookbooks + end # referencing cookbook files end # loading all cookbooks + context "loading all cookbooks when one has invalid metadata" do + + let(:repo_paths) do + [ + File.join(CHEF_SPEC_DATA, "kitchen"), + File.join(CHEF_SPEC_DATA, "cookbooks"), + File.join(CHEF_SPEC_DATA, "invalid-metadata-chef-repo") + ] + end + + it "does not squelch the exception" do + expect { cookbook_loader.load_cookbooks }.to raise_error("THIS METADATA HAS A BUG") + end + + end + describe "loading only one cookbook" do before(:each) do cookbook_loader.load_cookbook("openldap") @@ -217,6 +233,25 @@ describe Chef::CookbookLoader do cookbook_loader["apache2"].should be_a_kind_of(Chef::CookbookVersion) end + context "when an unrelated cookbook has invalid metadata" do + + let(:repo_paths) do + [ + File.join(CHEF_SPEC_DATA, "kitchen"), + File.join(CHEF_SPEC_DATA, "cookbooks"), + File.join(CHEF_SPEC_DATA, "invalid-metadata-chef-repo") + ] + end + + it "ignores the invalid cookbook" do + expect { cookbook_loader["openldap"] }.to_not raise_error + end + + it "surfaces the exception if the cookbook is loaded later" do + expect { cookbook_loader["invalid-metadata"] }.to raise_error("THIS METADATA HAS A BUG") + end + + end describe "loading all cookbooks after loading only one cookbook" do before(:each) do |