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.

Sign up for GitHub

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

Jump to bottom

(PDK-819) Ensure checks for mandatory type attributes #40

Merged
merged 1 commit into from
Mar 23, 2018
Merged

(PDK-819) Ensure checks for mandatory type attributes #40

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions lib/puppet/resource_api.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}"

Expand Down Expand Up @@ -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
Expand Down
18 changes: 13 additions & 5 deletions spec/acceptance/device_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'] }

Expand All @@ -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
Expand All @@ -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"\}'
Expand All @@ -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"\}'
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
30 changes: 8 additions & 22 deletions spec/fixtures/test_module/lib/puppet/type/device_provider.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand All @@ -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,
},
},
)
2 changes: 1 addition & 1 deletion spec/fixtures/test_module/lib/puppet/type/test_autorequire.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
behaviour: :namevar,
},
target: {
type: 'String',
type: 'Optional[String]',
desc: 'The resource to autorequire.',
behaviour: :parameter,
},
Expand Down
2 changes: 1 addition & 1 deletion spec/fixtures/test_module/lib/puppet/type/test_simple_get_filter.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
behaviour: :namevar,
},
test_string: {
type: 'String',
type: 'Optional[String]',
desc: 'Used for testing our expected outcomes',
},
},
Expand Down
37 changes: 14 additions & 23 deletions spec/integration/resource_api_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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',
Expand All @@ -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,
},
},
}
Expand Down Expand Up @@ -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 }

Expand Down
23 changes: 19 additions & 4 deletions spec/puppet/resource_api_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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' }

Expand All @@ -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
{
Expand All @@ -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

Expand Down Expand Up @@ -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' }

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down