[PR #8695/2942eda8 backport][stable-9] keycloak_user_federation: add mapper removal (#8753)

keycloak_user_federation: add mapper removal (#8695)

* add unwanted mapper removal

* check for mapper updates in already fetched data to remove unnecessary API calls

* added mock answers and updated request count to match the added delete and fetch after_comp calls

* fix sanity issues

* add changelog fragment

* removed automatic field numbering from format

* replace filter expression with list comprehension

Co-authored-by: Felix Fontein <felix@fontein.de>

* add module name, link to issue and link to PR to changelog

Co-authored-by: Felix Fontein <felix@fontein.de>

* Use list comprehension.

---------

Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit 2942eda8e0)

Co-authored-by: fgruenbauer <gruenbauer@b1-systems.de>

changelogs/fragments/8695-keycloak_user_federation-mapper-removal.yml

@@ -0,0 +1,2 @@
+bugfixes:
+  - keycloak_user_federation - remove existing user federation mappers if they are not present in the federation configuration and will not be updated (https://github.com/ansible-collections/community.general/issues/7169, https://github.com/ansible-collections/community.general/pull/8695).

plugins/modules/keycloak_user_federation.py

@@ -892,11 +892,11 @@ def main():
             if cid is None:
                 old_mapper = {}
             elif change.get('id') is not None:
-                old_mapper = kc.get_component(change['id'], realm)
+                old_mapper = next((before_mapper for before_mapper in before_mapper.get('mappers', []) if before_mapper["id"] == change['id']), None)
                 if old_mapper is None:
                     old_mapper = {}
             else:
-                found = kc.get_components(urlencode(dict(parent=cid, name=change['name'])), realm)
+                found = [before_mapper for before_mapper in before_comp.get('mappers', []) if before_mapper['name'] == change['name']]
                 if len(found) > 1:
                     module.fail_json(msg='Found multiple mappers with name `{name}`. Cannot continue.'.format(name=change['name']))
                 if len(found) == 1:
@@ -905,7 +905,7 @@ def main():
                     old_mapper = {}
             new_mapper = old_mapper.copy()
             new_mapper.update(change)
-            if new_mapper != old_mapper:
+            # changeset contains all desired mappers: those existing, to update or to create
             if changeset.get('mappers') is None:
                 changeset['mappers'] = list()
             changeset['mappers'].append(new_mapper)
@@ -931,42 +931,51 @@ def main():
         # Process a creation
         result['changed'] = True
 
+        if module.check_mode:
             if module._diff:
                 result['diff'] = dict(before='', after=sanitize(desired_comp))
-
-        if module.check_mode:
             module.exit_json(**result)
 
         # create it
-        desired_comp = desired_comp.copy()
-        updated_mappers = desired_comp.pop('mappers', [])
+        desired_mappers = desired_comp.pop('mappers', [])
         after_comp = kc.create_component(desired_comp, realm)
-
         cid = after_comp['id']
+        updated_mappers = []
+        # when creating a user federation, keycloak automatically creates default mappers
+        default_mappers = kc.get_components(urlencode(dict(parent=cid)), realm)
 
-        for mapper in updated_mappers:
-            found = kc.get_components(urlencode(dict(parent=cid, name=mapper['name'])), realm)
+        # create new mappers or update existing default mappers
+        for desired_mapper in desired_mappers:
+            found = [default_mapper for default_mapper in default_mappers if default_mapper['name'] == desired_mapper['name']]
             if len(found) > 1:
-                module.fail_json(msg='Found multiple mappers with name `{name}`. Cannot continue.'.format(name=mapper['name']))
+                module.fail_json(msg='Found multiple mappers with name `{name}`. Cannot continue.'.format(name=desired_mapper['name']))
             if len(found) == 1:
                 old_mapper = found[0]
             else:
                 old_mapper = {}
 
             new_mapper = old_mapper.copy()
-            new_mapper.update(mapper)
+            new_mapper.update(desired_mapper)
 
             if new_mapper.get('id') is not None:
                 kc.update_component(new_mapper, realm)
+                updated_mappers.append(new_mapper)
             else:
                 if new_mapper.get('parentId') is None:
-                    new_mapper['parentId'] = after_comp['id']
-                mapper = kc.create_component(new_mapper, realm)
+                    new_mapper['parentId'] = cid
+                updated_mappers.append(kc.create_component(new_mapper, realm))
 
-        after_comp['mappers'] = updated_mappers
+        # we remove all unwanted default mappers
+        # we use ids so we dont accidently remove one of the previously updated default mapper
+        for default_mapper in default_mappers:
+            if not default_mapper['id'] in [x['id'] for x in updated_mappers]:
+                kc.delete_component(default_mapper['id'], realm)
+
+        after_comp['mappers'] = kc.get_components(urlencode(dict(parent=cid)), realm)
+        if module._diff:
+            result['diff'] = dict(before='', after=sanitize(after_comp))
         result['end_state'] = sanitize(after_comp)
-
-        result['msg'] = "User federation {id} has been created".format(id=after_comp['id'])
+        result['msg'] = "User federation {id} has been created".format(id=cid)
         module.exit_json(**result)
 
     else:
@@ -990,22 +999,32 @@ def main():
                 module.exit_json(**result)
 
             # do the update
-            desired_comp = desired_comp.copy()
-            updated_mappers = desired_comp.pop('mappers', [])
+            desired_mappers = desired_comp.pop('mappers', [])
             kc.update_component(desired_comp, realm)
-            after_comp = kc.get_component(cid, realm)
 
-            for mapper in updated_mappers:
+            for before_mapper in before_comp.get('mappers', []):
+                # remove unwanted existing mappers that will not be updated
+                if not before_mapper['id'] in [x['id'] for x in desired_mappers]:
+                    kc.delete_component(before_mapper['id'], realm)
+
+            for mapper in desired_mappers:
+                if mapper in before_comp.get('mappers', []):
+                    continue
                 if mapper.get('id') is not None:
                     kc.update_component(mapper, realm)
                 else:
                     if mapper.get('parentId') is None:
                         mapper['parentId'] = desired_comp['id']
-                    mapper = kc.create_component(mapper, realm)
-
-            after_comp['mappers'] = updated_mappers
-            result['end_state'] = sanitize(after_comp)
+                    kc.create_component(mapper, realm)
 
+            after_comp = kc.get_component(cid, realm)
+            after_comp['mappers'] = kc.get_components(urlencode(dict(parent=cid)), realm)
+            after_comp_sanitized = sanitize(after_comp)
+            before_comp_sanitized = sanitize(before_comp)
+            result['end_state'] = after_comp_sanitized
+            if module._diff:
+                result['diff'] = dict(before=before_comp_sanitized, after=after_comp_sanitized)
+            result['changed'] = before_comp_sanitized != after_comp_sanitized
             result['msg'] = "User federation {id} has been updated".format(id=cid)
             module.exit_json(**result)
 

tests/unit/plugins/modules/test_keycloak_user_federation.py

@@ -144,8 +144,9 @@ class TestKeycloakUserFederation(ModuleTestCase):
                 }
             }
         ]
