[PUP-8320] True/truthy inconsistencies between function filter() and functions all() and any() Created: 2018/01/05  Updated: 2018/09/19  Resolved: 2018/09/17

Status: Closed
Project: Puppet
Component/s: Docs
Affects Version/s: None
Fix Version/s: PUP 6.0.0

Type: Improvement Priority: Normal
Reporter: Thomas Hallgren Assignee: Unassigned
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Template:
Sub-team: Language
Team: Platform Core
Story Points: 1
Release Notes: Bug Fix
Release Notes Summary: The {{filter}} function did not accept truthy value returned from the block as indication of values to include in the result. Only exactly boolean {{true}} was earlier accepted.
QA Risk Assessment: Needs Assessment

 Description   

There are inconsistencies in how the result of a predicate block sent to the Puppet functions all, any, and filter is acted upon.

The filter function checks if the value returned from the block is exactly equal to the literal true whereas the all and any functions checks if it is truthy (i.e. not literal false or undef).

Although the inconsistency is also present in the documentation, it would be desirable if all iterative functions that uses a predicate block use the same behavior.

Proposed change:
Modify the filter function so that it also acts on truthy returns since this is also consistent with how if and unless works and also modify the documentation to state this.

This is a breaking change.



 Comments   
Comment by Thomas Hallgren [ 2018/01/05 ]

We might want to add a deprecation warning for filter blocks that returns values that are not false, undef, or true since the behavior of such blocks will change when this ticket is addressed. Perhaps we'd even want to put this under --strict control?

Comment by Henrik Lindberg [ 2018/01/08 ]

To do this, a "migration issue" is needed, and it needs to be supported in catalog preview (like we did for other similar runtime changes 3.x to 4.x).
It is not a deprecation per se - as there is only a change in semantics.

If we go ahead with this change we need:

  • a ticket for Puppet 6 (next major) for the breaking change
  • a ticket to update Catalog Preview and the "migration checker" (an option for migration checking 5-to-6 is needed etc)
  • add the "migration check" to puppet (the runtime check)

Not sure users appreciate this change as it may require changing their code, and requires all modules that they use to also potentially be changed.
Alternatives are to add a filter6 function, or to give filter an optional parameter to select "5" or "6" behavior - that way the new (desired) behaviour is opt-in.

Comment by Josh Cooper [ 2018/01/12 ]

FWIW we've started using the Puppet[:future_features] feature flag for breaking changes that are/will be landing in puppet 5.x.

Comment by Josh Cooper [ 2018/08/03 ]

Henrik Lindberg, Thomas Hallgren what is the status of this?

Comment by Henrik Lindberg [ 2018/08/15 ]

This is a very simple change to make for Puppet 6. I am concerned about this potentially breaking someone, but I am perhaps too cautious.

We could just change filter in Puppet 6, and document the change.

If we are concerned about backwards compatibility then there is a lot of things to do, and then I think the cost of fixing this inconsistency is perhaps too high. We have no 5.x release planned for adding a deprecation, and such deprecations would be both (somewhat) expensive and annoying. Since we missed the opportunity for 5.x and if we are concerned about migration this will have to wait until Puppet 7.

Comment by Henrik Lindberg [ 2018/08/15 ]

I added a PR with the simple change in the filter function.

Comment by Josh Cooper [ 2018/08/16 ]

Merged to master in https://github.com/puppetlabs/puppet/commit/454ca15f5e61af92c001c04d1b443f5b5918db28

Comment by Josh Cooper [ 2018/08/16 ]

Henrik Lindberg could you add release notes?

Comment by Scott Garman [ 2018/09/17 ]

Since we had a green run of CI puppet-agent 6.0.0 last night, I'm bulk-changing these issues from Ready for CI -> Resolved in preparation of the release.

Generated at Sat Aug 08 08:42:40 PDT 2020 using Jira 8.5.2#805002-sha1:a66f9354b9e12ac788984e5d84669c903a370049.