Merge branch 'ticket/2.6.next/5838' into 2.6.next

* ticket/2.6.next/5838:
  (#5838) Support paths as part of file bucket requests.
  (#5838) Improve the quality of file bucket specs.
  (#5838) Make file bucket dipper efficient when saving a file that already exists
  (#5838) Implemented the "head" method for FileBucketFile::File terminus.
  (#5838) Reworked file dipper spec to perform less stubbing.
  (#5838) Added support for HEAD requests to the indirector.
  (#5838) Refactored error handling logic into find_in_cache.
  (#5838) Refactored Puppet::Network::Rights#fail_on_deny

21 files changed, 505 insertions, 650 deletions

diff --git a/lib/puppet/file_bucket/dipper.rb b/lib/puppet/file_bucket/dipper.rb
index dbfcdcd43..f4bef28a8 100644
--- a/lib/puppet/file_bucket/dipper.rb
+++ b/lib/puppet/file_bucket/dipper.rb
@@ -33,10 +33,15 @@ class Puppet::FileBucket::Dipper
     raise(ArgumentError, "File #{file} does not exist") unless ::File.exist?(file)
     contents = ::File.read(file)
     begin
-      file_bucket_file = Puppet::FileBucket::File.new(contents, :bucket_path => @local_path, :path => absolutize_path(file) )
+      file_bucket_file = Puppet::FileBucket::File.new(contents, :bucket_path => @local_path)
       dest_path = "#{@rest_path}#{file_bucket_file.name}"
 
-      file_bucket_file.save(dest_path)
+      # Make a HEAD request for the file so that we don't waste time
+      # uploading it if it already exists in the bucket.
+      unless Puppet::FileBucket::File.head("#{@rest_path}#{file_bucket_file.checksum_type}/#{file_bucket_file.checksum_data}")
+        file_bucket_file.save(dest_path)
+      end
+
       return file_bucket_file.checksum_data
     rescue => detail
       puts detail.backtrace if Puppet[:trace]
diff --git a/lib/puppet/file_bucket/file.rb b/lib/puppet/file_bucket/file.rb
index 96fd8e225..08c0329f1 100644
--- a/lib/puppet/file_bucket/file.rb
+++ b/lib/puppet/file_bucket/file.rb
@@ -1,10 +1,9 @@
 require 'puppet/file_bucket'
 require 'puppet/indirector'
 require 'puppet/util/checksums'
+require 'digest/md5'
 
 class Puppet::FileBucket::File
-  include Puppet::Util::Checksums
-
   # This class handles the abstract notion of a file in a filebucket.
   # There are mechanisms to save and load this file locally and remotely in puppet/indirector/filebucketfile/*
   # There is a compatibility class that emulates pre-indirector filebuckets in Puppet::FileBucket::Dipper
@@ -12,71 +11,27 @@ class Puppet::FileBucket::File
   require 'puppet/file_bucket/file/indirection_hooks'
   indirects :file_bucket_file, :terminus_class => :file, :extend => Puppet::FileBucket::File::IndirectionHooks
 
-  attr :path, true
-  attr :paths, true
-  attr :contents, true
-  attr :checksum_type
-  attr :bucket_path, true
-
-  def self.default_checksum_type
-    "md5"
-  end
+  attr :contents
+  attr :bucket_path
 
   def initialize( contents, options = {} )
-    @bucket_path   = options[:bucket_path]
-    @path          = options[:path]
-    @paths         = options[:paths] || []
-
-    @checksum      = options[:checksum]
-    @checksum_type = options[:checksum_type]
-
-    self.contents  = contents
-
-    yield(self) if block_given?
-
-    validate!
-  end
+    raise ArgumentError if !contents.is_a?(String)
+    @contents  = contents
 
-  def validate!
-    validate_checksum_type!(checksum_type)
-    validate_checksum!(checksum) if checksum
+    @bucket_path = options.delete(:bucket_path)
+    raise ArgumentError if options != {}
   end
 
-  def contents=(str)
-    raise "You may not change the contents of a FileBucket File" if @contents
-    validate_content!(str)
-    @contents = str
+  def checksum_type
+    'md5'
   end
 
   def checksum
-    return @checksum if @checksum
-    @checksum = calculate_checksum if contents
-    @checksum
-  end
-
-  def checksum=(checksum)
-    validate_checksum!(checksum)
-    @checksum = checksum
-  end
-
-  def checksum_type=( new_checksum_type )
-    @checksum = nil
-    @checksum_type = new_checksum_type
-  end
-
-  def checksum_type
-    unless @checksum_type
-      if @checksum
-        @checksum_type = sumtype(checksum)
-      else
-        @checksum_type = self.class.default_checksum_type
-      end
-    end
-    @checksum_type
+    "{#{checksum_type}}#{checksum_data}"
   end
 
   def checksum_data
-    sumdata(checksum)
+    @checksum_data ||= Digest::MD5.hexdigest(contents)
   end
 
   def to_s
@@ -84,18 +39,7 @@ class Puppet::FileBucket::File
   end
 
   def name
-    [checksum_type, checksum_data, path].compact.join('/')
-  end
-
-  def name=(name)
-    data = name.split('/',3)
-    self.path = data.pop
-    @checksum_type = nil
-    self.checksum = "{#{data[0]}}#{data[1]}"
-  end
-
-  def conflict_check?
-    true
+    "#{checksum_type}/#{checksum_data}"
   end
 
   def self.from_s( contents )
@@ -103,34 +47,10 @@ class Puppet::FileBucket::File
   end
 
   def to_pson
-    hash = { "contents" => contents }
-    hash["path"] = @path if @path
-    hash.to_pson
+    { "contents" => contents }.to_pson
   end
 
   def self.from_pson( pson )
-    self.new( pson["contents"], :path => pson["path"] )
-  end
-
-  private
-
-  def calculate_checksum
-    "{#{checksum_type}}" + send(checksum_type, contents)
-  end
-
-  def validate_content!(content)
-    raise ArgumentError, "Contents must be a string" if content and ! content.is_a?(String)
-  end
-
-  def validate_checksum!(new_checksum)
-    newtype = sumtype(new_checksum)
-
-    unless sumdata(new_checksum) == (calc_sum = send(newtype, contents))
-      raise Puppet::Error, "Checksum #{new_checksum} does not match contents #{calc_sum}"
-    end
-  end
-
-  def validate_checksum_type!(type)
-    raise ArgumentError, "Invalid checksum type #{type}" unless respond_to?(type)
+    self.new( pson["contents"] )
   end
 end
diff --git a/lib/puppet/indirector.rb b/lib/puppet/indirector.rb
index 5b737578b..e6472f4d9 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/file_bucket_file/file.rb b/lib/puppet/indirector/file_bucket_file/file.rb
index 9d9cee793..8bea2d767 100644
--- a/lib/puppet/indirector/file_bucket_file/file.rb
+++ b/lib/puppet/indirector/file_bucket_file/file.rb
@@ -14,25 +14,31 @@ module Puppet::FileBucketFile
     end
 
     def find( request )
-      checksum, path = request_to_checksum_and_path( request )
-      file = find_by_checksum( checksum, request.options )
+      checksum = request_to_checksum( request )
+      file_path = path_for(request.options[:bucket_path], checksum, 'contents')
 
-      if file && request.options[:diff_with]
+      return nil unless ::File.exists?(file_path)
+
+      if request.options[:diff_with]
         hash_protocol = sumtype(checksum)
-        file2 = find_by_checksum( "{#{hash_protocol}}#{request.options[:diff_with]}", request.options )
-        raise "could not find diff_with #{request.options[:diff_with]}" unless file2
-        return `diff #{path_for(file).inspect}/contents #{path_for(file2).inspect}/contents`
+        file2_path = path_for(request.options[:bucket_path], request.options[:diff_with], 'contents')
+        raise "could not find diff_with #{request.options[:diff_with]}" unless ::File.exists?(file2_path)
+        return `diff #{file_path.inspect} #{file2_path.inspect}`
+      else
+        contents = ::File.read file_path
+        Puppet.info "FileBucket read #{checksum}"
+        model.new(contents)
       end
+    end
 
-      file
+    def head(request)
+      checksum = request_to_checksum(request)
+      file_path = path_for(request.options[:bucket_path], checksum, 'contents')
+      ::File.exists?(file_path)
     end
 
     def save( request )
-      checksum, path = request_to_checksum_and_path( request )
-
       instance = request.instance
-      instance.checksum = checksum if checksum
-      instance.path = path if path
 
       save_to_disk(instance)
       instance.to_s
@@ -40,66 +46,41 @@ module Puppet::FileBucketFile
 
     private
 
-    def find_by_checksum( checksum, options )
-      model.new( nil, :checksum => checksum ) do |bucket_file|
-        bucket_file.bucket_path = options[:bucket_path]
-        filename = contents_path_for( bucket_file )
-
-        return nil if ! ::File.exist? filename
-
-        begin
-          contents = ::File.read filename
-          Puppet.info "FileBucket read #{bucket_file.checksum}"
-        rescue RuntimeError => e
-          raise Puppet::Error, "file could not be read: #{e.message}"
-        end
-
-        if ::File.exist?(paths_path_for( bucket_file) )
-          ::File.open(paths_path_for( bucket_file) ) do |f|
-            bucket_file.paths = f.readlines.map { |l| l.chomp }
-          end
-        end
-
-        bucket_file.contents = contents
-      end
-    end
-
     def save_to_disk( bucket_file )
-      # If the file already exists, just return the md5 sum.
-      if ::File.exist?(contents_path_for( bucket_file) )
+      filename = path_for(bucket_file.bucket_path, bucket_file.checksum_data, 'contents')
+      dirname = path_for(bucket_file.bucket_path, bucket_file.checksum_data)
+
+      # If the file already exists, do nothing.
+      if ::File.exist?(filename)
         verify_identical_file!(bucket_file)
       else
         # Make the directories if necessary.
-        unless ::File.directory?( path_for( bucket_file) )
+        unless ::File.directory?(dirname)
           Puppet::Util.withumask(0007) do
-            ::FileUtils.mkdir_p( path_for( bucket_file) )
+            ::FileUtils.mkdir_p(dirname)
           end
         end
 
-        Puppet.info "FileBucket adding #{bucket_file.path} as #{bucket_file.checksum}"
+        Puppet.info "FileBucket adding #{bucket_file.checksum}"
 
         # Write the file to disk.
         Puppet::Util.withumask(0007) do
-          ::File.open(contents_path_for(bucket_file), ::File::WRONLY|::File::CREAT, 0440) do |of|
+          ::File.open(filename, ::File::WRONLY|::File::CREAT, 0440) do |of|
             of.print bucket_file.contents
           end
         end
       end
-
-      save_path_to_paths_file(bucket_file)
-      bucket_file.checksum_data
     end
 
-    def request_to_checksum_and_path( request )
-      return [request.key, nil] if checksum?(request.key)
-
-      checksum_type, checksum, path = request.key.split(/\//, 3)
-      return(checksum_type.to_s == "" ? nil : [ "{#{checksum_type}}#{checksum}", path ])
+    def request_to_checksum( request )
+      checksum_type, checksum, path = request.key.split(/\//, 3) # Note: we ignore path if present.
+      raise "Unsupported checksum type #{checksum_type.inspect}" if checksum_type != 'md5'
+      raise "Invalid checksum #{checksum.inspect}" if checksum !~ /^[0-9a-f]{32}$/
+      checksum
     end
 
-    def path_for(bucket_file, subfile = nil)
-      bucket_path = bucket_file.bucket_path || Puppet[:bucketdir]
-      digest      = bucket_file.checksum_data
+    def path_for(bucket_path, digest, subfile = nil)
+      bucket_path ||= Puppet[:bucketdir]
 
       dir     = ::File.join(digest[0..7].split(""))
       basedir = ::File.join(bucket_path, dir, digest)
@@ -108,48 +89,18 @@ module Puppet::FileBucketFile
       ::File.join(basedir, subfile)
     end
 
-    def contents_path_for(bucket_file)
-      path_for(bucket_file, "contents")
-    end
-
-    def paths_path_for(bucket_file)
-      path_for(bucket_file, "paths")
-    end
-
-    def content_check?
-      true
-    end
-
     # If conflict_check is enabled, verify that the passed text is
     # the same as the text in our file.
     def verify_identical_file!(bucket_file)
-      return unless content_check?
-      disk_contents = ::File.read(contents_path_for(bucket_file))
+      disk_contents = ::File.read(path_for(bucket_file.bucket_path, bucket_file.checksum_data, 'contents'))
 
       # If the contents don't match, then we've found a conflict.
       # Unlikely, but quite bad.
       if disk_contents != bucket_file.contents
-        raise Puppet::FileBucket::BucketError, "Got passed new contents for sum #{bucket_file.checksum}", caller
+        raise Puppet::FileBucket::BucketError, "Got passed new contents for sum #{bucket_file.checksum}"
       else
-        Puppet.info "FileBucket got a duplicate file #{bucket_file.path} (#{bucket_file.checksum})"
+        Puppet.info "FileBucket got a duplicate file #{bucket_file.checksum}"
       end
     end
-
-    def save_path_to_paths_file(bucket_file)
-      return unless bucket_file.path
-
-      # check for dupes
-      if ::File.exist?(paths_path_for( bucket_file) )
-        ::File.open(paths_path_for( bucket_file) ) do |f|
-          return if f.readlines.collect { |l| l.chomp }.include?(bucket_file.path)
-        end
-      end
-
-      # if it's a new file, or if our path isn't in the file yet, add it
-      ::File.open(paths_path_for(bucket_file), ::File::WRONLY|::File::CREAT|::File::APPEND) do |of|
-        of.puts bucket_file.path
-      end
-    end
-
   end
 end
diff --git a/lib/puppet/indirector/indirection.rb b/lib/puppet/indirector/indirection.rb
index a010c4e40..ec147ec69 100644
--- a/lib/puppet/indirector/indirection.rb
+++ b/lib/puppet/indirector/indirection.rb
@@ -180,13 +180,8 @@ class Puppet::Indirector::Indirection
     request = request(:find, key, *args)
     terminus = prepare(request)
 
-    begin
-      if result = find_in_cache(request)
-        return result
-      end
-    rescue => detail
-      puts detail.backtrace if Puppet[:trace]
-      Puppet.err "Cached #{self.name} for #{request.key} failed: #{detail}"
+    if result = find_in_cache(request)
+      return result
     end
 
     # Otherwise, return the result from the terminus, caching if appropriate.
@@ -203,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)
@@ -213,6 +219,10 @@ class Puppet::Indirector::Indirection
 
     Puppet.debug "Using cached #{self.name} for #{request.key}"
     cached
+  rescue => detail
+    puts detail.backtrace if Puppet[:trace]
+    Puppet.err "Cached #{self.name} for #{request.key} failed: #{detail}"
+    nil
   end
 
   # Remove something via the terminus.
diff --git a/lib/puppet/indirector/rest.rb b/lib/puppet/indirector/rest.rb
index eb41ff3b1..e50dc68ae 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 dd4612a14..8aa1f0ee1 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 f22498b70..9e9356b2f 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 7abe06956..7a6147a82 100644
--- a/lib/puppet/network/rest_authconfig.rb
+++ b/lib/puppet/network/rest_authconfig.rb
@@ -38,14 +38,10 @@ module Puppet
       # fail_on_deny could as well be called in the XMLRPC context
       # with a ClientRequest.
 
-            @rights.fail_on_deny(
-        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
     end
 
     def initialize(file = nil, parsenow = true)
@@ -88,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 e3cd3179a..00ee04f8d 100755
--- a/lib/puppet/network/rights.rb
+++ b/lib/puppet/network/rights.rb
@@ -26,19 +26,34 @@ class Rights
 
   # Check that name is allowed or not
   def allowed?(name, *args)
-    begin
-      fail_on_deny(name, :node => args[0], :ip => args[1])
-    rescue AuthorizationError
-      return false
-    rescue ArgumentError
-      # the namespace contract says we should raise this error
-      # if we didn't find the right acl
-      raise
+    !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
-    true
   end
 
-  def fail_on_deny(name, args = {})
+  def is_forbidden_and_why?(name, args = {})
     res = :nomatch
     right = @rights.find do |acl|
       found = false
@@ -49,7 +64,7 @@ class Rights
         args[:match] = match
         if (res = acl.allowed?(args[:node], args[:ip], args)) != :dunno
           # return early if we're allowed
-          return if res
+          return nil if res
           # we matched, select this acl
           found = true
         end
@@ -70,13 +85,12 @@ class Rights
         error.file = right.file
         error.line = right.line
       end
-      Puppet.warning("Denying access: #{error}")
     else
       # there were no rights allowing/denying name
       # if name is not a path, let's throw
-      error = ArgumentError.new "Unknown namespace right '#{name}'"
+      raise ArgumentError.new "Unknown namespace right '#{name}'"
     end
-    raise error
+    error
   end
 
   def initialize
diff --git a/spec/unit/file_bucket/dipper_spec.rb b/spec/unit/file_bucket/dipper_spec.rb
index 799e899e7..db40c6296 100755
--- a/spec/unit/file_bucket/dipper_spec.rb
+++ b/spec/unit/file_bucket/dipper_spec.rb
@@ -2,121 +2,113 @@
 
 require File.dirname(__FILE__) + '/../../spec_helper'
 
+require 'pathname'
+
 require 'puppet/file_bucket/dipper'
+require 'puppet/indirector/file_bucket_file/rest'
+
 describe Puppet::FileBucket::Dipper do
-  before do
-    ['/my/file'].each do |x|
-      Puppet::FileBucket::Dipper.any_instance.stubs(:absolutize_path).with(x).returns(x)
-    end
+  include PuppetSpec::Files
+
+  def make_tmp_file(contents)
+    file = tmpfile("file_bucket_file")
+    File.open(file, 'w') { |f| f.write(contents) }
+    file
   end
 
-  it "should fail in an informative way when there are failures backing up to the server" do
-    File.stubs(:exist?).returns true
-    File.stubs(:read).returns "content"
+  it "should fail in an informative way when there are failures checking for the file on the server" do
+    @dipper = Puppet::FileBucket::Dipper.new(:Path => "/my/bucket")
+
+    file = make_tmp_file('contents')
+    Puppet::FileBucket::File.expects(:head).raises ArgumentError
 
+    lambda { @dipper.backup(file) }.should raise_error(Puppet::Error)
+  end
+
+  it "should fail in an informative way when there are failures backing up to the server" do
     @dipper = Puppet::FileBucket::Dipper.new(:Path => "/my/bucket")
 
-    filemock = stub "bucketfile"
-    Puppet::FileBucket::File.stubs(:new).returns(filemock)
-    filemock.expects(:name).returns "name"
-    filemock.expects(:save).raises ArgumentError
+    file = make_tmp_file('contents')
+    Puppet::FileBucket::File.expects(:head).returns false
+    Puppet::FileBucket::File.any_instance.expects(:save).raises ArgumentError
 
-    lambda { @dipper.backup("/my/file") }.should raise_error(Puppet::Error)
+    lambda { @dipper.backup(file) }.should raise_error(Puppet::Error)
   end
 
   it "should backup files to a local bucket" do
-    @dipper = Puppet::FileBucket::Dipper.new(
-      :Path => "/my/bucket"
-    )
+    Puppet[:bucketdir] = "/non/existent/directory"
+    file_bucket = tmpdir("bucket")
 
-    File.stubs(:exist?).returns true
-    File.stubs(:read).with("/my/file").returns "my contents"
+    @dipper = Puppet::FileBucket::Dipper.new(:Path => file_bucket)
 
-    bucketfile = stub "bucketfile"
-    bucketfile.stubs(:name).returns('md5/DIGEST123')
-    bucketfile.stubs(:checksum_data).returns("DIGEST123")
-    bucketfile.expects(:save).with('md5/DIGEST123')
+    file = make_tmp_file('my contents')
+    checksum = "2975f560750e71c478b8e3b39a956adb"
+    Digest::MD5.hexdigest('my contents').should == checksum
 
+    @dipper.backup(file).should == checksum
+    File.exists?("#{file_bucket}/2/9/7/5/f/5/6/0/2975f560750e71c478b8e3b39a956adb/contents").should == true
+  end
+
+  it "should not backup a file that is already in the bucket" do
+    @dipper = Puppet::FileBucket::Dipper.new(:Path => "/my/bucket")
 
-          Puppet::FileBucket::File.stubs(:new).with(
-                
-      "my contents",
-      :bucket_path => '/my/bucket',
-        
-      :path => '/my/file'
-    ).returns(bucketfile)
+    file = make_tmp_file('my contents')
+    checksum = Digest::MD5.hexdigest('my contents')
 
-    @dipper.backup("/my/file").should == "DIGEST123"
+    Puppet::FileBucket::File.expects(:head).returns true
+    Puppet::FileBucket::File.any_instance.expects(:save).never
+    @dipper.backup(file).should == checksum
   end
 
   it "should retrieve files from a local bucket" do
-    @dipper = Puppet::FileBucket::Dipper.new(
-      :Path => "/my/bucket"
-    )
+    @dipper = Puppet::FileBucket::Dipper.new(:Path => "/my/bucket")
+
+    checksum = Digest::MD5.hexdigest('my contents')
 
-    File.stubs(:exist?).returns true
-    File.stubs(:read).with("/my/file").returns "my contents"
+    request = nil
 
-    bucketfile = stub "bucketfile"
-    bucketfile.stubs(:to_s).returns "Content"
+    Puppet::FileBucketFile::File.any_instance.expects(:find).with{ |r| request = r }.once.returns(Puppet::FileBucket::File.new('my contents'))
 
-    Puppet::FileBucket::File.expects(:find).with{|x,opts|
-      x == 'md5/DIGEST123'
-    }.returns(bucketfile)
+    @dipper.getfile(checksum).should == 'my contents'
 
-    @dipper.getfile("DIGEST123").should == "Content"
+    request.key.should == "md5/#{checksum}"
   end
 
   it "should backup files to a remote server" do
+    @dipper = Puppet::FileBucket::Dipper.new(:Server => "puppetmaster", :Port => "31337")
 
-          @dipper = Puppet::FileBucket::Dipper.new(
-                
-      :Server => "puppetmaster",
-        
-      :Port   => "31337"
-    )
+    file = make_tmp_file('my contents')
+    checksum = Digest::MD5.hexdigest('my contents')
 
-    File.stubs(:exist?).returns true
-    File.stubs(:read).with("/my/file").returns "my contents"
+    real_path = Pathname.new(file).realpath
 
-    bucketfile = stub "bucketfile"
-    bucketfile.stubs(:name).returns('md5/DIGEST123')
-    bucketfile.stubs(:checksum_data).returns("DIGEST123")
-    bucketfile.expects(:save).with('https://puppetmaster:31337/production/file_bucket_file/md5/DIGEST123')
+    request1 = nil
+    request2 = nil
 
+    Puppet::FileBucketFile::Rest.any_instance.expects(:head).with { |r| request1 = r }.once.returns(nil)
+    Puppet::FileBucketFile::Rest.any_instance.expects(:save).with { |r| request2 = r }.once
 
-          Puppet::FileBucket::File.stubs(:new).with(
-                
-      "my contents",
-      :bucket_path => nil,
-        
-      :path => '/my/file'
-    ).returns(bucketfile)
-
-    @dipper.backup("/my/file").should == "DIGEST123"
+    @dipper.backup(file).should == checksum
+    [request1, request2].each do |r|
+      r.server.should == 'puppetmaster'
+      r.port.should == 31337
+      r.key.should == "md5/#{checksum}"
+    end
   end
 
   it "should retrieve files from a remote server" do
+    @dipper = Puppet::FileBucket::Dipper.new(:Server => "puppetmaster", :Port => "31337")
 
-          @dipper = Puppet::FileBucket::Dipper.new(
-                
-      :Server => "puppetmaster",
-        
-      :Port   => "31337"
-    )
+    checksum = Digest::MD5.hexdigest('my contents')
 
-    File.stubs(:exist?).returns true
-    File.stubs(:read).with("/my/file").returns "my contents"
+    request = nil
 
-    bucketfile = stub "bucketfile"
-    bucketfile.stubs(:to_s).returns "Content"
+    Puppet::FileBucketFile::Rest.any_instance.expects(:find).with { |r| request = r }.returns(Puppet::FileBucket::File.new('my contents'))
 
-    Puppet::FileBucket::File.expects(:find).with{|x,opts|
-      x == 'https://puppetmaster:31337/production/file_bucket_file/md5/DIGEST123'
-    }.returns(bucketfile)
+    @dipper.getfile(checksum).should == "my contents"
 
-    @dipper.getfile("DIGEST123").should == "Content"
+    request.server.should == 'puppetmaster'
+    request.port.should == 31337
+    request.key.should == "md5/#{checksum}"
   end
-
-
 end
diff --git a/spec/unit/file_bucket/file_spec.rb b/spec/unit/file_bucket/file_spec.rb
index 3ad70c203..82063c2e3 100644
--- a/spec/unit/file_bucket/file_spec.rb
+++ b/spec/unit/file_bucket/file_spec.rb
@@ -7,13 +7,16 @@ require 'digest/md5'
 require 'digest/sha1'
 
 describe Puppet::FileBucket::File do
+  include PuppetSpec::Files
+
   before do
     # this is the default from spec_helper, but it keeps getting reset at odd times
-    Puppet[:bucketdir] = "/dev/null/bucket"
+    @bucketdir = tmpdir('bucket')
+    Puppet[:bucketdir] = @bucketdir
 
     @digest = "4a8ec4fa5f01b4ab1a0ab8cbccb709f0"
     @checksum = "{md5}4a8ec4fa5f01b4ab1a0ab8cbccb709f0"
-    @dir = '/dev/null/bucket/4/a/8/e/c/4/f/a/4a8ec4fa5f01b4ab1a0ab8cbccb709f0'
+    @dir = File.join(@bucketdir, '4/a/8/e/c/4/f/a/4a8ec4fa5f01b4ab1a0ab8cbccb709f0')
 
     @contents = "file contents"
   end
@@ -22,17 +25,6 @@ describe Puppet::FileBucket::File do
     Puppet::FileBucket::File.new(@contents).to_s.should == @contents
   end
 
-  it "should calculate the checksum type from the passed in checksum" do
-    Puppet::FileBucket::File.new(@contents, :checksum => @checksum).checksum_type.should == "md5"
-  end
-
-  it "should allow contents to be specified in a block" do
-    bucket = Puppet::FileBucket::File.new(nil) do |fb|
-      fb.contents = "content"
-    end
-    bucket.contents.should == "content"
-  end
-
   it "should raise an error if changing content" do
     x = Puppet::FileBucket::File.new("first")
     proc { x.contents = "new" }.should raise_error
@@ -42,14 +34,6 @@ describe Puppet::FileBucket::File do
     proc { Puppet::FileBucket::File.new(5) }.should raise_error(ArgumentError)
   end
 
-  it "should raise an error if setting contents to a non-string" do
-    proc do
-      Puppet::FileBucket::File.new(nil) do |x|
-        x.contents = 5
-      end
-    end.should raise_error(ArgumentError)
-  end
-
   it "should set the contents appropriately" do
     Puppet::FileBucket::File.new(@contents).contents.should == @contents
   end
@@ -62,33 +46,6 @@ describe Puppet::FileBucket::File do
     Puppet::FileBucket::File.new(@contents).checksum.should == @checksum
   end
 
-  it "should remove the old checksum value if the algorithm is changed" do
-    sum = Puppet::FileBucket::File.new(@contents)
-    sum.checksum.should_not be_nil
-
-    newsum = Digest::SHA1.hexdigest(@contents).to_s
-    sum.checksum_type = :sha1
-    sum.checksum.should == "{sha1}#{newsum}"
-  end
-
-  it "should support specifying the checksum_type during initialization" do
-    sum = Puppet::FileBucket::File.new(@contents, :checksum_type => :sha1)
-    sum.checksum_type.should == :sha1
-  end
-
-  it "should fail when an unsupported checksum_type is used" do
-    proc { Puppet::FileBucket::File.new(@contents, :checksum_type => :nope) }.should raise_error(ArgumentError)
-  end
-
-  it "should fail if given an checksum at initialization that does not match the contents" do
-    proc { Puppet::FileBucket::File.new(@contents, :checksum => "{md5}00000000000000000000000000000000") }.should raise_error(RuntimeError)
-  end
-
-  it "should fail if assigned a checksum that does not match the contents" do
-    bucket = Puppet::FileBucket::File.new(@contents)
-    proc { bucket.checksum = "{md5}00000000000000000000000000000000" }.should raise_error(RuntimeError)
-  end
-
   describe "when using back-ends" do
     it "should redirect using Puppet::Indirector" do
       Puppet::Indirector::Indirection.instance(:file_bucket_file).model.should equal(Puppet::FileBucket::File)
@@ -129,26 +86,6 @@ describe Puppet::FileBucket::File do
 
       Puppet::FileBucket::File.new(@contents).save
     end
-
-    it "should append the path to the paths file" do
-      remote_path = '/path/on/the/remote/box'
-
-      ::File.expects(:directory?).with(@dir).returns(true)
-      ::File.expects(:open).with("#{@dir}/contents", ::File::WRONLY|::File::CREAT, 0440)
-      ::File.expects(:exist?).with("#{@dir}/contents").returns false
-
-      mockfile = mock "file"
-      mockfile.expects(:puts).with('/path/on/the/remote/box')
-      ::File.expects(:exist?).with("#{@dir}/paths").returns false
-      ::File.expects(:open).with("#{@dir}/paths", ::File::WRONLY|::File::CREAT|::File::APPEND).yields mockfile
-      Puppet::FileBucket::File.new(@contents, :path => remote_path).save
-
-    end
-  end
-
-  it "should accept a path" do
-    remote_path = '/path/on/the/remote/box'
-    Puppet::FileBucket::File.new(@contents, :path => remote_path).path.should == remote_path
   end
 
   it "should return a url-ish name" do
@@ -160,18 +97,6 @@ describe Puppet::FileBucket::File do
     lambda { bucket.name = "sha1/4a8ec4fa5f01b4ab1a0ab8cbccb709f0/new/path" }.should raise_error
   end
 
-  it "should accept a url-ish name" do
-    bucket = Puppet::FileBucket::File.new(@contents)
-    lambda { bucket.name = "sha1/034fa2ed8e211e4d20f20e792d777f4a30af1a93/new/path" }.should_not raise_error
-    bucket.checksum_type.should == "sha1"
-    bucket.checksum_data.should == '034fa2ed8e211e4d20f20e792d777f4a30af1a93'
-    bucket.path.should == "new/path"
-  end
-
-  it "should return a url-ish name with a path" do
-    Puppet::FileBucket::File.new(@contents, :path => 'my/path').name.should == "md5/4a8ec4fa5f01b4ab1a0ab8cbccb709f0/my/path"
-  end
-
   it "should convert the contents to PSON" do
     Puppet::FileBucket::File.new(@contents).to_pson.should == '{"contents":"file contents"}'
   end
@@ -191,36 +116,31 @@ describe Puppet::FileBucket::File do
 
   end
 
+  def make_bucketed_file
+    FileUtils.mkdir_p(@dir)
+    File.open("#{@dir}/contents", 'w') { |f| f.write @contents }
+  end
+
   describe "using the indirector's find method" do
     it "should return nil if a file doesn't exist" do
-      ::File.expects(:exist?).with("#{@dir}/contents").returns false
-
-      bucketfile = Puppet::FileBucket::File.find("{md5}#{@digest}")
+      bucketfile = Puppet::FileBucket::File.find("md5/#{@digest}")
       bucketfile.should == nil
     end
 
     it "should find a filebucket if the file exists" do
-      ::File.expects(:exist?).with("#{@dir}/contents").returns true
-      ::File.expects(:exist?).with("#{@dir}/paths").returns false
-      ::File.expects(:read).with("#{@dir}/contents").returns @contents
-
-      bucketfile = Puppet::FileBucket::File.find("{md5}#{@digest}")
+      make_bucketed_file
+      bucketfile = Puppet::FileBucket::File.find("md5/#{@digest}")
       bucketfile.should_not == nil
     end
 
     describe "using RESTish digest notation" do
       it "should return nil if a file doesn't exist" do
-        ::File.expects(:exist?).with("#{@dir}/contents").returns false
-
         bucketfile = Puppet::FileBucket::File.find("md5/#{@digest}")
         bucketfile.should == nil
       end
 
       it "should find a filebucket if the file exists" do
-        ::File.expects(:exist?).with("#{@dir}/contents").returns true
-        ::File.expects(:exist?).with("#{@dir}/paths").returns false
-        ::File.expects(:read).with("#{@dir}/contents").returns @contents
-
+        make_bucketed_file
         bucketfile = Puppet::FileBucket::File.find("md5/#{@digest}")
         bucketfile.should_not == nil
       end
diff --git a/spec/unit/indirector/file_bucket_file/file_spec.rb b/spec/unit/indirector/file_bucket_file/file_spec.rb
index 7bf02b5b3..9187f4da0 100755
--- a/spec/unit/indirector/file_bucket_file/file_spec.rb
+++ b/spec/unit/indirector/file_bucket_file/file_spec.rb
@@ -5,6 +5,8 @@ require ::File.dirname(__FILE__) + '/../../../spec_helper'
 require 'puppet/indirector/file_bucket_file/file'
 
 describe Puppet::FileBucketFile::File do
+  include PuppetSpec::Files
+
   it "should be a subclass of the Code terminus class" do
     Puppet::FileBucketFile::File.superclass.should equal(Puppet::Indirector::Code)
   end
@@ -66,186 +68,90 @@ HERE
   end
 
 
-  describe "the find_by_checksum method" do
-    before do
-      # this is the default from spec_helper, but it keeps getting reset at odd times
-      Puppet[:bucketdir] = "/dev/null/bucket"
-
-      @digest = "4a8ec4fa5f01b4ab1a0ab8cbccb709f0"
-      @checksum = "{md5}4a8ec4fa5f01b4ab1a0ab8cbccb709f0"
-      @dir = '/dev/null/bucket/4/a/8/e/c/4/f/a/4a8ec4fa5f01b4ab1a0ab8cbccb709f0'
-
-      @contents = "file contents"
-    end
-
-    it "should return nil if a file doesn't exist" do
-      ::File.expects(:exist?).with("#{@dir}/contents").returns false
-
-      bucketfile = Puppet::FileBucketFile::File.new.send(:find_by_checksum, "{md5}#{@digest}", {})
-      bucketfile.should == nil
-    end
-
-    it "should find a filebucket if the file exists" do
-      ::File.expects(:exist?).with("#{@dir}/contents").returns true
-      ::File.expects(:exist?).with("#{@dir}/paths").returns false
-      ::File.expects(:read).with("#{@dir}/contents").returns @contents
-
-      bucketfile = Puppet::FileBucketFile::File.new.send(:find_by_checksum, "{md5}#{@digest}", {})
-      bucketfile.should_not == nil
-    end
-
-    it "should load the paths" do
-      paths = ["path1", "path2"]
-      ::File.expects(:exist?).with("#{@dir}/contents").returns true
-      ::File.expects(:exist?).with("#{@dir}/paths").returns true
-      ::File.expects(:read).with("#{@dir}/contents").returns @contents
-
-      mockfile = mock "file"
-      mockfile.expects(:readlines).returns( paths )
-      ::File.expects(:open).with("#{@dir}/paths").yields mockfile
-
-      Puppet::FileBucketFile::File.new.send(:find_by_checksum, "{md5}#{@digest}", {}).paths.should == paths
-    end
-
-  end
-
-  describe "when retrieving files" do
-    before :each do
-      Puppet.settings.stubs(:use)
-      @store = Puppet::FileBucketFile::File.new
-
-      @digest = "70924d6fa4b2d745185fa4660703a5c0"
-      @sum = stub 'sum', :name => @digest
-
-      @dir = "/what/ever"
-
-      Puppet.stubs(:[]).with(:bucketdir).returns(@dir)
-
-      @contents_path = '/what/ever/7/0/9/2/4/d/6/f/70924d6fa4b2d745185fa4660703a5c0/contents'
-      @paths_path    = '/what/ever/7/0/9/2/4/d/6/f/70924d6fa4b2d745185fa4660703a5c0/paths'
-
-      @request = stub 'request', :key => "md5/#{@digest}/remote/path", :options => {}
-    end
-
-    it "should call find_by_checksum" do
-      @store.expects(:find_by_checksum).with{|x,opts| x == "{md5}#{@digest}"}.returns(false)
-      @store.find(@request)
-    end
-
-    it "should look for the calculated path" do
-      ::File.expects(:exist?).with(@contents_path).returns(false)
-      @store.find(@request)
-    end
-
-    it "should return an instance of Puppet::FileBucket::File created with the content if the file exists" do
-      content = "my content"
-      bucketfile = stub 'bucketfile'
-      bucketfile.stubs(:bucket_path)
-      bucketfile.stubs(:bucket_path=)
-      bucketfile.stubs(:checksum_data).returns(@digest)
-      bucketfile.stubs(:checksum).returns(@checksum)
-
-      bucketfile.expects(:contents=).with(content)
-      Puppet::FileBucket::File.expects(:new).with(nil, {:checksum => "{md5}#{@digest}"}).yields(bucketfile).returns(bucketfile)
-
-      ::File.expects(:exist?).with(@contents_path).returns(true)
-      ::File.expects(:exist?).with(@paths_path).returns(false)
-      ::File.expects(:read).with(@contents_path).returns(content)
-
-      @store.find(@request).should equal(bucketfile)
-    end
-
-    it "should return nil if no file is found" do
-      ::File.expects(:exist?).with(@contents_path).returns(false)
-      @store.find(@request).should be_nil
-    end
-
-    it "should fail intelligently if a found file cannot be read" do
-      ::File.expects(:exist?).with(@contents_path).returns(true)
-      ::File.expects(:read).with(@contents_path).raises(RuntimeError)
-      proc { @store.find(@request) }.should raise_error(Puppet::Error)
-    end
-
-  end
-
-  describe "when determining file paths" do
-    before do
-      Puppet[:bucketdir] = '/dev/null/bucketdir'
-      @digest = 'DEADBEEFC0FFEE'
-      @bucket = stub_everything "bucket"
-      @bucket.expects(:checksum_data).returns(@digest)
-    end
-
-    it "should use the value of the :bucketdir setting as the root directory" do
-      path = Puppet::FileBucketFile::File.new.send(:contents_path_for, @bucket)
-      path.should =~ %r{^/dev/null/bucketdir}
-    end
-
-    it "should choose a path 8 directories deep with each directory name being the respective character in the filebucket" do
-      path = Puppet::FileBucketFile::File.new.send(:contents_path_for, @bucket)
-      dirs = @digest[0..7].split("").join(File::SEPARATOR)
-      path.should be_include(dirs)
-    end
-
-    it "should use the full filebucket as the final directory name" do
-      path = Puppet::FileBucketFile::File.new.send(:contents_path_for, @bucket)
-      ::File.basename(::File.dirname(path)).should == @digest
-    end
-
-    it "should use 'contents' as the actual file name" do
-      path = Puppet::FileBucketFile::File.new.send(:contents_path_for, @bucket)
-      ::File.basename(path).should == "contents"
-    end
-
-    it "should use the bucketdir, the 8 sum character directories, the full filebucket, and 'contents' as the full file name" do
-      path = Puppet::FileBucketFile::File.new.send(:contents_path_for, @bucket)
-      path.should == ['/dev/null/bucketdir', @digest[0..7].split(""), @digest, "contents"].flatten.join(::File::SEPARATOR)
-    end
-  end
-
-  describe "when saving files" do
-    before do
-      # this is the default from spec_helper, but it keeps getting reset at odd times
-      Puppet[:bucketdir] = "/dev/null/bucket"
-
-      @digest = "4a8ec4fa5f01b4ab1a0ab8cbccb709f0"
-      @checksum = "{md5}4a8ec4fa5f01b4ab1a0ab8cbccb709f0"
-      @dir = '/dev/null/bucket/4/a/8/e/c/4/f/a/4a8ec4fa5f01b4ab1a0ab8cbccb709f0'
-
-      @contents = "file contents"
-
-      @bucket = stub "bucket file"
-      @bucket.stubs(:bucket_path)
-      @bucket.stubs(:checksum_data).returns(@digest)
-      @bucket.stubs(:path).returns(nil)
-      @bucket.stubs(:checksum).returns(nil)
-      @bucket.stubs(:contents).returns("file contents")
-    end
-
-    it "should save the contents to the calculated path" do
-      ::File.stubs(:directory?).with(@dir).returns(true)
-      ::File.expects(:exist?).with("#{@dir}/contents").returns false
-
-      mockfile = mock "file"
-      mockfile.expects(:print).with(@contents)
-      ::File.expects(:open).with("#{@dir}/contents", ::File::WRONLY|::File::CREAT, 0440).yields(mockfile)
-
-      Puppet::FileBucketFile::File.new.send(:save_to_disk, @bucket)
-    end
-
-    it "should make any directories necessary for storage" do
-      FileUtils.expects(:mkdir_p).with do |arg|
-        ::File.umask == 0007 and arg == @dir
+  [true, false].each do |override_bucket_path|
+    describe "when bucket path #{if override_bucket_path then 'is' else 'is not' end} overridden" do
+      [true, false].each do |supply_path|
+        describe "when #{supply_path ? 'supplying' : 'not supplying'} a path" do
+          before :each do
+            Puppet.settings.stubs(:use)
+            @store = Puppet::FileBucketFile::File.new
+            @contents = "my content"
+
+            @digest = "f2bfa7fc155c4f42cb91404198dda01f"
+            @digest.should == Digest::MD5.hexdigest(@contents)
+
+            @bucket_dir = tmpdir("bucket")
+
+            if override_bucket_path
+              Puppet[:bucketdir] = "/bogus/path" # should not be used
+            else
+              Puppet[:bucketdir] = @bucket_dir
+            end
+
+            @dir = "#{@bucket_dir}/f/2/b/f/a/7/f/c/f2bfa7fc155c4f42cb91404198dda01f"
+            @contents_path = "#{@dir}/contents"
+          end
+
+          describe "when retrieving files" do
+            before :each do
+
+              request_options = {}
+              if override_bucket_path
+                request_options[:bucket_path] = @bucket_dir
+              end
+
+              key = "md5/#{@digest}"
+              if supply_path
+                key += "//path/to/file"
+              end
+
+              @request = Puppet::Indirector::Request.new(:indirection_name, :find, key, request_options)
+            end
+
+            def make_bucketed_file
+              FileUtils.mkdir_p(@dir)
+              File.open(@contents_path, 'w') { |f| f.write @contents }
+            end
+
+            it "should return an instance of Puppet::FileBucket::File created with the content if the file exists" do
+              make_bucketed_file
+
+              bucketfile = @store.find(@request)
+              bucketfile.should be_a(Puppet::FileBucket::File)
+              bucketfile.contents.should == @contents
+              @store.head(@request).should == true
+            end
+
+            it "should return nil if no file is found" do
+              @store.find(@request).should be_nil
+              @store.head(@request).should == false
+            end
+          end
+
+          describe "when saving files" do
+            it "should save the contents to the calculated path" do
+              options = {}
+              if override_bucket_path
+                options[:bucket_path] = @bucket_dir
+              end
+
+              key = "md5/#{@digest}"
+              if supply_path
+                key += "//path/to/file"
+              end
+
+              file_instance = Puppet::FileBucket::File.new(@contents, options)
+              request = Puppet::Indirector::Request.new(:indirection_name, :save, key, file_instance)
+
+              @store.save(request)
+              File.read("#{@dir}/contents").should == @contents
+            end
+          end
+        end
       end
-      ::File.expects(:directory?).with(@dir).returns(false)
-      ::File.expects(:open).with("#{@dir}/contents", ::File::WRONLY|::File::CREAT, 0440)
-      ::File.expects(:exist?).with("#{@dir}/contents").returns false
-
-      Puppet::FileBucketFile::File.new.send(:save_to_disk, @bucket)
     end
   end
 
-
   describe "when verifying identical files" do
     before do
       # this is the default from spec_helper, but it keeps getting reset at odd times
@@ -276,60 +182,4 @@ HERE
     end
 
   end
-
-
-  describe "when writing to the paths file" do
-    before do
-      Puppet[:bucketdir] = '/dev/null/bucketdir'
-      @digest = '70924d6fa4b2d745185fa4660703a5c0'
-      @bucket = stub_everything "bucket"
-
-      @paths_path    = '/dev/null/bucketdir/7/0/9/2/4/d/6/f/70924d6fa4b2d745185fa4660703a5c0/paths'
-
-      @paths = []
-      @bucket.stubs(:paths).returns(@paths)
-      @bucket.stubs(:checksum_data).returns(@digest)
-    end
-
-    it "should create a file if it doesn't exist" do
-      @bucket.expects(:path).returns('path/to/save').at_least_once
-      File.expects(:exist?).with(@paths_path).returns(false)
-      file = stub "file"
-      file.expects(:puts).with('path/to/save')
-      File.expects(:open).with(@paths_path, ::File::WRONLY|::File::CREAT|::File::APPEND).yields(file)
-
-      Puppet::FileBucketFile::File.new.send(:save_path_to_paths_file, @bucket)
-    end
-
-    it "should append to a file if it exists" do
-      @bucket.expects(:path).returns('path/to/save').at_least_once
-      File.expects(:exist?).with(@paths_path).returns(true)
-      old_file = stub "file"
-      old_file.stubs(:readlines).returns []
-      File.expects(:open).with(@paths_path).yields(old_file)
-
-      file = stub "file"
-      file.expects(:puts).with('path/to/save')
-      File.expects(:open).with(@paths_path, ::File::WRONLY|::File::CREAT|::File::APPEND).yields(file)
-
-      Puppet::FileBucketFile::File.new.send(:save_path_to_paths_file, @bucket)
-    end
-
-    it "should not alter a file if it already contains the path" do
-      @bucket.expects(:path).returns('path/to/save').at_least_once
-      File.expects(:exist?).with(@paths_path).returns(true)
-      old_file = stub "file"
-      old_file.stubs(:readlines).returns ["path/to/save\n"]
-      File.expects(:open).with(@paths_path).yields(old_file)
-
-      Puppet::FileBucketFile::File.new.send(:save_path_to_paths_file, @bucket)
-    end
-
-    it "should do nothing if there is no path" do
-      @bucket.expects(:path).returns(nil).at_least_once
-
-      Puppet::FileBucketFile::File.new.send(:save_path_to_paths_file, @bucket)
-    end
-  end
-
 end
diff --git a/spec/unit/indirector/file_server_spec.rb b/spec/unit/indirector/file_server_spec.rb
index 686f79a24..eafcf2bb7 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 1e774fb2e..a910cb6f2 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 5f0fe363a..7bc1cc513 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 acbfc1726..6f44764da 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 c593242c0..23a291cf3 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 cdbce41f7..8464ae68e 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 06436e723..d629f8670 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(:fail_on_deny).with("/path/to/resource", :node => "me", :ip => "127.0.0.1", :method => :save, :environment => :env, :authenticated => true)
+    @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 969fc189e..3b9e48374 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)
@@ -155,19 +175,19 @@ describe Puppet::Network::Rights do
       Puppet::Network::Rights::Right.stubs(:new).returns(@pathacl)
     end
 
-    it "should delegate to fail_on_deny" do
-      @right.expects(:fail_on_deny).with("namespace", :node => "host.domain.com", :ip => "127.0.0.1")
+    it "should delegate to is_forbidden_and_why?" do
+      @right.expects(:is_forbidden_and_why?).with("namespace", :node => "host.domain.com", :ip => "127.0.0.1").returns(nil)
 
       @right.allowed?("namespace", "host.domain.com", "127.0.0.1")
     end
 
-    it "should return true if fail_on_deny doesn't fail" do
-      @right.stubs(:fail_on_deny)
+    it "should return true if is_forbidden_and_why? returns nil" do
+      @right.stubs(:is_forbidden_and_why?).returns(nil)
       @right.allowed?("namespace", :args).should be_true
     end
 
-    it "should return false if fail_on_deny raises an AuthorizationError" do
-      @right.stubs(:fail_on_deny).raises(Puppet::Network::AuthorizationError.new("forbidden"))
+    it "should return false if is_forbidden_and_why? returns an AuthorizationError" do
+      @right.stubs(:is_forbidden_and_why?).returns(Puppet::Network::AuthorizationError.new("forbidden"))
       @right.allowed?("namespace", :args1, :args2).should be_false
     end
 
@@ -179,7 +199,7 @@ describe Puppet::Network::Rights do
       acl.expects(:match?).returns(true)
       acl.expects(:allowed?).with { |node,ip,h| node == "node" and ip == "ip" }.returns(true)
 
-      @right.fail_on_deny("namespace", { :node => "node", :ip => "ip" } )
+      @right.is_forbidden_and_why?("namespace", { :node => "node", :ip => "ip" } ).should == nil
     end
 
     it "should then check for path rights if no namespace match" do
@@ -195,7 +215,7 @@ describe Puppet::Network::Rights do
       acl.expects(:allowed?).never
       @pathacl.expects(:allowed?).returns(true)
 
-      @right.fail_on_deny("/path/to/there", {})
+      @right.is_forbidden_and_why?("/path/to/there", {}).should == nil
     end
 
     it "should pass the match? return to allowed?" do
@@ -204,12 +224,12 @@ describe Puppet::Network::Rights do
       @pathacl.expects(:match?).returns(:match)
       @pathacl.expects(:allowed?).with { |node,ip,h| h[:match] == :match }.returns(true)
 
-      @right.fail_on_deny("/path/to/there", {})
+      @right.is_forbidden_and_why?("/path/to/there", {}).should == nil
     end
 
     describe "with namespace acls" do
-      it "should raise an error if this namespace right doesn't exist" do
-        lambda{ @right.fail_on_deny("namespace") }.should raise_error
+      it "should return an ArgumentError if this namespace right doesn't exist" do
+        lambda { @right.is_forbidden_and_why?("namespace") }.should raise_error(ArgumentError)
       end
     end
 
@@ -235,7 +255,7 @@ describe Puppet::Network::Rights do
         @long_acl.expects(:allowed?).returns(true)
         @short_acl.expects(:allowed?).never
 
-        @right.fail_on_deny("/path/to/there/and/there", {})
+        @right.is_forbidden_and_why?("/path/to/there/and/there", {}).should == nil
       end
 
       it "should select the first match that doesn't return :dunno" do
@@ -248,7 +268,7 @@ describe Puppet::Network::Rights do
         @long_acl.expects(:allowed?).returns(:dunno)
         @short_acl.expects(:allowed?).returns(true)
 
-        @right.fail_on_deny("/path/to/there/and/there", {})
+        @right.is_forbidden_and_why?("/path/to/there/and/there", {}).should == nil
       end
 
       it "should not select an ACL that doesn't match" do
@@ -261,7 +281,7 @@ describe Puppet::Network::Rights do
         @long_acl.expects(:allowed?).never
         @short_acl.expects(:allowed?).returns(true)
 
-        @right.fail_on_deny("/path/to/there/and/there", {})
+        @right.is_forbidden_and_why?("/path/to/there/and/there", {}).should == nil
       end
 
       it "should not raise an AuthorizationError if allowed" do
@@ -270,7 +290,7 @@ describe Puppet::Network::Rights do
         @long_acl.stubs(:match?).returns(true)
         @long_acl.stubs(:allowed?).returns(true)
 
-        lambda { @right.fail_on_deny("/path/to/there/and/there", {}) }.should_not raise_error(Puppet::Network::AuthorizationError)
+        @right.is_forbidden_and_why?("/path/to/there/and/there", {}).should == nil
       end
 
       it "should raise an AuthorizationError if the match is denied" do
@@ -279,11 +299,11 @@ describe Puppet::Network::Rights do
         @long_acl.stubs(:match?).returns(true)
         @long_acl.stubs(:allowed?).returns(false)
 
-        lambda{ @right.fail_on_deny("/path/to/there", {}) }.should raise_error(Puppet::Network::AuthorizationError)
+        @right.is_forbidden_and_why?("/path/to/there", {}).should be_instance_of(Puppet::Network::AuthorizationError)
       end
 
       it "should raise an AuthorizationError if no path match" do
-        lambda { @right.fail_on_deny("/nomatch", {}) }.should raise_error(Puppet::Network::AuthorizationError)
+        @right.is_forbidden_and_why?("/nomatch", {}).should be_instance_of(Puppet::Network::AuthorizationError)
       end
     end
 
@@ -309,7 +329,7 @@ describe Puppet::Network::Rights do
         @regex_acl1.expects(:allowed?).returns(true)
         @regex_acl2.expects(:allowed?).never
 
-        @right.fail_on_deny("/files/repository/myfile/other", {})
+        @right.is_forbidden_and_why?("/files/repository/myfile/other", {}).should == nil
       end
 
       it "should select the first match that doesn't return :dunno" do
@@ -322,7 +342,7 @@ describe Puppet::Network::Rights do
         @regex_acl1.expects(:allowed?).returns(:dunno)
         @regex_acl2.expects(:allowed?).returns(true)
 
-        @right.fail_on_deny("/files/repository/myfile/other", {})
+        @right.is_forbidden_and_why?("/files/repository/myfile/other", {}).should == nil
       end
 
       it "should not select an ACL that doesn't match" do
@@ -335,7 +355,7 @@ describe Puppet::Network::Rights do
         @regex_acl1.expects(:allowed?).never
         @regex_acl2.expects(:allowed?).returns(true)
 
-        @right.fail_on_deny("/files/repository/myfile/other", {})
+        @right.is_forbidden_and_why?("/files/repository/myfile/other", {}).should == nil
       end
 
       it "should not raise an AuthorizationError if allowed" do
@@ -344,15 +364,15 @@ describe Puppet::Network::Rights do
         @regex_acl1.stubs(:match?).returns(true)
         @regex_acl1.stubs(:allowed?).returns(true)
 
-        lambda { @right.fail_on_deny("/files/repository/myfile/other", {}) }.should_not raise_error(Puppet::Network::AuthorizationError)
+        @right.is_forbidden_and_why?("/files/repository/myfile/other", {}).should == nil
       end
 
       it "should raise an error if no regex acl match" do
-        lambda{ @right.fail_on_deny("/path", {}) }.should raise_error(Puppet::Network::AuthorizationError)
+        @right.is_forbidden_and_why?("/path", {}).should be_instance_of(Puppet::Network::AuthorizationError)
       end
 
       it "should raise an AuthorizedError on deny" do
-        lambda { @right.fail_on_deny("/path", {}) }.should raise_error(Puppet::Network::AuthorizationError)
+        @right.is_forbidden_and_why?("/path", {}).should be_instance_of(Puppet::Network::AuthorizationError)
       end
 
     end