Merge pull request #5234 from MsysTechnologiesllc/ali/allow_symlink_directory_delete_on_windows

Added code to handle deletion of directories on Windows that are symlinks.

3 files changed, 199 insertions, 3 deletions

diff --git a/lib/chef/provider/link.rb b/lib/chef/provider/link.rb
index 5fce97e5b3..16d30319b3 100644
--- a/lib/chef/provider/link.rb
+++ b/lib/chef/provider/link.rb
@@ -141,9 +141,16 @@ class Chef
 
       def action_delete
         if @current_resource.to # Exists
-          converge_by("delete link at #{@new_resource.target_file}") do
-            ::File.delete(@new_resource.target_file)
-            Chef::Log.info("#{@new_resource} deleted")
+          if Chef::Platform.windows? && ::File.directory?(@current_resource.target_file)
+            converge_by("delete link to dir at #{@new_resource.target_file}") do
+              ::Dir.delete(@new_resource.target_file)
+              Chef::Log.info("#{@new_resource} deleted")
+            end
+          else
+            converge_by("delete link to file at #{@new_resource.target_file}") do
+              ::File.delete(@new_resource.target_file)
+              Chef::Log.info("#{@new_resource} deleted")
+            end
           end
         end
       end
diff --git a/spec/functional/resource/link_spec.rb b/spec/functional/resource/link_spec.rb
index 5e58d0918a..de6976448e 100644
--- a/spec/functional/resource/link_spec.rb
+++ b/spec/functional/resource/link_spec.rb
@@ -388,6 +388,14 @@ describe Chef::Resource::Link do
             symlink(other_dir, target_file)
           end
           include_context "create symbolic link succeeds"
+          include_context "delete succeeds"
+        end
+        context "and the link already exists and points at the target" do
+          before do
+            symlink(to, target_file)
+          end
+          include_context "create symbolic link is noop"
+          include_context "delete succeeds"
         end
       end
       context "when the link destination is a symbolic link" do
@@ -406,6 +414,19 @@ describe Chef::Resource::Link do
             include_context "create symbolic link succeeds"
             include_context "delete is noop"
           end
+          context "and the destination itself has another symbolic link" do
+            context "to a link that exist" do
+              before do
+                symlink(to, target_file)
+              end
+              include_context "create symbolic link is noop"
+              include_context "delete succeeds"
+            end
+            context "to a link that does not exist" do
+              include_context "create symbolic link succeeds"
+              include_context "delete is noop"
+            end
+          end
         end
         context "to a file that does not exist" do
           before(:each) do
@@ -418,6 +439,19 @@ describe Chef::Resource::Link do
             include_context "create symbolic link succeeds"
             include_context "delete is noop"
           end
+          context "and the destination itself has another symbolic link" do
+            context "to a link that exist" do
+              before do
+                symlink(to, target_file)
+              end
+              include_context "create symbolic link is noop"
+              include_context "delete succeeds"
+            end
+            context "to a link that does not exist" do
+              include_context "create symbolic link succeeds"
+              include_context "delete is noop"
+            end
+          end
         end
       end
       context "when the link destination does not exist" do
diff --git a/spec/unit/provider/link_spec.rb b/spec/unit/provider/link_spec.rb
index 6bb08551a2..9426cf41dc 100644
--- a/spec/unit/provider/link_spec.rb
+++ b/spec/unit/provider/link_spec.rb
@@ -249,4 +249,159 @@ describe Chef::Resource::Link, :not_supported_on_win2k3 do
       end
     end
   end
