From 0f1c083d25a309e346ce9a85602d7cb23af3ee5f Mon Sep 17 00:00:00 2001 From: Tom Melendez Date: Mon, 11 Jul 2016 16:13:13 +0000 Subject: [PATCH 1/2] Allow GCE firewall rules to be updated when attributes changes. Fixes #2111. Previously, when the attributes of a GCE firewall change, they were ignored. This PR changes that behavior and now updates them. Note that the "update" also removes attributes that are not specified. An overview of the firewall rule behavior is as follows: 1. firewall name in GCP, state=absent in PLAYBOOK: Delete from GCP 2. firewall name in PLAYBOOK, not in GCP: Add to GCP. 3. firewall name in GCP, name not in PLAYBOOK: No change. 4. firewall names exist in both GCP and PLAYBOOK, attributes differ: Update GCP to match attributes from PLAYBOOK. --- cloud/google/gce_net.py | 51 +++++++++++++++++++++++++++++++++-------- 1 file changed, 42 insertions(+), 9 deletions(-) diff --git a/cloud/google/gce_net.py b/cloud/google/gce_net.py index e222c385d84..a97f9d3210e 100644 --- a/cloud/google/gce_net.py +++ b/cloud/google/gce_net.py @@ -34,6 +34,7 @@ options: allowed: description: - the protocol:ports to allow ('tcp:80' or 'tcp:80,443' or 'tcp:80-800;udp:1-25') + this parameter is mandatory when creating or updating a firewall rule required: false default: null aliases: [] @@ -246,12 +247,48 @@ def main(): allowed_list = format_allowed(allowed) + # Fetch existing rule and if it exists, compare attributes + # update if attributes changed. Create if doesn't exist. try: - gce.ex_create_firewall(fwname, allowed_list, network=name, + fw_changed = False + fw = gce.ex_get_firewall(fwname) + + # If old and new attributes are different, we update the firewall rule. + # This implicitly let's us clear out attributes as well. + # allowed_list is required and must not be None for firewall rules. + if allowed_list and (allowed_list != fw.allowed): + fw.allowed = allowed_list + fw_changed = True + + if src_range != fw.source_ranges: + fw.source_ranges = src_range + fw_changed = True + + if src_tags != fw.source_tags: + fw.source_tags = src_tags + fw_changed = True + + if src_tags != fw.target_tags: + fw.target_tags = target_tags + fw_changed = True + + if fw_changed is True: + try: + gce.ex_update_firewall(fw) + changed = True + except Exception as e: + module.fail_json(msg=unexpected_error_msg(e), changed=False) + + # Firewall rule not found so we try to create it. + except ResourceNotFoundError: + try: + gce.ex_create_firewall(fwname, allowed_list, network=name, source_ranges=src_range, source_tags=src_tags, target_tags=target_tags) - changed = True - except ResourceExistsError: - pass + changed = True + + except Exception as e: + module.fail_json(msg=unexpected_error_msg(e), changed=False) + except Exception as e: module.fail_json(msg=unexpected_error_msg(e), changed=False) @@ -279,17 +316,13 @@ def main(): network = None try: network = gce.ex_get_network(name) -# json_output['d1'] = 'found network name %s' % name + except ResourceNotFoundError: -# json_output['d2'] = 'not found network name %s' % name pass except Exception as e: -# json_output['d3'] = 'error with %s' % name module.fail_json(msg=unexpected_error_msg(e), changed=False) if network: -# json_output['d4'] = 'deleting %s' % name gce.ex_destroy_network(network) -# json_output['d5'] = 'deleted %s' % name changed = True json_output['changed'] = changed From 7c8d972d8fe80890d4189751f0ddc97677ecc878 Mon Sep 17 00:00:00 2001 From: Tom Melendez Date: Wed, 13 Jul 2016 19:56:43 +0000 Subject: [PATCH 2/2] Added helper function and logic to sort attributes before comparing. --- cloud/google/gce_net.py | 49 +++++++++++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 12 deletions(-) diff --git a/cloud/google/gce_net.py b/cloud/google/gce_net.py index a97f9d3210e..5a1852ac9ec 100644 --- a/cloud/google/gce_net.py +++ b/cloud/google/gce_net.py @@ -174,6 +174,14 @@ def format_allowed(allowed): return_value.append(format_allowed_section(section)) return return_value +def sorted_allowed_list(allowed_list): + """Sort allowed_list (output of format_allowed) by protocol and port.""" + # sort by protocol + allowed_by_protocol = sorted(allowed_list,key=lambda x: x['IPProtocol']) + # sort the ports list + return sorted(allowed_by_protocol, key=lambda y: y['ports'].sort()) + + def main(): module = AnsibleModule( argument_spec = dict( @@ -256,21 +264,38 @@ def main(): # If old and new attributes are different, we update the firewall rule. # This implicitly let's us clear out attributes as well. # allowed_list is required and must not be None for firewall rules. - if allowed_list and (allowed_list != fw.allowed): + if allowed_list and (sorted_allowed_list(allowed_list) != sorted_allowed_list(fw.allowed)): fw.allowed = allowed_list fw_changed = True - if src_range != fw.source_ranges: - fw.source_ranges = src_range - fw_changed = True - - if src_tags != fw.source_tags: - fw.source_tags = src_tags - fw_changed = True - - if src_tags != fw.target_tags: - fw.target_tags = target_tags - fw_changed = True + # If these attributes are lists, we sort them first, then compare. + # Otherwise, we update if they differ. + if fw.source_ranges != src_range: + if isinstance(src_range, list): + if sorted(fw.source_ranges) != sorted(src_range): + fw.source_ranges = src_range + fw_changed = True + else: + fw.source_ranges = src_range + fw_changed = True + + if fw.source_tags != src_tags: + if isinstance(src_range, list): + if sorted(fw.source_tags) != sorted(src_tags): + fw.source_tags = src_tags + fw_changed = True + else: + fw.source_tags = src_tags + fw_changed = True + + if fw.target_tags != target_tags: + if isinstance(target_tags, list): + if sorted(fw.target_tags) != sorted(target_tags): + fw.target_tags = target_tags + fw_changed = True + else: + fw.target_tags = target_tags + fw_changed = True if fw_changed is True: try: