The introduction of composite namevars caused the resource title used in
resource aliases to be set as an array, even when the resource only had one
namevar. This would fail to conflict with non-alias entries in the resource
table, which used a string for the title, even though the single element array
contained the same string.
Now, we flatten the key used in the resource table, so that single element
arrays are represented as strings, and will properly conflict with resource
titles.
Paired-With: Jacob Helwig <ja...@puppetlabs.com>
Signed-off-by: Nick Lewis <n...@puppetlabs.com>
---
lib/puppet/resource/catalog.rb | 9 +++--
spec/unit/resource/catalog_spec.rb | 61 +++++++++++++++++++++++++++++++----
2 files changed, 60 insertions(+), 10 deletions(-)
diff --git a/lib/puppet/resource/catalog.rb b/lib/puppet/resource/catalog.rb
index 0a63ff1..2fdd19b 100644
--- a/lib/puppet/resource/catalog.rb
+++ b/lib/puppet/resource/catalog.rb
@@ -98,7 +98,7 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph
resource.ref =~ /^(.+)\[/
class_name = $1 || resource.class.name
- newref = [class_name, key]
+ newref = [class_name, key].flatten
if key.is_a? String
ref_string = "#{class_name}[#{key}]"
@@ -111,7 +111,10 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph
# isn't sufficient.
if existing = @resource_table[newref]
return if existing == resource
- raise(ArgumentError, "Cannot alias #{resource.ref} to #{key.inspect};
resource #{newref.inspect} already exists")
+ resource_definition = " at #{resource.file}:#{resource.line}" if
resource.file and resource.line
+ existing_definition = " at #{existing.file}:#{existing.line}" if
existing.file and existing.line
+ msg = "Cannot alias #{resource.ref} to
#{key.inspect}#{resource_definition}; resource #{newref.inspect} already
defined#{existing_definition}"
+ raise ArgumentError, msg
end
@resource_table[newref] = resource
@aliases[resource.ref] ||= []
@@ -377,7 +380,7 @@ class Puppet::Resource::Catalog < Puppet::SimpleGraph
res = Puppet::Resource.new(nil, type)
end
title_key = [res.type, res.title.to_s]
- uniqueness_key = [res.type, res.uniqueness_key]
+ uniqueness_key = [res.type, res.uniqueness_key].flatten
@resource_table[title_key] || @resource_table[uniqueness_key]
end
diff --git a/spec/unit/resource/catalog_spec.rb
b/spec/unit/resource/catalog_spec.rb
index a15a912..4600022 100755
--- a/spec/unit/resource/catalog_spec.rb
+++ b/spec/unit/resource/catalog_spec.rb
@@ -507,7 +507,7 @@ describe Puppet::Resource::Catalog, "when compiling" do
lambda { @catalog.alias(@one, "other") }.should_not raise_error
end
- it "should create aliases for resources isomorphic resources whose names
do not match their titles" do
+ it "should create aliases for isomorphic resources whose names do not
match their titles" do
resource = Puppet::Type::File.new(:title => "testing", :path =>
@basepath+"/something")
@catalog.add_resource(resource)
@@ -515,7 +515,7 @@ describe Puppet::Resource::Catalog, "when compiling" do
@catalog.resource(:file, @basepath+"/something").should equal(resource)
end
- it "should not create aliases for resources non-isomorphic resources whose
names do not match their titles" do
+ it "should not create aliases for non-isomorphic resources whose names do
not match their titles" do
resource = Puppet::Type.type(:exec).new(:title => "testing", :command =>
"echo", :path => %w{/bin /usr/bin /usr/local/bin})
@catalog.add_resource(resource)
@@ -531,11 +531,6 @@ describe Puppet::Resource::Catalog, "when compiling" do
@catalog.resource("notify", "other").should equal(@one)
end
- it "should ignore conflicting aliases that point to the aliased resource"
do
- @catalog.alias(@one, "other")
- lambda { @catalog.alias(@one, "other") }.should_not raise_error
- end
-
it "should fail to add an alias if the aliased name already exists" do
@catalog.add_resource @one
proc { @catalog.alias @two, "one" }.should raise_error(ArgumentError)
@@ -589,6 +584,58 @@ describe Puppet::Resource::Catalog, "when compiling" do
@catalog.create_resource :file, args
@catalog.resource("File[/yay]").should equal(resource)
end
+
+ describe "when adding resources with multiple namevars" do
+ before :each do
+ Puppet::Type.newtype(:multiple) do
+ newparam(:color, :namevar => true)
+ newparam(:designation, :namevar => true)
+
+ def self.title_patterns
+ [ [
+ /^(\w+) (\w+)$/,
+ [
+ [:color, lambda{|x| x}],
+ [:designation, lambda{|x| x}]
+ ]
+ ] ]
+ end
+ end
+ end
+
+ it "should add an alias using the uniqueness key" do
+ @resource = Puppet::Type.type(:multiple).new(:title => "some
resource", :color => "red", :designation => "5")
+
+ @catalog.add_resource(@resource)
+ @catalog.resource(:multiple, "some resource").must == @resource
+ @catalog.resource("Multiple[some resource]").must == @resource
+ @catalog.resource("Multiple[red 5]").must == @resource
+ end
+
+ it "should conflict with a resource with the same uniqueness key" do
+ @resource = Puppet::Type.type(:multiple).new(:title => "some
resource", :color => "red", :designation => "5")
+ @other = Puppet::Type.type(:multiple).new(:title => "another
resource", :color => "red", :designation => "5")
+
+ @catalog.add_resource(@resource)
+ expect { @catalog.add_resource(@other) }.to raise_error(ArgumentError,
/Cannot alias Multiple\[another resource\] to \["red", "5"\].*resource
\["Multiple", "red", "5"\] already defined/)
+ end
+
+ it "should conflict when its uniqueness key matches another resource's
title" do
+ @resource = Puppet::Type.type(:file).new(:title => "/tmp/foo")
+ @other = Puppet::Type.type(:file).new(:title => "another file",
:path => "/tmp/foo")
+
+ @catalog.add_resource(@resource)
+ expect { @catalog.add_resource(@other) }.to raise_error(ArgumentError,
/Cannot alias File\[another file\] to \["\/tmp\/foo"\].*resource \["File",
"\/tmp\/foo"\] already defined/)
+ end
+
+ it "should conflict when its uniqueness key matches the uniqueness key
derived from another resource's title" do
+ @resource = Puppet::Type.type(:multiple).new(:title => "red leader")
+ @other = Puppet::Type.type(:multiple).new(:title => "another
resource", :color => "red", :designation => "leader")
+
+ @catalog.add_resource(@resource)
+ expect { @catalog.add_resource(@other) }.to raise_error(ArgumentError,
/Cannot alias Multiple\[another resource\] to \["red", "leader"\].*resource
\["Multiple", "red", "leader"\] already defined/)
+ end
+ end
end
describe "when applying" do
--
1.7.5.4
--
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to puppet-dev@googlegroups.com.
To unsubscribe from this group, send email to
puppet-dev+unsubscr...@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en.
