Skip to content

Commit 10f6cb8

Browse files
pcarlislenicklewis
authored andcommitted
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.
1 parent 34b9c0b commit 10f6cb8

File tree

11 files changed

+79
-34
lines changed

11 files changed

+79
-34
lines changed

Diff for: lib/puppet/application/master.rb

+3
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,9 @@ def setup
231231
# Cache our nodes in yaml. Currently not configurable.
232232
Puppet::Node.indirection.cache_class = :yaml
233233

234+
Puppet::FileServing::Content.indirection.terminus_class = :file_server
235+
Puppet::FileServing::Metadata.indirection.terminus_class = :file_server
236+
234237
# Configure all of the SSL stuff.
235238
if Puppet::SSL::CertificateAuthority.ca?
236239
Puppet::SSL::Host.ca_location = :local

Diff for: lib/puppet/file_serving/configuration.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ def split_path(request)
6464

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

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

6969
return nil unless mount = find_mount(mount_name, request.environment)
7070
if mount.name == "modules" and mount_name != "modules"

Diff for: lib/puppet/file_serving/content.rb

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,13 @@
11
require 'puppet/indirector'
22
require 'puppet/file_serving'
33
require 'puppet/file_serving/base'
4-
require 'puppet/file_serving/indirection_hooks'
54

65
# A class that handles retrieving file contents.
76
# It only reads the file when its content is specifically
87
# asked for.
98
class Puppet::FileServing::Content < Puppet::FileServing::Base
109
extend Puppet::Indirector
11-
indirects :file_content, :extend => Puppet::FileServing::IndirectionHooks
10+
indirects :file_content, :terminus_class => :selector
1211

1312
attr_writer :content
1413

Diff for: lib/puppet/file_serving/metadata.rb

+1-2
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,14 @@
33
require 'puppet/file_serving'
44
require 'puppet/file_serving/base'
55
require 'puppet/util/checksums'
6-
require 'puppet/file_serving/indirection_hooks'
76

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

1110
include Puppet::Util::Checksums
1211

1312
extend Puppet::Indirector
14-
indirects :file_metadata, :extend => Puppet::FileServing::IndirectionHooks
13+
indirects :file_metadata, :terminus_class => :selector
1514

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

Diff for: lib/puppet/file_serving/indirection_hooks.rb renamed to lib/puppet/file_serving/terminus_selector.rb

+2-5
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,12 @@
1-
require 'uri'
21
require 'puppet/file_serving'
3-
require 'puppet/util'
42

53
# This module is used to pick the appropriate terminus
64
# in file-serving indirections. This is necessary because
75
# the terminus varies based on the URI asked for.
8-
module Puppet::FileServing::IndirectionHooks
6+
module Puppet::FileServing::TerminusSelector
97
PROTOCOL_MAP = {"puppet" => :rest, "file" => :file}
108

11-
# Pick an appropriate terminus based on the protocol.
12-
def select_terminus(request)
9+
def select(request)
1310
# We rely on the request's parsing of the URI.
1411

1512
# Short-circuit to :file if it's a fully-qualified path or specifies a 'file' protocol.

Diff for: lib/puppet/indirector/file_content/selector.rb

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
require 'puppet/indirector/file_content'
2+
require 'puppet/indirector/code'
3+
require 'puppet/file_serving/terminus_selector'
4+
5+
class Puppet::Indirector::FileContent::Selector < Puppet::Indirector::Code
6+
desc "Select the terminus based on the request"
7+
include Puppet::FileServing::TerminusSelector
8+
9+
def get_terminus(request)
10+
indirection.terminus(select(request))
11+
end
12+
13+
def find(request)
14+
get_terminus(request).find(request)
15+
end
16+
17+
def search(request)
18+
get_terminus(request).search(request)
19+
end
20+
21+
def authorized?(request)
22+
terminus = get_terminus(request)
23+
if terminus.respond_to?(:authorized?)
24+
terminus.authorized?(request)
25+
else
26+
true
27+
end
28+
end
29+
end

Diff for: lib/puppet/indirector/file_metadata/selector.rb

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
require 'puppet/indirector/file_metadata'
2+
require 'puppet/indirector/code'
3+
require 'puppet/file_serving/terminus_selector'
4+
5+
class Puppet::Indirector::FileMetadata::Selector < Puppet::Indirector::Code
6+
desc "Select the terminus based on the request"
7+
include Puppet::FileServing::TerminusSelector
8+
9+
def get_terminus(request)
10+
indirection.terminus(select(request))
11+
end
12+
13+
def find(request)
14+
get_terminus(request).find(request)
15+
end
16+
17+
def search(request)
18+
get_terminus(request).search(request)
19+
end
20+
21+
def authorized?(request)
22+
terminus = get_terminus(request)
23+
if terminus.respond_to?(:authorized?)
24+
terminus.authorized?(request)
25+
else
26+
true
27+
end
28+
end
29+
end

