Skip to content

Remove deprecated functions #1352

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.

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

Merged
merged 30 commits into from
May 11, 2023

Conversation

smortex
Copy link
Collaborator

@smortex smortex commented May 4, 2023

This PR remove a bunch of legacy functions that are now part of Puppet or which where added to stdlib to allow parameters validation prior to Puppet 4 introduction of data types.

All these functions produced deprecation warnings.

Nots:

  • The has_interface_with() function deprecation was excluded because it is a "new" deprecation due to the function being namespaced (see RFC: Convert all Puppet 4.x API functions to namespaced variants #1346 for details).
  • The validate_legacy() function was also kept. It is not used anymore by the module, but prior to removing it we may want to deprecate it.

@smortex

This comment was marked as resolved.

@smortex smortex force-pushed the remove-deprecated-functions branch from 05890d6 to d728fdf Compare May 5, 2023 00:06
smortex added 22 commits May 8, 2023 07:33
> This method is deprecated, please use the stdlib validate_legacy
> function, with Stdlib::Compat::Bool.
> This method is deprecated, please use the stdlib validate_legacy
> function, with Stdlib::Compat::Bool.
> This method is deprecated, please use the stdlib validate_legacy
> function, with Stdlib::Compat::String.
> This method is deprecated, please use the stdlib validate_legacy
> function, with Stdlib::Compat::String.
> This method is deprecated, please use the stdlib validate_legacy
> function, with Stdlib::Compat::Numeric.
> This method is deprecated, please use the stdlib validate_legacy
> function, with Stdlib::Compat::Numeric.
> This method is deprecated, please use the stdlib validate_legacy
> function, with Stdlib::Compat::Array.
> This method is deprecated, please use the stdlib validate_legacy
> function, with Stdlib::Compat::Integer.
> This method is deprecated, please use the stdlib validate_legacy
> function, with Stdlib::Compat::Integer.
> This method is deprecated, please use the stdlib validate_legacy
> function, with Stdlib::Compat::Hash.
> This method is deprecated, please use match expressions with
> Stdlib::Compat::Float instead.
> This method is deprecated, please use the stdlib validate_legacy
> function, with Stdlib::Compat::Hash.
> This method is deprecated, please use the stdlib validate_legacy
> function, with Stdlib::Compat::Absolute_path.
> This method is deprecated, please use the stdlib validate_legacy
> function, with Stdlib::Compat::Re.
> This method is deprecated, please use the stdlib validate_legacy
> function, with String.
> This method is deprecated, please use match expressions with
> Stdlib::Compat::Ipv6
> This method is deprecated, please use the stdlib validate_legacy function,
> with Stdlib::Compat::Ipv6.
> This method is deprecated, please use match expressions with
> Stdlib::Compat::Ipv4.
> This method is deprecated, please use the stdlib validate_legacy function,
> with Stdlib::Compat::IPv4.
> This method is deprecated, please use match expressions with
> Stdlib::Compat::Ip_address.
> This method is deprecated, please use the stdlib validate_legacy function,
> with Stdlib::Compat::Ip_Address.
> The length() function in Puppet is preferred over this.
smortex added 8 commits May 8, 2023 07:33
> From Puppet 4.10.10/5.3.4 please use the built-in sprintf instead
> Deprecated 3x version of the `ensure_packages` function
> Will be removed in a future version of stdlib.
> Will be removed in a future version of stdlib.
> Will be removed in a future version of stdlib.
> Will be removed in a future version of stdlib.
> From Puppet 6.0.0, this function has been replaced with a built-in function.
These data types where introduced in Puppet 4 when deprecating the
validate_* functions and provided backwards compatibility with older
versions of Puppet where all parameters where String.

With Puppet 4, we got the ability to manage proper data types and it is
now time to remove these compatibility types.
@smortex smortex force-pushed the remove-deprecated-functions branch from d728fdf to fcbd426 Compare May 8, 2023 17:34
@smortex
Copy link
Collaborator Author

smortex commented May 8, 2023

[RFC] I started some research to determine how we should remove validate_legacy().

It appear that while there are a lot of calls to these functions into the wild, they are from numerous vendors and the "puppet-centric-shops" are quite good students with few usage of it 💯:

  • Puppetlabs, 1 module affected:
    • puppetlabs/puppetlabs-wsus_client
  • Voxpupuli, 1 module affected:
    • voxpupuli/puppet-dropbear

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, so validate_legacy() needs to be taken care of.

The table bellow summarize the current behavior of validate_legacy():

Data Type Legacy function result
✔️ pass ✔️ pass ✔️ pass
✔️ pass ❌ fail ⚠️ notice (Accepting previously invalid value for target type)
❌ fail ✔️ pass ⚠️ deprecation(validate_legacy)
❌ fail ❌ fail ❌ fail

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():

Data Type Legacy function result
✔️ pass ❔ ignored ⚠️ deprecation(replace validate_legacy() with data type)
❌ fail ❔ ignored ❌ fail

Copy link
Contributor

@LukasAud LukasAud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@LukasAud LukasAud merged commit f8845d3 into puppetlabs:main May 11, 2023
@traylenator
Copy link
Contributor

traylenator commented May 11, 2023

Amazing is_integer('1234') with a string used to be true , Pattern[/\d+/] it is.

@smortex
Copy link
Collaborator Author

smortex commented May 11, 2023

D'oh! The case for #1352 (comment) was still unresolved 🙃 (maybe I should have made this a draft).

Looks like I had a 🚀 reaction that might indicate it makes sense, so I'll fill in a PR with the proposed change and we will be able to focus on this discussion there. Will do this when I reach $WORK later today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants