+1

On Aug 11, 2009, at 11:06 PM, Markus Roberts wrote:

>
> This was a "There's A Hole In The Bucket" problem caused when trying
>    to establish a connection to get a certificate before there was
>    a certificate with which to establish the connection, ad
>    infinitum.  The solution was to test for the presence of the
>    CA cert as well as the host cert before attempting to use them.

Is that really a named pattern?

>
> This patch modifies existing tests to pass with the new code (by
>    stubbing out the additional FileTests) and adds a new test which
>    catches the original problem.
>
> Signed-off-by: Markus Roberts <[email protected]>
> ---
> lib/puppet/network/http_pool.rb |   10 +++-------
> spec/unit/network/http_pool.rb  |   11 ++++++++++-
> 2 files changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/lib/puppet/network/http_pool.rb b/lib/puppet/network/ 
> http_pool.rb
> index b1206f7..83a0820 100644
> --- a/lib/puppet/network/http_pool.rb
> +++ b/lib/puppet/network/http_pool.rb
> @@ -51,7 +51,7 @@ module Puppet::Network::HttpPool
>     # Use cert information from a Puppet client to set up the http  
> object.
>     def self.cert_setup(http)
>         # Just no-op if we don't have certs.
> -        return false unless FileTest.exist?(Puppet[:hostcert]) #  
> ssl_host.certificate
> +        return false unless FileTest.exist?(Puppet[:hostcert]) and   
> FileTest.exist?(Puppet[:localcacert])
>
>         http.cert_store = ssl_host.ssl_store
>         http.ca_file = Puppet[:localcacert]
> @@ -60,7 +60,7 @@ module Puppet::Network::HttpPool
>         http.key = ssl_host.key.content
>     end
>
> -    # Retrieve a cached http instance of caching is enabled, else  
> return
> +    # Retrieve a cached http instance if caching is enabled, else  
> return
>     # a new one.
>     def self.http_instance(host, port, reset = false)
>         # We overwrite the uninitialized @http here with a cached one.
> @@ -95,11 +95,7 @@ module Puppet::Network::HttpPool
>         http.read_timeout = Puppet[:configtimeout]
>         http.open_timeout = Puppet[:configtimeout]
>         # JJM Configurable fix for #896.
> -        if Puppet[:http_enable_post_connection_check]
> -            http.enable_post_connection_check = true
> -        else
> -            http.enable_post_connection_check = false
> -        end
> +        http.enable_post_connection_check =  
> Puppet[:http_enable_post_connection_check]
>
>         cert_setup(http)
>
> diff --git a/spec/unit/network/http_pool.rb b/spec/unit/network/ 
> http_pool.rb
> index ce76309..65f91ef 100755
> --- a/spec/unit/network/http_pool.rb
> +++ b/spec/unit/network/http_pool.rb
> @@ -147,8 +147,10 @@ describe Puppet::Network::HttpPool do
>             Puppet[:confdir] = "/sometthing/else"
>             Puppet.settings.stubs(:value).returns "/some/file"
>             Puppet.settings.stubs(:value).with(:hostcert).returns "/ 
> host/cert"
> +             
> Puppet.settings.stubs(:value).with(:localcacert).returns "/local/ca/ 
> cert"
>
>             FileTest.stubs(:exist?).with("/host/cert").returns true
> +            FileTest.stubs(:exist?).with("/local/ca/cert").returns  
> true
>
>             Puppet::Network::HttpPool.stubs(:ssl_host).returns @host
>         end
> @@ -157,12 +159,18 @@ describe Puppet::Network::HttpPool do
>             Puppet.settings.clear
>         end
>
> -        it "should do nothing if no certificate is on disk" do
> +        it "should do nothing if no host certificate is on disk" do
>             FileTest.expects(:exist?).with("/host/cert").returns false
>             @http.expects(:cert=).never
>             Puppet::Network::HttpPool.cert_setup(@http)
>         end
>
> +        it "should do nothing if no local certificate is on disk" do
> +            FileTest.expects(:exist?).with("/local/ca/ 
> cert").returns false
> +            @http.expects(:cert=).never
> +            Puppet::Network::HttpPool.cert_setup(@http)
> +        end
> +
>         it "should add a certificate store from the ssl host" do
>             @http.expects(:cert_store=).with(@store)
>
> @@ -192,6 +200,7 @@ describe Puppet::Network::HttpPool do
>             FileTest.stubs(:exist?).with(Puppet[:hostcert]).returns  
> true
>
>             Puppet.settings.stubs(:value).with(:localcacert).returns  
> "/ca/cert/file"
> +            FileTest.stubs(:exist?).with("/ca/cert/file").returns  
> true
>             @http.expects(:ca_file=).with("/ca/cert/file")
>
>             Puppet::Network::HttpPool.cert_setup(@http)
> -- 
> 1.6.0.4
>
>
> >


-- 
The truth is that there is nothing noble in being superior to somebody
else. The only real nobility is in being superior to your former self.
     -- Whitney Young
---------------------------------------------------------------------
Luke Kanies | http://reductivelabs.com | http://madstop.com


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups 
"Puppet Developers" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to 
[email protected]
For more options, visit this group at 
http://groups.google.com/group/puppet-dev?hl=en
-~----------~----~----~----~------~----~------~--~---

Reply via email to