[PUP-4386] Windows Group resource reports errors incorrectly when specifying an invalid group member Created: 2015/04/07  Updated: 2015/11/11  Resolved: 2015/06/17

Status: Closed
Project: Puppet
Component/s: Types and Providers, Windows
Affects Version/s: PUP 3.8.0
Fix Version/s: PUP 3.8.5, PUP 4.2.0

Type: Bug Priority: Normal
Reporter: Ethan Brown Assignee: Unassigned
Resolution: Fixed Votes: 0
Labels: windows
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: PNG File Screen Shot 2015-06-17 at 10.59.49 AM.png    
Issue Links:
Relates
relates to PUP-1542 Puppet::Util::Log requires a message Closed
relates to PUP-4426 Backport PUP-4386 - group resource do... Closed
relates to PUP-4737 Group resource should use autorequire... Accepted
Template:
Story Points: 1
Sprint: Windows 2015-06-17
Release Notes: Bug Fix
QA Contact: Ryan Gard

 Description   

Because of the bug described in PUP-1542, extraneous messages may be output from Puppet when error message generation for an event fails in the resource harness sync_if_needed method.

There are 2 issues specifically in the Windows group code:

  • members_to_s expects a string, but is passed an array when trying to create the message, which is an easily correctable issue
  • members_to_s tries to resolve bad usernames to sid objects, then tries to access the account member of nil sid object

Initially spiked a solution for both of these issues in: https://github.com/Iristyle/puppet/commit/a6393b07772146101ae1894381e96cea25f05ed2

Simple repo example. Assume that the current system has:

bundle exec puppet resource group guests
 
group { 'guests':
  ensure  => 'present',
  gid     => 'S-1-5-32-546',
  members => ['Guest', 'Administrator'],
}

Trying to apply the following manifest with a bad user:

group { 'guests':
  ensure => present,
  members => ['Guest', 'Administrator', ''],
}
 
# generates
 
Notice: Compiled catalog for vagrant-2008r2.corp.puppetlabs.net in environment production in 0.22 seconds Error: Could not resolve username:
Error: /Group[Guests]: Could not evaluate: Puppet::Util::Log requires a message
Notice: Finished catalog run in 0.05 seconds

This spiked change does not lose error detail, nor trigger an extraneous /
misleading message:

Notice: Compiled catalog for vagrant-2008r2.corp.puppetlabs.net in environment production in 0.22 seconds Error: Could not resolve username:
Error: /Stage[main]/Main/Group[Guests]/members: change from VAGRANT-2008R2\Guest,VAGRANT-2008R2\Administrator to ["Guest", "Administrator", ""] failed: Could not resolve username:
Notice: Finished catalog run in 0.05 seconds



 Comments   
Comment by Josh Cooper [ 2015/04/09 ]

I removed 4.0 from the affected version because the problem has been present for awhile (as opposed to a newly introduced regression in 4.0)

Comment by Ethan Brown [ 2015/06/10 ]

There are two separate approaches up for review.

4010 - throws an exception in the Windows group provider that the corresponding type must catch
4024 - performs validation on values if the provider has a member_valid? method available

Both have to implement some sanity checking is the types is_to_s given that can be called from the resource_harness message building - https://github.com/puppetlabs/puppet/blob/stable/lib/puppet/transaction/resource_harness.rb#L147

Comment by Rob Reynolds [ 2015/06/10 ]

Leaning towards the validating PR (# 4024) instead of the earlier PR. Seems better to run validate instead.

Comment by Rob Reynolds [ 2015/06/10 ]

Merged into stable at fefe57c26.

Comment by Rob Reynolds [ 2015/06/11 ]

Passed specs at https://jenkins.puppetlabs.com/view/All%20in%20One%20Agent/view/Stable/view/Puppet/job/platform_aio-puppet_pkg-promote_stable/98/

Comment by James Pogran [ 2015/06/17 ]

Tested with http://builds.puppetlabs.lan/puppet-agent/42427827d2d923e040f0a6083fccdd2bda876858/artifacts/windows/puppet-agent-x64.msi

Using the following manifest example

group

{ 'guests': ensure => present, members => ['Guest', 'vagrant', ''], }

Run against win2012r2x64

Verified expected message is shown. Screenshot of test vm with output attached

Generated at Thu Aug 22 11:09:24 PDT 2019 using JIRA 7.7.1#77002-sha1:e75ca93d5574d9409c0630b81c894d9065296414.