From 559d31561ad1e0fcf8dd14523bd3eb4262a8a3c1 Mon Sep 17 00:00:00 2001 From: Jonas L Date: Fri, 2 Feb 2024 09:48:56 +0100 Subject: [PATCH] feat: allow forcing the deletion of firewalls that are still in use (#447) ##### SUMMARY - Do not silence 'firewall still in use' deletions errors. - Allow forcing the deletion of a firewall that is still in use. Fixes #380 ##### ISSUE TYPE - Feature Pull Request ##### COMPONENT NAME firewall --- .../fragments/fix-firewall-deletion.yml | 3 ++ plugins/modules/firewall.py | 31 +++++++++++++++---- .../targets/firewall/defaults/main/main.yml | 1 + .../targets/firewall/tasks/cleanup.yml | 5 +++ .../targets/firewall/tasks/prepare.yml | 8 +++++ .../targets/firewall/tasks/test.yml | 21 ++++++++++++- 6 files changed, 62 insertions(+), 7 deletions(-) create mode 100644 changelogs/fragments/fix-firewall-deletion.yml create mode 100644 tests/integration/targets/firewall/tasks/prepare.yml diff --git a/changelogs/fragments/fix-firewall-deletion.yml b/changelogs/fragments/fix-firewall-deletion.yml new file mode 100644 index 0000000..a9797e3 --- /dev/null +++ b/changelogs/fragments/fix-firewall-deletion.yml @@ -0,0 +1,3 @@ +minor_changes: + - firewall - Do not silence 'firewall still in use' delete failures. + - firewall - Allow forcing the deletion of firewalls that are still in use. diff --git a/plugins/modules/firewall.py b/plugins/modules/firewall.py index 8f6d0fb..3c51b5c 100644 --- a/plugins/modules/firewall.py +++ b/plugins/modules/firewall.py @@ -74,6 +74,11 @@ options: type: list elements: str default: [] + force: + description: + - Force the deletion of the Firewall when still in use. + type: bool + default: false state: description: - State of the firewall. @@ -350,19 +355,32 @@ class AnsibleHCloudFirewall(AnsibleHCloud): self._get_firewall() if self.hcloud_firewall is not None: if not self.module.check_mode: + if self.hcloud_firewall.applied_to: + if self.module.params.get("force"): + actions = self.hcloud_firewall.remove_from_resources(self.hcloud_firewall.applied_to) + for action in actions: + action.wait_until_finished() + else: + self.module.warn( + f"Firewall {self.hcloud_firewall.name} is currently used by " + "other resources. You need to unassign the resources before " + "deleting the Firewall or use force=true." + ) + retry_count = 0 - while retry_count < 10: + while True: try: - self.client.firewalls.delete(self.hcloud_firewall) + self.hcloud_firewall.delete() break except APIException as exception: - if "is still in use" in exception.message: - retry_count = retry_count + 1 + if "is still in use" in exception.message and retry_count < 10: + retry_count += 1 time.sleep(0.5 * retry_count) - else: - self.fail_json_hcloud(exception) + continue + self.fail_json_hcloud(exception) except HCloudException as exception: self.fail_json_hcloud(exception) + self._mark_as_changed() self.hcloud_firewall = None @@ -390,6 +408,7 @@ class AnsibleHCloudFirewall(AnsibleHCloud): ["protocol", "tcp", ["port"]], ], ), + force={"type": "bool", "default": False}, state={ "choices": ["absent", "present"], "default": "present", diff --git a/tests/integration/targets/firewall/defaults/main/main.yml b/tests/integration/targets/firewall/defaults/main/main.yml index 3c62b70..989e14f 100644 --- a/tests/integration/targets/firewall/defaults/main/main.yml +++ b/tests/integration/targets/firewall/defaults/main/main.yml @@ -2,3 +2,4 @@ # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) --- hcloud_firewall_name: "{{ hcloud_ns }}" +hcloud_server_name: "{{ hcloud_ns }}" diff --git a/tests/integration/targets/firewall/tasks/cleanup.yml b/tests/integration/targets/firewall/tasks/cleanup.yml index 8d5c1e1..37fbd34 100644 --- a/tests/integration/targets/firewall/tasks/cleanup.yml +++ b/tests/integration/targets/firewall/tasks/cleanup.yml @@ -1,4 +1,9 @@ --- +- name: Cleanup test_server + hetzner.hcloud.server: + name: "{{ hcloud_server_name }}" + state: absent + - name: Cleanup test_firewall hetzner.hcloud.firewall: name: "{{ hcloud_firewall_name }}" diff --git a/tests/integration/targets/firewall/tasks/prepare.yml b/tests/integration/targets/firewall/tasks/prepare.yml new file mode 100644 index 0000000..cf6daa3 --- /dev/null +++ b/tests/integration/targets/firewall/tasks/prepare.yml @@ -0,0 +1,8 @@ +--- +- name: Create test_server + hetzner.hcloud.server: + name: "{{ hcloud_server_name }}" + server_type: cx11 + image: ubuntu-22.04 + state: stopped + register: test_server diff --git a/tests/integration/targets/firewall/tasks/test.yml b/tests/integration/targets/firewall/tasks/test.yml index ff58170..5705984 100644 --- a/tests/integration/targets/firewall/tasks/test.yml +++ b/tests/integration/targets/firewall/tasks/test.yml @@ -69,6 +69,12 @@ that: - result is not changed +- name: Assign firewall to test_server + hetzner.hcloud.firewall_resource: + firewall: "{{ hcloud_firewall_name }}" + servers: ["{{ test_server.hcloud_server.name }}"] + state: present + - name: Test update hetzner.hcloud.firewall: name: "{{ hcloud_firewall_name }}" @@ -150,7 +156,7 @@ ansible.builtin.assert: that: - result is changed - - result.hcloud_firewall.name == "changed-{{ hcloud_firewall_name }}" + - result.hcloud_firewall.name == "changed-" + hcloud_firewall_name - name: Test update name and labels hetzner.hcloud.firewall: @@ -171,8 +177,21 @@ hetzner.hcloud.firewall: name: "{{ hcloud_firewall_name }}" state: absent + ignore_errors: true register: result - name: Verify delete + ansible.builtin.assert: + that: + - result is failed + - '"is still in use" in result.msg' + +- name: Test delete with force + hetzner.hcloud.firewall: + name: "{{ hcloud_firewall_name }}" + force: true + state: absent + register: result +- name: Verify delete with force ansible.builtin.assert: that: - result is changed