win_scheduled_task: add deprecation warning for repetition format (#45468)

* win_scheduled_task: add deprecation warning for repetition format

* fixed up sanity issues

changelogs/fragments/win_scheduled_task-repetition.yaml

@@ -0,0 +1,3 @@
+minor_changes:
+- win_scheduled_task - defining a trigger repetition as an array is deprecated and will be removed in Ansible 2.12.
+  Define the repetition as a dictionary instead.

docs/docsite/rst/porting_guides/porting_guide_2.8.rst

@@ -54,6 +54,8 @@ Noteworthy module changes
 * The ``tower_credential`` module originally required the ``ssh_key_data`` to be the path to a ssh_key_file.
   In order to work like Tower/AWX, ``ssh_key_data`` now contains the content of the file.
   The previous behavior can be achieved with ``lookup('file', '/path/to/file')``.
+* The ``win_scheduled_task`` module deprecated support for specifying a trigger repetition as a list and this format
+  will be removed in Ansible 2.12. Instead specify the repetition as a dictionary value.
 
 Plugins
 =======

lib/ansible/modules/windows/win_scheduled_task.ps1

@@ -10,7 +10,49 @@
 
 $ErrorActionPreference = "Stop"
 
+Function ConvertTo-Hashtable {
+    param([Object]$Value)
+
+    if ($null -eq $Value) {
+        return $null
+    }
+    $value_type = $Value.GetType()
+    if ($value_type.IsGenericType) {
+        $value_type = $value_type.GetGenericTypeDefinition()
+    }
+    if ($value_type -eq [System.Collections.Generic.Dictionary`2]) {
+        $new_value = @{}
+        foreach ($kv in $Value.GetEnumerator()) {
+            $new_value.Add($kv.Key, (ConvertTo-Hashtable -Value $kv.Value))
+        }
+        return ,$new_value
+    } elseif ($value_type -eq [System.Collections.ArrayList]) {
+        for ($i = 0; $i -lt $Value.Count; $i++) {
+            $Value[$i] = ConvertTo-Hashtable -Value $Value[$i]
+        }
+        return ,$Value.ToArray()
+    } else {
+        return ,$Value
+    }
+}
+
 $params = Parse-Args -arguments $args -supports_check_mode $true
+
+# FUTURE: remove this once exec_wrapper has this behaviour inbuilt with the new
+# json changes in the exec_wrapper.
+# Currently ConvertFrom-Json creates a PSObject for the deserialized JSON and the
+# exec_wrapper converts all dicts as Hashtable. Unfortunately it doesn't
+# convert any dict in lists leaving to some confusing behaviour. We manually
+# use JavaScriptSerializer to ensure we have the type of objects to simply the
+# code in the module when it comes to type checking
+$params_json = ConvertTo-Json -InputObject $params -Depth 99 -Compress
+
+Add-Type -AssemblyName System.Web.Extensions
+$json = New-Object -TypeName System.Web.Script.Serialization.JavaScriptSerializer
+$json.MaxJsonLength = [Int32]::MaxValue
+$json.RecursionLimit = [Int32]::MaxValue
+$params = ConvertTo-Hashtable -Value ($json.Deserialize($params_json, [System.Collections.Generic.Dictionary`2[[String], [Object]]]))
+
 $check_mode = Get-AnsibleParam -obj $params -name "_ansible_check_mode" -type "bool" -default $false
 $diff_mode = Get-AnsibleParam -obj $params -name "_ansible_diff" -type "bool" -default $false
 $_remote_tmp = Get-AnsibleParam $params "_ansible_remote_tmp" -type "path" -default $env:TMP
@@ -132,23 +174,6 @@ $env:TMP = $original_tmp
 ########################
 ### HELPER FUNCTIONS ###
 ########################
-Function ConvertTo-HashtableFromPsCustomObject($object) {
-    if ($object -is [Hashtable]) {
-        return ,$object
-    }
-
-    $hashtable = @{}
-    $object | Get-Member -MemberType *Property | % {
-        $value = $object.$($_.Name)
-        if ($value -is [PSObject]) {
-            $value = ConvertTo-HashtableFromPsCustomObject -object $value
-        }
-        $hashtable.$($_.Name) = $value
-    }
-
-    return ,$hashtable
-}
-
 Function Convert-SnakeToPascalCase($snake) {
     # very basic function to convert snake_case to PascalCase for use in COM
     # objects
@@ -275,9 +300,9 @@ Function Compare-PropertyList {
                     $property_value = $new_property.$property_arg
 
                     if ($property_value -is [Hashtable]) {
-                        foreach ($sub_property_arg in $property_value.Keys) {
-                            $sub_com_name = Convert-SnakeToPascalCase -snake $sub_property_arg
-                            $sub_property_value = $property_value.$sub_property_arg
+                        foreach ($kv in $property_value.GetEnumerator()) {
+                            $sub_com_name = Convert-SnakeToPascalCase -snake $kv.Key
+                            $sub_property_value = $kv.Value
                             [void]$diff_list.Add("+$com_name.$sub_com_name=$sub_property_value")
                         }
                     } else {
@@ -299,9 +324,9 @@ Function Compare-PropertyList {
                         $property_value = $new_property.$property_arg
 
                         if ($property_value -is [Hashtable]) {
-                            foreach ($sub_property_arg in $property_value.Keys) {
-                                $sub_com_name = Convert-SnakeToPascalCase -snake $sub_property_arg
-                                $sub_property_value = $property_value.$sub_property_arg
+                            foreach ($kv in $property_value.GetEnumerator()) {
+                                $sub_com_name = Convert-SnakeToPascalCase -snake $kv.Key
+                                $sub_property_value = $kv.Value
                                 [void]$diff_list.Add("+$com_name.$sub_com_name=$sub_property_value")
                             }
                         } else {
@@ -320,9 +345,9 @@ Function Compare-PropertyList {
                     $existing_value = $existing_property.$com_name
 
                     if ($property_value -is [Hashtable]) {
-                        foreach ($sub_property_arg in $property_value.Keys) {
-                            $sub_property_value = $property_value.$sub_property_arg
-                            $sub_com_name = Convert-SnakeToPascalCase -snake $sub_property_arg
+                        foreach ($kv in $property_value.GetEnumerator()) {
+                            $sub_property_value = $kv.Value
+                            $sub_com_name = Convert-SnakeToPascalCase -snake $kv.Key
                             $sub_existing_value = $existing_property.$com_name.$sub_com_name
 
                             if ($sub_property_value -ne $null) {
@@ -356,11 +381,11 @@ Function Compare-PropertyList {
                 $existing_value = $existing_property.$com_name
                 
                 if ($property_value -is [Hashtable]) {
-                    foreach ($sub_property_arg in $property_value.Keys) {
-                        $sub_property_value = $property_value.$sub_property_arg
+                    foreach ($kv in $property_value.GetEnumerator()) {
+                        $sub_property_value = $kv.Value
                         
                         if ($sub_property_value -ne $null) {
-                            $sub_com_name = Convert-SnakeToPascalCase -snake $sub_property_arg
+                            $sub_com_name = Convert-SnakeToPascalCase -snake $kv.Key
                             $sub_existing_value = $existing_property.$com_name.$sub_com_name
 
                             if ($sub_property_value -cne $sub_existing_value) {
@@ -388,10 +413,10 @@ Function Compare-PropertyList {
                 $com_name = Convert-SnakeToPascalCase -snake $property_arg
                 $new_object_property = $new_object.$com_name
     
-                foreach ($key in $new_value.Keys) {
-                    $value = $new_value.$key
+                foreach ($kv in $new_value.GetEnumerator()) {
+                    $value = $kv.Value
                     if ($value -ne $null) {
-                        Set-PropertyForComObject -com_object $new_object_property -name $property_name -arg $key -value $value
+                        Set-PropertyForComObject -com_object $new_object_property -name $property_name -arg $kv.Key -value $value
                     }
                 }
             } elseif ($new_value -ne $null) {
@@ -731,7 +756,7 @@ if ($run_level -ne $null) {
 
 # manually add the only support action type for each action - also convert PSCustomObject to Hashtable
 for ($i = 0; $i -lt $actions.Count; $i++) {
-    $action = ConvertTo-HashtableFromPsCustomObject -object $actions[$i]
+    $action = $actions[$i]
     $action.type = [TASK_ACTION_TYPE]::TASK_ACTION_EXEC
     if (-not $action.ContainsKey("path")) {
         Fail-Json -obj $result -message "action entry must contain the key 'path'"
@@ -741,7 +766,7 @@ for ($i = 0; $i -lt $actions.Count; $i++) {
 
 # convert and validate the triggers - and convert PSCustomObject to Hashtable
 for ($i = 0; $i -lt $triggers.Count; $i++) {
-    $trigger = ConvertTo-HashtableFromPsCustomObject -object $triggers[$i]
+    $trigger = $triggers[$i]
     $valid_trigger_types = @('event', 'time', 'daily', 'weekly', 'monthly', 'monthlydow', 'idle', 'registration', 'boot', 'logon', 'session_state_change')
     if (-not $trigger.ContainsKey("type")) {
         Fail-Json -obj $result -message "a trigger entry must contain a key 'type' with a value of '$($valid_trigger_types -join "', '")'"
@@ -781,7 +806,10 @@ for ($i = 0; $i -lt $triggers.Count; $i++) {
     }
 
     if ($trigger.ContainsKey("repetition")) {
-        $trigger.repetition = ConvertTo-HashtableFromPsCustomObject -object $trigger.repetition
+        if ($trigger.repetition -is [Array]) {
+            Add-DeprecationWarning -obj $result -message "repetition is a list, should be defined as a dict" -version "2.12"
+            $trigger.repetition = $trigger.repetition[0]
+        }
 
         $interval_timespan = $null
         if ($trigger.repetition.ContainsKey("interval") -and $trigger.repetition.interval -ne $null) {

lib/ansible/modules/windows/win_scheduled_task.py

@@ -468,7 +468,7 @@ EXAMPLES = r'''
     triggers:
     - type: registration
       repetition:
-      - interval: PT1M
+        interval: PT1M
         duration: PT5M
         stop_at_duration_end: yes
 '''

lib/ansible/modules/windows/win_scheduled_task_stat.ps1

@@ -252,7 +252,10 @@ try {
     $result.folder_exists = $true
 } catch {
     $result.folder_exists = $false
-    $task_folder = $null
+    if ($null -ne $name) {
+        $result.task_exists = $false
+    }
+    Exit-Json -obj $result
 }
 
 $folder_tasks = $task_folder.GetTasks(1)

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

@@ -7,7 +7,7 @@
     include_tasks: failures.yml
 
   - name: Test normal scenarios
-    include_tasks: new_tests.yml
+    include_tasks: tests.yml
 
   - include_tasks: clean.yml
   

test/integration/targets/win_scheduled_task/tasks/triggers.yml

@@ -297,6 +297,7 @@
     triggers:
     - type: registration
       repetition:
+      # TODO: change to dict in 2.12 as a list format is deprecated
       - interval: PT1M
         duration: PT5M
         stop_at_duration_end: yes
@@ -313,6 +314,9 @@
   assert:
     that:
     - create_trigger_repetition_check is changed
+    - create_trigger_repetition_check.deprecations|count == 1
+    - create_trigger_repetition_check.deprecations[0].version == "2.12"
+    - create_trigger_repetition_check.deprecations[0].msg == "repetition is a list, should be defined as a dict"
     - create_trigger_repetition_result_check.task_exists == True
     - create_trigger_repetition_result_check.triggers|count == 1
     - create_trigger_repetition_result_check.triggers[0].type == "TASK_TRIGGER_MONTHLYDOW"
@@ -334,7 +338,7 @@
     triggers:
     - type: registration
       repetition:
-      - interval: PT1M
+        interval: PT1M
         duration: PT5M
         stop_at_duration_end: yes
   register: create_trigger_repetition
@@ -368,7 +372,7 @@
     triggers:
     - type: registration
       repetition:
-      - interval: PT1M
+        interval: PT1M
         duration: PT5M
         stop_at_duration_end: yes
   register: create_trigger_repetition_again
@@ -387,7 +391,7 @@
     triggers:
     - type: registration
       repetition:
-      - interval: PT10M
+        interval: PT10M
         duration: PT20M
         stop_at_duration_end: no
   register: change_trigger_repetition_check
@@ -422,7 +426,7 @@
     triggers:
     - type: registration
       repetition:
-      - interval: PT10M
+        interval: PT10M
         duration: PT20M
         stop_at_duration_end: no
   register: change_trigger_repetition
@@ -456,7 +460,7 @@
     triggers:
     - type: registration
       repetition:
-      - interval: PT10M
+        interval: PT10M
         duration: PT20M
         stop_at_duration_end: no
   register: change_trigger_repetition_again

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

@@ -77,6 +77,18 @@
     - stat_folder_with_task.task_exists is not defined
 
 # task stat tests
+- name: get stat of missing task with invalid folder
+  win_scheduled_task_stat:
+    path: fake path
+    name: fake task
+  register: stat_task_missing_folder
+
+- name: assert get stat of missing task with invalid folder
+  assert:
+    that:
+    - stat_task_missing_folder.folder_exists == False
+    - stat_task_missing_folder.task_exists == False
+
 - name: get stat of missing task
   win_scheduled_task_stat:
     path: '{{test_scheduled_task_stat_path}}'

test/sanity/pslint/ignore.txt

@@ -83,7 +83,6 @@ lib/ansible/modules/windows/win_rabbitmq_plugin.ps1 PSAvoidUsingCmdletAliases
 lib/ansible/modules/windows/win_rabbitmq_plugin.ps1 PSAvoidUsingInvokeExpression
 lib/ansible/modules/windows/win_region.ps1 PSAvoidUsingEmptyCatchBlock
 lib/ansible/modules/windows/win_route.ps1 PSAvoidUsingCmdletAliases
-lib/ansible/modules/windows/win_scheduled_task.ps1 PSAvoidUsingCmdletAliases
 lib/ansible/modules/windows/win_scheduled_task_stat.ps1 PSAvoidUsingCmdletAliases
 lib/ansible/modules/windows/win_scheduled_task_stat.ps1 PSUseDeclaredVarsMoreThanAssignments
 lib/ansible/modules/windows/win_security_policy.ps1 PSUseApprovedVerbs