[PUP-6542] Group resource emits misleading change notification with auth_membership => false Created: 2016/07/22  Updated: 2018/10/24  Resolved: 2018/09/26

Status: Closed
Project: Puppet
Component/s: Types and Providers, Windows
Affects Version/s: PUP 4.5.2
Fix Version/s: PUP 5.5.7, PUP 6.0.1

Type: Bug Priority: Normal
Reporter: Ethan Brown Assignee: Enis Inan
Resolution: Fixed Votes: 0
Labels: group, type_and_provider, user, windows
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Duplicate
is duplicated by PUP-7585 Windows group's "members changed" mes... Closed
Relates
relates to PUP-2628 Ability to add a member to a group, i... Closed
relates to PUP-3719 Group resource non-authoritative by d... Closed
relates to DOCUMENT-356 confusing documentation of *_membersh... Closed
Template: PUP Bug Template
Epic Link: WINning
Team: Platform OS
Sprint: Platform OS Kanban
CS Priority: Normal
CS Frequency: 2 - 5-25% of Customers
CS Severity: 3 - Serious
CS Business Value: 5 - $$$$$$
CS Impact: The output messages for this is pretty confusing. It appears as though it changes it to only the members specified, but it doesn't actually, which makes it behave like the user resource.

It's unclear which behavior will be less confusing. Should the group resource manage the entire state of a group, or be additive in the way the user resource is?

Windows is different in that groups can be members of other groups and having to respecify all members of a group may be frustrating.
Release Notes: Bug Fix
Release Notes Summary: The members property in the group resource has now been fixed to report the right change notifications to Puppet.

 Description   

When Puppet is adding users to an existing group, its change report is misleading.

Take the following manifest (from French Windows):

group { 'Administrateurs': 
	ensure => 'present', 
	auth_membership => false, 
	members => ['AUTORITE NT\\Utilisateurs authentifiés']
}

It emits the following message

# $ bundle exec puppet apply utf8.pp
# DL is deprecated, please use Fiddle
# Notice: Compiled catalog for wbsk7zl860xwiqa.delivery.puppetlabs.net in environment production in 0.06 seconds
# Notice: /Stage[main]/Main/Group[Administrateurs]/members: members changed 'WBSK7ZL860XWIQA\Administrateur,WBSK7ZL860XWIQA\tester,WBSK7ZL860XWIQA\Administrator,WBSK7ZL860XWIQA\cyg_server,WBSK7ZL860XWIQA\bla' to 'AUTORITE NT\Utilisateurs authentifiés'
# Notice: Applied catalog in 0.11 seconds

However, the message is incorrect, given the actual group membership:

$ bundle exec puppet resource group Administrateurs
DL is deprecated, please use Fiddle
group { 'Administrateurs':
  ensure  => 'present',
  gid     => 'S-1-5-32-544',
  members => ['Administrateur', 'tester', 'Administrator', 'cyg_server', 'bla', 'Utilisateurs authentifiés'],
}

Expected behavior should be like what the user resource does:

user { 'Invité': 
	ensure => 'present', 
	auth_membership => minimum, 
	groups => ['BUILTIN\\Administrateurs']
}

Emits the following message:

bundle exec puppet apply .\user.pp
DL is deprecated, please use Fiddle
Notice: Compiled catalog for wbsk7zl860xwiqa.delivery.puppetlabs.net in environment production in 0.08 seconds
Notice: /Stage[main]/Main/User[Invité]/groups: groups changed 'BUILTIN\Invités' to ['BUILTIN\Administrateurs', 'BUILTIN\Invités']
Notice: Applied catalog in 0.11 seconds

For resource

bundle exec puppet resource user
 
<snip/>
 
user { 'Invité':
  ensure  => 'present',
  comment => 'Compte d'utilisateur invité',
  groups  => ['BUILTIN\Invités', 'BUILTIN\Administrateurs'],
  uid     => 'S-1-5-21-441295449-3808246871-2843121223-501',
}



 Comments   
Comment by Branan Riley [ 2018/08/07 ]

I suspect this is not windows-specific - I'm pretty sure that notice comes from the resource harness. This might involve some surgery there.

Comment by Sean McDonald [ 2018/09/17 ]

Moving this back to ready. We are going to wait on a session with Branan to learn about the resource harness before tackling this. Should happen this week (week of 9/17) and we'll pull the ticket back in to doing then.

Comment by Enis Inan [ 2018/09/18 ]

This is something that will require some surgery into the members property in the group resource – I do not think anything needs to be done to the resource harness. Specifically, we'll need to configure this property to work like https://github.com/puppetlabs/puppet/blob/master/lib/puppet/property/list.rb, which is what the groups property in the User resource derives from – https://github.com/puppetlabs/puppet/blob/master/lib/puppet/type/user.rb#L318. The configuration would be to ensure that the right should value is returned based on what's passed into the auth_membership parameter (it currently returns just the raw should value, which is why we're getting the erroneous report). However, it seems like it'd be better to just have the members property work like the groups property in the User resource. This is why I think the work in this ticket consists of two parts:

  • Fix the bug in the affected series (maybe 5.5.x and 6.0.x ?). This will likely require some surgery into the other group providers to remove auth_membership logic over from the provider (which is how it's currently done) to the property.
  • Make the members property work like the groups property in the User resource. This is a breaking change, so maybe we target this at master?
Comment by Enis Inan [ 2018/09/18 ]

Actually, cleaner solution is to just derive from the `list` property and then override `inclusive?` to use the `auth_membership` param. as a boolean. This way, we don't need to do anything to the property itself. Going to implement that instead.

Comment by Kris Bosland [ 2018/09/26 ]

Passed CI in e2f7ccfb7

Generated at Wed May 22 22:41:48 PDT 2019 using JIRA 7.7.1#77002-sha1:e75ca93d5574d9409c0630b81c894d9065296414.