(MODULES-9428) make the composite namevar implementation usable #174
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DavidS
changed the title
DavidS
changed the title
DavidS
force-pushed
the
maint-fix-composite-simple_get_filter
branch
2 times, most recently
from
623dad3
to
91d0230
Compare
|
Moved the test-order randomization to #175 to reduce churn here. |
DavidS
force-pushed
the
maint-fix-composite-simple_get_filter
branch
8 times, most recently
from
7674cf3
to
79df584
Compare
|
I'm still not completely sure that this is the full solution. Note the |
|
Hi David, Did you had any chance to review the issue with the Title ?
Cheers, |
|
@vlours I've been chipping away at it and have something to share in a few days. I also now realise, I never linked the specification changes: puppetlabs/puppet-specifications#140 |
DavidS
force-pushed
the
maint-fix-composite-simple_get_filter
branch
from
79df584
to
76af7db
Compare
DavidS
changed the title
da-ar
approved these changes
Jun 27, 2019
|
Looks good, do you want to update the docs to include the impact of the changes on the |
This task can be used by developers for a quick check of their changes. This also includes switching the `Rake::Task[:spec_prep].invoke` to a task dependency to avoid double calls.
These calls add to the complexity of the code without adding any value. Since the introduction of the `type_definition` there are very few reasons for methods to remain defined as a block. This commit changes all of them and a few remaining carte blanche references to `definition`.
This change adds acceptance tests showing how enabling `simple_get_filter`
for a type with composite namevars will break when a user sets a custom
title for a resource and uses attributes to pass in the namevar values
instead:
```
let(:manifest) { 'composite_namevar { "sometitle": ensure => "present", package => "php", manager => "wibble" }' }
it { expect(@stdout_str).to match %r{Looking for \["sometitle"\]} }
```
This behaviour is misleading at best (as authors might expect `name` to
always match all namevars through a title pattern) and completely
debilitating at worst, when a custom title is passed to `delete` and
there is no indication which resource to delete.
…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.
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',
behaviour: :namevar,
},
},
)
```
```
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
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 '1561397939'
Debug: gpgkey: Returning pre-canned example data for: [{:name=>"foo", :gpgdir=>"/root"}]
Debug: Current State: {:title=>"/root/foo", :name=>"foo", :gpgdir=>"/root", :ensure=>"present"}
Debug: gpgkey: Returning pre-canned example data for: [{:name=>"bar", :gpgdir=>"/root"}]
Debug: Current State: {:name=>"bar", :gpgdir=>"/root", :ensure=>:absent}
Notice: /Stage[main]/Main/Gpgkey[bar]/ensure: defined 'ensure' as 'present'
Debug: Target State: {:name=>"bar", :gpgdir=>"/root", :ensure=>"present"}
Debug: gpgkey[{:name=>"bar", :gpgdir=>"/root"}]: Creating: Start
Notice: gpgkey[{:name=>"bar", :gpgdir=>"/root"}]: Creating: Creating '{:title=>{:name=>"bar", :gpgdir=>"/root"}, :name=>"bar", :gpgdir=>"/root"}' with {:name=>"bar", :gpgdir=>"/root", :ensure=>"present"}
Notice: gpgkey[{:name=>"bar", :gpgdir=>"/root"}]: Creating: Finished in 0.000091 seconds
Debug: /Stage[main]/Main/Gpgkey[bar]: The container Class[Main] will propagate my refresh event
Debug: gpgkey: Returning pre-canned example data for: [{:name=>"foo2", :gpgdir=>"/root"}]
Debug: Current State: {:name=>"foo2", :gpgdir=>"/root", :ensure=>:absent}
Notice: /Stage[main]/Main/Gpgkey[foo2]/ensure: defined 'ensure' as 'present'
Debug: Target State: {:name=>"foo2", :gpgdir=>"/root", :ensure=>"present"}
Debug: gpgkey[{:name=>"foo2", :gpgdir=>"/root"}]: Creating: Start
Notice: gpgkey[{:name=>"foo2", :gpgdir=>"/root"}]: Creating: Creating '{:title=>{:name=>"foo2", :gpgdir=>"/root"}, :name=>"foo2", :gpgdir=>"/root"}' with {:name=>"foo2", :gpgdir=>"/root", :ensure=>"present"}
Notice: gpgkey[{:name=>"foo2", :gpgdir=>"/root"}]: Creating: Finished in 0.000116 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 47326046031800
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.
# Conflicts:
# spec/acceptance/composite_namevar_spec.rb
Before this change, edgecases where `is` and `should` were not provided would lead to failures due to `name:` being filled wrong.
DavidS
force-pushed
the
maint-fix-composite-simple_get_filter
branch
from
76af7db
to
781083f
Compare
da-ar
approved these changes
Jun 28, 2019
netbsd-srcmastr
pushed a commit
to NetBSD/pkgsrc
that referenced
this pull request
Mar 24, 2020
Update ruby-puppet-resource_api to 1.8.12. ## [1.8.7](https://github.com/puppetlabs/puppet-resource_api/tree/1.8.7) (2019-09-11) [Full Changelog](puppetlabs/puppet-resource_api@1.8.6...1.8.7) **Fixed bugs:** - \(FM-8092\) Fix caching scope of transport schemas [\#200](puppetlabs/puppet-resource_api#200) ([DavidS](https://github.com/DavidS)) **Merged pull requests:** - \(FM-8485\) - Addition of CODEOWNERS file [\#203](puppetlabs/puppet-resource_api#203) ([david22swan](https://github.com/david22swan)) - \(MODULES-9258\) Improve referencing and add summary [\#199](puppetlabs/puppet-resource_api#199) ([MaxMagill](https://github.com/MaxMagill)) - \(maint\) Pin both Jruby cells to use `dist: trusty` [\#197](puppetlabs/puppet-resource_api#197) ([da-ar](https://github.com/da-ar)) ## [v1.8.6](https://github.com/puppetlabs/puppet-resource_api/tree/v1.8.6) (2019-07-01) [Full Changelog](puppetlabs/puppet-resource_api@1.8.5...v1.8.6) **Implemented enhancements:** - \(SERVER-2470\) list\_all\_transports implementation for puppetserver [\#187](puppetlabs/puppet-resource_api#187) ([DavidS](https://github.com/DavidS)) **Fixed bugs:** - \(MODULES-9428\) make the composite namevar implementation usable [\#174](puppetlabs/puppet-resource_api#174) ([DavidS](https://github.com/DavidS)) **Merged pull requests:** - Merge 1.6.x [\#194](puppetlabs/puppet-resource_api#194) ([da-ar](https://github.com/da-ar)) - \(maint\) test fixes [\#193](puppetlabs/puppet-resource_api#193) ([DavidS](https://github.com/DavidS)) - \(packaging\) Revert to version '1.8.5' \[no-promote\] [\#192](puppetlabs/puppet-resource_api#192) ([gimmyxd](https://github.com/gimmyxd)) - \(packaging\) Bump to version '1.9.0' \[no-promote\] [\#191](puppetlabs/puppet-resource_api#191) ([gimmyxd](https://github.com/gimmyxd)) ## [1.8.5](https://github.com/puppetlabs/puppet-resource_api/tree/1.8.5) (2019-06-24) [Full Changelog](puppetlabs/puppet-resource_api@1.8.4...1.8.5) **Fixed bugs:** - \(maint\) Mergeup 1.6.x: FM-7839, desc/docs cleanup [\#186](puppetlabs/puppet-resource_api#186) ([DavidS](https://github.com/DavidS)) **Merged pull requests:** - \(maint\) reduce debug noise caused by `feature?` [\#189](puppetlabs/puppet-resource_api#189) ([da-ar](https://github.com/da-ar)) - \(FM-8265\) Merge branch '1.6.x' into master [\#188](puppetlabs/puppet-resource_api#188) ([da-ar](https://github.com/da-ar)) - \(maint\) test fixes [\#185](puppetlabs/puppet-resource_api#185) ([DavidS](https://github.com/DavidS)) - \(maint\) make test order really random [\#175](puppetlabs/puppet-resource_api#175) ([DavidS](https://github.com/DavidS)) - \(packaging\) Update reported version to 1.8.4 \[no-promote\] [\#171](puppetlabs/puppet-resource_api#171) ([gimmyxd](https://github.com/gimmyxd)) ## [1.8.4](https://github.com/puppetlabs/puppet-resource_api/tree/1.8.4) (2019-06-12) [Full Changelog](puppetlabs/puppet-resource_api@1.8.3...1.8.4) **Implemented enhancements:** - \(FM-7839\) Implement `to\_json` method for ResourceShim [\#168](puppetlabs/puppet-resource_api#168) ([da-ar](https://github.com/da-ar)) **Fixed bugs:** - \(maint\) backport minor fixes from master to 1.6.x [\#184](puppetlabs/puppet-resource_api#184) ([DavidS](https://github.com/DavidS)) - \(PUP-9747\) Relax validation for bolt [\#182](puppetlabs/puppet-resource_api#182) ([DavidS](https://github.com/DavidS)) - \(maint\) Add to\_hash function to resourceShim for compatibility [\#180](puppetlabs/puppet-resource_api#180) ([da-ar](https://github.com/da-ar)) - \(maint\) implement `desc`/`docs` fallback [\#177](puppetlabs/puppet-resource_api#177) ([DavidS](https://github.com/DavidS)) **Closed issues:** - ResourceShim should respond to to\_hash [\#179](puppetlabs/puppet-resource_api#179) **Merged pull requests:** - \(maint\) Merge 1.6.x to master [\#183](puppetlabs/puppet-resource_api#183) ([mihaibuzgau](https://github.com/mihaibuzgau)) - \(maint\) Fixup Gemfile for JRuby 1.7 installs [\#173](puppetlabs/puppet-resource_api#173) ([da-ar](https://github.com/da-ar)) - \(maint\) test cleanups [\#172](puppetlabs/puppet-resource_api#172) ([DavidS](https://github.com/DavidS)) ## [1.8.3](https://github.com/puppetlabs/puppet-resource_api/tree/1.8.3) (2019-04-12) [Full Changelog](puppetlabs/puppet-resource_api@1.8.2...1.8.3) **Fixed bugs:** - \(FM-7867\) Always throw when transport schema validation fails [\#169](puppetlabs/puppet-resource_api#169) ([da-ar](https://github.com/da-ar)) **Merged pull requests:** - \(PA-2496\) Bump version and remove v from version number [\#170](puppetlabs/puppet-resource_api#170) ([mihaibuzgau](https://github.com/mihaibuzgau)) ## [1.8.2](https://github.com/puppetlabs/puppet-resource_api/tree/1.8.2) (2019-04-10) [Full Changelog](puppetlabs/puppet-resource_api@v1.6.4...1.8.2) **Merged pull requests:** - \(packaging\) Update reported version to 1.8.2 \[no-promote\] [\#167](puppetlabs/puppet-resource_api#167) ([mihaibuzgau](https://github.com/mihaibuzgau)) ## [v1.6.4](https://github.com/puppetlabs/puppet-resource_api/tree/v1.6.4) (2019-03-25) [Full Changelog](puppetlabs/puppet-resource_api@v1.8.1...v1.6.4) **Merged pull requests:** - Add `implementations` to reserved bolt keywords [\#165](puppetlabs/puppet-resource_api#165) ([DavidS](https://github.com/DavidS)) - \(MAINT\) Bump version [\#164](puppetlabs/puppet-resource_api#164) ([sebastian-miclea](https://github.com/sebastian-miclea)) - Release prep for v1.8.1 [\#163](puppetlabs/puppet-resource_api#163) ([DavidS](https://github.com/DavidS)) # Changelog All significant changes to this repo will be summarized in this file. ## [v1.8.1](https://github.com/puppetlabs/puppet-resource_api/tree/v1.8.1) (2019-03-13) [Full Changelog](puppetlabs/puppet-resource_api@v1.8.0...v1.8.1) **Fixed bugs:** - \(maint\) Fixes sensitive transport values where absent keys are wrapped [\#161](puppetlabs/puppet-resource_api#161) ([da-ar](https://github.com/da-ar)) **Merged pull requests:** - 1.6.x mergeup [\#162](puppetlabs/puppet-resource_api#162) ([DavidS](https://github.com/DavidS)) - \(FM-7829\) Update README with transports examples [\#160](puppetlabs/puppet-resource_api#160) ([willmeek](https://github.com/willmeek)) - \(maint\) update release docs [\#159](puppetlabs/puppet-resource_api#159) ([DavidS](https://github.com/DavidS)) - Improve travis cells and testing [\#145](puppetlabs/puppet-resource_api#145) ([DavidS](https://github.com/DavidS)) ## [v1.8.0](https://github.com/puppetlabs/puppet-resource_api/tree/v1.8.0) (2019-02-26) [Full Changelog](puppetlabs/puppet-resource_api@v1.7.0...v1.8.0) **Implemented enhancements:** - \(FM-7695\) Transports - the remote content framework [\#157](puppetlabs/puppet-resource_api#157) ([DavidS](https://github.com/DavidS)) - \(FM-7698\) implement `sensitive:true` handling [\#156](puppetlabs/puppet-resource_api#156) ([da-ar](https://github.com/da-ar)) - \(PDK-1271\) Allow a transport to be wrapped and used like a device [\#155](puppetlabs/puppet-resource_api#155) ([da-ar](https://github.com/da-ar)) - \(FM-7701\) Support device providers when using Transport Wrapper [\#154](puppetlabs/puppet-resource_api#154) ([da-ar](https://github.com/da-ar)) - \(FM-7726\) implement `context.transport` to provide access [\#152](puppetlabs/puppet-resource_api#152) ([DavidS](https://github.com/DavidS)) - \(FM-7674\) Allow wrapping a Transport in a legacy Device [\#149](puppetlabs/puppet-resource_api#149) ([da-ar](https://github.com/da-ar)) - \(FM-7600\) Add Transport.connect method [\#148](puppetlabs/puppet-resource_api#148) ([da-ar](https://github.com/da-ar)) **Fixed bugs:** - \(FM-7690\) Fix transports cache to be environment aware [\#151](puppetlabs/puppet-resource_api#151) ([da-ar](https://github.com/da-ar)) **Merged pull requests:** - \(FM-7726\) cleanups for the transport [\#153](puppetlabs/puppet-resource_api#153) ([DavidS](https://github.com/DavidS)) - \(FM-7691,FM-7696\) refactoring definition handling in contexts [\#150](puppetlabs/puppet-resource_api#150) ([DavidS](https://github.com/DavidS)) ## [v1.7.0](https://github.com/puppetlabs/puppet-resource_api/tree/v1.7.0) (2019-01-07) [Full Changelog](puppetlabs/puppet-resource_api@v1.6.3...v1.7.0) **Implemented enhancements:** - \(maint\) Validate Type Schema [\#142](puppetlabs/puppet-resource_api#142) ([da-ar](https://github.com/da-ar)) **Merged pull requests:** - \(maint\) Bundler 2.0 dropped support for Ruby versions \< 2.2 [\#147](puppetlabs/puppet-resource_api#147) ([da-ar](https://github.com/da-ar)) - \(FM-7597\) RSAPI Transport register function [\#146](puppetlabs/puppet-resource_api#146) ([da-ar](https://github.com/da-ar)) - \(packaging\) Update version to 1.7.0 [\#144](puppetlabs/puppet-resource_api#144) ([branan](https://github.com/branan)) ## [v1.6.3](https://github.com/puppetlabs/puppet-resource_api/tree/v1.6.3) (2018-12-11) [Full Changelog](puppetlabs/puppet-resource_api@v1.6.2...v1.6.3) **Closed issues:** - Trying to understand stubbing in the examples [\#136](puppetlabs/puppet-resource_api#136) **Merged pull requests:** - \(packaging\) Update version to 1.6.3 [\#143](puppetlabs/puppet-resource_api#143) ([branan](https://github.com/branan)) - Move parameter and property logic to separate classes [\#140](puppetlabs/puppet-resource_api#140) ([bpietraga](https://github.com/bpietraga)) - \(maint\) Predeclare Puppet module before ResourceApi [\#139](puppetlabs/puppet-resource_api#139) ([caseywilliams](https://github.com/caseywilliams)) - \(maint\) minor fix to make data\_type\_handling change work [\#138](puppetlabs/puppet-resource_api#138) ([DavidS](https://github.com/DavidS)) - \(maint\) extract data type handling code [\#137](puppetlabs/puppet-resource_api#137) ([bpietraga](https://github.com/bpietraga)) - Release prep for v1.6.2 [\#135](puppetlabs/puppet-resource_api#135) ([DavidS](https://github.com/DavidS))
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a series of changes to make the composite namevar behaviour work. See the individual commits for details and puppetlabs/puppet-specifications#140 for the specification changes/clarifications.