Windows: Add backup parameter to modules (#50033)

* Windows: Add backup parameter to modules

This PR adds a backup infrastructure for modules.

* Fixes based on review feedback

* Various fixes to check-mode and backup

* Add integration tests

* Fix win_xml integration test

* Add backup support to copy action plugin

* Added integration tests

* Improve test efficiencies and other minor impv

lib/ansible/modules/windows/win_copy.ps1

@@ -5,6 +5,7 @@
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
 #Requires -Module Ansible.ModuleUtils.Legacy
+#Requires -Module Ansible.ModuleUtils.Backup
 
 $ErrorActionPreference = 'Stop'
 
@@ -22,6 +23,7 @@ $copy_mode = Get-AnsibleParam -obj $params -name "_copy_mode" -type "str" -defau
 # used in explode, remote and single mode
 $src = Get-AnsibleParam -obj $params -name "src" -type "path" -failifempty ($copy_mode -in @("explode","process","single"))
 $dest = Get-AnsibleParam -obj $params -name "dest" -type "path" -failifempty $true
+$backup = Get-AnsibleParam -obj $params -name "backup" -type "bool" -default $false
 
 # used in single mode
 $original_basename = Get-AnsibleParam -obj $params -name "_original_basename" -type "str"
@@ -74,6 +76,10 @@ Function Copy-File($source, $dest) {
             $diff += "+$file_dir\`n"
         }
 
+        if ($backup) {
+            $result.backup_file = Backup-File -path $dest -WhatIf:$check_mode
+        }
+
         if (Test-Path -Path $dest -PathType Leaf) {
             Remove-Item -Path $dest -Force -Recurse -WhatIf:$check_mode | Out-Null
             $diff += "-$dest`n"
@@ -390,6 +396,10 @@ if ($copy_mode -eq "query") {
         }
     }
 
+    if ($backup) {
+        $result.backup_file = Backup-File -path $remote_dest -WhatIf:$check_mode
+    }
+
     Copy-Item -Path $src -Destination $remote_dest -Force | Out-Null
     $result.changed = $true
 }

lib/ansible/modules/windows/win_copy.py

@@ -43,8 +43,18 @@ options:
       with "/" or "\", or C(src) is a directory.
     - If C(src) and C(dest) are files and if the parent directory of C(dest)
       doesn't exist, then the task will fail.
-    required: yes
     type: path
+    required: yes
+  backup:
+    description:
+    - Determine whether a backup should be created.
+    - When set to C(yes), create a backup file including the timestamp information
+      so you can get the original file back if you somehow clobbered it incorrectly.
+    - No backup is taken when C(remote_src=False) and multiple files are being
+      copied.
+    type: bool
+    default: no
+    version_added: '2.8'
   force:
     description:
     - If set to C(yes), the file will only be transferred if the content
@@ -107,6 +117,12 @@ EXAMPLES = r'''
     src: /srv/myfiles/foo.conf
     dest: C:\Temp\renamed-foo.conf
 
+- name: Copy a single file, but keep a backup
+  win_copy:
+    src: /srv/myfiles/foo.conf
+    dest: C:\Temp\renamed-foo.conf
+    backup: yes
+
 - name: Copy a single file keeping the filename
   win_copy:
     src: /src/myfiles/foo.conf
@@ -141,6 +157,11 @@ EXAMPLES = r'''
 '''
 
 RETURN = r'''
+backup_file:
+    description: Name of the backup file that was created.
+    returned: if backup=yes
+    type: str
+    sample: C:\Path\To\File.txt.11540.20150212-220915.bak
 dest:
     description: Destination file/path.
     returned: changed

lib/ansible/modules/windows/win_lineinfile.ps1

@@ -3,6 +3,7 @@
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
 #Requires -Module Ansible.ModuleUtils.Legacy
