Win feature error cp (#36559)

* win_feature: better error handling to make it easier to debug issues (#36491)

* win_feature: better error handling to make it easier to debug issues

* removed ignroed pslint rules that are no longer needed

(cherry picked from commit ef4f8851dc46429622465d9fc24912abc1303271)

* Added changelog for win_feature error handling fix

5 files changed, 91 insertions, 154 deletions

diff --git a/changelogs/fragments/win_feature-better-errors.yaml b/changelogs/fragments/win_feature-better-errors.yaml
new file mode 100644
index 0000000000..ed3b9647b5
--- /dev/null
+++ b/changelogs/fragments/win_feature-better-errors.yaml
@@ -0,0 +1,2 @@
+bugfixes:
+- win_feature - will display a more helpful error when it fails during execution (https://github.com/ansible/ansible/pull/36491)
diff --git a/lib/ansible/modules/windows/win_feature.ps1 b/lib/ansible/modules/windows/win_feature.ps1
index e7feabd330..214560b687 100644
--- a/lib/ansible/modules/windows/win_feature.ps1
+++ b/lib/ansible/modules/windows/win_feature.ps1
@@ -1,25 +1,12 @@
 #!powershell
-# This file is part of Ansible.
-#
-# Copyright 2014, Paul Durivage <paul.durivage@rackspace.com>
-#
-# 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/>.
-
-# WANT_JSON
-# POWERSHELL_COMMON
-
-Import-Module Servermanager
+# This file is part of Ansible
+
+# Copyright: (c) 2014, Paul Durivage <paul.durivage@rackspace.com>
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
+
+#Requires -Module Ansible.ModuleUtils.Legacy
+
+Import-Module -Name ServerManager
 
 $result = @{
     changed = $false
@@ -28,141 +15,107 @@ $result = @{
 $params = Parse-Args $args -supports_check_mode $true
 $check_mode = Get-AnsibleParam -obj $params -name "_ansible_check_mode" -type "bool" -default $false
 
-$name = Get-AnsibleParam -obj $params -name "name" -type "str" -failifempty $true
+$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
-$includesubfeatures = Get-AnsibleParam -obj $params -name "include_sub_features" -type "bool" -default $false
-$includemanagementtools = Get-AnsibleParam -obj $params -name "include_management_tools" -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"
 
-$name = $name -split ',' | % { $_.Trim() }
-
 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
 }
 
-# Determine which cmdlets we need to work with. Then we can set options appropriate for the cmdlet
-$installWF= $false
-$addWF = $false
-
-try {
-    # We can infer uninstall/remove if install/add cmdlets exist
-    if (Get-Command "Install-WindowsFeature" -ErrorAction SilentlyContinue) {
-        $addCmdlet = "Install-WindowsFeature"
-        $removeCmdlet = "Uninstall-WindowsFeature"
-        $installWF = $true
-    }
-    elseif (Get-Command "Add-WindowsFeature" -ErrorAction SilentlyContinue) {
-        $addCmdlet = "Add-WindowsFeature"
-        $removeCmdlet = "Remove-WindowsFeature"
-        $addWF = $true
-    }
-    else {
-        throw [System.Exception] "Not supported on this version of Windows"
-    }
-}
-catch {
-    Fail-Json $result $_.Exception.Message
+$install_cmdlet = $false
+if (Get-Command -Name Install-WindowsFeature -ErrorAction SilentlyContinue) {
+    Set-Alias -Name Install-AnsibleWindowsFeature -Value Install-WindowsFeature
+    Set-Alias -Name Uninstall-AnsibleWindowsFeature -Value Uninstall-WindowsFeature
+    $install_cmdlet = $true
+} elseif (Get-Command -Name Add-WindowsFeature -ErrorAction SilentlyContinue) {
+    Set-Alias -Name Install-AnsibleWindowsFeature -Value Add-WindowsFeature
+    Set-Alias -Name Uninstall-AnsibleWindowsFeature -Value Remove-WindowsFeature
+} else {
+    Fail-Json -obj $result -message "This version of Windows does not support the cmdlets Install-WindowsFeature or Add-WindowsFeature"
 }
 
-
-If ($state -eq "present") {
-
-    # Base params to cover both Add/Install-WindowsFeature
-    $InstallParams = @{
+if ($state -eq "present") {
+    $install_args = @{
         Name = $name
         Restart = $restart
-        IncludeAllSubFeature = $includesubfeatures
-        ErrorAction = "SilentlyContinue"
+        IncludeAllSubFeature = $include_sub_features
         WhatIf = $check_mode
+        ErrorAction = "Stop"
     }
 
-    # IncludeManagementTools and source are options only for Install-WindowsFeature
-    if ($installWF) {
-
+    if ($install_cmdlet) {
+        $install_args.IncludeManagementTools = $include_management_tools
+        $install_args.Confirm = $false
         if ($source) {
             if (-not (Test-Path -Path $source)) {
-                Fail-Json $result "Failed to find source path $source"
+                Fail-Json -obj $result -message "Failed to find source path $source for feature install"
             }
-
-            $InstallParams.add("Source",$source)
-        }
-
-        if ($IncludeManagementTools) {
-            $InstallParams.add("IncludeManagementTools",$includemanagementtools)
+            $install_args.Source = $source
         }
     }
 
     try {
-        $featureresult = Invoke-Expression "$addCmdlet @InstallParams"
-    }
-    catch {
-        Fail-Json $result $_.Exception.Message
+        $action_results = Install-AnsibleWindowsFeature @install_args
+    } catch {
+        Fail-Json -obj $result -message "Failed to install Windows Feature: $($_.Exception.Message)"
     }
-}
-ElseIf ($state -eq "absent") {
-
-    $UninstallParams = @{
+} else {
+    $uninstall_args = @{
         Name = $name
         Restart = $restart
-        ErrorAction = "SilentlyContinue"
         WhatIf = $check_mode
+        ErrorAction = "Stop"
+    }
+    if ($install_cmdlet) {
+        $uninstall_args.IncludeManagementTools = $include_management_tools
     }
 
     try {
-        $featureresult = Invoke-Expression "$removeCmdlet @UninstallParams"
-    }
-    catch {
-        Fail-Json $result $_.Exception.Message
+        $action_results = Uninstall-AnsibleWindowsFeature @uninstall_args
+    } catch {
+        Fail-Json -obj $result -message "Failed to uninstall Windows Feature: $($_.Exception.Message)"
     }
 }
 
 # Loop through results and create a hash containing details about
 # each role/feature that is installed/removed
-$installed_features = @()
-#$featureresult.featureresult is filled if anything was changed
-If ($featureresult.FeatureResult)
-{
-    ForEach ($item in $featureresult.FeatureResult) {
-        $message = @()
-        ForEach ($msg in $item.Message) {
-            $message += @{
-                message_type = $msg.MessageType.ToString()
-                error_code = $msg.ErrorCode
-                text = $msg.Text
-            }
-        }
-        $installed_features += @{
-            id = $item.Id
-            display_name = $item.DisplayName
-            message = $message
-            reboot_required = $item.RestartNeeded.ToString() | ConvertTo-Bool
-            skip_reason = $item.SkipReason.ToString()
-            success = $item.Success.ToString() | ConvertTo-Bool
-
-            # DEPRECATED 2.4, potential removal in 2.6+ (standardize naming to "reboot_required")
-            restart_needed = $item.RestartNeeded.ToString() | ConvertTo-Bool
+# $action_results.FeatureResult is not empty if anything was changed
+$feature_results = @()
+foreach ($action_result in $action_results.FeatureResult) {
+    $message = @()
+    foreach ($msg in $action_result.Message) {
+        $message += @{
+            message_type = $msg.MessageType.ToString()
+            error_code = $msg.ErrorCode
+            text = $msg.Text
         }
     }
+
+    $feature_results += @{
+        id = $action_result.Id
+        display_name = $action_result.DisplayName
+        message = $message
+        reboot_required = ConvertTo-Bool -obj $action_result.RestartNeeded
+        skip_reason = $action_result.SkipReason.ToString()
+        success = ConvertTo-Bool -obj $action_result.Success
+        restart_needed = ConvertTo-Bool -obj $action_result.RestartNeeded
+    }
     $result.changed = $true
 }
-
-$result.feature_result = $installed_features
-$result.success = ($featureresult.Success.ToString() | ConvertTo-Bool)
-$result.exitcode = $featureresult.ExitCode.ToString()
-$result.reboot_required = ($featureresult.RestartNeeded.ToString() | ConvertTo-Bool)
+$result.feature_result = $feature_results
+$result.success = ConvertTo-Bool -obj $action_results.Success
+$result.exitcode = $action_results.ExitCode.ToString()
+$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")
 $result.restart_needed = $result.reboot_required
 
-If ($result.success) {
-    Exit-Json $result
-}
-ElseIf ($state -eq "present") {
-    Fail-Json $result "Failed to add feature"
-}
-Else {
-    Fail-Json $result "Failed to remove feature"
-}
+Exit-Json -obj $result
diff --git a/lib/ansible/modules/windows/win_feature.py b/lib/ansible/modules/windows/win_feature.py
index 66164acd72..fce6b65f1d 100644
--- a/lib/ansible/modules/windows/win_feature.py
+++ b/lib/ansible/modules/windows/win_feature.py
@@ -1,22 +1,10 @@
 #!/usr/bin/python
 # -*- coding: utf-8 -*-
 
-# (c) 2014, Paul Durivage <paul.durivage@rackspace.com>, Trond Hindenes <trond@hindenes.com> and others
-#
 # 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/>.
+
+# (c) 2014, Paul Durivage <paul.durivage@rackspace.com>, Trond Hindenes <trond@hindenes.com> and others
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
 # this is a windows documentation stub.  actual code lives in the .ps1
 # file of the same name
@@ -32,16 +20,16 @@ module: win_feature
 version_added: "1.7"
 short_description: Installs and uninstalls Windows Features on Windows Server
 description:
-     - Installs or uninstalls Windows Roles or Features on Windows Server. This module uses the Add/Remove-WindowsFeature Cmdlets on Windows 2008
+     - Installs or uninstalls Windows Roles or Features on Windows Server. This module uses the Add/Remove-WindowsFeature Cmdlets on Windows 2008 R2
        and Install/Uninstall-WindowsFeature Cmdlets on Windows 2012, which are not available on client os machines.
 options:
   name:
     description:
-      - Names of roles or features to install as a single feature or a comma-separated list of features
+      - Names of roles or features to install as a single feature or a comma-separated list of features.
     required: true
   state:
     description:
-      - State of the features or roles on the system
+      - State of the features or roles on the system.
     choices:
       - present
       - absent
@@ -51,27 +39,24 @@ options:
       - 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.
-    choices:
-      - yes
-      - no
+    type: bool
+    default: 'no'
   include_sub_features:
     description:
-      - Adds all subfeatures of the specified feature
-    choices:
-      - yes
-      - no
+      - Adds all subfeatures of the specified feature.
+    type: bool
+    default: 'no'
   include_management_tools:
     description:
       - Adds the corresponding management tools to the specified feature.
-      - Not supported in Windows 2008. If present when using Windows 2008 this option will be ignored.
-    choices:
-      - yes
-      - no
+      - Not supported in Windows 2008 R2 and will be ignored.
+    type: bool
+    default: 'no'
   source:
     description:
       - Specify a source to install the feature from.
-      - Not supported in Windows 2008. If present when using Windows 2008 this option will be ignored.
-    choices: [ ' {driveletter}:\sources\sxs', ' {IP}\Share\sources\sxs' ]
+      - Not supported in Windows 2008 R2 and will be ignored.
+      - Can either be C({driveletter}:\sources\sxs) or C(\\{IP}\share\sources\sxs).
     version_added: "2.1"
 author:
     - "Paul Durivage (@angstwad)"
@@ -86,7 +71,9 @@ EXAMPLES = r'''
 
 - name: Install IIS (Web-Server and Web-Common-Http)
   win_feature:
-    name: Web-Server,Web-Common-Http
+    name:
+    - Web-Server
+    - Web-Common-Http
     state: present
 
 - name: Install NET-Framework-Core from file
diff --git a/test/integration/targets/win_feature/tasks/main.yml b/test/integration/targets/win_feature/tasks/main.yml
index 806f30f95b..b8f3f55b7c 100644
--- a/test/integration/targets/win_feature/tasks/main.yml
+++ b/test/integration/targets/win_feature/tasks/main.yml
@@ -127,8 +127,7 @@
     that:
       - "win_feature_install_invalid_result is failed"
       - "win_feature_install_invalid_result is not changed"
-      - "win_feature_install_invalid_result.msg"
-      - "win_feature_install_invalid_result.exitcode == 'InvalidArgs'"
+      - "'The name was not found' in win_feature_install_invalid_result.msg"
   when: win_feature_has_servermanager is successful
 
 - name: try to remove an invalid feature name
@@ -144,6 +143,5 @@
     that:
       - "win_feature_remove_invalid_result is failed"
       - "win_feature_remove_invalid_result is not changed"
-      - "win_feature_remove_invalid_result.msg"
-      - "win_feature_remove_invalid_result.exitcode == 'InvalidArgs'"
+      - "'The name was not found' in win_feature_remove_invalid_result.msg"
   when: win_feature_has_servermanager is successful
diff --git a/test/sanity/pslint/ignore.txt b/test/sanity/pslint/ignore.txt
index 7f54725681..06abd52e9e 100644
--- a/test/sanity/pslint/ignore.txt
+++ b/test/sanity/pslint/ignore.txt
@@ -64,9 +64,6 @@ lib/ansible/modules/windows/win_dsc.ps1 PSAvoidUsingEmptyCatchBlock
 lib/ansible/modules/windows/win_dsc.ps1 PSPossibleIncorrectComparisonWithNull
 lib/ansible/modules/windows/win_dsc.ps1 PSUseApprovedVerbs
 lib/ansible/modules/windows/win_eventlog.ps1 PSUseDeclaredVarsMoreThanAssignments
-lib/ansible/modules/windows/win_feature.ps1 PSAvoidUsingCmdletAliases
-lib/ansible/modules/windows/win_feature.ps1 PSAvoidUsingInvokeExpression
-lib/ansible/modules/windows/win_feature.ps1 PSUseDeclaredVarsMoreThanAssignments
 lib/ansible/modules/windows/win_file.ps1 PSPossibleIncorrectComparisonWithNull
 lib/ansible/modules/windows/win_file_version.ps1 PSUseBOMForUnicodeEncodedFile
 lib/ansible/modules/windows/win_find.ps1 PSAvoidUsingEmptyCatchBlock