powershell - Improve Add-Type tempdir handler (#83080)

Improves the Add-Type temporary directory handler to include a retry
mechanism and not fail on an error. Deleting a temporary file used in
compilation is not a critical error and should improve the reliability
of Ansible on Windows hosts.

changelogs/fragments/PowerShell-AddType-temp.yml

@@ -0,0 +1,7 @@
+bugfixes:
+- >-
+  powershell - Implement more robust deletion mechanism for C# code compilation
+  temporary files. This should avoid scenarios where the underlying temporary
+  directory may be temporarily locked by antivirus tools or other IO problems.
+  A failure to delete one of these temporary directories will result in a
+  warning rather than an outright failure.

lib/ansible/executor/powershell/exec_wrapper.ps1

@@ -178,6 +178,7 @@ $($ErrorRecord.InvocationInfo.PositionMessage)
 
     Write-AnsibleLog "INFO - converting json raw to a payload" "exec_wrapper"
     $payload = ConvertFrom-AnsibleJson -InputObject $json_raw
+    $payload.module_args._ansible_exec_wrapper_warnings = [System.Collections.Generic.List[string]]@()
 
     # TODO: handle binary modules
     # TODO: handle persistence

lib/ansible/executor/powershell/module_powershell_wrapper.ps1

@@ -29,7 +29,18 @@ if ($csharp_utils.Count -gt 0) {
 
     # add any C# references so the module does not have to do so
     $new_tmp = [System.Environment]::ExpandEnvironmentVariables($Payload.module_args["_ansible_remote_tmp"])
-    Add-CSharpType -References $csharp_utils -TempPath $new_tmp -IncludeDebugInfo
+
+    # We use a fake module object to capture warnings
+    $fake_module = [PSCustomObject]@{
+        Tmpdir = $new_tmp
+        Verbosity = 3
+    }
+    $warning_func = New-Object -TypeName System.Management.Automation.PSScriptMethod -ArgumentList Warn, {
+        param($message)
+        $Payload.module_args._ansible_exec_wrapper_warnings.Add($message)
+    }
+    $fake_module.PSObject.Members.Add($warning_func)
+    Add-CSharpType -References $csharp_utils -AnsibleModule $fake_module
 }
 
 if ($Payload.ContainsKey("coverage") -and $null -ne $host.Runspace -and $null -ne $host.Runspace.Debugger) {

lib/ansible/module_utils/csharp/Ansible.Basic.cs

@@ -1025,7 +1025,16 @@ namespace Ansible.Basic
             foreach (DictionaryEntry entry in param)
             {
                 string paramKey = (string)entry.Key;
-                if (!legalInputs.Contains(paramKey, StringComparer.OrdinalIgnoreCase))
+                if (paramKey == "_ansible_exec_wrapper_warnings")
+                {
+                    // Special key used in module_powershell_wrapper to pass
+                    // along any warnings that should be returned back to
+                    // Ansible.
+                    removedParameters.Add(paramKey);
+                    foreach (string warning in (IList<string>)entry.Value)
+                        Warn(warning);
+                }
+                else if (!legalInputs.Contains(paramKey, StringComparer.OrdinalIgnoreCase))
                     unsupportedParameters.Add(paramKey);
                 else if (!legalInputs.Contains(paramKey))
                     // For backwards compatibility we do not care about the case but we need to warn the users as this will

lib/ansible/module_utils/powershell/Ansible.ModuleUtils.AddType.psm1

@@ -75,7 +75,7 @@ Function Add-CSharpType {
         [Switch]$IgnoreWarnings,
         [Switch]$PassThru,
         [Parameter(Mandatory = $true, ParameterSetName = "Module")][Object]$AnsibleModule,
-        [Parameter(ParameterSetName = "Manual")][String]$TempPath = $env:TMP,
+        [Parameter(ParameterSetName = "Manual")][String]$TempPath,
         [Parameter(ParameterSetName = "Manual")][Switch]$IncludeDebugInfo,
         [String[]]$CompileSymbols = @()
     )
@@ -280,9 +280,11 @@ Function Add-CSharpType {
             $include_debug = $AnsibleModule.Verbosity -ge 3
         }
         else {
-            $temp_path = $TempPath
+            $temp_path = [System.IO.Path]::GetTempPath()
             $include_debug = $IncludeDebugInfo.IsPresent
         }
+        $temp_path = Join-Path -Path $temp_path -ChildPath ([Guid]::NewGuid().Guid)
+
         $compiler_options = [System.Collections.ArrayList]@("/optimize")
         if ($defined_symbols.Count -gt 0) {
             $compiler_options.Add("/define:" + ([String]::Join(";", $defined_symbols.ToArray()))) > $null
@@ -304,8 +306,12 @@ Function Add-CSharpType {
         )
 
         # create a code snippet for each reference and check if we need
-        # to reference any extra assemblies
-        $ignore_warnings = [System.Collections.ArrayList]@()
+        # to reference any extra assemblies.
+        # CS1610 is a warning when csc.exe failed to delete temporary files.
+        # We use our own temp dir deletion mechanism so this doesn't become a
+        # fatal error.
+        # https://github.com/ansible-collections/ansible.windows/issues/598
+        $ignore_warnings = [System.Collections.ArrayList]@('1610')
         $compile_units = [System.Collections.Generic.List`1[System.CodeDom.CodeSnippetCompileUnit]]@()
         foreach ($reference in $References) {
             # scan through code and add any assemblies that match
@@ -373,7 +379,26 @@ Function Add-CSharpType {
                 }
             }
 
-            $compile = $provider.CompileAssemblyFromDom($compile_parameters, $compile_units)
+            $null = New-Item -Path $temp_path -ItemType Directory -Force
+            try {
+                $compile = $provider.CompileAssemblyFromDom($compile_parameters, $compile_units)
+            }
+            finally {
+                # Try to delete the temp path, if this fails and we are running
+                # with a module object write a warning instead of failing.
+                try {
+                    [System.IO.Directory]::Delete($temp_path, $true)
+                }
+                catch {
+                    $msg = "Failed to cleanup temporary directory '$temp_path' used for compiling C# code."
+                    if ($AnsibleModule) {
+                        $AnsibleModule.Warn("$msg Files may still be present after the task is complete. Error: $_")
+                    }
+                    else {
+                        throw "$msg Error: $_"
+                    }
+                }
+            }
         }
         finally {
             foreach ($kvp in $originalEnv.GetEnumerator()) {

test/integration/targets/module_utils_Ansible.Basic/library/ansible_basic_tests.ps1

@@ -1323,6 +1323,37 @@ test_no_log - Invoked with:
         $actual | Assert-DictionaryEqual -Expected $expected
     }
 
+    "Run with exec wrapper warnings" = {
+        Set-Variable -Name complex_args -Scope Global -Value @{
+            _ansible_exec_wrapper_warnings = [System.Collections.Generic.List[string]]@(
+                'Warning 1',
+                'Warning 2'
+            )
+        }
+        $m = [Ansible.Basic.AnsibleModule]::Create(@(), @{})
+        $m.Warn("Warning 3")
+
+        $failed = $false
+        try {
+            $m.ExitJson()
+        }
+        catch [System.Management.Automation.RuntimeException] {
+            $failed = $true
+            $_.Exception.Message | Assert-Equal -Expected "exit: 0"
+            $actual = [Ansible.Basic.AnsibleModule]::FromJson($_.Exception.InnerException.Output)
+        }
+        $failed | Assert-Equal -Expected $true
+
+        $expected = @{
+            changed = $false
+            invocation = @{
+                module_args = @{}
+            }
+            warnings = @("Warning 1", "Warning 2", "Warning 3")
+        }
+        $actual | Assert-DictionaryEqual -Expected $expected
+    }
+
     "FailJson with message" = {
         $m = [Ansible.Basic.AnsibleModule]::Create(@(), @{})
 

test/integration/targets/win_exec_wrapper/tasks/main.yml

@@ -194,16 +194,33 @@
     become_test_username: ansible_become_test
     gen_pw: "{{ 'password123!' + lookup('password', '/dev/null chars=ascii_letters,digits length=8') }}"
 
-- name: create unprivileged user
-  win_user:
-    name: "{{ become_test_username }}"
-    password: "{{ gen_pw }}"
-    update_password: always
-    groups: Users
-  register: become_test_user_result
-
 - name: execute tests and ensure that test user is deleted regardless of success/failure
   block:
+  - name: create unprivileged user
+    win_user:
+      name: "{{ become_test_username }}"
+      password: "{{ gen_pw }}"
+      update_password: always
+      groups: Users
+    register: become_test_user_result
+
+  - name: create tempdir for test user
+    win_file:
+      path: C:\Windows\TEMP\test-dir
+      state: directory
+
+  - name: deny delete permissions on new temp dir for test user
+    win_acl:
+      path: C:\Windows\TEMP\test-dir
+      user: '{{ become_test_user_result.sid }}'
+      type: '{{ item.type }}'
+      rights: '{{ item.rights }}'
+    loop:
+    - type: allow
+      rights: ListDirectory, CreateFiles, CreateDirectories, ReadAttributes, ReadExtendedAttributes, WriteData, WriteAttributes, WriteExtendedAttributes, Synchronize
+    - type: deny
+      rights: DeleteSubdirectoriesAndFiles, Delete
+
   - name: ensure current user is not the become user
     win_shell: whoami
     register: whoami_out
@@ -238,6 +255,21 @@
       - become_system is successful
       - become_system.output == become_test_user_result.sid
 
+  - name: run module with tempdir with no delete access
+    win_ping:
+    register: temp_deletion_warning
+    vars:
+      <<: *become_vars
+      ansible_remote_tmp: C:\Windows\TEMP\test-dir
+
+  - name: assert warning about tmpdir deletion is present
+    assert:
+      that:
+      - temp_deletion_warning.warnings | count == 1
+      - >-
+        temp_deletion_warning.warnings[0] is
+        regex("(?i).*Failed to cleanup temporary directory 'C:\\\\Windows\\\\TEMP\\\\test-dir\\\\.*' used for compiling C# code\\. Files may still be present after the task is complete\\..*")
+
   always:
   - name: ensure test user is deleted
     win_user:
@@ -249,7 +281,12 @@
     win_shell: rmdir /S /Q {{ profile_dir_out.stdout_lines[0] }}
     args:
       executable: cmd.exe
-    when: become_test_username in profile_dir_out.stdout_lines[0]
+    when: become_test_username in profile_dir_out.stdout_lines[0] | default("")
+
+  - name: remove test tempdir
+    win_file:
+      path: C:\Windows\TEMP\test-dir
+      state: absent
 
 - name: test common functions in exec
   test_common_functions: