Merge pull request #8902 from joshcooper/deferred

(PUP-9323) Allow deferred functions to adhere to puppet relationships and ordering

Author: joshcooper

lib/puppet/application/apply.rb

@@ -241,7 +241,7 @@ def main
         end
 
         # Resolve all deferred values and replace them / mutate the catalog
-        Puppet::Pops::Evaluator::DeferredResolver.resolve_and_replace(node.facts, catalog, apply_environment)
+        Puppet::Pops::Evaluator::DeferredResolver.resolve_and_replace(node.facts, catalog, apply_environment, Puppet[:preprocess_deferred])
 
         # Translate it to a RAL catalog
         catalog = catalog.to_ral
@@ -350,7 +350,7 @@ def read_catalog(text)
         raise Puppet::Error, _("Could not deserialize catalog from %{format}: %{detail}") % { format: format, detail: detail }, detail.backtrace
       end
       # Resolve all deferred values and replace them / mutate the catalog
-      Puppet::Pops::Evaluator::DeferredResolver.resolve_and_replace(node.facts, catalog, configured_environment)
+      Puppet::Pops::Evaluator::DeferredResolver.resolve_and_replace(node.facts, catalog, configured_environment, Puppet[:preprocess_deferred])
 
       catalog.to_ral
     end

lib/puppet/configurer.rb

@@ -112,7 +112,7 @@ def convert_catalog(result, duration, facts, options = {})
     catalog_conversion_time = thinmark do
       # Will mutate the result and replace all Deferred values with resolved values
       if facts
-        Puppet::Pops::Evaluator::DeferredResolver.resolve_and_replace(facts, result, Puppet.lookup(:current_environment))
+        Puppet::Pops::Evaluator::DeferredResolver.resolve_and_replace(facts, result, Puppet.lookup(:current_environment), Puppet[:preprocess_deferred])
       end
 
       catalog = result.to_ral

lib/puppet/defaults.rb

@@ -2021,6 +2021,16 @@ def self.initialize_default_settings!(settings)
         being evaluated.  This allows you to interactively see exactly
         what is being done.",
     },
