Skip to content
Permalink
Browse files Browse the repository at this point in the history
Add Selector terminus for file_content/file_metadata
This terminus is now the default, and encapsulates the behavior that was
previously in the IndirectionHooks. That is, we dynamically select the
terminus to use for a file request based on the key. However, for the
puppet master, we instead explicitly always use the FileServer terminus,
so that *all* requests for files from the master will go through
the file server. This ensures that we will never accidentally
serve local files on the puppet master.
  • Loading branch information
pcarlisle authored and nicklewis committed Jun 27, 2012
1 parent d174a9f commit c3c7462
Show file tree
Hide file tree
Showing 11 changed files with 78 additions and 31 deletions.
3 changes: 3 additions & 0 deletions lib/puppet/application/master.rb
Expand Up @@ -141,6 +141,9 @@ def setup
# Cache our nodes in yaml. Currently not configurable.
Puppet::Node.cache_class = :yaml

Puppet::FileServing::Content.indirection.terminus_class = :file_server
Puppet::FileServing::Metadata.indirection.terminus_class = :file_server

# Configure all of the SSL stuff.
if Puppet::SSL::CertificateAuthority.ca?
Puppet::SSL::Host.ca_location = :local
Expand Down
2 changes: 1 addition & 1 deletion lib/puppet/file_serving/configuration.rb
Expand Up @@ -70,7 +70,7 @@ def split_path(request)

mount_name, path = request.key.split(File::Separator, 2)

raise(ArgumentError, "Cannot find file: Invalid path '#{mount_name}'") unless mount_name =~ %r{^[-\w]+$}
raise(ArgumentError, "Cannot find file: Invalid mount '#{mount_name}'") unless mount_name =~ %r{^[-\w]+$}

return nil unless mount = find_mount(mount_name, request.environment)
if mount.name == "modules" and mount_name != "modules"
Expand Down
3 changes: 1 addition & 2 deletions lib/puppet/file_serving/content.rb
Expand Up @@ -5,14 +5,13 @@
require 'puppet/indirector'
require 'puppet/file_serving'
require 'puppet/file_serving/base'
require 'puppet/file_serving/indirection_hooks'

# A class that handles retrieving file contents.
# It only reads the file when its content is specifically
# asked for.
class Puppet::FileServing::Content < Puppet::FileServing::Base
extend Puppet::Indirector
indirects :file_content, :extend => Puppet::FileServing::IndirectionHooks
indirects :file_content, :terminus_class => :selector

attr_writer :content

Expand Down
3 changes: 1 addition & 2 deletions lib/puppet/file_serving/metadata.rb
Expand Up @@ -7,15 +7,14 @@
require 'puppet/file_serving'
require 'puppet/file_serving/base'
require 'puppet/util/checksums'
require 'puppet/file_serving/indirection_hooks'

# A class that handles retrieving file metadata.
class Puppet::FileServing::Metadata < Puppet::FileServing::Base

include Puppet::Util::Checksums

extend Puppet::Indirector
indirects :file_metadata, :extend => Puppet::FileServing::IndirectionHooks
indirects :file_metadata, :terminus_class => :selector

attr_reader :path, :owner, :group, :mode, :checksum_type, :checksum, :ftype, :destination

Expand Down
Expand Up @@ -8,11 +8,10 @@
# This module is used to pick the appropriate terminus
# in file-serving indirections. This is necessary because
# the terminus varies based on the URI asked for.
module Puppet::FileServing::IndirectionHooks
module Puppet::FileServing::TerminusSelector
PROTOCOL_MAP = {"puppet" => :rest, "file" => :file}

# Pick an appropriate terminus based on the protocol.
def select_terminus(request)
def select(request)
# We rely on the request's parsing of the URI.

# Short-circuit to :file if it's a fully-qualified path or specifies a 'file' protocol.
Expand Down
29 changes: 29 additions & 0 deletions lib/puppet/indirector/file_content/selector.rb
@@ -0,0 +1,29 @@
require 'puppet/indirector/file_content'
require 'puppet/indirector/code'
require 'puppet/file_serving/terminus_selector'

class Puppet::Indirector::FileContent::Selector < Puppet::Indirector::Code
desc "Select the terminus based on the request"
include Puppet::FileServing::TerminusSelector

def get_terminus(request)
indirection.terminus(select(request))
end

def find(request)
get_terminus(request).find(request)
end

def search(request)
get_terminus(request).search(request)
end

def authorized?(request)
terminus = get_terminus(request)
if terminus.respond_to?(:authorized?)
terminus.authorized?(request)
else
true
end
end
end
29 changes: 29 additions & 0 deletions lib/puppet/indirector/file_metadata/selector.rb
@@ -0,0 +1,29 @@
require 'puppet/indirector/file_metadata'
require 'puppet/indirector/code'
require 'puppet/file_serving/terminus_selector'

