Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added new win_package_msp module. #52211

Open
wants to merge 11 commits into
base: devel
from

Conversation

Projects
None yet
5 participants
@Finzzownt
Copy link

Finzzownt commented Feb 14, 2019

SUMMARY

Addition of new Windows module win_msp. Features installation or removal of Microsoft Installer Patch files (.msp).

Fixes #22789

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

win_msp

ADDITIONAL INFORMATION

Use win_msp to install or remove patches (.msp). This module complements the win_package module, which is used to install or remove applications.

Finzzownt added some commits Dec 14, 2018

Removed obsolete msi check.
Corrected documentation.
@ansibot

This comment has been minimized.

@ansibot

This comment was marked as resolved.

Copy link
Contributor

ansibot commented Feb 14, 2019

The test ansible-test sanity --test ansible-doc --python 2.7 [explain] failed with the error:

Output on stderr from ansible-doc is considered an error.

Command "ansible-doc -t module win_msp" returned exit status 0.
>>> Standard Error
[WARNING]: While constructing a mapping from
/root/ansible/lib/ansible/modules/windows/win_msp.py, line 3, column 1, found a
duplicate dict key (author). Using last defined value only.

The test ansible-test sanity --test ansible-doc --python 2.6 [explain] failed with the error:

Output on stderr from ansible-doc is considered an error.

Command "ansible-doc -t module win_msp" returned exit status 0.
>>> Standard Error
[WARNING]: While constructing a mapping from
/root/ansible/lib/ansible/modules/windows/win_msp.py, line 3, column 1, found a
duplicate dict key (author). Using last defined value only.

The test ansible-test sanity --test ansible-doc --python 3.5 [explain] failed with the error:

Output on stderr from ansible-doc is considered an error.

Command "ansible-doc -t module win_msp" returned exit status 0.
>>> Standard Error
[WARNING]: While constructing a mapping from
/root/ansible/lib/ansible/modules/windows/win_msp.py, line 3, column 1, found a
duplicate dict key (author). Using last defined value only.

The test ansible-test sanity --test ansible-doc --python 3.6 [explain] failed with the error:

Output on stderr from ansible-doc is considered an error.

Command "ansible-doc -t module win_msp" returned exit status 0.
>>> Standard Error
[WARNING]: While constructing a mapping from
/root/ansible/lib/ansible/modules/windows/win_msp.py, line 3, column 1, found a
duplicate dict key (author). Using last defined value only.

The test ansible-test sanity --test ansible-doc --python 3.7 [explain] failed with the error:

Output on stderr from ansible-doc is considered an error.

Command "ansible-doc -t module win_msp" returned exit status 0.
>>> Standard Error
[WARNING]: While constructing a mapping from
/root/ansible/lib/ansible/modules/windows/win_msp.py, line 3, column 1, found a
duplicate dict key (author). Using last defined value only.

The test ansible-test sanity --test docs-build [explain] failed with 1 error:

docs/docsite/rst/modules/win_msp_module.rst:8:0: warning: Inline emphasis start-string without end-string.

The test ansible-test sanity --test pslint [explain] failed with 2 errors:

lib/ansible/modules/windows/win_msp.ps1:85:10: PSUseApprovedVerbs The cmdlet 'Download-File' uses an unapproved verb.
lib/ansible/modules/windows/win_msp.ps1:94:69: PSUsePSCredentialType The Credential parameter in 'Get-ProgramMetadata' must be of type PSCredential. For PowerShell 4.0 and earlier, please define a credential transformation attribute, e.g. [System.Management.Automation.Credential()], after the PSCredential type attribute.

The test ansible-test sanity --test validate-modules [explain] failed with 3 errors:

lib/ansible/modules/windows/win_msp.py:0:0: E305 DOCUMENTATION.version_added: required key not provided @ data['version_added']. Got None
lib/ansible/modules/windows/win_msp.py:0:0: E307 version_added should be '2.8'. Currently None
lib/ansible/modules/windows/win_msp.py:0:0: E319 RETURN.log.type: not a valid value for dictionary value @ data['log']['type']. Got 'string'

The test ansible-test sanity --test yamllint [explain] failed with 1 error:

lib/ansible/modules/windows/win_msp.py:101:1: key-duplicates DOCUMENTATION: duplication of key "author" in mapping

click here for bot help

@ansibot ansibot removed the ci_verified label Feb 14, 2019

@ansibot

This comment was marked as resolved.

Copy link
Contributor

ansibot commented Feb 14, 2019

The test ansible-test sanity --test docs-build [explain] failed with 1 error:

docs/docsite/rst/modules/win_msp_module.rst:8:0: warning: Inline emphasis start-string without end-string.

The test ansible-test sanity --test pslint [explain] failed with 2 errors:

lib/ansible/modules/windows/win_msp.ps1:85:10: PSUseApprovedVerbs The cmdlet 'Download-File' uses an unapproved verb.
lib/ansible/modules/windows/win_msp.ps1:94:69: PSUsePSCredentialType The Credential parameter in 'Get-ProgramMetadata' must be of type PSCredential. For PowerShell 4.0 and earlier, please define a credential transformation attribute, e.g. [System.Management.Automation.Credential()], after the PSCredential type attribute.

