Issue #1478 Avoid validate_legacy deprecation failure on Puppet 8

[alex-harvey-z3q]

Summary

Fixes the deprecated validate_legacy() function so that it emits a deprecation warning, as intended, and no longer lets Puppet's global strict setting turn that warning into a catalog failure.

Note that the stdlib deprecation helper accepts an optional fourth argument named use_strict_setting. It defaults to true, which means the helper follows Puppet.settings[:strict]:

• :warning logs a warning
• :error raises an error
• :off suppresses the message

This PR changes the validate_legacy() call to pass false for that argument:

call_function('deprecation', 'validate_legacy', 'This method is deprecated, please use Puppet data types to validate parameters', false)

With use_strict_setting set to false, validate_legacy() still calls the deprecation helper and still emits the warning, but the warning is always handled as a soft deprecation warning instead of being promoted to an error under strict mode.

That preserves the migration signal for module maintainers while avoiding a hard failure before validate_legacy() can perform its type assertion.

Existing stdlib precedent

This matches the pattern already used by stdlib's generated unnamespaced compatibility shims. Those functions are deprecated, emit a deprecation warning, and then delegate to the namespaced replacement while passing false to deprecation() so the compatibility path remains usable under strict mode.

Existing examples include:

• merge
• ensure_packages
• batch_escape
• fqdn_rand_string
• fqdn_rotate
• has_interface_with
• has_ip_address
• has_ip_network
• os_version_gte
• parsehocon
• powershell_escape
• seeded_rand
• seeded_rand_string
• shell_escape
• to_json
• to_json_pretty
• to_python
• to_ruby
• to_toml
• to_yaml
• type_of
• validate_domain_name
• validate_email_address

The specs have also been updated to assert that validate_legacy() continues to call the deprecation helper with use_strict_setting set to false.

Checklist

• Spec tests.
• Acceptance tests.
• Manually verified.

[CLAassistant]

All committers have signed the CLA.

[alex-harvey-z3q]

@CLAassistant Note that I changed my GitHub handle from alexharv074 to alex-harvey-z3q which could explain why it shows me as having not accepted the CLA. I've accepted again.

[alex-harvey-z3q]

The failing build does not appear related to my PR.

[jst-cyr]

@alex-harvey-z3q : Thanks for putting in this PR. If I'm reading your request correctly, you have a scenario where you are running on strict mode so the deprecation on validate_legacy is causing a full error on other modules.

Have you identified which module(s) in your dependency tree are still calling validate_legacy (or the older validate_* functions) directly? If it's something you maintain, updating those modules seems like the more durable fix than patching stdlib's runtime behavior. If it's a third-party module you don't control, perhaps we can get PRs in on those modules to fix their use of a deprecated function.

stdlib has had this method deprecated for three years, and the fix here would actually remove deprecation warnings for non-strict (not only strict) mode meaning it would lose the messaging to module maintainers that they need to update.

[alex-harvey-z3q]

@jst-cyr Yes I see your point.

How is this updated version of the PR that keeps the validate_legacy deprecation warning, but passes false for the strict-setting flag when calling deprecation. I think that matches the pattern already used?

E.g. lib/puppet/functions/validate_legacy.rb (line 55)

I will also trace the dependency tree that is still calling validate_legacy/older validate_* functions, but hopefully this change should avoid breaking strict-mode Puppet 8 consumers while those downstream fixes make their way through?

[smortex]

Given these functions where deprecated in 9.0.0, I would have expected them to be removed in 10.0.0 (but they have not). I really do not think that making them less "harmful" is a move in the right direction.

[alex-harvey-z3q]

Given these functions where deprecated in 9.0.0, I would have expected them to be removed in 10.0.0 (but they have not). I really do not think that making them less "harmful" is a move in the right direction.

I have different expectations because this is enterprise software. I would expect enterprise software not to have bugs in it, and I would expect the maintainers of enterprise software to understand that enterprises move slowly.

[alexjfisher]

Given these functions where deprecated in 9.0.0, I would have expected them to be removed in 10.0.0 (but they have not). I really do not think that making them less "harmful" is a move in the right direction.

By removing the function, you make it harder for people to upgrade to newer stdlib.
By not accepting changes like this, you make it harder for people to upgrade Puppet, (or make them disable strict mode at the expense of not getting its benefits elsewhere).

[cvquesty]

LGTM

[cvquesty]

I'm with Alex fisher... we do need to make attempts to keep the barrier lower and the file changes nominal if we can. I think it's important to realize, as mentioned, that we're all volunteers but at the same time this project exists because the puppet folks couldn't keep momentum nor get to the really necessary updates and changes in a timely fashion largely due to blast radius, being overwhelmed when there are indeed several changes that need to happen, thus they did nothing (or next to it...or moved painfully slow)

The conversation around appropriateness for a purpose is important, though. we can't prefer one over the other, and have to strike a medium balance here in that yes, we need to see these as enterprise software but at the same time, there is a mechanism to duplicate and expand on forge modules that are poor or not meeting one's needs. The validate functions have been gone for quite some time now (3 years), but this represents 10 releases. I think they can safely be let go now.

[jst-cyr]

My apologies, been out of office and this conversation took off and I'm getting caught up here. I see an approval on the changes here, but the conversation seems to indicate that we SHOULDN'T be removing deprecation warnings.

From my view, I agree that we should keep these deprecated and then remove them completely in the next major version (it was not done in stdlib 10 to keep things stable for that release).

So it seems to me that the correct fix for the situation that @alex-harvey-z3q finds themself into is updating the references to the old validate functions instead of changing stdlib to avoid strict mode errors?

[alex-harvey-z3q]

@jst-cyr Thanks, and we already agreed that we should not remove the deprecation warning.

That is not what the updated PR is doing, and that is not what the conversation above is about. The PR still calls the stdlib deprecation helper, so the warning remains visible to module maintainers. The change is only to pass false for the helper's use_strict_setting argument, so this particular compatibility function continues to warn without Puppet strict mode promoting the warning to a hard catalog failure.

I have updated the PR description to make that clearer and added links to all of the (many) existing stdlib compatibility shims that already use the same deprecation(..., false) pattern.

[jst-cyr]

@alex-harvey-z3q : Thank you, I definitely misread the flow of the conversation while trying to catch up. I see now the changes have been updated to match with other deprecations in the module instead of the original changes that had proposed removing the deprecation warning altogether.

I had already added the PR into the list for engineering review, but given that the change has been reduced and scope is minimal and resolves a bug I've boosted it a bit versus some other changes in the backlog.

[alex-harvey-z3q]

Thank you I appreciate it @jst-cyr !