Jira (PUP-10602) Registry key values being truncated or superfluous newline present

බැලීම් 0
පළමු නොකියවූ පණිවිඩය දක්වා මඟ හරින්න

Ciaran McCrisken (Jira)

නොකියවූ,
2020 ජූලි 28, 10.31.032020-07-28
සිට puppe...@googlegroups.com
Ciaran McCrisken updated an issue
 
Puppet / Bug PUP-10602
Registry key values being truncated or superfluous newline present
Change By: Ciaran McCrisken
Summary: {brief summary of issue} Registry key values being truncated or superfluous newline present
Add Comment Add Comment
 
This message was sent by Atlassian Jira (v8.5.2#805002-sha1:a66f935)
Atlassian logo

Ciaran McCrisken (Jira)

නොකියවූ,
2020 ජූලි 28, 10.32.032020-07-28
සිට puppe...@googlegroups.com
Ciaran McCrisken commented on Bug PUP-10602
 
Re: Registry key values being truncated or superfluous newline present

Will attach a manifest and logs when ABS is back up and running for me to provision hosts with. Would use Vagrant locally, but hitting issue with PA-3351 too.

Tests will generate manifests in system Temp folder that could be useful for testing.

Also see IAC-967 for details.

Ciaran McCrisken (Jira)

නොකියවූ,
2020 ජූලි 28, 10.34.032020-07-28
සිට puppe...@googlegroups.com

See this conversation on Slack: https://puppet.slack.com/archives/CF418PB9A/p1595942267015200

Thanks to Gheorghe Popescu for highlighting this change as a potential (and likely) cause: https://github.com/puppetlabs/puppet/commit/8255b92ac12558fa73b9ada07932f204fa48df93

Ciaran McCrisken (Jira)

නොකියවූ,
2020 ජූලි 28, 10.44.042020-07-28
සිට puppe...@googlegroups.com
Ciaran McCrisken updated an issue
Change By: Ciaran McCrisken
*Puppet Version:* 7.0.0 (7.0.0.104.g28ab897c nightly)
*Puppet Server Version:* n/a
*OS Name/Version:* Windows 10 / Server 2012 R2 / Server 2016 / Server 2019

*Problem Description*
The acceptance tests in the [puppetlabs-motd|https://github.com/puppetlabs/puppetlabs-motd] module are failing with the latest Puppet 7 nightly build (2020-07-27) due to the registry values being read back with either the last character being truncated or an additional newline character present:
{code}
# ./spec/acceptance/motd_spec.rb:75
1) Message of the day when static message from content
     On host `profound-letter.delivery.puppetlabs.net'
     Failure/Error: idempotent_apply(pp)
     RuntimeError:
       apply manifest expected no changes
       ` puppet apply manifest_20200728_88010_1qq4dui.pp --trace --detailed-exitcodes`
       ====== Start output of Puppet apply with unexpected changes ======
       Notice: Compiled catalog for profound-letter.delivery.puppetlabs.net in environment production in 0.15 seconds
       Notice: /Stage[main]/Motd/Registry_value[HKEY_LOCAL_MACHINE\Software\Microsoft\Windows\CurrentVersion\policies\system\legalnoticecaption]/data: data changed 'Message of the da' to 'Message of the day'
       Notice: /Stage[main]/Motd/Registry_value[HKEY_LOCAL_MACHINE\Software\Microsoft\Windows\CurrentVersion\policies\system\legalnoticetext]/data: data changed 'Hello world!' to "Hello world!\n"
       Notice: Applied catalog in 0.17 seconds
{code}


{code}
'Message of the da' to 'Message of the day'
{code}
{code}
'Hello world!' to "Hello world!\n"
{code}

*Steps to Reproduce:*
Clone the [puppetlabs-motd|https://github.com/puppetlabs/puppetlabs-motd] module, then, append the following lines to the end of the *provision.yaml:*
{code}
win_test:
  provisioner: abs
  images: ['win-2012r2-x86_64', 'win-2016-core-x86_64', 'win-2019-core-x86_64', 'win-10-pro-x86_64']
{code}
Then run:
{code}
bundle install
bundle exec rake 'litmus:provision_list[win_test]'
bundle exec rake 'litmus:install_agent[puppet7-nightly]'
bundle exec rake 'litmus:install_module'
bundle exec rake 'litmus:acceptance:parallel'
{code}
 *Then please do not forget to tear down and hand back the hosts after you're done:*
{code}
bundle exec rake 'litmus:tear_down'
{code}
 *Desired Behavior:*
All acceptance tests should pass on the module

*Actual Behavior:*
Acceptance tests fail due to registry values either being truncated or a superfluous new line character present.

Ciaran McCrisken (Jira)

නොකියවූ,
2020 ජූලි 28, 12.36.042020-07-28
සිට puppe...@googlegroups.com
 
Re: Registry key values being truncated or superfluous newline present

This manifest should reproduce the issue:

    class { motd:
 
      content => "Hello world!
 
",
 
    }

Josh Cooper (Jira)

නොකියවූ,
2020 ජූලි 28, 14.40.032020-07-28
සිට puppe...@googlegroups.com
Josh Cooper commented on Bug PUP-10602

The registry module was relying on puppet's wide_string method to embed a wide null within the string content, so when using registry module with puppet 7, the module ends up writing a value that is not properly null terminated. Note the call to
RegSetValueExW doesn't append a wide null for REG_SZ or REG_EXPAND_SZ or two nulls for REG_MULTI_SZ.

The next time puppet runs, the registry module uses ruby's builtin Win32::Registry code to read the value, and that code chop's the trailing newline. Since "hello world" doesn't match "hello world\n", puppet applies the change again.

Fixing the puppetlabs-registry module should be fairly straightforward, I'll take a look at that.

Josh Cooper (Jira)

නොකියවූ,
2020 ජූලි 28, 20.07.022020-07-28
සිට puppe...@googlegroups.com
Josh Cooper assigned an issue to Josh Cooper
 
Change By: Josh Cooper
Assignee: Josh Cooper

Josh Cooper (Jira)

නොකියවූ,
2020 ජූලි 28, 20.07.032020-07-28
සිට puppe...@googlegroups.com
Josh Cooper commented on Bug PUP-10602
 
Re: Registry key values being truncated or superfluous newline present

I have a topic branch that fixes the problem for the registry module when using puppet 7. It doesn't work with puppet 6, as registry values end up with an extra set of terminators: https://github.com/puppetlabs/puppetlabs-registry/compare/master...joshcooper:registry_wide_string_iac?expand=1

So some options:

1. The PR could be updated to only append the terminators when running on puppet 7 or above
2. We could add an optional argument to `wide_string` in puppet. If the optional argument is specified, then fallback to the old behavior. The registry module would have to check the arity of the method which is kind of gross.
3. We could revert the change in puppet, and add a `P::U::W::String` method that has the new behavior. The registry module could update to the new thing whenever.
4. Others?

3 is the safest, but also means puppet has to carry technical debt, and the whole point of the major release is to reduce debt.

I think 1 is my preference but it depends on how many other Windows modules could be affected? My guess is that it's very usual for a Windows provider (outside of puppetlabs) to be doing wide string conversions? I also don't think reboot, acl or other windows modules from puppetlabs have this issue, but could be wrong.

Ciaran McCrisken (Jira)

නොකියවූ,
2020 ජූලි 29, 08.54.032020-07-29
සිට puppe...@googlegroups.com

Thanks Josh Cooper for the explanation and potential solutions. Doing an audit of what falls under our supported module, there were only a few hits:

I'll be chatting with the rest of the IAC Team today about the issue and the options you put forward to try and determine how we want to proceed. There's a few things we'll need to consider. I'll get back after that conversation.

Josh Cooper (Jira)

නොකියවූ,
2020 ජූලි 29, 10.53.032020-07-29
සිට puppe...@googlegroups.com
Josh Cooper commented on Bug PUP-10602

Thanks Ciaran McCrisken! I also realized the wide_string behavior change should not affect any of the other windows modules, because the registry is a bit special. When setting a registry value, the caller must convert the wide string to a byte array taking into account REG_SZ/REG_EXPAND_SZ vs REG_MULTI_SZ wide terminator differences, and call RegSetValueExW with that binary blob and a cbData parameter which includes the terminators. However, most other places where wide_string is called, just pass the returned wide string directly to a Windows API, and should work before and after the puppet change. For example the scheduled task https://github.com/puppetlabs/puppetlabs-scheduled_task/blob/master/spec/legacy_taskscheduler.rb#L250 works as is with puppet 7. Given that I'm inclined to just fix the registry and call it good.

Josh Cooper (Jira)

නොකියවූ,
2020 ජූලි 29, 10.57.042020-07-29
සිට puppe...@googlegroups.com
Josh Cooper updated an issue
 
Change By: Josh Cooper
Team: Coremunity

Ciaran McCrisken (Jira)

නොකියවූ,
2020 ජූලි 29, 11.10.042020-07-29
සිට puppe...@googlegroups.com
Ciaran McCrisken commented on Bug PUP-10602
 
Re: Registry key values being truncated or superfluous newline present

We were coming to the same conclusion ourselves after a chat earlier Josh Cooper. If you're also saying this is quite specific to handle the different registry value types, then we've narrowed the impact further, which pushes more in the direction of applying the fix you did to puppetlabs-registry.

A quick check shows 671 Windows modules on the Forge - I've pinged Ben Ford about any way we could potentially audit those modules to look for any calls to wide_string. I would suspect the hits to be very low, if not 0, but do you think this is a worthwhile endeavour?

FWIW, I did a quick check through Voxpupuli's Github repo for "wide_string" and got no results back.

I'm running tests against your PR locally. I created a PR to kick off the CI tests, but there was a Rubocop violation causing it to bomb out early. Would you mind approving and merging this PR to yours when you get a chance? https://github.com/joshcooper/puppetlabs-registry/pull/2

Thanks again for your help!

Josh Cooper (Jira)

නොකියවූ,
2020 ජූලි 29, 13.14.032020-07-29
සිට puppe...@googlegroups.com
Josh Cooper commented on Bug PUP-10602

but do you think this is a worthwhile endeavour?

If it's easy to query, then sure, but I wouldn't spend a lot of time on it.
සියල්ලට පිළිතුරු දෙන්න
කර්තෘට පිළිතුරු දෙන්න
ඉදිරියට යවන්න
නව පණිවිඩ 0