Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

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...
commit c3c7462e4066bf3a563987a402bf3ddf278bcd87 1 parent d174a9f
Patrick Carlisle authored June 21, 2012 nicklewis committed June 27, 2012
3  lib/puppet/application/master.rb
@@ -141,6 +141,9 @@ def setup
141 141
     # Cache our nodes in yaml.  Currently not configurable.
142 142
     Puppet::Node.cache_class = :yaml
143 143
 
  144
+    Puppet::FileServing::Content.indirection.terminus_class = :file_server
  145
+    Puppet::FileServing::Metadata.indirection.terminus_class = :file_server
  146
+
144 147
     # Configure all of the SSL stuff.
145 148
     if Puppet::SSL::CertificateAuthority.ca?
146 149
       Puppet::SSL::Host.ca_location = :local
2  lib/puppet/file_serving/configuration.rb
@@ -70,7 +70,7 @@ def split_path(request)
70 70
 
71 71
     mount_name, path = request.key.split(File::Separator, 2)
72 72
 
73  
-    raise(ArgumentError, "Cannot find file: Invalid path '#{mount_name}'") unless mount_name =~ %r{^[-\w]+$}
  73
+    raise(ArgumentError, "Cannot find file: Invalid mount '#{mount_name}'") unless mount_name =~ %r{^[-\w]+$}
74 74
 
75 75
     return nil unless mount = find_mount(mount_name, request.environment)
76 76
     if mount.name == "modules" and mount_name != "modules"
3  lib/puppet/file_serving/content.rb
@@ -5,14 +5,13 @@
5 5
 require 'puppet/indirector'
6 6
 require 'puppet/file_serving'
7 7
 require 'puppet/file_serving/base'
8  
-require 'puppet/file_serving/indirection_hooks'
9 8
 
10 9
 # A class that handles retrieving file contents.
11 10
 # It only reads the file when its content is specifically
12 11
 # asked for.
13 12
 class Puppet::FileServing::Content < Puppet::FileServing::Base
14 13
   extend Puppet::Indirector
15  
-  indirects :file_content, :extend => Puppet::FileServing::IndirectionHooks
  14
+  indirects :file_content, :terminus_class => :selector
16 15
 
17 16
   attr_writer :content
18 17
 
3  lib/puppet/file_serving/metadata.rb
@@ -7,7 +7,6 @@
7 7
 require 'puppet/file_serving'
8 8
 require 'puppet/file_serving/base'
9 9
 require 'puppet/util/checksums'
10  
-require 'puppet/file_serving/indirection_hooks'
11 10
 
12 11
 # A class that handles retrieving file metadata.
13 12
 class Puppet::FileServing::Metadata < Puppet::FileServing::Base
@@ -15,7 +14,7 @@ class Puppet::FileServing::Metadata < Puppet::FileServing::Base
15 14
   include Puppet::Util::Checksums
16 15
 
17 16
   extend Puppet::Indirector
18  
-  indirects :file_metadata, :extend => Puppet::FileServing::IndirectionHooks
  17
+  indirects :file_metadata, :terminus_class => :selector
19 18
 
20 19
   attr_reader :path, :owner, :group, :mode, :checksum_type, :checksum, :ftype, :destination
21 20
 
5  lib/puppet/file_serving/indirection_hooks.rb → lib/puppet/file_serving/terminus_selector.rb
@@ -8,11 +8,10 @@
8 8
 # This module is used to pick the appropriate terminus
9 9
 # in file-serving indirections.  This is necessary because
10 10
 # the terminus varies based on the URI asked for.
11  
-module Puppet::FileServing::IndirectionHooks
  11
+module Puppet::FileServing::TerminusSelector
12 12
   PROTOCOL_MAP = {"puppet" => :rest, "file" => :file}
13 13
 
14  
-  # Pick an appropriate terminus based on the protocol.
15  
-  def select_terminus(request)
  14
+  def select(request)
16 15
     # We rely on the request's parsing of the URI.
17 16
 
18 17
     # Short-circuit to :file if it's a fully-qualified path or specifies a 'file' protocol.
29  lib/puppet/indirector/file_content/selector.rb
... ...
@@ -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
29  lib/puppet/indirector/file_metadata/selector.rb
... ...
@@ -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
4  spec/shared_behaviours/file_serving.rb
@@ -9,9 +9,7 @@
9 9
 
10 10
     # It appears that the mocking somehow interferes with the caching subsystem.
11 11
     # This mock somehow causes another terminus to get generated.
12  
-    term = @indirection.terminus(:rest)
13  
-    @indirection.stubs(:terminus).with(:rest).returns term
14  
-    term.expects(:find)
  12
+    @indirection.terminus(:rest).expects(:find)
15 13
     @test_class.find(uri)
