From 79ecb4c41f8bca158d999344c0e331684091fce6 Mon Sep 17 00:00:00 2001 From: Sloane Hertel Date: Thu, 23 Aug 2018 19:43:18 -0400 Subject: [PATCH] Add diff mode for ec2_group (#44533) * Add (preview) diff mode support ec2_group * Add diff mode to some ec2_group integration tests * Remove unnecessary arguments and add comment to the module notes * Add changelog --- .../fragments/ec2_group_diff_mode_support.yml | 4 + lib/ansible/module_utils/aws/core.py | 1 + lib/ansible/modules/cloud/amazon/ec2_group.py | 112 +++++++++++++++++- .../targets/ec2_group/tasks/egress_tests.yml | 23 ++++ .../targets/ec2_group/tasks/main.yml | 14 +++ 5 files changed, 149 insertions(+), 5 deletions(-) create mode 100644 changelogs/fragments/ec2_group_diff_mode_support.yml diff --git a/changelogs/fragments/ec2_group_diff_mode_support.yml b/changelogs/fragments/ec2_group_diff_mode_support.yml new file mode 100644 index 0000000000..aa87deb68c --- /dev/null +++ b/changelogs/fragments/ec2_group_diff_mode_support.yml @@ -0,0 +1,4 @@ +--- +minor_changes: + - ec2_group - Add diff mode support with and without check mode. This feature + is preview and may change when a common framework is adopted for AWS modules. diff --git a/lib/ansible/module_utils/aws/core.py b/lib/ansible/module_utils/aws/core.py index aa54018df8..759a03e5ac 100644 --- a/lib/ansible/module_utils/aws/core.py +++ b/lib/ansible/module_utils/aws/core.py @@ -117,6 +117,7 @@ class AnsibleAWSModule(object): msg='Python modules "botocore" or "boto3" are missing, please install both') self.check_mode = self._module.check_mode + self._diff = self._module._diff self._name = self._module._name @property diff --git a/lib/ansible/modules/cloud/amazon/ec2_group.py b/lib/ansible/modules/cloud/amazon/ec2_group.py index 7e007582a3..a06b3c9f38 100644 --- a/lib/ansible/modules/cloud/amazon/ec2_group.py +++ b/lib/ansible/modules/cloud/amazon/ec2_group.py @@ -110,6 +110,7 @@ notes: - If a rule declares a group_name and that group doesn't exist, it will be automatically created. In that case, group_desc should be provided as well. The module will refuse to create a depended-on group without a description. + - Preview diff mode support is added in version 2.7. ''' EXAMPLES = ''' @@ -222,6 +223,7 @@ EXAMPLES = ''' - 64:ff9b::/96 group_id: - sg-edcd9784 + diff: True - name: "Delete group by its id" ec2_group: @@ -847,6 +849,92 @@ def verify_rules_with_descriptions_permitted(client, module, rules, rules_egress module.fail_json(msg="Using rule descriptions requires botocore version >= 1.7.2.") +def get_diff_final_resource(client, module, security_group): + def get_account_id(security_group, module): + try: + owner_id = security_group.get('owner_id', module.client('sts').get_caller_identity()['Account']) + except (BotoCoreError, ClientError) as e: + owner_id = "Unable to determine owner_id: {0}".format(to_text(e)) + return owner_id + + def get_final_tags(security_group_tags, specified_tags, purge_tags): + if specified_tags is None: + return security_group_tags + tags_need_modify, tags_to_delete = compare_aws_tags(security_group_tags, specified_tags, purge_tags) + end_result_tags = dict((k, v) for k, v in specified_tags.items() if k not in tags_to_delete) + end_result_tags.update(dict((k, v) for k, v in security_group_tags.items() if k not in tags_to_delete)) + end_result_tags.update(tags_need_modify) + return end_result_tags + + def get_final_rules(client, module, security_group_rules, specified_rules, purge_rules): + if specified_rules is None: + return security_group_rules + if purge_rules: + final_rules = [] + else: + final_rules = list(security_group_rules) + for rule in specified_rules: + format_rule = { + 'from_port': None, 'to_port': None, 'ip_protocol': rule.get('proto', 'tcp'), + 'ip_ranges': [], 'ipv6_ranges': [], 'prefix_list_ids': [], 'user_id_group_pairs': [] + } + if rule.get('proto', 'tcp') in ('all', '-1', -1): + format_rule['ip_protocol'] = '-1' + format_rule.pop('from_port') + format_rule.pop('to_port') + elif rule.get('ports'): + if rule.get('ports') and isinstance(rule.get('ports'), string_types): + rule['ports'] = [rule['ports']] + for port in rule.get('ports'): + if isinstance(port, string_types) and '-' in port: + format_rule['from_port'], format_rule['to_port'] = port.split('-') + else: + format_rule['from_port'] = format_rule['to_port'] = port + elif rule.get('from_port') or rule.get('to_port'): + format_rule['from_port'] = rule.get('from_port', rule.get('to_port')) + format_rule['to_port'] = rule.get('to_port', rule.get('from_port')) + for source_type in ('cidr_ip', 'cidr_ipv6', 'prefix_list_id'): + if rule.get(source_type): + rule_key = {'cidr_ip': 'ip_ranges', 'cidr_ipv6': 'ipv6_ranges', 'prefix_list_id': 'prefix_list_ids'}.get(source_type) + if rule.get('rule_desc'): + format_rule[rule_key] = [{source_type: rule[source_type], 'description': rule['rule_desc']}] + else: + format_rule[rule_key] = [{source_type: rule[source_type]}] + if rule.get('group_id') or rule.get('group_name'): + rule_sg = camel_dict_to_snake_dict(group_exists(client, module, module.params['vpc_id'], rule.get('group_id'), rule.get('group_name'))[0]) + format_rule['user_id_group_pairs'] = [{ + 'description': rule_sg.get('description', rule_sg.get('group_desc')), + 'group_id': rule_sg.get('group_id', rule.get('group_id')), + 'group_name': rule_sg.get('group_name', rule.get('group_name')), + 'peering_status': rule_sg.get('peering_status'), + 'user_id': rule_sg.get('user_id', get_account_id(security_group, module)), + 'vpc_id': rule_sg.get('vpc_id', module.params['vpc_id']), + 'vpc_peering_connection_id': rule_sg.get('vpc_peering_connection_id') + }] + for k, v in format_rule['user_id_group_pairs'][0].items(): + if v is None: + format_rule['user_id_group_pairs'][0].pop(k) + final_rules.append(format_rule) + # Order final rules consistently + final_rules.sort(key=lambda x: x.get('cidr_ip', x.get('ip_ranges', x.get('ipv6_ranges', x.get('prefix_list_ids', x.get('user_id_group_pairs')))))) + return final_rules + security_group_ingress = security_group.get('ip_permissions', []) + specified_ingress = module.params['rules'] + purge_ingress = module.params['purge_rules'] + security_group_egress = security_group.get('ip_permissions_egress', []) + specified_egress = module.params['rules_egress'] + purge_egress = module.params['purge_rules_egress'] + return { + 'description': module.params['description'], + 'group_id': security_group.get('group_id', 'sg-xxxxxxxx'), + 'group_name': security_group.get('group_name', module.params['name']), + 'ip_permissions': get_final_rules(client, module, security_group_ingress, specified_ingress, purge_ingress), + 'ip_permissions_egress': get_final_rules(client, module, security_group_egress, specified_egress, purge_egress), + 'owner_id': get_account_id(security_group, module), + 'tags': get_final_tags(security_group.get('tags', {}), module.params['tags'], module.params['purge_tags']), + 'vpc_id': security_group.get('vpc_id', module.params['vpc_id'])} + + def main(): argument_spec = dict( name=dict(), @@ -893,10 +981,15 @@ def main(): global current_account_id current_account_id = get_aws_account_id(module) + before = {} + after = {} + # Ensure requested group is absent if state == 'absent': if group: # found a match, delete it + before = camel_dict_to_snake_dict(group, ignore_list=['Tags']) + before['tags'] = boto3_tag_list_to_ansible_dict(before.get('tags', [])) try: if not module.check_mode: client.delete_security_group(GroupId=group['GroupId']) @@ -913,6 +1006,8 @@ def main(): elif state == 'present': if group: # existing group + before = camel_dict_to_snake_dict(group, ignore_list=['Tags']) + before['tags'] = boto3_tag_list_to_ansible_dict(before.get('tags', [])) if group['Description'] != description: module.warn("Group description does not match existing group. Descriptions cannot be changed without deleting " "and re-creating the security group. Try using state=absent to delete, then rerunning this task.") @@ -1009,12 +1104,19 @@ def main(): security_group = wait_for_rule_propagation(module, group, named_tuple_ingress_list, named_tuple_egress_list, purge_rules, purge_rules_egress) else: security_group = get_security_groups_with_backoff(client, GroupIds=[group['GroupId']])['SecurityGroups'][0] - security_group = camel_dict_to_snake_dict(security_group) - security_group['tags'] = boto3_tag_list_to_ansible_dict(security_group.get('tags', []), - tag_name_key_name='key', tag_value_key_name='value') - module.exit_json(changed=changed, **security_group) + security_group = camel_dict_to_snake_dict(security_group, ignore_list=['Tags']) + security_group['tags'] = boto3_tag_list_to_ansible_dict(security_group.get('tags', [])) + else: - module.exit_json(changed=changed, group_id=None) + security_group = {'group_id': None} + + if module._diff: + if module.params['state'] == 'present': + after = get_diff_final_resource(client, module, security_group) + + security_group['diff'] = [{'before': before, 'after': after}] + + module.exit_json(changed=changed, **security_group) if __name__ == '__main__': diff --git a/test/integration/targets/ec2_group/tasks/egress_tests.yml b/test/integration/targets/ec2_group/tasks/egress_tests.yml index 5a7c986086..aafb16ec80 100644 --- a/test/integration/targets/ec2_group/tasks/egress_tests.yml +++ b/test/integration/targets/ec2_group/tasks/egress_tests.yml @@ -121,6 +121,29 @@ that: - result.ip_permissions_egress|length == 2 + - name: Purge the second rule (CHECK MODE) (DIFF MODE) + ec2_group: + name: '{{ec2_group_name}}-egress-tests' + description: '{{ec2_group_description}}' + vpc_id: '{{ vpc_result.vpc.id }}' + rules_egress: + - proto: tcp + ports: + - 1212 + cidr_ip: 1.2.1.2/32 + <<: *aws_connection_info + state: present + register: result + check_mode: True + diff: True + + - name: assert first rule will be left + assert: + that: + - result.changed + - result.diff.0.after.ip_permissions_egress|length == 1 + - result.diff.0.after.ip_permissions_egress[0].ip_ranges[0].cidr_ip == '1.2.1.2/32' + - name: Purge the second rule ec2_group: name: '{{ec2_group_name}}-egress-tests' diff --git a/test/integration/targets/ec2_group/tasks/main.yml b/test/integration/targets/ec2_group/tasks/main.yml index 2a4264b699..069dc0b7aa 100644 --- a/test/integration/targets/ec2_group/tasks/main.yml +++ b/test/integration/targets/ec2_group/tasks/main.yml @@ -288,12 +288,15 @@ cidr_ipv6: "64:ff9b::/96" <<: *aws_connection_info check_mode: true + diff: true register: result - name: assert state=present (expected changed=true) assert: that: - 'result.changed' + - 'result.diff.0.before.ip_permissions == result.diff.0.after.ip_permissions' + - 'result.diff.0.before.ip_permissions_egress != result.diff.0.after.ip_permissions_egress' # ============================================================ - name: test rules_egress state=present for ipv6 (expected changed=true) @@ -330,12 +333,14 @@ vpc_id: '{{ vpc_result.vpc.id }}' <<: *aws_connection_info check_mode: true + diff: true register: result - name: assert group was removed assert: that: - 'result.changed' + - 'not result.diff.0.after' # ============================================================ - name: test state=absent (expected changed=true) @@ -405,8 +410,13 @@ to_port: 8182 cidr_ip: "1.1.1.1/32" check_mode: true + diff: true register: check_result + - assert: + that: + - not check_result.changed + - check_result.diff.0.before.ip_permissions.0 == check_result.diff.0.after.ip_permissions.0 # ============================================================ - name: add same rule to the existing group (expected changed=false) @@ -877,12 +887,16 @@ tag1: test1 tag2: test2 check_mode: true + diff: true register: result - name: assert that tags were added (expected changed=true) assert: that: - 'result.changed' + - 'not result.diff.0.before.tags' + - 'result.diff.0.after.tags.tag1 == "test1"' + - 'result.diff.0.after.tags.tag2 == "test2"' # ============================================================ - name: test adding tags (expected changed=true)