Final specs & code for CHEF-21

Two pending tests that I can't quite work out.

2 files changed, 165 insertions, 26 deletions

diff --git a/chef/lib/chef/daemon.rb b/chef/lib/chef/daemon.rb
index 967d49712d..0bd3746642 100644
--- a/chef/lib/chef/daemon.rb
+++ b/chef/lib/chef/daemon.rb
@@ -41,7 +41,7 @@ class Chef
             Process.setsid
             exit if fork
             change_privilege
-            Chef::Log.info("Forked, in #{Process.pid}")
+            Chef::Log.info("Forked, in #{Process.pid}. Priveleges: #{Process.euid} #{Process.egid}")
             File.umask 0000
             $stdin.reopen("/dev/null")
             $stdout.reopen("/dev/null", "a")
@@ -151,7 +151,7 @@ class Chef
    
         begin
           target_gid = Etc.getgrnam(group).gid
-        rescue ArgumentERror => e
+        rescue ArgumentError => e
           Chef.fatal!("Failed to get GID for group #{group}, does it exist? #{e.message}")
           return false
         end
@@ -163,7 +163,7 @@ class Chef
         end
         true
       rescue Errno::EPERM => e
-        Chef.fatal!("Permission denied when trying to change #{uid}:#{gid} to #{user}:#{group}. #{e.message}")
+        Chef.fatal!("Permission denied when trying to change #{uid}:#{gid} to #{target_uid}:#{target_gid}. #{e.message}")
       end
     end
   end
diff --git a/chef/spec/unit/daemon_spec.rb b/chef/spec/unit/daemon_spec.rb
index 3cd3dc13fe..5bf7f67282 100644
--- a/chef/spec/unit/daemon_spec.rb
+++ b/chef/spec/unit/daemon_spec.rb
@@ -18,28 +18,15 @@
 require File.expand_path(File.join(File.dirname(__FILE__), "..", "spec_helper"))
 
 describe Chef::Daemon do  
-=begin
-  # I don't think I can test daemonize, cause, well, it exits! SystemExit
-  # Feel free to re-enable this
-  describe "daemonize" do
-    it "should set @name" do
-     Chef::Daemon.should_receive(:name).with("chef-client")  
-     Chef::Daemon.daemonize("chef-client")
-    end
-    
-    it "should check for a pid file based on name" do
-      Chef::Daemon.should_receive(:running?)
-      Chef::Daemon.daemonize("chef-client")
-    end
-  end
-=end
   
-  describe "running?" do
+  describe ".running?" do
+    
     before do
       Chef::Daemon.name = "spec"
     end
     
     describe "when a pid file exists" do
+      
       before do
         Chef::Daemon.stub!(:pid_from_file).and_return(1337)
       end
@@ -48,9 +35,11 @@ describe Chef::Daemon do
         Process.should_receive(:kill).with(0, 1337)
         Chef::Daemon.running?
       end
+      
     end
     
     describe "when the pid file is nonexistent" do
+      
       before do
         Chef::Daemon.stub!(:pid_from_file).and_return(nil)
       end
@@ -58,12 +47,14 @@ describe Chef::Daemon do
       it "should return false" do
         Chef::Daemon.running?.should be_false
       end
+      
     end
   end
  
-  describe "pid_file" do
+  describe ".pid_file" do
     
     describe "when the pid_file option has been set" do
+      
       before do
         Chef::Config.stub!(:[]).with(:pid_file).and_return("/var/run/chef/chef-client.pid")
       end
@@ -74,6 +65,7 @@ describe Chef::Daemon do
     end
     
     describe "without the pid_file option set" do
+      
       before do
         Chef::Config.stub!(:[]).with(:pid_file).and_return(nil)
         Chef::Daemon.name = "chef-client"
@@ -82,32 +74,179 @@ describe Chef::Daemon do
       it "should return a valued based on @name" do
         Chef::Daemon.pid_file.should eql("/tmp/chef-client.pid")
       end
+      
     end
   end
   
-  describe "pid_from_file" do
+  describe ".pid_from_file" do
+    
     before do
       Chef::Config.stub!(:[]).with(:pid_file).and_return("/var/run/chef/chef-client.pid")
     end
+    
     it "should suck the pid out of pid_file" do
       File.should_receive(:read).with("/var/run/chef/chef-client.pid").and_return("1337")
       Chef::Daemon.pid_from_file
     end
   end
   
