Skip to content

Commit 91d0230

Browse files
committed
(maint) Pass through full composite namevar values for simple_get_filter
To make sure that `simple_get_filter` can work for types with composite namevars, we need to pass through the full set of namevar values, since `resource_hash[type_definition.namevars.first]` is not enough for the provider to return an instance. # Previous Behaviour When a provider implements `simple_get_filter`, and composite namevars, the Resource API does not provide sufficient information to `get` to retrieve the required resource state: ``` Puppet::ResourceApi.register_type( name: 'gpgkey', docs: <<-EOS, This type provides Puppet with the capabilities to manage ... EOS title_patterns: [ { pattern: %r{^(?<gpgdir>.*)/(?<name>[^/]*)$}, desc: 'Where the gpgdir and the key_name is provided as the last field of the path', }, { pattern: %r{^(?<name>.*)$}, desc: 'Where only the key_name is provided, if using the default folder', }, ], features: ['simple_get_filter'], attributes: { ensure: { type: 'Enum[present, absent]', desc: 'Whether this resource should be present or absent on the target system.', default: 'present', }, name: { type: 'String', desc: 'The name of the resource you want to manage.', behaviour: :namevar, }, gpgdir: { type: 'String', desc: 'Path to store the GPG key.', default: '/root', }, }, ) ``` ``` class Puppet::Provider::Gpgkey::Gpgkey < Puppet::ResourceApi::SimpleProvider def get(context, name = []) context.debug('Returning pre-canned example data for: %s' % name.inspect) [ { title: '/root/foo', name: 'foo', gpgdir: '/root', ensure: 'present', }, #{ # title: '/root/bar', # name: 'bar', # gpgdir: '/root', # ensure: 'present', #}, ] end def create(context, name, should) context.notice("Creating '#{name}' with #{should.inspect}") end def update(context, name, should) context.notice("Updating '#{name}' with #{should.inspect}") end def delete(context, name) context.notice("Deleting '#{name}'") end end ``` ``` gpgkey { 'baz1': ensure => present, name => 'foo', gpgdir => '/root'; '/root/bar': ensure => present; '/root/foo2': ensure => present; } ``` ``` Info: Applying configuration version '1558363097' Debug: gpgkey supports `simple_get_filter` Debug: gpgkey: Returning pre-canned example data for: ["foo"] Debug: gpgkey does not support `canonicalize` Debug: Current State: {:title=>"/root/foo", :name=>"foo", :gpgdir=>"/root", :ensure=>"present"} Debug: gpgkey supports `simple_get_filter` Debug: gpgkey: Returning pre-canned example data for: ["bar"] Debug: Current State: {:title=>"bar", :ensure=>:absent} Notice: /Stage[main]/Main/Gpgkey[bar]/ensure: defined 'ensure' as 'present' Debug: gpgkey does not support `canonicalize` Debug: Target State: {:name=>"bar", :ensure=>"present", :gpgdir=>"/root"} Debug: gpgkey does not support `supports_noop` Debug: gpgkey supports `simple_get_filter` Debug: gpgkey[bar]: Creating: Start Notice: gpgkey[bar]: Creating: Creating 'bar' with {:name=>"bar", :ensure=>"present", :gpgdir=>"/root"} Notice: gpgkey[bar]: Creating: Finished in 0.000085 seconds Debug: /Stage[main]/Main/Gpgkey[bar]: The container Class[Main] will propagate my refresh event Debug: gpgkey supports `simple_get_filter` Debug: gpgkey: Returning pre-canned example data for: ["foo2"] Debug: Current State: {:title=>"foo2", :ensure=>:absent} Notice: /Stage[main]/Main/Gpgkey[foo2]/ensure: defined 'ensure' as 'present' Debug: gpgkey does not support `canonicalize` Debug: Target State: {:name=>"foo2", :ensure=>"present", :gpgdir=>"/root"} Debug: gpgkey does not support `supports_noop` Debug: gpgkey supports `simple_get_filter` Debug: gpgkey[foo2]: Creating: Start Notice: gpgkey[foo2]: Creating: Creating 'foo2' with {:name=>"foo2", :ensure=>"present", :gpgdir=>"/root"} Notice: gpgkey[foo2]: Creating: Finished in 0.000069 seconds Debug: /Stage[main]/Main/Gpgkey[foo2]: The container Class[Main] will propagate my refresh event Debug: Class[Main]: The container Stage[main] will propagate my refresh event Debug: Finishing transaction 47323787575300 Debug: Storing state Debug: Pruned old state cache entries in 0.00 seconds Debug: Stored state in 0.01 seconds Notice: Applied catalog in 0.03 seconds ``` As can be seen in the `Returning pre-canned example data for` messages, only `name`, but not `gpgdir` is passed through. This is because of the weakness of title generation in https://github.com/puppetlabs/puppet-resource_api/blob/d249941cf7544005ad66b9bb54e079ffdfc0ac45/lib/puppet/resource_api.rb#L230-L235 # New Behaviour After these changes, a hash of namevars and their values is used as a title when requesting resources from the provider: ``` Info: Applying configuration version '1558450029' Debug: gpgkey supports `simple_get_filter` Debug: gpgkey: Returning pre-canned example data for: [{:name=>"foo", :gpgdir=>"/root"}] Debug: gpgkey does not support `canonicalize` Debug: Current State: {:title=>"/root/foo", :name=>"foo", :gpgdir=>"/root", :ensure=>"present"} Debug: gpgkey supports `simple_get_filter` Debug: gpgkey: Returning pre-canned example data for: [{:name=>"bar", :gpgdir=>"/root"}] Debug: gpgkey does not support `canonicalize` Debug: Current State: {:title=>"/root/bar", :name=>"bar", :gpgdir=>"/root", :ensure=>"present"} Debug: gpgkey supports `simple_get_filter` Debug: gpgkey: Returning pre-canned example data for: [{:name=>"foo2", :gpgdir=>"/root"}] Debug: Current State: {:title=>{:name=>"foo2", :gpgdir=>"/root"}, :ensure=>:absent} Notice: /Stage[main]/Main/Gpgkey[foo2]/ensure: defined 'ensure' as 'present' Debug: gpgkey does not support `canonicalize` Debug: Target State: {:name=>"foo2", :gpgdir=>"/root", :ensure=>"present"} Debug: gpgkey does not support `supports_noop` Debug: gpgkey supports `simple_get_filter` Debug: gpgkey[foo2]: Creating: Start Notice: gpgkey[foo2]: Creating: Creating '{:title=>"foo2", :name=>"foo2", :gpgdir=>"/root"}' with {:name=>"foo2", :gpgdir=>"/root", :ensure=>"present"} Notice: gpgkey[foo2]: Creating: Finished in 0.000071 seconds Debug: /Stage[main]/Main/Gpgkey[foo2]: The container Class[Main] will propagate my refresh event Debug: Class[Main]: The container Stage[main] will propagate my refresh event Debug: Finishing transaction 47273694891400 Debug: Storing state Debug: Pruned old state cache entries in 0.00 seconds Debug: Stored state in 0.01 seconds Notice: Applied catalog in 0.02 seconds ``` The provider should return a properly formatted `title` value from `get` to enable pretty formatting in logs and `puppet resource` output.
1 parent efaf767 commit 91d0230

