Hi, A few minor comments inline.
Is it against latest master? I'm asking the question because since #1875 has been merged I think some of your rack/rest test might not pass because of the call to check_authorization. I'll check later. On 28/04/09 15:41, Christian Hofstaedtler wrote: > From: Christian Hofstaedtler <[email protected]> > > This lays the ground: a wrapper for the REST handler, and an application > confirming to the Rack standard. Also includes a base class for Rack > handlers, as RackREST will not stay the only one, and there needs to be > a central place where client authentication data can be checked. > > Signed-off-by: Christian Hofstaedtler <[email protected]> > --- > lib/puppet/feature/base.rb | 3 + > lib/puppet/network/http/rack.rb | 62 +++++++++ > lib/puppet/network/http/rack/httphandler.rb | 35 +++++ > lib/puppet/network/http/rack/rest.rb | 75 +++++++++++ > spec/unit/network/http/rack.rb | 102 ++++++++++++++ > spec/unit/network/http/rack/rest.rb | 193 > +++++++++++++++++++++++++++ > 6 files changed, 470 insertions(+), 0 deletions(-) > create mode 100644 lib/puppet/network/http/rack.rb > create mode 100644 lib/puppet/network/http/rack/httphandler.rb > create mode 100644 lib/puppet/network/http/rack/rest.rb > create mode 100644 spec/unit/network/http/rack.rb You forgot, as I always do, to chmod 755 this spec :-) > create mode 100755 spec/unit/network/http/rack/rest.rb > > diff --git a/lib/puppet/feature/base.rb b/lib/puppet/feature/base.rb > index c3fb9a2..7c0f241 100644 > --- a/lib/puppet/feature/base.rb > +++ b/lib/puppet/feature/base.rb > @@ -28,3 +28,6 @@ Puppet.features.add(:augeas, :libs => ["augeas"]) > > # We have RRD available > Puppet.features.add(:rrd, :libs => ["RRDtool"]) > + > +# We have rack available, an HTTP Application Stack > +Puppet.features.add(:rack, :libs => ["rack"]) > diff --git a/lib/puppet/network/http/rack.rb b/lib/puppet/network/http/rack.rb > new file mode 100644 > index 0000000..cf6cdc8 > --- /dev/null > +++ b/lib/puppet/network/http/rack.rb > @@ -0,0 +1,62 @@ > + > +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 > + > + def initialize(args) > + raise ArgumentError, ":protocols must be specified." if > !args[:protocols] or args[:protocols].empty? > + protocols = args[:protocols] > + > + # Always prepare a REST handler > + @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) > + > + @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 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 > + handler.process(request, response) > + rescue => detail > + # Send a Status 500 Error on unhandled exceptions. > + response.status = 500 > + response['Content-Type'] = 'text/plain' > + response.write 'Internal Server Error' > + # log what happened > + Puppet.err "Puppet Server (Rack): Internal Server Error: > Unhandled Exception: \"%s\"" % detail.message > + Puppet.err "Backtrace:" > + detail.backtrace.each { |line| Puppet.err " > %s" % line } Other part of Puppet just do: puts detail.backtrace if Puppet[:trace] Are you doing this so that it ends in the passenger log or something? > + end > + response.finish() > + end > +end > + > diff --git a/lib/puppet/network/http/rack/httphandler.rb > b/lib/puppet/network/http/rack/httphandler.rb > new file mode 100644 > index 0000000..ac611a8 > --- /dev/null > +++ b/lib/puppet/network/http/rack/httphandler.rb > @@ -0,0 +1,35 @@ > +require 'openssl' > +require 'puppet/ssl/certificate' > + > +class Puppet::Network::HTTP::RackHttpHandler > + > + def initialize() > + end > + > + # do something useful with request (a Rack::Request) and use > + # response to fill your Rack::Response > + def process(request, response) > + raise NotImplementedError, "Your RackHttpHandler subclass is > supposed to override service(request)" > + end > + > + def extract_client_info(request) > + ip = request.ip > + valid = false > + client = nil > + > + # if we find SSL info in the headers, use them to get a hostname. > + # try this with :ssl_client_header, which defaults should work for > + # Apache with StdEnvVars. > + if dn = request.env[Puppet[:ssl_client_header]] and dn_matchdata = > dn.match(/^.*?CN\s*=\s*(.*)/) > + client = dn_matchdata[1].to_str > + valid = (request.env[Puppet[:ssl_client_verify_header]] == > 'SUCCESS') > + end > + > + result = {:ip => ip, :authenticated => valid} > + if client > + result[:node] = client > + end > + result > + end > +end > + > diff --git a/lib/puppet/network/http/rack/rest.rb > b/lib/puppet/network/http/rack/rest.rb > new file mode 100644 > index 0000000..24d2a26 > --- /dev/null > +++ b/lib/puppet/network/http/rack/rest.rb > @@ -0,0 +1,75 @@ > +require 'puppet/network/http/handler' > +require 'puppet/network/http/rack/httphandler' > + > +class Puppet::Network::HTTP::RackREST < > Puppet::Network::HTTP::RackHttpHandler > + > + include Puppet::Network::HTTP::Handler > + > + HEADER_ACCEPT = 'HTTP_ACCEPT'.freeze > + ContentType = 'Content-Type'.freeze > + > + def initialize(args={}) > + super() > + initialize_for_puppet(args) > + end > + > + def set_content_type(response, format) > + response[ContentType] = format > + end > + > + # produce the body of the response > + def set_response(response, result, status = 200) > + response.status = status > + response.write result > + end > + > + # Retrieve the accept header from the http request. > + def accept_header(request) > + request.env[HEADER_ACCEPT] > + end > + > + # Return which HTTP verb was used in this request. > + def http_method(request) > + request.request_method > + end > + > + # Return the query params for this request. > + def params(request) > + result = decode_params(request.params) > + result.merge(extract_client_info(request)) > + end > + > + # what path was requested? > + def path(request) > + request.fullpath > + end > + > + # return the request body > + # request.body has some limitiations, so we need to concat it back > + # into a regular string, which is something puppet can use. > + def body(request) > + body = '' > + request.body.each { |part| body += part } > + body > + end > + > + def extract_client_info(request) > + ip = request.ip > + valid = false > + client = nil > + > + # if we find SSL info in the headers, use them to get a hostname. > + # try this with :ssl_client_header, which defaults should work for > + # Apache with StdEnvVars. > + if dn = request.env[Puppet[:ssl_client_header]] and dn_matchdata = > dn.match(/^.*?CN\s*=\s*(.*)/) > + client = dn_matchdata[1].to_str > + valid = (request.env[Puppet[:ssl_client_verify_header]] == > 'SUCCESS') > + end > + > + result = {:ip => ip, :authenticated => valid} > + if client > + result[:node] = client > + end > + result While fixing #1875, I added a case for unauthenticated case where we resolve the IP address to fill the resul[:node] (and if it fails we fill it with the IP). I did that to still be able to use the authorization layer in the unauthenticated code. Note that this code seems to be a copy of the one you have in Puppet::Network::HTTP::RackHttpHandler. > + end > +end > diff --git a/spec/unit/network/http/rack.rb b/spec/unit/network/http/rack.rb > new file mode 100644 > index 0000000..f639b2d > --- /dev/null > +++ b/spec/unit/network/http/rack.rb > @@ -0,0 +1,102 @@ > +#!/usr/bin/env ruby > + > +require File.dirname(__FILE__) + '/../../../spec_helper' > +require 'puppet/network/http/rack' > + > +describe "Puppet::Network::HTTP::Rack" do > + confine "Rack is not available" => Puppet.features.rack? > + > + describe "while initializing" do > + > + it "should require a protocol specification" do > + Proc.new { Puppet::Network::HTTP::Rack.new({}) }.should > raise_error(ArgumentError) > + end > + > + it "should not accept imaginary protocols" do > + Proc.new { Puppet::Network::HTTP::Rack.new({:protocols => > [:foo]}) }.should raise_error(ArgumentError) > + end > + > + it "should accept the REST protocol" do > + Proc.new { Puppet::Network::HTTP::Rack.new({:protocols => > [:rest]}) }.should_not raise_error(ArgumentError) > + end > + > + it "should create a RackREST instance" do > + Puppet::Network::HTTP::RackREST.expects(:new) > + Puppet::Network::HTTP::Rack.new({:protocols => [:rest]}) > + end > + > + describe "with XMLRPC enabled" 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 "when called" do > + > + before :all do > + @app = Puppet::Network::HTTP::Rack.new({:protocols => [:rest]}) > + # let's use Rack::Lint to verify that we're OK with the rack > specification > + @linted = Rack::Lint.new(@app) > + end > + > + before :each do > + @env = Rack::MockRequest.env_for('/') > + end > + > + it "should create a Request object" do > + request = Rack::Request.new(@env) > + Rack::Request.expects(:new).returns request > + @linted.call(@env) > + end > + > + it "should create a Response object" do > + Rack::Response.expects(:new).returns stub_everything > + @app.call(@env) # can't lint when Rack::Response is a stub > + end > + > + it "should let RackREST process the request" do > + > Puppet::Network::HTTP::RackREST.any_instance.expects(:process).once > + @linted.call(@env) > + end > + > + it "should catch unhandled exceptions from RackREST" do > + > Puppet::Network::HTTP::RackREST.any_instance.expects(:process).raises(ArgumentError, > 'test error') > + Proc.new { @linted.call(@env) }.should_not raise_error > + end > + > + it "should finish() the Response" do > + Rack::Response.any_instance.expects(:finish).once > + @app.call(@env) # can't lint when finish is a stub > + end > + > + 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/rest.rb > b/spec/unit/network/http/rack/rest.rb > new file mode 100755 > index 0000000..fc7b48f > --- /dev/null > +++ b/spec/unit/network/http/rack/rest.rb > @@ -0,0 +1,193 @@ > +#!/usr/bin/env ruby > + > +require File.dirname(__FILE__) + '/../../../../spec_helper' > +require 'puppet/network/http/rack' > +require 'puppet/network/http/rack/rest' > + > +describe "Puppet::Network::HTTP::RackREST" do > + confine "Rack is not available" => Puppet.features.rack? > + > + it "should include the Puppet::Network::HTTP::Handler module" do > + Puppet::Network::HTTP::RackREST.ancestors.should > be_include(Puppet::Network::HTTP::Handler) > + end > + > + describe "when initializing" do > + it "should call the Handler's initialization hook with its provided > arguments" do > + > Puppet::Network::HTTP::RackREST.any_instance.expects(:initialize_for_puppet).with(:server > => "my", :handler => "arguments") > + Puppet::Network::HTTP::RackREST.new(:server => "my", :handler => > "arguments") > + end > + end > + > + describe "when serving a request" do > + before :all do > + @model_class = stub('indirected model class') > + > Puppet::Indirector::Indirection.stubs(:model).with(:foo).returns(@model_class) > + @handler = Puppet::Network::HTTP::RackREST.new(:handler => :foo) > + end > + > + before :each do > + @response = Rack::Response.new() > + end > + > + def mk_req(uri, opts = {}) > + env = Rack::MockRequest.env_for(uri, opts) > + Rack::Request.new(env) > + end > + > + describe "and using the HTTP Handler interface" do > + it "should return the HTTP_ACCEPT parameter as the accept > header" do > + req = mk_req('/', 'HTTP_ACCEPT' => 'myaccept') > + @handler.accept_header(req).should == "myaccept" > + end > + > + it "should use the REQUEST_METHOD as the http method" do > + req = mk_req('/', :method => 'mymethod') > + @handler.http_method(req).should == "mymethod" > + end > + > + it "should return the request path as the path" do > + req = mk_req('/foo/bar') > + @handler.path(req).should == "/foo/bar" > + end > + > + it "should return the request body as the body" do > + req = mk_req('/foo/bar', :input => 'mybody') > + @handler.body(req).should == "mybody" > + end > + > + it "should set the response's content-type header when setting > the content type" do > + @header = mock 'header' > + @response.expects(:header).returns @header > + @header.expects(:[]=).with('Content-Type', "mytype") > + > + @handler.set_content_type(@response, "mytype") > + end > + > + it "should set the status and write the body when setting the > response for a request" do > + @response.expects(:status=).with(400) > + @response.expects(:write).with("mybody") > + > + @handler.set_response(@response, "mybody", 400) > + end > + end > + > + describe "and determining the request parameters" do > + it "should include the HTTP request parameters, with the keys as > symbols" do > + req = mk_req('/?foo=baz&bar=xyzzy') > + result = @handler.params(req) > + result[:foo].should == "baz" > + result[:bar].should == "xyzzy" > + end > + > + it "should URI-decode the HTTP parameters" do > + encoding = URI.escape("foo bar") > + req = mk_req("/?foo=#{encoding}") > + result = @handler.params(req) > + result[:foo].should == "foo bar" > + end > + > + it "should convert the string 'true' to the boolean" do > + req = mk_req("/?foo=true") > + result = @handler.params(req) > + result[:foo].should be_true > + end > + > + it "should convert the string 'false' to the boolean" do > + req = mk_req("/?foo=false") > + result = @handler.params(req) > + result[:foo].should be_false > + end > + > + it "should convert integer arguments to Integers" do > + req = mk_req("/?foo=15") > + result = @handler.params(req) > + result[:foo].should == 15 > + end > + > + it "should convert floating point arguments to Floats" do > + req = mk_req("/?foo=1.5") > + result = @handler.params(req) > + result[:foo].should == 1.5 > + end > + > + it "should YAML-load and URI-decode values that are > YAML-encoded" do > + escaping = URI.escape(YAML.dump(%w{one two})) > + req = mk_req("/?foo=#{escaping}") > + result = @handler.params(req) > + result[:foo].should == %w{one two} > + end > + > + it "should not allow the client to set the node via the query > string" do > + req = mk_req("/?node=foo") > + @handler.params(req)[:node].should be_nil > + end > + > + it "should not allow the client to set the IP address via the > query string" do > + req = mk_req("/?ip=foo") > + @handler.params(req)[:ip].should be_nil > + end > + > + it "should pass the client's ip address to model find" do > + req = mk_req("/", 'REMOTE_ADDR' => 'ipaddress') > + @handler.params(req)[:ip].should == "ipaddress" > + end > + > + it "should set 'authenticated' to false if no certificate is > present" do > + req = mk_req('/') > + @handler.params(req)[:authenticated].should be_false > + end > + 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.params(req) > + 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" > + req = mk_req('/', "myheader" => "/CN=host.domain.com") > + @handler.params(req)[:node].should == "host.domain.com" > + 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.params(req) > + 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" > + req = mk_req('/', "myheader" => "SUCCESS", "certheader" => > "/CN=host.domain.com") > + @handler.params(req)[:authenticated].should be_true > + 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" > + req = mk_req('/', "myheader" => "whatever", "certheader" => > "/CN=host.domain.com") > + @handler.params(req)[:authenticated].should be_false > + 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" > + req = mk_req('/', "myheader" => nil, "certheader" => > "/CN=host.domain.com") > + @handler.params(req)[:authenticated].should be_false > + end > + > + it "should not pass a node name to model method if no > certificate information is present" do > + Puppet.settings.stubs(:value).returns "eh" > + > Puppet.settings.expects(:value).with(:ssl_client_header).returns "myheader" > + req = mk_req('/', "myheader" => nil) > + @handler.params(req).should_not be_include(:node) > + end > + end > + end > +end -- Brice Figureau http://www.masterzen.fr/ --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
