Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Win group membership fix check mode final #544

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
bugfixes:
- win_group_membership - Return accurate results when using check_mode - https://github.com/ansible-collections/ansible.windows/issues/532
15 changes: 12 additions & 3 deletions plugins/modules/win_group_membership.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -136,15 +136,15 @@ foreach ($member in $members) {
if ($state -in @("present", "pure") -and !$user_in_group) {
if (!$check_mode) {
$group.Add($member_sid)
$result.added += $group_member.account_name
}
$result.added += $group_member.account_name
$result.changed = $true
}
elseif ($state -eq "absent" -and $user_in_group) {
if (!$check_mode) {
$group.Remove($member_sid)
$result.removed += $group_member.account_name
}
$result.removed += $group_member.account_name
$result.changed = $true
}
}
Expand Down Expand Up @@ -172,8 +172,8 @@ if ($state -eq "pure") {
if ($user_to_remove) {
if (!$check_mode) {
$group.Remove($member_sid)
$result.removed += $current_member.account_name
}
$result.removed += $current_member.account_name
$result.changed = $true
}
}
Expand All @@ -192,4 +192,13 @@ else {
$result.members = @()
}

if ($check_mode) {
if ($result.added.count -gt 0) {
$result.members += $result.added
}
if ($result.removed.count -gt 0) {
$result.members = $result.members | Where-Object { $_ -notin $result.removed }
}
}

Exit-Json -obj $result
55 changes: 39 additions & 16 deletions tests/integration/targets/win_group_membership/tasks/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@
assert:
that:
- add_users_to_group.changed == true
- add_users_to_group.added == []
- add_users_to_group.members == []
- add_users_to_group.added == ["{{ ansible_hostname }}\\{{ admin_account_name }}", "{{ ansible_hostname }}\\{{ win_local_user }}", "NT AUTHORITY\\SYSTEM"]
- add_users_to_group.members == ["{{ ansible_hostname }}\\{{ admin_account_name }}", "{{ ansible_hostname }}\\{{ win_local_user }}", "NT AUTHORITY\\SYSTEM"]
when: in_check_mode


Expand All @@ -81,6 +81,13 @@
- add_users_to_group_again.members == ["{{ ansible_hostname }}\\{{ admin_account_name }}", "{{ ansible_hostname }}\\{{ win_local_user }}", "NT AUTHORITY\\SYSTEM"]
when: not in_check_mode

- name: Test add_users_to_group_again (check mode)
assert:
that:
- add_users_to_group_again.changed == true
- add_users_to_group_again.added == ["{{ ansible_hostname }}\\{{ admin_account_name }}", "{{ ansible_hostname }}\\{{ win_local_user }}", "NT AUTHORITY\\SYSTEM"]
- add_users_to_group_again.members == ["{{ ansible_hostname }}\\{{ admin_account_name }}", "{{ ansible_hostname }}\\{{ win_local_user }}", "NT AUTHORITY\\SYSTEM"]
when: in_check_mode

- name: Add different syntax users to group (again)
win_group_membership:
Expand All @@ -102,8 +109,8 @@
assert:
that:
- add_different_syntax_users_to_group_again.changed == true
- add_different_syntax_users_to_group_again.added == []
- add_different_syntax_users_to_group_again.members == []
- add_different_syntax_users_to_group_again.added == ["{{ ansible_hostname }}\\{{ admin_account_name }}", "{{ ansible_hostname }}\\{{ win_local_user }}"]
- add_different_syntax_users_to_group_again.members == ["{{ ansible_hostname }}\\{{ admin_account_name }}", "{{ ansible_hostname }}\\{{ win_local_user }}"]
when: in_check_mode


Expand All @@ -126,8 +133,8 @@
assert:
that:
- add_another_user_to_group.changed == true
- add_another_user_to_group.added == []
- add_another_user_to_group.members == []
- add_another_user_to_group.added == ["NT AUTHORITY\\NETWORK SERVICE"]
- add_another_user_to_group.members == ["NT AUTHORITY\\NETWORK SERVICE"]
when: in_check_mode


