Hi Brice,
* Brice Figureau <[email protected]> [090428 22:38]:
> 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 :-)
Fixed.
>> 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?
Inded, with passenger this ends up in Apaches error.log, which I
found convenient for finding errors.
>> + 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.
Okay; I've added the resolve_node call here.
> Note that this code seems to be a copy of the one you have in
> Puppet::Network::HTTP::RackHttpHandler.
This was an rebase -i going wrong. The next patch will drop the code
from RackHttpHandler and have specific versions for REST and XMLRPC.
>> + 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
>
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---
