From 87c66a075c21f28cbbe9e1bee64f9b0f3c36fcfc Mon Sep 17 00:00:00 2001 From: Lamont Granquist Date: Fri, 3 Apr 2020 16:01:17 -0700 Subject: Implement eager_load_libraries metadata option This implements RFC-40 from the old chef rfc repo: https://github.com/chef-boneyard/chef-rfc/blob/master/rfc040-on-demand-cookbook-libraries.md Signed-off-by: Lamont Granquist --- lib/chef/cookbook/metadata.rb | 28 ++++- lib/chef/run_context/cookbook_compiler.rb | 34 ++++-- spec/integration/client/client_spec.rb | 125 ++++++++++++++++++++++- spec/integration/knife/cookbook_show_spec.rb | 56 +++++----- spec/support/shared/integration/knife_support.rb | 17 +-- spec/unit/cookbook/metadata_spec.rb | 37 ++++++- spec/unit/knife/cookbook_show_spec.rb | 3 +- 7 files changed, 252 insertions(+), 48 deletions(-) diff --git a/lib/chef/cookbook/metadata.rb b/lib/chef/cookbook/metadata.rb index ad7acb0e7b..d924be0c85 100644 --- a/lib/chef/cookbook/metadata.rb +++ b/lib/chef/cookbook/metadata.rb @@ -1,7 +1,7 @@ # Author:: Adam Jacob () # Author:: AJ Christensen () # Author:: Seth Falcon () -# Copyright:: Copyright 2008-2018, Chef Software Inc. +# Copyright:: Copyright 2008-2020, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -52,11 +52,13 @@ class Chef CHEF_VERSIONS = "chef_versions".freeze OHAI_VERSIONS = "ohai_versions".freeze GEMS = "gems".freeze + EAGER_LOAD_LIBRARIES = "eager_load_libraries".freeze COMPARISON_FIELDS = %i{name description long_description maintainer maintainer_email license platforms dependencies providing recipes version source_url issues_url - privacy chef_versions ohai_versions gems}.freeze + privacy chef_versions ohai_versions gems + eager_load_libraries}.freeze VERSION_CONSTRAINTS = { depends: DEPENDENCIES, provides: PROVIDING, @@ -109,6 +111,7 @@ class Chef @chef_versions = [] @ohai_versions = [] @gems = [] + @eager_load_libraries = true @errors = [] end @@ -344,6 +347,25 @@ class Chef @gems end + # Metadata DSL to control the behavior of library loading. + # + # Can be set to: + # + # true - libraries are eagerly loaded in alphabetical order (backcompat) + # false - libraries are not eagerly loaded, the libraries dir is added to the LOAD_PATH + # String - a file or glob pattern to eagerly load, otherwise it is treated like `false` + # Array - array of files or globs to eagerly load, otherwise it is treated like `false` + # + # @params arg [Array,String,TrueClass,FalseClass] + # @params [Array,TrueClass,FalseCalss] + def eager_load_libraries(arg = nil) + set_or_return( + :eager_load_libraries, + arg, + kind_of: [ Array, String, TrueClass, FalseClass ] + ) + end + # Adds a description for a recipe. # # === Parameters @@ -435,6 +457,7 @@ class Chef CHEF_VERSIONS => gem_requirements_to_array(*chef_versions), OHAI_VERSIONS => gem_requirements_to_array(*ohai_versions), GEMS => gems, + EAGER_LOAD_LIBRARIES => eager_load_libraries, } end @@ -468,6 +491,7 @@ class Chef @chef_versions = gem_requirements_from_array("chef", o[CHEF_VERSIONS]) if o.key?(CHEF_VERSIONS) @ohai_versions = gem_requirements_from_array("ohai", o[OHAI_VERSIONS]) if o.key?(OHAI_VERSIONS) @gems = o[GEMS] if o.key?(GEMS) + @eager_load_libraries = o[EAGER_LOAD_LIBRARIES] if o.key?(EAGER_LOAD_LIBRARIES) self end diff --git a/lib/chef/run_context/cookbook_compiler.rb b/lib/chef/run_context/cookbook_compiler.rb index 116020de19..e91c440ff1 100644 --- a/lib/chef/run_context/cookbook_compiler.rb +++ b/lib/chef/run_context/cookbook_compiler.rb @@ -1,6 +1,6 @@ # # Author:: Daniel DeLeo () -# Copyright:: Copyright 2012-2019, Chef Software Inc. +# Copyright:: Copyright 2012-2020, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -100,7 +100,15 @@ class Chef def compile_libraries @events.library_load_start(count_files_by_segment(:libraries)) cookbook_order.each do |cookbook| - load_libraries_from_cookbook(cookbook) + eager_load_libraries = cookbook_collection[cookbook].metadata.eager_load_libraries + if eager_load_libraries == true # actully true, not truthy + load_libraries_from_cookbook(cookbook, "**/*.rb") + else + $LOAD_PATH.unshift File.expand_path("libraries", cookbook_collection[cookbook].root_dir) + if eager_load_libraries # we have a String or Array and not false + load_libraries_from_cookbook(cookbook, eager_load_libraries) + end + end end @events.library_load_complete end @@ -221,10 +229,8 @@ class Chef raise end - def load_libraries_from_cookbook(cookbook_name) - files_in_cookbook_by_segment(cookbook_name, :libraries).each do |filename| - next unless File.extname(filename) == ".rb" - + def load_libraries_from_cookbook(cookbook_name, globs) + each_file_in_cookbook_by_segment(cookbook_name, :libraries, globs) do |filename| begin logger.trace("Loading cookbook #{cookbook_name}'s library file: #{filename}") Kernel.require(filename) @@ -327,6 +333,22 @@ class Chef cookbook_collection[cookbook].files_for(segment).map { |record| record[:full_path] }.sort end + # Iterates through all files in given cookbook segment, yielding the full path to the file + # if it matches one of the given globs. Returns matching files in lexical sort order. Supports + # extended globbing. The segment should not be included in the glob. + # + def each_file_in_cookbook_by_segment(cookbook, segment, globs) + cookbook_collection[cookbook].files_for(segment).sort_by { |record| record[:path] }.each do |record| + Array(globs).each do |glob| + target = record[:path].delete_prefix("#{segment}/") + if File.fnmatch(glob, target, File::FNM_PATHNAME | File::FNM_EXTGLOB | File::FNM_DOTMATCH) + yield record[:full_path] + break + end + end + end + end + # Yields the name, as a symbol, of each cookbook depended on by # +cookbook_name+ in lexical sort order. def each_cookbook_dep(cookbook_name, &block) diff --git a/spec/integration/client/client_spec.rb b/spec/integration/client/client_spec.rb index 24ec35d5e0..0b206623b1 100644 --- a/spec/integration/client/client_spec.rb +++ b/spec/integration/client/client_spec.rb @@ -11,8 +11,8 @@ describe "chef-client" do File.join(CHEF_SPEC_DATA, "recipes.tgz") end - def start_tiny_server(server_opts = {}) - @server = TinyServer::Manager.new(server_opts) + def start_tiny_server(**server_opts) + @server = TinyServer::Manager.new(**server_opts) @server.start @api = TinyServer::API.instance @api.clear @@ -739,4 +739,125 @@ EOM expect(command.stdout).to include("NOFORK") end end + + when_the_repository "has an eager_load_libraries false cookbook" do + before do + file "cookbooks/x/libraries/require_me.rb", <<~'EOM' + class RequireMe + end + EOM + file "cookbooks/x/recipes/default.rb", <<~'EOM' + # shouldn't be required by default + raise "boom" if defined?(RequireMe) + require "require_me" + # should be in the LOAD_PATH + raise "boom" unless defined?(RequireMe) + EOM + file "cookbooks/x/metadata.rb", <<~EOM + name 'x' + version '0.0.1' + eager_load_libraries false + EOM + file "config/client.rb", <<~EOM + local_mode true + cookbook_path "#{path_to("cookbooks")}" + EOM + end + + it "should not eagerly load the library" do + result = shell_out("#{chef_client} -c \"#{path_to("config/client.rb")}\" -o 'x::default'", cwd: chef_dir) + result.error! + end + end + + when_the_repository "has an eager_load_libraries cookbook with a default hook" do + before do + file "cookbooks/x/libraries/aa_require_me.rb", <<~'EOM' + class RequireMe + end + EOM + file "cookbooks/x/libraries/default.rb", <<~'EOM' + raise "boom" if defined?(RequireMe) + require "aa_require_me" + EOM + file "cookbooks/x/libraries/nope/default.rb", <<~'EOM' + raise "boom" # this should never be required + EOM + file "cookbooks/x/recipes/default.rb", <<~'EOM' + raise "boom" unless defined?(RequireMe) + EOM + file "cookbooks/x/metadata.rb", <<~EOM + name 'x' + version '0.0.1' + eager_load_libraries "default.rb" + EOM + file "config/client.rb", <<~EOM + local_mode true + cookbook_path "#{path_to("cookbooks")}" + always_dump_stacktrace true + EOM + end + + it "should properly load the library via the hook" do + result = shell_out("#{chef_client} -c \"#{path_to("config/client.rb")}\" -o 'x::default'", cwd: chef_dir) + result.error! + end + end + + when_the_repository "has an eager_load_libraries false cookbook" do + before do + # this is loaded by default.rb + file "cookbooks/x/libraries/aa_require_me.rb", <<~'EOM' + class RequireMe + end + EOM + # this is loaded by eager_load_libraries + file "cookbooks/x/libraries/default.rb", <<~'EOM' + raise "boom" if defined?(RequireMe) + require "aa_require_me" + EOM + # this is loaded by the recipe using the LOAD_PATH + file "cookbooks/x/libraries/require_me.rb", <<~'EOM' + class RequireMe4 + end + EOM + # these two are loaded by eager_load_libraries glob + file "cookbooks/x/libraries/loadme/foo/require_me.rb", <<~'EOM' + class RequireMe2 + end + EOM + file "cookbooks/x/libraries/loadme/require_me.rb", <<~'EOM' + class RequireMe3 + end + EOM + # this should nevrer be loaded + file "cookbooks/x/libraries/nope/require_me.rb", <<~'EOM' + raise "boom" # this should never be required + EOM + file "cookbooks/x/recipes/default.rb", <<~'EOM' + # all these are loaded by the eager_load_libraries + raise "boom" unless defined?(RequireMe) + raise "boom" unless defined?(RequireMe2) + raise "boom" unless defined?(RequireMe3) + raise "boom" if defined?(RequireMe4) + require "require_me" + raise "boom" unless defined?(RequireMe4) + EOM + file "cookbooks/x/metadata.rb", <<~EOM + name 'x' + version '0.0.1' + eager_load_libraries [ "default.rb", "loadme/**/*.rb" ] + EOM + file "config/client.rb", <<~EOM + local_mode true + cookbook_path "#{path_to("cookbooks")}" + always_dump_stacktrace true + EOM + end + + it "should not eagerly load the library" do + result = shell_out("#{chef_client} -c \"#{path_to("config/client.rb")}\" -o 'x::default'", cwd: chef_dir) + result.error! + end + end end diff --git a/spec/integration/knife/cookbook_show_spec.rb b/spec/integration/knife/cookbook_show_spec.rb index 32b1324a0a..8d4b4b660f 100644 --- a/spec/integration/knife/cookbook_show_spec.rb +++ b/spec/integration/knife/cookbook_show_spec.rb @@ -1,5 +1,5 @@ # -# Copyright:: Copyright 2013-2016, Chef Software Inc. +# Copyright:: Copyright 2013-2020, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -43,25 +43,26 @@ describe "knife cookbook show", :workstation do metadata: chef_versions: dependencies: - description: + description: + eager_load_libraries: true gems: - issues_url: - license: All rights reserved - long_description: - maintainer: - maintainer_email: - name: x + issues_url: + license: All rights reserved + long_description: + maintainer: + maintainer_email: + name: x ohai_versions: platforms: - privacy: false + privacy: false providing: x: >= 0.0.0 x::x: >= 0.0.0 recipes: - x: - x::x: - source_url: - version: 1.0.0 + x: + x::x: + source_url: + version: 1.0.0 name: x-1.0.0 recipes: checksum: 4631b34cf58de10c5ef1304889941b2e @@ -69,7 +70,7 @@ describe "knife cookbook show", :workstation do path: recipes/default.rb specificity: default url: http://127.0.0.1:8900/file_store/checksums/4631b34cf58de10c5ef1304889941b2e - + checksum: d41d8cd98f00b204e9800998ecf8427e name: recipes/x.rb path: recipes/x.rb @@ -89,25 +90,26 @@ describe "knife cookbook show", :workstation do knife("cookbook show x 1.0.0 metadata").should_succeed <<~EOM chef_versions: dependencies: - description: + description: + eager_load_libraries: true gems: - issues_url: - license: All rights reserved - long_description: - maintainer: - maintainer_email: - name: x + issues_url: + license: All rights reserved + long_description: + maintainer: + maintainer_email: + name: x ohai_versions: platforms: - privacy: false + privacy: false providing: x: >= 0.0.0 x::x: >= 0.0.0 recipes: - x: - x::x: - source_url: - version: 1.0.0 + x: + x::x: + source_url: + version: 1.0.0 EOM end @@ -118,7 +120,7 @@ describe "knife cookbook show", :workstation do path: recipes/default.rb specificity: default url: http://127.0.0.1:8900/file_store/checksums/4631b34cf58de10c5ef1304889941b2e - + checksum: d41d8cd98f00b204e9800998ecf8427e name: recipes/x.rb path: recipes/x.rb diff --git a/spec/support/shared/integration/knife_support.rb b/spec/support/shared/integration/knife_support.rb index 8de665c370..b1e01e97f2 100644 --- a/spec/support/shared/integration/knife_support.rb +++ b/spec/support/shared/integration/knife_support.rb @@ -1,6 +1,6 @@ # # Author:: John Keiser () -# Copyright:: Copyright 2013-2017, Chef Software Inc. +# Copyright:: Copyright 2013-2020, Chef Software Inc. # License:: Apache License, Version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -170,21 +170,24 @@ module KnifeSupport def should_result_in(expected) expected[:stdout] = "" unless expected[:stdout] + expected[:stdout] = expected[:stdout].is_a?(String) ? expected[:stdout].gsub(/[ \t\f\v]+$/, "") : expected[:stdout] expected[:stderr] = "" unless expected[:stderr] + expected[:stderr] = expected[:stderr].is_a?(String) ? expected[:stderr].gsub(/[ \t\f\v]+$/, "") : expected[:stderr] expected[:exit_code] = 0 unless expected[:exit_code] # TODO make this go away stderr_actual = @stderr.sub(/^WARNING: No knife configuration file found\n/, "") - - if expected[:stderr].is_a?(Regexp) - expect(stderr_actual).to match(expected[:stderr]) - else - expect(stderr_actual).to eq(expected[:stderr]) - end + stderr_actual = stderr_actual.gsub(/[ \t\f\v]+$/, "") stdout_actual = @stdout + stdout_actual = stdout_actual.gsub(/[ \t\f\v]+$/, "") if ChefUtils.windows? stderr_actual = stderr_actual.gsub("\r\n", "\n") stdout_actual = stdout_actual.gsub("\r\n", "\n") end + if expected[:stderr].is_a?(Regexp) + expect(stderr_actual).to match(expected[:stderr]) + else + expect(stderr_actual).to eq(expected[:stderr]) + end expect(@exit_code).to eq(expected[:exit_code]) if expected[:stdout].is_a?(Regexp) expect(stdout_actual).to match(expected[:stdout]) diff --git a/spec/unit/cookbook/metadata_spec.rb b/spec/unit/cookbook/metadata_spec.rb index 3c52198310..afb293cd37 100644 --- a/spec/unit/cookbook/metadata_spec.rb +++ b/spec/unit/cookbook/metadata_spec.rb @@ -29,7 +29,7 @@ describe Chef::Cookbook::Metadata do @fields = %i{name description long_description maintainer maintainer_email license platforms dependencies providing recipes version source_url issues_url - privacy ohai_versions chef_versions gems} + privacy ohai_versions chef_versions gems eager_load_libraries} end it "does not depend on object identity for equality" do @@ -127,6 +127,10 @@ describe Chef::Cookbook::Metadata do it "is not private" do expect(metadata.privacy).to eq(false) end + + it "has eager_load_libraries set to true" do + expect(metadata.eager_load_libraries).to eq(true) + end end describe "validation" do @@ -179,6 +183,7 @@ describe Chef::Cookbook::Metadata do source_url: "http://example.com", issues_url: "http://example.com/issues", privacy: true, + eager_load_libraries: false, } params.sort_by(&:to_s).each do |field, field_value| describe field do @@ -269,8 +274,33 @@ describe Chef::Cookbook::Metadata do it "errors on self-dependencies" do metadata.name("foo") - expect { metadata.depends("foo") }.to raise_error - # FIXME: add the error type + expect { metadata.depends("foo") }.to raise_error(RuntimeError, /Cookbook depends on itself/) + end + end + + describe "eager_load_libraries" do + it "can be set to true" do + metadata.send(:eager_load_libraries, true) + expect(metadata.send(:eager_load_libraries)).to eql(true) + end + + it "can be set to false" do + metadata.send(:eager_load_libraries, false) + expect(metadata.send(:eager_load_libraries)).to eql(false) + end + + it "can be set to a string" do + metadata.send(:eager_load_libraries, "default.rb") + expect(metadata.send(:eager_load_libraries)).to eql("default.rb") + end + + it "can be set to an array" do + metadata.send(:eager_load_libraries, [ "default.rb", "foo/*/**.rb" ]) + expect(metadata.send(:eager_load_libraries)).to eql([ "default.rb", "foo/*/**.rb" ]) + end + + it "cannot be set to a number" do + expect { metadata.send(:eager_load_libraries, 1) }.to raise_error(Chef::Exceptions::ValidationFailed) end end @@ -452,6 +482,7 @@ describe Chef::Cookbook::Metadata do metadata.chef_version "< 12.5.1", ">= 12.2.1" metadata.ohai_version "< 7.5.0", ">= 7.1.0" metadata.ohai_version "< 8.6.0", ">= 8.0.1" + metadata.eager_load_libraries [ "default.rb", "foo/*/**.rb" ] end it "should produce the same output from to_json and Chef::JSONCompat" do diff --git a/spec/unit/knife/cookbook_show_spec.rb b/spec/unit/knife/cookbook_show_spec.rb index b1e7d60e5b..9fe2e9ff90 100644 --- a/spec/unit/knife/cookbook_show_spec.rb +++ b/spec/unit/knife/cookbook_show_spec.rb @@ -1,6 +1,6 @@ # # Author:: Adam Jacob () -# Copyright:: Copyright 2008-2017, Chef Software Inc. +# Copyright:: Copyright 2008-2020, Chef Software Inc. # License:: Apache License, version 2.0 # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -109,6 +109,7 @@ describe Chef::Knife::CookbookShow do "metadata" => { "name" => nil, "description" => "", + "eager_load_libraries" => true, "long_description" => "", "maintainer" => "", "maintainer_email" => "", -- cgit v1.2.1 From 55251665bd38853466e63d8d649a744662e8884d Mon Sep 17 00:00:00 2001 From: Lamont Granquist Date: Fri, 3 Apr 2020 16:48:56 -0700 Subject: Fix chefspec monkeypatching madness Apparently chefspec considers this a public API (it isn't) Signed-off-by: Lamont Granquist --- lib/chef/run_context/cookbook_compiler.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/chef/run_context/cookbook_compiler.rb b/lib/chef/run_context/cookbook_compiler.rb index e91c440ff1..8e8c8c6e25 100644 --- a/lib/chef/run_context/cookbook_compiler.rb +++ b/lib/chef/run_context/cookbook_compiler.rb @@ -102,7 +102,7 @@ class Chef cookbook_order.each do |cookbook| eager_load_libraries = cookbook_collection[cookbook].metadata.eager_load_libraries if eager_load_libraries == true # actully true, not truthy - load_libraries_from_cookbook(cookbook, "**/*.rb") + load_libraries_from_cookbook(cookbook) else $LOAD_PATH.unshift File.expand_path("libraries", cookbook_collection[cookbook].root_dir) if eager_load_libraries # we have a String or Array and not false @@ -229,7 +229,7 @@ class Chef raise end - def load_libraries_from_cookbook(cookbook_name, globs) + def load_libraries_from_cookbook(cookbook_name, globs = "*/*.rb") each_file_in_cookbook_by_segment(cookbook_name, :libraries, globs) do |filename| begin logger.trace("Loading cookbook #{cookbook_name}'s library file: #{filename}") -- cgit v1.2.1 From b5dc6520fdf7a371f4add52734f88bcd0daf74bc Mon Sep 17 00:00:00 2001 From: Lamont Granquist Date: Fri, 3 Apr 2020 17:01:40 -0700 Subject: fix accidental mangling of the glob pattern Signed-off-by: Lamont Granquist --- lib/chef/run_context/cookbook_compiler.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/chef/run_context/cookbook_compiler.rb b/lib/chef/run_context/cookbook_compiler.rb index 8e8c8c6e25..4dda6aeb2e 100644 --- a/lib/chef/run_context/cookbook_compiler.rb +++ b/lib/chef/run_context/cookbook_compiler.rb @@ -229,7 +229,7 @@ class Chef raise end - def load_libraries_from_cookbook(cookbook_name, globs = "*/*.rb") + def load_libraries_from_cookbook(cookbook_name, globs = "**/*.rb") each_file_in_cookbook_by_segment(cookbook_name, :libraries, globs) do |filename| begin logger.trace("Loading cookbook #{cookbook_name}'s library file: #{filename}") -- cgit v1.2.1