Fixing #911 and #912 -- there's a default environment (development)

and you have to specify the valid environments for your site.

4 files changed, 137 insertions, 4 deletions

diff --git a/CHANGELOG b/CHANGELOG
index cc999c1e8..040f9a176 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,3 +1,6 @@
+    You now must specify an environment and you are required to specify
+    the valid environments for your site. (#911) 
+
     Certificates now always specify a subjectAltName, but it defaults
     to '*', meaning that it doesn't require DNS names to match.  You
     can override that behaviour by specifying a value for
diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb
index a3991bbaa..da830952a 100644
--- a/lib/puppet/defaults.rb
+++ b/lib/puppet/defaults.rb
@@ -127,10 +127,15 @@ module Puppet
             namespaces and methods.  This can be used as a coarse-grained
             authorization system for both ``puppetd`` and ``puppetmasterd``."
         ],
-        :environment => ["", "The environment Puppet is running in.  For clients (e.g., ``puppetd``) this
-            determines the environment itself, which is used to find modules and much more.  For
-            servers (i.e., ``puppetmasterd``) this provides the default environment for nodes we
-            know nothing about."],
+        :environments => ["production,development", "The valid environments for Puppet clients.
+            This is more useful as a server-side setting than client, but any
+            environment chosen must be in this list.  Values should be
+            separated by a comma."],
+        :environment => ["development", "The environment Puppet is running in.  For clients
+            (e.g., ``puppetd``) this determines the environment itself, which
+            is used to find modules and much more.  For servers (i.e.,
+            ``puppetmasterd``) this provides the default environment for nodes
+            we know nothing about."],
         :diff_args => ["", "Which arguments to pass to the diff command when printing differences between files."],
         :diff => ["diff", "Which diff command to use when printing differences between files."],
         :show_diff => [false, "Whether to print a contextual diff when files are being replaced.  The diff
diff --git a/lib/puppet/node/environment.rb b/lib/puppet/node/environment.rb
new file mode 100644
index 000000000..ada7f4eea
--- /dev/null
+++ b/lib/puppet/node/environment.rb
@@ -0,0 +1,45 @@
+# Model the environment that a node can operate in.  This class just
+# provides a simple wrapper for the functionality around environments.
+class Puppet::Node::Environment
+    # Return the list of valid environments.  Just looks them up in
+    # the settings.
+    def self.valid
+        Puppet.settings.value(:environments).split(",").collect { |e| e.to_sym }
+    end
+
+    # Is the provided environment valid?
+    def self.valid?(name)
+        valid.include?(name.to_sym)
+    end
+
+    @seen = {}
+
+    # Return an existing environment instance, or create a new one,
+    # validating the environment name.
+    def self.new(name = nil)
+        name ||= Puppet.settings.value(:environment)
+
+        raise ArgumentError, "Environment name must be specified" unless name
+
+        raise(ArgumentError, "'%s' is not a valid environment" % name) unless valid?(name)
+
+        symbol = name.to_sym
+
+        return @seen[symbol] if @seen[symbol]
+
+        obj = self.allocate
+        obj.send :initialize, symbol
+        @seen[symbol] = obj
+    end
+
+    attr_reader :name
+
+    # Return an environment-specific setting.
+    def [](param)
+        Puppet.settings.value(param, self.name)
+    end
+
+    def initialize(name)
+        @name = name
+    end
+end
diff --git a/spec/unit/node/environment.rb b/spec/unit/node/environment.rb
new file mode 100755
index 000000000..cccff7b97
--- /dev/null
+++ b/spec/unit/node/environment.rb
@@ -0,0 +1,80 @@
+#!/usr/bin/env ruby
+
+require File.dirname(__FILE__) + '/../../spec_helper'
+
+require 'puppet/node/environment'
+
+describe Puppet::Node::Environment do
+    it "should provide a list of valid environments" do
+        Puppet::Node::Environment.valid.should be_instance_of(Array)
+    end
+
+    it "should determine its list of valid environments from splitting the :environments setting on commas" do
+        Puppet.settings.stubs(:value).with(:environments).returns("one,two")
+        Puppet::Node::Environment.valid.collect { |e| e.to_s }.sort.should == %w{one two}.sort
+    end
+
+    it "should not use an environment when determining the list of valid environments" do
+        Puppet.settings.expects(:value).with(:environments).returns("one,two")
+        Puppet::Node::Environment.valid
+    end
+
+    it "should provide a means of identifying invalid environments" do
+        Puppet.settings.expects(:value).with(:environments).returns("one,two")
+        Puppet::Node::Environment.valid?(:three).should be_false
+    end
+
+    it "should provide a means of identifying valid environments" do
+        Puppet.settings.expects(:value).with(:environments).returns("one,two")
+        Puppet::Node::Environment.valid?(:one).should be_true
+    end
+
+    it "should use the default environment if no name is provided while initializing an environment" do
+        Puppet.settings.expects(:value).with(:environments).returns("one,two")
+        Puppet.settings.expects(:value).with(:environment).returns("one")
+        Puppet::Node::Environment.new().name.should == :one
+    end
+
+    it "should treat environment instances as singletons" do
+        Puppet.settings.stubs(:value).with(:environments).returns("one")
+        Puppet::Node::Environment.new("one").should equal(Puppet::Node::Environment.new("one"))
+    end
+
+    it "should treat an environment specified as names or strings as equivalent" do
+        Puppet.settings.stubs(:value).with(:environments).returns("one")
+        Puppet::Node::Environment.new(:one).should equal(Puppet::Node::Environment.new("one"))
+    end
+
+    it "should fail if an invalid environment instance is asked for" do
+        Puppet.settings.stubs(:value).with(:environments).returns("one,two")
+        proc { Puppet::Node::Environment.new("three") }.should raise_error(ArgumentError)
+    end
+
+    it "should fail if a no-longer-valid environment instance is asked for" do
+        Puppet.settings.expects(:value).with(:environments).returns("one")
+        Puppet::Node::Environment.new("one")
+        Puppet.settings.expects(:value).with(:environments).returns("two")
+        proc { Puppet::Node::Environment.new("one") }.should raise_error(ArgumentError)
+    end
+end
+
+describe Puppet::Node::Environment, " when modeling a specific environment" do
+    before do
+        Puppet.settings.expects(:value).with(:environments).returns("testing")
+    end
+
+    it "should have a method for returning the environment name" do
+        Puppet::Node::Environment.new("testing").name.should == :testing
+    end
+
+    it "should provide an array-like accessor method for returning any environment-specific setting" do
+        env = Puppet::Node::Environment.new("testing")
+        env.should respond_to(:[])
+    end
+
+    it "should ask the Puppet settings instance for the setting qualified with the environment name" do
+        Puppet.settings.expects(:value).with("myvar", :testing).returns("myval")
+        env = Puppet::Node::Environment.new("testing")
+        env["myvar"].should == "myval"
+    end
+end