Deprecate the validate_legacy() function
#1353
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
smortex
requested review from
a team,
alexjfisher,
b4ldr,
bastelfreak and
ekohl
as code owners
`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.
smortex
force-pushed
the
deprecate-validate-legacy
branch
from
3539d65
to
1bcbf87
Compare
david22swan
approved these changes
May 15, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a follow-up to #1352 and in particular #1352 (comment).
I reproduce the content bellow so that we can discuss this, and provide an implementation.
Context
validate_legacy()was used with a data type and a validation function. If the value was matched by both the data type and the validation function, it produced no warning, otherwise it produced a warning if the new data type was matched but the legacy validation function was not accepting the value or an deprecation message if the data type was not matched but the legacy validation was accepted. The validation functions used are the ones being removed by this PR, sovalidate_legacy()needs to be taken care of.The table bellow summarize the current behavior of
validate_legacy():Assumptions
The last line (fail/fail) is not expected to be seen into the wild because the compilation failure makes modules unusable.
For the 3 first line, only the first one does not produce annoying deprecation warning / notice, so we expect that such issues have been fixed.
Proposal
Update the function to always emit a deprecation message, and only validate the value against the provided data type. The validation function is therefore completely ignored (but the function prototype is not changed for backwards compatibility).
The table bellow summarize the proposed behavior of
validate_legacy():