Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(MODULES-9428) make the composite namevar implementation usable #174

Merged
merged 5 commits into from
Jun 28, 2019

Commits on Jun 28, 2019

  1. (maint) add a spec:unit rake task

    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.
    DavidS committed Jun 28, 2019
    Configuration menu
    Copy the full SHA
    e90f638 View commit details
    Browse the repository at this point in the history
  2. (maint) clean up unneeded define_method 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`.
    DavidS committed Jun 28, 2019
    Configuration menu
    Copy the full SHA
    d75a134 View commit details
    Browse the repository at this point in the history
  3. (MODULES-9428) show how simple_get_filter+composite namevars is broken

    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.
    DavidS committed Jun 28, 2019
    Configuration menu
    Copy the full SHA
    3f52b58 View commit details
    Browse the repository at this point in the history
  4. (MODULES-9428) 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.
    
    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
    DavidS committed Jun 28, 2019
    Configuration menu
    Copy the full SHA
    7ab1d9e View commit details
    Browse the repository at this point in the history
  5. (MODULES-9428) Improve handling of composite namevars in SimpleProvider

    Before this change, edgecases where `is` and `should` were not
    provided would lead to failures due to `name:` being filled wrong.
    DavidS committed Jun 28, 2019
    Configuration menu
    Copy the full SHA
    781083f View commit details
    Browse the repository at this point in the history