Please review pull request #677: (#13070) Fix undefined method 'downcase' for nil:NilClass Error opened by (jeffmccune)
Description:
Without this patch puppet describe --list blows up with the
mount_providers module with the following error:
root@ubuntu-10:/etc/puppetlabs/puppet/modules# puppet describe mounttab
Could not run: Could not autoload /etc/puppetlabs/puppet/modules/puppetlabs-mount-providers/lib/puppet/type/mountpoint.rb:undefined method 'downcase' for nil:NilClass
The root cause of this problem is Puppet evaluating a block of code that
assumes the name of the class it is evaluated in is named something
likePuppet::Type::Ssh_authorized_key::ParameterProviderThis assumption is wrong, in the case of the mount_providers module the
class generated by genthing() is named simply "Class" without any
delimiters. This causes the split('::')[2] to return a nil object which
cannot respond to the downcase method.This patch solves the problem in two ways. First and foremost, the
scope the block of code is evaluated in is unpredictable without this
patch. This patch moves the block of code into a place where the
assumption being made is in fact correct. By moving the description
assignment outside of the block class_eval()'d by genthing() and into
the scope of the Type#providify() we're matching up assumptions and
providing a consistent context to work with.The second solution is to be much more defensive about calling methods
on split strings. We simply check if we got something back from the
split, and if so call downcase on it. If we don't get something back we
simply call self.to_s which should always return some string and not a
nil object.The spec tests are not changed because the original commit 27057a6
contains the necessary test coverage.
- Opened: Tue Apr 17 18:49:51 UTC 2012
- Based on: puppetlabs:2.7.x (3a4ac604c85952511fd45c2f0699ce956eda3773)
- Requested merge: jeffmccune:ticket/2.7.x/13070_puppetlabs-mount-provider_module_fails_to_autoload (40aa6992f8199f04fe7b661c421945ab4e9f2bc6)
Diff follows:
diff --git a/lib/puppet/type.rb b/lib/puppet/type.rb
index e416665..dfe0dc9 100644
--- a/lib/puppet/type.rb
+++ b/lib/puppet/type.rb
@@ -1498,19 +1498,18 @@ def self.provide(name, options = {}, &block)
def self.providify
return if @paramhash.has_key? :provider
- newparam(:provider) do
- # We're using a hacky way to get the name of our type, since there doesn't
- # seem to be a correct way to introspect this at the time this code is run.
- # We expect that the class in which this code is executed will be something
- # like Puppet::Type::Ssh_authorized_key::ParameterProvider.
- desc <<-EOT
- The specific backend to use for this `#{self.to_s.split('::')[2].downcase}`
- resource. You will seldom need to specify this --- Puppet will usually
- discover the appropriate provider for your platform.
- EOT
-
- # This is so we can refer back to the type to get a list of
- # providers for documentation.
+ # JJM - Be careful with this block's scope... It will be evaluated using
+ # class_eval inside of a generated class from genthing(). There will be no
+ # reference to the type the parameter is associated with when the block is
+ # evaluated. As a result, it is likely much more robust to call methods on
+ # provider_param directly, after the class and instance have been created,
+ # becausue you'll be sure to have your statements evaluated in a known
+ # context instead of somewhere off in the magical world of genthing().
+ # Please see #13070 for more information about why this is a risky section
+ # of user-facing code.
+ provider_param = newparam(:provider) do
+ # This is so we can refer back to the type to get a list of providers for
+ # documentation.
class << self
attr_accessor :parenttype
end
@@ -1552,7 +1551,28 @@ def self.doc
provider
end
end
- end.parenttype = self
+ end
+
+ # JJM - Explicitly set the description. We do this here, outside the
+ # initilization block passed to newparam() because we don't have a binding
+ # to this scope when the block passed to newparam() is evaluated. As a
+ # result, it's difficult to figure out which type this specific parameter
+ # is associated with. Setting the description here, outside of the block,
+ # allows us to much more easily figure out the type the parameter is
+ # associated with because we maintain a binding to this scope.
+ # The ( ... || self.to_s).downcase is simply a defensive mechanism to make
+ # absolutely sure we don't send a message to a nil object as reported in
+ # #13070.
+ provider_param.desc <<-EOT
+ The specific backend to use for this `#{(self.to_s.split('::')[2] || self.to_s).downcase}`
+ resource. You will seldom need to specify this --- Puppet will usually
+ discover the appropriate provider for your platform.
+ EOT
+
+ # Bind the parameter to the type
+ provider_param.parenttype = self
+
+ provider_param
end
def self.unprovide(name)
-- You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to [email protected].
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
