Feature #1177

You should be able to test within a template for whether a variable or fact is defined

Added by micah - over 2 years ago. Updated over 2 years ago.

Status:Closed Start:
Priority:Normal Due date:
Assignee:Luke Kanies % Done:

0%
Category:language
Target version:0.24.5
Affected version:0.25.4 Branch:
Keywords:
Votes: 0

Description

If you create a fact that does not have a value for some class of hosts, then there is no variable available to the template at all (rather than the variable existing and returning ‘nil’), so if you test for that fact in a template and a node that doesn’t have a value for that fact runs, an exception is raised (because method_missing cannot find a variable).

The only way to test for an empty/nil fact value is by using @scope.lookupvar(“variable”), false) which is a bit of black magic and that is not a good way to go about because that relies on the implementation details of the TemplateWrapper class.

The exception being raised is probably there to catch common cases, like people screwing up a fact value in a template.

Holoway looked at things on IRC and he thinks that the Puppet::Parser::TemplateWrapper class is doing the right thing, it just needs the equivilant of either has_fact? or exists? As a possible patch, he suggested the following:

diff --git a/lib/puppet/parser/templatewrapper.rb b/lib/puppet/parser/templatewrapper.rb
index 13823d4..47857c4 100644
--- a/lib/puppet/parser/templatewrapper.rb
+++ b/lib/puppet/parser/templatewrapper.rb
@@ -19,6 +19,14 @@ class Puppet::Parser::TemplateWrapper
             @scope.parser.watch_file(@file)
         end
     end
+
+    def has_fact?(fact)
+      if @scope.lookupvar(name.to_s, false) == :undefined
+        false
+      else
+        true
+      end
+    end

     # Ruby treats variables like methods, so we can cheat here and
     # trap missing vars like they were missing methods.

0001-Adding-has_variable-support-fixing-ticket-1177.patch (3.9 KB) Redmine Admin, 05/13/2008 06:44 am

History

#1
Updated by AJ Christensen over 2 years ago

I feel this is requiring of Luke’s input; that is, should the fact evaluation be done in Puppet (with a case/if) or should the templatewrapper support it with this or a similar patch.

The supplied code needs a test written, also.

#2
Updated by Luke Kanies over 2 years ago

Well, the first point that must be made is that the compiler doesn’t distinguish between facts and other variables. It would be much more correct to call the method ‘has_variable?’, or even ‘variable_defined?’

Second, I guess I’m fine with the general idea, but you can’t actually set variables to ‘nil’ in Puppet, so it’s just as effective to say ‘variable.nil?’, which makes me think this is a touch overkill.

Is there something about ‘variable.nil?’ that doesn’t work for you?

#3
Updated by Redmine Admin over 2 years ago

Makes sense that it should be has_variable?.

I don’t think it is as effective to say variable.nil?. The issue here is you are using method_missing in Puppet::Parser::TemplateWrapper:

    # Ruby treats variables like methods, so we can cheat here and
    # trap missing vars like they were missing methods.
    def method_missing(name, *args)
        # We have to tell lookupvar to return :undefined to us when
        # appropriate; otherwise it converts to "".
        value = @scope.lookupvar(name.to_s, false)
        if value != :undefined
            return value
        else
            # Just throw an error immediately, instead of searching for
            # other missingmethod things or whatever.
            raise Puppet::ParseError,
                "Could not find value for '%s'" % name
        end
    end

So anything that calls a variable that isn’t returned from lookupvar is going to generate an exception. Which is the right behavior, since you want the template to explode if you fat-finger a variable name.

I haven’t tested it, but I’m pretty sure variable.nil? is going to explode.

Regarding tests, I don’t see any tests for TemplateWrapper at all — am I missing something?

#4
Updated by Luke Kanies over 2 years ago

As we discussed on IRC, TemplateWrapper is currently considered a hidden class, so it’s not directly tested; all of the related tests are in the functions tests.

#5
Updated by Luke Kanies over 2 years ago

It’d be great to get this into 0.24.5, since it’s been coming up a lot more.

#6
Updated by Redmine Admin over 2 years ago

The attached patch fixes this bug, and adds full test coverage to Puppet::Parser::TemplateWrapper.

#7
Updated by Redmine Admin over 2 years ago

At fujin’s request, you would use this in a template with:

<% if has_variable?(“something”) –%> .. do something if true <% end –%>

#8
Updated by Luke Kanies over 2 years ago

  • Status changed from 1 to Closed
  • 7 set to fixed

Merged.

Also available in: Atom PDF