Windows File Provider#134
Merged
ChaseMotorman merged 18 commits intopuppetlabs:2.7.xfrom Sep 30, 2011
Merged
Conversation
This commit significantly reorganizes spec/unit/type/file_spec.rb, in preparation for refactoring of the type to support Windows. It also adds tests for most of the non-trivial untested methods of the file type. Disabled symlink tests on Windows, removed duplicate #remove_existing tests, fixed absolute path drive letter issue with the title, and changed tests that added an expectation to File.unlink, as cleanup is not able to unlink them.
These changes are to simplify and improve the specs in preparation for refactoring of the file type to support Windows.
This code was taking special care to properly validate and munge the path parameter in the case where a non-Windows master was compiling a catalog for a Windows agent. However, validation and munging are only performed on the agent, so we can simply use the available platform-specific methods for verifying and splitting the path.
Rather than implementing this logic of ascending to the root directory and searching for parents ourselves, we now just use the functionality provided by the Pathname class.
When munging file sources, we previously only removed trailing forward slashes, but on Windows, a trailing backslash is equivalent, so in that case we ought to remove that as well.
The Windows FILE_DELETE_CHILD right was not being set on directories when the user had write and execute permission. When translating a POSIX permission for a specific class, e.g. user, we right shift the appropriate number of bits, e.g. 6, and then compare using the 'other' permission contants, e.g. S_IWOTH.
Previously, the logic for getting and setting file mode was implemented in the type's retrieve and sync methods of the mode property itself. This commit adds a validate method to the type (previously validating was being done in munge), and adds 'mode' and 'mode=' methods to the POSIX and Windows file providers. The POSIX provider does what it did before, and the Windows provider contains the same broken mode-logic that it did before. Actually translating file permissions, e.g. 0755, to Windows will be done in a later commit. This commit also fixes tests on Windows where the file mode was being set by Puppet::Util::Windows::Security, but read back using File.stat, which doesn't round-trip. Now the test uses the security module for both setting and getting. Disabled symlink tests on Windows, which are not supported.
This commit adds the ability to retrieve the Security ID for Windows users and groups. This will be necessary for translating from (trustee) names to SIDs as part of managing file owners and groups.
Previously the Puppet::Util::Windows::Security#string_to_sid_ptr method required a block to be passed to it. This commit makes the block optional, and simply returns true if the string SID, e.g. 'S-1-5-18', can be converted into a SID pointer (pointer to the binary SID structure). If the string SID cannot be converted, then a Puppet::Util::Windows::Error will be thrown. If the string SID is invalid, then the P::U::W::E#code will return 1337. This commit is being implemented so that we can validate SIDs prior to converting from string SIDs to names.
Puppet::File_Serving::MetaData was not able to retrieve owner, group or mode on Windows, because the underlying Ruby File.stat implementation always returns 0 for uid and gid, and returns either 644 or 444 (the latter for read-only). This commit wraps File::Stat in platform-specific objects that delegate to the appropriate object and method. By default, owner, group, mode, ftype delegate to methods on the normal File::Stat object. On Windows, we delegate owner, group, and mode to the get/set class methods of Puppet::Util::Windows::Security.
Previously, the logic for getting and setting the owner of a file, including validating owner names, detecting if the property is in sync, and detecting if we are running as root (and can manage file ownership) was spread across the owner property (type) and file providers. This commit changes the owner property (type) to only perform logic that is common across all file providers and to delegate to its provider for operations that are platform-specific. Examples of the former are detecting if the properties are in sync, detecting if we are running as root, and overriding the is_to_s and should_to_s methods (for displaying owners as strings). Examples of the latter are mapping owner names to user ids and back, and implementing the 'owner' and 'owner=' methods. This commit also changes the group property to always be insync? on Windows, as this functionality will be implemented in a later commit. It also re-enables file integration tests that previously had been failing on Windows due to the lack of file provider support.
Previously the User and Group classes defined their own method for resolving a name into a SID with the assumption that we might need to customize the WMI query for each type of object (by including the SIDType in the where clause). But it turns out that the Name and Domain properties of the Win32_Account table are a compound key, so the method for resolving names into SIDs can be refactored. Also, eliminated duplicate, and slightly incorrect code from the Windows security integration test (it wasn't restricting the query to local accounts only).
Move provider-specific logic from the file group property to the posix file provider. For example, mapping group IDs to names and vice-versa is provider specific, but detecting if the property value is insync? is common across all file providers. Modify the windows file provider to allow getting and setting the group via its 'group' and 'group=' methods. Also, on Windows the file owner can be a group, and the file group can be a user, so the same method for mapping names and SIDs can be used for both owner and group. Added test cases where there weren't any.
Previously the regex used to detect Windows absolute paths only applied the caret to the first part of the regex (the part that matched drive letters). But the parts used to detect UNC paths (\\server\name) and prefixed paths (\\?\C:\foo) were matching any part of the string. And since / and \ are valid path separators on Windows, URIs like puppet://foo/bar (used as file sources) were incorrectly thought to be absolute paths. This commit ensures the absolute path regex is restricted to the start of the string for all cases.
Added Puppet::Util.path_to_uri and uri_to_path. These methods will be needed to support Windows file paths as the file 'source' property, metadata, and indirector. The path_to_uri method takes an absolute path and constructs a file URI, e.g. 'file:/foo'. Note this is equivalent to 'file:///foo' as in both cases the authority component of the URI is optional. On Windows local file paths are typically URI-ized as 'file:/C:/foo' (or 'file:///C:/foo'), whereas 'file://host/C:/foo' is used to express UNC paths. The uri_to_path method takes a URI (not necessarily a file URI) and extracts the unescaped path portion. So given a URI of 'file:/foo%20bar', the method returns '/foo bar'. On Windows, this method strips the leading slash from file URIs that contain drive letters. It also converts UNC URIs of the form 'file://host/share/file' to '//host/share/file'
The file serving indirection hooks attempts to short-circuit requests whose key is an absolute path or a file protocol. Previously, it was performing platform-specific regex matching to detect absolute paths. This commit changes it to use the Puppet::Util.absolute_path? method that does just that.
Previously, we were trying to detect absolute-path-ness if File.expand_path == path, but we now have a method for checking absolute pathness, so we use that.
Previously, specifying a Windows file URI of the form 'file:///C:/foo'
as a file source failed to strip the leading slash when attempting to
source the file. Also there was ambiguity after values were munged (a
value of the form 'C:/foo' could either be a Windows file path or a
URI whose scheme is 'C').
This commit changes the file source to be more deliberate in how it
validates source properties, including only allowing absolute paths
and 'puppet' and 'file' URIs, which are both absolute and
hierarchical. Also it uses the Puppet::Util.path_to_uri method to
handle file path to URI translation issues.
Previously, if a request was created using a Windows file URI of the
form 'file:///C:/foo', then the set_uri_key method wasn't stripping
the leading slash, and setting the request key to '/C:/foo'. This
caused problems when attempting to collect metadata for Windows
files. This commit changes the request class to use the
Puppet::Util.uri_to_path method to handle URI path to file path
translation issues.
Previously, if a file URI was created programmatically using ruby's
built-in URI class, such as occurs when specifying file source URIs,
then calling URI#to_s omits the authority component, e.g. 'file:/foo'
instead of 'file:///foo'. This commit changes the URI regex to not
require two slashes, but note that the order of operations is
important as Windows file paths will match the URI regex and can be
successfully parsed: URI.parse('c:/foo').scheme == 'c'
Contributor
|
This pull request is really large, and it seems many of the commits, although associated, are unrelated. Since a build failure resulting from any single commit might invalidate the entire pull, I would recommend breaking it up into smaller pull requests. In particular, all the mods relating to file paths and URIs seems general enough to be refactored. |
Contributor
Author
|
FWIW, I ran rake unit spec after each commit (as part of rebase interactive) to ensure there were no inter-commit dependencies. |
Contributor
|
Good commit. I ran through the big changes and tried to break it, but couldn't find any obvious weaknesses. Good test coverage, as well. However, by setting group ownership to one in which the user is not a member, you can get into a state where it is impossible to recover. For example, it became impossible for the MSI installer (which runs as SYSTEM) to read or update a package (I had to manually take ownership, and reset permissions to recover). When trying to change the owner/group properties on a large file tree (Apache, for instance), it took a very long time (over 10 minutes on on WS2008). |
Contributor
|
Auto-merge failed due to ADSI code/spec conflicts. The merge was resolved, and commit was manually pushed upstream. Commit: ec587b4: Merge branch 'file-providers' of https://github.com/joshcooper/puppet into 2.7.x |
Iristyle
pushed a commit
to Iristyle/puppet
that referenced
this pull request
Sep 9, 2014
(re-1208) Update Requires for sles12
hlindberg
pushed a commit
to hlindberg/puppet
that referenced
this pull request
Oct 16, 2014
…masterd_block_thanks Switch to [master] for config block in example
melissa
pushed a commit
to melissa/puppet
that referenced
this pull request
Mar 30, 2018
(PCP-57) Determine default path for modules dir
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This patch series adds Windows file provider support mostly implemented by Nick Lewis. The initial set of changes provide a cleaner separation between file properties (owner, group, and mode) and the POSIX and Windows file providers. The second set of changes fixed issues with Windows based file URIs when used as indirector request keys and file sources. Also many tests were added in places where there weren't any.