diff options
| -rw-r--r-- | lib/puppet/executables/client/certhandler.rb | 15 | ||||
| -rw-r--r-- | lib/puppet/parser/interpreter.rb | 19 | ||||
| -rwxr-xr-x | spec/unit/executables/client/certhandler.rb | 93 | ||||
| -rwxr-xr-x | spec/unit/parser/interpreter.rb | 27 |
4 files changed, 86 insertions, 68 deletions
diff --git a/lib/puppet/executables/client/certhandler.rb b/lib/puppet/executables/client/certhandler.rb index 6f23de0e3..b73642941 100644 --- a/lib/puppet/executables/client/certhandler.rb +++ b/lib/puppet/executables/client/certhandler.rb @@ -4,8 +4,7 @@ module Puppet module Client class CertHandler attr_writer :wait_for_cert, :one_time - - attr_reader :caclient + attr_reader :caclient, :new_cert def initialize(wait_time, is_one_time) @wait_for_cert = wait_time @@ -14,7 +13,14 @@ module Puppet @caclient = Puppet::Network::Client.ca.new() end + + # Did we just read a cert? + def new_cert? + new_cert + end + # Read, or retrieve if necessary, our certificate. Returns true if we retrieved + # a new cert, false if the cert already exists. def read_retrieve #NOTE: ACS this is checking that a file exists, maybe next time just do that? unless read_cert @@ -23,7 +29,7 @@ module Puppet retrieve_cert end - !@new_cert + ! new_cert? end def retrieve_cert @@ -54,7 +60,8 @@ module Puppet def read_new_cert if caclient.read_cert # If we read it in, then we need to get rid of our existing http connection. - # The @new_cert flag will help us do that + # The @new_cert flag will help us do that, in that it provides a way + # to notify that the cert status has changed. @new_cert = true Puppet.notice "Got signed certificate" else diff --git a/lib/puppet/parser/interpreter.rb b/lib/puppet/parser/interpreter.rb index 04ca41494..423c34a4e 100644 --- a/lib/puppet/parser/interpreter.rb +++ b/lib/puppet/parser/interpreter.rb @@ -50,23 +50,8 @@ class Puppet::Parser::Interpreter # Return the parser for a specific environment. def parser(environment) if ! @parsers[environment] or @parsers[environment].reparse? - # This will throw an exception if it does not succeed. We only - # want to get rid of the old parser if we successfully create a new - # one. - begin - tmp = create_parser(environment) - @parsers[environment].clear if @parsers[environment] - @parsers[environment] = tmp - rescue => detail - # If a parser already exists, than assume that we logged the - # exception elsewhere and reuse the parser. If one doesn't - # exist, then reraise. - if @parsers[environment] - Puppet.err(detail.to_s + "; using previously parsed manifests") - else - raise detail - end - end + # This will throw an exception if it does not succeed. + @parsers[environment] = create_parser(environment) end @parsers[environment] end diff --git a/spec/unit/executables/client/certhandler.rb b/spec/unit/executables/client/certhandler.rb index 3737c9a57..5950aa4dc 100755 --- a/spec/unit/executables/client/certhandler.rb +++ b/spec/unit/executables/client/certhandler.rb @@ -27,62 +27,109 @@ describe cert_handler, "when handling certificates" do ch.read_retrieve.should be_false end + describe "when reading or retrieving the certificate" do + before do + @handler = cert_handler.new(1,true) + end + + it "should attempt to read the certificate" do + @handler.expects(:read_cert).returns true + @handler.read_retrieve + end + + it "should delegate to the ca client to read the certificate" do + @caclient.expects(:read_cert).returns(true) + @handler.read_retrieve + end + + it "should not attempt to retrieve a certificate if one can be read" do + @handler.stubs(:read_cert).returns true + @handler.expects(:retrieve_cert).never + @handler.read_retrieve + end + + it "should attempt to retrieve a certificate if none can be read" do + @handler.stubs(:read_cert).returns false + @handler.expects(:retrieve_cert) + @handler.read_retrieve + end + + it "should delegate to caclient to retrieve a certificate" do + @handler.stubs(:read_cert).returns false + @caclient.expects(:request_cert).returns(true) + @handler.stubs(:read_new_cert).returns(true) + @handler.read_retrieve + end + + it "should return true if the certificate exists" do + @handler.stubs(:read_cert).returns true + @handler.read_retrieve.should be_true + end + + it "should return false when getting a new cert" do + #This is the second call to httppool that happens in 'read_new_cert' + @caclient.expects(:read_cert).returns(true) + @caclient.stubs(:request_cert).returns(true) + @handler.stubs(:read_cert).returns(false) + @handler.read_retrieve.should be_false + end + end + describe "when waiting for cert" do before do - @caclient.stubs(:read_cert).returns false + @handler = cert_handler.new(1,false) + @handler.stubs(:read_cert).returns false + #all waiting for cert tests should loop, which will always happen if sleep is called + #yeah, I put the expectation in the setup, deal with it + @handler.expects(:sleep).with(1) + + #This is needed to get out of the loop + @handler.stubs(:read_new_cert).returns(true) end it "should loop when the cert request does not return a certificate" do @caclient.stubs(:request_cert).times(2).returns(false).then.returns(true) - ch = cert_handler.new(1,false) - ch.expects(:sleep) - ch.expects(:read_new_cert).returns(true) - ch.read_retrieve + @handler.retrieve_cert end it "should loop when the cert request raises an Error" do @caclient.stubs(:request_cert).times(2).raises(StandardError, 'Testing').then.returns(true) - ch = cert_handler.new(1,false) - ch.expects(:sleep) - ch.expects(:read_new_cert).returns(true) - ch.read_retrieve + @handler.retrieve_cert end it "should loop when the new cert can't be read" do @caclient.stubs(:request_cert).returns(true) - ch = cert_handler.new(1,false) - ch.expects(:sleep) - ch.expects(:read_new_cert).times(2).returns(false).then.returns(true) - ch.read_retrieve + @handler.stubs(:read_new_cert).times(2).returns(false).then.returns(true) + @handler.retrieve_cert end end describe "when in one time mode" do before do - @caclient.stubs(:read_cert).returns false + #true puts us in onetime mode + @handler = cert_handler.new(1,true) + @handler.stubs(:read_cert).returns false end it "should exit if the cert request does not return a certificate" do @caclient.stubs(:request_cert).returns(false) - ch = cert_handler.new(1,true) - ch.expects(:exit).with(1).raises(SystemExit) - lambda { ch.read_retrieve }.should raise_error(SystemExit) + @handler.expects(:exit).with(1).raises(SystemExit) + lambda { @handler.retrieve_cert }.should raise_error(SystemExit) end it "should exit if the cert request raises an exception" do @caclient.stubs(:request_cert).raises(StandardError, 'Testing') - ch = cert_handler.new(1,true) - ch.expects(:exit).with(23).raises(SystemExit) - lambda { ch.read_retrieve }.should raise_error(SystemExit) + @handler.expects(:exit).with(23).raises(SystemExit) + lambda { @handler.retrieve_cert }.should raise_error(SystemExit) end it "should exit if the new cert can't be read" do @caclient.stubs(:request_cert).returns(true) + #this is the second, call to httppool inside read_new_cert @caclient.stubs(:read_cert).returns(false) - ch = cert_handler.new(1,true) - ch.expects(:exit).with(34).raises(SystemExit) - lambda { ch.read_retrieve }.should raise_error(SystemExit) + @handler.expects(:exit).with(34).raises(SystemExit) + lambda { @handler.retrieve_cert }.should raise_error(SystemExit) end end end diff --git a/spec/unit/parser/interpreter.rb b/spec/unit/parser/interpreter.rb index f2526c73d..27cf7056d 100755 --- a/spec/unit/parser/interpreter.rb +++ b/spec/unit/parser/interpreter.rb @@ -84,7 +84,6 @@ describe Puppet::Parser::Interpreter do oldparser = mock('oldparser') newparser = mock('newparser') oldparser.expects(:reparse?).returns(true) - oldparser.expects(:clear) @interp.expects(:create_parser).with(:myenv).returns(oldparser) @interp.send(:parser, :myenv).should equal(oldparser) @@ -92,36 +91,16 @@ describe Puppet::Parser::Interpreter do @interp.send(:parser, :myenv).should equal(newparser) end - it "should keep the old parser if a new parser cannot be created" do + it "should raise an exception if a new parser cannot be created" do # Get the first parser in the hash. @interp.expects(:create_parser).with(:myenv).returns(@parser) @interp.send(:parser, :myenv).should equal(@parser) - # Have it indicate something has changed @parser.expects(:reparse?).returns(true) - # But fail to create a new parser - @interp.expects(:create_parser).with(:myenv).raises(ArgumentError) + @interp.expects(:create_parser).with(:myenv).raises(Puppet::Error, "Could not parse") - # And make sure we still get the old valid parser - @interp.send(:parser, :myenv).should equal(@parser) - end - - it "should log syntax errors when using the old parser" do - # Get the first parser in the hash. - @interp.stubs(:create_parser).with(:myenv).returns(@parser) - @interp.send(:parser, :myenv) - - # Have it indicate something has changed - @parser.stubs(:reparse?).returns(true) - - # But fail to create a new parser - @interp.stubs(:create_parser).with(:myenv).raises(ArgumentError) - - Puppet.expects(:err) - - # And make sure we still get the old valid parser - @interp.send(:parser, :myenv) + lambda { @interp.parser(:myenv) }.should raise_error(Puppet::Error) end end end |