win_firewall_rule doesn't fail when profile is "any" or remoteip is IPv4 and the task runs more than once. (#22555)

* Fixed #22554

* Wrote tests for win_firewall_rule module

* Fixed #22786

* Fixed review comments

* Fixed #22799

* Added test when RemoteIP containt a netmask

* Revert comment

lib/ansible/modules/windows/win_firewall_rule.ps1

@@ -20,6 +20,49 @@
 # WANT_JSON
 # POWERSHELL_COMMON
 
+function convertToNetmask($maskLength) {
+    [IPAddress] $ip = 0;
+    $ip.Address = ([UInt32]::MaxValue) -shl (32 - $maskLength) -shr (32 - $maskLength)
+    return $ip.IPAddressToString
+}
+
+function preprocessAndCompare($key, $outputValue, $fwsettingValue) {
+    if ($key -eq 'RemoteIP') {
+        if ($outputValue -eq $fwsettingValue) {
+            return $true
+        }
+
+        if ($outputValue -eq $fwsettingValue+'-'+$fwsettingValue) {
+            return $true
+        }
+
+        if (($outputValue -eq $fwsettingValue+'/32') -or ($outputValue -eq $fwsettingValue+'/255.255.255.255')) {
+            return $true
+        }
+
+        if ($outputValue -match '^([\d\.]+)\/(\d+)$') {
+            $netmask = convertToNetmask($Matches[2])
+            if ($fwsettingValue -eq $Matches[1]+"/"+$netmask) {
+                return $true
+            }
+        }
+
+        if ($fwsettingValue -match '^([\d\.]+)\/(\d+)$') {
+            $netmask = convertToNetmask($Matches[2])
+            if ($outputValue -eq $Matches[1]+"/"+$netmask) {
+                return $true
+            }
+        }
+    }
+    elseif ($key -eq 'Profiles') {
+        if (($fwsettingValue -eq "any") -and ($outputValue -eq "Domain,Private,Public")) {
+            return $true
+        }
+    }
+
+    return $false
+}
+
 function getFirewallRule ($fwsettings) {
     try {
 
@@ -69,12 +112,11 @@ function getFirewallRule ($fwsettings) {
                 };
             } else {
                 ForEach($fwsetting in $fwsettings.GetEnumerator()) {
-                    if ( $output.$($fwsetting.Key) -ne $fwsettings.$($fwsetting.Key)) {
-
-                        if (($fwsetting.Key -eq 'RemoteIP') -and ($output.$($fwsetting.Key) -eq ($fwsettings.$($fwsetting.Key)+'-'+$fwsettings.$($fwsetting.Key)))) {
-                            $donothing=$false
+                    if ($output.$($fwsetting.Key) -ne $fwsettings.$($fwsetting.Key)) {
+                        if ((preprocessAndCompare -key $fwsetting.Key -outputValue $output.$($fwsetting.Key) -fwsettingValue $fwsettings.$($fwsetting.Key))) {
+                            Continue
                         } elseif (($fwsetting.Key -eq 'DisplayName') -and ($output."Rule Name" -eq $fwsettings.$($fwsetting.Key))) {
-                            $donothing=$false
+                            Continue
                         } else {
                             $diff=$true;
                             $difference+=@($fwsettings.$($fwsetting.Key));

test/integration/targets/win_firewall_rule/aliases

@@ -0,0 +1 @@
+windows/ci/group3

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

@@ -0,0 +1,245 @@
+- name: Remove potentially leftover firewall rule
+  win_firewall_rule:
+    name: http
+    state: absent
+    action: "{{ item }}"
+    direction: In
+  with_items:
+     - allow
+     - block
+
+- name: Add firewall rule
+  win_firewall_rule:
+    name: http
+    enable: yes
+    state: present
+    localport: 80
+    action: allow
+    direction: In
+    protocol: TCP
+  register: add_firewall_rule
+
+- name: Check that creating new firewall rule succeeds with a change
+  assert:
+    that:
+    - add_firewall_rule.failed == false
+    - add_firewall_rule.changed == true
+
+- name: Add same firewall rule (again)
+  win_firewall_rule:
+    name: http
+    enable: yes
+    state: present
+    localport: 80
+    action: allow
+    direction: In
+    protocol: TCP
+  register: add_firewall_rule_again
+
+- name: Check that creating same firewall rule succeeds without a change
+  assert:
+    that:
+    - add_firewall_rule_again.failed == false
+    - add_firewall_rule_again.changed == false
+
+- name: Remove firewall rule
+  win_firewall_rule:
+    name: http
+    enable: yes
+    state: absent
+    localport: 80
+    action: allow
+    direction: In
+    protocol: TCP
+  register: remove_firewall_rule
+
+- name: Check that removing existing firewall rule succeeds with a change
+  assert:
+    that:
+    - remove_firewall_rule.failed == false
+    - remove_firewall_rule.changed == true
+
+- name: Remove absent firewall rule
+  win_firewall_rule:
+    name: http
+    enable: yes
+    state: absent
+    localport: 80
+    action: allow
+    direction: In
+    protocol: TCP
+  register: remove_absent_firewall_rule
+
+- name: Check that removing non existing firewall rule succeeds without a change
+  assert:
+    that:
+    - remove_absent_firewall_rule.failed == false
+    - remove_absent_firewall_rule.changed == false
+
+- name: Add firewall rule
+  win_firewall_rule:
+    name: http
+    enable: yes
+    state: present
+    localport: 80
+    action: allow
+    direction: In
+    protocol: TCP
+
+- name: Add different firewall rule
+  win_firewall_rule:
+    name: http
+    enable: yes
+    state: present
+    localport: 80
+    action: block
+    direction: In
+    protocol: TCP
+  ignore_errors: yes
+  register: add_different_firewall_rule_without_force
+
+- name: Check that creating different firewall rule without enabling force setting fails
+  assert:
+    that:
+    - add_different_firewall_rule_without_force.failed == true
+    - add_different_firewall_rule_without_force.changed == false
+    - add_different_firewall_rule_without_force.difference == ["block"]
+
+- name: Add different firewall rule with force setting
+  win_firewall_rule:
+    name: http
+    enable: yes
+    state: present
+    localport: 80
+    action: block
+    direction: In
+    protocol: TCP
+    force: yes
+  register: add_different_firewall_rule_with_force
+
+- name: Check that creating different firewall rule with enabling force setting succeeds
+  assert:
+    that:
+    - add_different_firewall_rule_with_force.failed == false
+    - add_different_firewall_rule_with_force.changed == true
+    - add_different_firewall_rule_with_force.difference == ["block"]
+
+- name: Add firewall rule when remoteip is range
+  win_firewall_rule:
+    name: http
+    enable: yes
+    state: present
+    localport: 80
+    remoteip: 192.168.0.1-192.168.0.5
+    action: allow
+    direction: In
+    protocol: TCP
+    force: yes
+
+- name: Add same firewall rule when remoteip is range (again)
+  win_firewall_rule:
+    name: http
+    enable: yes
+    state: present
+    localport: 80
+    remoteip: 192.168.0.1-192.168.0.5
+    action: allow
+    direction: In
+    protocol: TCP
+  register: add_firewall_rule_with_range_remoteip_again
+
+- name: Check that creating same firewall rule when remoteip is range succeeds without a change
+  assert:
+    that:
+    - add_firewall_rule_with_range_remoteip_again.failed == false
+    - add_firewall_rule_with_range_remoteip_again.changed == false
+
+- name: Add firewall rule when remoteip in CIDR notation
+  win_firewall_rule:
+    name: http
+    enable: yes
+    state: present
+    localport: 80
+    remoteip: 192.168.0.0/24
+    action: allow
+    direction: In
+    protocol: TCP
+    force: yes
+
+- name: Add same firewall rule when remoteip in CIDR notation (again)
+  win_firewall_rule:
+    name: http
+    enable: yes
+    state: present
+    localport: 80
+    remoteip: 192.168.0.0/24
+    action: allow
+    direction: In
+    protocol: TCP
+  register: add_firewall_rule_with_cidr_remoteip_again
+
+- name: Check that creating same firewall rule succeeds without a change when remoteip in CIDR notation
+  assert:
+    that:
+    - add_firewall_rule_with_cidr_remoteip_again.failed == false
+    - add_firewall_rule_with_cidr_remoteip_again.changed == false
+
+- name: Add firewall rule when remoteip contains a netmask
+  win_firewall_rule:
+    name: http
+    enable: yes
+    state: present
+    localport: 80
+    remoteip: 192.168.0.0/255.255.255.0
+    action: allow
+    direction: In
+    protocol: TCP
+    force: yes
+
+- name: Add same firewall rule when remoteip contains a netmask (again)
+  win_firewall_rule:
+    name: http
+    enable: yes
+    state: present
+    localport: 80
+    remoteip: 192.168.0.0/255.255.255.0
+    action: allow
+    direction: In
+    protocol: TCP
+  register: add_firewall_rule_remoteip_contains_netmask_again
+
+- name: Check that creating same firewall rule succeeds without a change when remoteip contains a netmask
+  assert:
+    that:
+    - add_firewall_rule_remoteip_contains_netmask_again.failed == false
+    - add_firewall_rule_remoteip_contains_netmask_again.changed == false
+
+- name: Add firewall rule when remoteip is IPv4
+  win_firewall_rule:
+    name: http
+    enable: yes
+    state: present
+    localport: 80
+    remoteip: 192.168.0.1
+    action: allow
+    direction: In
+    protocol: TCP
+    force: yes
+
+- name: Add same firewall rule when remoteip is IPv4 (again)
+  win_firewall_rule:
+    name: http
+    enable: yes
+    state: present
+    localport: 80
+    remoteip: 192.168.0.1
+    action: allow
+    direction: In
+    protocol: TCP
+  register: add_firewall_rule_with_ipv4_remoteip_again
+
+- name: Check that creating same firewall rule when remoteip is IPv4 succeeds without a change
+  assert:
+    that:
+    - add_firewall_rule_with_ipv4_remoteip_again.failed == false
+    - add_firewall_rule_with_ipv4_remoteip_again.changed == false