Skip to content

Commit

Permalink
(PDK-536) Proper datatype parsing and checking
Browse files Browse the repository at this point in the history 
This uses Puppet4+ pops to parse and check datatypes. This allows types
to use the full range of expressibility when declaring attributes.
  • Loading branch information
@DavidS
DavidS committed Feb 21, 2018
1 parent b069db2 commit 0764325
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 38 deletions.
48 changes: 30 additions & 18 deletions lib/puppet/resource_api.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ def my_provider
# if type.instance?(v)
# return true
# else
# inferred_type = Puppet::Pops::Types::TypeCalculator.infer_set(value)
# inferred_type = Puppet::Pops::Types::TypeCalculator.infer_set(v)
# error_msg = Puppet::Pops::Types::TypeMismatchDescriber.new.describe_mismatch("#{DEFINITION[:name]}.#{name}", type, inferred_type)
# raise Puppet::ResourceError, error_msg
# end
Expand All @@ -99,12 +99,33 @@ def my_provider
defaultto options[:default]
end

case options[:type]
when 'String'
type = Puppet::Pops::Types::TypeParser.singleton.parse(options[:type])
validate do |value|
return true if type.instance?(value)

inferred_type = Puppet::Pops::Types::TypeCalculator.infer_set(value)
if inferred_type.is_a? Puppet::Pops::Types::PStringType
# when running under `puppet resource`, we need to try to coerce from strings to the real type
case value
when %r{^-?\d+$}, Puppet::Pops::Patterns::NUMERIC
value = Puppet::Pops::Utils.to_n(value)
when %r{\Atrue|false\Z}
value = value == 'true'
end
return true if type.instance?(value)

error_msg = Puppet::Pops::Types::TypeMismatchDescriber.new.describe_mismatch("#{options[:name]}.#{name}", type, inferred_type)
raise Puppet::ResourceError, error_msg
end
end

# provide better handling of the standard types
case type
when Puppet::Pops::Types::PStringType
# require any string value
Puppet::ResourceApi.def_newvalues(self, param_or_property, %r{})
# rubocop:disable Lint/BooleanSymbol
when 'Boolean'
when Puppet::Pops::Types::PBooleanType
Puppet::ResourceApi.def_newvalues(self, param_or_property, 'true', 'false')
aliasvalue true, 'true'
aliasvalue false, 'false'
Expand All @@ -122,30 +143,21 @@ def my_provider
end
end
# rubocop:enable Lint/BooleanSymbol
when 'Integer'
when Puppet::Pops::Types::PIntegerType
Puppet::ResourceApi.def_newvalues(self, param_or_property, %r{^-?\d+$})
munge do |v|
Puppet::Pops::Utils.to_n(v)
end
when 'Float', 'Numeric'
when Puppet::Pops::Types::PFloatType, Puppet::Pops::Types::PNumericType
Puppet::ResourceApi.def_newvalues(self, param_or_property, Puppet::Pops::Patterns::NUMERIC)
munge do |v|
Puppet::Pops::Utils.to_n(v)
end
end

