(MODULES-7554) Fix crash on Puppet 4 / WMF < 5 #72
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
- 914d934 introduced a new feature for dsc_lite. In situations where the proper WMF5 version was not available, a warning would be generated using the Puppet.warn_once helper. However, it was not realized that Puppet 4 and Puppet 5 have different numbers of arguments that can be passed to warn_once, and that only Puppet 5 supported changing warning to error. That change was introduced in puppetlabs/puppet@3d82337 - Calling a Ruby method with too many arguments causes a Ruby crash and results in a completely terminated Puppet run that produces no report for the master. In other words, this is a completely catastrophic failure case. - Resolve this problem by checking the arity of the Puppet.warn_once call to send the right number of arguments on Puppet 4. The concession was made to vary the message type on Puppet 4 (warning) vs Puppet 5 (error), because the alternative solution did not preserve the warn_once style behavior. Instead, it would have produced an error for every single dsc_lite resource, which was far less desirable.
| ] | ||
|
|
||
| # Puppet 5 allows changing warning to error | ||
| params << :err if Puppet.method(:warn_once).arity > 5 |
There was a problem hiding this comment.
Should we leave a message to "de-splat" when puppet 4 reaches EOL?
There was a problem hiding this comment.
Maybe just file a ticket for later? I could go either way...
There was a problem hiding this comment.
Ticket ensures it's added to a backlog with a priority equal to other work. A todo in code is forgotten until the file is looked at again.
jpogran
approved these changes
Jul 31, 2018
ThoughtCrhyme
approved these changes
Jul 31, 2018
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.
914d934 introduced a new feature for
dsc_lite. In situations where the proper WMF5 version was not
available, a warning would be generated using the Puppet.warn_once
helper.
However, it was not realized that Puppet 4 and Puppet 5 have
different numbers of arguments that can be passed to warn_once, and
that only Puppet 5 supported changing warning to error.
That change was introduced in
puppetlabs/puppet@3d82337
Calling a Ruby method with too many arguments causes a Ruby crash and
results in a completely terminated Puppet run that produces no report
for the master. In other words, this is a completely catastrophic
failure case.
Resolve this problem by checking the arity of the Puppet.warn_once
call to send the right number of arguments on Puppet 4.
The concession was made to vary the message type on Puppet 4 (warning)
vs Puppet 5 (error), because the alternative solution did not
preserve the warn_once style behavior. Instead, it would have
produced an error for every single dsc_lite resource, which was far
less desirable.