From 9f6dec267467334dcc5e1157b9dce57a325cbb73 Mon Sep 17 00:00:00 2001 From: Nick Lewis Date: Thu, 26 May 2011 13:32:11 -0700 Subject: (#7681) Allow array variables as resource references The commit df088c9ba16dce50c17a79920c1ac186db67b9e9 introduced a regression where $files = ["/tmp/one", "/tmp/two"] file { "/tmp/one": content => "one", } file { "/tmp/two": content => "two", } file { "/tmp/three": content => "three", require => File[$files], } no longer worked. File[$files] was concatenating the elements of $files to create a single title, instead of expanding to multiple File dependencies. Since resource reference titles are implicitly wrapped in an array, if one of the elements of that array is a variable containing an array, the list of titles is a nested array. Prior to the change causing the regression, we would flatten arrays when evaluating them, under certain circumstances. We no longer ever flatten AST arrays when evaluating them, so anywhere that we really do need a flattened array, we have to manually flatten it. ResourceReference expects its list of titles to be a single, flat list of titles, so we have to make it so. Paired-with: Jacob Helwig --- lib/puppet/parser/ast/resource_reference.rb | 2 +- spec/unit/parser/ast/resource_reference_spec.rb | 24 +++++++++++++++++++----- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/lib/puppet/parser/ast/resource_reference.rb b/lib/puppet/parser/ast/resource_reference.rb index 0f8e655bf..256a99d75 100644 --- a/lib/puppet/parser/ast/resource_reference.rb +++ b/lib/puppet/parser/ast/resource_reference.rb @@ -7,7 +7,7 @@ class Puppet::Parser::AST::ResourceReference < Puppet::Parser::AST::Branch # Evaluate our object, but just return a simple array of the type # and name. def evaluate(scope) - titles = Array(title.safeevaluate(scope)) + titles = Array(title.safeevaluate(scope)).flatten a_type, titles = scope.resolve_type_and_titles(type, titles) diff --git a/spec/unit/parser/ast/resource_reference_spec.rb b/spec/unit/parser/ast/resource_reference_spec.rb index 627754dd1..4d1c191cf 100755 --- a/spec/unit/parser/ast/resource_reference_spec.rb +++ b/spec/unit/parser/ast/resource_reference_spec.rb @@ -9,21 +9,25 @@ describe Puppet::Parser::AST::ResourceReference do @scope = Puppet::Parser::Scope.new end + def ast_name(value) + Puppet::Parser::AST::Name.new(:value => value) + end + def newref(type, title) - title = stub 'title', :safeevaluate => title - ref = Puppet::Parser::AST::ResourceReference.new(:type => type, :title => title) + title_array = Puppet::Parser::AST::ASTArray.new(:children => [title]) + ref = Puppet::Parser::AST::ResourceReference.new(:type => type, :title => title_array) end it "should correctly produce reference strings" do - newref("File", "/tmp/yay").evaluate(@scope).to_s.should == "File[/tmp/yay]" + newref("File", ast_name("/tmp/yay")).evaluate(@scope).to_s.should == "File[/tmp/yay]" end it "should produce a single resource when the title evaluates to a string" do - newref("File", "/tmp/yay").evaluate(@scope).should == Puppet::Resource.new("file", "/tmp/yay") + newref("File", ast_name("/tmp/yay")).evaluate(@scope).should == Puppet::Resource.new("file", "/tmp/yay") end it "should return an array of resources if given an array of titles" do - titles = mock 'titles', :safeevaluate => ["title1","title2"] + titles = Puppet::Parser::AST::ASTArray.new(:children => [ast_name("title1"), ast_name("title2")]) ref = ast::ResourceReference.new( :title => titles, :type => "File" ) ref.evaluate(@scope).should == [ Puppet::Resource.new("file", "title1"), @@ -31,6 +35,16 @@ describe Puppet::Parser::AST::ResourceReference do ] end + it "should return an array of resources if given a variable containing an array of titles" do + @scope.setvar("my_files", ["foo", "bar"]) + titles = Puppet::Parser::AST::Variable.new(:value => "my_files") + ref = newref('File', titles) + ref.evaluate(@scope).should == [ + Puppet::Resource.new("file", "foo"), + Puppet::Resource.new("file", "bar") + ] + end + it "should return a correct representation when converting to string" do type = stub 'type', :is_a? => true, :to_s => "file" title = stub 'title', :is_a? => true, :to_s => "[/tmp/a, /tmp/b]" -- cgit From 163ff6b39120815d52811b20ef5f7f46f2fd950f Mon Sep 17 00:00:00 2001 From: Nick Lewis Date: Thu, 26 May 2011 13:44:23 -0700 Subject: (#7681) Add an acceptance test for resource refs with array variables Paired-with: Jacob Helwig Reviewed-by: Dominic Maraglia --- .../language/resource_refs_with_nested_arrays.rb | 27 ++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 acceptance/tests/language/resource_refs_with_nested_arrays.rb diff --git a/acceptance/tests/language/resource_refs_with_nested_arrays.rb b/acceptance/tests/language/resource_refs_with_nested_arrays.rb new file mode 100644 index 000000000..7bc797885 --- /dev/null +++ b/acceptance/tests/language/resource_refs_with_nested_arrays.rb @@ -0,0 +1,27 @@ +test_name "#7681: Allow using array variables in resource references" + +test_manifest = < "echo the first command", + path => "/usr/bin:/bin", + logoutput => true, +} +exec { "second": + command => "echo the second command", + path => "/usr/bin:/bin", + logoutput => true, +} +exec { "third": + command => "echo the final command", + path => "/usr/bin:/bin", + logoutput => true, + require => Exec[$exec_names], +} +MANIFEST + +results = apply_manifest_on agents, test_manifest + +results.each do |result| + assert_match(/Exec\[third\].*the final command/, "#{result.stdout}") +end -- cgit