diff --git a/changelogs/fragments/71-hcloud_firewall_fix_idempotence.yml b/changelogs/fragments/71-hcloud_firewall_fix_idempotence.yml new file mode 100644 index 0000000..5a6f5f5 --- /dev/null +++ b/changelogs/fragments/71-hcloud_firewall_fix_idempotence.yml @@ -0,0 +1,3 @@ +bugfixes: +- hcloud_firewall - fix idempotence related to rules comparison (https://github.com/ansible-collections/hetzner.hcloud/pull/71). +- hcloud_server - fix idempotence related to firewall handling (https://github.com/ansible-collections/hetzner.hcloud/pull/71). diff --git a/plugins/modules/hcloud_firewall.py b/plugins/modules/hcloud_firewall.py index 6c2b475..32bb817 100644 --- a/plugins/modules/hcloud_firewall.py +++ b/plugins/modules/hcloud_firewall.py @@ -252,7 +252,7 @@ class AnsibleHcloudFirewall(Hcloud): self._mark_as_changed() rules = self.module.params.get("rules") - if rules is not None and self.hcloud_firewall.rules != rules: + if rules is not None and rules != [self._prepare_result_rule(rule) for rule in self.hcloud_firewall.rules]: if not self.module.check_mode: new_rules = [ FirewallRule( @@ -297,7 +297,7 @@ class AnsibleHcloudFirewall(Hcloud): protocol={"type": "str", "choices": ["icmp", "udp", "tcp"]}, port={"type": "str"}, source_ips={"type": "list", "elements": "str"}, - destination_ips={"type": "list", "elements": "str"}, + destination_ips={"type": "list", "elements": "str", "default": []}, ), required_together=[["direction", "protocol", "source_ips"]] ), diff --git a/plugins/modules/hcloud_server.py b/plugins/modules/hcloud_server.py index 25e8445..dca81dc 100644 --- a/plugins/modules/hcloud_server.py +++ b/plugins/modules/hcloud_server.py @@ -417,33 +417,34 @@ class AnsibleHcloudServer(Hcloud): self.hcloud_server.update(labels=labels) self._mark_as_changed() - firewalls = self.module.params.get("firewalls") - if firewalls is not None and firewalls != self.hcloud_server.public_net.firewalls: - if not self.module.check_mode: - for f in self.hcloud_server.public_net.firewalls: - found = False - for fname in firewalls: - if f.firewall.name == fname: - found = True - if not found: + wanted_firewalls = self.module.params.get("firewalls") + if wanted_firewalls is not None: + # Removing existing but not wanted firewalls + for current_firewall in self.hcloud_server.public_net.firewalls: + if current_firewall.firewall.name not in wanted_firewalls: + self._mark_as_changed() + if not self.module.check_mode: r = FirewallResource(type="server", server=self.hcloud_server) - actions = self.client.firewalls.remove_from_resources(f.firewall, [r]) + actions = self.client.firewalls.remove_from_resources(current_firewall.firewall, [r]) for a in actions: a.wait_until_finished() - for fname in firewalls: - found = False - fw = None - for f in self.hcloud_server.public_net.firewalls: - if f.firewall.name == fname: - found = True - fw = f - if not found and fw is not None: + # Adding wanted firewalls that doesn't exist yet + for fname in wanted_firewalls: + found = False + for f in self.hcloud_server.public_net.firewalls: + if f.firewall.name == fname: + found = True + break + + if not found: + self._mark_as_changed() + if not self.module.check_mode: + fw = self.client.firewalls.get_by_name(fname) r = FirewallResource(type="server", server=self.hcloud_server) actions = self.client.firewalls.apply_to_resources(fw, [r]) for a in actions: a.wait_until_finished() - self._mark_as_changed() server_type = self.module.params.get("server_type") if server_type is not None and self.hcloud_server.server_type.name != server_type: diff --git a/tests/integration/targets/hcloud_firewall/tasks/main.yml b/tests/integration/targets/hcloud_firewall/tasks/main.yml index 684ca0d..e59da36 100644 --- a/tests/integration/targets/hcloud_firewall/tasks/main.yml +++ b/tests/integration/targets/hcloud_firewall/tasks/main.yml @@ -1,6 +1,11 @@ # Copyright: (c) 2020, Hetzner Cloud GmbH # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) --- +- name: setup firewall to be absent + hcloud_firewall: + name: "{{ hcloud_firewall_name }}" + state: absent + - name: test missing required parameters on create firewall hcloud_firewall: register: result @@ -46,6 +51,12 @@ - name: test create firewall idempotence hcloud_firewall: name: "{{ hcloud_firewall_name }}" + rules: + - direction: in + protocol: icmp + source_ips: + - 0.0.0.0/0 + - ::/0 labels: key: value my-label: label @@ -88,6 +99,18 @@ - name: test update firewall rules idempotence hcloud_firewall: name: "{{ hcloud_firewall_name }}" + rules: + - direction: in + protocol: icmp + source_ips: + - 0.0.0.0/0 + - ::/0 + - direction: in + protocol: tcp + port: 80 + source_ips: + - 0.0.0.0/0 + - ::/0 labels: key: value my-label: label @@ -97,7 +120,6 @@ that: - result is not changed - - name: test update firewall with check mode hcloud_firewall: id: "{{ firewall.hcloud_firewall.id }}" diff --git a/tests/integration/targets/hcloud_server/tasks/main.yml b/tests/integration/targets/hcloud_server/tasks/main.yml index 18132f1..4a9cb43 100644 --- a/tests/integration/targets/hcloud_server/tasks/main.yml +++ b/tests/integration/targets/hcloud_server/tasks/main.yml @@ -609,7 +609,6 @@ - result_after_test.hcloud_server.delete_protection is sameas true - result_after_test.hcloud_server.rebuild_protection is sameas true - - name: test delete server fails if it is protected hcloud_server: name: "{{hcloud_server_name}}" @@ -647,35 +646,18 @@ that: - result is success -- name: test create firewall +- name: setup create firewalls hcloud_firewall: - name: "{{ hcloud_firewall_name }}" + name: "{{ item }}" rules: - direction: in protocol: icmp source_ips: - 0.0.0.0/0 - ::/0 - register: firewall -- name: verify create firewall - assert: - that: - - firewall is changed - -- name: test create firewall - hcloud_firewall: - name: "{{ hcloud_firewall_name }}2" - rules: - - direction: in - protocol: icmp - source_ips: - - 0.0.0.0/0 - - ::/0 - register: firewall2 -- name: verify create firewall - assert: - that: - - firewall2 is changed + with_items: + - "{{ hcloud_firewall_name }}" + - "{{ hcloud_firewall_name }}2" - name: test create server with firewalls hcloud_server: @@ -688,11 +670,27 @@ - ci@ansible.hetzner.cloud state: present register: result -- name: verify enable backups +- name: verify test create server with firewalls assert: that: - result is changed +- name: test create server with firewalls idempotence + hcloud_server: + name: "{{ hcloud_server_name }}" + server_type: cpx11 + firewalls: + - "{{ hcloud_firewall_name }}" + image: "ubuntu-20.04" + ssh_keys: + - ci@ansible.hetzner.cloud + state: present + register: result +- name: verify test create server with firewalls idempotence + assert: + that: + - result is not changed + - name: test update server with firewalls hcloud_server: name: "{{ hcloud_server_name }}" @@ -704,27 +702,36 @@ - ci@ansible.hetzner.cloud state: present register: result -- name: verify enable backups +- name: verify test update server with firewalls assert: that: - result is changed -- name: cleanup test create firewall - hcloud_firewall: - name: "{{ hcloud_firewall_name }}" - state: absent +- name: test update server with firewalls idempotence + hcloud_server: + name: "{{ hcloud_server_name }}" + server_type: cpx11 + firewalls: + - "{{ hcloud_firewall_name }}2" + image: "ubuntu-20.04" + ssh_keys: + - ci@ansible.hetzner.cloud + state: present register: result -- name: verify create firewall +- name: verify test update server with firewalls idempotence assert: that: - - result is changed + - result is not changed -- name: cleanup test create server with firewalls +- name: cleanup server with firewalls hcloud_server: name: "{{ hcloud_server_name }}" state: absent - register: result -- name: verify cleanup - assert: - that: - - result is success + +- name: cleanup test create firewall + hcloud_firewall: + name: "{{ item }}" + state: absent + with_items: + - "{{ hcloud_firewall_name }}" + - "{{ hcloud_firewall_name }}2"