diff options
author | Lamont Granquist <lamont@scriptkiddie.org> | 2018-10-30 21:21:23 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-10-30 21:21:23 -0700 |
commit | 82b322c634755b924a9f03c09c456bc3b017a222 (patch) | |
tree | 4b9b4c2576ced31a76992c08e167e383bc78c55f | |
parent | 60f6b36f0f8b8bfe94ec8bf01a02c865912cd0bd (diff) | |
parent | a114046532e8c9576ec185b9fba071a28bab0b3d (diff) | |
download | chef-82b322c634755b924a9f03c09c456bc3b017a222.tar.gz |
Merge pull request #7820 from chef/lcg/chef-15-cleanup-cookbook-loader
More cookbook loader cleanup and documentation
-rw-r--r-- | lib/chef/cookbook/cookbook_version_loader.rb | 111 | ||||
-rw-r--r-- | lib/chef/cookbook_loader.rb | 68 | ||||
-rw-r--r-- | spec/unit/cookbook/cookbook_version_loader_spec.rb | 4 | ||||
-rw-r--r-- | spec/unit/cookbook_loader_spec.rb | 4 |
4 files changed, 110 insertions, 77 deletions
diff --git a/lib/chef/cookbook/cookbook_version_loader.rb b/lib/chef/cookbook/cookbook_version_loader.rb index c864c30505..96bc4aa303 100644 --- a/lib/chef/cookbook/cookbook_version_loader.rb +++ b/lib/chef/cookbook/cookbook_version_loader.rb @@ -7,12 +7,21 @@ require "find" class Chef class Cookbook + # This class is only used drectly from the Chef::CookbookLoader and from chef-fs, + # so it only affects legacy-mode chef-client runs and knife. It is not used by + # server or zolo/zero modes. + # + # This seems to be mostly a glorified factory method for creating CookbookVersion + # objects now, with creating Metadata objects bolted onto the side? It used + # to be also responsible for the merging of multiple objects when creating + # shadowed/merged cookbook versions from multiple sources. It also handles + # Chefignore files. + # class CookbookVersionLoader UPLOADED_COOKBOOK_VERSION_FILE = ".uploaded-cookbook-version.json".freeze attr_reader :cookbook_settings - attr_reader :cookbook_paths attr_reader :frozen attr_reader :uploaded_cookbook_version_file @@ -25,13 +34,11 @@ class Chef 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 - @cookbook_paths = [ cookbook_path ] @inferred_cookbook_name = File.basename( path ) @chefignore = chefignore @metadata = nil - @relative_path = /#{Regexp.escape(@cookbook_path)}\/(.+)$/ + @relative_path = /#{Regexp.escape(cookbook_path)}\/(.+)$/ @metadata_loaded = false @cookbook_settings = { all_files: {}, @@ -68,53 +75,22 @@ class Chef if empty? Chef::Log.warn "Found a directory #{cookbook_name} in the cookbook path, but it contains no cookbook files. skipping." end - @cookbook_settings + cookbook_settings end alias :load_cookbooks :load - def metadata_filenames - return @metadata_filenames unless @metadata_filenames.empty? - if File.exists?(File.join(cookbook_path, UPLOADED_COOKBOOK_VERSION_FILE)) - @uploaded_cookbook_version_file = File.join(cookbook_path, UPLOADED_COOKBOOK_VERSION_FILE) - end - - if File.exists?(File.join(cookbook_path, "metadata.json")) - @metadata_filenames << File.join(cookbook_path, "metadata.json") - elsif File.exists?(File.join(cookbook_path, "metadata.rb")) - @metadata_filenames << File.join(cookbook_path, "metadata.rb") - elsif @uploaded_cookbook_version_file - @metadata_filenames << @uploaded_cookbook_version_file - end - - # Set frozen based on .uploaded-cookbook-version.json - set_frozen - @metadata_filenames - end - def cookbook_version return nil if empty? - Chef::CookbookVersion.new(cookbook_name, *cookbook_paths).tap do |c| + Chef::CookbookVersion.new(cookbook_name, cookbook_path).tap do |c| c.all_files = cookbook_settings[:all_files].values c.metadata = metadata - c.freeze_version if @frozen + c.freeze_version if frozen end end - def cookbook_name - # The `name` attribute is now required in metadata, so - # inferred_cookbook_name generally should not be used. Per CHEF-2923, - # we have to not raise errors in cookbook metadata immediately, so that - # users can still `knife cookbook upload some-cookbook` when an - # unrelated cookbook has an error in its metadata. This situation - # could prevent us from reading the `name` attribute from the metadata - # entirely, but the name is used as a hash key in CookbookLoader, so we - # fall back to the inferred name here. - (metadata.name || @inferred_cookbook_name).to_sym - end - # Generates the Cookbook::Metadata object def metadata return @metadata unless @metadata.nil? @@ -125,7 +101,7 @@ class Chef case metadata_file when /\.rb$/ apply_ruby_metadata(metadata_file) - when @uploaded_cookbook_version_file + when uploaded_cookbook_version_file apply_json_cookbook_version_metadata(metadata_file) when /\.json$/ apply_json_metadata(metadata_file) @@ -146,13 +122,46 @@ class Chef @metadata end + def cookbook_name + # The `name` attribute is now required in metadata, so + # inferred_cookbook_name generally should not be used. Per CHEF-2923, + # we have to not raise errors in cookbook metadata immediately, so that + # users can still `knife cookbook upload some-cookbook` when an + # unrelated cookbook has an error in its metadata. This situation + # could prevent us from reading the `name` attribute from the metadata + # entirely, but the name is used as a hash key in CookbookLoader, so we + # fall back to the inferred name here. + (metadata.name || inferred_cookbook_name).to_sym + end + + private + + def metadata_filenames + return @metadata_filenames unless @metadata_filenames.empty? + if File.exists?(File.join(cookbook_path, UPLOADED_COOKBOOK_VERSION_FILE)) + @uploaded_cookbook_version_file = File.join(cookbook_path, UPLOADED_COOKBOOK_VERSION_FILE) + end + + if File.exists?(File.join(cookbook_path, "metadata.json")) + @metadata_filenames << File.join(cookbook_path, "metadata.json") + elsif File.exists?(File.join(cookbook_path, "metadata.rb")) + @metadata_filenames << File.join(cookbook_path, "metadata.rb") + elsif uploaded_cookbook_version_file + @metadata_filenames << uploaded_cookbook_version_file + end + + # Set frozen based on .uploaded-cookbook-version.json + set_frozen + @metadata_filenames + end + def raise_metadata_error! - raise @metadata_error unless @metadata_error.nil? + 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('; ')}" + message = "Cookbook loaded at path [#{cookbook_path}] has invalid metadata: #{metadata.errors.join('; ')}" raise Exceptions::MetadataNotValid, message end false @@ -162,18 +171,6 @@ class Chef cookbook_settings.values.all? { |files_hash| files_hash.empty? } && metadata_filenames.size == 0 end - def merge!(other_cookbook_loader) - other_cookbook_settings = other_cookbook_loader.cookbook_settings - cookbook_settings.each do |file_type, file_list| - file_list.merge!(other_cookbook_settings[file_type]) - end - metadata_filenames.concat(other_cookbook_loader.metadata_filenames) - @cookbook_paths += other_cookbook_loader.cookbook_paths - @frozen = true if other_cookbook_loader.frozen - @metadata = nil # reset metadata so it gets reloaded and all metadata files applied. - self - end - def chefignore @chefignore ||= Chefignore.new(File.basename(cookbook_path)) end @@ -223,14 +220,14 @@ class Chef def apply_ruby_metadata(file) @metadata.from_file(file) rescue Chef::Exceptions::JSON::ParseError - Chef::Log.error("Error evaluating metadata.rb for #{@inferred_cookbook_name} in " + file) + Chef::Log.error("Error evaluating metadata.rb for #{inferred_cookbook_name} in " + file) raise end def apply_json_metadata(file) @metadata.from_json(IO.read(file)) rescue Chef::Exceptions::JSON::ParseError - Chef::Log.error("Couldn't parse cookbook metadata JSON for #{@inferred_cookbook_name} in " + file) + Chef::Log.error("Couldn't parse cookbook metadata JSON for #{inferred_cookbook_name} in " + file) raise end @@ -247,7 +244,7 @@ class Chef # metadata contains a name key. @metadata.name(data["cookbook_name"]) unless data["metadata"].key?("name") rescue Chef::Exceptions::JSON::ParseError - Chef::Log.error("Couldn't parse cookbook metadata JSON for #{@inferred_cookbook_name} in " + file) + Chef::Log.error("Couldn't parse cookbook metadata JSON for #{inferred_cookbook_name} in " + file) raise end @@ -257,7 +254,7 @@ class Chef data = Chef::JSONCompat.parse(IO.read(uploaded_cookbook_version_file)) @frozen = data["frozen?"] rescue Chef::Exceptions::JSON::ParseError - Chef::Log.error("Couldn't parse cookbook metadata JSON for #{@inferred_cookbook_name} in #{uploaded_cookbook_version_file}") + Chef::Log.error("Couldn't parse cookbook metadata JSON for #{inferred_cookbook_name} in #{uploaded_cookbook_version_file}") raise end end diff --git a/lib/chef/cookbook_loader.rb b/lib/chef/cookbook_loader.rb index ed6f9342df..1a7dec8b03 100644 --- a/lib/chef/cookbook_loader.rb +++ b/lib/chef/cookbook_loader.rb @@ -25,42 +25,72 @@ require "chef/cookbook_version" require "chef/cookbook/chefignore" require "chef/cookbook/metadata" -# -# CookbookLoader class loads the cookbooks lazily as read -# class Chef + # This class is used by knife, cheffs and legacy chef-solo modes. It is not used by the server mode + # of chef-client or zolo/zero modes. + # + # This class implements orchestration around producing a single cookbook_version for a cookbook or + # loading a Mash of all cookbook_versions, using the cookbook_version_loader class, and doing + # lazy-access and memoization to only load each cookbook once on demand. + # + # This implements a key-value style each which makes it appear to be a Hash of String => CookbookVersion + # pairs where the String is the cookbook name. The use of Enumerable combined with the Hash-style + # each is likely not entirely sane. + # + # This object is also passed and injected into the CookbookCollection object where it is converted + # to a Mash that looks almost exactly like the cookbook_by_name Mash in this object. + # class CookbookLoader - # FIXME: doc public api - attr_reader :cookbook_paths + # @return [Array<String>] the array of repo paths containing cookbook dirs + attr_reader :repo_paths + # XXX: this is highly questionable combined with the Hash-style each method include Enumerable + # @param repo_paths [Array<String>] the array of repo paths containing cookbook dirs def initialize(*repo_paths) @repo_paths = repo_paths.flatten.map { |p| File.expand_path(p) } - raise ArgumentError, "You must specify at least one cookbook repo path" if @repo_paths.empty? + raise ArgumentError, "You must specify at least one cookbook repo path" if repo_paths.empty? end + # The primary function of this class is to build this Mash mapping cookbook names as a string to + # the CookbookVersion objects for them. Callers must call "load_cookbooks" first. + # + # @return [Mash<String, Chef::CookbookVersion>] def cookbooks_by_name @cookbooks_by_name ||= Mash.new end + # This class also builds a mapping of cookbook names to their Metadata objects. Callers must call + # "load_cookbooks" first. + # + # @return [Mash<String, Chef::Cookbook::Metadata>] def metadata @metadata ||= Mash.new end + # Loads all cookbooks across all repo_paths + # + # @return [Mash<String, Chef::CookbookVersion>] the cookbooks_by_name Mash def load_cookbooks - cookbook_loaders.each_key do |cookbook_name| + cookbook_version_loaders.each_key do |cookbook_name| load_cookbook(cookbook_name) end cookbooks_by_name end + # Loads a single cookbook by its name. + # + # @param [String] + # @return [Chef::CookbookVersion] def load_cookbook(cookbook_name) - return nil unless cookbook_loaders.key?(cookbook_name) + unless cookbook_version_loaders.key?(cookbook_name) + raise Exceptions::CookbookNotFoundInRepo, "Cannot find a cookbook named #{cookbook_name}; did you forget to add metadata to a cookbook? (https://docs.chef.io/config_rb_metadata.html)" + end return cookbooks_by_name[cookbook_name] if cookbooks_by_name.key?(cookbook_name) - loader = cookbook_loaders[cookbook_name] + loader = cookbook_version_loaders[cookbook_name] loader.load @@ -71,11 +101,7 @@ class Chef end def [](cookbook) - if cookbooks_by_name.key?(cookbook.to_sym) || load_cookbook(cookbook.to_sym) - cookbooks_by_name[cookbook.to_sym] - else - raise Exceptions::CookbookNotFoundInRepo, "Cannot find a cookbook named #{cookbook}; did you forget to add metadata to a cookbook? (https://docs.chef.io/config_rb_metadata.html)" - end + load_cookbook(cookbook) end alias :fetch :[] @@ -113,6 +139,11 @@ class Chef private + # Helper method to lazily create and remember the chefignore object + # for a given repo_path. + # + # @param [String] repo_path the full path to the cookbook directory of the repo + # @return [Chef::Cookbook::Chefignore] the chefignore object for the repo_path def chefignore(repo_path) @chefignores ||= {} @chefignores[repo_path] ||= Cookbook::Chefignore.new(repo_path) @@ -126,14 +157,17 @@ class Chef def all_files_in_repo_paths @all_files_in_repo_paths ||= begin - @repo_paths.inject([]) do |all_children, repo_path| + repo_paths.inject([]) do |all_children, repo_path| all_children + Dir[File.join(Chef::Util::PathHelper.escape_glob_dir(repo_path), "*")] end end end - def cookbook_loaders - @cookbook_loaders ||= + # This method creates a Mash of the CookbookVersionLoaders for each cookbook. + # + # @return [Mash<String, Cookbook::CookbookVersionLoader>] + def cookbook_version_loaders + @cookbook_version_loaders ||= begin mash = Mash.new all_directories_in_repo_paths.each do |cookbook_path| diff --git a/spec/unit/cookbook/cookbook_version_loader_spec.rb b/spec/unit/cookbook/cookbook_version_loader_spec.rb index 40a054abee..c38af059f5 100644 --- a/spec/unit/cookbook/cookbook_version_loader_spec.rb +++ b/spec/unit/cookbook/cookbook_version_loader_spec.rb @@ -1,6 +1,6 @@ # # Author:: Daniel DeLeo (<dan@chef.io>) -# Copyright:: Copyright 2014-2016, Chef Software, Inc. +# Copyright:: Copyright 2014-2018, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -173,7 +173,7 @@ describe Chef::Cookbook::CookbookVersionLoader 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" + "Cookbook loaded at path [#{cookbook_path}] has invalid metadata: The `name' attribute is required in cookbook metadata" end it "raises an error when loading with #load!" do diff --git a/spec/unit/cookbook_loader_spec.rb b/spec/unit/cookbook_loader_spec.rb index 7c15cc9030..ddb4f00f35 100644 --- a/spec/unit/cookbook_loader_spec.rb +++ b/spec/unit/cookbook_loader_spec.rb @@ -196,7 +196,9 @@ describe Chef::CookbookLoader do end it "should not load the cookbook again when accessed" do - expect(cookbook_loader).not_to receive("load_cookbook") + cookbook_loader.send(:cookbook_version_loaders).each do |cbv_loader| + expect(cbv_loader).not_to receive(:load) + end cookbook_loader["openldap"] end |