16 14
   end
17 15
 
4  spec/unit/file_serving/content_spec.rb
@@ -13,10 +13,6 @@
13 13
     Puppet::FileServing::Content.indirection.name.should == :file_content
14 14
   end
15 15
 
16  
-  it "should should include the IndirectionHooks module in its indirection" do
17  
-    Puppet::FileServing::Content.indirection.singleton_class.included_modules.should include(Puppet::FileServing::IndirectionHooks)
18  
-  end
19  
-
20 16
   it "should only support the raw format" do
21 17
     Puppet::FileServing::Content.supported_formats.should == [:raw]
22 18
   end
4  spec/unit/file_serving/metadata_spec.rb
@@ -13,10 +13,6 @@
13 13
     Puppet::FileServing::Metadata.indirection.name.should == :file_metadata
14 14
   end
15 15
 
16  
-  it "should should include the IndirectionHooks module in its indirection" do
17  
-    Puppet::FileServing::Metadata.indirection.singleton_class.included_modules.should include(Puppet::FileServing::IndirectionHooks)
18  
-  end
19  
-
20 16
   it "should have a method that triggers attribute collection" do
21 17
     Puppet::FileServing::Metadata.new("/foo/bar").should respond_to(:collect)
22 18
   end
23  spec/unit/file_serving/indirection_hooks_spec.rb → spec/unit/file_serving/terminus_selector_spec.rb
@@ -5,30 +5,30 @@
5 5
 
6 6
 require File.dirname(__FILE__) + '/../../spec_helper'
7 7
 
8  
-require 'puppet/file_serving/indirection_hooks'
  8
+require 'puppet/file_serving/terminus_selector'
9 9
 
10  
-describe Puppet::FileServing::IndirectionHooks do
  10
+describe Puppet::FileServing::TerminusSelector do
11 11
   before do
12 12
     @object = Object.new
13  
-    @object.extend(Puppet::FileServing::IndirectionHooks)
  13
+    @object.extend(Puppet::FileServing::TerminusSelector)
14 14
 
15 15
     @request = stub 'request', :key => "mymod/myfile", :options => {:node => "whatever"}, :server => nil, :protocol => nil
16 16
   end
17 17
 
18 18
   describe "when being used to select termini" do
19 19
     it "should return :file if the request key is fully qualified" do
20  
-      @request.expects(:key).returns "#{File::SEPARATOR}foo"
21  
-      @object.select_terminus(@request).should == :file
  20
+      @request.expects(:key).returns File.expand_path('/foo')
  21
+      @object.select(@request).should == :file
22 22
     end
23 23
 
24 24
     it "should return :file if the URI protocol is set to 'file'" do
25 25
       @request.expects(:protocol).returns "file"
26  
-      @object.select_terminus(@request).should == :file
  26
+      @object.select(@request).should == :file
27 27
     end
28 28
 
29 29
     it "should fail when a protocol other than :puppet or :file is used" do
30 30
       @request.stubs(:protocol).returns "http"
31  
-      proc { @object.select_terminus(@request) }.should raise_error(ArgumentError)
  31
+      proc { @object.select(@request) }.should raise_error(ArgumentError)
32 32
     end
33 33
 
34 34
     describe "and the protocol is 'puppet'" do
@@ -39,15 +39,15 @@
39 39
       it "should choose :rest when a server is specified" do
40 40
         @request.stubs(:protocol).returns "puppet"
41 41
         @request.expects(:server).returns "foo"
42  
-        @object.select_terminus(@request).should == :rest
  42
+        @object.select(@request).should == :rest
43 43
       end
44 44
 
45 45
       # This is so a given file location works when bootstrapping with no server.
46 46
       it "should choose :rest when the Settings name isn't 'puppet'" do
47 47
         @request.stubs(:protocol).returns "puppet"
48  
-        @request.stubs(:server).returns "foo"
  48
+        # We have to stub this because we can't set name
49 49
         Puppet.settings.stubs(:value).with(:name).returns "foo"
50  
-        @object.select_terminus(@request).should == :rest
  50
+        @object.select(@request).should == :rest
51 51
       end
52 52
 
53 53
       it "should choose :file_server when the settings name is 'puppet' and no server is specified" do
@@ -55,8 +55,7 @@
55 55
 
56 56
         @request.expects(:protocol).returns "puppet"
57 57
         @request.expects(:server).returns nil
58  
-        Puppet.settings.expects(:value).with(:name).returns "puppet"
59  
-        @object.select_terminus(@request).should == :file_server
  58
+        @object.select(@request).should == :file_server
60 59
       end
61 60
     end
62 61
   end

0 notes on commit c3c7462

Please sign in to comment.
Something went wrong with that request. Please try again.