-  describe "save_pid_file" do
+  describe ".save_pid_file" do
+    
+    before do
+      Process.stub!(:pid).and_return(1337)
+      Chef::Config.stub!(:[]).with(:pid_file).and_return("/var/run/chef/chef-client.pid")
+      Chef.stub!(:fatal!).and_return(true)
+      @f_mock = mock(File, { :print => true, :close => true, :write => true })
+      File.stub!(:open).with("/var/run/chef/chef-client.pid", "w").and_yield(@f_mock)
+    end
+    
+    it "should try and create the parent directory" do
+      FileUtils.should_receive(:mkdir_p).with("/var/run/chef")
+      Chef::Daemon.save_pid_file
+    end
+    
+    it "should open the pid file for writing" do
+      File.should_receive(:open).with("/var/run/chef/chef-client.pid", "w")
+      Chef::Daemon.save_pid_file
+    end
+    
+    it "should write the pid, converted to string, to the pid file" do
+      @f_mock.should_receive(:write, "1337").once.and_return(true)
+      Chef::Daemon.save_pid_file
+    end
     
   end
   
-  describe "remove_pid_file" do
+  describe ".remove_pid_file" do
+    before do
+      Chef::Config.stub!(:[]).with(:pid_file).and_return("/var/run/chef/chef-client.pid")
+    end
     
+    describe "when the pid file exists" do
+      
+      before do
+        File.stub!(:exists?).with("/var/run/chef/chef-client.pid").and_return(true)
+      end
+      
+      it "should remove the file" do
+        FileUtils.should_receive(:rm).with("/var/run/chef/chef-client.pid")
+        Chef::Daemon.remove_pid_file
+      end
+      
+      
+    end
+    
+    describe "when the pid file does not exist" do
+      
+      before do
+        File.stub!(:exists?).with("/var/run/chef/chef-client.pid").and_return(false)
+      end
+      
+      it "should not remove the file" do
+        FileUtils.should_not_receive(:rm)
+        Chef::Daemon.remove_pid_file
+      end     
+       
+    end
   end
   
-  describe "change_privilege" do
+  describe ".change_privilege" do
     
+    before do
+      Chef.stub!(:fatal!).and_return(true)
+      Chef::Config.stub!(:[]).with(:user).and_return("aj")
+    end
+    
+    describe "when the user and group options are supplied" do
+      
+      before do
+        Chef::Config.stub!(:[]).with(:group).and_return("staff")
+      end
+      
+      it "should log an appropriate info message" do
+        Chef::Log.should_receive(:info, "About to change privilege to aj:staff")
+        Chef::Daemon.change_privilege
+      end
+      
+      it "should call _change_privilege with the user and group" do
+        Chef::Daemon.should_receive(:_change_privilege).with("aj", "staff")
+        Chef::Daemon.change_privilege
+      end
+    end
+    
+    describe "when just the user option is supplied" do
+      before do
+        Chef::Config.stub!(:[]).with(:group).and_return(nil)
+      end
+            
+      it "should log an appropriate info message" do
+        Chef::Log.should_receive(:info, "About to change privilege to aj")
+        Chef::Daemon.change_privilege
+      end
+      
+      it "should call _change_privilege with just the user" do
+        Chef::Daemon.should_receive(:_change_privilege).with("aj")
+        Chef::Daemon.change_privilege
+      end
+    end
   end
   
-  describe "_change_privilege" do
+  describe "._change_privilege" do
     
-  end
+    before do
+      Process.stub!(:euid).and_return(0)
+      Process.stub!(:egid).and_return(0)
+      
+      Process::UID.stub!(:change_privilege).and_return(nil)
+      Process::GID.stub!(:change_privilege).and_return(nil)
+      
+      @pw_user = mock("Struct::Passwd", :null_object => true, :uid => 501)
+      @pw_group = mock("Struct::Group", :null_object => true, :gid => 20)
+      
+      Etc.stub!(:getpwnam).and_return(@pw_user)
+      Etc.stub!(:getgrnam).and_return(@pw_group)
+    end
+
+    describe "with sufficient privileges" do  
+      before do
+        Process.stub!(:euid).and_return(0)
+        Process.stub!(:egid).and_return(0)
+      end
+     
+      it "should initialize the supplemental group list" do
+        Process.should_receive(:initgroups).with("aj", 20)
+        Chef::Daemon._change_privilege("aj")
+      end
+   
+      it "should attempt to change the process GID"
+      it "should attempt to change the process UID"
+
+# I'm not sure how we can test these - requires root privileges?..
+=begin
+      it "should attempt to change the process GID" do
+        Process::GID.should_receive(:change_privilege).with(20).and_return(20)
+        Chef::Daemon._change_privilege("aj")
+      end
+      
+      it "should attempt to change the process UID" do
+        Process::UID.should_receive(:change_privilege).with(501).and_return(501)
+        Chef::Daemon._change_privilege("aj")
+      end
+=end
+    end
+    
+    describe "with insufficient privileges" do
+      before do
+        Process.stub!(:euid).and_return(999)
+        Process.stub!(:egid).and_return(999)
+      end
+      
+      it "should log an appropriate error message and fail miserably" do
+        Chef.should_receive(:fatal!).with("Permission denied when trying to change 999:999 to 501:20. Operation not permitted")
+        Chef::Daemon._change_privilege("aj")
+      end
+    end
+
+  end  
+
 end
\ No newline at end of file