From f81410a6af0afd7ad93c1fd0746f494c59597221 Mon Sep 17 00:00:00 2001 From: Nico Agten <68544113+nagten@users.noreply.github.com> Date: Tue, 15 Aug 2023 15:03:40 +0200 Subject: [PATCH 1/3] Add changelog --- .../fragments/win_group_membership-check_mode_results.yml | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 changelogs/fragments/win_group_membership-check_mode_results.yml diff --git a/changelogs/fragments/win_group_membership-check_mode_results.yml b/changelogs/fragments/win_group_membership-check_mode_results.yml new file mode 100644 index 00000000..3a4f0f33 --- /dev/null +++ b/changelogs/fragments/win_group_membership-check_mode_results.yml @@ -0,0 +1,2 @@ +bugfixes: +- win_group_membership - Return accurate results when using check_mode - https://github.com/ansible-collections/ansible.windows/issues/532 From 6bf8cf63f2277322117e270ddc36a1e09196d9d7 Mon Sep 17 00:00:00 2001 From: Nico Agten <68544113+nagten@users.noreply.github.com> Date: Tue, 15 Aug 2023 15:06:25 +0200 Subject: [PATCH 2/3] Return accurate result values when check_mode is used --- plugins/modules/win_group_membership.ps1 | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/plugins/modules/win_group_membership.ps1 b/plugins/modules/win_group_membership.ps1 index ad542a74..163f302e 100644 --- a/plugins/modules/win_group_membership.ps1 +++ b/plugins/modules/win_group_membership.ps1 @@ -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 } } @@ -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 } } @@ -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 From 6a04136214c33a39c3fb31a80238057a3946324e Mon Sep 17 00:00:00 2001 From: Nico Agten <68544113+nagten@users.noreply.github.com> Date: Tue, 15 Aug 2023 15:15:45 +0200 Subject: [PATCH 3/3] Update tests, added additional check_mode tests and disabled check mode for seting up pure users. --- .../win_group_membership/tasks/tests.yml | 55 +++++++++++++------ 1 file changed, 39 insertions(+), 16 deletions(-) diff --git a/tests/integration/targets/win_group_membership/tasks/tests.yml b/tests/integration/targets/win_group_membership/tasks/tests.yml index 44a94a04..69426abc 100644 --- a/tests/integration/targets/win_group_membership/tasks/tests.yml +++ b/tests/integration/targets/win_group_membership/tasks/tests.yml @@ -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 @@ -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: @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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: @@ -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 \ No newline at end of file + win_group_membership: *wgm_absent