From 10af3874b5bdb4bc95e569aaffbcb56360cc7ce8 Mon Sep 17 00:00:00 2001 From: Michael Cassaniti Date: Thu, 22 Nov 2018 07:55:10 +1100 Subject: [PATCH] win_snmp: Initial commit (#45710) * win_snmp: Initial commit * win_snmp: Better handling of lists * win_snmp: Documentation fixes * win_snmp: Updated documentation to match parameters * win_snmp: Added integration tests * win_snmp: Fixed typo in test * win_snmp: Adjusted parameter checks to match documentation * win_snmp: Updated option descriptions to be full sentences * win_snmp: Better type checking and output suppression * win_snmp: Fixed unset managers and communities * win_snmp: Fixed skipping default registry keys * win_snmp: Migrated to using add/set/remove action from replace * win_snmp: Fixed check mode * win_snmp: Fixed setting action and documentation. Expanded tests. * win_snmp: Efficiency changes and documentation cleanup * Added example of explicitly setting an empty set of managers to documentation * Made sure set will only remove items if there is a list of items provided. This list can be of length 0 * Improved efficiency in selecting next index for SNMP manager * Updated tests * win_snmp: Added output of permitted managers and community strings * win_snmp: Documentation fix --- lib/ansible/modules/windows/win_snmp.ps1 | 126 +++++++++++++ lib/ansible/modules/windows/win_snmp.py | 79 +++++++++ test/integration/targets/win_snmp/aliases | 1 + .../targets/win_snmp/tasks/cleanup.yml | 16 ++ .../win_snmp/tasks/cleanup_using_module.yml | 26 +++ .../targets/win_snmp/tasks/main.yml | 8 + .../targets/win_snmp/tasks/output_only.yml | 24 +++ .../targets/win_snmp/tasks/snmp_community.yml | 165 ++++++++++++++++++ .../targets/win_snmp/tasks/snmp_managers.yml | 158 +++++++++++++++++ .../targets/win_snmp/vars/main.yml | 3 + 10 files changed, 606 insertions(+) create mode 100644 lib/ansible/modules/windows/win_snmp.ps1 create mode 100644 lib/ansible/modules/windows/win_snmp.py create mode 100644 test/integration/targets/win_snmp/aliases create mode 100644 test/integration/targets/win_snmp/tasks/cleanup.yml create mode 100644 test/integration/targets/win_snmp/tasks/cleanup_using_module.yml create mode 100644 test/integration/targets/win_snmp/tasks/main.yml create mode 100644 test/integration/targets/win_snmp/tasks/output_only.yml create mode 100644 test/integration/targets/win_snmp/tasks/snmp_community.yml create mode 100644 test/integration/targets/win_snmp/tasks/snmp_managers.yml create mode 100644 test/integration/targets/win_snmp/vars/main.yml diff --git a/lib/ansible/modules/windows/win_snmp.ps1 b/lib/ansible/modules/windows/win_snmp.ps1 new file mode 100644 index 0000000000..8e3942bd38 --- /dev/null +++ b/lib/ansible/modules/windows/win_snmp.ps1 @@ -0,0 +1,126 @@ +#!powershell + +# Copyright: (c) 2017, Ansible Project +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +#Requires -Module Ansible.ModuleUtils.Legacy + +$params = Parse-Args -arguments $args -supports_check_mode $true; +$check_mode = Get-AnsibleParam -obj $params -name "_ansible_check_mode" -type "bool" -default $false +$managers = Get-AnsibleParam -obj $params -name "permitted_managers" -type "list" -default $null +$communities = Get-AnsibleParam -obj $params -name "community_strings" -type "list" -default $null +$action_in = Get-AnsibleParam -obj $params -name "action" -type "str" -default "set" -ValidateSet @("set", "add", "remove") +$action = $action_in.ToLower() + +$result = @{ + failed = $False + changed = $False + community_strings = [System.Collections.ArrayList]@() + permitted_managers = [System.Collections.ArrayList]@() +} + +# Make sure lists are modifyable +[System.Collections.ArrayList]$managers = $managers +[System.Collections.ArrayList]$communities = $communities +[System.Collections.ArrayList]$indexes = @() + +# Type checking +# You would think that "$managers -ne $null" would work, but it doesn't. +# A proper type check is required. If a user provides an empty list then $managers +# is still of the correct type. If a user provides no option then $managers is $null. +If ($managers -Is [System.Collections.ArrayList] -And $managers.Count -gt 0 -And $managers[0] -IsNot [String]) { + Fail-Json $result "Permitted managers must be an array of strings" +} + +If ($communities -Is [System.Collections.ArrayList] -And $communities.Count -gt 0 -And $communities[0] -IsNot [String]) { + Fail-Json $result "SNMP communities must be an array of strings" +} + +$Managers_reg_key = "HKLM:\System\CurrentControlSet\services\SNMP\Parameters\PermittedManagers" +$Communities_reg_key = "HKLM:\System\CurrentControlSet\services\SNMP\Parameters\ValidCommunities" + +ForEach ($idx in (Get-Item $Managers_reg_key).Property) { + $manager = (Get-ItemProperty $Managers_reg_key).$idx + If ($idx.ToLower() -eq '(default)') { + continue + } + + $remove = $False + If ($managers -Is [System.Collections.ArrayList] -And $managers.Contains($manager)) { + If ($action -eq "remove") { + $remove = $True + } Else { + # Remove manager from list to add since it already exists + $managers.Remove($manager) + } + } ElseIf ($action -eq "set" -And $managers -Is [System.Collections.ArrayList]) { + # Will remove this manager since it is not in the set list + $remove = $True + } + + If ($remove) { + $result.changed = $True + Remove-ItemProperty -Path $Managers_reg_key -Name $idx -WhatIf:$check_mode + } Else { + # Remember that this index is in use + $indexes.Add([int]$idx) | Out-Null + $result.permitted_managers.Add($manager) | Out-Null + } +} + +ForEach ($community in (Get-Item $Communities_reg_key).Property) { + If ($community.ToLower() -eq '(default)') { + continue + } + + $remove = $False + If ($communities -Is [System.Collections.ArrayList] -And $communities.Contains($community)) { + If ($action -eq "remove") { + $remove = $True + } Else { + # Remove community from list to add since it already exists + $communities.Remove($community) + } + } ElseIf ($action -eq "set" -And $communities -Is [System.Collections.ArrayList]) { + # Will remove this community since it is not in the set list + $remove = $True + } + + If ($remove) { + $result.changed = $True + Remove-ItemProperty -Path $Communities_reg_key -Name $community -WhatIf:$check_mode + } Else { + $result.community_strings.Add($community) | Out-Null + } +} + +If ($action -eq "remove") { + Exit-Json $result +} + +# Add managers that don't already exist +$next_index = 0 +If ($managers -Is [System.Collections.ArrayList]) { + ForEach ($manager in $managers) { + While ($True) { + $next_index = $next_index + 1 + If (-Not $indexes.Contains($next_index)) { + $result.changed = $True + New-ItemProperty -Path $Managers_reg_key -Name $next_index -Value "$manager" -WhatIf:$check_mode | Out-Null + $result.permitted_managers.Add($manager) | Out-Null + break + } + } + } +} + +# Add communities that don't already exist +If ($communities -Is [System.Collections.ArrayList]) { + ForEach ($community in $communities) { + $result.changed = $True + New-ItemProperty -Path $Communities_reg_key -Name $community -PropertyType DWord -Value 4 -WhatIf:$check_mode | Out-Null + $result.community_strings.Add($community) | Out-Null + } +} + +Exit-Json $result diff --git a/lib/ansible/modules/windows/win_snmp.py b/lib/ansible/modules/windows/win_snmp.py new file mode 100644 index 0000000000..8dd28a225c --- /dev/null +++ b/lib/ansible/modules/windows/win_snmp.py @@ -0,0 +1,79 @@ +#!/usr/bin/python +# -*- coding: utf-8 -*- + +# Copyright: (c) 2018, Ansible, inc +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +ANSIBLE_METADATA = { + 'metadata_version': '1.1', + 'status': ['preview'], + 'supported_by': 'community' +} + +DOCUMENTATION = ''' +--- +module: win_snmp +version_added: '2.8' +short_description: Configures the Windows SNMP service +author: + - Michael Cassaniti (@mcassaniti) +description: + - This module configures the Windows SNMP service. +options: + permitted_managers: + description: + - The list of permitted SNMP managers. + type: list + community_strings: + description: + - The list of read-only SNMP community strings. + type: list + action: + description: + - C(add) will add new SNMP community strings and/or SNMP managers + - C(set) will replace SNMP community strings and/or SNMP managers. An + empty list for either C(community_strings) or C(permitted_managers) + will result in the respective lists being removed entirely. + - C(remove) will remove SNMP community strings and/or SNMP managers + default: set + choices: [ add, set, remove ] +''' + +EXAMPLES = ''' +--- + - hosts: Windows + tasks: + - name: Replace SNMP communities and managers + win_snmp: + communities: + - public + managers: + - 192.168.1.2 + action: set + + - hosts: Windows + tasks: + - name: Replace SNMP communities and clear managers + win_snmp: + communities: + - public + managers: [] + action: set +''' + +RETURN = ''' +community_strings: + description: The list of community strings for this machine + type: list + returned: always + sample: + - public + - snmp-ro +permitted_managers: + description: The list of permitted managers for this machine + type: list + returned: always + sample: + - 192.168.1.1 + - 192.168.1.2 +''' diff --git a/test/integration/targets/win_snmp/aliases b/test/integration/targets/win_snmp/aliases new file mode 100644 index 0000000000..423ce39108 --- /dev/null +++ b/test/integration/targets/win_snmp/aliases @@ -0,0 +1 @@ +shippable/windows/group2 diff --git a/test/integration/targets/win_snmp/tasks/cleanup.yml b/test/integration/targets/win_snmp/tasks/cleanup.yml new file mode 100644 index 0000000000..28d98debea --- /dev/null +++ b/test/integration/targets/win_snmp/tasks/cleanup.yml @@ -0,0 +1,16 @@ +--- + - name: Make sure there are no existing SNMP configuration settings + win_regedit: + path: "{{ item }}" + state: absent + loop: + - "{{ permitted_managers_key }}" + - "{{ valid_communities_key }}" + + - name: Create skeleton registry keys for SNMP + win_regedit: + path: "{{ item }}" + state: present + loop: + - "{{ permitted_managers_key }}" + - "{{ valid_communities_key }}" diff --git a/test/integration/targets/win_snmp/tasks/cleanup_using_module.yml b/test/integration/targets/win_snmp/tasks/cleanup_using_module.yml new file mode 100644 index 0000000000..290de7fc1f --- /dev/null +++ b/test/integration/targets/win_snmp/tasks/cleanup_using_module.yml @@ -0,0 +1,26 @@ +--- + - name: Set no SNMP community or SNMP manager + register: snmp_cleanup + win_snmp: + action: set + community_strings: [] + permitted_managers: [] + + - name: Check registry for no SNMP community + register: snmp_cleanup_reg_community + win_reg_stat: + path: "{{ valid_communities_key }}" + name: snmp-cleanup + + - name: Check registry for no SNMP manager + register: snmp_cleanup_reg_manager + win_reg_stat: + path: "{{ permitted_managers_key }}" + name: 1 + + - name: Asset SNMP set operation results in no remaining SNMP details + assert: + that: + - snmp_cleanup.changed + - snmp_cleanup_reg_community.exists == false + - snmp_cleanup_reg_manager.exists == false diff --git a/test/integration/targets/win_snmp/tasks/main.yml b/test/integration/targets/win_snmp/tasks/main.yml new file mode 100644 index 0000000000..09000d6e90 --- /dev/null +++ b/test/integration/targets/win_snmp/tasks/main.yml @@ -0,0 +1,8 @@ +--- + - include_tasks: cleanup.yml + - include_tasks: snmp_community.yml + - include_tasks: cleanup.yml + - include_tasks: snmp_managers.yml + - include_tasks: output_only.yml + - include_tasks: cleanup_using_module.yml + - include_tasks: cleanup.yml diff --git a/test/integration/targets/win_snmp/tasks/output_only.yml b/test/integration/targets/win_snmp/tasks/output_only.yml new file mode 100644 index 0000000000..7115da45d2 --- /dev/null +++ b/test/integration/targets/win_snmp/tasks/output_only.yml @@ -0,0 +1,24 @@ +--- + # Already tested + - name: Add an SNMP manager and an SNMP community + win_snmp: + action: add + community_strings: + - snmp-cleanup + permitted_managers: + - 192.168.1.1 + + - name: Run without options + register: snmp_no_options + win_snmp: + + - name: Assert no changes occurred when no options provided + assert: + that: + - not snmp_no_options.changed + + - name: Assert community strings and permitted managers are correctly returned + assert: + that: + - "'snmp-cleanup' in snmp_no_options.community_strings" + - "'192.168.1.1' in snmp_no_options.permitted_managers" diff --git a/test/integration/targets/win_snmp/tasks/snmp_community.yml b/test/integration/targets/win_snmp/tasks/snmp_community.yml new file mode 100644 index 0000000000..47a06297f2 --- /dev/null +++ b/test/integration/targets/win_snmp/tasks/snmp_community.yml @@ -0,0 +1,165 @@ +--- + - name: Add initial SNMP community + register: snmp_community + win_snmp: + action: add + community_strings: + - ansible-ro-test + + - name: Check initial SNMP community exists in registry + register: snmp_community_reg + win_reg_stat: + path: "{{ valid_communities_key }}" + name: ansible-ro-test + + - name: Assert initial SNMP community is correct + assert: + that: + - snmp_community is changed + - snmp_community_reg.exists + - snmp_community_reg.type == 'REG_DWORD' + - snmp_community_reg.value == 4 + + - name: Add initial SNMP community again + register: snmp_community_again + win_snmp: + action: add + community_strings: + - ansible-ro-test + + - name: Check no change occurred when adding SNMP community again + assert: + that: + - snmp_community_again is not changed + + - name: Add next SNMP community + register: snmp_community_next + win_snmp: + action: add + community_strings: + - ansible-ro-test-next + + - name: Check initial SNMP community still exists in registry + register: snmp_community_reg_orig + win_reg_stat: + path: "{{ valid_communities_key }}" + name: ansible-ro-test + + - name: Check next SNMP community exists in registry + register: snmp_community_reg_next + win_reg_stat: + path: "{{ valid_communities_key }}" + name: ansible-ro-test-next + + - name: Assert initial SNMP community still exists + assert: + that: + - snmp_community_reg_orig.exists + - snmp_community_reg_orig.type == 'REG_DWORD' + - snmp_community_reg_orig.value == 4 + + - name: Assert next SNMP community exists + assert: + that: + - snmp_community_next is changed + - snmp_community_reg_next.exists + - snmp_community_reg_next.type == 'REG_DWORD' + - snmp_community_reg_next.value == 4 + + - name: Replace SNMP community + register: snmp_community_replace + win_snmp: + action: set + community_strings: + - ansible-ro-test-replace + + - name: Check initial SNMP community does not exist in registry + register: snmp_community_reg_orig_replace + win_reg_stat: + path: "{{ valid_communities_key }}" + name: ansible-ro-test + + - name: Check next SNMP community does not exist in registry + register: snmp_community_reg_next_replace + win_reg_stat: + path: "{{ valid_communities_key }}" + name: ansible-ro-test-next + + - name: Check replace SNMP community exists in registry + register: snmp_community_reg_replace + win_reg_stat: + path: "{{ valid_communities_key }}" + name: ansible-ro-test-replace + + - name: Assert replace SNMP community exists and others are replaced + assert: + that: + - snmp_community_replace is changed + - snmp_community_reg_orig_replace.exists == false + - snmp_community_reg_next_replace.exists == false + - snmp_community_reg_replace.exists + - snmp_community_reg_replace.type == 'REG_DWORD' + - snmp_community_reg_replace.value == 4 + + # This task has already been tested + - name: Add another SNMP community before testing removal + win_snmp: + action: add + community_strings: + - ansible-ro-remove-add + + - name: Remove the replaced SNMP community + register: snmp_community_remove + win_snmp: + action: remove + community_strings: + - ansible-ro-test-replace + + - name: Check replace SNMP community is removed in registry + register: snmp_community_reg_remove + win_reg_stat: + path: "{{ valid_communities_key }}" + name: ansible-ro-test-replace + + - name: Check SNMP community that was added for testing removal exists in registry + register: snmp_community_reg_remove_add + win_reg_stat: + path: "{{ valid_communities_key }}" + name: ansible-ro-remove-add + + - name: Assert removal of SNMP community succeeded and next SNMP community remains + assert: + that: + - snmp_community_remove is changed + - snmp_community_reg_remove.exists == false + - snmp_community_reg_remove_add.exists + - snmp_community_reg_remove_add.type == 'REG_DWORD' + - snmp_community_reg_remove_add.value == 4 + + - name: Remove the replaced SNMP community (again) + register: snmp_community_remove + win_snmp: + action: remove + community_strings: + - ansible-ro-test-replace + + - name: Check replace SNMP community is removed in registry (again) + register: snmp_community_reg_remove + win_reg_stat: + path: "{{ valid_communities_key }}" + name: ansible-ro-test-replace + + - name: Check SNMP community that was added for testing removal exists in registry (again) + register: snmp_community_reg_remove_add + win_reg_stat: + path: "{{ valid_communities_key }}" + name: ansible-ro-remove-add + + - name: Assert removal of SNMP community succeeded and next SNMP community remains (again) + assert: + that: + - snmp_community_remove is not changed + - snmp_community_reg_remove.exists == false + - snmp_community_reg_remove_add.exists + - snmp_community_reg_remove_add.type == 'REG_DWORD' + - snmp_community_reg_remove_add.value == 4 diff --git a/test/integration/targets/win_snmp/tasks/snmp_managers.yml b/test/integration/targets/win_snmp/tasks/snmp_managers.yml new file mode 100644 index 0000000000..dedd0767f1 --- /dev/null +++ b/test/integration/targets/win_snmp/tasks/snmp_managers.yml @@ -0,0 +1,158 @@ +--- + - name: Add initial SNMP manager + register: snmp_manager + win_snmp: + action: add + permitted_managers: + - 192.168.1.1 + + - name: Check initial SNMP manager exists in registry + register: snmp_manager_reg + win_reg_stat: + path: "{{ permitted_managers_key }}" + name: 1 + + - name: Assert initial SNMP manager is correct + assert: + that: + - snmp_manager is changed + - snmp_manager_reg.exists + - snmp_manager_reg.type == 'REG_SZ' + - snmp_manager_reg.value == '192.168.1.1' + + - name: Add initial SNMP manager again + register: snmp_manager_again + win_snmp: + action: add + permitted_managers: + - 192.168.1.1 + + - name: Check no change occurred when adding SNMP manager again + assert: + that: + - snmp_manager_again is not changed + + - name: Add next SNMP manager + register: snmp_manager_next + win_snmp: + action: add + permitted_managers: + - 192.168.1.2 + + - name: Check initial SNMP manager still exists in registry + register: snmp_manager_reg_orig + win_reg_stat: + path: "{{ permitted_managers_key }}" + name: 1 + + - name: Check next SNMP manager exists in registry + register: snmp_manager_reg_next + win_reg_stat: + path: "{{ permitted_managers_key }}" + name: 2 + + - name: Assert initial SNMP manager still exists + assert: + that: + - snmp_manager_reg_orig.exists + - snmp_manager_reg_orig.type == 'REG_SZ' + - snmp_manager_reg_orig.value == '192.168.1.1' + + - name: Assert next SNMP manager exists + assert: + that: + - snmp_manager_next is changed + - snmp_manager_reg_next.exists + - snmp_manager_reg_next.type == 'REG_SZ' + - snmp_manager_reg_next.value == '192.168.1.2' + + - name: Replace SNMP manager + register: snmp_manager_replace + win_snmp: + action: set + permitted_managers: + - 192.168.1.10 + + - name: Check next SNMP manager does not exist in registry + register: snmp_manager_reg_next_replace + win_reg_stat: + path: "{{ permitted_managers_key }}" + name: 2 + + - name: Check replace SNMP manager exists in registry (overrides original slot) + register: snmp_manager_reg_replace + win_reg_stat: + path: "{{ permitted_managers_key }}" + name: 1 + + - name: Assert replace SNMP manager exists and others are replaced + assert: + that: + - snmp_manager_replace is changed + - snmp_manager_reg_next_replace.exists == false + - snmp_manager_reg_replace.exists + - snmp_manager_reg_replace.type == 'REG_SZ' + - snmp_manager_reg_replace.value == '192.168.1.10' + + # This task has already been tested + - name: Add another SNMP manager before testing removal + win_snmp: + action: add + permitted_managers: + - 192.168.1.20 + + - name: Remove the replaced SNMP manager + register: snmp_manager_remove + win_snmp: + action: remove + permitted_managers: + - 192.168.1.10 + + - name: Check replace SNMP manager is removed in registry + register: snmp_manager_reg_remove + win_reg_stat: + path: "{{ permitted_managers_key }}" + name: 1 + + - name: Check SNMP manager that was added for testing removal exists in registry + register: snmp_manager_reg_remove_add + win_reg_stat: + path: "{{ permitted_managers_key }}" + name: 2 + + - name: Assert removal of SNMP manager succeeded and next SNMP manager remains + assert: + that: + - snmp_manager_remove is changed + - snmp_manager_reg_remove.exists == false + - snmp_manager_reg_remove_add.exists + - snmp_manager_reg_remove_add.type == 'REG_SZ' + - snmp_manager_reg_remove_add.value == '192.168.1.20' + + - name: Remove the replaced SNMP manager (again) + register: snmp_manager_remove + win_snmp: + action: remove + permitted_managers: + - 192.168.1.10 + + - name: Check replace SNMP manager is removed in registry (again) + register: snmp_manager_reg_remove + win_reg_stat: + path: "{{ permitted_managers_key }}" + name: 1 + + - name: Check SNMP manager that was added for testing removal exists in registry (again) + register: snmp_manager_reg_remove_add + win_reg_stat: + path: "{{ permitted_managers_key }}" + name: 2 + + - name: Assert removal of SNMP manager succeeded and next SNMP manager remains (again) + assert: + that: + - snmp_manager_remove is not changed + - snmp_manager_reg_remove.exists == false + - snmp_manager_reg_remove_add.exists + - snmp_manager_reg_remove_add.type == 'REG_SZ' + - snmp_manager_reg_remove_add.value == '192.168.1.20' diff --git a/test/integration/targets/win_snmp/vars/main.yml b/test/integration/targets/win_snmp/vars/main.yml new file mode 100644 index 0000000000..610be839fc --- /dev/null +++ b/test/integration/targets/win_snmp/vars/main.yml @@ -0,0 +1,3 @@ +--- + permitted_managers_key: 'HKLM:\System\CurrentControlSet\services\SNMP\Parameters\PermittedManagers' + valid_communities_key: 'HKLM:\System\CurrentControlSet\services\SNMP\Parameters\ValidCommunities'