[PUP-4520] Future parser is not correctly handling the default case of a case statement Created: 2015/05/04  Updated: 2015/05/20  Resolved: 2015/05/20

Status: Closed
Project: Puppet
Component/s: Language
Affects Version/s: PUP 3.7.5
Fix Version/s: PUP 3.8.1, PUP 4.1.0

Type: Bug Priority: Major
Reporter: Dave Roberts Assignee: Unassigned
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Blocks
is blocked by PUP-4541 Create acceptance test for handling o... Closed
Template:
Epic Link: 4.x Language
Story Points: 1
Sprint: Language 2015-05-13
Release Notes: Bug Fix
QA Contact: Kurt Wall

 Description   

According to https://docs.puppetlabs.com/puppet/latest/reference/future_lang_conditional.html#behavior-2 the future parser should always evaluate the default case last; however, that does not seem to be happening. The following code, executed on a VM always returns the default case.

[root@master ~]# facter is_virtual
true
[root@master ~]# cat foo.pp
case $is_virtual {
  default: { notify { "default case": } }
  true: { notify { "nondefault case": } }
}
[root@master ~]# puppet apply foo.pp
Notice: Compiled catalog for master.inf.puppetlabs.demo in environment production in 0.51 seconds
Notice: default case
Notice: /Stage[main]/Main/Notify[default case]/message: defined 'message' as 'default case'
Notice: Finished catalog run in 0.28 seconds



 Comments   
Comment by Eric Sorenson [ 2015/05/04 ]

I think this is actually due to your stringify_facts setting on the agent/apply side. I get the correct answer with --no-stringify_facts and the default case without, regardless of the order of the statements.

[vagrant@glitcher ~]$ puppet apply --parser future ./enterprise-665.pp
Notice: Compiled catalog for glitcher.local in environment production in 0.42 seconds
Notice: default case
Notice: /Stage[main]/Main/Notify[default case]/message: defined 'message' as 'default case'
Notice: Finished catalog run in 0.01 seconds
[vagrant@glitcher ~]$ puppet apply --parser future --no-stringify_facts ./enterprise-665.pp
Notice: Compiled catalog for glitcher.local in environment production in 0.43 seconds
Notice: nondefault case
Notice: /Stage[main]/Main/Notify[nondefault case]/message: defined 'message' as 'nondefault case'
Notice: Finished catalog run in 0.01 seconds

(The stringified "true" from is_virtual is not the same as the unquoted boolean true in your conditional)

Comment by Dave Roberts [ 2015/05/04 ]

[root@centos6a ~]# cat /tmp/foo.pp
case $is_virtual {
  default: { notify { "default case": } }
  true: { notify { "nondefault case": } }
}
[root@centos6a ~]# puppet config print stringify_facts
false
[root@centos6a ~]# puppet apply --parser future /tmp/foo.pp
Notice: Compiled catalog for centos6a.pdx.puppetlabs.demo in environment production in 0.38 seconds
Notice: default case
Notice: /Stage[main]/Main/Notify[default case]/message: defined 'message' as 'default case'
Notice: Finished catalog run in 0.05 seconds
[root@centos6a ~]# puppet apply --parser future --no-stringify_facts /tmp/foo.pp
Notice: Compiled catalog for centos6a.pdx.puppetlabs.demo in environment production in 0.33 seconds
Notice: default case
Notice: /Stage[main]/Main/Notify[default case]/message: defined 'message' as 'default case'
Notice: Finished catalog run in 0.06 seconds

Comment by Dave Roberts [ 2015/05/04 ]

Incidentally, this always returns the default case as well

[root@centos6a ~]# cat foo.pp
case $is_virtual {
  default: { notify { "default case": } }
  true, "true": { notify { "nondefault case": } }
}
[root@centos6a ~]# puppet apply --parser future foo.pp
Notice: Compiled catalog for centos6a.pdx.puppetlabs.demo in environment production in 0.34 seconds
Notice: default case
Notice: /Stage[main]/Main/Notify[default case]/message: defined 'message' as 'default case'
Notice: Finished catalog run in 0.06 seconds

Comment by Eric Sorenson [ 2015/05/04 ]

OK I was able to reproduce but it's weirdly something that seems to affect the PE 3.8.0 version and not OSS Puppet 3.7.5 (or, incidentally, Puppet 4)

PE:

[vagrant@deglitch ~]$ puppet apply --parser future /Sandbox/pup-4520.pp
Notice: Compiled catalog for deglitch.local in environment production in 0.36 seconds
Notice: default case
Notice: /Stage[main]/Main/Notify[default case]/message: defined 'message' as 'default case'
Notice: Finished catalog run in 0.02 seconds
[vagrant@deglitch ~]$ puppet --version
3.7.5 (Puppet Enterprise 3.8.0-rc0-532-ga37f033)

And on FOSS:

[vagrant@glitcher ~]$ puppet apply --parser future /Sandbox/pup-4520.pp
Notice: Compiled catalog for glitcher.local in environment production in 0.43 seconds
Notice: nondefault case
Notice: /Stage[main]/Main/Notify[nondefault case]/message: defined 'message' as 'nondefault case'
Notice: Finished catalog run in 0.01 seconds
[vagrant@glitcher ~]$ puppet --version
3.7.5

And the code:

case $is_virtual {
  default: { notify { "default case": } }
  true, "true": { notify { "nondefault case": } }
}

Comment by Henrik Lindberg [ 2015/05/04 ]

This is a regression in future parser between 3.7.5 and 3.8.0 that was introduced when the deep-matching feature was added.

Comment by Henrik Lindberg [ 2015/05/04 ]

Also check that selector is not broken the same way, and check that language specification is correct.

Comment by Henrik Lindberg [ 2015/05/04 ]

Selector works as it should.

Comment by Henrik Lindberg [ 2015/05/04 ]

Specification reviewed - there were several unclear statements made about how a literal default is processed. The specification is now updated to clearly state that a default option is skipped and only makes the proposition be selected if no other option matched.

Comment by Thomas Hallgren [ 2015/05/07 ]

Merged to 3.x at d2c2441

Comment by Thomas Hallgren [ 2015/05/07 ]

Merged to stable at 14c39ed and onwards to master at 74ffa1a

Comment by Henrik Lindberg [ 2015/05/07 ]

Kurt Wall regarding QA status - given that this has been broken for some time and not reported until now is an indication of people typically placing their default case last (probably because there historically was the very same problem in puppet).

Comment by Kurt Wall [ 2015/05/07 ]

Henrik Lindberg Okay. I've dropped the risk level because it's been in the wild for some time. Thanks for the information!

Comment by Kurt Wall [ 2015/05/11 ]

Validated in master at SHA=e6ba084. The default case is now handled properly if it is not the last option:

# foo.pp
case $is_virtual {
  default: { notice('default case') }
  true: { notice('nondefault case') }
}
 
# facter is_virtual
true
 
# puppet apply foo.pp
Notice: Scope(Class[main]): nondefault case
Notice: Compiled catalog for c4ov0fesj1vk8qz.delivery.puppetlabs.net in environment production in 0.35 seconds
Notice: Applied catalog in 0.16 seconds

Generated at Wed Aug 21 00:08:36 PDT 2019 using JIRA 7.7.1#77002-sha1:e75ca93d5574d9409c0630b81c894d9065296414.