Fix bug in updating group.spec.inlinePolicies (#84)

The bug occurs when updating an existing item in `spec.inlinePolicies`.
The code incorrectly includes both the old(to remove) and new(to
add)values when using `lo.Difference`. This causes the code to first
attempt to replace the xisting policies with a call to
`iam::PutGroupPolicy` and then mistakenly "delete" them right after.

This patch fixes this bug by calling `lo.Find` to double check whether
we need to keep an inline policy (if it only needs to be updated)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Author: a-hilaly

pkg/resource/group/hooks.go

@@ -177,6 +177,11 @@ func (rm *resourceManager) syncInlinePolicies(
 		}
 	}
 	for _, pair := range toDelete {
+		// do not remove elements we just updated with `addInlinePolicy`
+		if _, ok := lo.Find(toAdd, func(entry lo.Entry[string, string]) bool { return entry.Key == pair.Key }); ok {
+			continue
+		}
+
 		polName := pair.Key
 		rlog.Debug(
 			"removing inline policy from group",

test/e2e/requirements.txt

@@ -1 +1 @@
-acktest @ git+https://github.com/aws-controllers-k8s/test-infra.git@75752b2
+acktest @ git+https://github.com/aws-controllers-k8s/test-infra.git@371852014efcb8c26c454f861eb546c93a48f205

test/e2e/tests/test_group.py

@@ -109,11 +109,20 @@ def test_crud(self, simple_group):
 "Action": ["ec2:Get*"],
 "Resource": ["*"]
 }]
+}'''
+        inline_doc_2 = '''{
+"Version": "2012-10-17",
+"Statement": [{
+"Effect": "Allow",
+"Action": ["s3:Get*"],
+"Resource": ["*"]
+}]
 }'''
         updates = {
             "spec": {
                 "inlinePolicies": {
                     "ec2get": inline_doc,
+                    "s3get": inline_doc_2,
                 },
             },
         }
@@ -122,21 +131,58 @@ def test_crud(self, simple_group):
 
         expect_inline_policies = {
             'ec2get': inline_doc,
+            's3get': inline_doc_2,
         }
         cr = k8s.get_resource(ref)
         assert cr is not None
         assert 'spec' in cr
         assert 'inlinePolicies' in cr['spec']
-        assert len(cr['spec']['inlinePolicies']) == 1
+        assert len(cr['spec']['inlinePolicies']) == 2
         assert expect_inline_policies == cr['spec']['inlinePolicies']
 
         latest_inline_policies = group.get_inline_policies(group_name)
-        assert len(latest_inline_policies) == 1
+        assert len(latest_inline_policies) == 2
         assert 'ec2get' in latest_inline_policies
+
         got_pol_doc = latest_inline_policies['ec2get']
         nospace_got_doc = "".join(c for c in got_pol_doc if not c.isspace())
         nospace_exp_doc = "".join(c for c in inline_doc if not c.isspace())
+        assert nospace_exp_doc == nospace_got_doc
 
+        got_pol_doc = latest_inline_policies['s3get']
+        nospace_got_doc = "".join(c for c in got_pol_doc if not c.isspace())
+        nospace_exp_doc = "".join(c for c in inline_doc_2 if not c.isspace())
+        assert nospace_exp_doc == nospace_got_doc
+
+        inline_doc_s3_get_object = '''{
+"Version": "2012-10-17",
+"Statement": [{
+"Effect": "Allow",
+"Action": ["s3:GetObject"],
+"Resource": ["*"]
+}]
+}'''
+        # update s3get policy document
+        updates = {
+            "spec": {
+                "inlinePolicies": {
+                    "ec2get": inline_doc,
+                    "s3get": inline_doc_s3_get_object,
+                },
+            },
+        }
+        k8s.patch_custom_resource(ref, updates)
+        time.sleep(MODIFY_WAIT_AFTER_SECONDS)
+
+        latest_inline_policies = group.get_inline_policies(group_name)
+        assert len(latest_inline_policies) == 2
+        assert 's3get' in latest_inline_policies
+        assert 'ec2get' in latest_inline_policies
+
+        # expect s3get policy document to change into inlinde_doc_s3_get_object
+        got_pol_doc = latest_inline_policies['s3get']
+        nospace_got_doc = "".join(c for c in got_pol_doc if not c.isspace())
+        nospace_exp_doc = "".join(c for c in inline_doc_s3_get_object if not c.isspace())
         assert nospace_exp_doc == nospace_got_doc
 
         # Remove the inline policy we just added and check the updates are