diff --git a/Rakefile b/Rakefile index e30132c6..1134d81a 100644 --- a/Rakefile +++ b/Rakefile @@ -19,6 +19,7 @@ RSpec::Core::RakeTask.new(:spec) do |t| if RUBY_PLATFORM == 'java' excludes += ['acceptance/**/*.rb', 'integration/**/*.rb', 'puppet/resource_api/*_context_spec.rb', 'puppet/util/network_device/simple/device_spec.rb'] t.rspec_opts = '--tag ~agent_test' + t.rspec_opts << ' --tag ~j17_exclude' if Gem::Version.new(RUBY_VERSION) < Gem::Version.new('2.0.0') end t.exclude_pattern = "spec/{#{excludes.join ','}}" end diff --git a/lib/puppet/resource_api.rb b/lib/puppet/resource_api.rb index 7824be94..4c5e7ed2 100644 --- a/lib/puppet/resource_api.rb +++ b/lib/puppet/resource_api.rb @@ -133,6 +133,20 @@ def name title end + def self.build_title(type_definition, resource_hash) + if type_definition.namevars.size > 1 + # use a MonkeyHash to allow searching in Puppet's RAL + 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 @@ -261,7 +275,7 @@ def self.instances 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 @@ -270,7 +284,7 @@ def self.instances def refresh_current_state @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 @@ -279,7 +293,11 @@ def refresh_current_state 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 = if rsapi_title.is_a? Hash + rsapi_title.dup + else + { title: rsapi_title } + end @rsapi_current_state[:ensure] = :absent if type_definition.ensurable? end end @@ -340,9 +358,9 @@ def flush end if type_definition.feature?('supports_noop') - my_provider.set(context, { title => { is: @rsapi_current_state, should: target_state } }, noop: noop?) + my_provider.set(context, { rsapi_title => { is: @rsapi_current_state, should: target_state } }, noop: noop?) else - my_provider.set(context, title => { is: @rsapi_current_state, should: target_state }) unless noop? + my_provider.set(context, rsapi_title => { is: @rsapi_current_state, should: target_state }) unless noop? end raise 'Execution encountered an error' if context.failed? diff --git a/lib/puppet/resource_api/glue.rb b/lib/puppet/resource_api/glue.rb index 30773a08..3e8a2911 100644 --- a/lib/puppet/resource_api/glue.rb +++ b/lib/puppet/resource_api/glue.rb @@ -57,4 +57,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 diff --git a/spec/acceptance/composite_namevar_spec.rb b/spec/acceptance/composite_namevar_spec.rb index ed808ac6..30a31110 100644 --- a/spec/acceptance/composite_namevar_spec.rb +++ b/spec/acceptance/composite_namevar_spec.rb @@ -31,14 +31,14 @@ stdout_str, status = Open3.capture2e("puppet resource #{common_args} composite_namevar php/gem") expect(stdout_str.strip).to match %r{^composite_namevar \{ \'php/gem\'} expect(stdout_str.strip).to match %r{ensure\s*=> \'present\'} - expect(stdout_str.strip).to match %r{Looking for \["php/gem"\]} + expect(stdout_str.strip).to match %r{Looking for \[\{:package=>"php", :manager=>"gem"\}\]} expect(status.exitstatus).to eq 0 end it 'properly identifies an absent resource if only the title is provided' do stdout_str, status = Open3.capture2e("puppet resource #{common_args} composite_namevar php-wibble") expect(stdout_str.strip).to match %r{^composite_namevar \{ \'php-wibble\'} expect(stdout_str.strip).to match %r{ensure\s*=> \'absent\'} - expect(stdout_str.strip).to match %r{Looking for \["php-wibble"\]} + expect(stdout_str.strip).to match %r{Looking for \[\{:package=>"php", :manager=>"wibble"\}\]} expect(status.exitstatus).to eq 0 end it 'creates a previously absent resource' do @@ -47,7 +47,7 @@ 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*=> \'wibble\'} - expect(stdout_str.strip).to match %r{Looking for \["php-wibble"\]} + expect(stdout_str.strip).to match %r{Looking for \[\{:package=>"php", :manager=>"wibble"\}\]} expect(status.exitstatus).to eq 0 end it 'will remove an existing resource' do @@ -56,7 +56,7 @@ expect(stdout_str.strip).to match %r{package\s*=> \'php\'} expect(stdout_str.strip).to match %r{manager\s*=> \'gem\'} expect(stdout_str.strip).to match %r{ensure\s*=> \'absent\'} - expect(stdout_str.strip).to match %r{Looking for \["php-gem"\]} + expect(stdout_str.strip).to match %r{Looking for \[\{:package=>"php", :manager=>"gem"\}\]} expect(status.exitstatus).to eq 0 end end @@ -79,7 +79,7 @@ let(:manifest) { 'composite_namevar { php-gem: }' } it { expect(@stdout_str).to match %r{Current State: \{:title=>"php-gem", :package=>"php", :manager=>"gem", :ensure=>"present", :value=>"b"\}} } - it { expect(@stdout_str).to match %r{Looking for \["php-gem"\]} } + it { expect(@stdout_str).to match %r{Looking for \[\{:package=>"php", :manager=>"gem"\}\]} } it { expect(@status.exitstatus).to eq 0 } end @@ -87,7 +87,7 @@ let(:manifest) { 'composite_namevar { php-wibble: ensure=>\'absent\' }' } it { expect(@stdout_str).to match %r{Composite_namevar\[php-wibble\]: Nothing to manage: no ensure and the resource doesn't exist} } - it { expect(@stdout_str).to match %r{Looking for \["php-wibble"\]} } + it { expect(@stdout_str).to match %r{Looking for \[\{:package=>"php", :manager=>"wibble"\}\]} } it { expect(@status.exitstatus).to eq 0 } end @@ -95,7 +95,7 @@ let(:manifest) { 'composite_namevar { php-wibble: ensure=>\'present\' }' } it { expect(@stdout_str).to match %r{Composite_namevar\[php-wibble\]/ensure: defined 'ensure' as 'present'} } - it { expect(@stdout_str).to match %r{Looking for \["php-wibble"\]} } + it { expect(@stdout_str).to match %r{Looking for \[\{:package=>"php", :manager=>"wibble"\}\]} } it { expect(@status.exitstatus).to eq 2 } end @@ -103,7 +103,7 @@ let(:manifest) { 'composite_namevar { php-yum: ensure=>\'absent\' }' } it { expect(@stdout_str).to match %r{Composite_namevar\[php-yum\]/ensure: undefined 'ensure' from 'present'} } - it { expect(@stdout_str).to match %r{Looking for \["php-yum"\]} } + it { expect(@stdout_str).to match %r{Looking for \[\{:package=>"php", :manager=>"yum"\}\]} } it { expect(@status.exitstatus).to eq 2 } end @@ -112,7 +112,7 @@ it { expect(@stdout_str).to match %r{Current State: \{:title=>"php-gem", :package=>"php", :manager=>"gem", :ensure=>"present", :value=>"b"\}} } it { expect(@stdout_str).to match %r{Target State: \{:package=>"php", :manager=>"gem", :value=>"c", :ensure=>"present"\}} } - it { expect(@stdout_str).to match %r{Looking for \["php/gem"\]} } + it { expect(@stdout_str).to match %r{Looking for \[\{:package=>"php", :manager=>"gem"\}\]} } it { expect(@status.exitstatus).to eq 2 } end end @@ -122,7 +122,7 @@ let(:manifest) { 'composite_namevar { "sometitle": package => "php", manager => "gem" }' } it { expect(@stdout_str).to match %r{Current State: \{:title=>"php-gem", :package=>"php", :manager=>"gem", :ensure=>"present", :value=>"b"\}} } - it { expect(@stdout_str).to match %r{Looking for \["sometitle"\]} } + it { expect(@stdout_str).to match %r{Looking for \[\{:package=>"php", :manager=>"gem"\}\]} } it { expect(@status.exitstatus).to eq 0 } end @@ -130,7 +130,7 @@ let(:manifest) { 'composite_namevar { "sometitle": ensure => "absent", package => "php", manager => "wibble" }' } it { expect(@stdout_str).to match %r{Composite_namevar\[sometitle\]: Nothing to manage: no ensure and the resource doesn't exist} } - it { expect(@stdout_str).to match %r{Looking for \["sometitle"\]} } + it { expect(@stdout_str).to match %r{Looking for \[\{:package=>"php", :manager=>"wibble"\}\]} } it { expect(@status.exitstatus).to eq 0 } end @@ -138,7 +138,7 @@ let(:manifest) { 'composite_namevar { "sometitle": ensure => "present", package => "php", manager => "wibble" }' } it { expect(@stdout_str).to match %r{Composite_namevar\[sometitle\]/ensure: defined 'ensure' as 'present'} } - it { expect(@stdout_str).to match %r{Looking for \["sometitle"\]} } + it { expect(@stdout_str).to match %r{Looking for \[\{:package=>"php", :manager=>"wibble"\}\]} } it { expect(@status.exitstatus).to eq 2 } end @@ -146,7 +146,7 @@ let(:manifest) { 'composite_namevar { "sometitle": ensure => "absent", package => "php", manager => "yum" }' } it { expect(@stdout_str).to match %r{Composite_namevar\[sometitle\]/ensure: undefined 'ensure' from 'present'} } - it { expect(@stdout_str).to match %r{Looking for \["sometitle"\]} } + it { expect(@stdout_str).to match %r{Looking for \[\{:package=>"php", :manager=>"yum"\}\]} } it { expect(@status.exitstatus).to eq 2 } end end diff --git a/spec/acceptance/namevar_spec.rb b/spec/acceptance/namevar_spec.rb index 2a434548..a162151d 100644 --- a/spec/acceptance/namevar_spec.rb +++ b/spec/acceptance/namevar_spec.rb @@ -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 diff --git a/spec/fixtures/test_module/lib/puppet/provider/composite_namevar/composite_namevar.rb b/spec/fixtures/test_module/lib/puppet/provider/composite_namevar/composite_namevar.rb index 8b588071..9b450f2a 100644 --- a/spec/fixtures/test_module/lib/puppet/provider/composite_namevar/composite_namevar.rb +++ b/spec/fixtures/test_module/lib/puppet/provider/composite_namevar/composite_namevar.rb @@ -1,7 +1,7 @@ require 'puppet/resource_api' require 'puppet/resource_api/simple_provider' -# Implementation for the title_provider type using the Resource API. +# Implementation for the composite_namevar type using the Resource API. class Puppet::Provider::CompositeNamevar::CompositeNamevar < Puppet::ResourceApi::SimpleProvider def initialize @current_values ||= [ diff --git a/spec/fixtures/test_module/lib/puppet/provider/multiple_namevar/multiple_namevar.rb b/spec/fixtures/test_module/lib/puppet/provider/multiple_namevar/multiple_namevar.rb index 857e73b9..c1eea1c0 100644 --- a/spec/fixtures/test_module/lib/puppet/provider/multiple_namevar/multiple_namevar.rb +++ b/spec/fixtures/test_module/lib/puppet/provider/multiple_namevar/multiple_namevar.rb @@ -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 diff --git a/spec/integration/resource_api_spec.rb b/spec/integration/resource_api_spec.rb index b8d1cdf8..543f15ea 100644 --- a/spec/integration/resource_api_spec.rb +++ b/spec/integration/resource_api_spec.rb @@ -7,12 +7,27 @@ let(:definition) do { name: 'integration', + title_patterns: [ + { + pattern: %r{^(?.*[^-])-(?.*)$}, + desc: 'Where the name and the name2 are provided with a hyphen separator', + }, + { + pattern: %r{^(?.*)$}, + desc: 'Where only the name is provided', + }, + ], attributes: { name: { type: 'String', behaviour: :namevar, desc: 'the title', }, + name2: { + type: 'String', + behaviour: :namevar, + desc: 'the other title', + }, string: { type: 'String', desc: 'a string attribute', @@ -170,7 +185,18 @@ s = setter Class.new do def get(_context) - [] + [ + { + name: 'foo', + name2: 'bar', + title: 'foo-bar', + }, + { + name: 'foo2', + name2: 'bar2', + title: 'foo2-bar2', + }, + ] end attr_reader :last_changes @@ -198,7 +224,7 @@ def get(_context) # See spec/acceptance/metaparam_spec.rb for more in-depth testing with a full puppet apply let(:catalog) { instance_double('Puppet::Resource::Catalog', 'catalog') } let(:instance) do - type.new(name: 'somename', ensure: 'present', boolean: true, integer: 15, float: 1.23, + type.new(name: 'somename', name2: 'othername', ensure: 'present', boolean: true, integer: 15, float: 1.23, variant_pattern: '0x1234ABCD', url: 'http://www.google.com', sensitive: Puppet::Pops::Types::PSensitiveType::Sensitive.new('a password'), string_array: %w[a b c], variant_array: 'not_an_array', array_of_arrays: [%w[a b c], %w[d e f]], @@ -225,7 +251,7 @@ def get(_context) end it 'can serialize to json' do - expect({ 'resources' => [instance.to_resource] }.to_json).to eq '{"resources":[{"somename":{"ensure":"absent","boolean_param":false,"integer_param":99,"float_param":3.21,"ensure_param":"present","variant_pattern_param":"1234ABCD","url_param":"http://www.puppet.com","string_array_param":"d","e":"f","string_param":"default value"}}]}' # rubocop:disable Metrics/LineLength + expect({ 'resources' => [instance.to_resource] }.to_json).to eq '{"resources":[{"somename":{"name":"somename","name2":"othername","ensure":"absent","boolean_param":false,"integer_param":99,"float_param":3.21,"ensure_param":"present","variant_pattern_param":"1234ABCD","url_param":"http://www.puppet.com","string_array_param":"d","e":"f","string_param":"default value"}}]}' # rubocop:disable Metrics/LineLength end end diff --git a/spec/puppet/resource_api/glue_spec.rb b/spec/puppet/resource_api/glue_spec.rb index b8b656ea..80ae642f 100644 --- a/spec/puppet/resource_api/glue_spec.rb +++ b/spec/puppet/resource_api/glue_spec.rb @@ -78,4 +78,20 @@ it { expect(instance.to_hash).to eq(namevarname: 'title', attr: 'value', attr_ro: 'fixed') } end end + + describe Puppet::ResourceApi::MonkeyHash do + it { expect(described_class.ancestors).to include Hash } + + describe '#<=>(other)' do + subject(:value) { described_class[b: 'b', c: 'c'] } + + it { expect(value <=> 'string').to eq(-1) } + # Avoid this test on jruby 1.7, where it is hitting a implementation inconsistency and `'string' <=> value` returns `nil` + it('compares to a string', j17_exclude: true) { expect('string' <=> value).to eq 1 } + it { expect(value <=> described_class[b: 'b', c: 'c']).to eq 0 } + it { expect(value <=> described_class[d: 'd']).to eq(-1) } + it { expect(value <=> described_class[a: 'a']).to eq 1 } + it { expect(value <=> described_class[b: 'a', c: 'c']).to eq 1 } + end + end end diff --git a/spec/puppet/resource_api_spec.rb b/spec/puppet/resource_api_spec.rb index 59a32121..e87f1d40 100644 --- a/spec/puppet/resource_api_spec.rb +++ b/spec/puppet/resource_api_spec.rb @@ -1133,7 +1133,9 @@ def set(_context, changes) end } end - it { expect { described_class.register_type(definition) }.not_to raise_error } + before(:each) do + described_class.register_type(definition) + end describe 'the registered type' do subject(:type) { Puppet::Type.type(:composite) } @@ -1142,7 +1144,7 @@ def set(_context, changes) end it { expect(type.parameters).to eq [:package, :manager] } end - describe 'an instance of the type' do + describe 'the type\'s class' do let(:provider_class) do Class.new do def get(_context) @@ -1152,33 +1154,54 @@ def get(_context) def set(_context, _changes); end end end - let(:instance) { Puppet::Type.type(:composite) } + let(:type_class) { Puppet::Type.type(:composite) } before(:each) do stub_const('Puppet::Provider::Composite', Module.new) stub_const('Puppet::Provider::Composite::Composite', provider_class) end - context 'when title_patterns called' do + describe '.title_patterns' do it 'returns correctly generated pattern' do # [[ %r{^(?.*[^/])/(?.*)$},[[:package],[:manager]]],[%r{^(?.*)$},[[:package]]]] - expect(instance.title_patterns.first[0]).to be_a Regexp - expect(instance.title_patterns.first[0]).to eq(%r{^(?.*[^/])/(?.*)$}) - expect(instance.title_patterns.first[1].size).to eq 2 - expect(instance.title_patterns.first[1][0][0]).to eq :package - expect(instance.title_patterns.first[1][1][0]).to eq :manager + expect(type_class.title_patterns.first[0]).to be_a Regexp + expect(type_class.title_patterns.first[0]).to eq(%r{^(?.*[^/])/(?.*)$}) + expect(type_class.title_patterns.first[1].size).to eq 2 + expect(type_class.title_patterns.first[1][0][0]).to eq :package + expect(type_class.title_patterns.first[1][1][0]).to eq :manager - expect(instance.title_patterns.last[0]).to be_a Regexp - expect(instance.title_patterns.last[0]).to eq(%r{^(?.*)$}) - expect(instance.title_patterns.last[1].size).to eq 1 - expect(instance.title_patterns.last[1][0][0]).to eq :package + expect(type_class.title_patterns.last[0]).to be_a Regexp + expect(type_class.title_patterns.last[0]).to eq(%r{^(?.*)$}) + expect(type_class.title_patterns.last[1].size).to eq 1 + expect(type_class.title_patterns.last[1][0][0]).to eq :package end end - context 'when instances called' do + describe '.instances' do it 'uses the title provided by the provider' do - expect(instance.instances[0].title).to eq('php/yum') + expect(type_class.instances[0].title).to eq('php/yum') + end + end + + context 'when flushing an instance' do + let(:provider_instance) { instance_double(provider_class, 'provider_instance') } + + before(:each) do + allow(provider_class).to receive(:new).and_return(provider_instance) + end + + after(:each) do + # reset cached provider between tests + type_class.instance_variable_set(:@my_provider, nil) + end + + it 'uses a hash as `name` when setting values' do + allow(provider_instance).to receive(:get).and_return([{ title: 'php/yum', package: 'php', manager: 'yum', ensure: 'present' }]) + expect(provider_instance).to receive(:set) { |_context, changes| + expect(changes.keys).to eq [{ package: 'php', manager: 'yum' }] + } + type_class.new(title: 'php/yum', ensure: :absent).flush end end end