From 61a352f7ec204e31e1cc9540f5301eb4f434c3b6 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Mon, 23 Oct 2023 21:49:26 +0200 Subject: [PATCH 1/2] Use resource titles as identifiers in preloading When using composite namevars for resources then `resource.name` always `nil`. Prior to this it was used as a key in a hash when prefetching resources. By using the title it should work, since that's guaranteed to be set and unique. This can break providers where the name is different from the title. --- lib/puppet/transaction.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb index de2aa6d6631..ce24767685d 100644 --- a/lib/puppet/transaction.rb +++ b/lib/puppet/transaction.rb @@ -361,7 +361,7 @@ def resources_by_provider(type_name, provider_name) @catalog.vertices.each do |resource| if resource.class.attrclass(:provider) prov = resource.provider && resource.provider.class.name - @resources_by_provider[resource.type][prov][resource.name] = resource + @resources_by_provider[resource.type][prov][resource.title] = resource end end end From 37fd9416fd510f70fd9a3e95d4d8b2996a7dc693 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Mon, 23 Oct 2023 21:51:39 +0200 Subject: [PATCH 2/2] Make ParsedFile provider work with composite namevars Previously it was assumed that the name was the key. This changes the implementation to instead match on the key attributes. This allows it to work with composite namevars. While there was previously code for multiple target support, this makes it possible to use the same key in multiple files. It also gets rid of some code that wasn't needed. --- lib/puppet/provider/parsedfile.rb | 45 +++++++++++-------------------- 1 file changed, 15 insertions(+), 30 deletions(-) diff --git a/lib/puppet/provider/parsedfile.rb b/lib/puppet/provider/parsedfile.rb index d8e458bfec0..a4b8efec74d 100644 --- a/lib/puppet/provider/parsedfile.rb +++ b/lib/puppet/provider/parsedfile.rb @@ -21,11 +21,9 @@ class Puppet::Provider::ParsedFile < Puppet::Provider extend Puppet::Util::FileParsing class << self - attr_accessor :default_target, :target, :raise_prefetch_errors + attr_accessor :default_target, :raise_prefetch_errors end - attr_accessor :property_hash - def self.clean(hash) newhash = hash.dup [:record_type, :on_disk].each do |p| @@ -247,7 +245,9 @@ def self.match_providers_with_resources(resources) resource = match(record, matchers) if resource matchers.delete(resource.title) - record[:name] = resource[:name] + resource.class.key_attributes.each do |name| + record[name] = resource[name] + end resource.provider = new(record) end end @@ -263,9 +263,10 @@ def self.match_providers_with_resources(resources) # # @return [Puppet::Resource, nil] The resource if found, else nil def self.resource_for_record(record, resources) - name = record[:name] - if name - resources[name] + resources.values.find do |resource| + resource.class.key_attributes.all? do |name| + record[name] == resource[name] + end end end @@ -311,12 +312,6 @@ def self.prefetch_target(target) target_records end - # Is there an existing record with this name? - def self.record?(name) - return nil unless @records - @records.find { |r| r[:name] == name } - end - # Retrieve the text for the file. Returns nil in the unlikely # event that it doesn't exist. def self.retrieve(path) @@ -386,12 +381,10 @@ def self.targets(resources = nil) targets += @target_objects.keys # Lastly, check the file from any resource instances - if resources - resources.each do |name, resource| - value = resource.should(:target) - if value - targets << value - end + resources&.each do |name, resource| + value = resource[:target] + if value + targets << value end end @@ -429,17 +422,15 @@ def create end end mark_target_modified - (@resource.class.name.to_s + "_created").intern end def destroy # We use the method here so it marks the target as modified. self.ensure = :absent - (@resource.class.name.to_s + "_deleted").intern end def exists? - !(@property_hash[:ensure] == :absent or @property_hash[:ensure].nil?) + @property_hash[:ensure] == :present end # Write our data to disk. @@ -449,7 +440,7 @@ def flush # If the target isn't set, then this is our first modification, so # mark it for flushing. unless @property_hash[:target] - @property_hash[:target] = @resource.should(:target) || self.class.default_target + @property_hash[:target] = @resource[:target] || self.class.default_target self.class.modified(@property_hash[:target]) end @resource.class.key_attributes.each do |attr| @@ -465,13 +456,7 @@ def initialize(record) # The 'record' could be a resource or a record, depending on how the provider # is initialized. If we got an empty property hash (probably because the resource # is just being initialized), then we want to set up some defaults. - @property_hash = self.class.record?(resource[:name]) || {:record_type => self.class.name, :ensure => :absent} if @property_hash.empty? - end - - # Retrieve the current state from disk. - def prefetch - raise Puppet::DevError, _("Somehow got told to prefetch with no resource set") unless @resource - self.class.prefetch(@resource[:name] => @resource) + @property_hash[:record_type] ||= self.class.name end def record_type