From 54a2f21f93c54c4a10e378e500efcc52999d6408 Mon Sep 17 00:00:00 2001
From: Sloane Hertel <shertel@redhat.com>
Date: Tue, 20 Nov 2018 17:04:10 -0500
Subject: [PATCH] Fix comparison of determining which rules to purge by
 ignoring descriptions (#48443)

AWS uses rule type, protocol, port range, and source as an idempotent identifier.
There can only be one rule with that unique combination. Rules that differ only by description are allowed but overwritten by AWS.
Add a test

Co-authored-by: Will Thames <will@thames.id.au>
---
 lib/ansible/modules/cloud/amazon/ec2_group.py | 12 ++++++----
 .../targets/ec2_group/tasks/main.yml          | 24 +++++++++++++++++++
 2 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/lib/ansible/modules/cloud/amazon/ec2_group.py b/lib/ansible/modules/cloud/amazon/ec2_group.py
index b266972b307..2bd0bf65b99 100644
--- a/lib/ansible/modules/cloud/amazon/ec2_group.py
+++ b/lib/ansible/modules/cloud/amazon/ec2_group.py
@@ -1107,8 +1107,6 @@ def main():
         # List comprehensions for rules to add, rules to modify, and rule ids to determine purging
         new_ingress_permissions = [to_permission(r) for r in (set(named_tuple_ingress_list) - set(current_ingress))]
         new_egress_permissions = [to_permission(r) for r in (set(named_tuple_egress_list) - set(current_egress))]
-        present_ingress = list(set(named_tuple_ingress_list).union(set(current_ingress)))
-        present_egress = list(set(named_tuple_egress_list).union(set(current_egress)))
 
         if module.params.get('rules_egress') is None and 'VpcId' in group:
             # when no egress rules are specified and we're in a VPC,
@@ -1125,7 +1123,10 @@ def main():
         present_egress = list(set(named_tuple_egress_list).union(set(current_egress)))
 
         if purge_rules:
-            revoke_ingress = [to_permission(r) for r in set(present_ingress) - set(named_tuple_ingress_list)]
+            revoke_ingress = []
+            for p in present_ingress:
+                if not any([rule_cmp(p, b) for b in named_tuple_ingress_list]):
+                    revoke_ingress.append(to_permission(p))
         else:
             revoke_ingress = []
         if purge_rules_egress and module.params.get('rules_egress') is not None:
@@ -1135,7 +1136,10 @@ def main():
                     if r != Rule((None, None), '-1', '0.0.0.0/0', 'ipv4', None)
                 ]
             else:
-                revoke_egress = [to_permission(r) for r in set(present_egress) - set(named_tuple_egress_list)]
+                revoke_egress = []
+                for p in present_egress:
+                    if not any([rule_cmp(p, b) for b in named_tuple_egress_list]):
+                        revoke_egress.append(to_permission(p))
         else:
             revoke_egress = []
 
diff --git a/test/integration/targets/ec2_group/tasks/main.yml b/test/integration/targets/ec2_group/tasks/main.yml
index d34e0a4d0fd..2eb9768f1ef 100644
--- a/test/integration/targets/ec2_group/tasks/main.yml
+++ b/test/integration/targets/ec2_group/tasks/main.yml
@@ -1114,6 +1114,29 @@
           - 'result.changed'
       when: result.ip_permissions_egress[0].ip_ranges[0].description is undefined
 
+    # =========================================================================================
+    - name: add rules without descriptions ready for adding descriptions to existing rules
+      ec2_group:
+        name: '{{ec2_group_name}}'
+        description: '{{ec2_group_description}}'
+        <<: *aws_connection_info
+        vpc_id: '{{ vpc_result.vpc.id }}'
+        # purge the other rules so assertions work for the subsequent tests for rule descriptions
+        purge_rules_egress: true
+        purge_rules: true
+        state: present
+        rules:
+        - proto: "tcp"
+          ports:
+          - 8281
+          cidr_ipv6: 1001:d00::/24
+        rules_egress:
+        - proto: "tcp"
+          ports:
+          - 8282
+          cidr_ip: 2.2.2.2/32
+      register: result
+
     # ============================================================
     - name: test adding a rule and egress rule descriptions (expected changed=true)
       ec2_group:
@@ -1187,6 +1210,7 @@
       # compatibility with this feature.
       assert:
         that:
+          - 'result.ip_permissions | length > 0'
           - 'result.changed'
       when: result.ip_permissions_egress[0].ip_ranges[0].description is defined