[#PUP-5350] Puppet filter function does not behave consistently across all supported argument types.

[PUP-5350] Puppet filter function does not behave consistently across all supported argument types. Created: 2015/10/09 Updated: 2015/10/29 Resolved: 2015/10/21
Status:	Closed
Project:	Puppet
Component/s:	None
Affects Version/s:	PUP 4.2.2
Fix Version/s:	PUP 3.8.4, PUP 4.2.3

Type:

Bug

Priority:

Normal

Reporter:

Peter Huene

Assignee:

Unassigned

Resolution:

Fixed

Votes:

Labels:

None

Remaining Estimate:

Not Specified

Time Spent:

Not Specified

Original Estimate:

Not Specified

Template:

Story Points:

Sprint:

Language 2015-10-14, Language 2015-10-28

Release Notes:

Bug Fix

Release Notes Summary:

The filter() function did not behave according to specification when filtering a hash as it did not enforce that only boolean true as a return from the code block would include the element in the result. Instead, any "truthy" value was accepted. Now, only boolean true will include an element in the result.

Description

The overload of the filter function that takes a Hash behaves differently from the other overloads that take an "enumerable" type.

For example:

notice({a => 1, b => 2, c => 3}.filter |$k, $v| { $v })

Will output

{a => 1, b=> 2, c => 3}

However:

notice([1, 2, 3].filter |$v| { $v })

Will output:

[]

The first example returns an Integer and results in not filtering the element. The second example also returns an Integer, but this results in filtering the element.

One of these behaviors is incorrect and should be made consistent.

Cause of bug:

In the filter function's implementation, the Hash overload uses hash#select and returns the result of yielding to the lambda, which Ruby treats as true if the value is truthy. The enumerable overload instead has a direct comparison of the lambda's return value to true.

Comments

Comment by Henrik Lindberg [ 2015/10/10 ]

A fix for this should be backported to 3.x as well.

Comment by Thomas Hallgren [ 2015/10/14 ]

The documented behavior is to return an object "with the entries for which the block evaluates to true" which describes the behavior for the second example. I will assume that the fix for this should change the Hash behavior. I.e. the first example should return an empty hash.

Comment by Henrik Lindberg [ 2015/10/14 ]

ok, then lets use boolean true as oppose to thruthy.

Comment by Henrik Lindberg [ 2015/10/14 ]

merged to 3.x here: 85fd467

Comment by Sean Griffin [ 2015/10/20 ]

1. Fixed versions (noted above) include 4.3.0, but this fix doesn't appear to be merged to master.
2. This function doesn't seem to work at all in 3.x, current version or versions before the fix date.

[root@k9xyz4acfmlsqq7 tmp]# puppet --version

3.8.3

[root@k9xyz4acfmlsqq7 tmp]# cat test.pp

notice([1,2,3].filter |$x| {$x})

[root@k9xyz4acfmlsqq7 tmp]# puppet apply test.pp

Error: Could not parse for environment production: Syntax error at '.'; expected ')' at /tmp/test.pp:3 on node k9xyz4acfmlsqq7.delivery.puppetlabs.net

Error: Could not parse for environment production: Syntax error at '.'; expected ')' at /tmp/test.pp:3 on node k9xyz4acfmlsqq7.delivery.puppetlabs.net

[root@k9xyz4acfmlsqq7 tmp]#

The hash version of the call fails in a similar way.
Thomas Hallgren, could you please check this out? Thanks.

Comment by Henrik Lindberg [ 2015/10/20 ]

You need to use --parser future on 3.x to use iterative functions, lambdas etc.

Comment by Henrik Lindberg [ 2015/10/20 ]

afaict, it is merged to 3.x, stable, and master. This shows all branches where a commit is included (-r searches your remote branches; need to do pull/fethch first naturally).

git branch -r --contains 85fd467

Comment by Sean Griffin [ 2015/10/21 ]

Using --parser=future for the 3.8.x tests. Thanks Henrik.

The arrays and the hashes behave in a consistent way now, the block must evaluate to TRUE and not just to a truthy value.

Is this what the customer will expect from reading the documentation of the filter function? (https://docs.puppetlabs.com/references/latest/function.html#filter) Should that document be worded more strongly to prevent misunderstanding? cc Jorie Tappa

Verified on redhat-7-x86_64:
3.8.3, before fix:

[root@swtuf7f3znrh2qz puppet]# puppet --version

3.8.3

[root@swtuf7f3znrh2qz puppet]# puppet apply -e 'notice({"a" => 44, "b" => true}.filter |$k, $v| {$v})' --parser=future

Notice: Scope(Class[main]): {a => 44, b => true}

Notice: Compiled catalog for swtuf7f3znrh2qz.delivery.puppetlabs.net in environment production in 0.45 seconds

Notice: Finished catalog run in 0.06 seconds

[root@swtuf7f3znrh2qz puppet]#

3.8.4, after fix:

[root@p72k0s0obo5sgss puppet]# puppet --version

3.8.4

[root@p72k0s0obo5sgss puppet]# puppet apply -e 'notice({"a" => 44, "b" => true}.filter |$k, $v| {$v})' --parser=future

Notice: Scope(Class[main]): {b => true}

Notice: Compiled catalog for p72k0s0obo5sgss.delivery.puppetlabs.net in environment production in 0.47 seconds

Notice: Finished catalog run in 0.01 seconds

[root@p72k0s0obo5sgss puppet]#

Also verified in the master branch:

[root@i7ykvf2snzu7y5g ~]# puppet --version

4.2.3

[root@i7ykvf2snzu7y5g ~]# puppet apply -e 'notice({"a" => 44, "b" => true}.filter |$k, $v| {$v})'

Notice: Scope(Class[main]): {b => true}

Notice: Compiled catalog for i7ykvf2snzu7y5g.delivery.puppetlabs.net in environment production in 0.37 seconds

Notice: Applied catalog in 0.02 seconds

[root@i7ykvf2snzu7y5g ~]#

Generated at Sat Jul 11 02:00:45 PDT 2020 using Jira 8.5.2#805002-sha1:a66f9354b9e12ac788984e5d84669c903a370049.

[PUP-5350] Puppet filter function does not behave consistently across all supported argument types. Created: 2015/10/09 Updated: 2015/10/29 Resolved: 2015/10/21