diff options
author | Lamont Granquist <lamont@scriptkiddie.org> | 2018-10-26 17:30:35 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-10-26 17:30:35 -0700 |
commit | 52c7265008ee36cc8cc064a829961b6fc25e6585 (patch) | |
tree | ce13b93cc90dd72a60928a1e2053d874f5f2712d | |
parent | 19fa0db16f3f2952c378eeb37a4d15e9a142a0be (diff) | |
parent | 1a420297efc12ee84de9c3c71be948458d80f17b (diff) | |
download | chef-52c7265008ee36cc8cc064a829961b6fc25e6585.tar.gz |
Merge pull request #7792 from chef/lcg/chef-15-cookbook-loader
Remove cookbook merging/shadowing from the cookbooker loader
-rw-r--r-- | lib/chef/cookbook_loader.rb | 42 | ||||
-rw-r--r-- | lib/chef/exceptions.rb | 3 | ||||
-rw-r--r-- | lib/chef/knife/cookbook_upload.rb | 25 | ||||
-rw-r--r-- | spec/unit/cookbook_loader_spec.rb | 68 | ||||
-rw-r--r-- | spec/unit/knife/cookbook_upload_spec.rb | 31 |
5 files changed, 27 insertions, 142 deletions
diff --git a/lib/chef/cookbook_loader.rb b/lib/chef/cookbook_loader.rb index 32fd71f604..322de539f1 100644 --- a/lib/chef/cookbook_loader.rb +++ b/lib/chef/cookbook_loader.rb @@ -44,7 +44,6 @@ class Chef @cookbooks_by_name = Mash.new @loaded_cookbooks = {} @metadata = Mash.new - @cookbooks_paths = Hash.new { |h, k| h[k] = [] } # for deprecation warnings @chefignores = {} @repo_paths = repo_paths.map do |repo_path| File.expand_path(repo_path) @@ -52,31 +51,9 @@ class Chef @preloaded_cookbooks = false @loaders_by_name = {} - - # Used to track which cookbooks appear in multiple places in the cookbook repos - # and are merged in to a single cookbook by file shadowing. This behavior is - # deprecated, so users of this class may issue warnings to the user by checking - # this variable - @merged_cookbooks = [] - end - - def merged_cookbook_paths # for deprecation warnings - merged_cookbook_paths = {} - @merged_cookbooks.each { |c| merged_cookbook_paths[c] = @cookbooks_paths[c] } - merged_cookbook_paths end - def warn_about_cookbook_shadowing - unless merged_cookbooks.empty? - Chef::Log.deprecation "The cookbook(s): #{merged_cookbooks.join(', ')} exist in multiple places in your cookbook_path. " + - "A composite version has been compiled. This has been deprecated since 0.10.4, in Chef 13 this behavior will be REMOVED." - end - end - - # Will be removed when cookbook shadowing is removed, do NOT create new consumers of this API. - # - # @api private - def load_cookbooks_without_shadow_warning + def load_cookbooks preload_cookbooks @loaders_by_name.each_key do |cookbook_name| load_cookbook(cookbook_name) @@ -84,12 +61,6 @@ class Chef @cookbooks_by_name end - def load_cookbooks - ret = load_cookbooks_without_shadow_warning - warn_about_cookbook_shadowing - ret - end - def load_cookbook(cookbook_name) preload_cookbooks @@ -102,14 +73,11 @@ class Chef next if loader.empty? - @cookbooks_paths[cookbook_name] << loader.cookbook_path # for deprecation warnings - if @loaded_cookbooks.key?(cookbook_name) - @merged_cookbooks << cookbook_name # for deprecation warnings - @loaded_cookbooks[cookbook_name].merge!(loader) - else - @loaded_cookbooks[cookbook_name] = loader + raise Chef::Exceptions::CookbookMergingError, "Cookbook merging is no longer supported, the cookbook named #{cookbook_name} can only appear once in the cookbook_path" end + + @loaded_cookbooks[cookbook_name] = loader end if @loaded_cookbooks.key?(cookbook_name) @@ -133,6 +101,7 @@ class Chef def has_key?(cookbook_name) not self[cookbook_name.to_sym].nil? end + alias :cookbook_exists? :has_key? alias :key? :has_key? @@ -157,6 +126,7 @@ class Chef def values @cookbooks_by_name.values end + alias :cookbooks :values private diff --git a/lib/chef/exceptions.rb b/lib/chef/exceptions.rb index dafd445d2d..41e0cdb33e 100644 --- a/lib/chef/exceptions.rb +++ b/lib/chef/exceptions.rb @@ -2,7 +2,7 @@ # Author:: Adam Jacob (<adam@chef.io>) # Author:: Seth Falcon (<seth@chef.io>) # Author:: Kyle Goodwin (<kgoodwin@primerevenue.com>) -# Copyright:: Copyright 2008-2017, Chef Software Inc. +# Copyright:: Copyright 2008-2018, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -99,6 +99,7 @@ class Chef # Cookbook loader used to raise an argument error when cookbook not found. # for back compat, need to raise an error that inherits from ArgumentError class CookbookNotFoundInRepo < ArgumentError; end + class CookbookMergingError < RuntimeError; end class RecipeNotFound < ArgumentError; end # AttributeNotFound really means the attribute file could not be found class AttributeNotFound < RuntimeError; end diff --git a/lib/chef/knife/cookbook_upload.rb b/lib/chef/knife/cookbook_upload.rb index ba63e7d0fd..4260ee01e2 100644 --- a/lib/chef/knife/cookbook_upload.rb +++ b/lib/chef/knife/cookbook_upload.rb @@ -93,7 +93,6 @@ class Chef end assert_environment_valid! - warn_about_cookbook_shadowing version_constraints_to_update = {} upload_failures = 0 upload_ok = 0 @@ -103,7 +102,7 @@ class Chef @server_side_cookbooks = Chef::CookbookVersion.list_all_versions justify_width = @server_side_cookbooks.map { |name| name.size }.max.to_i + 2 if config[:all] - cookbook_repo.load_cookbooks_without_shadow_warning + cookbook_repo.load_cookbooks cookbooks_for_upload = [] cookbook_repo.each do |cookbook_name, cookbook| cookbooks_for_upload << cookbook @@ -164,7 +163,7 @@ class Chef def cookbooks_to_upload @cookbooks_to_upload ||= if config[:all] - cookbook_repo.load_cookbooks_without_shadow_warning + cookbook_repo.load_cookbooks else upload_set = {} @name_args.each do |cookbook_name| @@ -202,26 +201,6 @@ class Chef @environment ||= config[:environment] ? Environment.load(config[:environment]) : nil end - def warn_about_cookbook_shadowing - # because cookbooks are lazy-loaded, we have to force the loader - # to load the cookbooks the user intends to upload here: - cookbooks_to_upload - - unless cookbook_repo.merged_cookbooks.empty? - ui.warn "* " * 40 - ui.warn(<<~WARNING) - The cookbooks: #{cookbook_repo.merged_cookbooks.join(', ')} exist in multiple places in your cookbook_path. - A composite version of these cookbooks has been compiled for uploading. - - #{ui.color('IMPORTANT:', :red, :bold)} In a future version of Chef, this behavior will be removed and you will no longer - be able to have the same version of a cookbook in multiple places in your cookbook_path. -WARNING - ui.warn "The affected cookbooks are located:" - ui.output ui.format_for_display(cookbook_repo.merged_cookbook_paths) - ui.warn "* " * 40 - end - end - private def assert_environment_valid! diff --git a/spec/unit/cookbook_loader_spec.rb b/spec/unit/cookbook_loader_spec.rb index 6553dad433..15c7d1e299 100644 --- a/spec/unit/cookbook_loader_spec.rb +++ b/spec/unit/cookbook_loader_spec.rb @@ -24,7 +24,6 @@ describe Chef::CookbookLoader do end let(:repo_paths) do [ - File.expand_path(File.join(CHEF_SPEC_DATA, "kitchen")), File.expand_path(File.join(CHEF_SPEC_DATA, "cookbooks")), ] end @@ -49,26 +48,25 @@ describe Chef::CookbookLoader do .once .and_call_original end - expect(Chef::Log).to receive(:deprecation).with(/The cookbook\(s\): openldap exist in multiple places in your cookbook_path./) cookbook_loader.load_cookbooks end - context "after loading all cookbooks" do - before(:each) do - expect(Chef::Log).to receive(:deprecation).with(/The cookbook\(s\): openldap exist in multiple places in your cookbook_path./) - cookbook_loader.load_cookbooks + context "removed cookbook merging" do + let(:repo_paths) do + [ + File.expand_path(File.join(CHEF_SPEC_DATA, "kitchen")), + File.expand_path(File.join(CHEF_SPEC_DATA, "cookbooks")), + ] end - - it "should be possible to reload all the cookbooks without triggering deprecation warnings on all of them", chef: "< 15" do + it "should not support multiple merged cookbooks in the cookbook path" do start_merged_cookbooks = cookbook_loader.merged_cookbooks - expect(Chef::Log).to receive(:deprecation).with(/The cookbook\(s\): openldap exist in multiple places in your cookbook_path./) - cookbook_loader.load_cookbooks - expect(cookbook_loader.merged_cookbooks).to eql(start_merged_cookbooks) + expect { cookbook_loader.load_cookbooks }.to raise_error(Chef::Exceptions::CookbookMergingError) end + end - it "should not support multiple merged cookbooks in the cookbook path", chef: ">= 15" do - start_merged_cookbooks = cookbook_loader.merged_cookbooks - expect { cookbook_loader.load_cookbooks }.to raise_error("FIXME WITH THE CLASS YOU DECIDE TO USE HERE") + context "after loading all cookbooks" do + before(:each) do + cookbook_loader.load_cookbooks end describe "[]" do @@ -114,57 +112,24 @@ describe Chef::CookbookLoader do expect(cookbook_loader).to have_key(:apache2) end - it "should allow you to override an attribute file via cookbook_path" do - expect(full_paths_for_part(:openldap, "attributes").detect do |f| - f =~ /cookbooks\/openldap\/attributes\/default.rb/ - end).not_to eql(nil) - expect(full_paths_for_part(:openldap, "attributes").detect do |f| - f =~ /kitchen\/openldap\/attributes\/default.rb/ - end).to eql(nil) - end - it "should load different attribute files from deeper paths" do expect(full_paths_for_part(:openldap, "attributes").detect do |f| - f =~ /kitchen\/openldap\/attributes\/robinson.rb/ - end).not_to eql(nil) - end - - it "should allow you to override a definition file via cookbook_path" do - expect(full_paths_for_part(:openldap, "definitions").detect do |f| - f =~ /cookbooks\/openldap\/definitions\/client.rb/ + f =~ /cookbooks\/openldap\/attributes\/smokey.rb/ end).not_to eql(nil) - expect(full_paths_for_part(:openldap, "definitions").detect do |f| - f =~ /kitchen\/openldap\/definitions\/client.rb/ - end).to eql(nil) end it "should load definition files from deeper paths" do expect(full_paths_for_part(:openldap, "definitions").detect do |f| - f =~ /kitchen\/openldap\/definitions\/drewbarrymore.rb/ + f =~ /cookbooks\/openldap\/definitions\/server.rb/ end).not_to eql(nil) end - it "should allow you to override a recipe file via cookbook_path" do - expect(full_paths_for_part(:openldap, "recipes").detect do |f| - f =~ /cookbooks\/openldap\/recipes\/gigantor.rb/ - end).not_to eql(nil) - expect(full_paths_for_part(:openldap, "recipes").detect do |f| - f =~ /kitchen\/openldap\/recipes\/gigantor.rb/ - end).to eql(nil) - end - it "should load recipe files from deeper paths" do expect(full_paths_for_part(:openldap, "recipes").detect do |f| - f =~ /kitchen\/openldap\/recipes\/woot.rb/ + f =~ /cookbooks\/openldap\/recipes\/one.rb/ end).not_to eql(nil) end - it "should allow you to have an 'ignore' file, which skips loading files in later cookbooks" do - expect(full_paths_for_part(:openldap, "recipes").detect do |f| - f =~ /kitchen\/openldap\/recipes\/ignoreme.rb/ - end).to eql(nil) - end - it "should find files that start with a ." do expect(full_paths_for_part(:openldap, "files").detect do |f| f =~ /\.dotfile$/ @@ -187,7 +152,6 @@ describe Chef::CookbookLoader 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"), ] @@ -253,7 +217,6 @@ describe Chef::CookbookLoader 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"), ] @@ -271,7 +234,6 @@ describe Chef::CookbookLoader do describe "loading all cookbooks after loading only one cookbook" do before(:each) do - expect(Chef::Log).to receive(:deprecation).with(/The cookbook\(s\): openldap exist in multiple places in your cookbook_path./) cookbook_loader.load_cookbooks end diff --git a/spec/unit/knife/cookbook_upload_spec.rb b/spec/unit/knife/cookbook_upload_spec.rb index 74612f520d..ba0d57c5d6 100644 --- a/spec/unit/knife/cookbook_upload_spec.rb +++ b/spec/unit/knife/cookbook_upload_spec.rb @@ -1,7 +1,7 @@ # # Author:: Matthew Kent (<mkent@magoazul.com>) # Author:: Steven Danna (<steve@chef.io>) -# Copyright:: Copyright 2012-2016, Chef Software Inc. +# Copyright:: Copyright 2012-2018, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -32,7 +32,7 @@ describe Chef::Knife::CookbookUpload do let(:cookbook_loader) do cookbook_loader = cookbooks_by_name.dup allow(cookbook_loader).to receive(:merged_cookbooks).and_return([]) - allow(cookbook_loader).to receive(:load_cookbooks_without_shadow_warning).and_return(cookbook_loader) + allow(cookbook_loader).to receive(:load_cookbooks).and_return(cookbook_loader) cookbook_loader end @@ -102,31 +102,6 @@ describe Chef::Knife::CookbookUpload do end end - context "when uploading a cookbook that uses deprecated overlays" do - - before do - allow(cookbook_loader).to receive(:merged_cookbooks).and_return(["test_cookbook"]) - allow(cookbook_loader).to receive(:merged_cookbook_paths) - .and_return({ "test_cookbook" => %w{/path/one/test_cookbook /path/two/test_cookbook} }) - end - - it "emits a warning" do - knife.run - expected_message = <<~E - WARNING: The cookbooks: test_cookbook exist in multiple places in your cookbook_path. - A composite version of these cookbooks has been compiled for uploading. - - IMPORTANT: In a future version of Chef, this behavior will be removed and you will no longer - be able to have the same version of a cookbook in multiple places in your cookbook_path. - WARNING: The affected cookbooks are located: - test_cookbook: - /path/one/test_cookbook - /path/two/test_cookbook -E - expect(output.string).to include(expected_message) - end - end - describe "when specifying a cookbook name among many" do let(:name_args) { ["test_cookbook1"] } @@ -145,7 +120,6 @@ E it "should not read all cookbooks" do expect(cookbook_loader).not_to receive(:load_cookbooks) - expect(cookbook_loader).not_to receive(:load_cookbooks_without_shadow_warning) knife.run end @@ -209,7 +183,6 @@ E it "should exit and not upload the cookbook" do expect(cookbook_loader).to receive(:[]).once.with("test_cookbook") expect(cookbook_loader).not_to receive(:load_cookbooks) - expect(cookbook_loader).not_to receive(:load_cookbooks_without_shadow_warning) expect(cookbook_uploader).not_to receive(:upload_cookbooks) expect { knife.run }.to raise_error(SystemExit) end |