diff --git a/lib/puppet/resource_api.rb b/lib/puppet/resource_api.rb index 0168ba1f..b3621b6a 100644 --- a/lib/puppet/resource_api.rb +++ b/lib/puppet/resource_api.rb @@ -443,18 +443,32 @@ def self.def_newvalues(this, param_or_property, *values) end end - # Validates and munges values coming from puppet into a shape palatable to the provider. - # This includes parsing values from strings, e.g. when running in `puppet resource`. + def self.caller_is_resource_app? + caller.any? { |c| c.match(%r{application/resource.rb:}) } + end + + # This method handles translating values from the runtime environment to the expected types for the provider. + # When being called from `puppet resource`, it tries to transform the strings from the command line into their + # expected ruby representations, e.g. `"2"` (a string), will be transformed to `2` (the number) if (and only if) + # the target `type` is `Integer`. + # Additionally this function also validates that the passed in (and optionally transformed) value matches the + # specified type. # @param type[Puppet::Pops::Types::TypedModelObject] the type to check/clean against # @param value the value to clean # @param error_msg_prefix[String] a prefix for the error messages # @return [type] the cleaned value # @raise [Puppet::ResourceError] if `value` could not be parsed into `type` def self.mungify(type, value, error_msg_prefix) - cleaned_value, error = try_mungify(type, value, error_msg_prefix) - - raise Puppet::ResourceError, error if error - + if caller_is_resource_app? + # When the provider is exercised from the `puppet resource` CLI, we need to unpack strings into + # the correct types, e.g. "1" (a string) to 1 (an integer) + cleaned_value, error_msg = try_mungify(type, value, error_msg_prefix) + raise Puppet::ResourceError, error_msg if error_msg + else + # Every other time, we can use the values as is + cleaned_value = value + end + Puppet::ResourceApi.validate(type, cleaned_value, error_msg_prefix) cleaned_value end @@ -527,6 +541,18 @@ def self.try_mungify(type, value, error_msg_prefix) end end + # Validates the `value` against the specified `type`. + # @param type[Puppet::Pops::Types::TypedModelObject] the type to check against + # @param value the value to clean + # @param error_msg_prefix[String] a prefix for the error messages + # @raise [Puppet::ResourceError] if `value` is not of type `type` + # @private + def self.validate(type, value, error_msg_prefix) + is_valid, error_msg = try_validate(type, value, error_msg_prefix) + + raise Puppet::ResourceError, error_msg unless is_valid + end + # Tries to validate the `value` against the specified `type`. # @param type[Puppet::Pops::Types::TypedModelObject] the type to check against # @param value the value to clean diff --git a/spec/acceptance/validation_spec.rb b/spec/acceptance/validation_spec.rb index b1d0a033..c702769e 100644 --- a/spec/acceptance/validation_spec.rb +++ b/spec/acceptance/validation_spec.rb @@ -78,6 +78,12 @@ expect(status.exitstatus).to eq 0 end + it 'rejects stringified integers' do + output, status = Open3.capture2e("puppet apply #{common_args} -e \"test_validation{ foo: prop => '2', param => '3' }\"") + expect(output.strip).to match %r{Parameter prop failed on Test_validation\[foo\]: test_validation.prop expects an Integer value, got String}i + expect(status.exitstatus).to eq 1 + end + it 'allows creating' do output, status = Open3.capture2e("puppet apply #{common_args} -e \"test_validation{ new: prop => 2, param => 3 }\"") expect(output.strip).to match %r{Test_validation\[new\]/ensure: defined 'ensure' as 'present'} diff --git a/spec/puppet/resource_api_spec.rb b/spec/puppet/resource_api_spec.rb index 55dbd2ea..152bcca4 100644 --- a/spec/puppet/resource_api_spec.rb +++ b/spec/puppet/resource_api_spec.rb @@ -185,63 +185,20 @@ def extract_values(function) it('the test_url value is set correctly') { expect(instance[:test_url]).to eq('hkp://example.com') } end - context 'when setting string values for the attributes' do - let(:params) do - { - title: 'test', - test_string: 'somevalue', - test_boolean: 'true', - test_integer: '-1', - test_float: '-1.5', - test_enum: 'a', - test_variant_pattern: 'a' * 8, - test_url: 'hkp://example.com', - } - end - - it('the test_string value is set correctly') { expect(instance[:test_string]).to eq 'somevalue' } - it('the test_integer value is set correctly') { expect(instance[:test_integer]).to eq(-1) } - it('the test_float value is set correctly') { expect(instance[:test_float]).to eq(-1.5) } - it('the test_enum value is set correctly') { expect(instance[:test_enum]).to eq('a') } - it('the test_variant_pattern value is set correctly') { expect(instance[:test_variant_pattern]).to eq('a' * 8) } - it('the test_url value is set correctly') { expect(instance[:test_url]).to eq('hkp://example.com') } - end - describe 'different boolean values' do let(:params) do { title: 'test', test_string: 'somevalue', test_boolean: the_boolean, - test_integer: '-1', - test_float: '-1.5', + test_integer: -1, + test_float: -1.5, test_enum: 'a', test_variant_pattern: 'a' * 8, test_url: 'http://example.com', } end - # rubocop:disable Lint/BooleanSymbol - context 'when using :true' do - let(:the_boolean) { :true } - - it('the test_boolean value is set correctly') { expect(instance[:test_boolean]).to eq :true } - end - context 'when using :false' do - let(:the_boolean) { :false } - - it('the test_boolean value is set correctly') { expect(instance[:test_boolean]).to eq :false } - end - context 'when using "true"' do - let(:the_boolean) { 'true' } - - it('the test_boolean value is set correctly') { expect(instance[:test_boolean]).to eq :true } - end - context 'when using "false"' do - let(:the_boolean) { 'false' } - - it('the test_boolean value is set correctly') { expect(instance[:test_boolean]).to eq :false } - end context 'when using true' do let(:the_boolean) { true } @@ -255,9 +212,13 @@ def extract_values(function) 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} } + it('an error is raised') { expect { instance }.to raise_error Puppet::ResourceError, %r{test_boolean expect.* Boolean .* got String} } + end + context 'when using a legacy symbol' do + let(:the_boolean) { :true } # rubocop:disable Lint/BooleanSymbol + + it('an error is raised') { expect { instance }.to raise_error Puppet::ResourceError, %r{test_boolean expect.* Boolean .* got Runtime} } end - # rubocop:enable Lint/BooleanSymbol end context 'with a basic provider', agent_test: true do @@ -535,9 +496,9 @@ def set(_context, _changes); end { parameters: { name: 'test', test_string: 'somevalue', - test_boolean: 'true', - test_integer: '-1', - test_float: '-1.5', + test_boolean: true, + test_integer: -1, + test_float: -1.5, test_variant_pattern: 'a' * 8, test_url: 'hkp://example.com', } } @@ -1500,19 +1461,39 @@ def set(_context, changes) end end describe '#mungify(type, value)' do - before(:each) do - allow(described_class).to receive(:try_mungify).with('type', 'input', 'error prefix').and_return(result) - end - context 'when the munge succeeds' do - let(:result) { ['result', nil] } + context 'when called from `puppet resource`' do + before(:each) do + expect(described_class).to receive(:caller_is_resource_app?).with(no_args).and_return(true) + expect(described_class).to receive(:try_mungify).with('type', 'input', 'error prefix').and_return(result) + end + + let(:caller_is_resource_app) { true } + + context 'when the munge succeeds' do + let(:result) { ['result', nil] } + + before(:each) do + expect(described_class).to receive(:validate).with('type', result[0], 'error prefix') + end - it('returns the cleaned result') { expect(described_class.mungify('type', 'input', 'error prefix')).to eq 'result' } + it('returns the cleaned result') { expect(described_class.mungify('type', 'input', 'error prefix')).to eq 'result' } + end + + context 'when the munge fails' do + let(:result) { [nil, 'some error'] } + + it('raises the error') { expect { described_class.mungify('type', 'input', 'error prefix') }.to raise_error Puppet::ResourceError, %r{\Asome error\Z} } + end end - context 'when the munge fails' do - let(:result) { [nil, 'some error'] } + context 'when called from something else' do + before(:each) do + expect(described_class).to receive(:caller_is_resource_app?).with(no_args).and_return(false) + expect(described_class).to receive(:try_mungify).never + expect(described_class).to receive(:validate).with('type', 'input', 'error prefix') + end - it('raises the error') { expect { described_class.mungify('type', 'input', 'error prefix') }.to raise_error Puppet::ResourceError, %r{\Asome error\Z} } + it('returns the value') { expect(described_class.mungify('type', 'input', 'error prefix')).to eq 'input' } end end