[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, upstream, updated. 2.6.1rc1-141-gcdb2b90

markus (none) markus at AVA-351181.
Mon Aug 16 12:48:52 UTC 2010


The following commit has been merged in the upstream branch:
commit be2b1f360fc15596098280265e6aa76e8043eb92
Author: Matt Robinson <matt at puppetlabs.com>
Date:   Mon Jul 26 19:34:27 2010 -0700

    [#4370] Fixes extlookup precedence getting overwritten between runs
    
    We found the gsub! in extlookup was actually modifying the value for
    extlookup_precedence, so the next node to call it just got the
    interpolated value from the first run.
    
    We did two things in the code to prevent this:
    1. We returned a dup of the ast string object so that modifying it
    wouldn’t change puppet’s state. We didn’t do this for all possible
    return values because we depend on using the original ast array object
    to do array concatenation
    2. We fixed extlookup to not do a destructive gsub
    
    Reviewed by: Jesse Wolfe

diff --git a/lib/puppet/parser/ast/function.rb b/lib/puppet/parser/ast/function.rb
index 602016c..74023f6 100644
--- a/lib/puppet/parser/ast/function.rb
+++ b/lib/puppet/parser/ast/function.rb
@@ -11,7 +11,6 @@ class Puppet::Parser::AST
     @settor = true
 
     def evaluate(scope)
-
       # Make sure it's a defined function
       raise Puppet::ParseError, "Unknown function #{@name}" unless Puppet::Parser::Functions.function(@name)
 
diff --git a/lib/puppet/parser/ast/leaf.rb b/lib/puppet/parser/ast/leaf.rb
index 49cde63..090d75c 100644
--- a/lib/puppet/parser/ast/leaf.rb
+++ b/lib/puppet/parser/ast/leaf.rb
@@ -42,7 +42,7 @@ class Puppet::Parser::AST
   # The base string class.
   class String < AST::Leaf
     def evaluate(scope)
-      @value
+      @value.dup
     end
 
     def to_s
diff --git a/lib/puppet/parser/functions/extlookup.rb b/lib/puppet/parser/functions/extlookup.rb
index ee230e7..177d9ae 100644
--- a/lib/puppet/parser/functions/extlookup.rb
+++ b/lib/puppet/parser/functions/extlookup.rb
@@ -99,19 +99,15 @@ module Puppet::Parser::Functions
     # this will result in /path/to/extdata/hosts/your.box.com.csv
     # being searched.
     #
-    # we parse the precedence here because the best place to specify
-    # it would be in site.pp but site.pp is only evaluated at startup
-    # so $fqdn etc would have no meaning there, this way it gets evaluated
-    # each run and has access to the right variables for that run
-    lookupvar('extlookup_precedence').each do |prec|
-      while prec =~ /%\{(.+?)\}/
-        prec.gsub!(/%\{#{$1}\}/, lookupvar($1))
+    # This is for back compatibility to interpolate variables with %.
+    # % interpolation is a workaround for a problem that has been fixed: Puppet variable
+    # interpolation at top scope used to only happen on each run
+    extlookup_precedence = lookupvar('extlookup_precedence').collect do |var|
+      var.gsub(/%\{(.+?)\}/) do |capture|
+        lookupvar($1)
       end
-
-      extlookup_precedence << prec
     end
 
-
     datafiles = Array.new
 
     # if we got a custom data file, put it first in the array of search files
diff --git a/spec/unit/parser/ast/leaf_spec.rb b/spec/unit/parser/ast/leaf_spec.rb
index 5bdca67..eb71f0d 100755
--- a/spec/unit/parser/ast/leaf_spec.rb
+++ b/spec/unit/parser/ast/leaf_spec.rb
@@ -47,6 +47,10 @@ describe Puppet::Parser::AST::String do
       value = stub 'value', :is_a? => true, :to_s => "ab"
       Puppet::Parser::AST::String.new( :value => value ).to_s.should == "\"ab\""
     end
+    it "should return a dup of its value" do
+      value = ""
+      Puppet::Parser::AST::String.new( :value => value ).evaluate(stub 'scope').should_not be_equal(value)
+    end
   end
 end
 
diff --git a/spec/unit/parser/functions/extlookup_spec.rb b/spec/unit/parser/functions/extlookup_spec.rb
index bf28803..a3dcaa7 100755
--- a/spec/unit/parser/functions/extlookup_spec.rb
+++ b/spec/unit/parser/functions/extlookup_spec.rb
@@ -7,8 +7,9 @@ describe "the extlookup function" do
 
   before :each do
     @scope = Puppet::Parser::Scope.new
-    
+
     @scope.stubs(:environment).returns(Puppet::Node::Environment.new('production'))
+    Puppet::Parser::Functions.function("extlookup")
   end
 
   it "should exist" do
@@ -62,9 +63,8 @@ describe "the extlookup function" do
   end
 
   describe "should look in $extlookup_datadir for data files listed by $extlookup_precedence" do
-    before do 
+    before do
       @scope.stubs(:lookupvar).with('extlookup_datadir').returns("/tmp")
-      @scope.stubs(:lookupvar).with('extlookup_precedence').returns(["one","two"])
       File.open("/tmp/one.csv","w"){|one| one.puts "key,value1" }
       File.open("/tmp/two.csv","w") do |two|
         two.puts "key,value2"
@@ -73,13 +73,23 @@ describe "the extlookup function" do
     end
 
     it "when the key is in the first file" do
+      @scope.stubs(:lookupvar).with('extlookup_precedence').returns(["one","two"])
       result = @scope.function_extlookup([ "key" ])
       result.should == "value1"
     end
 
     it "when the key is in the second file" do
+      @scope.stubs(:lookupvar).with('extlookup_precedence').returns(["one","two"])
       result = @scope.function_extlookup([ "key2" ])
       result.should == "value_two"
     end
+
+    it "should not modify extlookup_precedence data" do
+      variable = '%{fqdn}'
+      @scope.stubs(:lookupvar).with('extlookup_precedence').returns([variable,"one"])
+      @scope.stubs(:lookupvar).with('fqdn').returns('myfqdn')
+      result = @scope.function_extlookup([ "key" ])
+      variable.should == '%{fqdn}'
+    end
   end
 end

-- 
Puppet packaging for Debian



More information about the Pkg-puppet-devel mailing list