From 1bcbf877a879675b9fa69ebf74676f375cd951c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Romain=20Tarti=C3=A8re?= Date: Thu, 11 May 2023 08:29:58 -1000 Subject: [PATCH] Deprecate the `validate_legacy()` function `validate_legacy()` was added to help migrating from the legacy validation functions to the new Puppet data types which where not 100% backward compatible with the previous validation (e.g. `'42'` was a valid integer according to `validate_integer()` but indeed a String and not an Integer when using data types. The legacy validation functions have been removed from stdlib, so `validate_legacy()` will now fail if it tries to run them. But as they produced deprecation warning, they are supposed to have already been fixed. For the sake of security, instead of removing `validate_legacy()` now, deprecate it so that it is strictly equivalent to validating using a data type, but also emits a warning. --- lib/puppet/functions/validate_legacy.rb | 32 +++++++--------------- spec/functions/validate_legacy_spec.rb | 36 +++++++++---------------- 2 files changed, 21 insertions(+), 47 deletions(-) diff --git a/lib/puppet/functions/validate_legacy.rb b/lib/puppet/functions/validate_legacy.rb index 44335fb6f..fb8d9c687 100644 --- a/lib/puppet/functions/validate_legacy.rb +++ b/lib/puppet/functions/validate_legacy.rb @@ -1,13 +1,14 @@ # frozen_string_literal: true # @summary -# Validate a value against both the target_type (new) and the previous_validation function (old). +# **Deprecated:** Validate a value against both the target_type (new). Puppet::Functions.create_function(:validate_legacy) do - # The function checks a value against both the target_type (new) and the previous_validation function (old). + # The function checks a value against both the target_type (new). # @param scope # The main value that will be passed to the method # @param target_type # @param function_name + # Unused # @param value # @param args # Any additional values that are to be passed to the method @@ -25,6 +26,7 @@ # The main value that will be passed to the method # @param type_string # @param function_name + # Unused # @param value # @param args Any additional values that are to be passed to the method # @return Legacy validation method @@ -49,33 +51,17 @@ def validate_legacy_s(scope, type_string, *args) validate_legacy(scope, t, *args) end - def validate_legacy(scope, target_type, function_name, value, *prev_args) + def validate_legacy(_scope, target_type, _function_name, value, *_prev_args) + call_function('deprecation', 'validate_legacy', 'This method is deprecated, please use Puppet data types to validate parameters') if assert_type(target_type, value) - if previous_validation(scope, function_name, value, *prev_args) - # Silently passes - else - Puppet.notice("Accepting previously invalid value for target type '#{target_type}'") - end + # "Silently" passes else inferred_type = Puppet::Pops::Types::TypeCalculator.infer_set(value) - error_msg = Puppet::Pops::Types::TypeMismatchDescriber.new.describe_mismatch("validate_legacy(#{function_name})", target_type, inferred_type) - if previous_validation(scope, function_name, value, *prev_args) - call_function('deprecation', 'validate_legacy', error_msg) - else - call_function('fail', error_msg) - end + error_msg = Puppet::Pops::Types::TypeMismatchDescriber.new.describe_mismatch("validate_legacy(#{target_type}, ...)", target_type, inferred_type) + call_function('fail', error_msg) end end - def previous_validation(scope, function_name, value, *prev_args) - # Call the previous validation function and catch any errors. Return true if no errors are thrown. - - scope.send("function_#{function_name}".to_s, [value, *prev_args]) - true - rescue Puppet::ParseError - false - end - def assert_type(type, value) Puppet::Pops::Types::TypeCalculator.instance?(type, value) end diff --git a/spec/functions/validate_legacy_spec.rb b/spec/functions/validate_legacy_spec.rb index 0998cf618..4ab955334 100644 --- a/spec/functions/validate_legacy_spec.rb +++ b/spec/functions/validate_legacy_spec.rb @@ -7,41 +7,28 @@ it { is_expected.not_to eq(nil) } it { is_expected.to run.with_params.and_raise_error(ArgumentError) } - describe 'when passing the type assertion and passing the previous validation' do - it 'passes without notice' do - expect(scope).to receive(:function_validate_foo).with([5]).once + describe 'when passing the type assertion' do + it 'passes with a deprecation warning' do + expect(subject.func).to receive(:call_function).with('deprecation', 'validate_legacy', include('deprecated')).once + expect(scope).to receive(:function_validate_foo).never expect(Puppet).to receive(:notice).never is_expected.to run.with_params('Integer', 'validate_foo', 5) end end - describe 'when passing the type assertion and failing the previous validation' do - it 'passes with a notice about newly accepted value' do - expect(scope).to receive(:function_validate_foo).with([5]).and_raise(Puppet::ParseError, 'foo').once - expect(Puppet).to receive(:notice).with(include('Accepting previously invalid value for target type')) - is_expected.to run.with_params('Integer', 'validate_foo', 5) - end - end - - describe 'when failing the type assertion and passing the previous validation' do - it 'passes with a deprecation message' do - expect(scope).to receive(:function_validate_foo).with(['5']).once - expect(subject.func).to receive(:call_function).with('deprecation', 'validate_legacy', include('Integer')).once - is_expected.to run.with_params('Integer', 'validate_foo', '5') - end - end - - describe 'when failing the type assertion and failing the previous validation' do + describe 'when failing the type assertion' do it 'fails with a helpful message' do - expect(scope).to receive(:function_validate_foo).with(['5']).and_raise(Puppet::ParseError, 'foo').once - expect(subject.func).to receive(:call_function).with('fail', include('Integer')).once + expect(subject.func).to receive(:call_function).with('deprecation', 'validate_legacy', include('deprecated')).once + expect(scope).to receive(:function_validate_foo).never + expect(subject.func).to receive(:call_function).with('fail', 'validate_legacy(Integer, ...) expects an Integer value, got String').once is_expected.to run.with_params('Integer', 'validate_foo', '5') end end describe 'when passing in undef' do it 'works' do - expect(scope).to receive(:function_validate_foo).with([:undef]).once + expect(subject.func).to receive(:call_function).with('deprecation', 'validate_legacy', include('deprecated')).once + expect(scope).to receive(:function_validate_foo).never expect(Puppet).to receive(:notice).never is_expected.to run.with_params('Optional[Integer]', 'validate_foo', :undef) end @@ -49,7 +36,8 @@ describe 'when passing in multiple arguments' do it 'passes with a deprecation message' do - expect(scope).to receive(:function_validate_foo).with([:undef, 1, 'foo']).once + expect(subject.func).to receive(:call_function).with('deprecation', 'validate_legacy', include('deprecated')).once + expect(scope).to receive(:function_validate_foo).never expect(Puppet).to receive(:notice).never is_expected.to run.with_params('Optional[Integer]', 'validate_foo', :undef, 1, 'foo') end