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

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.

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

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 = camel_dict_to_snake_dict(security_group, ignore_list=['Tags'])
-        security_group['tags'] = boto3_tag_list_to_ansible_dict(security_group.get('tags', []),
+        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)
     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__':

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'

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)