diff --git a/plugins/modules/firewall.py b/plugins/modules/firewall.py index 498757549..c555e6e73 100644 --- a/plugins/modules/firewall.py +++ b/plugins/modules/firewall.py @@ -7,6 +7,7 @@ import copy import ipaddress +import time from typing import Any, List, Optional import ansible_collections.linode.cloud.plugins.module_utils.doc_fragments.firewall as docs @@ -219,14 +220,28 @@ def __init__(self) -> None: super().__init__(module_arg_spec=self.module_arg_spec) def _get_firewall_by_label(self, label: str) -> Optional[Firewall]: - try: - return self.client.networking.firewalls(Firewall.label == label)[0] - except IndexError: - return None - except Exception as exception: - return self.fail( - msg="failed to get firewall {0}: {1}".format(label, exception) - ) + attempt = 0 + retries = 5 + retry_delay = 3 + + while attempt < retries: + try: + return self.client.networking.firewalls( + Firewall.label == label + )[0] + except IndexError: + attempt += 1 + if attempt < retries: + time.sleep(retry_delay) + else: + return None + except Exception as exception: + return self.fail( + msg="failed to get firewall {0}: {1}".format( + label, exception + ) + ) + return None def _create_firewall(self) -> Optional[Firewall]: params = self.module.params @@ -348,20 +363,21 @@ def _update_rules(self, remote_rules: dict, local_rules: dict) -> dict: if self._state != "update": return local_rules - # Add new local rules to remote rules if they don't exist + # Amend existing rules and append new local rules for direction in ["inbound", "outbound"]: - rlr = { - remote_rule["label"] - for remote_rule in remote_rules.get(direction, {}) + remote_labeled = { + r["label"] for r in remote_rules.get(direction, []) } - for local_rule in local_rules.get(direction, {}): - if local_rule["label"] not in rlr: - remote_rules[direction].append(local_rule) - - for direction in ["inbound", "outbound"]: - local_rules[direction] = self._amend_rules( - remote_rules[direction], local_rules[direction] + # Start with amended existing remote rules + amended = self._amend_rules( + remote_rules.get(direction, []), + local_rules.get(direction, []), ) + # Append any new local rules not present in remote + for local_rule in local_rules.get(direction, []): + if local_rule["label"] not in remote_labeled: + amended.append(local_rule) + local_rules[direction] = amended return local_rules def _change_rules(self) -> Optional[dict]: @@ -438,6 +454,12 @@ def _handle_present(self) -> None: self._firewall = self._get_firewall_by_label(label) if self._firewall is None: + if self._state == "update": + self.fail( + msg="Firewall {0} does not exist and cannot be updated.".format( + label + ) + ) self._firewall = self._create_firewall() self.register_action("Created Firewall {0}".format(label)) diff --git a/tests/integration/targets/firewall_update/tasks/main.yaml b/tests/integration/targets/firewall_update/tasks/main.yaml index af77ed25c..65faf2171 100644 --- a/tests/integration/targets/firewall_update/tasks/main.yaml +++ b/tests/integration/targets/firewall_update/tasks/main.yaml @@ -79,7 +79,7 @@ - updated_firewall.firewall.rules.inbound | length == 1 - (updated_firewall.firewall.rules.inbound[0].addresses['ipv4'] is defined) - (updated_firewall.firewall.rules.inbound[0].addresses['ipv6'] is not defined) - + - firewall_info.firewall.id == create.firewall.id - firewall_info.devices|length == updated_firewall.devices|length - firewall_info.firewall.status == updated_firewall.firewall.status @@ -158,7 +158,7 @@ - updated_firewall.firewall.rules.inbound | length == 1 - (updated_firewall.firewall.rules.inbound[0].addresses['ipv4'] is not defined) - (updated_firewall.firewall.rules.inbound[0].addresses['ipv6'] is defined) - + - firewall_info.firewall.id == create.firewall.id - firewall_info.devices|length == updated_firewall.devices|length - firewall_info.firewall.status == updated_firewall.firewall.status @@ -238,7 +238,7 @@ - updated_firewall.firewall.rules.inbound | length == 1 - (updated_firewall.firewall.rules.inbound[0].addresses['ipv4'] is defined) and (updated_firewall.firewall.rules.inbound[0].addresses['ipv4'] == ['0.0.0.0/0']) - (updated_firewall.firewall.rules.inbound[0].addresses['ipv6'] is defined) and (updated_firewall.firewall.rules.inbound[0].addresses['ipv6'] == ['ff00::/8']) - + - firewall_info.firewall.id == create.firewall.id - firewall_info.devices|length == updated_firewall.devices|length - firewall_info.firewall.status == updated_firewall.firewall.status @@ -286,7 +286,7 @@ that: - updated_firewall.changed - updated_firewall.firewall.status == 'disabled' - + - firewall_info.firewall.id == create.firewall.id - firewall_info.devices|length == updated_firewall.devices|length - firewall_info.firewall.status == updated_firewall.firewall.status @@ -326,7 +326,7 @@ that: - updated_firewall.changed - updated_firewall.firewall.status == 'enabled' - + - firewall_info.firewall.id == create.firewall.id - firewall_info.devices|length == updated_firewall.devices|length - firewall_info.firewall.status == updated_firewall.firewall.status @@ -370,7 +370,7 @@ - updated_firewall.changed - updated_firewall.firewall.rules.inbound_policy == 'ACCEPT' - updated_firewall.firewall.rules.outbound_policy == 'ACCEPT' - + - firewall_info.firewall.id == create.firewall.id - firewall_info.devices|length == updated_firewall.devices|length - firewall_info.firewall.status == updated_firewall.firewall.status @@ -447,7 +447,7 @@ - updated_firewall.firewall.rules.outbound_policy == 'DROP' - updated_firewall.firewall.rules.outbound[0].addresses['ipv4'] == ['0.0.0.0/0'] - updated_firewall.firewall.rules.outbound[0].addresses['ipv6'] == ['ff00::/8'] - + - firewall_info.firewall.id == create.firewall.id - firewall_info.devices|length == updated_firewall.devices|length - firewall_info.firewall.status == updated_firewall.firewall.status @@ -507,7 +507,7 @@ - updated_firewall.firewall.rules.outbound[0].ports == '80,443' - updated_firewall.firewall.rules.outbound[0].protocol == 'TCP' - updated_firewall.firewall.rules.outbound[0].action == 'ACCEPT' - + - firewall_info.firewall.id == create.firewall.id - firewall_info.devices|length == updated_firewall.devices|length - firewall_info.firewall.status == updated_firewall.firewall.status @@ -570,7 +570,7 @@ - updated_firewall.firewall.rules.outbound[0].protocol == 'TCP' - updated_firewall.firewall.rules.outbound[0].action == 'ACCEPT' - updated_firewall.firewall.rules.outbound_policy == 'DROP' - + - firewall_info.firewall.id == create.firewall.id - firewall_info.devices|length == updated_firewall.devices|length - firewall_info.firewall.status == updated_firewall.firewall.status @@ -635,7 +635,7 @@ - updated_firewall.firewall.rules.outbound[0].action == 'ACCEPT' - updated_firewall.firewall.rules.outbound[0].addresses['ipv4'] == ['0.0.0.0/0','8.8.8.8/32' ] - updated_firewall.firewall.rules.outbound_policy == 'DROP' - + - firewall_info.firewall.id == create.firewall.id - firewall_info.devices|length == updated_firewall.devices|length - firewall_info.firewall.status == updated_firewall.firewall.status @@ -700,7 +700,7 @@ - updated_firewall.firewall.rules.outbound[0].action == 'ACCEPT' - updated_firewall.firewall.rules.outbound[0].addresses['ipv4'] == ['0.0.0.0/0','8.8.8.8/32' ] - updated_firewall.firewall.rules.outbound_policy == 'DROP' - + - firewall_info.firewall.id == create.firewall.id - firewall_info.devices|length == updated_firewall.devices|length - firewall_info.firewall.status == updated_firewall.firewall.status @@ -749,7 +749,7 @@ - updated_firewall.changed - updated_firewall.firewall.rules.inbound[0].description == 'Amazing firewall rule.' - updated_firewall.firewall.rules.outbound[0].description == 'Amazing firewall rule.' - + - firewall_info.firewall.id == create.firewall.id - firewall_info.devices|length == updated_firewall.devices|length - firewall_info.firewall.status == updated_firewall.firewall.status @@ -798,7 +798,7 @@ assert: that: - firewall_info.devices|length == 0 - + - firewall_info.firewall.id == create.firewall.id - firewall_info.devices|length == updated_firewall.devices|length - firewall_info.firewall.status == updated_firewall.firewall.status @@ -847,7 +847,7 @@ assert: that: - not updated_firewall.changed - + - firewall_info.firewall.id == create.firewall.id - firewall_info.devices|length == updated_firewall.devices|length - firewall_info.firewall.status == updated_firewall.firewall.status @@ -867,6 +867,70 @@ - (firewall_info.firewall.rules.outbound | length == 0 and updated_firewall.firewall.rules.outbound | length == 0) or ((firewall_info.firewall.rules.outbound[0].ports is not defined and updated_firewall.firewall.rules.outbound[0].ports is not defined) or (firewall_info.firewall.rules.outbound[0].ports == updated_firewall.firewall.rules.outbound[0].ports)) - (firewall_info.firewall.rules.outbound | length == 0 and updated_firewall.firewall.rules.outbound | length == 0) or ((firewall_info.firewall.rules.outbound[0].protocol is not defined and updated_firewall.firewall.rules.outbound[0].protocol is not defined) or (firewall_info.firewall.rules.outbound[0].protocol == updated_firewall.firewall.rules.outbound[0].protocol)) - (firewall_info.firewall.rules.outbound | length == 0 and updated_firewall.firewall.rules.outbound | length == 0) or ((firewall_info.firewall.rules.outbound[0].action is not defined and updated_firewall.firewall.rules.outbound[0].action is not defined) or (firewall_info.firewall.rules.outbound[0].action == updated_firewall.firewall.rules.outbound[0].action)) + + - name: Append a new inbound rule via update + linode.cloud.firewall: + state: update + api_version: v4beta + label: '{{ create.firewall.label }}' + rules: + inbound: + - label: new_rule + action: ACCEPT + protocol: TCP + addresses: + ipv4: ['2.2.2.2/32'] + description: 'appended rule' + ports: '8080' + register: append_rule + + - name: Assert new rule was appended + assert: + that: + - append_rule.changed + - append_rule.firewall.rules.inbound | length >= 2 + - append_rule.firewall.rules.inbound | selectattr('label', 'equalto', 'new_rule') | list | length == 1 + + - name: Get firewall info after appending a new inbound rule + linode.cloud.firewall_info: + api_version: v4beta + label: '{{ create.firewall.label }}' + register: append_rule_info + + - name: Assert appended rule exists in remote firewall state + assert: + that: + - append_rule_info.firewall.rules.inbound | length >= 2 + - append_rule_info.firewall.rules.inbound | selectattr('label', 'equalto', 'new_rule') | list | length == 1 + + - name: Re-run append update to verify idempotency + linode.cloud.firewall: + state: update + api_version: v4beta + label: '{{ create.firewall.label }}' + rules: + inbound: + - label: new_rule + action: ACCEPT + protocol: TCP + addresses: + ipv4: ['2.2.2.2/32'] + description: 'appended rule' + ports: '8080' + register: append_rule_again + + - name: Get firewall info after idempotency check + linode.cloud.firewall_info: + api_version: v4beta + label: '{{ create.firewall.label }}' + register: append_rule_info_again + + - name: Assert append update is idempotent + assert: + that: + - not append_rule_again.changed + - append_rule_info_again.firewall.rules.inbound | selectattr('label', 'equalto', 'new_rule') | list | length == 1 + always: - ignore_errors: yes block: