ansible-collections / microsoft.ad

Ansible collection for Active Directory management
GNU General Public License v3.0
36 stars 19 forks source link

microsoft.ad.group module : Set members to an empty list #104

Closed arnydbois closed 1 month ago

arnydbois commented 3 months ago

SUMMARY Process fails when trying to set tan empty list to remove all members from a group. microsoft.ad.group module – Manage Active Directory group objects module has an option under the members attribute: to set an empty list

ISSUE TYPE

COMPONENT NAME microsoft.ad.group module

ANSIBLE VERSION ansible [core 2.16.4] config file = None python version = 3.11.2 (main, Mar 13 2023, 12:18:29) jinja version = 3.1.3 libyaml = True

COLLECTION VERSION microsoft.ad collection (version 1.4.1)

CONFIGURATION
- name: Creat ADDC groups whith members
  microsoft.ad.group:
    name: "{{ item.name }}"
    description : "{{ item.description }}"
    scope: "{{ item.scope }}"
    path: "{{ group_fqdn_path }}"
    state: present
    members:
      set: ""    
    protect_from_deletion: true
  loop: "{{ site_ADDC_groups }}"

site_ADDC_groups:
  - { UID: 'GRP_001', name: "GG_F_{{site_name}}_ADMIN-EAR-RW", scope: "global", path: "", description: "Administrateurs Reseau (Switch/Firewall/NTP)", member_of: ""}

différent configuration as been tested

  1. members: set: ''
  2. members: set:

    • "" ...

    OS / ENVIRONMENT Debian 12

    STEPS TO REPRODUCE Execute command with an empty list

EXPECTED RESULTS remove groups if already existing

ACTUAL RESULTS An exception occurred during task execution. To see the full traceback, use -vvv. The error was: à System.Management.Automation.CommandProcessorBase.Complete() failed: [DR3SP-MULIWX01V] (item={'UID': 'GRP_002', 'name': 'GG_F_ULI_ADMIN-VIR-RW', 'scope': 'global', 'path': '', 'description': 'Administrateurs Virtualisation (Vmware, ProxMox, Hyper-V)', 'member_of': ''}) => {"ansible_loop_var": "item", "changed": false, "item": {"UID": "GRP_002", "description": "Administrateurs Virtualisation (Vmware, ProxMox, Hyper-V)", "member_of": "", "name": "GG_F_ULI_ADMIN-VIR-RW", "path": "", "scope": "global"}, "msg": "Unhandled exception while executing module: Impossible de lier l'argument au paramètre « Identity », car il s'agit d'une chaîne vide."}

arnydbois commented 3 months ago

Following Powershell remove groups if it's configure with an empty list

- name: remove groups members
  ansible.windows.win_powershell:
    script: |
      [CmdletBinding()]
      param (
          [String]
          $groups_name    
      )

      #Remove existing Group
      $Groups = (Get-ADGroup $groups_name -Properties Member).Member
      Remove-ADGroupMember -Identity $groups_name -Members $Groups -Confirm:$false 

    parameters:
      groups_name: "{{ item.name }}" 

  loop: "{{ site_ADDC_groups }}"
  when: item.member_of == ''
jborean93 commented 3 months ago

The problem here is that the members/set option is set to be a list type with string elements. Traditionally in Ansible the argument validation will convert the value to a list if a single value was specified for example foo: bar would become foo: ['bar']. The same applies to an empty string where foo: "" becomes foo: [""].

This is a problem because the code will see this list, go through each member and try to find the group's identity which in this case will be an empty string.

The best way to specify an empty list as the value is to just use an empty list like so

- name: Creat ADDC groups whith members
  microsoft.ad.group:
    name: "{{ item.name }}"
    description : "{{ item.description }}"
    scope: "{{ item.scope }}"
    path: "{{ group_fqdn_path }}"
    state: present
    members:
      set: []
    protect_from_deletion: true
  loop: "{{ site_ADDC_groups }}"

I'll leave this open as I think the code should probably just ignore empty strings for these values or at the very least it should document this scenario to help others who might come across it.

jborean93 commented 1 month ago

This issue will be fixed by https://github.com/ansible-collections/microsoft.ad/pull/117. It will explicitly ignore empty strings in each list.