Please review pull request #721: Add default_file_terminus setting opened by (pcarlisle)
Description:
This setting is used to determine the default source of files accessed with a puppet: url and no server specified, e.g. puppet:///path/to/file. Before now, this was done by checking if Puppet[:name] was apply. Since Puppet[:name] is removed and we don't want application specific logic spread throughout the code, we are adding a setting specific to this particular behavior. The apply application sets a default value of file_terminus, which retains the previous behavior of apply. Otherwise the default value is rest, causing Puppet[:server] to be used to retrieve the file.
- Opened: Thu Apr 26 20:49:32 UTC 2012
- Based on: puppetlabs:master (df8db7c6b298fc02637f936b8b0609ee8ca2bb0a)
- Requested merge: pcarlisle:file_terminus (48a768a2e33cdc0868ca8dfc9eb86c8fda2f5fcd)
Diff follows:
diff --git a/lib/puppet/application/apply.rb b/lib/puppet/application/apply.rb
index 647bba5..0fd054e 100644
--- a/lib/puppet/application/apply.rb
+++ b/lib/puppet/application/apply.rb
@@ -145,6 +145,13 @@ def help
HELP
end
+ def app_defaults
+ super.merge({
+ :default_file_terminus => "file_server",
+ :pluginsource => "puppet:///plugins"
+ })
+ end
+
def run_command
if options[:catalog]
apply
@@ -251,9 +258,6 @@ def setup
elsif options[:verbose]
Puppet::Util::Log.level = :info
end
-
- # Make pluginsync local
- Puppet[:pluginsource] = 'puppet:///plugins'
end
private
diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb
index 5f01c94..c233d15 100644
--- a/lib/puppet/defaults.rb
+++ b/lib/puppet/defaults.rb
@@ -248,6 +248,13 @@ module Puppet
:default => "$facts_terminus",
:desc => "Should usually be the same as the facts terminus",
},
+ :default_file_terminus => {
+ :default => "rest",
+ :desc => "The default source for files if no server is given in a
+ uri, e.g. puppet:///file. The default of `rest` causes the file to be
+ retrieved using the `server` setting. When running `apply` the default
+ is `file_server`, causing requests to be filled locally."
+ },
:httplog => {
:default => "$logdir/http.log",
:type => :file,
diff --git a/lib/puppet/file_serving/indirection_hooks.rb b/lib/puppet/file_serving/indirection_hooks.rb
index 735c1dd..9c97246 100644
--- a/lib/puppet/file_serving/indirection_hooks.rb
+++ b/lib/puppet/file_serving/indirection_hooks.rb
@@ -6,33 +6,28 @@
# in file-serving indirections. This is necessary because
# the terminus varies based on the URI asked for.
module Puppet::FileServing::IndirectionHooks
- PROTOCOL_MAP = {"puppet" => :rest, "file" => :file}
-
# Pick an appropriate terminus based on the protocol.
def select_terminus(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.
- return PROTOCOL_MAP["file"] if Puppet::Util.absolute_path?(request.key)
- return PROTOCOL_MAP["file"] if request.protocol == "file"
-
- # TODO: this seems like an incredibly fragile way to determine our protocol. (In fact, I broke it pretty nicely
- # during my changes relating to settings/defaults.) Nick said he's going to fix it. :)
- # --cprice 2012-03-14
- # TODO: we are special-casing both "puppet" and "apply" here... this shows up in tests as well
- # (file_serving.rb in shared_behaviors). I don't think we need to special-case ":puppet" any longer,
- # as I am not aware of any code path where the Puppet.settings[:name] could end up having that as a value.
-
- # We're heading over the wire the protocol is 'puppet' and we've got a server name or we're not named 'apply' or 'puppet'
- if request.protocol == "puppet" and (request.server or ![:puppet, :apply].include?(Puppet.settings[:name]))
- return PROTOCOL_MAP["puppet"]
+ if Puppet::Util.absolute_path?(request.key)
+ return :file
end
- if request.protocol and PROTOCOL_MAP[request.protocol].nil?
- raise(ArgumentError, "URI protocol '#{request.protocol}' is not currently supported for file serving")
+ case request.protocol
+ when "file"
+ :file
+ when "puppet"
+ if request.server
+ :rest
+ else
+ Puppet[:default_file_terminus].to_sym
+ end
+ when nil
+ :file_server
+ else
+ raise ArgumentError, "URI protocol '#{request.protocol}' is not currently supported for file serving"
end
-
- # If we're still here, we're using the file_server or modules.
- :file_server
end
end
diff --git a/lib/puppet/network/server.rb b/lib/puppet/network/server.rb
index 275eaf7..ae0a932 100644
--- a/lib/puppet/network/server.rb
+++ b/lib/puppet/network/server.rb
@@ -61,7 +61,6 @@ def initialize(args = {})
self.register(args[:handlers]) if args[:handlers]
# Make sure we have all of the directories we need to function.
- #Puppet.settings.use(:main, :ssl, Puppet[:name])
Puppet.settings.use(:main, :ssl, :application)
end
diff --git a/lib/puppet/test/test_helper.rb b/lib/puppet/test/test_helper.rb
index d561126..1761c13 100644
--- a/lib/puppet/test/test_helper.rb
+++ b/lib/puppet/test/test_helper.rb
@@ -131,7 +131,6 @@ def self.after_each_test()
def self.app_defaults_for_tests()
{
:run_mode => :user,
- :name => :apply,
:logdir => "/dev/null",
:confdir => "/dev/null",
:vardir => "/dev/null",
diff --git a/lib/puppet/type/file/content.rb b/lib/puppet/type/file/content.rb
index bd7030a..7a83cd1 100755
--- a/lib/puppet/type/file/content.rb
+++ b/lib/puppet/type/file/content.rb
@@ -168,14 +168,6 @@ def write(file)
}
end
- # TODO: this is another terrible, fragile means of determining whether or not to
- # make a web request... it makes me tempted to get rid of the ":name" setting
- # entirely... --cprice 2012-03-14
-
- def self.standalone?
- Puppet.settings[:name] == :apply
- end
-
# the content is munged so if it's a checksum source_or_content is nil
# unless the checksum indirectly comes from source
def each_chunk_from(source_or_content)
@@ -185,7 +177,7 @@ def each_chunk_from(source_or_content)
yield read_file_from_filebucket
elsif source_or_content.nil?
yield ''
- elsif self.class.standalone?
+ elsif Puppet[:default_file_terminus] == "file_server"
yield source_or_content.content
elsif source_or_content.local?
chunk_file_from_disk(source_or_content) { |chunk| yield chunk }
diff --git a/spec/shared_behaviours/file_serving.rb b/spec/shared_behaviours/file_serving.rb
index 7f35f8d..8bffa84 100755
--- a/spec/shared_behaviours/file_serving.rb
+++ b/spec/shared_behaviours/file_serving.rb
@@ -11,36 +11,18 @@
@indirection.find(uri)
end
- it "should use the rest terminus when the 'puppet' URI scheme is used, no host name is present, and the process name is not 'puppet' or 'apply'" do
+ it "should use the rest terminus when the 'puppet' URI scheme is used, no host name is present, and default_file_terminus is rest" do
uri = "puppet:///fakemod/my/file"
- Puppet.settings.stubs(:value).returns "foo"
- Puppet.settings.stubs(:value).with(:name).returns("puppetd")
- Puppet.settings.stubs(:value).with(:modulepath).returns("")
+ Puppet[:default_file_terminus] = "rest"
@indirection.terminus(:rest).expects(:find)
@indirection.find(uri)
end
-
- # TODO: This test may no longer be relevant. It's testing for scenarios when settings[:name] is set to "puppet",
- # and I am not aware of any code path where the Puppet.settings[:name] could still end up having that as a value.
- # --cprice 2012-03-14
- it "should use the file_server terminus when the 'puppet' URI scheme is used, no host name is present, and the process name is 'puppet'" do
- uri = "puppet:///fakemod/my/file"
- Puppet::Node::Environment.stubs(:new).returns(stub("env", :name => "testing", :module => nil, :modulepath => []))
- Puppet.settings.stubs(:value).returns ""
- Puppet.settings.stubs(:value).with(:name).returns(:puppet)
- Puppet.settings.stubs(:value).with(:fileserverconfig).returns("/whatever")
- @indirection.terminus(:file_server).expects(:find)
- @indirection.terminus(:file_server).stubs(:authorized?).returns(true)
- @indirection.find(uri)
- end
-
- it "should use the file_server terminus when the 'puppet' URI scheme is used, no host name is present, and the process name is 'apply'" do
+ it "should use the file_server terminus when the 'puppet' URI scheme is used, no host name is present, and default_file_terminus is file_server" do
uri = "puppet:///fakemod/my/file"
Puppet::Node::Environment.stubs(:new).returns(stub("env", :name => "testing", :module => nil, :modulepath => []))
- Puppet.settings.stubs(:value).returns ""
- Puppet.settings.stubs(:value).with(:name).returns(:apply)
- Puppet.settings.stubs(:value).with(:fileserverconfig).returns("/whatever")
+ Puppet[:default_file_terminus] = "file_server"
+ Puppet[:fileserverconfig] = "/whatever"
@indirection.terminus(:file_server).expects(:find)
@indirection.terminus(:file_server).stubs(:authorized?).returns(true)
@indirection.find(uri)
diff --git a/spec/unit/application/apply_spec.rb b/spec/unit/application/apply_spec.rb
index bea2b4c..3c27314 100755
--- a/spec/unit/application/apply_spec.rb
+++ b/spec/unit/application/apply_spec.rb
@@ -114,9 +114,12 @@
@apply.setup
end
- it "should set pluginsource to be local" do
- @apply.setup
- Puppet[:pluginsource].should == 'puppet:///plugins'
+ it "should set pluginsource with no hostname" do
+ @apply.app_defaults[:pluginsource].should == 'puppet:///plugins'
+ end
+
+ it "should set default_file_terminus to `file_server` to be local" do
+ @apply.app_defaults[:default_file_terminus].should == 'file_server'
end
end
diff --git a/spec/unit/file_serving/indirection_hooks_spec.rb b/spec/unit/file_serving/indirection_hooks_spec.rb
index 93e815e..c194c09 100755
--- a/spec/unit/file_serving/indirection_hooks_spec.rb
+++ b/spec/unit/file_serving/indirection_hooks_spec.rb
@@ -39,19 +39,18 @@
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
+ it "should choose :rest when default_file_terminus is rest" do
@request.stubs(:protocol).returns "puppet"
- @request.stubs(:server).returns "foo"
- Puppet.settings.stubs(:value).with(:name).returns "foo"
+ Puppet[:server] = 'localhost'
@object.select_terminus(@request).should == :rest
end
- it "should choose :file_server when the settings name is 'puppet' and no server is specified" do
+ it "should choose :file_server when default_file_terminus is file_server and no server is specified on the request" do
modules = mock 'modules'
@request.expects(:protocol).returns "puppet"
@request.expects(:server).returns nil
- Puppet.settings.expects(:value).with(:name).returns :puppet
+ Puppet[:default_file_terminus] = 'file_server'
@object.select_terminus(@request).should == :file_server
end
end
diff --git a/spec/unit/network/http/webrick_spec.rb b/spec/unit/network/http/webrick_spec.rb
index 8310ee0..b7340d6 100755
--- a/spec/unit/network/http/webrick_spec.rb
+++ b/spec/unit/network/http/webrick_spec.rb
@@ -125,9 +125,7 @@
File.stubs(:open).returns @filehandle
end
- it "should use the settings for :main, :ssl, and the process name" do
- #Puppet.settings.stubs(:value).with(:name).returns "myname"
- #Puppet.settings.expects(:use).with(:main, :ssl, "myname")
+ it "should use the settings for :main, :ssl, and :application" do
Puppet.settings.expects(:use).with(:main, :ssl, :application)
@server.setup_logger
diff --git a/spec/unit/network/server_spec.rb b/spec/unit/network/server_spec.rb
index 00fca42..6cc0785 100755
--- a/spec/unit/network/server_spec.rb
+++ b/spec/unit/network/server_spec.rb
@@ -104,7 +104,6 @@
end
it "should use the :application setting section" do
- #Puppet.settings.expects(:value).with(:name).returns "me"
Puppet.settings.expects(:use).with { |*args| args.include?(:application) }
@server = Puppet::Network::Server.new(:port => 31337)
diff --git a/spec/unit/type/file/content_spec.rb b/spec/unit/type/file/content_spec.rb
index 01e4939..74c0555 100755
--- a/spec/unit/type/file/content_spec.rb
+++ b/spec/unit/type/file/content_spec.rb
@@ -416,7 +416,7 @@
end
it "when running as puppet apply" do
- @content.class.expects(:standalone?).returns true
+ Puppet[:default_file_terminus] = "file_server"
source_or_content = stubs('source_or_content')
source_or_content.expects(:content).once.returns :whoo
@content.each_chunk_from(source_or_content) { |chunk| chunk.should == :whoo }
diff --git a/spec/unit/util/settings_spec.rb b/spec/unit/util/settings_spec.rb
index d8b3af2..57f94f4 100755
--- a/spec/unit/util/settings_spec.rb
+++ b/spec/unit/util/settings_spec.rb
@@ -264,10 +264,6 @@
@settings[:myval] = "memarg"
end
- it "should raise an error if we try to set 'name'" do
- lambda{ @settings[:name] = "foo" }.should raise_error(ArgumentError)
- end
-
it "should raise an error if we try to set 'run_mode'" do
lambda{ @settings[:run_mode] = "foo" }.should raise_error(ArgumentError)
end
-- You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to [email protected].
For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
