+1, one very minor comment below On Apr 19, 2009, at 1:51 PM, Christian Hofstaedtler wrote:
> > From: Christian Hofstaedtler <[email protected]> > > > Signed-off-by: Christian Hofstaedtler <[email protected]> > --- > lib/puppet/network/http/rack.rb | 21 ++++- > lib/puppet/network/http/rack/xmlrpc.rb | 46 ++++++++ > spec/unit/network/http/rack.rb | 29 +++++- > spec/unit/network/http/rack/xmlrpc.rb | 182 +++++++++++++++++++++++ > +++++++++ > 4 files changed, 275 insertions(+), 3 deletions(-) > create mode 100644 lib/puppet/network/http/rack/xmlrpc.rb > create mode 100755 spec/unit/network/http/rack/xmlrpc.rb > > diff --git a/lib/puppet/network/http/rack.rb b/lib/puppet/network/ > http/rack.rb > index 55e50c4..cf6cdc8 100644 > --- a/lib/puppet/network/http/rack.rb > +++ b/lib/puppet/network/http/rack.rb > @@ -2,6 +2,7 @@ > require 'rack' > require 'puppet/network/http' > require 'puppet/network/http/rack/rest' > +require 'puppet/network/http/rack/xmlrpc' > > # An rack application, for running the Puppet HTTP Server. > class Puppet::Network::HTTP::Rack > @@ -14,21 +15,37 @@ class Puppet::Network::HTTP::Rack > @rest_http_handler = Puppet::Network::HTTP::RackREST.new() > protocols.delete :rest > > + # Prepare the XMLRPC handler, for backward compatibility > (if requested) > + if args[:protocols].include?(:xmlrpc) > + raise ArgumentError, "XMLRPC was requested, but no > handlers were given" if !args.include?(:xmlrpc_handlers) I think this always looks a bit cleaner if it says 'unless args.include?...', rather than 'if !'. Small point, of course. > > + @xmlrpc_http_handler = > Puppet::Network::HTTP::RackXMLRPC.new(args[:xmlrpc_handlers]) > + protocols.delete :xmlrpc > + end > + > raise ArgumentError, "there were unknown :protocols > specified." if !protocols.empty? > end > > # The real rack application (which needs to respond to call). > # The work we need to do, roughly is: > # * Read request (from env) and prepare a response > - # * Route the request > + # * Route the request to the correct handler > # * Return the response (in rack-format) to our caller. > def call(env) > request = Rack::Request.new(env) > response = Rack::Response.new() > Puppet.debug 'Handling request: %s %s' % > [request.request_method, request.fullpath] > > + # if we shall serve XMLRPC, have /RPC2 go to the xmlrpc > handler > + if @xmlrpc_http_handler and request.path_info.start_with?('/ > RPC2') > + handler = @xmlrpc_http_handler > + else > + # everything else is handled by the new REST handler > + handler = @rest_http_handler > + end > + > begin > - @rest_http_handler.process(request, response) > + handler.process(request, response) > rescue => detail > # Send a Status 500 Error on unhandled exceptions. > response.status = 500 > diff --git a/lib/puppet/network/http/rack/xmlrpc.rb b/lib/puppet/ > network/http/rack/xmlrpc.rb > new file mode 100644 > index 0000000..88a3c5a > --- /dev/null > +++ b/lib/puppet/network/http/rack/xmlrpc.rb > @@ -0,0 +1,46 @@ > +require 'puppet/network/http/rack/httphandler' > +require 'puppet/network/xmlrpc/server' > + > +class Puppet::Network::HTTP::RackXMLRPC < > Puppet::Network::HTTP::RackHttpHandler > + def initialize(handlers) > + @xmlrpc_server = Puppet::Network::XMLRPCServer.new > + handlers.each do |name| > + Puppet.debug " -> register xmlrpc namespace %s" % name > + unless handler = Puppet::Network::Handler.handler(name) > + raise ArgumentError, "Invalid XMLRPC handler %s" % > name > + end > + @xmlrpc_server.add_handler(handler.interface, > handler.new({})) > + end > + super() > + end > + > + def process(request, response) > + # errors are sent as text/plain > + response['Content-Type'] = 'text/plain' > + if not request.post? then > + response.status = 405 > + response.write 'Method Not Allowed' > + return > + end > + if request.media_type() != "text/xml" then > + response.status = 400 > + response.write 'Bad Request' > + return > + end > + > + # get auth/certificate data > + info = extract_client_info(request) > + if not info[:node] > + info[:node] = nil > + end > + info = Puppet::Network::ClientRequest.new(info[:node], > info[:ip], info[:authenticated]) > + > + response_body = @xmlrpc_server.process(request.body, info) > + > + response.status = 200 > + response['Content-Type'] = 'text/xml; charset=utf-8' > + response.write response_body > + end > + > +end > + > diff --git a/spec/unit/network/http/rack.rb b/spec/unit/network/http/ > rack.rb > index 2a575e2..70fc8ad 100644 > --- a/spec/unit/network/http/rack.rb > +++ b/spec/unit/network/http/rack.rb > @@ -10,7 +10,7 @@ describe "Puppet::Network::HTTP::Rack", "while > initializing" do > Proc.new { Puppet::Network::HTTP::Rack.new({}) }.should > raise_error(ArgumentError) > end > > - it "should only accept the REST protocol" do > + it "should not accept imaginary protocols" do > Proc.new { Puppet::Network::HTTP::Rack.new({:protocols => > [:foo]}) }.should raise_error(ArgumentError) > end > > @@ -23,6 +23,17 @@ describe "Puppet::Network::HTTP::Rack", "while > initializing" do > Puppet::Network::HTTP::Rack.new({:protocols => [:rest]}) > end > > + describe "when serving XMLRPC" do > + it "should require XMLRPC handlers" do > + Proc.new { Puppet::Network::HTTP::Rack.new({:protocols > => [:xmlrpc]}) }.should raise_error(ArgumentError) > + end > + > + it "should create a RackXMLRPC instance" do > + Puppet::Network::HTTP::RackXMLRPC.expects(:new) > + Puppet::Network::HTTP::Rack.new({:protocols => > [:xmlrpc], :xmlrpc_handlers => [:Status]}) > + end > + end > + > end > > describe "Puppet::Network::HTTP::Rack", "when called" do > @@ -65,5 +76,21 @@ describe "Puppet::Network::HTTP::Rack", "when > called" do > Rack::Response.any_instance.expects(:finish).once > @app.call(@env) # can't lint when finish is a stub > end > + > + describe "when serving XMLRPC" do > + before :all do > + @app = Puppet::Network::HTTP::Rack.new({:protocols => > [:rest, :xmlrpc], :xmlrpc_handlers => [:Status]}) > + @linted = Rack::Lint.new(@app) > + end > + > + before :each do > + @env = Rack::MockRequest.env_for('/RPC2', :method => > 'POST') > + end > + > + it "should use RackXMLRPC to serve /RPC2 requests" do > + > Puppet::Network::HTTP::RackXMLRPC.any_instance.expects(:process).once > + @linted.call(@env) > + end > + end > end > > diff --git a/spec/unit/network/http/rack/xmlrpc.rb b/spec/unit/ > network/http/rack/xmlrpc.rb > new file mode 100755 > index 0000000..60a4cf7 > --- /dev/null > +++ b/spec/unit/network/http/rack/xmlrpc.rb > @@ -0,0 +1,182 @@ > +#!/usr/bin/env ruby > + > +require File.dirname(__FILE__) + '/../../../../spec_helper' > +require 'puppet/network/http/rack' > +require 'puppet/network/http/rack/xmlrpc' > + > +describe "Puppet::Network::HTTP::RackXMLRPC" do > + confine "Rack is not available" => Puppet.features.rack? > + > + describe "when initializing" do > + it "should create an Puppet::Network::XMLRPCServer" do > + Puppet::Network::XMLRPCServer.expects(:new).returns > stub_everything > + Puppet::Network::HTTP::RackXMLRPC.new([]) > + end > + > + it "should create each handler" do > + handler = stub_everything 'handler' > + > Puppet::Network::XMLRPCServer.any_instance.stubs(:add_handler) > + > Puppet::Network::Handler.expects(:handler).returns(handler).times(2) > + Puppet::Network::HTTP::RackXMLRPC.new([:foo, :bar]) > + end > + > + it "should add each handler to the XMLRPCserver" do > + handler = stub_everything 'handler' > + Puppet::Network::Handler.stubs(:handler).returns(handler) > + > Puppet > ::Network::XMLRPCServer.any_instance.expects(:add_handler).times(2) > + Puppet::Network::HTTP::RackXMLRPC.new([:foo, :bar]) > + end > + end > + > + describe "when serving a request" do > + > + before :each do > + foo_handler = stub_everything 'foo_handler' > + > Puppet::Network::Handler.stubs(:handler).with(:foo).returns > foo_handler > + > Puppet::Network::XMLRPCServer.any_instance.stubs(:add_handler) > + > Puppet > ::Network::XMLRPCServer.any_instance.stubs(:process).returns('<xml/>') > + @handler = Puppet::Network::HTTP::RackXMLRPC.new([:foo]) > + end > + > + before :each do > + @response = Rack::Response.new() > + end > + > + def mk_req(opts = {}) > + opts[:method] = 'POST' if !opts[:method] > + opts['CONTENT_TYPE'] = 'text/xml; foo=bar' if ! > opts['CONTENT_TYPE'] > + env = Rack::MockRequest.env_for('/RPC2', opts) > + Rack::Request.new(env) > + end > + > + it "should reject non-POST requests" do > + req = mk_req :method => 'PUT' > + @handler.process(req, @response) > + @response.status.should == 405 > + end > + > + it "should reject non text/xml requests" do > + req = mk_req 'CONTENT_TYPE' => 'yadda/plain' > + end > + > + it "should create a ClientRequest" do > + cr = Puppet::Network::ClientRequest.new(nil, > '127.0.0.1', false) > + Puppet::Network::ClientRequest.expects(:new).returns cr > + req = mk_req > + @handler.process(req, @response) > + end > + > + it "should let xmlrpcserver process the request" do > + > Puppet > ::Network::XMLRPCServer.any_instance.expects(:process).returns('yay') > + req = mk_req > + @handler.process(req, @response) > + end > + > + it "should report the response as OK" do > + req = mk_req > + @handler.process(req, @response) > + @response.status.should == 200 > + end > + > + it "should report the response with the correct content > type" do > + req = mk_req > + @handler.process(req, @response) > + @response['Content-Type'].should == 'text/xml; > charset=utf-8' > + end > + > + it "should set 'authenticated' to false if no certificate > is present" do > + req = mk_req > + Puppet::Network::ClientRequest.expects(:new).with() { | > node,ip,authenticated| authenticated == false } > + @handler.process(req, @response) > + end > + > + it "should use the client's ip address" do > + req = mk_req 'REMOTE_ADDR' => 'ipaddress' > + Puppet::Network::ClientRequest.expects(:new).with() { | > node,ip,authenticated| ip == 'ipaddress' } > + @handler.process(req, @response) > + end > + > + describe "with pre-validated certificates" do > + > + it "should use the :ssl_client_header to determine the > parameter when looking for the certificate" do > + Puppet.settings.stubs(:value).returns "eh" > + > Puppet.settings.expects(:value).with(:ssl_client_header).returns > "myheader" > + req = mk_req "myheader" => "/CN=host.domain.com" > + @handler.process(req, @response) > + end > + > + it "should retrieve the hostname by matching the > certificate parameter" do > + Puppet.settings.stubs(:value).returns "eh" > + > Puppet.settings.expects(:value).with(:ssl_client_header).returns > "myheader" > + Puppet::Network::ClientRequest.expects(:new).with() > { |node,ip,authenticated| node == "host.domain.com" } > + req = mk_req "myheader" => "/CN=host.domain.com" > + @handler.process(req, @response) > + end > + > + it "should use the :ssl_client_header to determine the > parameter for checking whether the host certificate is valid" do > + > Puppet.settings.stubs(:value).with(:ssl_client_header).returns > "certheader" > + > Puppet > .settings.expects(:value).with(:ssl_client_verify_header).returns > "myheader" > + req = mk_req "myheader" => "SUCCESS", "certheader" > => "/CN=host.domain.com" > + @handler.process(req, @response) > + end > + > + it "should consider the host authenticated if the > validity parameter contains 'SUCCESS'" do > + > Puppet.settings.stubs(:value).with(:ssl_client_header).returns > "certheader" > + > Puppet > .settings.stubs(:value).with(:ssl_client_verify_header).returns > "myheader" > + Puppet::Network::ClientRequest.expects(:new).with() > { |node,ip,authenticated| authenticated == true } > + req = mk_req "myheader" => "SUCCESS", "certheader" > => "/CN=host.domain.com" > + @handler.process(req, @response) > + end > + > + it "should consider the host unauthenticated if the > validity parameter does not contain 'SUCCESS'" do > + > Puppet.settings.stubs(:value).with(:ssl_client_header).returns > "certheader" > + > Puppet > .settings.stubs(:value).with(:ssl_client_verify_header).returns > "myheader" > + Puppet::Network::ClientRequest.expects(:new).with() > { |node,ip,authenticated| authenticated == false } > + req = mk_req "myheader" => "whatever", "certheader" > => "/CN=host.domain.com" > + @handler.process(req, @response) > + end > + > + it "should consider the host unauthenticated if no > certificate information is present" do > + > Puppet.settings.stubs(:value).with(:ssl_client_header).returns > "certheader" > + > Puppet > .settings.stubs(:value).with(:ssl_client_verify_header).returns > "myheader" > + Puppet::Network::ClientRequest.expects(:new).with() > { |node,ip,authenticated| authenticated == false } > + req = mk_req "myheader" => nil, "certheader" => "/ > CN=host.domain.com" > + @handler.process(req, @response) > + end > + > + it "should not have a node name if no certificate > information is present" do > + Puppet.settings.stubs(:value).returns "eh" > + > Puppet.settings.expects(:value).with(:ssl_client_header).returns > "myheader" > + Puppet::Network::ClientRequest.expects(:new).with() > { |node,ip,authenticated| node == nil } > + req = mk_req "myheader" => nil > + @handler.process(req, @response) > + end > + end > + > + describe "with in-header certificates" do > + it "should set 'authenticated' to true if a certificate > is present" do > + cert = stub 'cert', :subject => [%w{CN > host.domain.com}], :to_text => 'yay' > + > OpenSSL::X509::Certificate.expects(:new).returns(cert) > + > Puppet > ::SSL > ::Certificate.expects(:find).with('host.domain.com').returns(cert) > + Puppet::Network::ClientRequest.expects(:new).with() > { |node,ip,authenticated| authenticated == true } > + req = mk_req 'SSL_CLIENT_CERT' => cert > + @handler.process(req, @response) > + end > + > + it "should pass the client's certificate name to model > method if a certificate is present" do > + cert = stub 'cert', :subject => [%w{CN > host.domain.com}], :to_text => 'yay' > + > OpenSSL::X509::Certificate.expects(:new).returns(cert) > + > Puppet > ::SSL > ::Certificate.expects(:find).with('host.domain.com').returns(cert) > + Puppet::Network::ClientRequest.expects(:new).with() > { |node,ip,authenticated| node == "host.domain.com" } > + req = mk_req 'SSL_CLIENT_CERT' => cert > + @handler.process(req, @response) > + end > + > + it "should not have a node name if no certificate is > present" do > + req = mk_req 'SSL_CLIENT_CERT' => nil > + Puppet::Network::ClientRequest.expects(:new).with() > { |node,ip,authenticated| node == nil } > + @handler.process(req, @response) > + end > + end > + end > +end > -- > 1.5.6.5 > > > > -- It's not the voting that's democracy, it's the counting. -- Tom Stoppard --------------------------------------------------------------------- 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 -~----------~----~----~----~------~----~------~--~---
