Skip to content

Commit

Permalink
(maint) Pass through full composite namevar values for simple_get_filter
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
DavidS committed May 21, 2019
1 parent efaf767 commit 91d0230
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 12 deletions.
20 changes: 17 additions & 3 deletions lib/puppet/resource_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,20 @@ def name
title
end

def self.build_title(type_definition, resource_hash)
if type_definition.namevars.size > 1
# use a MonkeyHash to allow seaching in Puppet's RALM9
Puppet::ResourceApi::MonkeyHash[type_definition.namevars.map { |attr| [attr, resource_hash[attr]] }]
else
resource_hash[type_definition.namevars[0]]
end
end

def rsapi_title
@rsapi_title ||= self.class.build_title(type_definition, self)
@rsapi_title
end

def to_resource
to_resource_shim(super)
end
Expand Down Expand Up @@ -251,7 +265,7 @@ def to_resource
result = if resource_hash.key? :title
new(title: resource_hash[:title])
else
new(title: resource_hash[type_definition.namevars.first])
new(title: build_title(type_definition, resource_hash))
end
result.cache_current_state(resource_hash)
result
Expand All @@ -260,7 +274,7 @@ def to_resource

define_method(:refresh_current_state) do
@rsapi_current_state = if type_definition.feature?('simple_get_filter')
my_provider.get(context, [title]).find { |h| namevar_match?(h) }
my_provider.get(context, [rsapi_title]).find { |h| namevar_match?(h) }
else
my_provider.get(context).find { |h| namevar_match?(h) }
end
Expand All @@ -269,7 +283,7 @@ def to_resource
type_definition.check_schema(@rsapi_current_state)
strict_check(@rsapi_current_state) if type_definition.feature?('canonicalize')
else
@rsapi_current_state = { title: title }
@rsapi_current_state = { title: rsapi_title }
@rsapi_current_state[:ensure] = :absent if type_definition.ensurable?
end
end
Expand Down
15 changes: 15 additions & 0 deletions lib/puppet/resource_api/glue.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,19 @@ def filtered_keys
values.keys.reject { |k| k == :title || !attr_def[k] || (attr_def[k][:behaviour] == :namevar && @namevars.size == 1) }
end
end

# this hash allows key-value based ordering comparisons between instances of this and instances of this and other classes
# this is required for `lib/puppet/indirector/resource/ral.rb`'s `search` method which expects all titles to be comparable
class MonkeyHash < Hash
def <=>(other)
result = self.class.name <=> other.class.name
if result.zero?
result = keys.sort <=> other.keys.sort
end
if result.zero?
result = keys.sort.map { |k| self[k] } <=> other.keys.sort.map { |k| other[k] }
end
result
end
end
end
7 changes: 4 additions & 3 deletions spec/acceptance/namevar_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,12 @@
expect(stdout_str.strip).to match %r{ensure\s*=> \'present\'}
expect(status).to eq 0
end
it 'returns the match if title matches a namevar value' do
stdout_str, status = Open3.capture2e("puppet resource #{common_args} multiple_namevar php")
expect(stdout_str.strip).to match %r{^multiple_namevar \{ \'php\'}
it 'returns the match if title matches a title value' do
stdout_str, status = Open3.capture2e("puppet resource #{common_args} multiple_namevar php-gem")
expect(stdout_str.strip).to match %r{^multiple_namevar \{ \'php-gem\'}
expect(stdout_str.strip).to match %r{ensure\s*=> \'present\'}
expect(stdout_str.strip).to match %r{package\s*=> \'php\'}
expect(stdout_str.strip).to match %r{manager\s*=> \'gem\'}
expect(status).to eq 0
end
it 'creates a previously absent resource if all namevars are provided' do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@
class Puppet::Provider::MultipleNamevar::MultipleNamevar
def initialize
@current_values ||= [
{ package: 'php', manager: 'yum', ensure: 'present' },
{ package: 'php', manager: 'gem', ensure: 'present' },
{ package: 'mysql', manager: 'yum', ensure: 'present' },
{ package: 'mysql', manager: 'gem', ensure: 'present' },
{ package: 'foo', manager: 'bar', ensure: 'present' },
{ package: 'bar', manager: 'foo', ensure: 'present' },
{ title: 'php-yum', package: 'php', manager: 'yum', ensure: 'present' },
{ title: 'php-gem', package: 'php', manager: 'gem', ensure: 'present' },
{ title: 'mysql-yum', package: 'mysql', manager: 'yum', ensure: 'present' },
{ title: 'mysql-gem', package: 'mysql', manager: 'gem', ensure: 'present' },
{ title: 'foo-bar', package: 'foo', manager: 'bar', ensure: 'present' },
{ title: 'bar-foo', package: 'bar', manager: 'foo', ensure: 'present' },
]
end

Expand Down

0 comments on commit 91d0230

Please sign in to comment.