File tree

4 files changed

+42
-12
lines changed

4 files changed

+42
-12
lines changed

lib/puppet/resource_api.rb

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,20 @@ def name
123123
title
124124
end
125125

126+
def self.build_title(type_definition, resource_hash)
127+
if type_definition.namevars.size > 1
128+
# use a MonkeyHash to allow seaching in Puppet's RALM9
129+
Puppet::ResourceApi::MonkeyHash[type_definition.namevars.map { |attr| [attr, resource_hash[attr]] }]
130+
else
131+
resource_hash[type_definition.namevars[0]]
132+
end
133+
end
134+
135+
def rsapi_title
136+
@rsapi_title ||= self.class.build_title(type_definition, self)
137+
@rsapi_title
138+
end
139+
126140
def to_resource
127141
to_resource_shim(super)
128142
end
@@ -251,7 +265,7 @@ def to_resource
251265
result = if resource_hash.key? :title
252266
new(title: resource_hash[:title])
253267
else
254-
new(title: resource_hash[type_definition.namevars.first])
268+
new(title: build_title(type_definition, resource_hash))
255269
end
256270
result.cache_current_state(resource_hash)
257271
result
@@ -260,7 +274,7 @@ def to_resource
260274

261275
define_method(:refresh_current_state) do
262276
@rsapi_current_state = if type_definition.feature?('simple_get_filter')
263-
my_provider.get(context, [title]).find { |h| namevar_match?(h) }
277+
my_provider.get(context, [rsapi_title]).find { |h| namevar_match?(h) }
264278
else
265279
my_provider.get(context).find { |h| namevar_match?(h) }
266280
end
@@ -269,7 +283,7 @@ def to_resource
269283
type_definition.check_schema(@rsapi_current_state)
270284
strict_check(@rsapi_current_state) if type_definition.feature?('canonicalize')
271285
else
272-
@rsapi_current_state = { title: title }
286+
@rsapi_current_state = { title: rsapi_title }
273287
@rsapi_current_state[:ensure] = :absent if type_definition.ensurable?
274288
end
275289
end