Diff for: spec/shared_behaviours/file_serving.rb

+1-3
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,7 @@
55

66
# It appears that the mocking somehow interferes with the caching subsystem.
77
# This mock somehow causes another terminus to get generated.
8-
term = @indirection.terminus(:rest)
9-
@indirection.stubs(:terminus).with(:rest).returns term
10-
term.expects(:find)
8+
@indirection.terminus(:rest).expects(:find)
119
@indirection.find(uri)
1210
end
1311

Diff for: spec/unit/file_serving/content_spec.rb

+1-5
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,14 @@
66
describe Puppet::FileServing::Content do
77
let(:path) { File.expand_path('/path') }
88

9-
it "should should be a subclass of Base" do
9+
it "should be a subclass of Base" do
1010
Puppet::FileServing::Content.superclass.should equal(Puppet::FileServing::Base)
1111
end
1212

1313
it "should indirect file_content" do
1414
Puppet::FileServing::Content.indirection.name.should == :file_content
1515
end
1616

17-
it "should should include the IndirectionHooks module in its indirection" do
18-
Puppet::FileServing::Content.indirection.singleton_class.included_modules.should include(Puppet::FileServing::IndirectionHooks)
19-
end
20-
2117
it "should only support the raw format" do
2218
Puppet::FileServing::Content.supported_formats.should == [:raw]
2319
end

Diff for: spec/unit/file_serving/metadata_spec.rb

+1-5
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,14 @@
66
describe Puppet::FileServing::Metadata do
77
let(:foobar) { File.expand_path('/foo/bar') }
88

9-
it "should should be a subclass of Base" do
9+
it "should be a subclass of Base" do
1010
Puppet::FileServing::Metadata.superclass.should equal(Puppet::FileServing::Base)
1111
end
1212

1313
it "should indirect file_metadata" do
1414
Puppet::FileServing::Metadata.indirection.name.should == :file_metadata
1515
end
1616

17-
it "should should include the IndirectionHooks module in its indirection" do
18-
Puppet::FileServing::Metadata.indirection.singleton_class.included_modules.should include(Puppet::FileServing::IndirectionHooks)
19-
end
20-
2117
it "should have a method that triggers attribute collection" do
2218
Puppet::FileServing::Metadata.new(foobar).should respond_to(:collect)
2319
end

Diff for: spec/unit/file_serving/indirection_hooks_spec.rb renamed to spec/unit/file_serving/terminus_selector_spec.rb

+10-11
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,30 @@
11
#!/usr/bin/env rspec
22
require 'spec_helper'
33

4-
require 'puppet/file_serving/indirection_hooks'
4+
require 'puppet/file_serving/terminus_selector'
55

6-
describe Puppet::FileServing::IndirectionHooks do
6+
describe Puppet::FileServing::TerminusSelector do
77
before do
88
@object = Object.new
9-
@object.extend(Puppet::FileServing::IndirectionHooks)
9+
@object.extend(Puppet::FileServing::TerminusSelector)
1010

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

1414
describe "when being used to select termini" do
1515
it "should return :file if the request key is fully qualified" do
1616
@request.expects(:key).returns File.expand_path('/foo')
17-
@object.select_terminus(@request).should == :file
17+
@object.select(@request).should == :file
1818
end
1919

2020
it "should return :file if the URI protocol is set to 'file'" do
2121
@request.expects(:protocol).returns "file"
22-
@object.select_terminus(@request).should == :file
22+
@object.select(@request).should == :file
2323
end
2424

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

3030
describe "and the protocol is 'puppet'" do
@@ -35,24 +35,23 @@
3535
it "should choose :rest when a server is specified" do
3636
@request.stubs(:protocol).returns "puppet"
3737
@request.expects(:server).returns "foo"
38-
@object.select_terminus(@request).should == :rest
38+
@object.select(@request).should == :rest
3939
end
4040

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

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

5252
@request.expects(:protocol).returns "puppet"
5353
@request.expects(:server).returns nil
54-
Puppet.settings.expects(:value).with(:name).returns "puppet"
55-
@object.select_terminus(@request).should == :file_server
54+
@object.select(@request).should == :file_server
5655
end
5756
end
5857
end

0 commit comments

Comments
 (0)