diff options
author | John Keiser <john@johnkeiser.com> | 2015-04-18 15:23:44 -0700 |
---|---|---|
committer | John Keiser <john@johnkeiser.com> | 2015-05-13 13:42:32 -0700 |
commit | a6d5241a96a7882166af849f529b2a2b8bab5ab4 (patch) | |
tree | f4a7c3735db3698d0712536d8cc58d5166ce0173 | |
parent | c46776d45cd5a732441a4dc0bf0552e71bb500ee (diff) | |
download | chef-a6d5241a96a7882166af849f529b2a2b8bab5ab4.tar.gz |
Deprecate automatic method_missing and Chef::Resource lookup
- Declare all resource DSL as methods on Chef::DSL::Resources
- Declare all definition DSL as methods on Chef::DSL::Definitions
-rw-r--r-- | lib/chef/dsl/definitions.rb | 44 | ||||
-rw-r--r-- | lib/chef/dsl/recipe.rb | 85 | ||||
-rw-r--r-- | lib/chef/dsl/resources.rb | 19 | ||||
-rw-r--r-- | lib/chef/mixin/provides.rb | 3 | ||||
-rw-r--r-- | lib/chef/resource.rb | 33 | ||||
-rw-r--r-- | lib/chef/resource/lwrp_base.rb | 68 | ||||
-rw-r--r-- | lib/chef/resource_definition.rb | 1 | ||||
-rw-r--r-- | lib/chef/resource_resolver.rb | 68 | ||||
-rw-r--r-- | spec/unit/lwrp_spec.rb | 2 | ||||
-rw-r--r-- | spec/unit/recipe_spec.rb | 2 |
10 files changed, 240 insertions, 85 deletions
diff --git a/lib/chef/dsl/definitions.rb b/lib/chef/dsl/definitions.rb new file mode 100644 index 0000000000..1358f67720 --- /dev/null +++ b/lib/chef/dsl/definitions.rb @@ -0,0 +1,44 @@ +class Chef + module DSL + # + # Module containing a method for each declared definition + # + # Depends on declare_resource(name, created_at, &block) + # + # @api private + # + module Definitions + def self.add_definition(dsl_name) + module_eval <<-EOM, __FILE__, __LINE__+1 + def #{dsl_name}(*args, &block) + evaluate_resource_definition(#{dsl_name.inspect}, *args, &block) + end + EOM + end + + # @api private + def has_resource_definition?(name) + run_context.definitions.has_key?(name) + end + + # Processes the arguments and block as a resource definition. + # + # @api private + def evaluate_resource_definition(definition_name, *args, &block) + + # This dupes the high level object, but we still need to dup the params + new_def = run_context.definitions[definition_name].dup + + new_def.params = new_def.params.dup + new_def.node = run_context.node + # This sets up the parameter overrides + new_def.instance_eval(&block) if block + + new_recipe = Chef::Recipe.new(cookbook_name, recipe_name, run_context) + new_recipe.params = new_def.params + new_recipe.params[:name] = args[0] + new_recipe.instance_eval(&new_def.recipe) + end + end + end +end diff --git a/lib/chef/dsl/recipe.rb b/lib/chef/dsl/recipe.rb index c22f053292..e17e008432 100644 --- a/lib/chef/dsl/recipe.rb +++ b/lib/chef/dsl/recipe.rb @@ -21,6 +21,8 @@ require 'chef/mixin/convert_to_class_name' require 'chef/exceptions' require 'chef/resource_builder' require 'chef/mixin/shell_out' +require 'chef/dsl/resources' +require 'chef/dsl/definitions' class Chef module DSL @@ -31,49 +33,62 @@ class Chef module Recipe include Chef::Mixin::ShellOut - include Chef::Mixin::ConvertToClassName + # method_missing must live for backcompat purposes until Chef 13. def method_missing(method_symbol, *args, &block) - # If we have a definition that matches, we want to use that instead. This should - # let you do some really crazy over-riding of "native" types, if you really want - # to. - if has_resource_definition?(method_symbol) - evaluate_resource_definition(method_symbol, *args, &block) - elsif have_resource_class_for?(method_symbol) - # Otherwise, we're rocking the regular resource call route. - declare_resource(method_symbol, args[0], caller[0], &block) - else - begin - super - rescue NoMethodError - raise NoMethodError, "No resource or method named `#{method_symbol}' for #{describe_self_for_error}" - rescue NameError - raise NameError, "No resource, method, or local variable named `#{method_symbol}' for #{describe_self_for_error}" - end + # + # If there is already DSL for this, someone must have called + # method_missing manually. Not a fan. Not. A. Fan. + # + if respond_to?(method_symbol) + Chef::Log.warn("Calling method_missing(#{method_symbol.inspect}) directly is deprecated in Chef 12 and will be removed in Chef 13.") + Chef::Log.warn("Use public_send() or send() instead.") + return send(method_symbol, *args, &block) end - end - - def has_resource_definition?(name) - run_context.definitions.has_key?(name) - end - - # Processes the arguments and block as a resource definition. - def evaluate_resource_definition(definition_name, *args, &block) - # This dupes the high level object, but we still need to dup the params - new_def = run_context.definitions[definition_name].dup + # + # If a definition exists, then Chef::DSL::Definitions.add_definition was + # never called. DEPRECATED. + # + if run_context.definitions.has_key?(method_symbol.to_sym) + Chef::Log.warn("Definition #{method_symbol} (#{run_context.definitions[method_symbol.to_sym]}) was added to the run_context without calling Chef::DSL::Definitions.add_definition(#{method_symbol.to_sym.inspect}). This will become required in Chef 13.") + Chef::DSL::Definitions.add_definition(method_symbol) + return send(method_symbol, *args, &block) + end - new_def.params = new_def.params.dup - new_def.node = run_context.node - # This sets up the parameter overrides - new_def.instance_eval(&block) if block + # + # See if the resource exists anyway. If the user had set + # Chef::Resource::Blah = <resource>, a deprecation warning will be + # emitted and the DSL method 'blah' will be added to the DSL. + # + resource_class = Chef::ResourceResolver.new(run_context ? run_context.node : nil, method_symbol).resolve + if resource_class + # + # If the DSL method was *not* added, this is the case where the + # matching class implements 'provides?' and matches resources that it + # never declared "provides" for (which means we would never have + # created DSL). Anything where we don't create DSL is deprecated. + # + if !respond_to?(method_symbol) + Chef::Log.warn("#{resource_class} is marked as providing DSL #{method_symbol}, but provides #{method_symbol.inspect} was never called!") + Chef::Log.warn("In Chef 13, this will break: you must call provides to mark the names you provide, even if you also override provides? yourself.") + Chef::DSL::Resources.add_resource_dsl(method_symbol) + end + return send(method_symbol, *args, &block) + end - new_recipe = Chef::Recipe.new(cookbook_name, recipe_name, run_context) - new_recipe.params = new_def.params - new_recipe.params[:name] = args[0] - new_recipe.instance_eval(&new_def.recipe) + begin + super + rescue NoMethodError + raise NoMethodError, "No resource or method named `#{method_symbol}' for #{describe_self_for_error}" + rescue NameError + raise NameError, "No resource, method, or local variable named `#{method_symbol}' for #{describe_self_for_error}" + end end + include Chef::DSL::Resources + include Chef::DSL::Definitions + # # Instantiates a resource (via #build_resource), then adds it to the # resource collection. Note that resource classes are looked up directly, diff --git a/lib/chef/dsl/resources.rb b/lib/chef/dsl/resources.rb new file mode 100644 index 0000000000..dfa0788879 --- /dev/null +++ b/lib/chef/dsl/resources.rb @@ -0,0 +1,19 @@ +class Chef + module DSL + # + # Module containing a method for each globally declared Resource + # + # Depends on declare_resource(name, created_at, &block) + # + # @api private + module Resources + def self.add_resource_dsl(dsl_name) + module_eval <<-EOM, __FILE__, __LINE__+1 + def #{dsl_name}(name, created_at=nil, &block) + declare_resource(#{dsl_name.inspect}, name, created_at || caller[0], &block) + end + EOM + end + end + end +end diff --git a/lib/chef/mixin/provides.rb b/lib/chef/mixin/provides.rb index bc25af62b2..d71097ca4b 100644 --- a/lib/chef/mixin/provides.rb +++ b/lib/chef/mixin/provides.rb @@ -23,7 +23,8 @@ class Chef node_map.set(short_name, true, opts, &block) end - # provides a node on the resource (early binding) + # Check whether this resource provides the resource_name DSL for the given + # node def provides?(node, resource_name) resource_name = resource_name.resource_name if resource_name.is_a?(Chef::Resource) node_map.get(node, resource_name) diff --git a/lib/chef/resource.rb b/lib/chef/resource.rb index 3d319617d1..ef0b59e4dc 100644 --- a/lib/chef/resource.rb +++ b/lib/chef/resource.rb @@ -22,6 +22,7 @@ require 'chef/dsl/platform_introspection' require 'chef/dsl/data_query' require 'chef/dsl/registry_helper' require 'chef/dsl/reboot_pending' +require 'chef/dsl/resources' require 'chef/mixin/convert_to_class_name' require 'chef/guard_interpreter/resource_guard_interpreter' require 'chef/resource/conditional' @@ -820,7 +821,10 @@ class Chef # # @return [String] The DSL name of this resource. def self.dsl_name - convert_to_snake_case(name, 'Chef::Resource') + if name + name = self.name.split('::')[-1] + convert_to_snake_case(name) + end end # @@ -969,6 +973,19 @@ class Chef end end + # Cause each subclass to register itself with the DSL + def self.inherited(subclass) + super + if subclass.dsl_name + subclass.provides subclass.dsl_name.to_sym + end + end + + def self.provides(name, *args, &block) + super + Chef::DSL::Resources.add_resource_dsl(name) + end + # Helper for #notifies def validate_resource_spec!(resource_spec) run_context.resource_collection.validate_lookup_spec!(resource_spec) @@ -1099,25 +1116,23 @@ class Chef # === Returns # <Chef::Resource>:: returns the proper Chef::Resource class def self.resource_for_node(short_name, node) - require 'chef/resource_resolver' klass = Chef::ResourceResolver.new(node, short_name).resolve raise Chef::Exceptions::NoSuchResourceType.new(short_name, node) if klass.nil? klass end + # # Returns the class of a Chef::Resource based on the short name + # Only returns the *canonical* class with the given name, not the one that + # would be picked by the ResourceResolver. + # # ==== Parameters # short_name<Symbol>:: short_name of the resource (ie :directory) # # === Returns # <Chef::Resource>:: returns the proper Chef::Resource class def self.resource_matching_short_name(short_name) - begin - rname = convert_to_class_name(short_name.to_s) - Chef::Resource.const_get(rname) - rescue NameError - nil - end + Chef::ResourceResolver.new(Chef::Node.new, short_name).resolve end private @@ -1135,3 +1150,5 @@ class Chef end end end + +require 'chef/resource_resolver' diff --git a/lib/chef/resource/lwrp_base.rb b/lib/chef/resource/lwrp_base.rb index ce72e98028..f702db5c96 100644 --- a/lib/chef/resource/lwrp_base.rb +++ b/lib/chef/resource/lwrp_base.rb @@ -35,32 +35,64 @@ class Chef # Evaluates the LWRP resource file and instantiates a new Resource class. def self.build_from_file(cookbook_name, filename, run_context) - resource_class = nil - rname = filename_to_qualified_string(cookbook_name, filename) - - class_name = convert_to_class_name(rname) - if Resource.const_defined?(class_name, false) - Chef::Log.info("#{class_name} light-weight resource is already initialized -- Skipping loading #{filename}!") - Chef::Log.debug("Overriding already defined LWRPs is not supported anymore starting with Chef 12.") - resource_class = Resource.const_get(class_name) - else - resource_class = Class.new(self) + resource_name = filename_to_qualified_string(cookbook_name, filename) + + node = run_context ? run_context.node : Chef::Node.new + existing_class = Chef::ResourceResolver.new(node, resource_name).resolve + + if existing_class + Chef::Log.info("Skipping LWRP resource #{filename} from cookbook #{cookbook_name}. #{existing_class} already defined.") + Chef::Log.debug("Overriding resources with LWRPs is not supported anymore starting with Chef 12.") + return existing_class + end - Chef::Resource.const_set(class_name, resource_class) - resource_class.resource_name = rname - resource_class.run_context = run_context - resource_class.class_from_file(filename) + # We load the class first to give it a chance to set its own name + resource_class = Class.new(self) + resource_class.resource_name = resource_name + resource_class.run_context = run_context + resource_class.class_from_file(filename) - Chef::Log.debug("Loaded contents of #{filename} into a resource named #{rname} defined in Chef::Resource::#{class_name}") + # Respect resource_name set inside the LWRP + resource_class.instance_eval do + define_method(:to_s) do + "LWRP #{resource_name} from cookbook #{cookbook_name}" + end + define_method(:inspect) { to_s } end + Chef::Log.debug("Loaded contents of #{filename} into #{resource_class}") + + create_deprecated_class_in_chef_resource(resource_class) + resource_class end - # Set the resource name for this LWRP + def self.create_deprecated_class_in_chef_resource(resource_class) + # Create a class in Chef::Resource::MyResource with deprecation + # warnings if you try to instantiate or inherit it (for Chef 12 compatibility) + class_name = convert_to_class_name(resource_class.resource_name) + if Chef::Resource.const_defined?(class_name, false) + Chef::Log.debug "#{class_name} already exists! Cannot create deprecation class #{resource_class}" + else + Chef::Resource.const_set(class_name, Class.new(resource_class) do + self.resource_name = resource_class.resource_name + self.run_context = resource_class.run_context + define_method(:initialize) do |*args, &block| + Chef::Log.warn("Using an LWRP by its name (#{class_name}) directly is no longer supported in Chef 12 and will be removed.") + super(*args, &block) + end + define_singleton_method(:inherited) do |*args, &block| + Chef::Log.warn("Using an LWRP by its name (#{class_name}) directly is no longer supported in Chef 12 and will be removed.") + super(*args, &block) + end + end) + end + Chef::Resource.const_get(class_name) + end + def self.resource_name(arg = NULL_ARG) if arg.equal?(NULL_ARG) - @resource_name + @resource_name || dsl_name else @resource_name = arg end @@ -127,7 +159,7 @@ class Chef end def self.node - run_context.node + run_context ? run_context.node : nil end def self.lazy(&block) diff --git a/lib/chef/resource_definition.rb b/lib/chef/resource_definition.rb index 9d6844129c..cffabb6786 100644 --- a/lib/chef/resource_definition.rb +++ b/lib/chef/resource_definition.rb @@ -50,6 +50,7 @@ class Chef else raise ArgumentError, "You must pass a block to a definition." end + Chef::DSL::Definitions.add_definition(name) true end diff --git a/lib/chef/resource_resolver.rb b/lib/chef/resource_resolver.rb index ff9d7aeb9f..ac6b3d8839 100644 --- a/lib/chef/resource_resolver.rb +++ b/lib/chef/resource_resolver.rb @@ -18,9 +18,11 @@ require 'chef/exceptions' require 'chef/platform/resource_priority_map' +require 'chef/mixin/convert_to_class_name' class Chef class ResourceResolver + include Chef::Mixin::ConvertToClassName attr_reader :node attr_reader :resource @@ -37,23 +39,6 @@ class Chef end def resolve - maybe_dynamic_resource_resolution(resource) || - maybe_chef_platform_lookup(resource) - end - - # this cut looks at if the resource can handle the resource type on the node - def enabled_handlers - @enabled_handlers ||= - resources.select do |klass| - klass.provides?(node, resource) - end.sort {|a,b| a.to_s <=> b.to_s } - @enabled_handlers - end - - private - - # try dynamically finding a resource based on querying the resources to see what they support - def maybe_dynamic_resource_resolution(resource) # log this so we know what resources will work for the generic resource on the node (early cut) Chef::Log.debug "resources for generic #{resource} resource enabled on node include: #{enabled_handlers}" @@ -61,7 +46,7 @@ class Chef # enabled on the node to handle the why-run use case. handlers = enabled_handlers - if handlers.count >= 2 + if handlers.size >= 2 # this magic stack ranks the resources by where they appear in the resource_priority_map priority_list = [ get_priority_array(node, resource) ].flatten.compact handlers = handlers.sort_by { |x| i = priority_list.index x; i.nil? ? Float::INFINITY : i } @@ -70,7 +55,7 @@ class Chef # entry for this resource is missing -- we should probably raise here and force resolution of the ambiguity. Chef::Log.warn "Ambiguous resource precedence: #{handlers}, please use Chef.set_resource_priority_array to provide determinism" end - handlers = [ handlers.first ] + handlers = handlers[0..0] end Chef::Log.debug "resources that survived replacement include: #{handlers}" @@ -84,9 +69,48 @@ class Chef handlers[0] end - # try the old static lookup of resources by mangling name to resource klass - def maybe_chef_platform_lookup(resource) - Chef::Resource.resource_matching_short_name(resource) + # this cut looks at if the resource can handle the resource type on the node + def enabled_handlers + @enabled_handlers ||= begin + enabled_handlers = resources.select do |klass| + klass.provides?(node, resource) + end.sort {|a,b| a.to_s <=> b.to_s } + + # Add Chef::Resource::X as a possibility if it is not a handler already + check_for_deprecated_chef_resource_class(enabled_handlers) + + enabled_handlers + end + end + + private + + # + # Checks if the Chef::Resource::* class corresponding to the DSL name + # exists, emits a deprecation warning, marks it as providing the given + # short name and adds it to the DSL. This is used for method_missing + # deprecation and ResourceResolver checking, when people have created + # anonymous classes and assigned them to Chef::Resource::X. + # + # Returns the matched class, if it exists. + # + # @api private + def check_for_deprecated_chef_resource_class(enabled_handlers) + # If Chef::Resource::MyResource exists, but was not set, it won't have a + # DSL name. Add the DSL method and warn about this pattern. + class_name = convert_to_class_name(resource.to_s) + if Chef::Resource.const_defined?(class_name) + # If Chef::Resource::X already exists, and is *not* already marked as + # providing this resource, mark it as providing the resource and add it + # to the list of handlers for next time. + resource_class = Chef::Resource.const_get(class_name) + if resource_class <= Chef::Resource && !enabled_handlers.include?(resource_class) + Chef::Log.warn("Class #{resource_class} was created with Class.new and assigned directly to a constant (#{resource_class.name} = <class>) rather than being created directly (class #{resource_class.name} < <superclass>).") + Chef::Log.warn("This will no longer work in Chef 13: you can either declare your class directly (in any namespace), or specify 'provides #{resource.to_sym.inspect}' in the class definition.") + resource_class.provides resource.to_sym + enabled_handlers << resource_class + end + end end # dep injection hooks diff --git a/spec/unit/lwrp_spec.rb b/spec/unit/lwrp_spec.rb index ec39174da6..08273f3872 100644 --- a/spec/unit/lwrp_spec.rb +++ b/spec/unit/lwrp_spec.rb @@ -67,6 +67,8 @@ describe "LWRP" do Dir[File.expand_path( "lwrp/resources/*", CHEF_SPEC_DATA)].each do |file| expect(Chef::Log).to receive(:info).with(/Skipping/) + expect(Chef::Log).to receive(:debug).with(/enabled on node/) + expect(Chef::Log).to receive(:debug).with(/survived replacement/) expect(Chef::Log).to receive(:debug).with(/anymore/) Chef::Resource::LWRPBase.build_from_file("lwrp", file, nil) end diff --git a/spec/unit/recipe_spec.rb b/spec/unit/recipe_spec.rb index 7442f4477e..9c4fb49497 100644 --- a/spec/unit/recipe_spec.rb +++ b/spec/unit/recipe_spec.rb @@ -83,7 +83,7 @@ describe Chef::Recipe do it "should require a name argument" do expect { recipe.cat - }.to raise_error(ArgumentError, "You must supply a name when declaring a cat resource") + }.to raise_error(ArgumentError) end it "should allow regular errors (not NameErrors) to pass unchanged" do |