(PUP-3653) Create/set empty Windows groups#3786
Closed
ferventcoder wants to merge 1 commit intopuppetlabs:3.xfrom
Closed
(PUP-3653) Create/set empty Windows groups#3786ferventcoder wants to merge 1 commit intopuppetlabs:3.xfrom
ferventcoder wants to merge 1 commit intopuppetlabs:3.xfrom
Conversation
Contributor
Author
There was a problem hiding this comment.
I bumped this up to keep this entire section from running unless inclusive.
|
CLA signed by all contributors. |
ferventcoder
force-pushed
the
ticket/3.x/PUP-3653-empty-windows-groups
branch
2 times, most recently
from
a0f4935 to
da57181
Compare
Allow setting a Windows group to no users. Previously it was not possible to set Windows groups with no members due to the way the checking of should versus current was done. Without this change Puppet will not be able to set Windows groups to empty lists. - `group/windows_adsi.members_insync?` - Remove the short circuit check on `current.empty?` versus `should_empty` - this results in incorrect behavior when non-authoritative - `group/windows_adsi.members_insync?` - Return true when non-athoritative and should is empty. This prevents a check on keys of an empty hash, which will result in an error. - `adsi.set_members` - return early only if `desired_members` is nil - `adsi.set_members` - skip adding members when `desired_members` is empty - `adsi.set_members` - if `desired_members` is empty, remove all current members
ferventcoder
force-pushed
the
ticket/3.x/PUP-3653-empty-windows-groups
branch
from
da57181 to
ccb7d25
Compare
ferventcoder
changed the title
Contributor
Author
There was a problem hiding this comment.
Yanked b/c the check would need to be run only when inclusive and we already have logic that works for this below. If we put this back for any reason, it also has a bug in should_empty set because assignment to logical boolean.
So it would need to come back as
should_empty = should.nil? || should.empty?
Contributor
Author
There was a problem hiding this comment.
This changes what was brought in with 455834e
Contributor
|
Merged to 3.x in 6b8d187 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Allow setting a Windows group to no users. Previously it was not possible to set
Windows groups with no members due to the way the checking of should versus
current was done. Without this change Puppet will not be able to set Windows
groups to empty lists.
group/windows_adsi.members_insync?- Remove the short circuit check oncurrent.empty?versusshould_empty- this results in incorrect behavior when non-authoritativegroup/windows_adsi.members_insync?- Return true when non-athoritative and should is empty.This prevents a check on keys of an empty hash, which will result in an error.
adsi.set_members- return early only ifdesired_membersis niladsi.set_members- skip adding members whendesired_membersis emptyadsi.set_members- ifdesired_membersis empty, remove all current members