Expand All @@ -142,7 +149,14 @@
- add_another_user_to_group_again.added == []
- add_another_user_to_group_again.members == ["{{ ansible_hostname }}\\{{ admin_account_name }}", "{{ ansible_hostname }}\\{{ win_local_user }}", "NT AUTHORITY\\SYSTEM", "NT AUTHORITY\\NETWORK SERVICE"]
when: not in_check_mode


- name: Test add_another_user_to_group_1_again (check mode)
assert:
that:
- add_another_user_to_group_again.changed == true
- add_another_user_to_group_again.added == ["NT AUTHORITY\\NETWORK SERVICE"]
- add_another_user_to_group_again.members == ["NT AUTHORITY\\NETWORK SERVICE"]
when: in_check_mode

- name: Remove users from group
win_group_membership: &wgm_absent
Expand Down Expand Up @@ -196,7 +210,7 @@
- remove_different_syntax_users_from_group_again.members == ["NT AUTHORITY\\NETWORK SERVICE"]
when: not in_check_mode

- name: Test add_different_syntax_users_to_group_again (check-mode)
- name: Test remove_different_syntax_users_to_group_again (check-mode)
assert:
that:
- remove_different_syntax_users_from_group_again.changed == false
Expand Down Expand Up @@ -242,13 +256,14 @@
when: not in_check_mode


# Explicitly disable check_mode when seting up users for pure testing
- name: Setup users for pure testing
win_group_membership:
<<: *wgm_present
members:
- "{{ admin_account_name }}"
- NT AUTHORITY\NETWORK SERVICE

check_mode: false

- name: Define users as pure
win_group_membership: &wgm_pure
Expand All @@ -269,9 +284,9 @@
assert:
that:
- define_users_as_pure.changed == true
- define_users_as_pure.added == []
- define_users_as_pure.removed == []
- define_users_as_pure.members == []
- define_users_as_pure.added == ["{{ ansible_hostname }}\\{{ win_local_user }}", "NT AUTHORITY\\SYSTEM"]
- define_users_as_pure.removed == ["NT AUTHORITY\\NETWORK SERVICE"]
- define_users_as_pure.members == ["{{ ansible_hostname }}\\{{ admin_account_name }}", "{{ ansible_hostname }}\\{{ win_local_user }}", "NT AUTHORITY\\SYSTEM"]
when: in_check_mode


Expand All @@ -288,6 +303,14 @@
- define_users_as_pure_again.members == ["{{ ansible_hostname }}\\{{ admin_account_name }}", "{{ ansible_hostname }}\\{{ win_local_user }}", "NT AUTHORITY\\SYSTEM"]
when: not in_check_mode

- name: Test define_users_as_pure_again (check mode)
assert:
that:
- define_users_as_pure_again.changed == true
- define_users_as_pure_again.added == ["{{ ansible_hostname }}\\{{ win_local_user }}", "NT AUTHORITY\\SYSTEM"]
- define_users_as_pure_again.removed == ["NT AUTHORITY\\NETWORK SERVICE"]
- define_users_as_pure_again.members == ["{{ ansible_hostname }}\\{{ admin_account_name }}", "{{ ansible_hostname }}\\{{ win_local_user }}", "NT AUTHORITY\\SYSTEM"]
when: in_check_mode

- name: Define different syntax users as pure
win_group_membership:
Expand All @@ -310,11 +333,11 @@
assert:
that:
- define_different_syntax_users_as_pure.changed == true
- define_different_syntax_users_as_pure.added == []
- define_different_syntax_users_as_pure.removed == []
- define_different_syntax_users_as_pure.members == []
- define_different_syntax_users_as_pure.added == ["{{ ansible_hostname }}\\{{ win_local_user }}"]
- define_different_syntax_users_as_pure.removed == ["NT AUTHORITY\\NETWORK SERVICE"]
- define_different_syntax_users_as_pure.members == ["{{ ansible_hostname }}\\{{ admin_account_name }}", "{{ ansible_hostname }}\\{{ win_local_user }}"]
when: in_check_mode


- name: Teardown remaining pure users
win_group_membership: *wgm_absent
win_group_membership: *wgm_absent