membership - Improve domainjoin+rename process (#145)

* membership - Improve domainjoin+rename process * Add changelog fragment and integration tests --------- Co-authored-by: Jordan Borean <[email protected]>
ansible-collections · Aug 21, 2024 · c510555 · c510555
1 parent 33c981a
commit c510555
Show file tree

Hide file tree

Showing 5 changed files with 156 additions and 40 deletions.
diff --git a/changelogs/fragments/membership-rename.yml b/changelogs/fragments/membership-rename.yml
@@ -0,0 +1,4 @@
+bugfixes:
+  - >-
+    membership - allow domain join with hostname change if the account for that
+    host already exists - https://github.com/ansible-collections/microsoft.ad/pull/145
diff --git a/plugins/modules/membership.ps1 b/plugins/modules/membership.ps1
@@ -205,11 +205,20 @@ if ($state -eq 'domain') {
                 WhatIf = $module.CheckMode
             }
             if ($hostname -ne $currentState.HostName) {
-                $joinParams.NewName = $hostname
-
-                # By setting this here, the Rename-Computer call is skipped as
-                # joining the domain will rename the host for us.
+                $renameParams = @{
+                    NewName = $hostname
+                    WhatIf = $module.CheckMode
+                    Force = $true
+                }
+                Rename-Computer @renameParams
+                # Update this now to reflect the new name
                 $hostname = $currentState.HostName
+
+                $module.Result.changed = $true
+                $module.Result.reboot_required = $true
+
+                # Note: AccountCreate is default behavior, but needs included when Options is set
+                $joinParams.Options = 'AccountCreate,JoinWithNewName'
             }
             if ($domainOUPath) {
                 $joinParams.OUPath = $domainOUPath

diff --git a/tests/integration/targets/membership/tasks/join_with_name_conflict.yml b/tests/integration/targets/membership/tasks/join_with_name_conflict.yml
@@ -0,0 +1,101 @@
+- name: create existing AD account to match desired rename
+  microsoft.ad.computer:
+    name: RENAME-HOSTNAME
+    dns_hostname: RENAME-HOSTNAME1-long-toremove.{{ domain_realm }}
+    state: present
+  delegate_to: DC
+
+- name: get pre state before change
+  ansible.windows.win_powershell:
+    script: '{{ get_result_script }}'
+  register: previous_state
+
+- block:
+  - name: join domain with hostname that already exists - check mode
+    membership:
+      dns_domain_name: '{{ domain_realm }}'
+      domain_admin_user: '{{ domain_user_upn }}'
+      domain_admin_password: '{{ domain_password }}'
+      hostname: RENAME-HOSTNAME1-long
+      state: domain
+    register: rename_check
+    check_mode: true
+
+  - name: get result of join domain with hostname that already exists - check mode
+    ansible.windows.win_powershell:
+      script: '{{ get_result_script }}'
+    register: rename_check_actual
+
+  - name: assert join domain with hostname that already exists - check mode
+    assert:
+      that:
+      - rename_check is changed
+      - rename_check.reboot_required == True
+      - rename_check_actual.output[0] == previous_state.output[0]
+
+  - name: join domain with hostname that already exists
+    membership:
+      dns_domain_name: '{{ domain_realm }}'
+      domain_admin_user: '{{ domain_user_upn }}'
+      domain_admin_password: '{{ domain_password }}'
+      hostname: RENAME-HOSTNAME1-long
+      state: domain
+      reboot: true
+    register: rename
+
+  - name: get result of join domain with hostname that already exists
+    ansible.windows.win_powershell:
+      script: '{{ get_result_script }}'
+    register: rename_actual
+
+  - name: get ad result of join domain with hostname that already exists
+    ansible.windows.win_powershell:
+      script: '{{ get_ad_result_script }}'
+    delegate_to: DC
+    register: rename_ad_actual
+
+  - name: assert join domain with hostname that already exists
+    assert:
+      that:
+      - rename is changed
+      - rename.reboot_required == False
+      - rename_actual.output[0]['DnsDomainName'] == domain_realm
+      - rename_actual.output[0]['HostName'] == 'RENAME-HOSTNAME1-long'
+      - rename_actual.output[0]['NetbiosName'] == 'RENAME-HOSTNAME'
+      - rename_actual.output[0]['PartOfDomain'] == true
+      - rename_actual.output[0]['WorkgroupName'] == None
+      - rename_ad_actual.output | length == 1
+      - rename_ad_actual.output[0]['DNSHostName'] == 'RENAME-HOSTNAME1-long'
+      - rename_ad_actual.output[0]['DistinguishedName'] == 'CN=RENAME-HOSTNAME,CN=Computers,' ~ domain_dn_base
+      - rename_ad_actual.output[0]['Name'] == 'RENAME-HOSTNAME'
+
+  - name: join domain with hostname that already exists - idempotent
+    membership:
+      dns_domain_name: '{{ domain_realm }}'
+      domain_admin_user: '{{ domain_user_upn }}'
+      domain_admin_password: '{{ domain_password }}'
+      hostname: RENAME-HOSTNAME1-long
+      state: domain
+      reboot: true
+    register: rename_again
+
+  - name: assert join domain with hostname that already exists - idempotent
+    assert:
+      that:
+      - not rename_again is changed
+
+  always:
+  - name: change domain to workgroup
+    membership:
+      workgroup_name: WORKGROUP
+      domain_admin_user: '{{ domain_user_upn }}'
+      domain_admin_password: '{{ domain_password }}'
+      hostname: TEST
+      state: workgroup
+      reboot: true
+
+  - name: remove AD account if present
+    microsoft.ad.computer:
+      name: RENAME-HOSTNAME
+      state: absent
+    delegate_to: DC
diff --git a/tests/integration/targets/membership/tasks/main.yml b/tests/integration/targets/membership/tasks/main.yml
@@ -1,37 +1,3 @@
-- set_fact:
-    get_result_script: |
-      $Ansible.Changed = $false
-      $cs = Get-CimInstance -ClassName Win32_ComputerSystem -Property DNSHostName, Domain, PartOfDomain, Workgroup
-      $domainName = if ($cs.PartOfDomain) {
-          try {
-              [System.DirectoryServices.ActiveDirectory.Domain]::GetComputerDomain().Name
-          }
-          catch [System.Security.Authentication.AuthenticationException] {
-              $cs.Domain
-          }
-      }
-      else {
-          $null
-      }
-
-      [PSCustomObject]@{
-          HostName = $cs.DNSHostName
-          NetbiosName = $env:COMPUTERNAME
-          PartOfDomain = $cs.PartOfDomain
-          DnsDomainName = $domainName
-          WorkgroupName = $cs.Workgroup
-      }
-
-    get_ad_result_script: |
-      $Ansible.Changed = $false
-      Get-ADComputer -Filter { Name -ne 'DC' } -Properties DistinguishedName, DNSHostName, Name, Enabled |
-          Select-Object -Property @(
-              'DistinguishedName'
-              @{ N = 'DNSHostName'; E = { $_.DNSHostName.Substring(0, $_.DNSHostName.IndexOf('.')) } }
-              'Name'
-              'Enabled'
-          )
-
 - name: join domain invalid OU
   membership:
     dns_domain_name: '{{ domain_realm }}'
@@ -630,7 +596,7 @@
   register: join_ou_ad_actual
   delegate_to: DC
 
-- name: assert join domain with hostname and OUT
+- name: assert join domain with hostname and OU
   assert:
     that:
     - join_ou is changed
@@ -735,7 +701,8 @@
 
 - name: remove orphaned AD account for later tests
   microsoft.ad.computer:
-    name: BAR
+    name: TEST1-LONG-HOST
+    path: '{{ custom_ou.output[0] }}'
     state: absent
   delegate_to: DC
 

diff --git a/tests/integration/targets/membership/test.yml b/tests/integration/targets/membership/test.yml
@@ -27,4 +27,39 @@
   gather_facts: no
 
   tasks:
+  - set_fact:
+      get_result_script: |
+        $Ansible.Changed = $false
+        $cs = Get-CimInstance -ClassName Win32_ComputerSystem -Property DNSHostName, Domain, PartOfDomain, Workgroup
+        $domainName = if ($cs.PartOfDomain) {
+            try {
+                [System.DirectoryServices.ActiveDirectory.Domain]::GetComputerDomain().Name
+            }
+            catch [System.Security.Authentication.AuthenticationException] {
+                $cs.Domain
+            }
+        }
+        else {
+            $null
+        }
+
+        [PSCustomObject]@{
+            HostName = $cs.DNSHostName
+            NetbiosName = $env:COMPUTERNAME
+            PartOfDomain = $cs.PartOfDomain
+            DnsDomainName = $domainName
+            WorkgroupName = $cs.Workgroup
+        }
+
+      get_ad_result_script: |
+        $Ansible.Changed = $false
+        Get-ADComputer -Filter { Name -ne 'DC' } -Properties DistinguishedName, DNSHostName, Name, Enabled |
+            Select-Object -Property @(
+                'DistinguishedName'
+                @{ N = 'DNSHostName'; E = { $_.DNSHostName.Substring(0, $_.DNSHostName.IndexOf('.')) } }
+                'Name'
+                'Enabled'
+            )
+
   - import_tasks: tasks/main.yml
+  - import_tasks: tasks/join_with_name_conflict.yml