lib/puppet/resource_api/glue.rb

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,4 +46,19 @@ def filtered_keys
4646
values.keys.reject { |k| k == :title || !attr_def[k] || (attr_def[k][:behaviour] == :namevar && @namevars.size == 1) }
4747
end
4848
end
49+
50+
# this hash allows key-value based ordering comparisons between instances of this and instances of this and other classes
51+
# this is required for `lib/puppet/indirector/resource/ral.rb`'s `search` method which expects all titles to be comparable
52+
class MonkeyHash < Hash
53+
def <=>(other)
54+
result = self.class.name <=> other.class.name
55+
if result.zero?
56+
result = keys.sort <=> other.keys.sort
57+
end
58+
if result.zero?
59+
result = keys.sort.map { |k| self[k] } <=> other.keys.sort.map { |k| other[k] }
60+
end
61+
result
62+
end
63+
end
4964
end

spec/acceptance/namevar_spec.rb

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,12 @@
2424
expect(stdout_str.strip).to match %r{ensure\s*=> \'present\'}
2525
expect(status).to eq 0
2626
end
27-
it 'returns the match if title matches a namevar value' do
28-
stdout_str, status = Open3.capture2e("puppet resource #{common_args} multiple_namevar php")
29-
expect(stdout_str.strip).to match %r{^multiple_namevar \{ \'php\'}
27+
it 'returns the match if title matches a title value' do
28+
stdout_str, status = Open3.capture2e("puppet resource #{common_args} multiple_namevar php-gem")
29+
expect(stdout_str.strip).to match %r{^multiple_namevar \{ \'php-gem\'}
3030
expect(stdout_str.strip).to match %r{ensure\s*=> \'present\'}
3131
expect(stdout_str.strip).to match %r{package\s*=> \'php\'}
32+
expect(stdout_str.strip).to match %r{manager\s*=> \'gem\'}
3233
expect(status).to eq 0
3334
end
3435
it 'creates a previously absent resource if all namevars are provided' do

spec/fixtures/test_module/lib/puppet/provider/multiple_namevar/multiple_namevar.rb

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,12 @@
44
class Puppet::Provider::MultipleNamevar::MultipleNamevar
55
def initialize
66
@current_values ||= [
7-
{ package: 'php', manager: 'yum', ensure: 'present' },
8-
{ package: 'php', manager: 'gem', ensure: 'present' },
9-
{ package: 'mysql', manager: 'yum', ensure: 'present' },
10-
{ package: 'mysql', manager: 'gem', ensure: 'present' },
11-
{ package: 'foo', manager: 'bar', ensure: 'present' },
12-
{ package: 'bar', manager: 'foo', ensure: 'present' },
7+
{ title: 'php-yum', package: 'php', manager: 'yum', ensure: 'present' },
8+
{ title: 'php-gem', package: 'php', manager: 'gem', ensure: 'present' },
9+
{ title: 'mysql-yum', package: 'mysql', manager: 'yum', ensure: 'present' },
10+
{ title: 'mysql-gem', package: 'mysql', manager: 'gem', ensure: 'present' },
11+
{ title: 'foo-bar', package: 'foo', manager: 'bar', ensure: 'present' },
12+
{ title: 'bar-foo', package: 'bar', manager: 'foo', ensure: 'present' },
1313
]
1414
end
1515

0 commit comments

Comments
 (0)