+        # get before_comp, get default_mapper, get after_mapper
         return_value_components_get = [
-            [], []
+            [], [], []
         ]
         changed = True
 
@@ -159,7 +160,7 @@ class TestKeycloakUserFederation(ModuleTestCase):
                 with self.assertRaises(AnsibleExitJson) as exec_info:
                     self.module.main()
 
-        self.assertEqual(len(mock_get_components.mock_calls), 1)
+        self.assertEqual(len(mock_get_components.mock_calls), 3)
         self.assertEqual(len(mock_get_component.mock_calls), 0)
         self.assertEqual(len(mock_create_component.mock_calls), 1)
         self.assertEqual(len(mock_update_component.mock_calls), 0)
@@ -228,6 +229,7 @@ class TestKeycloakUserFederation(ModuleTestCase):
                     }
                 }
             ],
+            [],
             []
         ]
         return_value_component_get = [
@@ -281,7 +283,7 @@ class TestKeycloakUserFederation(ModuleTestCase):
                 with self.assertRaises(AnsibleExitJson) as exec_info:
                     self.module.main()
 
-        self.assertEqual(len(mock_get_components.mock_calls), 2)
+        self.assertEqual(len(mock_get_components.mock_calls), 3)
         self.assertEqual(len(mock_get_component.mock_calls), 1)
         self.assertEqual(len(mock_create_component.mock_calls), 0)
         self.assertEqual(len(mock_update_component.mock_calls), 1)
@@ -344,7 +346,47 @@ class TestKeycloakUserFederation(ModuleTestCase):
             ]
         }
         return_value_components_get = [
-            [], []
+            [],
+            # exemplary default mapper created by keylocak
+            [
+                {
+                    "config": {
+                        "always.read.value.from.ldap": "false",
+                        "is.mandatory.in.ldap": "false",
+                        "ldap.attribute": "mail",
+                        "read.only": "true",
+                        "user.model.attribute": "email"
+                    },
+                    "id": "77e1763f-c51a-4286-bade-75577d64803c",
+                    "name": "email",
+                    "parentId": "e5f48aa3-b56b-4983-a8ad-2c7b8b5e77cb",
+                    "providerId": "user-attribute-ldap-mapper",
+                    "providerType": "org.keycloak.storage.ldap.mappers.LDAPStorageMapper"
+                },
+            ],
+            [
+                {
+                    "id": "2dfadafd-8b34-495f-a98b-153e71a22311",
+                    "name": "full name",
+                    "providerId": "full-name-ldap-mapper",
+                    "providerType": "org.keycloak.storage.ldap.mappers.LDAPStorageMapper",
+                    "parentId": "eb691537-b73c-4cd8-b481-6031c26499d8",
+                    "config": {
+                        "ldap.full.name.attribute": [
+                            "cn"
+                        ],
+                        "read.only": [
+                            "true"
+                        ],
+                        "write.only": [
+                            "false"
+                        ]
+                    }
+                }
+            ]
+        ]
+        return_value_component_delete = [
+            None
         ]
         return_value_component_create = [
             {
@@ -462,11 +504,11 @@ class TestKeycloakUserFederation(ModuleTestCase):
                 with self.assertRaises(AnsibleExitJson) as exec_info:
                     self.module.main()
 
-        self.assertEqual(len(mock_get_components.mock_calls), 2)
+        self.assertEqual(len(mock_get_components.mock_calls), 3)
         self.assertEqual(len(mock_get_component.mock_calls), 0)
         self.assertEqual(len(mock_create_component.mock_calls), 2)
         self.assertEqual(len(mock_update_component.mock_calls), 0)
-        self.assertEqual(len(mock_delete_component.mock_calls), 0)
+        self.assertEqual(len(mock_delete_component.mock_calls), 1)
 
         # Verify that the module's changed status matches what is expected
         self.assertIs(exec_info.exception.args[0]['changed'], changed)