The test ansible-test sanity --test validate-modules [explain] failed with 3 errors:

lib/ansible/modules/windows/win_msp.py:0:0: E305 DOCUMENTATION.version_added: required key not provided @ data['version_added']. Got None
lib/ansible/modules/windows/win_msp.py:0:0: E307 version_added should be '2.8'. Currently None
lib/ansible/modules/windows/win_msp.py:0:0: E319 RETURN.log.type: not a valid value for dictionary value @ data['log']['type']. Got 'string'

click here for bot help

@ansibot

This comment was marked as resolved.

Copy link
Contributor

ansibot commented Feb 15, 2019

The test ansible-test sanity --test pslint [explain] failed with 1 error:

lib/ansible/modules/windows/win_msp.ps1:94:69: PSUsePSCredentialType The Credential parameter in 'Get-ProgramMetadata' must be of type PSCredential. For PowerShell 4.0 and earlier, please define a credential transformation attribute, e.g. [System.Management.Automation.Credential()], after the PSCredential type attribute.

The test ansible-test sanity --test validate-modules [explain] failed with 3 errors:

lib/ansible/modules/windows/win_msp.py:0:0: E305 DOCUMENTATION.version_added: required key not provided @ data['version_added']. Got None
lib/ansible/modules/windows/win_msp.py:0:0: E307 version_added should be '2.8'. Currently None
lib/ansible/modules/windows/win_msp.py:0:0: E319 RETURN.log.type: not a valid value for dictionary value @ data['log']['type']. Got 'string'

click here for bot help

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Feb 15, 2019

@ansibot ansibot removed the ci_verified label Feb 16, 2019

@ansibot

This comment was marked as resolved.

Copy link
Contributor

ansibot commented Feb 16, 2019

The test ansible-test sanity --test validate-modules [explain] failed with 1 error:

lib/ansible/modules/windows/win_msp.py:0:0: E319 RETURN.log.type: not a valid value for dictionary value @ data['log']['type']. Got 'string'

click here for bot help

@ansibot ansibot added the ci_verified label Feb 16, 2019

@ansibot ansibot removed the ci_verified label Feb 16, 2019

@ansibot

This comment has been minimized.

Copy link
Contributor

ansibot commented Feb 16, 2019

@Daniel-Sanchez-Fabregas @LiranNis @SamLiu79 @TimothyVandenbrande @andrewsaraceni @ar7z1 @blakfeld @brianlloyd @cchurch @chopraaa @chrishoffman @crossan007 @daBONDi @dlazz @elventear @equelin @henrikwallstrom @if-meaton @it-praktyk @itigoag @joshludwig @ksubileau @marqelme @mcassaniti @nwchandler @nwsparks @petemounce @ptemplier @richardcs @riponbanik @sbaerlocher @schwartzmx @smadam813 @themiwi @tksarah

As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add shipit if you would like to see it merged.

click here for bot help

@it-praktyk
Copy link
Contributor

it-praktyk left a comment

Lack of integration tests :-(

path:
description:
- Location of the MSP to be installed, e.g. I(state=present).
- This package can either be on the local file system, network share or a

This comment has been minimized.

@it-praktyk

it-praktyk Feb 16, 2019

Contributor

Implementation of downloading a path file - especially from http(s) - shouldn't be, IMHO, the part of that module. Please use win_get_url instead.

This comment has been minimized.

@Finzzownt

Finzzownt Feb 16, 2019

Author

I agree totally. For instance I've extended the win_get_url module to support downloading and checksum validation. One would argue why not extend that functionality in win_msp too? This was a refactor of the previous win_msi module. It's active in a production environment for six months now, and only now I found the time to contribute. By now it's outdated I noticed. Okay, long story short: I'll remove http(s) support and refer to win_get_url in the documentation.


DOCUMENTATION = r'''
---
module: win_msp

This comment has been minimized.

@it-praktyk

it-praktyk Feb 16, 2019

Contributor

What do you think about renaming the module to the win_package_msp?

This comment has been minimized.

@Finzzownt

Finzzownt Feb 16, 2019

Author

Ah, good point, as in the comment below, it was rework on the win_msi module. So win_msp was not a bad name back then (April 2018). I think renaming is better. This thread has a mind about it possibly: #22789, see if anyone else has objections.

@Finzzownt

This comment has been minimized.

Copy link
Author

Finzzownt commented Feb 16, 2019

Lack of integration tests :-(

Yeah, just discovered the topic on the net. Stating that the code is flawless doesn't help I guess :-)
I'll dive into it. Thanks.

Finzzownt added some commits Feb 17, 2019

Removed http(s) support; suggesting win_get_url in favor.
Satisfied PossibleIncorrectComparisonWithNull.

@Finzzownt Finzzownt changed the title Added new win_msp module. Added new win_package_msp module. Feb 17, 2019

@jborean93

This comment has been minimized.

Copy link
Contributor

jborean93 commented Feb 25, 2019

I think we've been on a consensus that win_package should handle these types of packages going forward. I think that code base may need a slight tweak to add future package types a bit easier but a lot of the code here is duplicated from there. I think we should look into adding this feature to win_package instead of having it as a separate module.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.