Uploaded image for project: 'Puppet'
  1. Puppet
  2. PUP-6959

Puppet::FileSystem.open / exclusive_open / replace_file should use Windows file system support for setting `mode`

    XMLWordPrintable

Details

    • Improvement
    • Status: Ready for Engineering
    • Normal
    • Resolution: Unresolved
    • PUP 4.8.1
    • None
    • Windows
    • Hide
      • Specifying a mode parameter demonstrably sets file permissions (based on octal mode) on Windows.
      • An audit of the forge should be done at the same time as an audit of the Puppet code to determine extent of reliance on existing behavior.
      Show
      Specifying a mode parameter demonstrably sets file permissions (based on octal mode) on Windows. An audit of the forge should be done at the same time as an audit of the Puppet code to determine extent of reliance on existing behavior.
    • Coremunity
    • 3
    • Needs Assessment

    Description

      While working on some test fixes for PUP-6188, it was noticed that the mode parameter being accepted mimics the behavior of the Ruby File APIs. These APIs are mostly broken on Windows and do weird things to calculate a mode based on Windows File attributes (like read-only, archive, reparse point, etc) instead of attempting to simulate based on actual ACL permissions (see code at https://github.com/ruby/ruby/blob/trunk/win32/win32.c#L5419-L5465). Therefore, the Ruby File.stat(path).mode will typically not roundtrip properly.

      Puppet already works around Rubys mode behavior, and will patch stat and the stats mode when using the Puppet::FileSystem.stat or similar methods. However, when specifying a mode parameter explicitly to Puppet::FileSystem.open or exclusive_open, instead of using the Puppet helpers, the Ruby behavior is observed. This is probably not what we want, as it's mostly useless.

      Some tests were added documenting this behavior in:
      https://github.com/puppetlabs/puppet/commit/0a582a3c3a2aae03a68efaea0556e3d8f3c68b47

      The proposal is to call the Windows helper when passing a mode in any of the Puppet::FileSystem APIs. This is technically a breaking change, even if its doubtful any existing code actually relies on this strange behavior. An audit of the forge should be done at the same time as an audit of the Puppet code. This is tentatively targeted at Puppet 5 to make the official break.

      Attachments

        Issue Links

          Activity

            People

              Unassigned Unassigned
              ethan Ethan Brown
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

                Created:
                Updated:

                Zendesk Support