Skip to content

Commit 815c9c7

Browse files
committed
(PUP-9323) Defer per-resource validation
If any parameters/properties for a resource are deferred, then also defer the per-resource validate method[1]. The latter is often used in cases when parameters are mutually exclusive or certain combinations are not allowed. Once all parameters for a resource are evaluated, then the resource's `validate` method will be called, if one is defined using the puppet DSL. [1] https://github.com/puppetlabs/puppet-specifications/blob/master/language/resource_types.md#validate
1 parent 6524805 commit 815c9c7

File tree

3 files changed

+69
-3
lines changed

3 files changed

+69
-3
lines changed

lib/puppet/transaction.rb

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -468,6 +468,8 @@ def split_qualified_tags?
468468
def resolve_resource(resource)
469469
return unless catalog.host_config?
470470

471+
deferred_validate = false
472+
471473
resource.eachparameter do |param|
472474
if param.value.instance_of?(Puppet::Pops::Evaluator::DeferredValue)
473475
# Puppet::Parameter#value= triggers validation and munging. Puppet::Property#value=
@@ -476,8 +478,13 @@ def resolve_resource(resource)
476478
resolved = param.value.resolve
477479
# resource.notice("Resolved deferred value to #{resolved}")
478480
param.value = resolved
481+
deferred_validate = true
479482
end
480483
end
484+
485+
if deferred_validate
486+
resource.validate_resource
487+
end
481488
end
482489
end
483490

lib/puppet/type.rb

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2282,7 +2282,13 @@ def self.to_s
22822282
# @api public
22832283
#
22842284
def self.validate(&block)
2285-
define_method(:validate, &block)
2285+
define_method(:unsafe_validate, &block)
2286+
2287+
define_method(:validate) do
2288+
return if enum_for(:eachparameter).any? { |p| p.value.instance_of?(Puppet::Pops::Evaluator::DeferredValue) }
2289+
2290+
unsafe_validate
2291+
end
22862292
end
22872293

22882294
# @return [String] The file from which this type originates from
@@ -2372,15 +2378,26 @@ def initialize(resource)
23722378

23732379
set_parameters(@original_parameters)
23742380

2381+
validate_resource
2382+
2383+
set_sensitive_parameters(resource.sensitive_parameters)
2384+
end
2385+
2386+
# Optionally validate the resource. This method is a noop if the type has not defined
2387+
# a `validate` method using the puppet DSL. If validation fails, then an exception will
2388+
# be raised with this resources as the context.
2389+
#
2390+
# @api public
2391+
#
2392+
# @return [void]
2393+
def validate_resource
23752394
begin
23762395
self.validate if self.respond_to?(:validate)
23772396
rescue Puppet::Error, ArgumentError => detail
23782397
error = Puppet::ResourceError.new("Validation of #{ref} failed: #{detail}")
23792398
adderrorcontext(error, detail)
23802399
raise error
23812400
end
2382-
2383-
set_sensitive_parameters(resource.sensitive_parameters)
23842401
end
23852402

23862403
protected

spec/integration/application/apply_spec.rb

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -713,5 +713,47 @@ def bogus()
713713
.and output(%r{defined 'message' as Binary\("MTIz"\)}).to_stdout
714714
end
715715

716+
it "validates the deferred resource before applying any resources" do
717+
undeferred_file = tmpfile('undeferred')
718+
719+
manifest = <<~END
720+
file { '#{undeferred_file}':
721+
ensure => file,
722+
}
723+
file { '#{deferred_file}':
724+
ensure => file,
725+
content => Deferred('inline_epp', ['<%= 42 %>']),
726+
source => 'http://example.com/content',
727+
}
728+
END
729+
apply.command_line.args = ['-e', manifest]
730+
expect {
731+
apply.run
732+
}.to exit_with(1)
733+
.and output(/Compiled catalog/).to_stdout
734+
.and output(/Validation of File.* failed: You cannot specify more than one of content, source, target/).to_stderr
735+
736+
# validation happens before all resources are applied, so this shouldn't exist
737+
expect(File).to_not be_exist(undeferred_file)
738+
end
739+
740+
it "evaluates resources before validating the deferred resource" do
741+
Puppet[:preprocess_deferred] = false
742+
743+
manifest = <<~END
744+
notify { 'runs before file': } ->
745+
file { '#{deferred_file}':
746+
ensure => file,
747+
content => Deferred('inline_epp', ['<%= 42 %>']),
748+
source => 'http://example.com/content',
749+
}
750+
END
751+
apply.command_line.args = ['-e', manifest]
752+
expect {
753+
apply.run
754+
}.to exit_with(1)
755+
.and output(/Notify\[runs before file\]/).to_stdout
756+
.and output(/Validation of File.* failed: You cannot specify more than one of content, source, target/).to_stderr
757+
end
716758
end
717759
end

0 commit comments

Comments
 (0)