[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, upstream, updated. 2.6.5rc1-120-g2247c80
Paul Berry
paul at puppetlabs.com
Mon Feb 7 06:40:16 UTC 2011
The following commit has been merged in the upstream branch:
commit c514c641d0c0090be29252dcc773385248d3fe93
Author: Paul Berry <paul at puppetlabs.com>
Date: Mon Jan 10 17:05:38 2011 -0800
(#5838) Added support for HEAD requests to the indirector.
Added the ability for the indirector to handle REST HEAD requests.
These are done using a new indirector method, head(), which should
return true if find() would return a result and false if find() would
return nil.
Access control for the head method is the union of that for the find
and save methods. That is, if either find or save is allowed, then
head is allowed. This is necessary so that users will not have to
change their authconfig to take advantage of the new feature.
Paired-with: Jesse Wolfe <jesse at puppetlabs.com>
diff --git a/lib/puppet/indirector.rb b/lib/puppet/indirector.rb
index 5b73757..e6472f4 100644
--- a/lib/puppet/indirector.rb
+++ b/lib/puppet/indirector.rb
@@ -50,6 +50,10 @@ module Puppet::Indirector
indirection.find(*args)
end
+ def head(*args)
+ indirection.head(*args)
+ end
+
def destroy(*args)
indirection.destroy(*args)
end
diff --git a/lib/puppet/indirector/indirection.rb b/lib/puppet/indirector/indirection.rb
index 4341f7c..ec147ec 100644
--- a/lib/puppet/indirector/indirection.rb
+++ b/lib/puppet/indirector/indirection.rb
@@ -198,6 +198,17 @@ class Puppet::Indirector::Indirection
nil
end
+ # Search for an instance in the appropriate terminus, and return a
+ # boolean indicating whether the instance was found.
+ def head(key, *args)
+ request = request(:head, key, *args)
+ terminus = prepare(request)
+
+ # Look in the cache first, then in the terminus. Force the result
+ # to be a boolean.
+ !!(find_in_cache(request) || terminus.head(request))
+ end
+
def find_in_cache(request)
# See if our instance is in the cache and up to date.
return nil unless cache? and ! request.ignore_cache? and cached = cache.find(request)
diff --git a/lib/puppet/indirector/rest.rb b/lib/puppet/indirector/rest.rb
index eb41ff3..e50dc68 100644
--- a/lib/puppet/indirector/rest.rb
+++ b/lib/puppet/indirector/rest.rb
@@ -53,11 +53,15 @@ class Puppet::Indirector::REST < Puppet::Indirector::Terminus
end
else
# Raise the http error if we didn't get a 'success' of some kind.
- message = "Error #{response.code} on SERVER: #{(response.body||'').empty? ? response.message : uncompress_body(response)}"
- raise Net::HTTPError.new(message, response)
+ raise convert_to_http_error(response)
end
end
+ def convert_to_http_error(response)
+ message = "Error #{response.code} on SERVER: #{(response.body||'').empty? ? response.message : uncompress_body(response)}"
+ Net::HTTPError.new(message, response)
+ end
+
# Provide appropriate headers.
def headers
add_accept_encoding({"Accept" => model.supported_formats.join(", ")})
@@ -73,6 +77,19 @@ class Puppet::Indirector::REST < Puppet::Indirector::Terminus
result
end
+ def head(request)
+ response = network(request).head(indirection2uri(request), headers)
+ case response.code
+ when "404"
+ return false
+ when /^2/
+ return true
+ else
+ # Raise the http error if we didn't get a 'success' of some kind.
+ raise convert_to_http_error(response)
+ end
+ end
+
def search(request)
unless result = deserialize(network(request).get(indirection2uri(request), headers), true)
return []
diff --git a/lib/puppet/network/http/api/v1.rb b/lib/puppet/network/http/api/v1.rb
index dd4612a..8aa1f0e 100644
--- a/lib/puppet/network/http/api/v1.rb
+++ b/lib/puppet/network/http/api/v1.rb
@@ -13,6 +13,9 @@ module Puppet::Network::HTTP::API::V1
},
"DELETE" => {
:singular => :destroy
+ },
+ "HEAD" => {
+ :singular => :head
}
}
diff --git a/lib/puppet/network/http/handler.rb b/lib/puppet/network/http/handler.rb
index f22498b..9e9356b 100644
--- a/lib/puppet/network/http/handler.rb
+++ b/lib/puppet/network/http/handler.rb
@@ -116,6 +116,17 @@ module Puppet::Network::HTTP::Handler
end
end
+ # Execute our head.
+ def do_head(indirection_request, request, response)
+ unless indirection_request.model.head(indirection_request.key, indirection_request.to_hash)
+ Puppet.info("Could not find #{indirection_request.indirection_name} for '#{indirection_request.key}'")
+ return do_exception(response, "Could not find #{indirection_request.indirection_name} #{indirection_request.key}", 404)
+ end
+
+ # No need to set a response because no response is expected from a
+ # HEAD request. All we need to do is not die.
+ end
+
# Execute our search.
def do_search(indirection_request, request, response)
result = indirection_request.model.search(indirection_request.key, indirection_request.to_hash)
diff --git a/lib/puppet/network/rest_authconfig.rb b/lib/puppet/network/rest_authconfig.rb
index 1704ea0..7a6147a 100644
--- a/lib/puppet/network/rest_authconfig.rb
+++ b/lib/puppet/network/rest_authconfig.rb
@@ -38,13 +38,7 @@ module Puppet
# fail_on_deny could as well be called in the XMLRPC context
# with a ClientRequest.
- if authorization_failure_exception = @rights.is_forbidden_and_why?(
- build_uri(request),
- :node => request.node,
- :ip => request.ip,
- :method => request.method,
- :environment => request.environment,
- :authenticated => request.authenticated)
+ if authorization_failure_exception = @rights.is_request_forbidden_and_why?(request)
Puppet.warning("Denying access: #{authorization_failure_exception}")
raise authorization_failure_exception
end
@@ -90,9 +84,5 @@ module Puppet
end
@rights.restrict_authenticated(acl[:acl], acl[:authenticated]) unless acl[:authenticated].nil?
end
-
- def build_uri(request)
- "/#{request.indirection_name}/#{request.key}"
- end
end
end
diff --git a/lib/puppet/network/rights.rb b/lib/puppet/network/rights.rb
index b214649..00ee04f 100755
--- a/lib/puppet/network/rights.rb
+++ b/lib/puppet/network/rights.rb
@@ -29,6 +29,30 @@ class Rights
!is_forbidden_and_why?(name, :node => args[0], :ip => args[1])
end
+ def is_request_forbidden_and_why?(request)
+ methods_to_check = if request.method == :head
+ # :head is ok if either :find or :save is ok.
+ [:find, :save]
+ else
+ [request.method]
+ end
+ authorization_failure_exceptions = methods_to_check.map do |method|
+ is_forbidden_and_why?("/#{request.indirection_name}/#{request.key}",
+ :node => request.node,
+ :ip => request.ip,
+ :method => method,
+ :environment => request.environment,
+ :authenticated => request.authenticated)
+ end
+ if authorization_failure_exceptions.include? nil
+ # One of the methods we checked is ok, therefore this request is ok.
+ nil
+ else
+ # Just need to return any of the failure exceptions.
+ authorization_failure_exceptions.first
+ end
+ end
+
def is_forbidden_and_why?(name, args = {})
res = :nomatch
right = @rights.find do |acl|
diff --git a/spec/unit/indirector/file_server_spec.rb b/spec/unit/indirector/file_server_spec.rb
index 686f79a..eafcf2b 100755
--- a/spec/unit/indirector/file_server_spec.rb
+++ b/spec/unit/indirector/file_server_spec.rb
@@ -229,6 +229,12 @@ describe Puppet::Indirector::FileServer do
describe "when checking authorization" do
before do
@request.method = :find
+
+ @mount = stub 'mount'
+ @configuration.stubs(:split_path).with(@request).returns([@mount, "rel/path"])
+ @request.stubs(:node).returns("mynode")
+ @request.stubs(:ip).returns("myip")
+ @mount.stubs(:allowed?).with("mynode", "myip").returns "something"
end
it "should return false when destroying" do
@@ -254,13 +260,6 @@ describe Puppet::Indirector::FileServer do
end
it "should return the results of asking the mount whether the node and IP are authorized" do
- @mount = stub 'mount'
- @configuration.expects(:split_path).with(@request).returns([@mount, "rel/path"])
-
- @request.stubs(:node).returns("mynode")
- @request.stubs(:ip).returns("myip")
- @mount.expects(:allowed?).with("mynode", "myip").returns "something"
-
@file_server.authorized?(@request).should == "something"
end
end
diff --git a/spec/unit/indirector/indirection_spec.rb b/spec/unit/indirector/indirection_spec.rb
index 1e774fb..a910cb6 100755
--- a/spec/unit/indirector/indirection_spec.rb
+++ b/spec/unit/indirector/indirection_spec.rb
@@ -383,6 +383,75 @@ describe Puppet::Indirector::Indirection do
end
end
+ describe "and doing a head operation" do
+ before { @method = :head }
+
+ it_should_behave_like "Indirection Delegator"
+ it_should_behave_like "Delegation Authorizer"
+
+ it "should return true if the head method returned true" do
+ @terminus.expects(:head).returns(true)
+ @indirection.head("me").should == true
+ end
+
+ it "should return false if the head method returned false" do
+ @terminus.expects(:head).returns(false)
+ @indirection.head("me").should == false
+ end
+
+ describe "when caching is enabled" do
+ before do
+ @indirection.cache_class = :cache_terminus
+ @cache_class.stubs(:new).returns(@cache)
+
+ @instance.stubs(:expired?).returns false
+ end
+
+ it "should first look in the cache for an instance" do
+ @terminus.stubs(:find).never
+ @terminus.stubs(:head).never
+ @cache.expects(:find).returns @instance
+
+ @indirection.head("/my/key").should == true
+ end
+
+ it "should not save to the cache" do
+ @cache.expects(:find).returns nil
+ @cache.expects(:save).never
+ @terminus.expects(:head).returns true
+ @indirection.head("/my/key").should == true
+ end
+
+ it "should not fail if the cache fails" do
+ @terminus.stubs(:head).returns true
+
+ @cache.expects(:find).raises ArgumentError
+ lambda { @indirection.head("/my/key") }.should_not raise_error
+ end
+
+ it "should look in the main terminus if the cache fails" do
+ @terminus.expects(:head).returns true
+ @cache.expects(:find).raises ArgumentError
+ @indirection.head("/my/key").should == true
+ end
+
+ it "should send a debug log if it is using the cached object" do
+ Puppet.expects(:debug)
+ @cache.stubs(:find).returns @instance
+
+ @indirection.head("/my/key")
+ end
+
+ it "should not accept the cached object if it is expired" do
+ @instance.stubs(:expired?).returns true
+
+ @cache.stubs(:find).returns @instance
+ @terminus.stubs(:head).returns false
+ @indirection.head("/my/key").should == false
+ end
+ end
+ end
+
describe "and storing a model instance" do
before { @method = :save }
diff --git a/spec/unit/indirector/rest_spec.rb b/spec/unit/indirector/rest_spec.rb
index 5f0fe36..7bc1cc5 100755
--- a/spec/unit/indirector/rest_spec.rb
+++ b/spec/unit/indirector/rest_spec.rb
@@ -282,6 +282,42 @@ describe Puppet::Indirector::REST do
end
end
+ describe "when doing a head" do
+ before :each do
+ @connection = stub('mock http connection', :head => @response)
+ @searcher.stubs(:network).returns(@connection)
+
+ # Use a key with spaces, so we can test escaping
+ @request = Puppet::Indirector::Request.new(:foo, :head, "foo bar")
+ end
+
+ it "should call the HEAD http method on a network connection" do
+ @searcher.expects(:network).returns @connection
+ @connection.expects(:head).returns @response
+ @searcher.head(@request)
+ end
+
+ it "should return true if there was a successful http response" do
+ @connection.expects(:head).returns @response
+ @response.stubs(:code).returns "200"
+
+ @searcher.head(@request).should == true
+ end
+
+ it "should return false if there was a successful http response" do
+ @connection.expects(:head).returns @response
+ @response.stubs(:code).returns "404"
+
+ @searcher.head(@request).should == false
+ end
+
+ it "should use the URI generated by the Handler module" do
+ @searcher.expects(:indirection2uri).with(@request).returns "/my/uri"
+ @connection.expects(:head).with { |path, args| path == "/my/uri" }.returns(@response)
+ @searcher.head(@request)
+ end
+ end
+
describe "when doing a search" do
before :each do
@connection = stub('mock http connection', :get => @response)
diff --git a/spec/unit/indirector_spec.rb b/spec/unit/indirector_spec.rb
index acbfc17..6f44764 100755
--- a/spec/unit/indirector_spec.rb
+++ b/spec/unit/indirector_spec.rb
@@ -118,6 +118,11 @@ describe Puppet::Indirector, "when redirecting a model" do
it_should_behave_like "Delegated Indirection Method"
end
+ describe "when performing a head request" do
+ before { @method = :head }
+ it_should_behave_like "Delegated Indirection Method"
+ end
+
# This is an instance method, so it behaves a bit differently.
describe "when saving instances via the model" do
before do
diff --git a/spec/unit/network/http/api/v1_spec.rb b/spec/unit/network/http/api/v1_spec.rb
index c593242..23a291c 100644
--- a/spec/unit/network/http/api/v1_spec.rb
+++ b/spec/unit/network/http/api/v1_spec.rb
@@ -68,6 +68,10 @@ describe Puppet::Network::HTTP::API::V1 do
@tester.uri2indirection("GET", "/env/foo/bar", {}).method.should == :find
end
+ it "should choose 'head' as the indirection method if the http method is a HEAD and the indirection name is singular" do
+ @tester.uri2indirection("HEAD", "/env/foo/bar", {}).method.should == :head
+ end
+
it "should choose 'search' as the indirection method if the http method is a GET and the indirection name is plural" do
@tester.uri2indirection("GET", "/env/foos/bar", {}).method.should == :search
end
diff --git a/spec/unit/network/http/handler_spec.rb b/spec/unit/network/http/handler_spec.rb
index cdbce41..8464ae6 100755
--- a/spec/unit/network/http/handler_spec.rb
+++ b/spec/unit/network/http/handler_spec.rb
@@ -256,6 +256,39 @@ describe Puppet::Network::HTTP::Handler do
end
end
+ describe "when performing head operation" do
+ before do
+ @irequest = stub 'indirection_request', :method => :head, :indirection_name => "my_handler", :to_hash => {}, :key => "my_result", :model => @model_class
+
+ @model_class.stubs(:head).returns true
+ end
+
+ it "should use the indirection request to find the model class" do
+ @irequest.expects(:model).returns @model_class
+
+ @handler.do_head(@irequest, @request, @response)
+ end
+
+ it "should use the escaped request key" do
+ @model_class.expects(:head).with do |key, args|
+ key == "my_result"
+ end.returns true
+ @handler.do_head(@irequest, @request, @response)
+ end
+
+ it "should not generate a response when a model head call succeeds" do
+ @handler.expects(:set_response).never
+ @handler.do_head(@irequest, @request, @response)
+ end
+
+ it "should return a 404 when the model head call returns false" do
+ @model_class.stubs(:name).returns "my name"
+ @handler.expects(:set_response).with { |response, body, status| status == 404 }
+ @model_class.stubs(:head).returns(false)
+ @handler.do_head(@irequest, @request, @response)
+ end
+ end
+
describe "when searching for model instances" do
before do
@irequest = stub 'indirection_request', :method => :find, :indirection_name => "my_handler", :to_hash => {}, :key => "key", :model => @model_class
diff --git a/spec/unit/network/rest_authconfig_spec.rb b/spec/unit/network/rest_authconfig_spec.rb
index 9892c2b..d629f86 100755
--- a/spec/unit/network/rest_authconfig_spec.rb
+++ b/spec/unit/network/rest_authconfig_spec.rb
@@ -47,7 +47,7 @@ describe Puppet::Network::RestAuthConfig do
end
it "should ask for authorization to the ACL subsystem" do
- @acl.expects(:is_forbidden_and_why?).with("/path/to/resource", :node => "me", :ip => "127.0.0.1", :method => :save, :environment => :env, :authenticated => true).returns(nil)
+ @acl.expects(:is_request_forbidden_and_why?).with(@request).returns(nil)
@authconfig.allowed?(@request)
end
diff --git a/spec/unit/network/rights_spec.rb b/spec/unit/network/rights_spec.rb
index ca3f224..3b9e483 100755
--- a/spec/unit/network/rights_spec.rb
+++ b/spec/unit/network/rights_spec.rb
@@ -9,6 +9,26 @@ describe Puppet::Network::Rights do
@right = Puppet::Network::Rights.new
end
+ describe "when validating a :head request" do
+ [:find, :save].each do |allowed_method|
+ it "should allow the request if only #{allowed_method} is allowed" do
+ rights = Puppet::Network::Rights.new
+ rights.newright("/")
+ rights.allow("/", "*")
+ rights.restrict_method("/", allowed_method)
+ rights.restrict_authenticated("/", :any)
+ request = Puppet::Indirector::Request.new(:indirection_name, :head, "key")
+ rights.is_request_forbidden_and_why?(request).should == nil
+ end
+ end
+
+ it "should disallow the request if neither :find nor :save is allowed" do
+ rights = Puppet::Network::Rights.new
+ request = Puppet::Indirector::Request.new(:indirection_name, :head, "key")
+ rights.is_request_forbidden_and_why?(request).should be_instance_of(Puppet::Network::AuthorizationError)
+ end
+ end
+
[:allow, :deny, :restrict_method, :restrict_environment, :restrict_authenticated].each do |m|
it "should have a #{m} method" do
@right.should respond_to(m)
--
Puppet packaging for Debian
More information about the Pkg-puppet-devel
mailing list