Skip to content

(PUP-6184) Remove win32/dir usage#8258

Merged
gimmyxd merged 8 commits intopuppetlabs:mainfrom
GabrielNagy:PUP-6184/remove-win32dir-usage
Aug 13, 2020
Merged

(PUP-6184) Remove win32/dir usage#8258
gimmyxd merged 8 commits intopuppetlabs:mainfrom
GabrielNagy:PUP-6184/remove-win32dir-usage

Conversation

@GabrielNagy
Copy link
Copy Markdown
Contributor

@GabrielNagy GabrielNagy commented Aug 7, 2020

Defined missing functions, constants and structs in puppet/util/windows/file, and a private wstrip function in our Dir monkey-patch. The original gem monkey-patches the String class to define the method, but since we only use it in one place, we add it to Dir.

Also fixed some Ruby 2.7 warnings, and removed a stale require to Win32API which we don't use anymore.

Replaced win32-dir usage in install.rb with environment variables, and also removed the win32-security gem from project_data.yaml.

After this PR is merged we should no longer have dependencies on external win32 gems.

@GabrielNagy GabrielNagy requested review from a team August 7, 2020 14:30
@puppetcla
Copy link
Copy Markdown

CLA signed by all contributors.

@GabrielNagy GabrielNagy force-pushed the PUP-6184/remove-win32dir-usage branch 16 times, most recently from def97fc to a772017 Compare August 10, 2020 16:34
@GabrielNagy
Copy link
Copy Markdown
Contributor Author

Jenkins please test this,pxp-agent on windows10ent-64a,windows2012r2-64a,windows2016-64a,windows2019-64a with servertests

@GabrielNagy GabrielNagy force-pushed the PUP-6184/remove-win32dir-usage branch from a772017 to 03e9c1f Compare August 11, 2020 11:59
@GabrielNagy
Copy link
Copy Markdown
Contributor Author

Jenkins please test this on windows10ent-64a,windows2012r2-64a,windows2016-64a,windows2019-64a

@Dorin-Pleava Dorin-Pleava requested a review from a team August 11, 2020 14:53
@GabrielNagy GabrielNagy force-pushed the PUP-6184/remove-win32dir-usage branch from 6cbd3b6 to cccb89a Compare August 12, 2020 07:52
Comment thread lib/puppet/util/windows/monkey_patches/dir.rb Outdated
Comment thread lib/puppet/util/windows/monkey_patches/dir.rb Outdated
Comment thread lib/puppet/util/windows/string.rb Outdated
Comment thread spec/integration/util/windows/monkey_patches/dir_spec.rb
@GabrielNagy GabrielNagy force-pushed the PUP-6184/remove-win32dir-usage branch from cccb89a to 61ef18a Compare August 12, 2020 13:20
Defined missing functions, constants and structs in
`puppet/util/windows/file`, and a private `wstrip` function in our
monkey-patched Dir class. We had a similar method but we cannot use it
since it's defined on FFI::Pointer, whereas we need it on FFI::Buffer.

Use constants instead of strings to specify encodings.

Remove requires to win32/dir.

Fix ruby 2.7 warning in api_types.rb.
The Win32API library has been deprecated since Ruby 1.9. It was
originally added in b5fd953 but
sometime ago puppet has switched to using FFI for Win32 API calls.
We cannot require puppet stuff in the install.rb script as Puppet is
not installed at that point. Use environment variables for what we
need so we don't depend on win32/dir at all.

Dir::COMMON_APPDATA maps to ENV['ALLUSERSPROFILE']
Dir::PROGRAM_FILES maps to ENV['PROGRAMFILES']
Before this commit, attempts of installing or running puppet on Windows
via AWS Session Manager failed constantly with a `<class:Dir>':
uninitialized constant Dir::PERSONAL (Name Error)` error. This behaviour
was identified on EC2 Windows Server 2019 instances.

The issue is found in the `win32-dir` gem which at some point tries to
assign the `Dir::PERSONAL` constant to `Dir::MYDOCUMENTS` without
checking if it is initialized/defined first, resulting in the `Name
Error` error above. The unlikely scenario where this happens is as
follows:

The AWS Session Manager communicates with a `SSM Agent` installed on the
desired instance. A user called `ssm-user` is created, either manually
or automatically by the `SSM Agent` installer (only versions older than
2.3.612.0), to facilitate the connection between the two entities. The
root cause comes from this user which does not have a unique user
profile and instead is using `C:\Windows\system32\config\systemprofile`
so the gem cannot find a `Documents\My Documents` folder since such
folders don't actually exist for it.

Since Puppet doesn't actually use/needs this path, current fix is
defining the `MYDOCUMENTS` constant only if `PERSONAL` is also defined.
@GabrielNagy GabrielNagy force-pushed the PUP-6184/remove-win32dir-usage branch from 61ef18a to b04b9d2 Compare August 12, 2020 13:28
Comment thread lib/puppet/util/windows/monkey_patches/dir.rb
Comment thread lib/puppet/util/windows/monkey_patches/dir.rb
Comment thread lib/puppet/util/windows/monkey_patches/dir.rb
@GabrielNagy GabrielNagy force-pushed the PUP-6184/remove-win32dir-usage branch from e8a5d98 to 229bf10 Compare August 13, 2020 06:40
@GabrielNagy
Copy link
Copy Markdown
Contributor Author

Jenkins please test this,pxp-agent on windows10ent-64a,windows2012r2-64a,windows2016-64a,windows2019-64a with servertests

@gimmyxd gimmyxd merged commit 13d0ccb into puppetlabs:main Aug 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants