[PUP-6274] include undef doesn't fail Created: 2016/05/06  Updated: 2019/04/04  Resolved: 2018/02/01

Status: Closed
Project: Puppet
Component/s: Compiler, Functions, Language
Affects Version/s: PUP 4.4.2
Fix Version/s: PUP 5.4.0

Type: Bug Priority: Minor
Reporter: Daniel Parks Assignee: Unassigned
Resolution: Fixed Votes: 0
Labels: low-hanging-fruit
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Template: PUP Bug Template
Acceptance Criteria:

That include undef raises an error

Epic Link: 5.y TechDebt
Sub-team: Language
Team: Platform Core
Sprint: Platform Core KANBAN
Release Notes: Bug Fix
Release Notes Summary: Attempt to use an empty string or undef as the name of a class when calling {{include}} will now raise an error instead of appearing to be silently ignored.


❯ puppet apply -e "include undef"
Notice: Compiled catalog for zhora.local in environment production in 0.04 seconds
Notice: Applied catalog in 0.03 seconds

Expected results: compilation failure

Similarly, include '' doesn't fail, though include nosuchclass does.

Example scenario: include $::some::class::tyypo (in my case, I accidentally grabbed the dollar sign while copying and pasting)

Comment by Henrik Lindberg [ 2016/05/06 ]

This is a real WAT caused by a series of tech debt amazeballs design decisions:

  • The include function is a 3.x function
  • The 3.x function API requires that undef as a puppet language value given directly to the function should be passed as an empty string
    (rather than the other two encodings in use for 3.x (Ruby symbol :undef or (what 4.x uses) Ruby nil).
  • The name of the resource of type 'class' that is know as 'main' is internally named - (drumroll) - you guessed it, it's '' (empty string) !!!!
  • There is always a class named '' (a.k.a 'main')
  • No wonder include undef works since class 'main' is always included.

We have not dared cleaning this up yet (which would entail rewriting the include function as a 4.x function and not accepting an include of undef, and that the class '' (while internally it may be called that, should really be referred to via 'main'). This is tricky though since it sort of also represents "top scope".

A simpler change would be to simply error (in the 3.x implementation) on include of empty string - since it is nonsensical to include 'main' (it is always there).

Comment by Henrik Lindberg [ 2016/05/06 ]

I lowered the priority since it is actually doing what it is asked to do "include top scope" and it is more surprising than harmful.

With proper typing and code/data hygiene (--strict_variables) one should never end up in a situation where an include undef is evaluated.

If someone wants to spend time adding an error message / test to the include function for undef check, a PR would be welcome. Otherwise I think we are better off spending time cleaning up the parts of the compiler / 3.x that we left behind.

Comment by Henrik Lindberg [ 2017/05/15 ]

The include function should be moved to 4.x function API for other reasons - and this can then be done as well.

Generated at Mon Dec 09 12:53:22 PST 2019 using JIRA 7.7.1#77002-sha1:e75ca93d5574d9409c0630b81c894d9065296414.