diff options
author | Lamont Granquist <lamont@scriptkiddie.org> | 2013-11-21 12:35:56 -0800 |
---|---|---|
committer | Lamont Granquist <lamont@scriptkiddie.org> | 2013-11-21 12:47:42 -0800 |
commit | aa946596728dfffec851e030e55bf427fb59a725 (patch) | |
tree | c5868d709255b92a3abf61926a8f13f3ada4da81 | |
parent | 06c0b8b13b326871710d4233b0cbc36744c57f79 (diff) | |
download | chef-aa946596728dfffec851e030e55bf427fb59a725.tar.gz |
back port of OC-10380 to 10-stable
-rw-r--r-- | chef/lib/chef/provider/cookbook_file.rb | 20 | ||||
-rw-r--r-- | chef/lib/chef/provider/directory.rb | 26 | ||||
-rw-r--r-- | chef/lib/chef/provider/file.rb | 40 | ||||
-rw-r--r-- | chef/lib/chef/provider/remote_file.rb | 10 | ||||
-rw-r--r-- | chef/lib/chef/provider/template.rb | 16 | ||||
-rw-r--r-- | chef/spec/unit/provider/file_spec.rb | 54 |
6 files changed, 99 insertions, 67 deletions
diff --git a/chef/lib/chef/provider/cookbook_file.rb b/chef/lib/chef/provider/cookbook_file.rb index 52539779ea..dd6274a3cd 100644 --- a/chef/lib/chef/provider/cookbook_file.rb +++ b/chef/lib/chef/provider/cookbook_file.rb @@ -33,7 +33,7 @@ class Chef end def action_create - if file_cache_location && content_stale? + if file_cache_location && content_stale? description = [] description << "create a new cookbook_file #{@new_resource.path}" description << diff_current(file_cache_location) @@ -83,22 +83,30 @@ class Chef # On the Windows platform, files in the temp directory # default to not inherit unless the new resource specifies rights of # some sort. Here we ensure that even when no rights are - # specified, the dacl's inheritance flag is set. + # specified, the dacl's inheritance flag is set. if Chef::Platform.windows? && @new_resource.rights.nil? && @new_resource.group.nil? && @new_resource.owner.nil? && @new_resource.deny_rights.nil? - + securable_tempfile = Chef::ReservedNames::Win32::Security::SecurableObject.new(tempfile_path) # No rights were specified, so the dacl will have no explicit aces default_dacl = Chef::ReservedNames::Win32::Security::ACL.create([]) - # In setting this default dacl, set inheritance to true + # In setting this default dacl, set inheritance to true securable_tempfile.set_dacl(default_dacl, true) - end - end + end + end + + private + + def managing_content? + return true if @new_resource.checksum + return true if !@new_resource.source.nil? && @action != :create_if_missing + false + end end end diff --git a/chef/lib/chef/provider/directory.rb b/chef/lib/chef/provider/directory.rb index 0329aeb1ad..a263662ef2 100644 --- a/chef/lib/chef/provider/directory.rb +++ b/chef/lib/chef/provider/directory.rb @@ -59,8 +59,8 @@ class Chef # find the lowest-level directory in @new_resource.path that already exists # make sure we have write permissions to that directory is_parent_writable = lambda do |base_dir| - base_dir = ::File.dirname(base_dir) - if ::File.exist?(base_dir) + base_dir = ::File.dirname(base_dir) + if ::File.exist?(base_dir) ::File.writable?(base_dir) else is_parent_writable.call(base_dir) @@ -70,27 +70,27 @@ class Chef else # in why run mode & parent directory does not exist no permissions check is required # If not in why run, permissions must be valid and we rely on prior assertion that dir exists - if !whyrun_mode? || ::File.exist?(parent_directory) + if !whyrun_mode? || ::File.exist?(parent_directory) ::File.writable?(parent_directory) else true end end end - a.failure_message(Chef::Exceptions::InsufficientPermissions, + a.failure_message(Chef::Exceptions::InsufficientPermissions, "Cannot create #{@new_resource} at #{@new_resource.path} due to insufficient permissions") end - requirements.assert(:delete) do |a| - a.assertion do + requirements.assert(:delete) do |a| + a.assertion do if ::File.exist?(@new_resource.path) - ::File.directory?(@new_resource.path) && ::File.writable?(@new_resource.path) + ::File.directory?(@new_resource.path) && ::File.writable?(@new_resource.path) else true end end a.failure_message(RuntimeError, "Cannot delete #{@new_resource} at #{@new_resource.path}!") - # No why-run handling here: + # No why-run handling here: # * if we don't have permissions, this is unlikely to be changed earlier in the run # * if the target is a file (not a dir), there's no reasonable path by which this would have been changed end @@ -98,14 +98,14 @@ class Chef def action_create unless ::File.exist?(@new_resource.path) - converge_by("create new directory #{@new_resource.path}") do + converge_by("create new directory #{@new_resource.path}") do if @new_resource.recursive == true ::FileUtils.mkdir_p(@new_resource.path) else ::Dir.mkdir(@new_resource.path) end Chef::Log.info("#{@new_resource} created directory #{@new_resource.path}") - end + end end set_all_access_controls end @@ -123,6 +123,12 @@ class Chef end end end + + private + + def managing_content? + false + end end end end diff --git a/chef/lib/chef/provider/file.rb b/chef/lib/chef/provider/file.rb index 77f5217027..297cdd0fd2 100644 --- a/chef/lib/chef/provider/file.rb +++ b/chef/lib/chef/provider/file.rb @@ -48,9 +48,9 @@ class Chef def diff_current_from_content(new_content) result = nil - Tempfile.open("chef-diff") do |file| + Tempfile.open("chef-diff") do |file| file.write new_content - file.close + file.close result = diff_current file.path end result @@ -96,12 +96,12 @@ class Chef # -u: Unified diff format result = shell_out("diff -u #{target_path} #{temp_path}" ) rescue Exception => e - # Should *not* receive this, but in some circumstances it seems that + # Should *not* receive this, but in some circumstances it seems that # an exception can be thrown even using shell_out instead of shell_out! return [ "Could not determine diff. Error: #{e.message}" ] end - # diff will set a non-zero return code even when there's + # diff will set a non-zero return code even when there's # valid stdout results, if it encounters something unexpected # So as long as we have output, we'll show it. if not result.stdout.empty? @@ -118,7 +118,7 @@ class Chef else [ "(no diff)" ] end - end + end def whyrun_supported? true @@ -132,14 +132,14 @@ class Chef @current_resource.path(@new_resource.path) if !::File.directory?(@new_resource.path) if ::File.exist?(@new_resource.path) - if @action != :create_if_missing + if managing_content? @current_resource.checksum(checksum(@new_resource.path)) end end end load_current_resource_attrs setup_acl - + @current_resource end @@ -159,7 +159,7 @@ class Chef if @new_resource.group.nil? @new_resource.group(@current_resource.group) - end + end if @new_resource.owner.nil? @new_resource.owner(@current_resource.owner) end @@ -168,7 +168,7 @@ class Chef end end end - + def setup_acl @acl_scanner = ScanAccessControl.new(@new_resource, @current_resource) @acl_scanner.set_all! @@ -191,7 +191,7 @@ class Chef # Make sure the file is deletable if it exists. Otherwise, fail. requirements.assert(:delete) do |a| a.assertion do - if ::File.exists?(@new_resource.path) + if ::File.exists?(@new_resource.path) ::File.writable?(@new_resource.path) else true @@ -211,7 +211,7 @@ class Chef unless compare_content description = [] description << "update content in file #{@new_resource.path} from #{short_cksum(@current_resource.checksum)} to #{short_cksum(new_resource_content_checksum)}" - description << diff_current_from_content(@new_resource.content) + description << diff_current_from_content(@new_resource.content) converge_by(description) do backup @new_resource.path if ::File.exists?(@new_resource.path) ::File.open(@new_resource.path, "w") {|f| f.write @new_resource.content } @@ -221,10 +221,10 @@ class Chef end # if you are using a tempfile before creating, you must - # override the default with the tempfile, since the + # override the default with the tempfile, since the # file at @new_resource.path will not be updated on converge def update_new_file_state(path=@new_resource.path) - if !::File.directory?(path) + if !::File.directory?(path) @new_resource.checksum(checksum(path)) end @@ -247,8 +247,8 @@ class Chef desc = "create new file #{@new_resource.path}" desc << " with content checksum #{short_cksum(new_resource_content_checksum)}" if new_resource.content description << desc - description << diff_current_from_content(@new_resource.content) - + description << diff_current_from_content(@new_resource.content) + converge_by(description) do Chef::Log.info("entered create") ::File.open(@new_resource.path, "w+") {|f| f.write @new_resource.content } @@ -264,7 +264,7 @@ class Chef def set_all_access_controls if access_controls.requires_changes? - converge_by(access_controls.describe_changes) do + converge_by(access_controls.describe_changes) do access_controls.set_all #Update file state with new access values update_new_file_state @@ -282,7 +282,7 @@ class Chef def action_delete if ::File.exists?(@new_resource.path) - converge_by("delete file #{@new_resource.path}") do + converge_by("delete file #{@new_resource.path}") do backup unless ::File.symlink?(@new_resource.path) ::File.delete(@new_resource.path) Chef::Log.info("#{@new_resource} deleted file at #{@new_resource.path}") @@ -342,6 +342,12 @@ class Chef private + def managing_content? + return true if @new_resource.checksum + return true if !@new_resource.content.nil? && @action != :create_if_missing + false + end + def short_cksum(checksum) return "none" if checksum.nil? checksum.slice(0,6) diff --git a/chef/lib/chef/provider/remote_file.rb b/chef/lib/chef/provider/remote_file.rb index fa79747846..9ba1297001 100644 --- a/chef/lib/chef/provider/remote_file.rb +++ b/chef/lib/chef/provider/remote_file.rb @@ -42,7 +42,7 @@ class Chef if matches_current_checksum?(raw_file) Chef::Log.debug "#{@new_resource} target and source checksums are the same - not updating" else - description = [] + description = [] description << "copy file downloaded from #{@new_resource.source} into #{@new_resource.path}" description << diff_current(raw_file.path) converge_by(description) do @@ -52,7 +52,7 @@ class Chef raw_file.close! end # whyrun mode cleanup - the temp file will never be used, - # so close/unlink it here. + # so close/unlink it here. if whyrun_mode? raw_file.close! end @@ -121,6 +121,12 @@ class Chef false end + def managing_content? + return true if @new_resource.checksum + return true if !@new_resource.source.nil? && @action != :create_if_missing + false + end + end end end diff --git a/chef/lib/chef/provider/template.rb b/chef/lib/chef/provider/template.rb index 4c0989ac26..9bb9706356 100644 --- a/chef/lib/chef/provider/template.rb +++ b/chef/lib/chef/provider/template.rb @@ -34,12 +34,12 @@ class Chef @current_resource = Chef::Resource::Template.new(@new_resource.name) super end - + def define_resource_requirements super - requirements.assert(:create, :create_if_missing) do |a| - a.assertion { ::File::exist?(template_location) } + requirements.assert(:create, :create_if_missing) do |a| + a.assertion { ::File::exist?(template_location) } a.failure_message "Template source #{template_location} could not be found." a.whyrun "Template source #{template_location} does not exist. Assuming it would have been created." a.block_action! @@ -54,7 +54,7 @@ class Chef Chef::Log.debug("#{@new_resource} content has not changed.") set_all_access_controls else - description = [] + description = [] action_message = update ? "update #{@current_resource} from #{short_cksum(@current_resource.checksum)} to #{short_cksum(@new_resource.checksum)}" : "create #{@new_resource}" description << action_message @@ -74,7 +74,7 @@ class Chef @new_resource.group(stat.gid) end end - end + end end @@ -112,6 +112,12 @@ class Chef render_template(IO.read(template_location), context, &block) end + def managing_content? + return true if @new_resource.checksum + return true if !@new_resource.source.nil? && @action != :create_if_missing + false + end + end end end diff --git a/chef/spec/unit/provider/file_spec.rb b/chef/spec/unit/provider/file_spec.rb index 23352a5124..17e1681611 100644 --- a/chef/spec/unit/provider/file_spec.rb +++ b/chef/spec/unit/provider/file_spec.rb @@ -56,68 +56,68 @@ describe Chef::Provider::File do context "load_current_resource_attrs", :unix_only do it "should collect the current state of the file on the filesystem and populate current_resource" do # test setup - stat_struct = mock("::File.stat", :mode => 0600, :uid => 0, :gid => 0, :mtime => 10000) - ::File.should_receive(:stat).exactly(3).with(@resource.path).and_return(stat_struct) - - # test execution + stat_struct = mock("::File.stat", :mode => 0600, :uid => 0, :gid => 0, :mtime => 10000) + ::File.should_receive(:stat).exactly(2).with(@resource.path).and_return(stat_struct) + + # test execution @provider.load_current_resource - + # post-condition checks @provider.current_resource.mode.should == 0600 @provider.current_resource.owner.should == 0 @provider.current_resource.group.should == 0 end - + it "should NOT update the new_resource state with the current_resourse state if new_resource state is already specified" do # test setup - stat_struct = mock("::File.stat", :mode => 0600, :uid => 0, :gid => 0, :mtime => 10000) - ::File.should_receive(:stat).exactly(3).with(@resource.path).and_return(stat_struct) - + stat_struct = mock("::File.stat", :mode => 0600, :uid => 0, :gid => 0, :mtime => 10000) + ::File.should_receive(:stat).exactly(2).with(@resource.path).and_return(stat_struct) + @provider.new_resource.group(1) @provider.new_resource.owner(1) @provider.new_resource.mode(0644) - - # test execution + + # test execution @provider.load_current_resource - + # post-condition checks @provider.new_resource.group.should == 1 @provider.new_resource.owner.should == 1 @provider.new_resource.mode.should == 0644 end - + it "should update the new_resource state with the current_resource state if the new_resource state is not specified." do # test setup - stat_struct = mock("::File.stat", :mode => 0600, :uid => 0, :gid => 0, :mtime => 10000) - ::File.should_receive(:stat).exactly(3).with(@resource.path).and_return(stat_struct) - + stat_struct = mock("::File.stat", :mode => 0600, :uid => 0, :gid => 0, :mtime => 10000) + ::File.should_receive(:stat).exactly(2).with(@resource.path).and_return(stat_struct) + @provider.new_resource.group(nil) @provider.new_resource.owner(nil) @provider.new_resource.mode(nil) - - # test execution + + # test execution @provider.load_current_resource - + # post-condition checks @provider.new_resource.group.should eql(@provider.current_resource.group) @provider.new_resource.owner.should eql(@provider.current_resource.owner) @provider.new_resource.mode.should eql(@provider.current_resource.mode) end - + it "should update the new_resource when attempting to set the new state" do # test setup - stat_struct = mock("::File.stat", :mode => 0600, :uid => 0, :gid => 0, :mtime => 10000) + stat_struct = mock("::File.stat", :mode => 0600, :uid => 0, :gid => 0, :mtime => 10000) # called once in update_new_file_state and once in checksum - ::File.should_receive(:stat).twice.with(@provider.new_resource.path).and_return(stat_struct) + ::File.should_receive(:stat).twice.with(@provider.new_resource.path).and_return(stat_struct) ::File.should_receive(:directory?).once.with(@provider.new_resource.path).and_return(false) - + @provider.new_resource.group(nil) @provider.new_resource.owner(nil) @provider.new_resource.mode(nil) - - # test exectution + + # test exectution @provider.update_new_file_state - + # post-condition checks @provider.new_resource.group.should == 0 @provider.new_resource.owner.should == 0 @@ -487,7 +487,7 @@ describe Chef::Provider::File do it "should return valid diff output when content does not match the string content provided" do Tempfile.open("some-temp") do |file| @resource.path file.path - @provider = Chef::Provider::File.new(@resource, @run_context) + @provider = Chef::Provider::File.new(@resource, @run_context) @provider.load_current_resource result = @provider.diff_current_from_content "foo baz" # remove the file name info which varies. |