Uploaded image for project: 'Puppet'
  1. Puppet
  2. PUP-1467

DIE :undef, DIE!

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Normal
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: PUP 4.0.0
    • Component/s: Compiler
    • Labels:
    • Template:
    • Story Points:
      1

      Description

      I'm not sure if this should be flagged as a bug, a feature, or a refactor because this issue covers all three.

      Yesterday afternoon I discovered a template in a fairly popular module that did not work correctly on puppet 3.2.1 due to it's use of `:undef`. The fix was to replace `:undef` with `#nil?`.

      @@ -14,10 +14,10 @@ server {
      ssl_protocols <%= @ssl_protocols %>;
      ssl_ciphers <%= @ssl_ciphers %>;
      ssl_prefer_server_ciphers on;
      -<% if @auth_basic != :undef -%>
      +<% unless @auth_basic.nil? -%>
      auth_basic "<%= @auth_basic %>";
      <% end -%>
      -<% if @auth_basic_user_file != :undef -%>
      +<% unless @auth_basic_user_file.nil? -%>
      auth_basic_user_file <%= @auth_basic_user_file %>;
      <% end -%>

      It appears that in at least some cases (perhaps limited to erb?) real `nil`s are leaking out instead of `:undef`. I've unable to pass an `:undef` into a template at all in testing in either 3.2.1 or 3.2.3. I don't know if this has ever worked or is a regression.

          $ cat undef_test.pp
          notify{'undeclared': message => inline_template('<%= @foo == :undef %> : <%= @foo.nil? %>')}
       
          $empty = ''
          notify{'empty': message => inline_template('<%= @empty == :undef %> : <%= @empty.nil? %>')}
       
          $undef = undef
          notify{'undef': message => inline_template('<%= @undef == :undef %> : <%= @undef.nil? %>')}
       
          $false = false
          notify{'false': message => inline_template('<%= @false == :undef %> : <%= @false.nil? %>')}
       
          $ puppet --version
          3.2.3
          $ puppet apply undef_test.pp                          
          Notice: false : false
          Notice: /Stage[main]//Notify[false]/message: defined 'message' as 'false : false'
          Notice: false : false
          Notice: /Stage[main]//Notify[empty]/message: defined 'message' as 'false : false'
          Notice: false : true
          Notice: /Stage[main]//Notify[undeclared]/message: defined 'message' as 'false : true'
          Notice: false : true
          Notice: /Stage[main]//Notify[undef]/message: defined 'message' as 'false : true'
          Notice: Finished catalog run in 0.11 seconds
      

      Upon digging into the puppet source, it appears that usage and testing for `:undef` is inconsistent in the core.

      Consider `Puppet::Parser::Scope#true?` that tests for `:undef` to be used as a boolean value. https://github.com/puppetlabs/puppet/blob/3.2.3/lib/puppet/parser/scope.rb#L113-L122 . Elsewhere in the same class `:undef` is being checked for directly https://github.com/puppetlabs/puppet/blob/3.2.3/lib/puppet/parser/scope.rb#L392

      The case against `:undef`

      • It violates the princible of 'least surprise' in that it duplicates a built in language feature
      • In at least some circumstances `nil` leaks out in place of `:undef`
      • Test coverage of `:undef` is insufficent
      • Usage of `:undef` appears inconsistent in the core

      I'd agrue that in stead of trying to replicate/maintain externally visible inconsistent behavior that it would be better to just remove `:undef` completely and deal with the API change all at once, perhaps in a 3.3.0 release.

      UPDATE


      The resolution of this is that Puppet 4.0.0 and 3x with future parser will translate puppet undef to nil when it calls to ruby, and vice versa. (Within Puppet code, undef remains a special value of the type Undef, as described in the spec.) The exceptions are:

      • 3x functions (written using the 3x function API) - they will receive an empty string instead of nil for the top level arguments (not inside hashes and arrays etc, where nil is passed).
      • the catalog/ compiler (which is also 3x still) receives the ruby symbol :undef for parameters - it then inconsistently uses empty strings and/or :undef, nil values throughout its computations (these are never observable by a user), when the catalog is produced, it either just does not contain a parameter/value, or it is an empty string.

      These changes affects those that write/have 3x functions, or that do things in resource types that execute on the server side. It also affects those that observed :undef in ERB templates.

      Recommendation to users is to move their functions to the 4x function API, and use nil wherever they mean undefined.

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              nick.fagerlund Nicholas Fagerlund
              Reporter:
              redmine.exporter redmine.exporter
              QA Contact:
              Kurt Wall Kurt Wall
              Votes:
              1 Vote for this issue
              Watchers:
              5 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:

                  Zendesk Support