From 2b63ae61f2847d55ae4f8882acf9fdac831f1ae6 Mon Sep 17 00:00:00 2001 From: Artem Zinenko Date: Tue, 10 Oct 2017 09:23:08 +0300 Subject: [PATCH] minor updates + tests to win_firewall_rule as per jborean93 review (#29148) * Added warning for 'force' option * Changed 'profiles' type to list * Changed 'interfacetypes' type to list * Added deprecation warning and fixed doc * updated force parameter --- .../modules/windows/win_firewall_rule.ps1 | 24 ++++---- .../modules/windows/win_firewall_rule.py | 7 +++ .../targets/win_firewall_rule/tasks/main.yml | 59 +++++++++++++++++-- 3 files changed, 76 insertions(+), 14 deletions(-) diff --git a/lib/ansible/modules/windows/win_firewall_rule.ps1 b/lib/ansible/modules/windows/win_firewall_rule.ps1 index b3077f7c95..f5f1760991 100644 --- a/lib/ansible/modules/windows/win_firewall_rule.ps1 +++ b/lib/ansible/modules/windows/win_firewall_rule.ps1 @@ -48,9 +48,9 @@ function Parse-Action { # Profile enum values: https://msdn.microsoft.com/en-us/library/windows/desktop/aa366303(v=vs.85).aspx function Parse-Profiles { - param($profilesStr) + param($profilesList) - $profiles = ($profilesStr.Split(',') | Select -uniq | ForEach { + $profiles = ($profilesList | Select -uniq | ForEach { switch ($_) { "domain" { return 1 } "private" { return 2 } @@ -65,9 +65,9 @@ function Parse-Profiles function Parse-InterfaceTypes { - param($interfaceTypesStr) + param($interfaceTypes) - return ($interfaceTypesStr.Split(',') | Select -uniq | ForEach { + return ($interfaceTypes | Select -uniq | ForEach { switch ($_) { "wireless" { return "Wireless" } "lan" { return "Lan" } @@ -117,8 +117,8 @@ function New-FWRule [string]$direction, [string]$action, [bool]$enabled, - [string]$profiles, - [string]$interfaceTypes, + [string[]]$profiles, + [string[]]$interfaceTypes, [string]$edgeTraversalOptions, [string]$secureFlags ) @@ -137,8 +137,8 @@ function New-FWRule if ($remoteAddresses -and $remoteAddresses -ne "any") { $rule.RemoteAddresses = $remoteAddresses } if ($direction) { $rule.Direction = Parse-Direction -directionStr $direction } if ($action) { $rule.Action = Parse-Action -actionStr $action } - if ($profiles) { $rule.Profiles = Parse-Profiles -profilesStr $profiles } - if ($interfaceTypes -and $interfaceTypes -ne "any") { $rule.InterfaceTypes = Parse-InterfaceTypes -interfaceTypesStr $interfaceTypes } + if ($profiles) { $rule.Profiles = Parse-Profiles -profilesList $profiles } + if ($interfaceTypes -and @(Compare-Object $interfaceTypes @("any")).Count -ne 0) { $rule.InterfaceTypes = Parse-InterfaceTypes -interfaceTypes $interfaceTypes } if ($edgeTraversalOptions -and $edgeTraversalOptions -ne "no") { # EdgeTraversalOptions property exists only from Windows 7/Windows Server 2008 R2: https://msdn.microsoft.com/en-us/library/windows/desktop/dd607256(v=vs.85).aspx if ($rule | Get-Member -Name 'EdgeTraversalOptions') { @@ -172,18 +172,22 @@ $action = Get-AnsibleParam -obj $params -name "action" -type "str" -failifempty $program = Get-AnsibleParam -obj $params -name "program" -type "str" $service = Get-AnsibleParam -obj $params -name "service" -type "str" $enabled = Get-AnsibleParam -obj $params -name "enabled" -type "bool" -default $true -aliases "enable" -$profiles = Get-AnsibleParam -obj $params -name "profiles" -type "str" -default "domain,private,public" -aliases "profile" +$profiles = Get-AnsibleParam -obj $params -name "profiles" -type "list" -default @("domain", "private", "public") -aliases "profile" $localip = Get-AnsibleParam -obj $params -name "localip" -type "str" -default "any" $remoteip = Get-AnsibleParam -obj $params -name "remoteip" -type "str" -default "any" $localport = Get-AnsibleParam -obj $params -name "localport" -type "str" $remoteport = Get-AnsibleParam -obj $params -name "remoteport" -type "str" $protocol = Get-AnsibleParam -obj $params -name "protocol" -type "str" -default "any" -$interfacetypes = Get-AnsibleParam -obj $params -name "interfacetypes" -type "str" -default "any" +$interfacetypes = Get-AnsibleParam -obj $params -name "interfacetypes" -type "list" -default @("any") $edge = Get-AnsibleParam -obj $params -name "edge" -type "str" -default "no" -validateset "no","yes","deferapp","deferuser" $security = Get-AnsibleParam -obj $params -name "security" -type "str" -default "notrequired" -validateset "notrequired","authnoencap","authenticate","authdynenc","authenc" $state = Get-AnsibleParam -obj $params -name "state" -type "str" -default "present" -validateset "present","absent" + $force = Get-AnsibleParam -obj $params -name "force" -type "bool" -default $false +if ($force) { + Add-DeprecationWarning -obj $result -message "'force' isn't required anymore" -version 2.9 +} if ($diff_support) { $result.diff = @{} diff --git a/lib/ansible/modules/windows/win_firewall_rule.py b/lib/ansible/modules/windows/win_firewall_rule.py index 2819a0fff3..4c81f2dd41 100644 --- a/lib/ansible/modules/windows/win_firewall_rule.py +++ b/lib/ansible/modules/windows/win_firewall_rule.py @@ -76,6 +76,13 @@ options: - The profile this rule applies to. default: 'domain,private,public' aliases: [ 'profile' ] + force: + description: + - Replace any existing rule by removing it first. + - This is no longer required in 2.4 as rules no longer need replacing when being modified. + - DEPRECATED in 2.4 and will be removed in 2.9. + default: 'no' + choices: [ 'no', 'yes' ] ''' EXAMPLES = r''' diff --git a/test/integration/targets/win_firewall_rule/tasks/main.yml b/test/integration/targets/win_firewall_rule/tasks/main.yml index b847367ee5..8e42be7722 100644 --- a/test/integration/targets/win_firewall_rule/tasks/main.yml +++ b/test/integration/targets/win_firewall_rule/tasks/main.yml @@ -251,7 +251,7 @@ that: - add_firewall_rule_with_multiple_ports.changed == true -- name: Add firewall rule with interface types +- name: Add firewall rule with interface types in string format win_firewall_rule: name: http enabled: yes @@ -261,12 +261,29 @@ direction: in protocol: tcp interfacetypes: 'ras,lan,wireless' - register: add_firewall_rule_with_interface_types + register: add_firewall_rule_with_string_interface_types -- name: Check that creating firewall rule with interface types succeeds with a change +- name: Check that creating firewall rule with interface types in string format succeeds with a change assert: that: - - add_firewall_rule_with_interface_types.changed == true + - add_firewall_rule_with_string_interface_types.changed == true + +- name: Add firewall rule with interface types in list format + win_firewall_rule: + name: http + enabled: yes + state: present + localport: 80 + action: allow + direction: in + protocol: tcp + interfacetypes: [ras, lan] + register: add_firewall_rule_with_list_interface_types + +- name: Check that creating firewall rule with interface types in list format succeeds with a change + assert: + that: + - add_firewall_rule_with_list_interface_types.changed == true - name: Add firewall rule with interface type 'any' win_firewall_rule: @@ -325,3 +342,37 @@ - add_firewall_rule_with_secure_flags.changed == true # Works on windows >= Windows 8/Windows Server 2012 when: ansible_distribution_version | version_compare('6.2', '>=') + +- name: Add firewall rule with profiles in string format + win_firewall_rule: + name: http + enabled: yes + state: present + localport: 80 + action: allow + direction: in + protocol: tcp + profiles: 'domain,public' + register: add_firewall_rule_with_string_profiles + +- name: Check that creating firewall rule with profiles in string format succeeds with a change + assert: + that: + - add_firewall_rule_with_string_profiles.changed == true + +- name: Add firewall rule with profiles in list format + win_firewall_rule: + name: http + enabled: yes + state: present + localport: 80 + action: allow + direction: in + protocol: tcp + profiles: [Domain, Private] + register: add_firewall_rule_with_list_profiles + +- name: Check that creating firewall rule with profiles in list format succeeds with a change + assert: + that: + - add_firewall_rule_with_list_profiles.changed == true