diff --git a/lib/puppet/resource_api.rb b/lib/puppet/resource_api.rb index 80300684..6ece22bc 100644 --- a/lib/puppet/resource_api.rb +++ b/lib/puppet/resource_api.rb @@ -61,6 +61,21 @@ def feature_support?(feature) super(attributes) end + validate do + # enforce mandatory attributes + missing_attrs = [] + definition[:attributes].each do |name, options| + type = Puppet::Pops::Types::TypeParser.singleton.parse(options[:type]) + unless [:read_only, :namevar].include? options[:behaviour] + missing_attrs << name if value(name).nil? && !(type.instance_of? Puppet::Pops::Types::POptionalType) + end + end + if missing_attrs.any? + error_msg = "The following mandatory attributes where not provided:\n * " + missing_attrs.join(", \n * ") + raise Puppet::ResourceError, error_msg + end + end + definition[:attributes].each do |name, options| # puts "#{name}: #{options.inspect}" @@ -117,6 +132,10 @@ def feature_support?(feature) end end + if type.instance_of? Puppet::Pops::Types::POptionalType + type = type.type + end + # provide better handling of the standard types case type when Puppet::Pops::Types::PStringType diff --git a/spec/acceptance/device_spec.rb b/spec/acceptance/device_spec.rb index 79a00cd6..57ffcddb 100644 --- a/spec/acceptance/device_spec.rb +++ b/spec/acceptance/device_spec.rb @@ -3,6 +3,11 @@ RSpec.describe 'exercising a device provider' do let(:common_args) { '--verbose --trace --strict=error --modulepath spec/fixtures' } + let(:default_type_values) do + 'string="meep" boolean=true integer=15 float=1.23 ensure=present variant_pattern=AE321EEF '\ + 'url="http://www.puppet.com" boolean_param=false integer_param=99 float_param=3.21 '\ + 'ensure_param=present variant_pattern_param=0xAE321EEF url_param="https://www.google.com"' + end before(:each) { skip 'No device --apply in the puppet gems yet' if ENV['PUPPET_GEM_VERSION'] } @@ -14,7 +19,7 @@ expect(status).to eq 0 end it 'manages resources on the target system' do - stdout_str, status = Open3.capture2e("puppet resource #{common_args} device_provider foo ensure=present") + stdout_str, status = Open3.capture2e("puppet resource #{common_args} device_provider foo ensure=present #{default_type_values}") expect(stdout_str).to match %r{Notice: /Device_provider\[foo\]/ensure: defined 'ensure' as 'present'} expect(status).to eq 0 end @@ -23,7 +28,7 @@ let(:common_args) { '--verbose --trace --strict=error --modulepath spec/fixtures' } it 'deals with canonicalized resources correctly' do - stdout_str, status = Open3.capture2e("puppet resource #{common_args} device_provider wibble ensure=present") + stdout_str, status = Open3.capture2e("puppet resource #{common_args} device_provider wibble ensure=present #{default_type_values}") stdmatch = 'Error: /Device_provider\[wibble\]: Could not evaluate: device_provider\[wibble\]#get has not provided canonicalized values.\n'\ 'Returned values: \{:name=>"wibble", :ensure=>:present, :string=>"sample"\}\n'\ 'Canonicalized values: \{:name=>"wibble", :ensure=>:present, :string=>"changed"\}' @@ -36,7 +41,7 @@ let(:common_args) { '--verbose --trace --strict=warning --modulepath spec/fixtures' } it 'deals with canonicalized resources correctly' do - stdout_str, status = Open3.capture2e("puppet resource #{common_args} device_provider wibble ensure=present") + stdout_str, status = Open3.capture2e("puppet resource #{common_args} device_provider wibble ensure=present #{default_type_values}") stdmatch = 'Warning: device_provider\[wibble\]#get has not provided canonicalized values.\n'\ 'Returned values: \{:name=>"wibble", :ensure=>:present, :string=>"sample"\}\n'\ 'Canonicalized values: \{:name=>"wibble", :ensure=>:present, :string=>"changed"\}' @@ -49,7 +54,7 @@ let(:common_args) { '--verbose --trace --strict=off --modulepath spec/fixtures' } it 'deals with canonicalized resources correctly' do - stdout_str, status = Open3.capture2e("puppet resource #{common_args} device_provider wibble ensure=present") + stdout_str, status = Open3.capture2e("puppet resource #{common_args} device_provider wibble ensure=present #{default_type_values}") stdmatch = 'Notice: /Device_provider\[wibble\]/string: string changed \'sample\' to \'changed\'' expect(stdout_str).to match %r{#{stdmatch}} expect(status.success?).to be_truthy # rubocop:disable RSpec/PredicateMatcher @@ -98,7 +103,10 @@ context 'with a device resource in the catalog' do it 'applies the catalog successfully' do - stdout_str, _status = Open3.capture2e("puppet device #{common_args} --deviceconfig #{device_conf.path} --apply 'device_provider{\"foo\": }'") + stdout_str, _status = Open3.capture2e("puppet device #{common_args} --deviceconfig #{device_conf.path} --apply 'device_provider{\"foo\": "\ + 'ensure => "present", boolean => true, integer => 15, float => 1.23, variant_pattern => 0xA245EED, '\ + 'url => "http://www.google.com", boolean_param => false, integer_param => 99, float_param => 3.21, '\ + "ensure_param => \"present\", variant_pattern_param => \"9A2222ED\", url_param => \"http://www.puppet.com\"}'") expect(stdout_str).not_to match %r{Error:} end end diff --git a/spec/fixtures/test_module/lib/puppet/type/device_provider.rb b/spec/fixtures/test_module/lib/puppet/type/device_provider.rb index b490abfa..5de40149 100644 --- a/spec/fixtures/test_module/lib/puppet/type/device_provider.rb +++ b/spec/fixtures/test_module/lib/puppet/type/device_provider.rb @@ -35,10 +35,6 @@ type: 'Variant[Pattern[/\A(0x)?[0-9a-fA-F]{8}\Z/], Pattern[/\A(0x)?[0-9a-fA-F]{16}\Z/], Pattern[/\A(0x)?[0-9a-fA-F]{40}\Z/]]', desc: 'a pattern parameter', }, - path: { - type: 'Variant[Stdlib::Absolutepath, Pattern[/\A(https?|ftp):\/\//]]', - desc: 'a path or URL parameter', - }, url: { type: 'Pattern[/\A((hkp|http|https):\/\/)?([a-z\d])([a-z\d-]{0,61}\.)+[a-z\d]+(:\d{2,5})?$/]', desc: 'a hkp or http(s) url parameter', @@ -78,11 +74,6 @@ desc: 'a pattern parameter', behaviour: :parameter, }, - path_param: { - type: 'Variant[Stdlib::Absolutepath, Pattern[/\A(https?|ftp):\/\//]]', - desc: 'a path or URL parameter', - behaviour: :parameter, - }, url_param: { type: 'Pattern[/\A((hkp|http|https):\/\/)?([a-z\d])([a-z\d-]{0,61}\.)+[a-z\d]+(:\d{2,5})?$/]', desc: 'a hkp or http(s) url parameter', @@ -97,47 +88,42 @@ type: 'String', desc: 'a string readonly', default: 'default value', - behaviour: :readonly, + behaviour: :read_only, }, boolean_ro: { type: 'Boolean', desc: 'a boolean readonly', - behaviour: :readonly, + behaviour: :read_only, }, integer_ro: { type: 'Integer', desc: 'an integer readonly', - behaviour: :readonly, + behaviour: :read_only, }, float_ro: { type: 'Float', desc: 'a floating point readonly', - behaviour: :readonly, + behaviour: :read_only, }, ensure_ro: { type: 'Enum[present, absent]', desc: 'a ensure readonly', - behaviour: :readonly, + behaviour: :read_only, }, variant_pattern_ro: { type: 'Variant[Pattern[/\A(0x)?[0-9a-fA-F]{8}\Z/], Pattern[/\A(0x)?[0-9a-fA-F]{16}\Z/], Pattern[/\A(0x)?[0-9a-fA-F]{40}\Z/]]', desc: 'a pattern readonly', - behaviour: :readonly, - }, - path_ro: { - type: 'Variant[Stdlib::Absolutepath, Pattern[/\A(https?|ftp):\/\//]]', - desc: 'a path or URL readonly', - behaviour: :readonly, + behaviour: :read_only, }, url_ro: { type: 'Pattern[/\A((hkp|http|https):\/\/)?([a-z\d])([a-z\d-]{0,61}\.)+[a-z\d]+(:\d{2,5})?$/]', desc: 'a hkp or http(s) url readonly', - behaviour: :readonly, + behaviour: :read_only, }, optional_string_ro: { type: 'Optional[String]', desc: 'a optional string readonly', - behaviour: :readonly, + behaviour: :read_only, }, }, ) diff --git a/spec/fixtures/test_module/lib/puppet/type/test_autorequire.rb b/spec/fixtures/test_module/lib/puppet/type/test_autorequire.rb index 47766e14..604ca6fd 100644 --- a/spec/fixtures/test_module/lib/puppet/type/test_autorequire.rb +++ b/spec/fixtures/test_module/lib/puppet/type/test_autorequire.rb @@ -18,7 +18,7 @@ behaviour: :namevar, }, target: { - type: 'String', + type: 'Optional[String]', desc: 'The resource to autorequire.', behaviour: :parameter, }, diff --git a/spec/fixtures/test_module/lib/puppet/type/test_simple_get_filter.rb b/spec/fixtures/test_module/lib/puppet/type/test_simple_get_filter.rb index 48d8231c..cfc7056e 100644 --- a/spec/fixtures/test_module/lib/puppet/type/test_simple_get_filter.rb +++ b/spec/fixtures/test_module/lib/puppet/type/test_simple_get_filter.rb @@ -18,7 +18,7 @@ behaviour: :namevar, }, test_string: { - type: 'String', + type: 'Optional[String]', desc: 'Used for testing our expected outcomes', }, }, diff --git a/spec/integration/resource_api_spec.rb b/spec/integration/resource_api_spec.rb index 04e34364..0ce2e08c 100644 --- a/spec/integration/resource_api_spec.rb +++ b/spec/integration/resource_api_spec.rb @@ -38,10 +38,6 @@ type: 'Variant[Pattern[/\A(0x)?[0-9a-fA-F]{8}\Z/], Pattern[/\A(0x)?[0-9a-fA-F]{16}\Z/], Pattern[/\A(0x)?[0-9a-fA-F]{40}\Z/]]', desc: 'a pattern parameter', }, - path: { - type: 'Variant[Stdlib::Absolutepath, Pattern[/\A(https?|ftp):\/\//]]', - desc: 'a path or URL parameter', - }, url: { type: 'Pattern[/\A((hkp|http|https):\/\/)?([a-z\d])([a-z\d-]{0,61}\.)+[a-z\d]+(:\d{2,5})?$/]', desc: 'a hkp or http(s) url parameter', @@ -81,11 +77,6 @@ desc: 'a pattern parameter', behaviour: :parameter, }, - path_param: { - type: 'Variant[Stdlib::Absolutepath, Pattern[/\A(https?|ftp):\/\//]]', - desc: 'a path or URL parameter', - behaviour: :parameter, - }, url_param: { type: 'Pattern[/\A((hkp|http|https):\/\/)?([a-z\d])([a-z\d-]{0,61}\.)+[a-z\d]+(:\d{2,5})?$/]', desc: 'a hkp or http(s) url parameter', @@ -100,47 +91,42 @@ type: 'String', desc: 'a string readonly', default: 'default value', - behaviour: :readonly, + behaviour: :read_only, }, boolean_ro: { type: 'Boolean', desc: 'a boolean readonly', - behaviour: :readonly, + behaviour: :read_only, }, integer_ro: { type: 'Integer', desc: 'an integer readonly', - behaviour: :readonly, + behaviour: :read_only, }, float_ro: { type: 'Float', desc: 'a floating point readonly', - behaviour: :readonly, + behaviour: :read_only, }, ensure_ro: { type: 'Enum[present, absent]', desc: 'a ensure readonly', - behaviour: :readonly, + behaviour: :read_only, }, variant_pattern_ro: { type: 'Variant[Pattern[/\A(0x)?[0-9a-fA-F]{8}\Z/], Pattern[/\A(0x)?[0-9a-fA-F]{16}\Z/], Pattern[/\A(0x)?[0-9a-fA-F]{40}\Z/]]', desc: 'a pattern readonly', - behaviour: :readonly, - }, - path_ro: { - type: 'Variant[Stdlib::Absolutepath, Pattern[/\A(https?|ftp):\/\//]]', - desc: 'a path or URL readonly', - behaviour: :readonly, + behaviour: :read_only, }, url_ro: { type: 'Pattern[/\A((hkp|http|https):\/\/)?([a-z\d])([a-z\d-]{0,61}\.)+[a-z\d]+(:\d{2,5})?$/]', desc: 'a hkp or http(s) url readonly', - behaviour: :readonly, + behaviour: :read_only, }, optional_string_ro: { type: 'Optional[String]', desc: 'a optional string readonly', - behaviour: :readonly, + behaviour: :read_only, }, }, } @@ -171,7 +157,12 @@ def get(_context) end context 'when setting an attribute' do - let(:instance) { type.new(name: 'somename', ensure: 'present') } + let(:instance) do + type.new(name: 'somename', ensure: 'present', boolean: true, integer: 15, float: 1.23, + variant_pattern: 0xA245EED, url: 'http://www.google.com', boolean_param: false, + integer_param: 99, float_param: 3.21, ensure_param: 'present', + variant_pattern_param: '9A2222ED', url_param: 'http://www.puppet.com') + end it('flushes') { expect { instance.flush }.not_to raise_exception } diff --git a/spec/puppet/resource_api_spec.rb b/spec/puppet/resource_api_spec.rb index edfd4ecc..8a92a608 100644 --- a/spec/puppet/resource_api_spec.rb +++ b/spec/puppet/resource_api_spec.rb @@ -157,7 +157,10 @@ def extract_values(function) describe 'an instance of this type' do subject(:instance) { Puppet::Type.type(:with_string).new(params) } - let(:params) { { title: 'test' } } + let(:params) do + { title: 'test', test_boolean: true, test_integer: 15, test_float: 1.23, test_ensure: :present, + test_variant_pattern: 0xAEF123FF, test_url: 'http://example.com' } + end it('uses defaults correctly') { expect(instance[:test_string]).to eq 'default value' } @@ -183,6 +186,12 @@ def extract_values(function) it('the test_url value is set correctly') { expect(instance[:test_url]).to eq('hkp://example.com') } end + context 'when mandatory attributes are missing' do + let(:params) { { title: 'test' } } + + it { expect { instance }.to raise_exception Puppet::ResourceError, %r{The following mandatory attributes where not provided} } + end + describe 'different boolean values' do let(:params) do { @@ -191,6 +200,9 @@ def extract_values(function) test_boolean: the_boolean, test_integer: '-1', test_float: '-1.5', + test_ensure: :present, + test_variant_pattern: 'a' * 8, + test_url: 'http://example.com', } end @@ -300,7 +312,10 @@ def extract_values(function) describe 'an instance of this type' do subject(:instance) { Puppet::Type.type(:with_parameters).new(params) } - let(:params) { { title: 'test' } } + let(:params) do + { title: 'test', test_boolean: true, test_integer: 15, test_float: 1.23, test_ensure: :present, + test_variant_pattern: 0xAEF123FF, test_url: 'http://example.com' } + end it('uses defaults correctly') { expect(instance[:test_string]).to eq 'default value' } @@ -777,7 +792,7 @@ def set(_context, changes) end describe 'an existing instance' do - let(:instance) { type.new(name: 'somename') } + let(:instance) { type.new(name: 'somename', test_string: 'foo') } it('its name is set correctly') { expect(resource[:name]).to eq 'somename' } it('its properties are set correctly') do @@ -793,7 +808,7 @@ def set(_context, changes) end describe 'an absent instance' do - let(:instance) { type.new(name: 'does_not_exist') } + let(:instance) { type.new(name: 'does_not_exist', test_string: 'foo') } it('its name is set correctly') { expect(resource[:name]).to eq 'does_not_exist' } it('its properties are set correctly') do