windows: removed deprecated features in 2.6 (#38930)

* windows: removed deprecated features in 2.6

* Comma surgery.

changelogs/fragments/windows-deprecation-removal.yaml

@@ -0,0 +1,6 @@
+removed_features:
+- win_chocolatey - removed deprecated upgrade option and choco_* output return values
+- win_feature - removed deprecated reboot option
+- win_iis_webapppool - removed the ability to supply attributes as a string in favour of a dictionary
+- win_package - removed deprecated name option
+- win_regedit - removed deprecated support for specifying HKCC as HCCC

docs/docsite/rst/porting_guides/porting_guide_2.6.rst

@@ -46,7 +46,11 @@ The following modules will be removed in Ansible 2.10. Please update your playbo
 Noteworthy module changes
 -------------------------
 
-No notable changes.
+* The ``upgrade`` module option for ``win_chocolatey`` has been removed; use ``state: latest`` instead.
+* The ``reboot`` module option for ``win_feature`` has been removed; use the ``win_reboot`` action plugin instead
+* The ``win_iis_webapppool`` module no longer accepts a string for the ``atributes`` module option; use the free form dictionary value instead
+* The ``name`` module option for ``win_package`` has been removed; this is not used anywhere and should just be removed from your playbooks
+* The ``win_regedit`` module no longer automatically corrects the hive path ``HCCC`` to ``HKCC``; use ``HKCC`` because this is the correct hive path
 
 Plugins
 =======

lib/ansible/modules/windows/win_chocolatey.ps1

@@ -4,8 +4,7 @@
 # Copyright: (c) 2017, Dag Wieers <dag@wieers.com>
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
-# WANT_JSON
-# POWERSHELL_COMMON
+#Requires -Module Ansible.ModuleUtils.Legacy
 
 $ErrorActionPreference = 'Stop'
 
@@ -19,7 +18,6 @@ $verbosity = Get-AnsibleParam -obj $params -name "_ansible_verbosity" -type "int
 
 $package = Get-AnsibleParam -obj $params -name "name" -type "str" -failifempty $true
 $force = Get-AnsibleParam -obj $params -name "force" -type "bool" -default $false
-$upgrade = Get-AnsibleParam -obj $params -name "upgrade" -type "bool" -default $false
 $version = Get-AnsibleParam -obj $params -name "version" -type "str"
 $source = Get-AnsibleParam -obj $params -name "source" -type "str"
 $showlog = Get-AnsibleParam -obj $params -name "showlog" -type "bool" -default $false
@@ -40,15 +38,6 @@ $result = @{
     changed = $false 
 }
 
-if ($upgrade)
-{
-    Add-DeprecationWarning -obj $result -message "Parameter upgrade=yes is replaced with state=latest" -version 2.6
-    if ($state -eq "present")
-    {
-        $state = "latest"
-    }
-}
-
 Function Chocolatey-Install-Upgrade
 {
     [CmdletBinding()]
@@ -78,8 +67,6 @@ Function Chocolatey-Install-Upgrade
         $result.rc = $LastExitCode
         $result.stdout = $install_output | Out-String
         if ($result.rc -ne 0) {
-            # Deprecated below result output in v2.4, remove in v2.6
-            $result.choco_bootstrap_output = $install_output
             Fail-Json -obj $result -message "Chocolatey bootstrap installation failed."
         }
         $result.changed = $true
@@ -163,9 +150,6 @@ Function Choco-IsInstalled
         $result.rc = $LastExitCode
         $result.command =  "$script:executable list $options"
         $result.stdout = $output | Out-String
-        # Deprecated below result output in v2.4, remove in v2.6
-        $result.choco_error_cmd = $result.command
-        $result.choco_error_log = $output
         Fail-Json -obj $result -message "Error checking installation status for $package 'package'"
     }
 
@@ -295,9 +279,6 @@ Function Choco-Upgrade
     if ($result.rc -notin $successexitcodes) {
         $result.command =  "$script:executable upgrade $script:options $options"
         $result.stdout = $output | Out-String
-        # Deprecated below result output in v2.4, remove in v2.6
-        $result.choco_error_cmd = $result.command
-        $result.choco_error_log = $output
         Fail-Json -obj $result -message "Error upgrading package '$package'"
     }
 
@@ -442,9 +423,6 @@ Function Choco-Install
     if ($result.rc -notin $successexitcodes) {
         $result.command =  "$script:executable install $script:options $options"
         $result.stdout = $output | Out-String
-        # Deprecated below result output in v2.4, remove in v2.6
-        $result.choco_error_cmd = $result.command
-        $result.choco_error_log = $output
         Fail-Json -obj $result -message "Error installing package '$package'"
     }
 
@@ -513,9 +491,6 @@ Function Choco-Uninstall
     if ($result.rc -notin $successexitcodes) {
         $result.command =  "$script:executable uninstall $script:options $options"
         $result.stdout = $output | Out-String
-        # Deprecated below result output in v2.4, remove in v2.6
-        $result.choco_error_cmd = $result.command
-        $result.choco_error_log = $output
         Fail-Json -obj $result -message "Error uninstalling package '$package'"
     }
 

lib/ansible/modules/windows/win_chocolatey.py

@@ -44,12 +44,6 @@ options:
       - Using C(force) will cause ansible to always report that a change was made.
     type: bool
     default: 'no'
-  upgrade:
-    description:
-      - If package is already installed it, try to upgrade to the latest version or to the specified version.
-      - As of Ansible v2.3 this is deprecated, set parameter C(state) to C(latest) for the same result.
-    type: bool
-    default: 'no'
   version:
     description:
       - Specific version of the package to be installed.
@@ -210,21 +204,6 @@ EXAMPLES = r'''
 '''
 
 RETURN = r'''
-choco_bootstrap_output:
-  description: DEPRECATED, will be removed in 2.6, use stdout instead.
-  returned: changed, choco task returned a failure
-  type: str
-  sample: Chocolatey upgraded 1/1 packages.
-choco_error_cmd:
-  description: DEPRECATED, will be removed in 2.6, use command instead.
-  returned: changed, choco task returned a failure
-  type: str
-  sample: choco.exe install -r --no-progress -y sysinternals --timeout 2700 --failonunfound
-choco_error_log:
-  description: DEPRECATED, will be removed in 2.6, use stdout instead.
-  returned: changed, choco task returned a failure
-  type: str
-  sample: sysinternals not installed. The package was not found with the source(s) listed
 command:
   description: The full command used in the chocolatey task.
   returned: changed

lib/ansible/modules/windows/win_feature.ps1

@@ -18,16 +18,10 @@ $check_mode = Get-AnsibleParam -obj $params -name "_ansible_check_mode" -type "b
 $name = Get-AnsibleParam -obj $params -name "name" -type "list" -failifempty $true
 $state = Get-AnsibleParam -obj $params -name "state" -type "str" -default "present" -validateset "present","absent"
 
-# DEPRECATED 2.4, potential removal in 2.6+
-$restart = Get-AnsibleParam -obj $params -name "restart" -type "bool" -default $false
 $include_sub_features = Get-AnsibleParam -obj $params -name "include_sub_features" -type "bool" -default $false
 $include_management_tools = Get-AnsibleParam -obj $params -name "include_management_tools" -type "bool" -default $false
 $source = Get-AnsibleParam -obj $params -name "source" -type "str"
 
-If ($restart) {
-    Add-DeprecationWarning -obj $result -message "The 'restart' parameter causes instability. Use the 'win_reboot' action conditionally on 'reboot_required' return value instead" -version 2.6
-}
-
 $install_cmdlet = $false
 if (Get-Command -Name Install-WindowsFeature -ErrorAction SilentlyContinue) {
     Set-Alias -Name Install-AnsibleWindowsFeature -Value Install-WindowsFeature
@@ -43,8 +37,8 @@ if (Get-Command -Name Install-WindowsFeature -ErrorAction SilentlyContinue) {
 if ($state -eq "present") {
     $install_args = @{
         Name = $name
-        Restart = $restart
         IncludeAllSubFeature = $include_sub_features
+        Restart = $false
         WhatIf = $check_mode
         ErrorAction = "Stop"
     }
@@ -68,7 +62,7 @@ if ($state -eq "present") {
 } else {
     $uninstall_args = @{
         Name = $name
-        Restart = $restart
+        Restart = $false
         WhatIf = $check_mode
         ErrorAction = "Stop"
     }
@@ -115,7 +109,7 @@ $result.reboot_required = ConvertTo-Bool -obj $action_results.RestartNeeded
 # controls whether Ansible will fail or not
 $result.failed = (-not $action_results.Success)
 
-# DEPRECATED 2.4, potential removal in 2.6+ (standardize naming to "reboot_required")
+# DEPRECATED 2.4, remove in 2.8 (standardize naming to "reboot_required")
 $result.restart_needed = $result.reboot_required
 
 Exit-Json -obj $result

lib/ansible/modules/windows/win_feature.py

@@ -32,13 +32,6 @@ options:
       - State of the features or roles on the system.
     choices: [ absent, present ]
     default: present
-  restart:
-    description:
-      - Restarts the computer automatically when installation is complete, if restarting is required by the roles or features installed.
-      - DEPRECATED in Ansible 2.4, as unmanaged reboots cause numerous issues under Ansible. Check the C(reboot_required) return value
-        from this module to determine if a reboot is necessary, and if so, use the M(win_reboot) action to perform it.
-    type: bool
-    default: 'no'
   include_sub_features:
     description:
       - Adds all subfeatures of the specified feature.

lib/ansible/modules/windows/win_iis_webapppool.ps1

@@ -1,24 +1,10 @@
 #!powershell
 
-# (c) 2015, Henrik Wallström <henrik@wallstroms.nu>
-#
-# This file is part of Ansible
-#
-# Ansible is free software: you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation, either version 3 of the License, or
-# (at your option) any later version.
-#
-# Ansible is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
+# Copyright: (c) 2015, Henrik Wallström <henrik@wallstroms.nu>
+# Copyright: (c) 2017, Ansible Project
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
-# WANT_JSON
-# POWERSHELL_COMMON
+#Requires -Module Ansible.ModuleUtils.Legacy
 
 $ErrorActionPreference = 'Stop'
 
@@ -51,12 +37,7 @@ if ($input_attributes) {
         # Uses dict style parameters, newer and recommended style
         $attributes = $input_attributes
     } else {
-        # Uses older style of separating with | per key pair and : for key:value (paramA:valueA|paramB:valueB)
-        Add-DeprecationWarning -obj $result -message "Using a string for the attributes parameter is deprecated, please use a dict instead" -version 2.6
-        $input_attributes -split '\|' | ForEach-Object {
-            $key, $value = $_ -split "\:"
-            $attributes.$key = $value
-        }
+        Fail-Json -obj $result -message "Using a string for the attributes parameter is not longer supported, please use a dict instead"
     }
 }
 $result.attributes = $attributes

lib/ansible/modules/windows/win_iis_webapppool.py

@@ -18,8 +18,8 @@ description:
 options:
   attributes:
     description:
-      - As of Ansible 2.4, this field can take in dict entries to set the
-        application pool attributes.
+      - This field is a free form dictionary value for the application pool
+        attributes.
       - These attributes are based on the naming standard at
         U(https://www.iis.net/configreference/system.applicationhost/applicationpools/add#005),
         see the examples section for more details on how to set this.
@@ -36,11 +36,6 @@ options:
         keystore. Please follow
         U(http://structuredsight.com/2014/10/26/im-out-of-range-youre-out-of-range/)
         to help fix your host.
-      - DEPRECATED As of Ansible 2.4 this field should be set using a dict
-        form, in older versions of Ansible this field used to be a string.
-      - This string has attributes that are separated by a pipe '|' and
-        attribute name/values by colon ':'
-        Ex. "startMode:OnDemand|managedPipelineMode:Classic".
   name:
     description:
       - Name of the application pool.
@@ -89,13 +84,6 @@ EXAMPLES = r'''
       managedRuntimeVersion: v4.0
       autoStart: no
 
-# Note this format style has been deprecated, please use the newer dict style instead
-- name: change application pool attributes using older string style
-  win_iis_webapppool:
-    name: AppPool
-    attributes: 'managedRuntimeVersion:v4.0|autoStart:false'
-
-# This is the preferred style to use when setting attributes
 - name: creates an application pool, sets attributes and starts it
   win_iis_webapppool:
     name: AnotherAppPool

lib/ansible/modules/windows/win_package.ps1

@@ -16,7 +16,6 @@ $check_mode = Get-AnsibleParam -obj $params -name "_ansible_check_mode" -type "b
 
 $arguments = Get-AnsibleParam -obj $params -name "arguments"
 $expected_return_code = Get-AnsibleParam -obj $params -name "expected_return_code" -type "list" -default @(0, 3010)
-$name = Get-AnsibleParam -obj $params -name "name" -type "str"
 $path = Get-AnsibleParam -obj $params -name "path" -type "str"
 $product_id = Get-AnsibleParam -obj $params -name "product_id" -type "str" -aliases "productid"
 $state = Get-AnsibleParam -obj $params -name "state" -type "str" -default "present" -validateset "absent","present" -aliases "ensure"
@@ -30,7 +29,7 @@ $creates_service = Get-AnsibleParam -obj $params -name "creates_service" -type "
 $result = @{
     changed = $false
     reboot_required = $false
-    restart_required = $false # deprecate in 2.6
+    restart_required = $false # deprecate in 2.8
 }
 
 if ($arguments -ne $null) {
@@ -60,15 +59,6 @@ if ($username -ne $null) {
     $credential = New-Object -TypeName PSCredential -ArgumentList $username, $sec_user_password
 }
 
-if ($name -ne $null) {
-    Add-DeprecationWarning -obj $result -message "the use of name has been deprecated, please remove from the task options" -version 2.6
-}
-
-# validate initial arguments, more is done after analysing the exec path
-if ($expected_return_code -eq "") {
-    Add-DeprecationWarning -obj $result -message "an empty string for expected_return_code will be deprecated in the future, omit the value or set it explicitly if you wish to override it" -version 2.6
-    $expected_return_code = @(0, 3010)
-}
 $valid_return_codes = @()
 foreach ($rc in ($expected_return_code)) {
     try {
@@ -373,7 +363,7 @@ if ($state -eq "absent") {
                 }
 
                 $result.rc = $process_result.rc
-                $result.exit_code = $process_result.rc # deprecate in 2.6
+                $result.exit_code = $process_result.rc # deprecate in 2.8
                 if ($valid_return_codes -notcontains $process_result.rc) {
                     $result.stdout = Convert-Encoding -string $process_result.stdout
                     $result.stderr = Convert-Encoding -string $process_result.stderr
@@ -456,7 +446,7 @@ if ($state -eq "absent") {
                 }
 
                 $result.rc = $process_result.rc
-                $result.exit_code = $process_result.rc # deprecate in 2.6
+                $result.exit_code = $process_result.rc # deprecate in 2.8
                 if ($valid_return_codes -notcontains $process_result.rc) {
                     $result.stdout = Convert-Encoding -string $process_result.stdout
                     $result.stderr = Convert-Encoding -string $process_result.stderr

lib/ansible/modules/windows/win_package.py

@@ -60,11 +60,6 @@ options:
     - A return code of C(3010) usually means that a reboot is required, the
       C(reboot_required) return value is set if the return code is C(3010).
     default: [0, 3010]
-  name:
-    description:
-    - Name of the package, if name isn't specified the path will be used for
-      log messages.
-    - As of Ansible 2.4 this is deprecated and no longer required.
   password:
     description:
     - The password for C(user_name), must be set when C(user_name) is.

lib/ansible/modules/windows/win_regedit.ps1

@@ -174,12 +174,6 @@ namespace Ansible
 }
 '@
 
-# Fix HCCC:\ PSDrive for pre-2.3 compatibility
-if ($path -match "^HCCC:\\") {
-    Add-DeprecationWarning -obj $result -message "Please use path: HKCC:\... instead of path: $path" -version 2.6
-    $path = $path -replace "HCCC:\\","HKCC:\\"
-}
-
 # fire a warning if the property name isn't specified, the (Default) key ($null) can only be a string
 if ($name -eq $null -and $type -ne "string") {
     Add-Warning -obj $result -message "the data type when name is not specified can only be 'string', the type has automatically been converted"

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

@@ -32,7 +32,6 @@
   win_feature:
     name: "{{ test_win_feature_name }}"
     state: present
-    restart: no
     include_sub_features: yes
     include_management_tools: yes
   register: win_feature_install_result
@@ -44,12 +43,12 @@
       - "win_feature_install_result is changed"
       - "win_feature_install_result.success"
       - "win_feature_install_result.exitcode == 'Success'"
-      - "not win_feature_install_result.restart_needed"
+      - "not win_feature_install_result.reboot_required"
       - "win_feature_install_result.feature_result|length == 1"
       - "win_feature_install_result.feature_result[0].id"
       - "win_feature_install_result.feature_result[0].display_name"
       - "win_feature_install_result.feature_result[0].message is defined"
-      - "win_feature_install_result.feature_result[0].restart_needed is defined"
+      - "win_feature_install_result.feature_result[0].reboot_required is defined"
       - "win_feature_install_result.feature_result[0].skip_reason"
       - "win_feature_install_result.feature_result[0].success is defined"
   when: win_feature_has_servermanager is successful
@@ -58,7 +57,6 @@
   win_feature:
     name: "{{ test_win_feature_name }}"
     state: present
-    restart: no
     include_sub_features: yes
     include_management_tools: yes
   register: win_feature_install_again_result
@@ -70,7 +68,7 @@
       - "win_feature_install_again_result is not changed"
       - "win_feature_install_again_result.success"
       - "win_feature_install_again_result.exitcode == 'NoChangeNeeded'"
-      - "not win_feature_install_again_result.restart_needed"
+      - "not win_feature_install_again_result.reboot_required"
       - "win_feature_install_again_result.feature_result == []"
   when: win_feature_has_servermanager is successful
 
@@ -87,12 +85,12 @@
       - "win_feature_remove_result is changed"
       - "win_feature_remove_result.success"
       - "win_feature_remove_result.exitcode == 'Success'"
-      - "not win_feature_remove_result.restart_needed"
+      - "not win_feature_remove_result.reboot_required"
       - "win_feature_remove_result.feature_result|length == 1"
       - "win_feature_remove_result.feature_result[0].id"
       - "win_feature_remove_result.feature_result[0].display_name"
       - "win_feature_remove_result.feature_result[0].message is defined"
-      - "win_feature_remove_result.feature_result[0].restart_needed is defined"
+      - "win_feature_remove_result.feature_result[0].reboot_required is defined"
       - "win_feature_remove_result.feature_result[0].skip_reason"
       - "win_feature_remove_result.feature_result[0].success is defined"
   when: win_feature_has_servermanager is successful
@@ -110,7 +108,7 @@
       - "win_feature_remove_again_result is not changed"
       - "win_feature_remove_again_result.success"
       - "win_feature_remove_again_result.exitcode == 'NoChangeNeeded'"
-      - "not win_feature_remove_again_result.restart_needed"
+      - "not win_feature_remove_again_result.reboot_required"
       - "win_feature_remove_again_result.feature_result == []"
   when: win_feature_has_servermanager is successful
 

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

@@ -9,15 +9,10 @@
 
 # Run actual tests
 - block:
-  # Setup for tests
-  - name: this is too finicky reboot before feature install
-    win_reboot:
-
   - name: ensure IIS features are installed
     win_feature:
       name: Web-Server
       state: present
-      includ_sub_features: True
       include_management_tools: True
     register: feature_install
   
@@ -45,17 +40,4 @@
     win_iis_webapppool:
       name: '{{test_iis_webapppool_name}}'
       state: absent
-
-  - name: remove IIS features after test
-    win_feature:
-      name: Web-Server
-      state: absent
-      includ_sub_features: True
-      include_management_tools: True
-    register: feature_uninstall
-  
-  - name: reboot after feature install
-    win_reboot:
-    when: feature_uninstall.reboot_required
-
   when: module_available.rc == 0

test/integration/targets/win_iis_webapppool/tasks/tests.yml

@@ -93,49 +93,6 @@
     - change_pool_attributes_again is not changed
     - change_pool_attributes_again.info == change_pool_attributes.info
 
-- name: change attributes of pool (old style) check
-  win_iis_webapppool:
-    name: '{{test_iis_webapppool_name}}'
-    state: present
-    attributes: "managedPipelineMode:0|startMode:OnDemand|cpu.limit:0|processModel.identityType:NetworkService"
-  register: change_pool_attributes_deprecated_check
-  check_mode: yes
-
-- name: assert change attributes of pool (old style) check
-  assert:
-    that:
-    - change_pool_attributes_deprecated_check is changed
-    - change_pool_attributes_deprecated_check.info == change_pool_attributes_again.info
-
-- name: change attributes of pool (old style)
-  win_iis_webapppool:
-    name: '{{test_iis_webapppool_name}}'
-    state: present
-    attributes: "managedPipelineMode:0|startMode:OnDemand|cpu.limit:10|processModel.identityType:NetworkService"
-  register: change_pool_attributes_deprecated
-
-- name: assert change attributes of pool (old style)
-  assert:
-    that:
-    - change_pool_attributes_deprecated is changed
-    - change_pool_attributes_deprecated.info.attributes.managedPipelineMode == 'Integrated'
-    - change_pool_attributes_deprecated.info.attributes.startMode == 'OnDemand'
-    - change_pool_attributes_deprecated.info.cpu.limit == 10
-    - change_pool_attributes_deprecated.info.processModel.identityType == 'NetworkService'
-
-- name: change attributes of pool (old style) again
-  win_iis_webapppool:
-    name: '{{test_iis_webapppool_name}}'
-    state: present
-    attributes: "managedPipelineMode:0|startMode:OnDemand|cpu.limit:10|processModel.identityType:NetworkService"
-  register: change_pool_attributes_deprecated_again
-
-- name: assert change attributes of pool (old style) again
-  assert:
-    that:
-    - change_pool_attributes_deprecated_again is not changed
-    - change_pool_attributes_deprecated_again.info == change_pool_attributes_deprecated.info
-
 - name: change more complex variables check
   win_iis_webapppool:
     name: '{{test_iis_webapppool_name}}'
@@ -152,7 +109,7 @@
   assert:
     that:
     - change_complex_attributes_check is changed
-    - change_complex_attributes_check.info == change_pool_attributes_deprecated_again.info
+    - change_complex_attributes_check.info == change_pool_attributes_again.info
 
 - name: change more complex variables
   win_iis_webapppool:

test/integration/targets/win_package/tasks/msi_tests.yml

@@ -29,7 +29,6 @@
   win_package:
     path: '{{test_win_package_path}}\good.msi'
     state: present
-    expected_return_code: "" # historical, an empty string means 0, 3010
   register: install_local_msi
 
 - name: get result of install local msi