+#Requires -Module Ansible.ModuleUtils.Backup
 
 function WriteLines($outlines, $path, $linesep, $encodingobj, $validate, $check_mode) {
 	Try {
@@ -42,14 +43,14 @@ function WriteLines($outlines, $path, $linesep, $encodingobj, $validate, $check_
 	# Commit changes to the path
 	$cleanpath = $path.Replace("/", "\");
 	Try {
-		Copy-Item $temppath $cleanpath -force -ErrorAction Stop -WhatIf:$check_mode;
+		Copy-Item -Path $temppath -Destination $cleanpath -Force -WhatIf:$check_mode;
 	}
 	Catch {
 		Fail-Json @{} "Cannot write to: $cleanpath ($($_.Exception.Message))";
 	}
 
 	Try {
-		Remove-Item $temppath -force -ErrorAction Stop -WhatIf:$check_mode;
+		Remove-Item -Path $temppath -Force -WhatIf:$check_mode;
 	}
 	Catch {
 		Fail-Json @{} "Cannot remove temporary file: $temppath ($($_.Exception.Message))";
@@ -60,19 +61,6 @@ function WriteLines($outlines, $path, $linesep, $encodingobj, $validate, $check_
 }
 
 
-# Backup the file specified with a date/time filename
-function BackupFile($path, $check_mode) {
-	$backuppath = $path + "." + [DateTime]::Now.ToString("yyyyMMdd-HHmmss");
-	Try {
-		Copy-Item $path $backuppath -WhatIf:$check_mode;
-	}
-	Catch {
-		Fail-Json @{} "Cannot copy backup file! ($($_.Exception.Message))";
-	}
-	return $backuppath;
-}
-
-
 # Implement the functionality for state == 'present'
 function Present($path, $regexp, $line, $insertafter, $insertbefore, $create, $backup, $backrefs, $validate, $encodingobj, $linesep, $check_mode, $diff_support) {
 
@@ -198,7 +186,9 @@ function Present($path, $regexp, $line, $insertafter, $insertbefore, $create, $b
 
 		# Write backup file if backup == "yes"
 		If ($backup) {
-			$result.backup = BackupFile $path $check_mode;
+			$result.backup_file = Backup-File -path $path -WhatIf:$check_mode
+			# Ensure backward compatibility (deprecate in future)
+			$result.backup = $result.backup_file
 		}
 
 		$after = WriteLines $lines $path $linesep $encodingobj $validate $check_mode;
@@ -278,7 +268,9 @@ function Absent($path, $regexp, $line, $backup, $validate, $encodingobj, $linese
 
 		# Write backup file if backup == "yes"
 		If ($backup) {
-			$result.backup = BackupFile $path $check_mode;
+			$result.backup_file = Backup-File -path $path -WhatIf:$check_mode
+			# Ensure backward compatibility (deprecate in future)
+			$result.backup = $result.backup_file
 		}
 
 		$after = WriteLines $left $path $linesep $encodingobj $validate $check_mode;

lib/ansible/modules/windows/win_lineinfile.py

@@ -24,6 +24,13 @@ options:
     type: path
     required: yes
     aliases: [ dest, destfile, name ]
+  backup:
+    description:
+      - Determine whether a backup should be created.
+      - When set to C(yes), create a backup file including the timestamp information
+        so you can get the original file back if you somehow clobbered it incorrectly.
+    type: bool
+    default: no
   regexp:
     description:
       - The regular expression to look for in every line of the file. For C(state=present), the pattern to replace if found; only the last line found
@@ -70,11 +77,6 @@ options:
       - Used with C(state=present). If specified, the file will be created if it does not already exist. By default it will fail if the file is missing.
     type: bool
     default: no
-  backup:
-    description:
-      - Create a backup file including the timestamp information so you can get the original file back if you somehow clobbered it incorrectly.
-    type: bool
-    default: no
   validate:
     description:
       - Validation to run before copying into place. Use %s in the command to indicate the current file to validate.
@@ -160,3 +162,18 @@ EXAMPLES = r'''
     regexp: '(^name=)'
     line: '$1JohnDoe'
 '''
+
+RETURN = r'''
+backup:
+  description:
+  - Name of the backup file that was created.
+  - This is now deprecated, use C(backup_file) instead.
+  returned: if backup=yes
+  type: str
+  sample: C:\Path\To\File.txt.11540.20150212-220915.bak
+backup_file:
+  description: Name of the backup file that was created.
+  returned: if backup=yes
+  type: str
+  sample: C:\Path\To\File.txt.11540.20150212-220915.bak
+'''

lib/ansible/modules/windows/win_template.py

@@ -40,8 +40,16 @@ options:
   dest:
     description:
       - Location to render the template to on the remote machine.
-    type: str
+    type: path
     required: yes
+  backup:
+    description:
+    - Determine whether a backup should be created.
+    - When set to C(yes), create a backup file including the timestamp information
+      so you can get the original file back if you somehow clobbered it incorrectly.
+    type: bool
+    default: no
+    version_added: '2.8'
   newline_sequence:
     description:
       - Specify the newline sequence to use for templating files.
@@ -99,6 +107,9 @@ notes:
     which changes the variable interpolation markers to  [% var %] instead of  {{ var }}.
     This is the best way to prevent evaluation of things that look like, but should not be Jinja2.
     raw/endraw in Jinja2 will not work as you expect because templates in Ansible are recursively evaluated."
+  - You can use the M(win_copy) module with the C(content:) option if you prefer the template inline,
+    as part of the playbook.
+
 seealso:
 - module: template
 - module: win_copy
@@ -117,4 +128,13 @@ EXAMPLES = r'''
     src: unix/config.conf.j2
     dest: C:\share\unix\config.conf
     newline_sequence: '\n'
+    backup: yes
+'''
+
+RETURN = r'''
+backup_file:
+    description: Name of the backup file that was created.
+    returned: if backup=yes
+    type: str
+    sample: C:\Path\To\File.txt.11540.20150212-220915.bak
 '''

lib/ansible/modules/windows/win_xml.ps1

@@ -4,6 +4,7 @@
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
 #Requires -Module Ansible.ModuleUtils.Legacy
+#Requires -Module Ansible.ModuleUtils.Backup
 
 Set-StrictMode -Version 2
 
@@ -79,12 +80,6 @@ function Compare-XmlDocs($actual, $expected) {
     }
 }
 
-function BackupFile($path) {
-	$backuppath = $path + "." + [DateTime]::Now.ToString("yyyyMMdd-HHmmss");
-	Copy-Item $path $backuppath;
-	return $backuppath;
-}
-
 $params = Parse-Args $args -supports_check_mode $true
 $check_mode = Get-AnsibleParam -obj $params -name "_ansible_check_mode" -type "bool" -default $false
 
@@ -94,7 +89,7 @@ $debug = $debug_level -gt 2
 $dest = Get-AnsibleParam $params "path" -type "path" -FailIfEmpty $true -aliases "dest", "file"
 $fragment = Get-AnsibleParam $params "fragment" -type "str" -FailIfEmpty $true -aliases "xmlstring"
 $xpath = Get-AnsibleParam $params "xpath" -type "str" -FailIfEmpty $true
-$backup = Get-AnsibleParam $params "backup" -type "bool" -Default $false
+$backup = Get-AnsibleParam $params "backup" -type "bool" -default $false
 $type = Get-AnsibleParam $params "type" -type "str" -Default "element" -ValidateSet "element", "attribute", "text"
 $attribute = Get-AnsibleParam $params "attribute" -type "str" -FailIfEmpty ($type -eq "attribute")
 $state = Get-AnsibleParam $params "state" -type "str" -Default "present"
@@ -174,15 +169,15 @@ if ($type -eq "element") {
     }
 
     if ($changed) {
-        $result.changed = $true
-        if (!$check_mode) {
-            if ($backup) {
-                $result.backup = BackupFile($dest)
-            }
-            $xmlorig.Save($dest)
-        } else {
-            $result.msg += " check mode"
+        if ($backup) {
+            $result.backup_file = Backup-File -path $dest -WhatIf:$check_mode
+            # Ensure backward compatibility (deprecate in future)
+            $result.backup = $result.backup_file
         }
+        if (-not $check_mode) {
+            $xmlorig.Save($dest)
+        }
+        $result.changed = $true
     } else {
         $result.msg = "not changed"
     }
@@ -190,17 +185,17 @@ if ($type -eq "element") {
     $node = $xmlorig.SelectSingleNode($xpath, $namespaceMgr)
     [bool]$add = ($node.get_InnerText() -ne $fragment)
     if ($add) {
-        $result.changed = $true
-        if (-Not $check_mode) {
-            if ($backup) {
-                $result.backup = BackupFile($dest)
-            }
-            $node.set_InnerText($fragment)
-            $xmlorig.Save($dest)
-            $result.msg = "text changed"
-        } else {
-            $result.msg = "text changed check mode"
+        if ($backup) {
+            $result.backup_file = Backup-File -path $dest -WhatIf:$check_mode
+            # Ensure backward compatibility (deprecate in future)
+            $result.backup = $result.backup_file
         }
+        $node.set_InnerText($fragment)
+        if (-not $check_mode) {
+            $xmlorig.Save($dest)
+        }
+        $result.changed = $true
+        $result.msg = "text changed"
     } else {
         $result.msg = "not changed"
     }
@@ -208,33 +203,35 @@ if ($type -eq "element") {
     $node = $xmlorig.SelectSingleNode($xpath, $namespaceMgr)
     [bool]$add = !$node.HasAttribute($attribute) -Or ($node.$attribute -ne $fragment)
     if ($add -And ($state -eq "present")) {
-        $result.changed = $true
-        if (-Not $check_mode) {
-            if ($backup) {
-                $result.backup = BackupFile($dest)
-            }
-            if (!$node.HasAttribute($attribute)) {
-	            $node.SetAttributeNode($attribute, $xmlorig.get_DocumentElement().get_NamespaceURI())
-            }
-            $node.SetAttribute($attribute, $fragment)
-            $xmlorig.Save($dest)
-            $result.msg = "text changed"
-        } else {
-            $result.msg = "text changed check mode"
+        if ($backup) {
+            $result.backup_file = Backup-File -path $dest -WhatIf:$check_mode
+            # Ensure backward compatibility (deprecate in future)
+            $result.backup = $result.backup_file
         }
+        if (!$node.HasAttribute($attribute)) {
+            $node.SetAttributeNode($attribute, $xmlorig.get_DocumentElement().get_NamespaceURI())
+        }
+        $node.SetAttribute($attribute, $fragment)
+        if (-not $check_mode) {
+            $xmlorig.Save($dest)
+        }
+        $result.changed = $true
+        $result.msg = "text changed"
     } elseif (!$add -And ($state -eq "absent")) {
-        $result.changed = $true
-        if (-Not $check_mode) {
-            if ($backup) {
-                $result.backup = BackupFile($dest)
-            }
-            $node.RemoveAttribute($attribute)
-            $xmlorig.Save($dest)
-            $result.msg = "text changed"
+        if ($backup) {
+            $result.backup_file = Backup-File -path $dest -WhatIf:$check_mode
+            # Ensure backward compatibility (deprecate in future)
+            $result.backup = $result.backup_file
         }
+        $node.RemoveAttribute($attribute)
+        if (-not $check_mode) {
+            $xmlorig.Save($dest)
+        }
+        $result.changed = $true
+        $result.msg = "text changed"
     } else {
         $result.msg = "not changed"
     }
 }
 
-Exit-Json $result
+Exit-Json $result

lib/ansible/modules/windows/win_xml.py

@@ -23,7 +23,7 @@ options:
     path:
         description:
         - The path of remote servers XML.
-        type: str
+        type: path
         required: true
         aliases: [ dest, file ]
     fragment:
@@ -39,7 +39,9 @@ options:
         required: true
     backup:
         description:
-        - Whether to backup the remote server's XML before applying the change.
+        - Determine whether a backup should be created.
+        - When set to C(yes), create a backup file including the timestamp information
+          so you can get the original file back if you somehow clobbered it incorrectly.
         type: bool
         default: no
     type:
@@ -75,6 +77,11 @@ EXAMPLES = r'''
 '''
 
 RETURN = r'''
+backup_file:
+    description: Name of the backup file that was created.
+    returned: if backup=yes
+    type: str
+    sample: C:\Path\To\File.txt.11540.20150212-220915.bak
 msg:
     description: What was done.
     returned: always
@@ -85,9 +92,4 @@ err:
     returned: always, for type element and -vvv or more
     type: list
     sample: attribute mismatch for actual=string
-backup:
-    description: Name of the backup file, if created.
-    returned: changed
-    type: str
-    sample: C:\config.xml.19700101-000000
 '''