Fixing #3791 - client environment is used

Node#environment wasn't being set correctly, in
that it had to have the right answer out of the gate
or it was never corrected.

It was lazy-binding in 0.25 but I managed to make
it no longer that way.  This resulted in the environment
basically not being set during compilation, so the default
server environment was always used.

Signed-off-by: Luke Kanies <luke@puppetlabs.com>

2 files changed, 55 insertions, 21 deletions

diff --git a/lib/puppet/node.rb b/lib/puppet/node.rb
index 721c9f586..5115e7fb1 100644
--- a/lib/puppet/node.rb
+++ b/lib/puppet/node.rb
@@ -9,13 +9,28 @@ class Puppet::Node
     # the node sources.
     extend Puppet::Indirector
 
+    # Adds the environment getter and setter, with some instance/string conversion
+    include Puppet::Node::Environment::Helper
+
     # Use the node source as the indirection terminus.
     indirects :node, :terminus_setting => :node_terminus, :doc => "Where to find node information.
         A node is composed of its name, its facts, and its environment."
 
-    attr_accessor :name, :classes, :source, :ipaddress, :environment
+    attr_accessor :name, :classes, :source, :ipaddress
     attr_reader :time, :parameters
 
+    def environment
+        return super if @environment
+
+        if env = parameters["environment"]
+            self.environment = env
+            return super
+        end
+
+        # Else, return the default
+        Puppet::Node::Environment.new
+    end
+
     def initialize(name, options = {})
         unless name
             raise ArgumentError, "Node names cannot be nil"
@@ -34,8 +49,9 @@ class Puppet::Node
 
         @parameters = options[:parameters] || {}
 
-        env = options[:environment] || @parameters[:environment] || @parameters["environment"] || Puppet::Node::Environment.new
-        @environment = env.is_a?(String) ? Puppet::Node::Environment.new(env) : env
+        if env = options[:environment]
+            self.environment = env
+        end
 
         @time = Time.now
     end
diff --git a/spec/unit/node.rb b/spec/unit/node.rb
index c2350da6b..e0b4530e8 100755
--- a/spec/unit/node.rb
+++ b/spec/unit/node.rb
@@ -2,6 +2,42 @@
 
 require File.dirname(__FILE__) + '/../spec_helper'
 
+describe Puppet::Node do
+    describe "when managing its environment" do
+        it "should use any set environment" do
+            Puppet::Node.new("foo", :environment => "bar").environment.name.should == :bar
+        end
+
+        it "should support providing an actual environment instance" do
+            Puppet::Node.new("foo", :environment => Puppet::Node::Environment.new(:bar)).environment.name.should == :bar
+        end
+
+        it "should determine its environment from its parameters if no environment is set" do
+            Puppet::Node.new("foo", :parameters => {"environment" => :bar}).environment.name.should == :bar
+        end
+
+        it "should use the default environment if no environment is provided" do
+            Puppet::Node.new("foo").environment.name.should == Puppet::Node::Environment.new.name
+        end
+
+        it "should always return an environment instance rather than a string" do
+            Puppet::Node.new("foo").environment.should be_instance_of(Puppet::Node::Environment)
+        end
+
+        it "should allow the environment to be set after initialization" do
+            node = Puppet::Node.new("foo")
+            node.environment = :bar
+            node.environment.name.should == :bar
+        end
+
+        it "should allow its environment to be set by parameters after initialization" do
+            node = Puppet::Node.new("foo")
+            node.parameters["environment"] = :bar
+            node.environment.name.should == :bar
+        end
+    end
+end
+
 describe Puppet::Node, "when initializing" do
     before do
         @node = Puppet::Node.new("testnode")
@@ -43,24 +79,6 @@ describe Puppet::Node, "when initializing" do
         @node = Puppet::Node.new("testing", :classes => "myclass")
         @node.classes.should == ["myclass"]
     end
-
-    it "should use any specified environment" do
-        env = Puppet::Node::Environment.new("foo")
-
-        Puppet::Node.new("testnode", :environment => env).environment.should equal(env)
-    end
-
-    it "should convert an environment specified as a string into an Environment instance" do
-        Puppet::Node.new("testnode", :environment => "foo").environment.should be_instance_of(Puppet::Node::Environment)
-    end
-
-    it "should return the 'environment' parameter if present and there is no explicit environment" do
-        Puppet::Node.new("testnode", :parameters => {"environment" => "two"}).environment.name.should == Puppet::Node::Environment.new("two").name
-    end
-
-    it "should use the default environment if there is no environment fact nor explicit environment" do
-        @node.environment.name.should == Puppet::Node::Environment.new.name
-    end
 end
 
 describe Puppet::Node, "when merging facts" do