From ec6d82435f065d5c9c791b8d43972528ae4842e6 Mon Sep 17 00:00:00 2001 From: Jordan Borean Date: Tue, 11 Sep 2018 14:22:57 +1000 Subject: [PATCH] win_scheduled_task: add deprecation warning for repetition format (#45468) * win_scheduled_task: add deprecation warning for repetition format * fixed up sanity issues --- .../win_scheduled_task-repetition.yaml | 3 + .../rst/porting_guides/porting_guide_2.8.rst | 2 + .../modules/windows/win_scheduled_task.ps1 | 98 ++++++++++++------- .../modules/windows/win_scheduled_task.py | 2 +- .../windows/win_scheduled_task_stat.ps1 | 5 +- .../targets/win_scheduled_task/tasks/main.yml | 2 +- .../tasks/{new_tests.yml => tests.yml} | 0 .../win_scheduled_task/tasks/triggers.yml | 14 ++- .../win_scheduled_task_stat/tasks/main.yml | 12 +++ test/sanity/pslint/ignore.txt | 1 - 10 files changed, 95 insertions(+), 44 deletions(-) create mode 100644 changelogs/fragments/win_scheduled_task-repetition.yaml rename test/integration/targets/win_scheduled_task/tasks/{new_tests.yml => tests.yml} (100%) diff --git a/changelogs/fragments/win_scheduled_task-repetition.yaml b/changelogs/fragments/win_scheduled_task-repetition.yaml new file mode 100644 index 00000000000..aedf685468c --- /dev/null +++ b/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. diff --git a/docs/docsite/rst/porting_guides/porting_guide_2.8.rst b/docs/docsite/rst/porting_guides/porting_guide_2.8.rst index 1094e905c6d..6fe035d890c 100644 --- a/docs/docsite/rst/porting_guides/porting_guide_2.8.rst +++ b/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 ======= diff --git a/lib/ansible/modules/windows/win_scheduled_task.ps1 b/lib/ansible/modules/windows/win_scheduled_task.ps1 index 92e918ac02e..ba5d36a4196 100644 --- a/lib/ansible/modules/windows/win_scheduled_task.ps1 +++ b/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) { diff --git a/lib/ansible/modules/windows/win_scheduled_task.py b/lib/ansible/modules/windows/win_scheduled_task.py index 8bed76adcd2..39aa0e5855d 100644 --- a/lib/ansible/modules/windows/win_scheduled_task.py +++ b/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 ''' diff --git a/lib/ansible/modules/windows/win_scheduled_task_stat.ps1 b/lib/ansible/modules/windows/win_scheduled_task_stat.ps1 index 71cbe7ebfed..00a790ac70d 100644 --- a/lib/ansible/modules/windows/win_scheduled_task_stat.ps1 +++ b/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) diff --git a/test/integration/targets/win_scheduled_task/tasks/main.yml b/test/integration/targets/win_scheduled_task/tasks/main.yml index 2189a160f57..53b062c0b0b 100644 --- a/test/integration/targets/win_scheduled_task/tasks/main.yml +++ b/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 diff --git a/test/integration/targets/win_scheduled_task/tasks/new_tests.yml b/test/integration/targets/win_scheduled_task/tasks/tests.yml similarity index 100% rename from test/integration/targets/win_scheduled_task/tasks/new_tests.yml rename to test/integration/targets/win_scheduled_task/tasks/tests.yml diff --git a/test/integration/targets/win_scheduled_task/tasks/triggers.yml b/test/integration/targets/win_scheduled_task/tasks/triggers.yml index e80d72b2b74..9ebf1378d95 100644 --- a/test/integration/targets/win_scheduled_task/tasks/triggers.yml +++ b/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 diff --git a/test/integration/targets/win_scheduled_task_stat/tasks/main.yml b/test/integration/targets/win_scheduled_task_stat/tasks/main.yml index a9bda9b81e7..ee78abb0c05 100644 --- a/test/integration/targets/win_scheduled_task_stat/tasks/main.yml +++ b/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}}' diff --git a/test/sanity/pslint/ignore.txt b/test/sanity/pslint/ignore.txt index 1915f87dd8a..4b6eb1b0178 100644 --- a/test/sanity/pslint/ignore.txt +++ b/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