Skip to content

#1076: User Type: Show warning if an empty group is specified#528

Merged
cprice404 merged 5 commits intopuppetlabs:2.7.xfrom
stschulte:ticket/2.7.x/1076
Apr 6, 2012
Merged

#1076: User Type: Show warning if an empty group is specified#528
cprice404 merged 5 commits intopuppetlabs:2.7.xfrom
stschulte:ticket/2.7.x/1076

Conversation

@stschulte
Copy link
Contributor

If one specifies an empty string for the group property of the user type, puppet builds an incorrect usermod command.

Example taken from #1076

user { "pinky": 
  gid => "pinky",
  ensure => present,
  groups => ""
}

The correct way to remove all group membership is to use groups => [] so we should guide the user in that direction.

The commit series also tries to clean up the user_spec.

If rspec already gives us the described class why not using it.
Add a real provider class we can use in the tests.
Remove all references to the mock resource object
Also remove the modification of the PATH environment variable because we
will always have our :simple provider which does rely on any commands in
/sbin.
If the user passes an empty group to the user type, puppet will execute
an invalid `usermod` command.

e.g. if groups of user 'foo' is currently 'bar' and we do

    user { 'foo':
      groups     => '',
      membership => minimum # this is the default
    }

puppet will build a should value of ',bar' (old group + the new group '') and
will pass '-G ,bar' to the usermod command (which will then fail).

To really specifiy 'no group' the group property does allow an empty
array so we should fail if we find an empty string as a group.
@pcarlisle
Copy link
Contributor

looks good to me

@cprice404
Copy link

have run specs and acceptance locally, all green. haven't reviewed much further than that yet.

@cprice404
Copy link

This pull request looks good to me, and I've run it through specs and 2-node acceptance tests against 2.7.x/HEAD. Will merge first thing tomorrow morning... too late in the day to do it right now.

cprice404 added a commit that referenced this pull request Apr 6, 2012
#1076: User Type: Show warning if an empty group is specified
@cprice404 cprice404 merged commit 537488f into puppetlabs:2.7.x Apr 6, 2012
ahpook pushed a commit to ahpook/puppet that referenced this pull request Aug 30, 2012
This reverts commit d2012ae.

The chkconfig line can use `-` to indicate the service should
not be enabled by default.  In this case, `reset` disables all
runlevels, which is the opposite of what `enable=>true` should do.

The original bug referenced in #15797, puppetlabs#528, was also reverted
for (presumably) the same reason:
    puppetlabs@41e1285
ahpook pushed a commit to ahpook/puppet that referenced this pull request Aug 30, 2012
This reverts commit d2012ae.

The chkconfig line can use `-` to indicate the service should
not be enabled by default.  In this case, `reset` disables all
runlevels, which is the opposite of what `enable=>true` should do.

The original bug referenced in #15797, puppetlabs#528, was also reverted
for (presumably) the same reason:
    puppetlabs@41e1285
melissa pushed a commit to melissa/puppet that referenced this pull request Mar 30, 2018
(PCP-631) Use Beaker 3.6.0 for fix to reboot
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.

3 participants