diff options
author | Jason Barnett <jason.w.barnett@gmail.com> | 2021-06-08 16:39:18 -0600 |
---|---|---|
committer | Jason Barnett <jason.w.barnett@gmail.com> | 2021-08-16 13:00:46 -0600 |
commit | f87eb43b59e691856dacd428997dc59d3ca0d70d (patch) | |
tree | 59a8c5d8135c099620b7b8d253b066b130a212d0 | |
parent | a9a073b0a3e5bc9f73ac57e95393c0060cbe7809 (diff) | |
download | chef-f87eb43b59e691856dacd428997dc59d3ca0d70d.tar.gz |
Optionally check for cookbook dependencies when using knife cookbook upload
Signed-off-by: Jason Barnett <jason.w.barnett@gmail.com>
-rw-r--r-- | knife/lib/chef/knife/cookbook_upload.rb | 47 | ||||
-rw-r--r-- | knife/spec/unit/knife/cookbook_upload_spec.rb | 134 |
2 files changed, 132 insertions, 49 deletions
diff --git a/knife/lib/chef/knife/cookbook_upload.rb b/knife/lib/chef/knife/cookbook_upload.rb index d9582a3ccc..357019212f 100644 --- a/knife/lib/chef/knife/cookbook_upload.rb +++ b/knife/lib/chef/knife/cookbook_upload.rb @@ -71,6 +71,11 @@ class Chef long: "--include-dependencies", description: "Also upload cookbook dependencies." + option :check_dependencies, + boolean: true, long: "--[no-]check-dependencies", + description: "Whether or not cookbook dependencies are verified before uploading cookbook(s) to #{ChefUtils::Dist::Server::PRODUCT}. You shouldn't disable this unless you really know what you're doing.", + default: true + def run # Sanity check before we load anything from the server if ! config[:all] && @name_args.empty? @@ -86,11 +91,6 @@ class Chef upload_failures = 0 upload_ok = 0 - # Get a list of cookbooks and their versions from the server - # to check for the existence of a cookbook's dependencies. - @server_side_cookbooks = Chef::CookbookVersion.list_all_versions - justify_width = @server_side_cookbooks.map(&:size).max.to_i + 2 - cookbooks = [] cookbooks_to_upload.each do |cookbook_name, cookbook| raise Chef::Exceptions::MetadataNotFound.new(cookbook.root_paths[0], cookbook_name) unless cookbook.has_metadata_file? @@ -120,7 +120,7 @@ class Chef if config[:all] if cookbooks_for_upload.any? begin - upload(cookbooks_for_upload, justify_width) + upload(cookbooks_for_upload) rescue Chef::Exceptions::CookbookFrozen ui.warn("Not updating version constraints for some cookbooks in the environment as the cookbook is frozen.") ui.error("Uploading of some of the cookbooks must be failed. Remove cookbook whose version is frozen from your cookbooks repo OR use --force option.") @@ -133,7 +133,7 @@ class Chef else tmp_cl.each do |cookbook_name, cookbook| - upload([cookbook], justify_width) + upload([cookbook]) upload_ok += 1 rescue Exceptions::CookbookNotFoundInRepo => e upload_failures += 1 @@ -165,6 +165,27 @@ class Chef end end + def server_side_cookbooks + @server_side_cookbooks ||= Chef::CookbookVersion.list_all_versions + end + + def justify_width + @justify_width ||= server_side_cookbooks.map(&:size).max.to_i + 2 + end + + # + # @param cookbook [Chef::CookbookVersion] + # + def left_justify_name(cookbook) + # We only want to lookup justify width value if we're already loading + # cookbooks to check dependencies exist in Chef Infra Server. + if config[:check_dependencies] == true + cookbook.name.to_s.ljust(justify_width + 10) + else + cookbook.name.to_s.ljust(24) + end + end + def cookbooks_to_upload @cookbooks_to_upload ||= if config[:all] @@ -220,11 +241,11 @@ class Chef end end - def upload(cookbooks, justify_width) + def upload(cookbooks) cookbooks.each do |cb| - ui.info("Uploading #{cb.name.to_s.ljust(justify_width + 10)} [#{cb.version}]") + ui.info("Uploading #{left_justify_name(cb)} [#{cb.version}]") check_for_broken_links!(cb) - check_for_dependencies!(cb) + check_for_dependencies!(cb) if config[:check_dependencies] == true end Chef::CookbookUploader.new(cookbooks, force: config[:force], concurrency: config[:concurrency]).upload_cookbooks rescue Chef::Exceptions::CookbookFrozen => e @@ -265,12 +286,12 @@ class Chef end def check_server_side_cookbooks(cookbook_name, version) - if @server_side_cookbooks[cookbook_name].nil? + if server_side_cookbooks[cookbook_name].nil? false else - versions = @server_side_cookbooks[cookbook_name]["versions"].collect { |versions| versions["version"] } + versions = server_side_cookbooks[cookbook_name]["versions"].collect { |versions| versions["version"] } Log.debug "Versions of cookbook '#{cookbook_name}' returned by the server: #{versions.join(", ")}" - @server_side_cookbooks[cookbook_name]["versions"].each do |versions_hash| + server_side_cookbooks[cookbook_name]["versions"].each do |versions_hash| if Chef::VersionConstraint.new(version).include?(versions_hash["version"]) Log.debug "Matched cookbook '#{cookbook_name}' with constraint '#{version}' to cookbook version '#{versions_hash["version"]}' on the server" return true diff --git a/knife/spec/unit/knife/cookbook_upload_spec.rb b/knife/spec/unit/knife/cookbook_upload_spec.rb index 0893f6a6b3..a61eee9249 100644 --- a/knife/spec/unit/knife/cookbook_upload_spec.rb +++ b/knife/spec/unit/knife/cookbook_upload_spec.rb @@ -60,11 +60,12 @@ describe Chef::Knife::CookbookUpload do before(:each) do allow(Chef::CookbookLoader).to receive(:new).and_return(cookbook_loader) allow(Chef::CookbookLoader).to receive(:copy_to_tmp_dir_from_array).and_yield(cookbook_loader) + allow(Chef::CookbookVersion).to receive(:list).and_return({}) + allow(Chef::CookbookVersion).to receive(:list_all_versions).and_return({}) end describe "with --concurrency" do it "should upload cookbooks with predefined concurrency" do - allow(Chef::CookbookVersion).to receive(:list_all_versions).and_return({}) knife.config[:concurrency] = 3 test_cookbook = Chef::CookbookVersion.new("test_cookbook", "/tmp/blah") allow(cookbook_loader).to receive(:each).and_yield("test_cookbook", test_cookbook) @@ -79,7 +80,6 @@ describe Chef::Knife::CookbookUpload do describe "run" do before(:each) do allow(Chef::CookbookUploader).to receive_messages(new: cookbook_uploader) - allow(Chef::CookbookVersion).to receive(:list_all_versions).and_return({}) end it "should print usage and exit when a cookbook name is not provided" do @@ -214,48 +214,110 @@ describe Chef::Knife::CookbookUpload do end end - describe "when specifying a cookbook name with missing dependencies" do - let(:cookbook_dependency) { Chef::CookbookVersion.new("dependency", "/tmp/blah") } + context "when chef_dependencies config is disabled" do + before do + knife.config[:check_dependencies] = false + end - before(:each) do - cookbook.metadata.depends("dependency") - allow(cookbook_loader).to receive(:[]) do |ckbk| - { "test_cookbook" => cookbook, - "dependency" => cookbook_dependency }[ckbk] + describe "when specifying a cookbook name with missing dependencies" do + let(:cookbook_dependency) { Chef::CookbookVersion.new("dependency", "/tmp/blah") } + + before(:each) do + cookbook.metadata.depends("dependency") + allow(cookbook_loader).to receive(:[]) do |ckbk| + { "test_cookbook" => cookbook, + "dependency" => cookbook_dependency }[ckbk] + end + allow(knife).to receive(:cookbook_names).and_return(%w{cookbook_dependency test_cookbook}) + @stdout, @stderr, @stdin = StringIO.new, StringIO.new, StringIO.new + knife.ui = Chef::Knife::UI.new(@stdout, @stderr, @stdin, {}) end - allow(knife).to receive(:cookbook_names).and_return(%w{cookbook_dependency test_cookbook}) - @stdout, @stderr, @stdin = StringIO.new, StringIO.new, StringIO.new - knife.ui = Chef::Knife::UI.new(@stdout, @stderr, @stdin, {}) - end - it "should exit and not upload the cookbook" do - expect(cookbook_loader).to receive(:[]).once.with("test_cookbook") - expect(cookbook_uploader).not_to receive(:upload_cookbooks) - expect { knife.run }.to raise_error(SystemExit) + it "should not fetch all cookbooks from Chef Infra Server" do + expect(Chef::CookbookVersion).not_to receive(:list_all_versions) + knife.run + end + + it "should upload the cookbook" do + expect(cookbook_loader).to receive(:[]).once.with("test_cookbook") + expect(cookbook_uploader).to receive(:upload_cookbooks) + knife.run + end + + it "should not output a message for a single missing dependency" do + knife.run + expect(@stderr.string).not_to include("Cookbook test_cookbook depends on cookbooks which are not currently") + expect(@stderr.string).not_to include("being uploaded and cannot be found on the server.") + expect(@stderr.string).not_to include("The missing cookbook(s) are: 'dependency' version '>= 0.0.0'") + end + + it "should not output a message for a multiple missing dependencies which are concatenated" do + cookbook_dependency2 = Chef::CookbookVersion.new("dependency2") + cookbook.metadata.depends("dependency2") + allow(cookbook_loader).to receive(:[]) do |ckbk| + { "test_cookbook" => cookbook, + "dependency" => cookbook_dependency, + "dependency2" => cookbook_dependency2 }[ckbk] + end + allow(knife).to receive(:cookbook_names).and_return(%w{dependency dependency2 test_cookbook}) + knife.run + expect(@stderr.string).not_to include("Cookbook test_cookbook depends on cookbooks which are not currently") + expect(@stderr.string).not_to include("being uploaded and cannot be found on the server.") + expect(@stderr.string).not_to include("The missing cookbook(s) are:") + expect(@stderr.string).not_to include("'dependency' version '>= 0.0.0'") + expect(@stderr.string).not_to include("'dependency2' version '>= 0.0.0'") + end end + end - it "should output a message for a single missing dependency" do - expect { knife.run }.to raise_error(SystemExit) - expect(@stderr.string).to include("Cookbook test_cookbook depends on cookbooks which are not currently") - expect(@stderr.string).to include("being uploaded and cannot be found on the server.") - expect(@stderr.string).to include("The missing cookbook(s) are: 'dependency' version '>= 0.0.0'") + context "when chef_dependencies config is enabled" do + before do + knife.config[:check_dependencies] = true end - it "should output a message for a multiple missing dependencies which are concatenated" do - cookbook_dependency2 = Chef::CookbookVersion.new("dependency2") - cookbook.metadata.depends("dependency2") - allow(cookbook_loader).to receive(:[]) do |ckbk| - { "test_cookbook" => cookbook, - "dependency" => cookbook_dependency, - "dependency2" => cookbook_dependency2 }[ckbk] + describe "when specifying a cookbook name with missing dependencies" do + let(:cookbook_dependency) { Chef::CookbookVersion.new("dependency", "/tmp/blah") } + + before(:each) do + cookbook.metadata.depends("dependency") + allow(cookbook_loader).to receive(:[]) do |ckbk| + { "test_cookbook" => cookbook, + "dependency" => cookbook_dependency }[ckbk] + end + allow(knife).to receive(:cookbook_names).and_return(%w{cookbook_dependency test_cookbook}) + @stdout, @stderr, @stdin = StringIO.new, StringIO.new, StringIO.new + knife.ui = Chef::Knife::UI.new(@stdout, @stderr, @stdin, {}) + end + + it "should exit and not upload the cookbook" do + expect(cookbook_loader).to receive(:[]).once.with("test_cookbook") + expect(cookbook_uploader).not_to receive(:upload_cookbooks) + expect { knife.run }.to raise_error(SystemExit) + end + + it "should output a message for a single missing dependency" do + expect { knife.run }.to raise_error(SystemExit) + expect(@stderr.string).to include("Cookbook test_cookbook depends on cookbooks which are not currently") + expect(@stderr.string).to include("being uploaded and cannot be found on the server.") + expect(@stderr.string).to include("The missing cookbook(s) are: 'dependency' version '>= 0.0.0'") + end + + it "should output a message for a multiple missing dependencies which are concatenated" do + cookbook_dependency2 = Chef::CookbookVersion.new("dependency2") + cookbook.metadata.depends("dependency2") + allow(cookbook_loader).to receive(:[]) do |ckbk| + { "test_cookbook" => cookbook, + "dependency" => cookbook_dependency, + "dependency2" => cookbook_dependency2 }[ckbk] + end + allow(knife).to receive(:cookbook_names).and_return(%w{dependency dependency2 test_cookbook}) + expect { knife.run }.to raise_error(SystemExit) + expect(@stderr.string).to include("Cookbook test_cookbook depends on cookbooks which are not currently") + expect(@stderr.string).to include("being uploaded and cannot be found on the server.") + expect(@stderr.string).to include("The missing cookbook(s) are:") + expect(@stderr.string).to include("'dependency' version '>= 0.0.0'") + expect(@stderr.string).to include("'dependency2' version '>= 0.0.0'") end - allow(knife).to receive(:cookbook_names).and_return(%w{dependency dependency2 test_cookbook}) - expect { knife.run }.to raise_error(SystemExit) - expect(@stderr.string).to include("Cookbook test_cookbook depends on cookbooks which are not currently") - expect(@stderr.string).to include("being uploaded and cannot be found on the server.") - expect(@stderr.string).to include("The missing cookbook(s) are:") - expect(@stderr.string).to include("'dependency' version '>= 0.0.0'") - expect(@stderr.string).to include("'dependency2' version '>= 0.0.0'") end end |