From 34dd4d4730656141889dc0cf9eee45e979b0a141 Mon Sep 17 00:00:00 2001 From: danielsdeleo Date: Mon, 24 Jun 2013 17:30:27 -0700 Subject: Allow file resources to manage files via symlink Fixes CHEF-4312 http://tickets.opscode.com/browse/CHEF-4312 Adds resource attribute `manage_symlink_source` to file resource and descendents. When true, file resources will manage the source file when a symlink exists at the destination path. When nil (default), the source file is managed, but a warning is emitted. When false, symlinks are not followed. In Chef 12, the default should be changed to false. --- lib/chef/exceptions.rb | 4 + lib/chef/provider/file.rb | 111 ++++++++++++-- lib/chef/resource/file.rb | 9 ++ spec/support/shared/functional/file_resource.rb | 190 ++++++++++++++++++++---- 4 files changed, 273 insertions(+), 41 deletions(-) diff --git a/lib/chef/exceptions.rb b/lib/chef/exceptions.rb index 0262d67e47..c0a5a185d6 100644 --- a/lib/chef/exceptions.rb +++ b/lib/chef/exceptions.rb @@ -156,6 +156,10 @@ class Chef # File-like resource found a non-file (socket, pipe, directory, etc) at its destination class FileTypeMismatch < RuntimeError; end + # File (or descendent) resource configured to manage symlink source, but + # the symlink that is there either loops or points to a nonexistent file + class InvalidSymlink < RuntimeError; end + class MissingRole < RuntimeError NULL = Object.new diff --git a/lib/chef/provider/file.rb b/lib/chef/provider/file.rb index 52c0da7a72..012073f674 100644 --- a/lib/chef/provider/file.rb +++ b/lib/chef/provider/file.rb @@ -102,19 +102,15 @@ class Chef end end - # Make sure if the destination already exists that it is a normal file - # for :create_if_missing, we're assuming the user wanted to avoid blowing away the non-file here - # for :touch, we can modify perms of whatever is at this path, regardless of its type - # for :delete, we can blow away whatever is here, regardless of its type - # for :create, we have a problem with devining the users intent, so we raise an exception - unless @new_resource.force_unlink - requirements.assert(:create) do |a| - # File should either not exist or be a real file in order - # to match this assertion. - a.assertion { !::File.exists?(@new_resource.path) || real_file?(@new_resource.path) } - a.failure_message(Chef::Exceptions::FileTypeMismatch, "File #{@new_resource.path} exists, but is a #{file_type_string(@new_resource.path)}, set force_unlink to true to remove") - a.whyrun("Assuming #{file_type_string(@new_resource.path)} at #{@new_resource.path} would have been removed by a previous resource") - end + error, reason, whyrun_message = inspect_existing_fs_entry + requirements.assert(:create) do |a| + a.assertion { error.nil? } + a.failure_message(error, reason) + a.whyrun(whyrun_message) + # Subsequent attempts to read the fs entry at the path (e.g., for + # calculating checksums) could blow up, so give up trying to continue + # why-running. + a.block_action! end end @@ -163,6 +159,85 @@ class Chef private + # Handles resource requirements for action :create when some fs entry + # already exists at the destination path. For actions other than create, + # we don't care what kind of thing is at the destination path because: + # * for :create_if_missing, we're assuming the user wanted to avoid blowing away the non-file here + # * for :touch, we can modify perms of whatever is at this path, regardless of its type + # * for :delete, we can blow away whatever is here, regardless of its type + # + # For the action :create case, we need to deal with user-selectable + # behavior to see if we're in an error condition. + # * If there's no file at the destination path currently, we're cool to + # create it. + # * If the fs entry that currently exists at the destination is a regular + # file, we're cool to update it with new content. + # * If the fs entry is a symlink AND the resource has + # `manage_symlink_source` enabled, we need to verify that the symlink is + # a valid pointer to a real file. If it is, we can manage content and + # permissions on the symlink source, otherwise, error. + # * If `manage_symlink_source` is not enabled, fall through. + # * If force_unlink is true, action :create will unlink whatever is in the way. + # * If force_unlink is false, we're in an exceptional situation, so we + # want to error. + # + # Note that this method returns values to be used with requirement + # assertions, which then decide whether or not to raise or issue a + # warning for whyrun mode. + def inspect_existing_fs_entry + path = @new_resource.path + + if !l_exist?(path) + [nil, nil, nil] + elsif real_file?(path) + [nil, nil, nil] + elsif file_class.symlink?(path) && @new_resource.manage_symlink_source + verify_symlink_sanity(path) + elsif file_class.symlink?(@new_resource.path) && @new_resource.manage_symlink_source.nil? + Chef::Log.warn("File #{path} managed by #{@new_resource} is really a symlink. Managing the source file instead.") + Chef::Log.warn("Disable this warning by setting `manage_symlink_source true` on the resource") + Chef::Log.warn("In a future Chef release, 'manage_symlink_source' will not be enabled by default") + verify_symlink_sanity(path) + elsif @new_resource.force_unlink + [nil, nil, nil] + else + [ Chef::Exceptions::FileTypeMismatch, + "File #{path} exists, but is a #{file_type_string(@new_resource.path)}, set force_unlink to true to remove", + "Assuming #{file_type_string(@new_resource.path)} at #{@new_resource.path} would have been removed by a previous resource" + ] + end + end + + # Returns values suitable for use in a requirements assertion statement + # when managing symlink source. If we're managing symlink source we can + # hit 3 error cases: + # 1. Symlink to nowhere: File.realpath(symlink) -> raise Errno::ENOENT + # 2. Symlink loop: File.realpath(symlink) -> raise Errno::ELOOP + # 3. Symlink to not-a-real-file: File.realpath(symlink) -> (directory|blockdev|etc.) + # If any of the above apply, returns a 3-tuple of Exception class, + # exception message, whyrun message; otherwise returns a 3-tuple of nil. + def verify_symlink_sanity(path) + real_path = ::File.realpath(path) + if real_file?(real_path) + [nil, nil, nil] + else + [ Chef::Exceptions::FileTypeMismatch, + "File #{path} exists, but is a symlink to #{real_path} which is a #{file_type_string(real_path)}. " + + "Disable manage_symlink_source and set force_unlink to remove it.", + "Assuming symlink #{path} or source file #{real_path} would have been fixed by a previous resource" + ] + end + rescue Errno::ELOOP + [ Chef::Exceptions::InvalidSymlink, + "Symlink at #{path} (pointing to #{::File.readlink(path)}) exists but attempting to resolve it creates a loop", + "Assuming symlink loop would be fixed by a previous resource" ] + rescue Errno::ENOENT + [ Chef::Exceptions::InvalidSymlink, + "Symlink at #{path} (pointing to #{::File.readlink(path)}) exists but attempting to resolve it leads to a nonexistent file", + "Assuming symlink source would be created by a previous resource" ] + end + + def content @content ||= begin load_current_resource if @current_resource.nil? @@ -193,6 +268,12 @@ class Chef !file_class.symlink?(path) && ::File.file?(path) end + # Similar to File.exist?, but also returns true in the case that the + # named file is a broken symlink. + def l_exist?(path) + ::File.exist?(path) || file_class.symlink?(path) + end + def unlink(path) # Directories can not be unlinked. Remove them using FileUtils. if ::File.directory?(path) @@ -246,7 +327,7 @@ class Chef def update_file_contents do_backup unless file_created? - deployment_strategy.deploy(tempfile.path, @new_resource.path) + deployment_strategy.deploy(tempfile.path, ::File.realpath(@new_resource.path)) Chef::Log.info("#{@new_resource} updated file contents #{@new_resource.path}") @new_resource.checksum(checksum(@new_resource.path)) # for reporting end @@ -280,7 +361,7 @@ class Chef if resource_updated? && Chef::Config[:enable_selinux_file_permission_fixup] if selinux_enabled? converge_by("restore selinux security context") do - restore_security_context(@new_resource.path, recursive) + restore_security_context(::File.realpath(@new_resource.path), recursive) end else Chef::Log.debug "selinux utilities can not be found. Skipping selinux permission fixup." diff --git a/lib/chef/resource/file.rb b/lib/chef/resource/file.rb index 879d5f0c86..15adb9dae8 100644 --- a/lib/chef/resource/file.rb +++ b/lib/chef/resource/file.rb @@ -48,6 +48,7 @@ class Chef @provider = Chef::Provider::File @atomic_update = Chef::Config[:file_atomic_update] @force_unlink = false + @manage_symlink_source = nil @diff = nil end @@ -108,6 +109,14 @@ class Chef ) end + def manage_symlink_source(arg=nil) + set_or_return( + :manage_symlink_source, + arg, + :kind_of => [ TrueClass, FalseClass ] + ) + end + end end end diff --git a/spec/support/shared/functional/file_resource.rb b/spec/support/shared/functional/file_resource.rb index ec0ddafb00..2b4330b539 100644 --- a/spec/support/shared/functional/file_resource.rb +++ b/spec/support/shared/functional/file_resource.rb @@ -369,45 +369,183 @@ shared_examples_for "a configured file resource" do File.join(CHEF_SPEC_DATA, "file-test-target") } - before do - FileUtils.touch(symlink_target) - end - after do - FileUtils.rm_rf(symlink_target) - end + describe "when configured not to manage symlink's target" do + before(:each) do + # configure not to manage symlink source + resource.manage_symlink_source(false) - before(:each) do - if windows? - Chef::ReservedNames::Win32::File.symlink(symlink_target, path) - else - File.symlink(symlink_target, path) + # create symlinks for test context + FileUtils.touch(symlink_target) + + if windows? + Chef::ReservedNames::Win32::File.symlink(symlink_target, path) + else + File.symlink(symlink_target, path) + end end - end - after(:each) do - FileUtils.rm_rf(path) - end + after(:each) do + FileUtils.rm_rf(symlink_target) + FileUtils.rm_rf(path) + end - describe "when symlink target has correct content" do - before(:each) do - File.open(symlink_target, "wb") { |f| f.print expected_content } + describe "when symlink target has correct content" do + before(:each) do + File.open(symlink_target, "wb") { |f| f.print expected_content } + end + + it_behaves_like "file resource not pointing to a real file" end - it_behaves_like "file resource not pointing to a real file" + describe "when symlink target has the wrong content" do + before(:each) do + File.open(symlink_target, "wb") { |f| f.print "This is so wrong!!!" } + end + + after(:each) do + # symlink should never be followed + binread(symlink_target).should == "This is so wrong!!!" + end + + it_behaves_like "file resource not pointing to a real file" + end end - describe "when symlink target has the wrong content" do - before(:each) do - File.open(symlink_target, "wb") { |f| f.print "This is so wrong!!!" } + # Unix-only for now. Windows behavior may differ because of how ACL + # management handles symlinks. Since symlinks are rare on Windows and this + # feature primarily exists to support the case where a well-known file + # (e.g., resolv.conf) has been converted to a symlink, we're okay with the + # discrepancy. + context "when configured to manage the symlink source", :unix_only do + + before do + resource.manage_symlink_source(true) end - after(:each) do - # symlink should never be followed - binread(symlink_target).should == "This is so wrong!!!" + context "but the symlink is part of a loop" do + let(:link1_path) { File.join(CHEF_SPEC_DATA, "points-to-link2") } + let(:link2_path) { File.join(CHEF_SPEC_DATA, "points-to-link1") } + + before do + # point resource at link1: + resource.path(link1_path) + # create symlinks for test context + File.symlink(link1_path, link2_path) + File.symlink(link2_path, link1_path) + end + + after(:each) do + FileUtils.rm_rf(link1_path) + FileUtils.rm_rf(link2_path) + end + + it "raises an InvalidSymlink error" do + lambda { resource.run_action(:create) }.should raise_error(Chef::Exceptions::InvalidSymlink) + end + + it "issues a warning/assumption in whyrun mode" do + begin + Chef::Config[:why_run] = true + resource.run_action(:create) # should not raise + ensure + Chef::Config[:why_run] = false + end + end + end + + context "but the symlink points to a nonexistent file" do + let(:link_path) { File.join(CHEF_SPEC_DATA, "points-to-nothing") } + let(:not_existent_source) { File.join(CHEF_SPEC_DATA, "i-am-not-here") } + + before do + resource.path(link_path) + # create symlinks for test context + File.symlink(not_existent_source, link_path) + FileUtils.rm_rf(not_existent_source) + end + + after(:each) do + FileUtils.rm_rf(link_path) + end + it "raises an InvalidSymlink error" do + lambda { resource.run_action(:create) }.should raise_error(Chef::Exceptions::InvalidSymlink) + end + + it "issues a warning/assumption in whyrun mode" do + begin + Chef::Config[:why_run] = true + resource.run_action(:create) # should not raise + ensure + Chef::Config[:why_run] = false + end + end + end + + context "but the symlink is points to a non-file fs entry" do + let(:link_path) { File.join(CHEF_SPEC_DATA, "points-to-dir") } + let(:not_a_file_path) { File.join(CHEF_SPEC_DATA, "dir-at-end-of-symlink") } + + before do + # point resource at link1: + resource.path(link_path) + # create symlinks for test context + File.symlink(not_a_file_path, link_path) + Dir.mkdir(not_a_file_path) + end + + after(:each) do + FileUtils.rm_rf(link_path) + FileUtils.rm_rf(not_a_file_path) + end + + it "raises an InvalidSymlink error" do + lambda { resource.run_action(:create) }.should raise_error(Chef::Exceptions::FileTypeMismatch) + end + + it "issues a warning/assumption in whyrun mode" do + begin + Chef::Config[:why_run] = true + resource.run_action(:create) # should not raise + ensure + Chef::Config[:why_run] = false + end + end + end + + context "when the symlink source is a real file" do + + let(:wrong_content) { "this is the wrong content" } + let(:link_path) { File.join(CHEF_SPEC_DATA, "points-to-real-file") } + + before do + # point resource at link: + resource.path(link_path) + # create symlinks for test context + File.symlink(path, link_path) + + # Create source (real) file + File.open(path, "wb") { |f| f.write(wrong_content) } + end + + include_context "setup broken permissions" + + include_examples "a securable resource with existing target" + + after(:each) do + # shared examples should not change our test setup of a file resource + # pointing at a symlink: + resource.path.should == link_path + FileUtils.rm_rf(link_path) + end + + it "does not replace the symlink with a real file" do + resource.run_action(:create) + File.should be_symlink(link_path) + end + end - it_behaves_like "file resource not pointing to a real file" end end -- cgit v1.2.1