case options[:type]
when 'Enum[present, absent]'
Puppet::ResourceApi.def_newvalues(self, param_or_property, :absent, :present)
when '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/]]'
# the namevar needs to be a Parameter, which only has newvalue*s*
Puppet::ResourceApi.def_newvalues(self, param_or_property, %r{\A(0x)?[0-9a-fA-F]{8}\Z}, %r{\A(0x)?[0-9a-fA-F]{16}\Z}, %r{\A(0x)?[0-9a-fA-F]{40}\Z})
when 'Optional[String]'
Puppet::ResourceApi.def_newvalues(self, param_or_property, %r{}, :undef)
when 'Variant[Stdlib::Absolutepath, Pattern[/\A(https?|ftp):\/\//]]'
# TODO: this is wrong, but matches original implementation
Puppet::ResourceApi.def_newvalues(self, param_or_property, /^\//, /\A(https?|ftp):\/\//) # rubocop:disable Style/RegexpLiteral
when 'Pattern[/\A((hkp|http|https):\/\/)?([a-z\d])([a-z\d-]{0,61}\.)+[a-z\d]+(:\d{2,5})?$/]'
Puppet::ResourceApi.def_newvalues(self, param_or_property, /\A((hkp|http|https):\/\/)?([a-z\d])([a-z\d-]{0,61}\.)+[a-z\d]+(:\d{2,5})?$/) # rubocop:disable Style/RegexpLiteral
else
raise Puppet::DevError, "Datatype #{options[:type]} is not yet supported in this prototype"
end
end
end
Expand Down
6 changes: 1 addition & 5 deletions spec/acceptance/device_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,7 @@
describe 'using `puppet resource`' do
it 'reads resources from the target system' do
stdout_str, status = Open3.capture2e("puppet resource #{common_args} device_provider")
if Gem::Version.new(RUBY_VERSION.dup) < Gem::Version.new('2.4')
expect(stdout_str.strip).to eq 'DL is deprecated, please use Fiddle'
else
expect(stdout_str.strip).to be_empty
end
expect(stdout_str.strip).to match %r{\A()|(DL is deprecated, please use Fiddle)\Z}
expect(status).to eq 0
end
it 'manages resources on the target system' do
Expand Down
29 changes: 14 additions & 15 deletions spec/puppet/resource_api_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,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 value',
},
test_path: {
type: 'Variant[Stdlib::Absolutepath, Pattern[/\A(https?|ftp):\/\//]]',
desc: 'a path or URL',
},
test_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',
Expand Down Expand Up @@ -107,7 +103,6 @@
test_float: '-1.5',
test_ensure: 'present',
test_variant_pattern: 'a' * 8,
test_path: '/var/log/example',
test_url: 'hkp://example.com',
}
end
Expand All @@ -117,7 +112,6 @@
it('the test_float value is set correctly') { expect(instance[:test_float]).to eq(-1.5) }
it('the test_ensure value is set correctly') { expect(instance[:test_ensure]).to eq(:present) }
it('the test_variant_pattern value is set correctly') { expect(instance[:test_variant_pattern]).to eq('a' * 8) }
it('the test_path value is set correctly') { expect(instance[:test_path]).to eq('/var/log/example') }
it('the test_url value is set correctly') { expect(instance[:test_url]).to eq('hkp://example.com') }
end

Expand Down Expand Up @@ -162,6 +156,11 @@

it('the test_boolean value is set correctly') { expect(instance[:test_boolean]).to eq false }
end
context 'when using an unparsable value' do
let(:the_boolean) { 'flubb' }

it('the test_boolean value is set correctly') { expect { instance }.to raise_error Puppet::ResourceError, %r{test_boolean expect.* Boolean .* got String} }
end
end
end
end
Expand Down Expand Up @@ -207,11 +206,6 @@
desc: 'a pattern parameter',
behaviour: :parameter,
},
test_path: {
type: 'Variant[Stdlib::Absolutepath, Pattern[/\A(https?|ftp):\/\//]]',
desc: 'a path or URL parameter',
behaviour: :parameter,
},
test_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 @@ -252,7 +246,6 @@
test_float: '-1.5',
test_ensure: 'present',
test_variant_pattern: 'a' * 8,
test_path: '/var/log/example',
test_url: 'hkp://example.com',
}
end
Expand All @@ -262,13 +255,12 @@
it('the test_float value is set correctly') { expect(instance[:test_float]).to eq(-1.5) }
it('the test_ensure value is set correctly') { expect(instance[:test_ensure]).to eq(:present) }
it('the test_variant_pattern value is set correctly') { expect(instance[:test_variant_pattern]).to eq('a' * 8) }
it('the test_path value is set correctly') { expect(instance[:test_path]).to eq('/var/log/example') }
it('the test_url value is set correctly') { expect(instance[:test_url]).to eq('hkp://example.com') }
end
end
end

context 'when registering an attribute with an invalid data type' do
context 'when registering an attribute with an optional data type' do
let(:definition) do
{
name: 'no_type',
Expand All @@ -281,7 +273,14 @@
}
end

it { expect { described_class.register_type(definition) }.to raise_error Puppet::DevError, %r{is not yet supported in this prototype} }
it { expect { described_class.register_type(definition) }.not_to raise_error }

describe 'the registered type' do
subject(:type) { Puppet::Type.type(:with_parameters) }

it { is_expected.not_to be_nil }
it { expect(type.parameters[1]).to eq :test_string }
end
end

context 'when registering a type with a malformed attributes' do
Expand Down

0 comments on commit 0764325

Please sign in to comment.