From 7e32f1ffb04a2829603c47651c900729bf5b072f Mon Sep 17 00:00:00 2001
From: Jordan Borean <jborean93@gmail.com>
Date: Tue, 27 Nov 2018 06:48:19 +1000
Subject: [PATCH] [ec2_group] fix comparison of determining which rules to
 purge - 2.7 (#48967)

* Added changelog fragment

* 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>
(cherry picked from commit 54a2f21f93c54c4a10e378e500efcc52999d6408)
---
 .../fragments/ec2_group-rule-purge-fix.yaml   |  2 ++
 lib/ansible/modules/cloud/amazon/ec2_group.py | 12 ++++++----
 .../targets/ec2_group/tasks/main.yml          | 24 +++++++++++++++++++
 3 files changed, 34 insertions(+), 4 deletions(-)
 create mode 100644 changelogs/fragments/ec2_group-rule-purge-fix.yaml

diff --git a/changelogs/fragments/ec2_group-rule-purge-fix.yaml b/changelogs/fragments/ec2_group-rule-purge-fix.yaml
new file mode 100644
index 0000000000..8167b82193
--- /dev/null
+++ b/changelogs/fragments/ec2_group-rule-purge-fix.yaml
@@ -0,0 +1,2 @@
+bugfixes:
+- ec2_group - Fix comparison of determining which rules to purge by ignoring descriptions - https://github.com/ansible/ansible/issues/47904
diff --git a/lib/ansible/modules/cloud/amazon/ec2_group.py b/lib/ansible/modules/cloud/amazon/ec2_group.py
index b266972b30..2bd0bf65b9 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 d34e0a4d0f..2eb9768f1e 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