Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fetch the registry value not the type #2684

Merged
merged 1 commit into from
Mar 1, 2024

Conversation

joshcooper
Copy link
Contributor

Facter's product release resolver called Win32::Registry#each and incorrectly assumed the second parameter was the value, when actually it was the type. Note the each method is an alias for each_value, which yields three parameters: name, type and value[1]`

The issue wasn't noticed because the code also called Win32::Registry#[] to get the value and the tests incorrectly stubbed the Windows registry behavior.

Commit 2c291fe assumed the second parameter was the value, as a result the os.windows facts had a value of 1, which corresponds to REG_SZ[2]:

C:\> facter -j os.windows
"os.windows": {
  "edition_id": 1,
  ..
}

This isn't the first time we've had problems with overstubbing in facter, see 6e7970e, 86048b5, 5818550

Fixes #2683

[1] https://github.com/ruby/ruby/blob/v3_2_3/ext/win32/lib/win32/registry.rb#L579
[2] https://github.com/ruby/ruby/blob/v3_2_3/ext/win32/lib/win32/registry.rb#L114

Facter's product release resolver called `Win32::Registry#each` and incorrectly
assumed the second parameter was the `value`, when actually it was the `type`.
Note the `each` method is an alias for `each_value`, which yields three
parameters: `name`, `type` and `value`[1]`

The issue wasn't noticed because the code also called `Win32::Registry#[]` to
get the value and the tests incorrectly stubbed the Windows registry behavior.

Commit 2c291fe assumed the second parameter was the `value`, as a result
the `os.windows` facts had a value of 1, which corresponds to `REG_SZ`[2]:

    C:\> facter -j os.windows
    "os.windows": {
      "edition_id": 1,
      ..
    }

This isn't the first time we've had problems with overstubbing in facter, see
6e7970e, 86048b5, 5818550

Fixes puppetlabs#2683

[1] https://github.com/ruby/ruby/blob/v3_2_3/ext/win32/lib/win32/registry.rb#L579
[2] https://github.com/ruby/ruby/blob/v3_2_3/ext/win32/lib/win32/registry.rb#L114
@joshcooper joshcooper requested a review from a team as a code owner March 1, 2024 20:44
@joshcooper
Copy link
Contributor Author

I verified the only fact differences between 4.5.2 and this are volatile facts like uptime:

# diff facts-old.json facts-fixed.json
11c11
<   "facterversion": "4.5.2",
---
>   "facterversion": "4.6.0",
28,30c28,30
<       "available": "2.19 GiB",
<       "available_bytes": 2355445760,
<       "capacity": "45.14%",
---
>       "available": "2.21 GiB",
>       "available_bytes": 2370854912,
>       "capacity": "44.79%",
33,34c33,34
<       "used": "1.81 GiB",
<       "used_bytes": 1938493440
---
>       "used": "1.79 GiB",
>       "used_bytes": 1923084288
118,120c118,120
<     "hours": 0,
<     "seconds": 2751,
<     "uptime": "0:45 hours"
---
>     "hours": 2,
>     "seconds": 8092,
>     "uptime": "2:14 hours"

@mhashizume mhashizume added the bug Something isn't working label Mar 1, 2024
@mhashizume mhashizume merged commit 0d2dcdb into puppetlabs:main Mar 1, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regression in os.windows facts
2 participants