CHEF-174 and cleanup around treating the mode as string or int, and converting to octal when necessary

also added feature tests

Author: Christopher Brown

chef-server/config/dependencies.rb

@@ -1,5 +1,5 @@
 # dependencies are generated using a strict version, don't forget to edit the dependency versions when upgrading.
-merb_gems_version = "1.0.10"
+merb_gems_version = "1.0.11"
 
 # For more information about each component, please read http://wiki.merbivore.com/faqs/merb_components
 dependency "merb-core", merb_gems_version 

chef/lib/chef/provider/file.rb

@@ -37,8 +37,12 @@ def negative_complement(big)
         end
         big
       end
+      
+      def octal_mode(mode)
+        ((mode.respond_to?(:oct) ? mode.oct : mode.to_i) & 007777)
+      end
 
-      private :negative_complement
+      private :negative_complement, :octal_mode
 
       def load_current_resource
         @current_resource = Chef::Resource::File.new(@new_resource.name)
@@ -47,7 +51,7 @@ def load_current_resource
           cstats = ::File.stat(@current_resource.path)
           @current_resource.owner(cstats.uid)
           @current_resource.group(cstats.gid)
-          @current_resource.mode("%o" % (cstats.mode & 007777))
+          @current_resource.mode(octal_mode(cstats.mode))
           @current_resource.checksum(checksum(@current_resource.path))
         end
         @current_resource
@@ -104,17 +108,17 @@ def set_group
       def compare_mode
         case @new_resource.mode
         when /^\d+$/, Integer
-          real_mode = sprintf("%o" % (@new_resource.mode & 007777))
-          real_mode.to_i == @current_resource.mode.to_i
+          octal_mode(@new_resource.mode) == octal_mode(@current_resource.mode)
         else
           false
         end
       end
 
       def set_mode
         unless compare_mode && @new_resource.mode != nil
-          Chef::Log.info("Setting mode to #{sprintf("%o" % (@new_resource.mode & 007777))} for #{@new_resource}")
-          ::File.chmod(@new_resource.mode.to_i, @new_resource.path)
+          Chef::Log.info("Setting mode to #{sprintf("%o" % octal_mode(@new_resource.mode))} for #{@new_resource}")
+          # CHEF-174, bad mojo around treating integers as octal.  If a string is passed, we try to do the "right" thing
+          ::File.chmod(octal_mode(@new_resource.mode), @new_resource.path)
           @new_resource.updated = true
         end
       end
@@ -125,9 +129,9 @@ def action_create
           ::File.open(@new_resource.path, "w+") { |f| }
           @new_resource.updated = true
         end
-        set_owner if @new_resource.owner != nil
-        set_group if @new_resource.group != nil
-        set_mode if @new_resource.mode != nil
+        set_owner unless @new_resource.owner.nil?
+        set_group unless @new_resource.group.nil?
+        set_mode unless @new_resource.mode.nil?
       end
 
       def action_create_if_missing

chef/lib/chef/resource/file.rb

@@ -59,7 +59,7 @@ def mode(arg=nil)
         set_or_return(
           :mode,
           arg,
-          :regex => /^\d{3,4}$/
+          :regex => /^0?\d{3,4}$/
         )
       end
 
@@ -81,4 +81,4 @@ def path(arg=nil)
 
     end
   end
-end
+end

features/data/cookbooks/manage_files/recipes/set_the_accessibility_of_a_created_file.rb

@@ -0,0 +1,61 @@
+#
+# Cookbook Name:: files
+# Recipe:: default
+#
+# Copyright 2009, Opscode
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+# 
+#     http://www.apache.org/licenses/LICENSE-2.0
+# 
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+file "#{node[:tmpdir]}/octal0644.txt" do
+  owner 'nobody'
+  mode 0644
+  action :create
+end
+
+file "#{node[:tmpdir]}/octal2644.txt" do
+  owner 'nobody'
+  mode 02644
+  action :create
+end
+
+file "#{node[:tmpdir]}/decimal644.txt" do
+  owner 'nobody'
+  mode 644
+  action :create
+end
+
+file "#{node[:tmpdir]}/decimal2644.txt" do
+  owner 'nobody'
+  mode 2644
+  action :create
+end
+
+file "#{node[:tmpdir]}/string644.txt" do
+  owner 'nobody'
+  mode "644"
+  action :create
+end
+
+file "#{node[:tmpdir]}/string0644.txt" do
+  owner 'nobody'
+  mode "0644"
+  action :create
+end
+
+file "#{node[:tmpdir]}/string2644.txt" do
+  owner 'nobody'
+  mode "2644"
+  action :create
+end
+

features/manage_files.feature

@@ -40,4 +40,18 @@ Feature: Manage Files
      Then the run should exit '0'
       And the atime of 'touch_test.txt' should be different
       And the mtime of 'touch_test.txt' should be different
-      
+ 
+  Scenario: Set the accessibility of a created file
+    Given a validated node
+      And it includes the recipe 'manage_files::set_the_accessibility_of_a_created_file'
+     When I run the chef-client
+     Then the run should exit '0'
+      And the file named 'octal0644.txt' should have octal mode '0644'
+      And the file named 'octal2644.txt' should have octal mode '2644'
+      And the file named 'decimal644.txt' should have decimal mode '644'
+      And the file named 'decimal2644.txt' should have decimal mode '2644'
+      And the file named 'string644.txt' should have octal mode '644'
+      And the file named 'string0644.txt' should have octal mode '0644'
+      And the file named 'string2644.txt' should have octal mode '2644'
+
+

features/steps/files.rb

@@ -80,3 +80,14 @@
   cstats = File.stat(File.join(tmpdir, filename))
   cstats.uid.should == uid
 end
+
+Then /^the file named '(.+)' should have octal mode '(.+)'$/ do |filename, expected_mode|
+  cstats = File.stat(File.join(tmpdir, filename))
+  (cstats.mode & 007777).should == expected_mode.oct
+end
+
+Then /^the file named '(.+)' should have decimal mode '(.+)'$/ do |filename, expected_mode|
+  cstats = File.stat(File.join(tmpdir, filename))
+  (cstats.mode & 007777).should == expected_mode.to_i
+end
+