Resource type defaults cleanup

This is again done to make support for hiera easier.
The way we were handling lookup of resource defaults
was over-complicated.

This does a decent bit of cleanup overall, but primarily
focused on resource type defaults and how they get
set during compilation.

Signed-off-by: Luke Kanies <luke@puppetlabs.com>
Reviewed-by: Nick Lewis <nick@puppetlabs.com>

3 files changed, 54 insertions, 23 deletions

diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb
index f43a31285..06cd80a1e 100644
--- a/lib/puppet/parser/compiler.rb
+++ b/lib/puppet/parser/compiler.rb
@@ -139,19 +139,21 @@ class Puppet::Parser::Compiler
   # evaluated later in the process.
   def evaluate_classes(classes, scope, lazy_evaluate = true)
     raise Puppet::DevError, "No source for scope passed to evaluate_classes" unless scope.source
-    param_classes = nil
+    class_parameters = nil
     # if we are a param class, save the classes hash
     # and transform classes to be the keys
     if classes.class == Hash
-      param_classes = classes
+      class_parameters = classes
       classes = classes.keys
     end
     classes.each do |name|
       # If we can find the class, then make a resource that will evaluate it.
       if klass = scope.find_hostclass(name)
 
-        if param_classes
-          resource = klass.ensure_in_catalog(scope, param_classes[name] || {})
+        # If parameters are passed, then attempt to create a duplicate resource
+        # so the appropriate error is thrown.
+        if class_parameters
+          resource = klass.ensure_in_catalog(scope, class_parameters[name] || {})
         else
           next if scope.class_scope(klass)
           resource = klass.ensure_in_catalog(scope)
diff --git a/lib/puppet/resource.rb b/lib/puppet/resource.rb
index 59e387d00..217eb11c8 100644
--- a/lib/puppet/resource.rb
+++ b/lib/puppet/resource.rb
@@ -343,6 +343,26 @@ class Puppet::Resource
     [ type, title ].join('/')
   end
 
+  def set_default_parameters(scope)
+    return [] unless resource_type and resource_type.respond_to?(:arguments)
+
+    result = []
+
+    resource_type.arguments.each do |param, default|
+      param = param.to_sym
+      next if parameters.include?(param)
+      unless is_a?(Puppet::Parser::Resource)
+        fail Puppet::DevError, "Cannot evaluate default parameters for #{self} - not a parser resource"
+      end
+
+      next if default.nil?
+
+      self[param] = default.safeevaluate(scope)
+      result << param
+    end
+    result
+  end
+
   def to_resource
     self
   end
@@ -351,6 +371,19 @@ class Puppet::Resource
     resource_type.valid_parameter?(name)
   end
 
+  # Verify that all required arguments are either present or
+  # have been provided with defaults.
+  # Must be called after 'set_default_parameters'.  We can't join the methods
+  # because Type#set_parameters needs specifically ordered behavior.
+  def validate_complete
+    return unless resource_type and resource_type.respond_to?(:arguments)
+
+    resource_type.arguments.each do |param, default|
+      param = param.to_sym
+      fail Puppet::ParseError, "Must pass #{param} to #{self}" unless parameters.include?(param)
+    end
+  end
+
   def validate_parameter(name)
     raise ArgumentError, "Invalid parameter #{name}" unless valid_parameter?(name)
   end
diff --git a/lib/puppet/resource/type.rb b/lib/puppet/resource/type.rb
index 7b251e8c7..ca6e8b53b 100644
--- a/lib/puppet/resource/type.rb
+++ b/lib/puppet/resource/type.rb
@@ -158,11 +158,7 @@ class Puppet::Resource::Type
       return resource
     end
     resource = Puppet::Parser::Resource.new(resource_type, name, :scope => scope, :source => self)
-    if parameters
-      parameters.each do |k,v|
-        resource.set_parameter(k,v)
-      end
-    end
+    assign_parameter_values(parameters, resource)
     instantiate_resource(scope, resource)
     scope.compiler.add_resource(scope, resource)
     resource
@@ -188,6 +184,14 @@ class Puppet::Resource::Type
     @name.is_a?(Regexp)
   end
 
+  def assign_parameter_values(parameters, resource)
+    return unless parameters
+    scope = resource.scope || {}
+    arguments.merge(parameters).each do |name, default|
+      resource.set_parameter name, default
+    end
+  end
+
   # MQR TODO:
   #
   # The change(s) introduced by the fix for #4270 are mostly silly & should be 
@@ -243,22 +247,14 @@ class Puppet::Resource::Type
       scope["caller_module_name"] = caller_name
     end
     scope.class_set(self.name,scope) if hostclass? or node?
-    # Verify that all required arguments are either present or
-    # have been provided with defaults.
-    arguments.each do |param, default|
-      param = param.to_sym
-      next if set.include?(param)
-
-      # Even if 'default' is a false value, it's an AST value, so this works fine
-      fail Puppet::ParseError, "Must pass #{param} to #{resource.ref}" unless default
 
-      value = default.safeevaluate(scope)
-      scope[param.to_s] = value
-
-      # Set it in the resource, too, so the value makes it to the client.
-      resource[param] = value
-    end
+    # Evaluate the default parameters, now that all other variables are set
+    default_params = resource.set_default_parameters(scope)
+    default_params.each { |param| scope[param.to_s] = resource[param] }
 
+    # This has to come after the above parameters so that default values
+    # can use their values
+    resource.validate_complete
   end
 
   # Check whether a given argument is valid.