+
+  describe "action_delete" do
+    before(:each) do
+      stat = double("stats", :ino => 5)
+      allow(stat).to receive(:uid).and_return(501)
+      allow(stat).to receive(:gid).and_return(501)
+      allow(stat).to receive(:mode).and_return(0755)
+      allow(provider.file_class).to receive(:stat).with(
+        "#{CHEF_SPEC_DATA}/fofile-link").and_return(stat)
+
+      provider.load_current_resource
+    end
+
+    shared_context "delete link to directories on Windows" do
+      before do
+        allow(::File).to receive(:directory?).with(
+          "#{CHEF_SPEC_DATA}/fofile-link").and_return(true)
+      end
+
+      it "invokes Dir.delete method to delete the link" do
+        expect(::Dir).to receive(:delete).with(provider.new_resource.target_file)
+        expect(Chef::Log).to receive(:info).with("#{provider.new_resource} deleted")
+        provider.run_action(:delete)
+      end
+    end
+
+    shared_context "delete link to directories on Linux" do
+      before do
+        allow(::File).to receive(:directory?).with(
+          "#{CHEF_SPEC_DATA}/fofile-link").and_return(true)
+      end
+
+      it "invokes File.delete method to delete the link" do
+        expect(::File).to receive(:delete).with(provider.new_resource.target_file)
+        expect(Chef::Log).to receive(:info).with("#{provider.new_resource} deleted")
+        provider.run_action(:delete)
+      end
+    end
+
+    shared_context "delete link to files" do
+      before do
+        allow(::File).to receive(:directory?).with(
+          "#{CHEF_SPEC_DATA}/fofile-link").and_return(false)
+      end
+
+      it "invokes File.delete method to delete the link" do
+        expect(::File).to receive(:delete).with(provider.new_resource.target_file)
+        expect(Chef::Log).to receive(:info).with("#{provider.new_resource} deleted")
+        provider.run_action(:delete)
+      end
+    end
+
+    shared_context "soft links prerequisites" do
+      before(:each) do
+        allow(provider.file_class).to receive(:symlink?).with(
+          "#{CHEF_SPEC_DATA}/fofile-link").and_return(true)
+        allow(provider.file_class).to receive(:readlink).with(
+          "#{CHEF_SPEC_DATA}/fofile-link").and_return("#{CHEF_SPEC_DATA}/fofile")
+      end
+    end
+
+    shared_context "hard links prerequisites" do
+      let(:new_resource) do
+        result = Chef::Resource::Link.new("#{CHEF_SPEC_DATA}/fofile-link")
+        result.to "#{CHEF_SPEC_DATA}/fofile"
+        result.link_type :hard
+        result
+      end
+
+      before(:each) do
+        stat = double("stats", :ino => 5)
+        allow(stat).to receive(:uid).and_return(502)
+        allow(stat).to receive(:gid).and_return(502)
+        allow(stat).to receive(:mode).and_return(0644)
+
+        allow(provider.file_class).to receive(:symlink?).with(
+          "#{CHEF_SPEC_DATA}/fofile-link").and_return(false)
+
+        allow(File).to receive(:exists?).with(
+          "#{CHEF_SPEC_DATA}/fofile-link").and_return(true)
+        allow(File).to receive(:exists?).with(
+          "#{CHEF_SPEC_DATA}/fofile").and_return(true)
+
+        allow(provider.file_class).to receive(:stat).with(
+          "#{CHEF_SPEC_DATA}/fofile").and_return(stat)
+      end
+    end
+
+    context "on Windows platform" do
+      let(:resource_link) do
+        Chef::Resource::Link.new(provider.new_resource.name)
+      end
+
+      before(:each) do
+        allow(Chef::Resource::Link).to receive(:new).with(
+          provider.new_resource.name).and_return(resource_link)
+        allow(resource_link).to receive(:verify_links_supported!)
+        allow(Chef::Platform).to receive(:windows?).and_return(true)
+      end
+
+      context "soft links" do
+        include_context "soft links prerequisites"
+
+        context "to directories" do
+          include_context "delete link to directories on Windows"
+        end
+
+        context "to files" do
+          include_context "delete link to files"
+        end
+      end
+
+      context "hard links" do
+        include_context "hard links prerequisites"
+
+        context "to directories" do
+          include_context "delete link to directories on Windows"
+        end
+
+        context "to files" do
+          include_context "delete link to files"
+        end
+      end
+    end
+
+    context "on Linux platform" do
+      before(:each) do
+        allow(Chef::Platform).to receive(:windows?).and_return(false)
+      end
+
+      context "soft links" do
+        include_context "soft links prerequisites"
+
+        context "to directories" do
+          include_context "delete link to directories on Linux"
+        end
+
+        context "to files" do
+          include_context "delete link to files"
+        end
+      end
+
+      context "hard links" do
+        include_context "hard links prerequisites"
+
+        context "to directories" do
+          include_context "delete link to directories on Linux"
+        end
+
+        context "to files" do
+          include_context "delete link to files"
+        end
+      end
+    end
+  end
 end