class Puppet::Indirector::FileMetadata::Selector < Puppet::Indirector::Code
desc "Select the terminus based on the request"
include Puppet::FileServing::TerminusSelector

def get_terminus(request)
indirection.terminus(select(request))
end

def find(request)
get_terminus(request).find(request)
end

def search(request)
get_terminus(request).search(request)
end

def authorized?(request)
terminus = get_terminus(request)
if terminus.respond_to?(:authorized?)
terminus.authorized?(request)
else
true
end
end
end
4 changes: 1 addition & 3 deletions spec/shared_behaviours/file_serving.rb
Expand Up @@ -9,9 +9,7 @@

# It appears that the mocking somehow interferes with the caching subsystem.
# This mock somehow causes another terminus to get generated.
term = @indirection.terminus(:rest)
@indirection.stubs(:terminus).with(:rest).returns term
term.expects(:find)
@indirection.terminus(:rest).expects(:find)
@test_class.find(uri)
end

Expand Down
4 changes: 0 additions & 4 deletions spec/unit/file_serving/content_spec.rb
Expand Up @@ -13,10 +13,6 @@
Puppet::FileServing::Content.indirection.name.should == :file_content
end

it "should should include the IndirectionHooks module in its indirection" do
Puppet::FileServing::Content.indirection.singleton_class.included_modules.should include(Puppet::FileServing::IndirectionHooks)
end

it "should only support the raw format" do
Puppet::FileServing::Content.supported_formats.should == [:raw]
end
Expand Down
4 changes: 0 additions & 4 deletions spec/unit/file_serving/metadata_spec.rb
Expand Up @@ -13,10 +13,6 @@
Puppet::FileServing::Metadata.indirection.name.should == :file_metadata
end

it "should should include the IndirectionHooks module in its indirection" do
Puppet::FileServing::Metadata.indirection.singleton_class.included_modules.should include(Puppet::FileServing::IndirectionHooks)
end

it "should have a method that triggers attribute collection" do
Puppet::FileServing::Metadata.new("/foo/bar").should respond_to(:collect)
end
Expand Down
Expand Up @@ -5,30 +5,30 @@

require File.dirname(__FILE__) + '/../../spec_helper'

require 'puppet/file_serving/indirection_hooks'
require 'puppet/file_serving/terminus_selector'

describe Puppet::FileServing::IndirectionHooks do
describe Puppet::FileServing::TerminusSelector do
before do
@object = Object.new
@object.extend(Puppet::FileServing::IndirectionHooks)
@object.extend(Puppet::FileServing::TerminusSelector)

@request = stub 'request', :key => "mymod/myfile", :options => {:node => "whatever"}, :server => nil, :protocol => nil
end

describe "when being used to select termini" do
it "should return :file if the request key is fully qualified" do
@request.expects(:key).returns "#{File::SEPARATOR}foo"
@object.select_terminus(@request).should == :file
@request.expects(:key).returns File.expand_path('/foo')
@object.select(@request).should == :file
end

it "should return :file if the URI protocol is set to 'file'" do
@request.expects(:protocol).returns "file"
@object.select_terminus(@request).should == :file
@object.select(@request).should == :file
end

it "should fail when a protocol other than :puppet or :file is used" do
@request.stubs(:protocol).returns "http"
proc { @object.select_terminus(@request) }.should raise_error(ArgumentError)
proc { @object.select(@request) }.should raise_error(ArgumentError)
end

describe "and the protocol is 'puppet'" do
Expand All @@ -39,24 +39,23 @@
it "should choose :rest when a server is specified" do
@request.stubs(:protocol).returns "puppet"
@request.expects(:server).returns "foo"
@object.select_terminus(@request).should == :rest
@object.select(@request).should == :rest
end

# This is so a given file location works when bootstrapping with no server.
it "should choose :rest when the Settings name isn't 'puppet'" do
@request.stubs(:protocol).returns "puppet"
@request.stubs(:server).returns "foo"
# We have to stub this because we can't set name
Puppet.settings.stubs(:value).with(:name).returns "foo"
@object.select_terminus(@request).should == :rest
@object.select(@request).should == :rest
end

it "should choose :file_server when the settings name is 'puppet' and no server is specified" do
modules = mock 'modules'

@request.expects(:protocol).returns "puppet"
@request.expects(:server).returns nil
Puppet.settings.expects(:value).with(:name).returns "puppet"
@object.select_terminus(@request).should == :file_server
@object.select(@request).should == :file_server
end
end
end
Expand Down

0 comments on commit c3c7462

Please sign in to comment.