diff options
author | Lamont Granquist <lamont@chef.io> | 2020-01-21 21:43:17 -0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-01-21 21:43:17 -0800 |
commit | b99534fe0de8d9188455d8391a686e601a72bf20 (patch) | |
tree | d004d4e58dd6c90b3cf52c51cabdb77e2920d903 | |
parent | 67a19dfb20c787a84a905859f3e6e76faa106bab (diff) | |
parent | d0d16ae5ed3e3b017df147656388840553797925 (diff) | |
download | chef-b99534fe0de8d9188455d8391a686e601a72bf20.tar.gz |
Raises error if there is empty cookbook directory at the time o… (#9011)
Raises error if there is empty cookbook directory at the time of cookbook upload.
-rw-r--r-- | lib/chef/cookbook/cookbook_version_loader.rb | 18 | ||||
-rw-r--r-- | lib/chef/cookbook_loader.rb | 4 | ||||
-rw-r--r-- | lib/chef/cookbook_uploader.rb | 2 | ||||
-rw-r--r-- | lib/chef/knife/cookbook_upload.rb | 4 | ||||
-rw-r--r-- | spec/integration/knife/chef_fs_data_store_spec.rb | 5 | ||||
-rw-r--r-- | spec/integration/knife/deps_spec.rb | 11 | ||||
-rw-r--r-- | spec/integration/knife/upload_spec.rb | 27 | ||||
-rw-r--r-- | spec/unit/cookbook/cookbook_version_loader_spec.rb | 11 |
8 files changed, 67 insertions, 15 deletions
diff --git a/lib/chef/cookbook/cookbook_version_loader.rb b/lib/chef/cookbook/cookbook_version_loader.rb index f9ec765e47..7e09fae60c 100644 --- a/lib/chef/cookbook/cookbook_version_loader.rb +++ b/lib/chef/cookbook/cookbook_version_loader.rb @@ -65,19 +65,24 @@ class Chef # 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 + metadata # force lazy evaluation to occur + + # re-raise any exception that occurred when reading the metadata + raise_metadata_error! + + load_all_files + + remove_ignored_files if empty? raise Exceptions::CookbookNotFoundInRepo, "The directory #{cookbook_path} does not contain a cookbook" end - file_paths_map + cookbook_settings 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 + Chef::Log.warn "load method is deprecated. Use load! instead" metadata # force lazy evaluation to occur # re-raise any exception that occurred when reading the metadata @@ -88,8 +93,9 @@ class Chef remove_ignored_files if empty? - Chef::Log.warn "Found a directory #{cookbook_name} in the cookbook path, but it contains no cookbook files. skipping." + raise Exceptions::CookbookNotFoundInRepo, "The directory #{cookbook_path} does not contain a cookbook" end + cookbook_settings end diff --git a/lib/chef/cookbook_loader.rb b/lib/chef/cookbook_loader.rb index bdb436e899..f02b07eaaf 100644 --- a/lib/chef/cookbook_loader.rb +++ b/lib/chef/cookbook_loader.rb @@ -95,11 +95,11 @@ class Chef loader = cookbook_version_loaders[cookbook_name] - loader.load + loader.load! cookbook_version = loader.cookbook_version cookbooks_by_name[cookbook_name] = cookbook_version - metadata[cookbook_name] = cookbook_version.metadata + metadata[cookbook_name] = cookbook_version.metadata unless cookbook_version.nil? cookbook_version end diff --git a/lib/chef/cookbook_uploader.rb b/lib/chef/cookbook_uploader.rb index ffc4040194..b9d4bfe5d1 100644 --- a/lib/chef/cookbook_uploader.rb +++ b/lib/chef/cookbook_uploader.rb @@ -139,6 +139,8 @@ class Chef def validate_cookbooks cookbooks.each do |cb| + next if cb.nil? + syntax_checker = Chef::Cookbook::SyntaxCheck.new(cb.root_dir) Chef::Log.info("Validating ruby files") exit(1) unless syntax_checker.validate_ruby_files diff --git a/lib/chef/knife/cookbook_upload.rb b/lib/chef/knife/cookbook_upload.rb index c8c9067800..7bda2815e7 100644 --- a/lib/chef/knife/cookbook_upload.rb +++ b/lib/chef/knife/cookbook_upload.rb @@ -124,7 +124,6 @@ class Chef cookbooks_for_upload << cookbook version_constraints_to_update[cookbook_name] = cookbook.version end - if config[:all] if cookbooks_for_upload.any? begin @@ -168,7 +167,6 @@ class Chef exit 1 end end - unless version_constraints_to_update.empty? update_version_constraints(version_constraints_to_update) if config[:environment] end @@ -193,7 +191,7 @@ class Chef end end rescue Exceptions::CookbookNotFoundInRepo => e - ui.error("Could not find cookbook #{cookbook_name} in your cookbook path, skipping it") + ui.error(e.message) Log.debug(e) end end diff --git a/spec/integration/knife/chef_fs_data_store_spec.rb b/spec/integration/knife/chef_fs_data_store_spec.rb index 95fee18257..a2d1a7ecec 100644 --- a/spec/integration/knife/chef_fs_data_store_spec.rb +++ b/spec/integration/knife/chef_fs_data_store_spec.rb @@ -54,6 +54,7 @@ describe "ChefFSDataStore tests", :workstation do context "GET /TYPE" do it "knife list -z -R returns everything" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).twice knife("list -z -Rfp /").should_succeed <<~EOM /acls/ /acls/clients/ @@ -118,6 +119,7 @@ describe "ChefFSDataStore tests", :workstation do end it "knife delete -z -r /cookbooks/x works" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).at_least(3).times knife("delete -z -r /cookbooks/x").should_succeed "Deleted /cookbooks/x\n" knife("list -z -Rfp /cookbooks").should_succeed "" end @@ -155,6 +157,7 @@ describe "ChefFSDataStore tests", :workstation do end it "knife show -z /cookbooks/x/metadata.rb works" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).once knife("show -z /cookbooks/x/metadata.rb").should_succeed "/cookbooks/x/metadata.rb:\n#{cookbook_x_100_metadata_rb}\n" end @@ -190,6 +193,7 @@ describe "ChefFSDataStore tests", :workstation do end it "knife cookbook upload works" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).once knife("cookbook upload -z --cookbook-path #{path_to("cookbooks_to_upload")} x").should_succeed stderr: <<~EOM Uploading x [1.0.0] Uploaded 1 cookbook. @@ -442,6 +446,7 @@ describe "ChefFSDataStore tests", :workstation do context "GET /TYPE" do it "knife list -z -R returns everything" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).once knife("list -z -Rfp /").should_succeed <<~EOM /clients/ /clients/x.json diff --git a/spec/integration/knife/deps_spec.rb b/spec/integration/knife/deps_spec.rb index 4dfccf38de..a6946790c7 100644 --- a/spec/integration/knife/deps_spec.rb +++ b/spec/integration/knife/deps_spec.rb @@ -41,6 +41,7 @@ describe "knife deps", :workstation do file "cookbooks/soup/recipes/chicken.rb", "" end it "knife deps reports all dependencies" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).twice knife("deps /roles/starring.json").should_succeed <<~EOM /roles/minor.json /cookbooks/quiche @@ -60,6 +61,7 @@ describe "knife deps", :workstation do file "cookbooks/soup/recipes/chicken.rb", "" end it "knife deps reports all dependencies" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).twice knife("deps /roles/starring.json").should_succeed <<~EOM /roles/minor.json /cookbooks/quiche @@ -94,6 +96,7 @@ describe "knife deps", :workstation do file "nodes/mort.json", { "run_list" => %w{role[minor] recipe[quiche] recipe[soup::chicken]} } end it "knife deps reports just the node" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).twice knife("deps /nodes/mort.json").should_succeed <<~EOM /roles/minor.json /cookbooks/quiche @@ -108,6 +111,7 @@ describe "knife deps", :workstation do file "cookbooks/quiche/recipes/default.rb", "" end it "knife deps reports just the cookbook" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).once knife("deps /cookbooks/quiche").should_succeed "/cookbooks/quiche\n" end end @@ -119,6 +123,7 @@ depends "kettle"' file "cookbooks/quiche/recipes/default.rb", "" end it "knife deps reports just the cookbook" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).twice knife("deps /cookbooks/quiche").should_succeed "/cookbooks/kettle\n/cookbooks/quiche\n" end end @@ -148,6 +153,7 @@ depends "kettle"' end it "knife deps reports all dependencies" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).twice knife("deps /nodes/mort.json").should_succeed <<~EOM /environments/desert.json /roles/minor.json @@ -158,6 +164,7 @@ depends "kettle"' EOM end it "knife deps * reports all dependencies of all things" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).twice knife("deps /nodes/*").should_succeed <<~EOM /roles/minor.json /nodes/bart.json @@ -169,6 +176,7 @@ depends "kettle"' EOM end it "knife deps a b reports all dependencies of a and b" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).twice knife("deps /nodes/bart.json /nodes/mort.json").should_succeed <<~EOM /roles/minor.json /nodes/bart.json @@ -180,6 +188,7 @@ depends "kettle"' EOM end it "knife deps --tree /* shows dependencies in a tree" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).twice knife("deps --tree /nodes/*").should_succeed <<~EOM /nodes/bart.json /roles/minor.json @@ -214,11 +223,13 @@ depends "foo"' end it "knife deps prints each once" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).at_least(3).times knife("deps /cookbooks/foo").should_succeed( stdout: "/cookbooks/baz\n/cookbooks/bar\n/cookbooks/foo\n" ) end it "knife deps --tree prints each once" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).at_least(3).times knife("deps --tree /cookbooks/foo").should_succeed( stdout: "/cookbooks/foo\n /cookbooks/bar\n /cookbooks/baz\n /cookbooks/foo\n" ) diff --git a/spec/integration/knife/upload_spec.rb b/spec/integration/knife/upload_spec.rb index 77db50fc22..7d1ef68e45 100644 --- a/spec/integration/knife/upload_spec.rb +++ b/spec/integration/knife/upload_spec.rb @@ -197,6 +197,7 @@ describe "knife upload", :workstation do end it "knife upload adds the new files" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).at_least(2).times knife("upload /").should_succeed <<~EOM Created /clients/y.json Updated /cookbooks/x @@ -216,6 +217,7 @@ describe "knife upload", :workstation do end it "knife upload --no-diff adds the new files" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).at_least(2).times knife("upload --no-diff /").should_succeed <<~EOM Created /clients/y.json Updated /cookbooks/x @@ -492,6 +494,7 @@ describe "knife upload", :workstation do # technically we shouldn't have deleted missing files. But ... cookbooks # are a special case. it "knife upload of the cookbook itself succeeds" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).at_least(2).times knife("upload /cookbooks/x").should_succeed <<~EOM Updated /cookbooks/x EOM @@ -501,6 +504,7 @@ describe "knife upload", :workstation do end it "knife upload --purge of the cookbook itself succeeds" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).at_least(2).times knife("upload /cookbooks/x").should_succeed <<~EOM Updated /cookbooks/x EOM @@ -516,6 +520,7 @@ describe "knife upload", :workstation do end it "knife upload of the cookbook succeeds" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).at_least(2).times knife("upload /cookbooks/x").should_succeed <<~EOM Updated /cookbooks/x EOM @@ -533,6 +538,7 @@ describe "knife upload", :workstation do end it "knife upload of the cookbook succeeds" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).at_least(2).times knife("upload /cookbooks/x").should_succeed <<~EOM Updated /cookbooks/x EOM @@ -548,6 +554,7 @@ describe "knife upload", :workstation do end it "knife upload --freeze freezes the cookbook" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).at_least(2).times knife("upload --freeze /cookbooks/x").should_succeed <<~EOM Updated /cookbooks/x EOM @@ -570,9 +577,11 @@ describe "knife upload", :workstation do end it "knife upload fails to upload the frozen cookbook" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).at_least(2).times knife("upload /cookbooks/frozencook").should_fail "ERROR: /cookbooks failed to write: Cookbook frozencook is frozen\n" end it "knife upload --force uploads the frozen cookbook" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).at_least(2).times knife("upload --force /cookbooks/frozencook").should_succeed <<~EOM Updated /cookbooks/frozencook EOM @@ -594,6 +603,7 @@ describe "knife upload", :workstation do end it "knife upload /cookbooks/x uploads the local version" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).at_least(2).times knife("diff --name-status /cookbooks").should_succeed <<~EOM M\t/cookbooks/x/metadata.rb D\t/cookbooks/x/onlyin1.0.1.rb @@ -631,6 +641,7 @@ describe "knife upload", :workstation do D\t/cookbooks/x/onlyin1.0.1.rb A\t/cookbooks/x/onlyin1.0.0.rb EOM + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).at_least(2).times knife("upload --purge /cookbooks/x").should_succeed <<~EOM Updated /cookbooks/x EOM @@ -649,6 +660,7 @@ describe "knife upload", :workstation do end it "knife upload /cookbooks/x uploads the local version generates metadata.json and uploads it." do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).at_least(2).times knife("upload --purge /cookbooks/x").should_succeed <<~EOM Updated /cookbooks/x EOM @@ -664,6 +676,7 @@ describe "knife upload", :workstation do end it "knife upload /cookbooks/x uploads the local version and generates metadata.json before upload and uploads it." do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).at_least(2).times knife("diff --name-status /cookbooks").should_succeed <<~EOM M\t/cookbooks/x/metadata.rb D\t/cookbooks/x/onlyin1.0.1.rb @@ -686,6 +699,7 @@ describe "knife upload", :workstation do end it "knife upload /cookbooks/x uploads the new version" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).at_least(2).times knife("upload --purge /cookbooks/x").should_succeed <<~EOM Updated /cookbooks/x EOM @@ -788,6 +802,7 @@ describe "knife upload", :workstation do file "cookbooks/x/metadata.rb", cb_metadata("x", "1.0.0", "\nchef_version '~> 999.0'") end it "knife upload succeeds" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).at_least(2).times knife("upload /cookbooks/x").should_succeed <<~EOM Created /cookbooks/x EOM @@ -938,6 +953,7 @@ describe "knife upload", :workstation do end it "knife upload adds the new files" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).at_least(3).times knife("upload /").should_succeed <<~EOM Created /clients/y.json Updated /cookbooks/x-1.0.0 @@ -1144,6 +1160,7 @@ describe "knife upload", :workstation do # technically we shouldn't have deleted missing files. But ... cookbooks # are a special case. it "knife upload of the cookbook itself succeeds" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).once knife("upload /cookbooks/x-1.0.0").should_succeed <<~EOM Updated /cookbooks/x-1.0.0 EOM @@ -1151,6 +1168,7 @@ describe "knife upload", :workstation do end it "knife upload --purge of the cookbook itself succeeds" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).once knife("upload /cookbooks/x-1.0.0").should_succeed <<~EOM Updated /cookbooks/x-1.0.0 EOM @@ -1164,6 +1182,7 @@ describe "knife upload", :workstation do end it "knife upload of the cookbook succeeds" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).once knife("upload /cookbooks/x-1.0.0").should_succeed <<~EOM Updated /cookbooks/x-1.0.0 EOM @@ -1179,6 +1198,7 @@ describe "knife upload", :workstation do end it "knife upload of the cookbook succeeds" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).once knife("upload /cookbooks/x-1.0.0").should_succeed <<~EOM Updated /cookbooks/x-1.0.0 EOM @@ -1200,6 +1220,7 @@ describe "knife upload", :workstation do end it "knife upload /cookbooks uploads the local version" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).once knife("diff --name-status /cookbooks").should_succeed <<~EOM M\t/cookbooks/x-1.0.0/onlyin1.0.0.rb D\t/cookbooks/x-1.0.1 @@ -1218,6 +1239,7 @@ describe "knife upload", :workstation do cookbook "x", "0.9.9", { "onlyin0.9.9.rb" => "hi" } end it "knife upload /cookbooks uploads the local version" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).once knife("upload --purge /cookbooks").should_succeed <<~EOM Updated /cookbooks/x-1.0.0 Deleted extra entry /cookbooks/x-0.9.9 (purge is on) @@ -1232,6 +1254,7 @@ describe "knife upload", :workstation do end it "knife upload /cookbooks/x uploads the local version" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).once knife("diff --name-status /cookbooks").should_succeed <<~EOM D\t/cookbooks/x-1.0.1 A\t/cookbooks/x-1.0.0 @@ -1250,6 +1273,7 @@ describe "knife upload", :workstation do end it "knife upload /cookbooks/x uploads the new version" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).once knife("upload --purge /cookbooks").should_succeed <<~EOM Created /cookbooks/x-1.0.0 Deleted extra entry /cookbooks/x-0.9.9 (purge is on) @@ -1324,6 +1348,7 @@ describe "knife upload", :workstation do file "cookbooks/x-1.0.0/metadata.rb", cb_metadata("x", "1.0.0", "\nchef_version '~> 999.0'") end it "knife upload succeeds" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).once knife("upload /cookbooks/x-1.0.0").should_succeed <<~EOM Created /cookbooks/x-1.0.0 EOM @@ -1387,6 +1412,7 @@ describe "knife upload", :workstation do end it "knife upload / uploads everything" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).at_least(2).times knife("upload /").should_succeed <<~EOM Updated /acls/groups/blah.json Created /clients/x.json @@ -1494,6 +1520,7 @@ describe "knife upload", :workstation do end it "knife upload updates everything" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/).at_least(2).times knife("upload /").should_succeed <<~EOM Updated /acls/groups/blah.json Updated /clients/x.json diff --git a/spec/unit/cookbook/cookbook_version_loader_spec.rb b/spec/unit/cookbook/cookbook_version_loader_spec.rb index 2cd86877cb..4095a91c02 100644 --- a/spec/unit/cookbook/cookbook_version_loader_spec.rb +++ b/spec/unit/cookbook/cookbook_version_loader_spec.rb @@ -124,8 +124,9 @@ describe Chef::Cookbook::CookbookVersionLoader do expect { cookbook_loader.load! }.to raise_error(Chef::Exceptions::CookbookNotFoundInRepo) end - it "skips the cookbook when called with #load" do - expect { cookbook_loader.load }.to_not raise_error + it "gives deprecation warning called with #load and raise error for Cookbook not found" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/) + expect { cookbook_loader.load }.to raise_error(Chef::Exceptions::CookbookNotFoundInRepo) end end @@ -148,7 +149,8 @@ describe Chef::Cookbook::CookbookVersionLoader do expect { cookbook_loader.load! }.to raise_error("THIS METADATA HAS A BUG") end - it "raises an error when called with #load" do + it "gives deprecation warning to us load! when called with #load and raises error" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/) expect { cookbook_loader.load }.to raise_error("THIS METADATA HAS A BUG") end @@ -180,7 +182,8 @@ describe Chef::Cookbook::CookbookVersionLoader do expect { cookbook_loader.load! }.to raise_error(Chef::Exceptions::MetadataNotValid, error_message) end - it "raises an error when called with #load" do + it "gives deprecation warning to use load! method when called with #load and raises error for invalid metadata" do + expect(Chef::Log).to receive(:warn).with(/load method is deprecated. Use load! instead/) expect { cookbook_loader.load }.to raise_error(Chef::Exceptions::MetadataNotValid, error_message) end |