From b2a008e30ea57f0c94d605de855c45c0fdf0e5ce Mon Sep 17 00:00:00 2001 From: Brice Figureau Date: Thu, 9 Jul 2009 20:59:41 +0200 Subject: Fix #2391 - Exported resources never make to the storeconfigs db The issue is that when we convert Puppet::Parser::Resource catalog to a Puppet::Resource catalog before storing it to the database, we don't allow virtual resource to be converted. Unfortunately exported resources are virtual by design, and as such aren't converted, and we lose them, so it isn't possible to store them in the database. Unfortunately, the client will get the exported resources too. The fix is dual-fold: * we make sure exported resource are skipped when the transaction is applied as a last safeguard * we filter-out the catalog through the catalog compiler terminus before the catalog is returned to the client Signed-off-by: Brice Figureau --- spec/unit/indirector/catalog/compiler.rb | 34 ++++++++++++++++++++++++++ spec/unit/indirector/indirection.rb | 8 +++++++ spec/unit/resource/catalog.rb | 41 ++++++++++++++++++++++++++++++++ spec/unit/transaction.rb | 27 +++++++++++++++++++++ spec/unit/type.rb | 6 ++++- 5 files changed, 115 insertions(+), 1 deletion(-) (limited to 'spec') diff --git a/spec/unit/indirector/catalog/compiler.rb b/spec/unit/indirector/catalog/compiler.rb index 6e49a6529..78a8028a8 100755 --- a/spec/unit/indirector/catalog/compiler.rb +++ b/spec/unit/indirector/catalog/compiler.rb @@ -223,4 +223,38 @@ describe Puppet::Resource::Catalog::Compiler do @compiler.find(@request) end end + + describe "when filtering resources" do + before :each do + @compiler = Puppet::Resource::Catalog::Compiler.new + @catalog = stub_everything 'catalog' + @catalog.stubs(:respond_to?).with(:filter).returns(true) + end + + it "should delegate to the catalog instance filtering" do + @catalog.expects(:filter) + @compiler.filter(@catalog) + end + + it "should filter out exported resources" do + resource = mock 'resource', :exported? => true + @catalog.stubs(:filter).yields(resource) + + @compiler.filter(@catalog) + end + + it "should return the same catalog if it doesn't support filtering" do + @catalog.stubs(:respond_to?).with(:filter).returns(false) + + @compiler.filter(@catalog).should == @catalog + end + + it "should return the filtered catalog" do + catalog = stub 'filtered catalog' + @catalog.stubs(:filter).returns(catalog) + + @compiler.filter(@catalog).should == catalog + end + + end end diff --git a/spec/unit/indirector/indirection.rb b/spec/unit/indirector/indirection.rb index 5d9efd2ea..220aa24fe 100755 --- a/spec/unit/indirector/indirection.rb +++ b/spec/unit/indirector/indirection.rb @@ -243,6 +243,14 @@ describe Puppet::Indirector::Indirection do @indirection.find("/my/key") end + it "should filter the result instance if the terminus supports it" do + @terminus.stubs(:find).returns(@instance) + @terminus.stubs(:respond_to?).with(:filter).returns(true) + + @terminus.expects(:filter).with(@instance) + + @indirection.find("/my/key") + end describe "when caching is enabled" do before do @indirection.cache_class = :cache_terminus diff --git a/spec/unit/resource/catalog.rb b/spec/unit/resource/catalog.rb index 2f4476a2b..97b6ad7cc 100755 --- a/spec/unit/resource/catalog.rb +++ b/spec/unit/resource/catalog.rb @@ -295,6 +295,47 @@ describe Puppet::Resource::Catalog, "when compiling" do end end + describe "when filtering" do + before :each do + @original = Puppet::Resource::Catalog.new("mynode") + @original.tag(*%w{one two three}) + @original.add_class *%w{four five six} + + @r1 = stub_everything 'r1', :ref => "File[/a]" + @r1.stubs(:respond_to?).with(:ref).returns(true) + @r1.stubs(:dup).returns(@r1) + @r1.stubs(:is_a?).returns(Puppet::Resource).returns(true) + + @r2 = stub_everything 'r2', :ref => "File[/b]" + @r2.stubs(:respond_to?).with(:ref).returns(true) + @r2.stubs(:dup).returns(@r2) + @r2.stubs(:is_a?).returns(Puppet::Resource).returns(true) + + @resources = [@r1,@r2] + + @original.add_resource(@r1,@r2) + end + + it "should transform the catalog to a resource catalog" do + @original.expects(:to_catalog).with { |h,b| h == :to_resource } + + @original.filter + end + + it "should scan each catalog resource in turn and apply filtering block" do + @resources.each { |r| r.expects(:exported?) } + @original.filter do |r| + r.exported? + end + end + + it "should filter out resources which produce true when the filter block is evaluated" do + @original.filter do |r| + r == @r1 + end.resource("File[/a]").should be_nil + end + end + describe "when functioning as a resource container" do before do @catalog = Puppet::Resource::Catalog.new("host") diff --git a/spec/unit/transaction.rb b/spec/unit/transaction.rb index 37870f126..26154e9b9 100755 --- a/spec/unit/transaction.rb +++ b/spec/unit/transaction.rb @@ -52,6 +52,33 @@ describe Puppet::Transaction do @transaction.generate_additional_resources(generator, :generate).should be_empty end end + + describe "when skipping a resource" do + before :each do + @resource = stub_everything 'res', :exported? => true + @catalog = Puppet::Resource::Catalog.new + @transaction = Puppet::Transaction.new(@catalog) + end + + it "should skip resource with missing tags" do + @transaction.stubs(:missing_tags?).returns(true) + @transaction.skip?(@resource).should be_true + end + + it "should skip not scheduled resources" do + @transaction.stubs(:scheduled?).returns(false) + @transaction.skip?(@resource).should be_true + end + + it "should skip resources with failed dependencies" do + @transaction.stubs(:failed_dependencies?).returns(false) + @transaction.skip?(@resource).should be_true + end + + it "should skip exported resource" do + @transaction.skip?(@resource).should be_true + end + end end describe Puppet::Transaction, " when determining tags" do diff --git a/spec/unit/type.rb b/spec/unit/type.rb index a9e48a274..a55009dfc 100755 --- a/spec/unit/type.rb +++ b/spec/unit/type.rb @@ -68,6 +68,10 @@ describe Puppet::Type do resource.tags = [:tag1,:tag2] end + it "should have a method to know if the resource is exported" do + Puppet::Type.type(:mount).new(:name => "foo").should respond_to(:exported?) + end + describe "when initializing" do describe "and passed a TransObject" do it "should fail" do @@ -87,7 +91,7 @@ describe Puppet::Type do Puppet::Type.type(:mount).new(resource).title.should == "User[foo]" end - [:line, :file, :catalog].each do |param| + [:line, :file, :catalog, :exported].each do |param| it "should copy '#{param}' from the resource if present" do resource = Puppet::Resource.new(:mount, "/foo") resource.send(param.to_s + "=", "foo") -- cgit