From 03c76deb60927375e9b35c74a903130930ffddf2 Mon Sep 17 00:00:00 2001 From: Daniel Pittman Date: Fri, 18 Jul 2008 15:21:57 +1000 Subject: Expose all puppet variables as instance member variables of the template wrapper. This helps resolve redmine #1427, by providing a safe mechanism to access variables. * Implement Puppet::Parser::Scope#to_hash, which returns a hash containing all the variable bindings in the current and, optionally, parent scope. * Use that to set instance member variables into Puppet::Parser::Templatewrapper * Report the time taken for variable binding at debug level, to help identify any performance regression that is encountered in the real world. * Rename the @scope and @file members of the template wrapper, to avoid clashing with a scope variable exposed within puppet. Signed-off-by: Daniel Pittman (cherry picked from commit ba220b41e4f509f2632e2664d332e49b20a70ea7) --- lib/puppet/parser/templatewrapper.rb | 53 +++++++++++++++++++++++++++--------- 1 file changed, 40 insertions(+), 13 deletions(-) (limited to 'lib/puppet/parser/templatewrapper.rb') diff --git a/lib/puppet/parser/templatewrapper.rb b/lib/puppet/parser/templatewrapper.rb index 4790cea30..298428f63 100644 --- a/lib/puppet/parser/templatewrapper.rb +++ b/lib/puppet/parser/templatewrapper.rb @@ -5,36 +5,63 @@ class Puppet::Parser::TemplateWrapper include Puppet::Util Puppet::Util.logmethods(self) - def initialize(scope, file) - @scope = scope - @file = Puppet::Module::find_template(file, @scope.compiler.environment) + def initialize(scope, filename) + @__scope__ = scope + @__file__ = Puppet::Module::find_template(filename, scope.compiler.environment) - unless FileTest.exists?(@file) + unless FileTest.exists?(file) raise Puppet::ParseError, "Could not find template %s" % file end # We'll only ever not have a parser in testing, but, eh. - if @scope.parser - @scope.parser.watch_file(@file) + if scope.parser + scope.parser.watch_file(file) end + + # Expose all the variables in our scope as instance variables of the + # current object, making it possible to access them without conflict + # to the regular methods. + benchmark(:debug, "Bound template variables for #{file}") do + scope.to_hash.each { |name, value| + instance_variable_set("@#{name}", value) + } + end + end + + def scope + @__scope__ + end + + def file + @__file__ end # Should return true if a variable is defined, false if it is not def has_variable?(name) - if @scope.lookupvar(name.to_s, false) != :undefined + if scope.lookupvar(name.to_s, false) != :undefined true else false end end - # Ruby treats variables like methods, so we can cheat here and - # trap missing vars like they were missing methods. + # Ruby treats variables like methods, so we used to expose variables + # within scope to the ERB code via method_missing. As per RedMine #1427, + # though, this means that conflicts between methods in our inheritance + # tree (Kernel#fork) and variable names (fork => "yes/no") could arise. + # + # Worse, /new/ conflicts could pop up when a new kernel or object method + # was added to Ruby, causing templates to suddenly fail mysteriously when + # Ruby was upgraded. + # + # To ensure that legacy templates using unqualified names work we retain + # the missing_method definition here until we declare the syntax finally + # dead. 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) + value = scope.lookupvar(name.to_s, false) if value != :undefined return value else @@ -47,8 +74,8 @@ class Puppet::Parser::TemplateWrapper def result result = nil - benchmark(:debug, "Interpolated template #{@file}") do - template = ERB.new(File.read(@file), 0, "-") + benchmark(:debug, "Interpolated template #{file}") do + template = ERB.new(File.read(file), 0, "-") result = template.result(binding) end @@ -56,7 +83,7 @@ class Puppet::Parser::TemplateWrapper end def to_s - "template[%s]" % @file + "template[%s]" % file end end -- cgit From 8a0cb16abd4c6a9cbf27d88593aa2b42a7375b94 Mon Sep 17 00:00:00 2001 From: Luke Kanies Date: Thu, 7 Aug 2008 18:53:02 -0700 Subject: Added tests for TemplateWrapper's use of Scope#to_hash. We should deprecate the method_missing stuff in 0.25. Signed-off-by: Luke Kanies --- lib/puppet/parser/templatewrapper.rb | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) (limited to 'lib/puppet/parser/templatewrapper.rb') diff --git a/lib/puppet/parser/templatewrapper.rb b/lib/puppet/parser/templatewrapper.rb index 298428f63..3b74e62d4 100644 --- a/lib/puppet/parser/templatewrapper.rb +++ b/lib/puppet/parser/templatewrapper.rb @@ -18,15 +18,6 @@ class Puppet::Parser::TemplateWrapper if scope.parser scope.parser.watch_file(file) end - - # Expose all the variables in our scope as instance variables of the - # current object, making it possible to access them without conflict - # to the regular methods. - benchmark(:debug, "Bound template variables for #{file}") do - scope.to_hash.each { |name, value| - instance_variable_set("@#{name}", value) - } - end end def scope @@ -67,12 +58,20 @@ class Puppet::Parser::TemplateWrapper 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 + raise Puppet::ParseError, "Could not find value for '%s'" % name end end def result + # Expose all the variables in our scope as instance variables of the + # current object, making it possible to access them without conflict + # to the regular methods. + benchmark(:debug, "Bound template variables for #{file}") do + scope.to_hash.each { |name, value| + instance_variable_set("@#{name}", value) + } + end + result = nil benchmark(:debug, "Interpolated template #{file}") do template = ERB.new(File.read(file), 0, "-") -- cgit