+    :preprocess_deferred => {
+      :default => true,
+      :type => :boolean,
+      :desc => "Whether puppet should call deferred functions before applying
+        the catalog. If set to `true`, then all prerequisites needed for the
+        deferred function must be satified prior to puppet running. If set to
+        `false`, then deferred functions will follow puppet relationships and
+        ordering. This allows puppet to install prerequisites needed for a
+        deferred function and call the deferred function in the same run."
+    },
     :summarize => {
         :default  => false,
         :type     => :boolean,

lib/puppet/parameter.rb

@@ -177,15 +177,15 @@ def munge(&block)
     end
 
     # @overload unmunge {|| ... }
-    # Defines an optional method used to convert the parameter value to DSL/string form from an internal form.
+    # Defines an optional method used to convert the parameter value from internal form to DSL/string form.
     # If an `unmunge` method is not defined, the internal form is used.
     # @see munge
-    # @note This adds a method with the name `unmunge` in the created parameter class.
+    # @note This adds a method with the name `unsafe_unmunge` in the created parameter class.
     # @dsl type
     # @api public
     #
     def unmunge(&block)
-      define_method(:unmunge, &block)
+      define_method(:unsafe_unmunge, &block)
     end
 
     # Sets a marker indicating that this parameter is the _namevar_ (unique identifier) of the type
@@ -415,17 +415,30 @@ def unsafe_munge(value)
   # @return [Object] the unmunged value
   #
   def unmunge(value)
+    return value if value.is_a?(Puppet::Pops::Evaluator::DeferredValue)
+
+    unsafe_unmunge(value)
+  end
+
+  # This is the default implementation of `unmunge` that simply produces the value (if it is valid).
+  # The DSL method {unmunge} should be used to define an overriding method if unmunging is required.
+  #
+  # @api private
+  #
+  def unsafe_unmunge(value)
     value
   end
 
-  # Munges the value to internal form.
+  # Munges the value from DSL form to internal form.
   # This implementation of `munge` provides exception handling around the specified munging of this parameter.
   # @note This method should not be overridden. Use the DSL method {munge} to define a munging method
   #   if required.
   # @param value [Object] the DSL value to munge
   # @return [Object] the munged (internal) value
   #
   def munge(value)
+    return value if value.is_a?(Puppet::Pops::Evaluator::DeferredValue)
+
     begin
       ret = unsafe_munge(value)
     rescue Puppet::Error => detail
@@ -459,6 +472,8 @@ def unsafe_validate(value)
   # @api public
   #
   def validate(value)
+    return if value.is_a?(Puppet::Pops::Evaluator::DeferredValue)
+
     begin
       unsafe_validate(value)
     rescue ArgumentError => detail

lib/puppet/pops/evaluator/deferred_resolver.rb

@@ -3,6 +3,16 @@
 module Puppet::Pops
 module Evaluator
 
+class DeferredValue
+  def initialize(proc)
+    @proc = proc
+  end
+
+  def resolve
+    @proc.call
+  end
+end
+
 # Utility class to help resolve instances of Puppet::Pops::Types::PDeferredType::Deferred
 #
 class DeferredResolver
@@ -20,9 +30,9 @@ class DeferredResolver
   #  are to be mixed into the scope
   # @return [nil] does not return anything - the catalog is modified as a side effect
   #
-  def self.resolve_and_replace(facts, catalog, environment = catalog.environment_instance)
-    compiler = Puppet::Parser::ScriptCompiler.new(environment, catalog.name, true)
-    resolver = new(compiler)
+  def self.resolve_and_replace(facts, catalog, environment = catalog.environment_instance, preprocess_deferred = true)
+    compiler = Puppet::Parser::ScriptCompiler.new(environment, catalog.name, preprocess_deferred)
+    resolver = new(compiler, preprocess_deferred)
     resolver.set_facts_variable(facts)
     # TODO:
     #    # When scripting the trusted data are always local, but set them anyway
@@ -53,11 +63,12 @@ def self.resolve(value, compiler)
     resolver.resolve(value)
   end
 
-  def initialize(compiler)
+  def initialize(compiler, preprocess_deferred = true)
     @compiler = compiler
     # Always resolve in top scope
     @scope = @compiler.topscope
     @deferred_class = Puppet::Pops::Types::TypeFactory.deferred.implementation_class
+    @preprocess_deferred = preprocess_deferred
   end
 
   # @param facts [Puppet::Node::Facts] the facts to set in $facts in the compiler's topscope
@@ -106,6 +117,24 @@ def resolve(x)
     end
   end
 
+  def resolve_lazy_args(x)
+    if x.is_a?(DeferredValue)
+      x.resolve
+    elsif x.is_a?(Array)
+      x.map {|v| resolve_lazy_args(v) }
+    elsif x.is_a?(Hash)
+      result = {}
+      x.each_pair {|k,v| result[k] = resolve_lazy_args(v) }
+      result
+    elsif x.is_a?(Puppet::Pops::Types::PSensitiveType::Sensitive)
+      # rewrap in a new Sensitive after resolving any nested deferred values
+      Puppet::Pops::Types::PSensitiveType::Sensitive.new(resolve_lazy_args(x.unwrap))
+    else
+      x
+    end
+  end
+  private :resolve_lazy_args
+
   def resolve_future(f)
     # If any of the arguments to a future is a future it needs to be resolved first
     func_name = f.name
@@ -117,8 +146,19 @@ def resolve_future(f)
       mapped_arguments.insert(0, @scope[var_name])
     end
 
-    # call the function (name in deferred, or 'dig' for a variable)
-    @scope.call_function(func_name, mapped_arguments)
+    if @preprocess_deferred
+      # call the function (name in deferred, or 'dig' for a variable)
+      @scope.call_function(func_name, mapped_arguments)
+    else
+      # call the function later
+      DeferredValue.new(
+        Proc.new {
+          # deferred functions can have nested deferred arguments
+          resolved_arguments = mapped_arguments.map { |arg| resolve_lazy_args(arg) }
+          @scope.call_function(func_name, resolved_arguments)
+        }
+      )
+    end
   end
 
   def map_arguments(args)

lib/puppet/transaction.rb

@@ -276,6 +276,7 @@ def apply(resource, ancestor = nil)
 
   # Evaluate a single resource.
   def eval_resource(resource, ancestor = nil)
+    resolve_resource(resource)
     propagate_failure(resource)
     if skip?(resource)
       resource_status(resource).skipped = true
@@ -464,6 +465,27 @@ def split_qualified_tags?
   public :skip?
   public :missing_tags?
 
+  def resolve_resource(resource)
+    return unless catalog.host_config?
+
+    deferred_validate = false
+
+    resource.eachparameter do |param|
+      if param.value.instance_of?(Puppet::Pops::Evaluator::DeferredValue)
+        # Puppet::Parameter#value= triggers validation and munging. Puppet::Property#value=
+        # overrides the method, but also triggers validation and munging, since we're
+        # setting the desired/should value.
+        resolved = param.value.resolve
+        # resource.notice("Resolved deferred value to #{resolved}")
+        param.value = resolved
+        deferred_validate = true
+      end
+    end
+
+    if deferred_validate
+      resource.validate_resource
+    end
+  end
 end
 
 require_relative 'transaction/report'

lib/puppet/type.rb

@@ -2282,7 +2282,13 @@ def self.to_s
   # @api public
   #
   def self.validate(&block)
-    define_method(:validate, &block)
+    define_method(:unsafe_validate, &block)
+
+    define_method(:validate) do
+      return if enum_for(:eachparameter).any? { |p| p.value.instance_of?(Puppet::Pops::Evaluator::DeferredValue) }
+
+      unsafe_validate
+    end
   end
 
   # @return [String] The file from which this type originates from
@@ -2372,15 +2378,26 @@ def initialize(resource)
 
     set_parameters(@original_parameters)
 
+    validate_resource
+
+    set_sensitive_parameters(resource.sensitive_parameters)
+  end
+
+  # Optionally validate the resource. This method is a noop if the type has not defined
+  # a `validate` method using the puppet DSL. If validation fails, then an exception will
+  # be raised with this resources as the context.
+  #
+  # @api public
+  #
+  # @return [void]
+  def validate_resource
     begin
       self.validate if self.respond_to?(:validate)
     rescue Puppet::Error, ArgumentError => detail
       error = Puppet::ResourceError.new("Validation of #{ref} failed: #{detail}")
       adderrorcontext(error, detail)
       raise error
     end
-
-    set_sensitive_parameters(resource.sensitive_parameters)
   end
 
   protected

spec/integration/application/agent_spec.rb

@@ -98,6 +98,18 @@
   end
 
   context 'rich data' do
+    let(:deferred_file) { tmpfile('deferred') }
+    let(:deferred_manifest) do <<~END
+      file { '#{deferred_file}':
+        ensure => file,
+        content => '123',
+      } ->
+      notify { 'deferred':
+        message => Deferred('binary_file', ['#{deferred_file}'])
+      }
+      END
+    end
+
     it "calls a deferred 4x function" do
       catalog_handler = -> (req, res) {
         catalog = compile_to_catalog(<<-MANIFEST, node)
@@ -142,6 +154,43 @@
       end
     end
 
+    it "fails to apply a deferred function with an unsatified prerequisite" do
+      catalog_handler = -> (req, res) {
+        catalog = compile_to_catalog(deferred_manifest, node)
+        res.body = formatter.render(catalog)
+        res['Content-Type'] = formatter.mime
+      }
+
+      server.start_server(mounts: {catalog: catalog_handler}) do |port|
+        Puppet[:serverport] = port
+        expect {
+          agent.command_line.args << '--test'
+          agent.run
+        }.to exit_with(1)
+         .and output(%r{Using environment}).to_stdout
+         .and output(%r{The given file '#{deferred_file}' does not exist}).to_stderr
+      end
+    end
+
+    it "applies a deferred function and its prerequisite in the same run" do
+      Puppet[:preprocess_deferred] = false
+
+      catalog_handler = -> (req, res) {
+        catalog = compile_to_catalog(deferred_manifest, node)
+        res.body = formatter.render(catalog)
+        res['Content-Type'] = formatter.mime
+      }
+
+      server.start_server(mounts: {catalog: catalog_handler}) do |port|
+        Puppet[:serverport] = port
+        expect {
+          agent.command_line.args << '--test'
+          agent.run
+        }.to exit_with(2)
+         .and output(%r{defined 'message' as Binary\("MTIz"\)}).to_stdout
+      end
+    end
+
     it "re-evaluates a deferred function in a cached catalog" do
       Puppet[:report] = false
       Puppet[:use_cached_catalog] = true