On Apr 20, 2009, at 2:54 PM, Christian Hofstaedtler wrote:
>
> * Luke Kanies <[email protected]> [090420 05:41]:
>>
>> On Apr 19, 2009, at 1:51 PM, 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.
>>>
>>> Signed-off-by: Christian Hofstaedtler <[email protected]>
>>> ---
>>> lib/puppet/feature/base.rb | 3 +
>>> lib/puppet/network/http/rack.rb | 45 +++++++
>>> lib/puppet/network/http/rack/rest.rb | 88 ++++++++++++++
>>> spec/unit/network/http/rack.rb | 69 +++++++++++
>>> spec/unit/network/http/rack/rest.rb | 216 +++++++++++++++++++++++
>>> ++
>>> +++++++++
>>> 5 files changed, 421 insertions(+), 0 deletions(-)
>>> create mode 100644 lib/puppet/network/http/rack.rb
>>> create mode 100644 lib/puppet/network/http/rack/rest.rb
>>> create mode 100644 spec/unit/network/http/rack.rb
>>> 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..55e50c4
>>> --- /dev/null
>>> +++ b/lib/puppet/network/http/rack.rb
>>> @@ -0,0 +1,45 @@
>>> +
>>> +require 'rack'
>>> +require 'puppet/network/http'
>>> +require 'puppet/network/http/rack/rest'
>>> +
>>> +# 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
>>> +
>>> + raise ArgumentError, "there were unknown :protocols
>>> specified." if !protocols.empty?
>>
>> Shouldn't this fail if the 'rack' feature is missing?
>
> Just requiring puppet/network/http/rack will fail in this case. I
> could change this though and/or fail explicitly in initialize().
As long as the error the user gets is helpful, I don't really care;
but 'Could not find constant' or whatever is a pretty bad error,
especially if Rails is installs because it monkey-patches the constant
lookup system so we get even more useless exceptions.
>
>>>
>>> + 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
>>> + # * 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]
>>> +
>>> + begin
>>> + @rest_http_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'
>>
>> It'd be great if this at least gave the error, too.
>
> I'll look into this.
> BTW, I can't seem to successfully test the rescue case with
> rspec/mocha: the Exception doesn't get caught while testing.
+ it "should catch unhandled exceptions from RackREST" do
+ pending("check why the exception doesn't get rescue'd
properly") do
+
Puppet
::Network
::HTTP::RackREST.any_instance.expects(:process).raises(Exception,
'test error')
+ Proc.new { @linted.call(@env) }.should_not raise_error
+ end
+ end
Try raising 'RuntimeError' or ArgumentError; I think 'rescue' doesn't
catch the Exception class (or things like LoadError) by default.
>>> + # 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 }
>>> + end
>>> + response.finish()
>>> + 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..5679c41
>>> --- /dev/null
>>> +++ b/lib/puppet/network/http/rack/rest.rb
>>> @@ -0,0 +1,88 @@
>>> +require 'puppet/network/http/handler'
>>> +
>>> +class Puppet::Network::HTTP::RackREST
>>> +
>>> + 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 an SSL cert in the headers, use it to get a
>>> hostname
>>> + # (for WEBrick, or Apache with ExportCertData)
>>> + if request.env['SSL_CLIENT_CERT']
>>
>> Should this environment variable be extracted into a setting?
>>
>> I would have figured we'd already have such a setting, but I guess
>> not.
>
> SSL_CLIENT_CERT doesn't seem to be configurable right now; but see
> below.
>
>>>
>>> + cert =
>>> OpenSSL::X509::Certificate.new(request.env['SSL_CLIENT_CERT'])
>>> + nameary = cert.subject.to_a.find { |ary|
>>> + ary[0] == "CN"
>>> + }
>>> + if nameary
>>> + client = nameary[1]
>>> + # XXX: certificate validation works by finding the
>>> supposed
>>> + # cert the client should be using, and comparing
>>> that to what
>>> + # got sent. this *should* be fine, but maybe it's
>>> not?
>>> + valid =
>>> (Puppet::SSL::Certificate.find(client).to_text == cert.to_text)
>>> + end
>>
>> This actually won't work. First, this is not the cheapest process,
>> such that you probably don't want to do it on every single
>> connection,
>> but most importantly, we can't promise that the server will have
>> every
>> certificate. SSL should handle this for us, but how Rack does it, I
>> don't know. I know when Apache is used with Mongrel, we have it set
>> a couple of variables, one with the client's DN and the other with
>> the
>> authentication status. I see you're familiar with the header
>> settings; there's got to be something similar here.
>
> Rack itself doesn't (mostly) care about SSL; it just passes through
> what
> the hosting webserver gives us.
>
> If we won't support hosting Rack on top of WEBrick, we can really
> just throw away this case (SSL_CLIENT_CERT). Apache with Passenger
> and Mongrel are covered by the code below (by comparison of
> Puppet[:ssl_client_header]), as you precisley describe.
Okay, that sounds best.
>>> + # now try with :ssl_client_header, which defaults should
>>> work for
>>> + # Apache with StdEnvVars.
>>> + elsif 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/spec/unit/network/http/rack.rb b/spec/unit/network/
>>> http/
>>> rack.rb
>>> new file mode 100644
>>> index 0000000..2a575e2
>>> --- /dev/null
>>> +++ b/spec/unit/network/http/rack.rb
>>> @@ -0,0 +1,69 @@
>>> +#!/usr/bin/env ruby
>>> +
>>> +require File.dirname(__FILE__) + '/../../../spec_helper'
>>> +require 'puppet/network/http/rack'
>>> +
>>> +describe "Puppet::Network::HTTP::Rack", "while initializing" do
>>> + confine "Rack is not available" => Puppet.features.rack?
>>
>> Nice catch.
>>
>>>
>>> + it "should require a protocol specification" do
>>> + Proc.new { Puppet::Network::HTTP::Rack.new({}) }.should
>>> raise_error(ArgumentError)
>>> + end
>>> +
>>> + it "should only accept the REST protocol" 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
>>> +
>>> +end
>>> +
>>> +describe "Puppet::Network::HTTP::Rack", "when called" do
>>> + confine "Rack is not available" => Puppet.features.rack?
>>
>> If you nest all of your contexts, then a single confine in the outer
>> context will suffice.
>
> Done.
>
>>> [...]
>
> Christian
>
>
> >
--
In science, 'fact' can only mean 'confirmed to such a degree that it
would be perverse to withhold provisional assent.' I suppose that
apples might start to rise tomorrow, but the possibility does not
merit equal time in physics classrooms. -- Stephen Jay Gould
---------------------------------------------------------------------
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
-~----------~----~----~----~------~----~------~--~---
