Merge branch 'ticket/master/19447-windows-symlinks'

* ticket/master/19447-windows-symlinks:
  (#19447) Skip symlink test if provider doesn't support it
  (#19447) Rescue when symlink privilege does not exist
  (#19447) Fully qualify access to ruby `File`
  (#19447) Fall back to stat if we don't support symlinks
  (#19447) Check for symlink permission only if we try to create one
  Maint: Re-word doc string for file type's ensure property
  (#19447) Rewrite symlink tests for file type
  (#19447) check Puppet.features.manages_symlinks?
  (#19447) Windows symlink check process token
  (#19447) Puppet::FileSystem::File Windows symlink

Closes GH-1997

Author: joshcooper

acceptance/tests/resource/file/should_create_symlink.rb

@@ -1,31 +1,37 @@
 test_name "should create symlink"
-confine :except, :platform => 'windows'
 
 message = 'hello world'
-target  = "/tmp/test-#{Time.new.to_i}"
-source  = "/tmp/test-#{Time.new.to_i}-source"
-
 agents.each do |agent|
+  confine_block :to, :platform => 'windows' do
+    # symlinks are supported only on Vista+ (version 6.0 and higher)
+    on agents, facter('kernelmajversion') do
+      skip_test "Test not supported on this plaform" if stdout.chomp.to_f < 6.0
+    end
+  end
+
+  link = agent.tmpfile("symlink-link")
+  target = agent.tmpfile("symlink-target")
+
   step "clean up the system before we begin"
-  on agent, "rm -rf #{target}"
-  on agent, "echo '#{message}' > #{source}"
+  on agent, "rm -rf #{target} #{link}"
+  on agent, "echo '#{message}' > #{target}"
 
   step "verify we can create a symlink"
-  on(agent, puppet_resource("file", target, "ensure=#{source}"))
+  on(agent, puppet_resource("file", link, "ensure=#{target}"))
 
   step "verify the symlink was created"
-  on agent, "test -L #{target} && test -f #{target}"
-  step "verify source file"
-  on agent, "test -f #{source}"
+  on agent, "test -L #{link} && test -f #{link}"
+  step "verify the symlink points to a file"
+  on agent, "test -f #{target}"
 
   step "verify the content is identical on both sides"
-  on(agent, "cat #{source}") do
-    fail_test "source missing content" unless stdout.include? message
+  on(agent, "cat #{link}") do
+    fail_test "link missing content" unless stdout.include? message
   end
   on(agent, "cat #{target}") do
     fail_test "target missing content" unless stdout.include? message
   end
 
   step "clean up after the test run"
-  on agent, "rm -rf #{target} #{source}"
+  on agent, "rm -rf #{target} #{link}"
 end

acceptance/tests/resource/file/ticket_7680-follow-symlinks.rb

@@ -1,15 +1,28 @@
 test_name "#7680: 'links => follow' should use the file source content"
-confine :except, :platform => 'windows'
 
 agents.each do |agent|
+  confine_block :to, :platform => 'windows' do
+    # symlinks are supported only on Vista+ (version 6.0 and higher)
+    on agents, facter('kernelmajversion') do
+      skip_test "Test not supported on this plaform" if stdout.chomp.to_f < 6.0
+    end
+  end
 
   step "Create file content"
   real_source = agent.tmpfile('follow_links_source')
   dest        = agent.tmpfile('follow_links_dest')
   symlink     = agent.tmpfile('follow_links_symlink')
 
   on agent, "echo 'This is the real content' > #{real_source}"
-  on agent, "ln -sf #{real_source} #{symlink}"
+  if agent['platform'].include?('windows')
+    # cygwin ln doesn't behave properly, fallback to mklink,
+    # but that requires backslashes, that need to be escaped,
+    # and the link cannot exist prior.
+    on agent, "rm -f #{symlink}"
+    on agent, "cmd /c mklink #{symlink.gsub('/', '\\\\\\\\')} #{real_source.gsub('/', '\\\\\\\\')}"
+  else
+    on agent, "ln -sf #{real_source} #{symlink}"
+  end
 
   manifest = <<-MANIFEST
     file { '#{dest}':

lib/puppet/feature/base.rb

@@ -73,5 +73,16 @@
 
 # We can manage symlinks
 Puppet.features.add(:manages_symlinks) do
-  ! Puppet::Util::Platform.windows?
+  if ! Puppet::Util::Platform.windows?
+    true
+  else
+    begin
+      require 'Win32API'
+      Win32API.new('kernel32', 'CreateSymbolicLink', 'SSL', 'B')
+      true
+    rescue LoadError => err
+      Puppet.debug("CreateSymbolicLink is not available")
+      false
+    end
+  end
 end

lib/puppet/file_system/file.rb

@@ -10,6 +10,9 @@ class Puppet::FileSystem::File
   IMPL = if RUBY_VERSION =~ /^1\.8/
            require 'puppet/file_system/file18'
            Puppet::FileSystem::File18
+         elsif Puppet::Util::Platform.windows?
+           require 'puppet/file_system/file19windows'
+           Puppet::FileSystem::File19Windows
          else
            require 'puppet/file_system/file19'
            Puppet::FileSystem::File19
@@ -104,6 +107,7 @@ def binread
   #
   # @return [Boolean] true if the named file exists.
   def self.exist?(path)
+    return IMPL.exist?(path) if IMPL.method(:exist?) != self.method(:exist?)
     File.exist?(path)
   end
 
@@ -131,18 +135,28 @@ def mkpath
     @path.mkpath
   end
 
-  # Creates a symbolic link dest which points to the current file. If dest
-  # already exists and it is a directory, creates a symbolic link dest/the
-  # current file. If dest already exists and it is not a directory,
-  # raises Errno::EEXIST. But if :force option is set, overwrite dest.
+  # Creates a symbolic link dest which points to the current file.
+  # If dest already exists:
   #
-  # @param dest [String] The mode to apply to the file if it is created
-  # @param [Hash] options the options to create a message with.
+  # * and is a file, will raise Errno::EEXIST
+  # * and is a directory, will return 0 but perform no action
+  # * and is a symlink referencing a file, will raise Errno::EEXIST
+  # * and is a symlink referencing a directory, will return 0 but perform no action
+  #
+  # With the :force option set to true, when dest already exists:
+  #
+  # * and is a file, will replace the existing file with a symlink (DANGEROUS)
+  # * and is a directory, will return 0 but perform no action
+  # * and is a symlink referencing a file, will modify the existing symlink
+  # * and is a symlink referencing a directory, will return 0 but perform no action
+  #
+  # @param dest [String] The path to create the new symlink at
+  # @param [Hash] options the options to create the symlink with
   # @option options [Boolean] :force overwrite dest
   # @option options [Boolean] :noop do not perform the operation
   # @option options [Boolean] :verbose verbose output
   #
-  # @raise [Errno::EEXIST] dest already exists and it is not a directory
+  # @raise [Errno::EEXIST] dest already exists as a file and, :force is not set
   #
   # @return [Integer] 0
   def symlink(dest, options = {})
@@ -166,6 +180,7 @@ def readlink
   #
   # @return [Integer] the number of names passed as arguments
   def self.unlink(*file_names)
+    return IMPL.unlink(*file_names) if IMPL.method(:unlink) != self.method(:unlink)
     File.unlink(*file_names)
   end
 

lib/puppet/file_system/file19windows.rb

@@ -0,0 +1,113 @@
+require 'puppet/file_system/file19'
+require 'puppet/util/windows'
+
+class Puppet::FileSystem::File19Windows < Puppet::FileSystem::File19
+
+  def self.exist?(path)
+    if ! Puppet.features.manages_symlinks?
+      return ::File.exist?(path)
+    end
+
+    path = path.to_str if path.respond_to?(:to_str) # support WatchedFile
+    path = path.to_s # support String and Pathname
+
+    begin
+      if Puppet::Util::Windows::File.symlink?(path)
+        path = Puppet::Util::Windows::File.readlink(path)
+      end
+      ! Puppet::Util::Windows::File.stat(path).nil?
+    rescue # generally INVALID_HANDLE_VALUE which means 'file not found'
+      false
+    end
+  end
+
+  def exist?
+    self.class.exist?(@path)
+  end
+
+  def symlink(dest, options = {})
+    raise_if_symlinks_unsupported
+
+    dest_exists = self.class.exist?(dest) # returns false on dangling symlink
+    dest_stat = Puppet::Util::Windows::File.stat(dest) if dest_exists
+    dest_symlink = Puppet::Util::Windows::File.symlink?(dest)
+
+    # silent fail to preserve semantics of original FileUtils
+    return 0 if dest_exists && dest_stat.ftype == 'directory'
+
+    if dest_exists && dest_stat.ftype == 'file' && options[:force] != true
+      raise(Errno::EEXIST, "#{dest} already exists and the :force option was not specified")
+    end
+
+    if options[:noop] != true
+      ::File.delete(dest) if dest_exists # can only be file
+      Puppet::Util::Windows::File.symlink(@path, dest)
+    end
+
+    0
+  end
+
+  def symlink?
+    return false if ! Puppet.features.manages_symlinks?
+    Puppet::Util::Windows::File.symlink?(@path)
+  end
+
+  def readlink
+    raise_if_symlinks_unsupported
+    Puppet::Util::Windows::File.readlink(@path)
+  end
+
+  def self.unlink(*file_names)
+    if ! Puppet.features.manages_symlinks?
+      return ::File.unlink(*file_names)
+    end
+
+    file_names.each do |file_name|
+      file_name = file_name.to_s # handle PathName
+      stat = Puppet::Util::Windows::File.stat(file_name) rescue nil
+
+      # sigh, Ruby + Windows :(
+      if stat && stat.ftype == 'directory'
+        if Puppet::Util::Windows::File.symlink?(file_name)
+          Dir.rmdir(file_name)
+        else
+          raise Errno::EPERM.new(file_name)
+        end
+      else
+        ::File.unlink(file_name)
+      end
+    end
+
+    file_names.length
+  end
+
+  def unlink
+    self.class.unlink(@path)
+  end
+
+  def stat
+    if ! Puppet.features.manages_symlinks?
+      return super
+    end
+    Puppet::Util::Windows::File.stat(@path)
+  end
+
+  def lstat
+    if ! Puppet.features.manages_symlinks?
+      return Puppet::Util::Windows::File.stat(@path)
+    end
+    Puppet::Util::Windows::File.lstat(@path)
+  end
+
+  private
+  def raise_if_symlinks_unsupported
+    if ! Puppet.features.manages_symlinks?
+      msg = "This version of Windows does not support symlinks.  Windows Vista / 2008 or higher is required."
+      raise Puppet::Util::Windows::Error.new(msg)
+    end
+
+    if ! Puppet::Util::Windows::Process.process_privilege_symlink?
+      Puppet.warning "The current user does not have the necessary permission to manage symlinks."
+    end
+  end
+end

lib/puppet/provider/file/windows.rb

@@ -2,6 +2,7 @@
   desc "Uses Microsoft Windows functionality to manage file ownership and permissions."
 
   confine :operatingsystem => :windows
+  has_feature :manages_symlinks if Puppet.features.manages_symlinks?
 
   include Puppet::Util::Warnings
 
@@ -35,33 +36,37 @@ def id2name(id)
   alias :name2uid :name2id
 
   def owner
-    return :absent unless resource.exist?
+    return :absent unless resource.stat
     get_owner(resource[:path])
   end
 
   def owner=(should)
     begin
-      set_owner(should, resource[:path])
+      path = resource[:links] == :manage ? file.path.to_s : file.readlink
+
+      set_owner(should, path)
     rescue => detail
       raise Puppet::Error, "Failed to set owner to '#{should}': #{detail}"
     end
   end
 
   def group
-    return :absent unless resource.exist?
+    return :absent unless resource.stat
     get_group(resource[:path])
   end
 
   def group=(should)
     begin
-      set_group(should, resource[:path])
+      path = resource[:links] == :manage ? file.path.to_s : file.readlink
+
+      set_group(should, path)
     rescue => detail
       raise Puppet::Error, "Failed to set group to '#{should}': #{detail}"
     end
   end
 
   def mode
-    if resource.exist?
+    if resource.stat
       mode = get_mode(resource[:path])
       mode ? mode.to_s(8) : :absent
     else
@@ -85,4 +90,10 @@ def validate
       resource.fail("Can only manage owner, group, and mode on filesystems that support Windows ACLs, such as NTFS")
     end
   end
+
+  attr_reader :file
+  private
+  def file
+    @file ||= Puppet::FileSystem::File.new(resource[:path])
+  end
 end

lib/puppet/type/file/ensure.rb

@@ -5,17 +5,35 @@ module Puppet
     require 'puppet/util/symbolic_file_mode'
     include Puppet::Util::SymbolicFileMode
 
-    desc <<-'EOT'
-      Whether to create files that don't currently exist.
-      Possible values are `absent`, `present`, `file`, `directory`, and `link`.
-      Specifying `present` will match any form of file existence, and
-      if the file is missing will create an empty file. Specifying
-      `absent` will delete the file (or directory, if `recurse => true` and
-      `force => true`). Specifying `link` requires that you also set the `target`
-      attribute; note that symlinks cannot be managed on Windows.
-
-      If you specify the path to another file as the ensure value, it is
-      equivalent to specifying `link` and using that path as the `target`:
+    desc <<-EOT
+      Whether the file should exist, and if so what kind of file it should be.
+      Possible values are `present`, `absent`, `file`, `directory`, and `link`.
+
+      * `present` will accept any form of file existence, and will create a
+        normal file if the file is missing. (The file will have no content
+        unless the `content` or `source` attribute is used.)
+      * `absent` will make sure the file doesn't exist, deleting it
+        if necessary.
+      * `file` will make sure it's a normal file, and enables use of the
+        `content` or `source` attribute.
+      * `directory` will make sure it's a directory, and enables use of the
+        `source`, `recurse`, `recurselimit`, `ignore`, and `purge` attributes.
+      * `link` will make sure the file is a symlink, and **requires** that you
+        also set the `target` attribute. Symlinks are supported on all Posix
+        systems and on Windows Vista / 2008 and higher. On Windows, managing
+        symlinks requires puppet agent's user account to have the "Create
+        Symbolic Links" privilege; this can be configured in the "User Rights
+        Assignment" section in the Windows policy editor. By default, puppet
+        agent runs as the Administrator account, which does have this privilege.
+
+      Puppet avoids destroying directories unless the `force` attribute is set
+      to `true`. This means that if a file is currently a directory, setting
+      `ensure` to anything but `directory` or `present` will cause Puppet to
+      skip managing the resource and log either a notice or an error.
+
+      There is one other non-standard value for `ensure`. If you specify the
+      path to another file as the ensure value, it is equivalent to specifying
+      `link` and using that path as the `target`:
 
           # Equivalent resources: