Keycloak: Improve diff mode on keycloak_authentication module (#2963) (#3000)

* Fix diff mode when updating authentication flow with keycloak_authentication module

* Add changelog fragment

* Fix unit test

* Update plugins/modules/identity/keycloak/keycloak_authentication.py

Co-authored-by: Ajpantuso <ajpantuso@gmail.com>

* Update changelogs/fragments/2963-improve-diff-mode-on-keycloak_authentication.yml

Co-authored-by: Ajpantuso <ajpantuso@gmail.com>

* Update documentation of create_or_update_executions function (return tuple instead of dict)

* Update plugins/modules/identity/keycloak/keycloak_authentication.py

Co-authored-by: Ajpantuso <ajpantuso@gmail.com>

* Update plugins/modules/identity/keycloak/keycloak_authentication.py

Co-authored-by: Ajpantuso <ajpantuso@gmail.com>

Co-authored-by: Ajpantuso <ajpantuso@gmail.com>
(cherry picked from commit 3fc97bf80a)

Co-authored-by: Gaetan2907 <48204380+Gaetan2907@users.noreply.github.com>

changelogs/fragments/2963-improve-diff-mode-on-keycloak_authentication.yml

@@ -0,0 +1,3 @@
+---
+minor_changes:
+- keycloak_authentication - enhanced diff mode to also return before and after state when the authentication flow is updated (https://github.com/ansible-collections/community.general/pull/2963).

plugins/modules/identity/keycloak/keycloak_authentication.py

@@ -196,9 +196,15 @@ def create_or_update_executions(kc, config, realm='master'):
     :param config: Representation of the authentication flow including it's executions.
     :param realm: Realm
     :return: True if executions have been modified. False otherwise.
+    :return: tuple (changed, dict(before, after)
+        WHERE
+        bool changed indicates if changes have been made
+        dict(str, str) shows state before and after creation/update
     """
     try:
         changed = False
+        after = ""
+        before = ""
         if "authenticationExecutions" in config:
             # Get existing executions on the Keycloak server for this alias
             existing_executions = kc.get_executions_representation(config, realm=realm)
@@ -221,17 +227,21 @@ def create_or_update_executions(kc, config, realm='master'):
                             exclude_key.append(key)
                     # Compare the executions to see if it need changes
                     if not is_struct_included(new_exec, existing_executions[exec_index], exclude_key) or exec_index != new_exec_index:
-                        changed = True
+                        exec_found = True
+                        before += str(existing_executions[exec_index]) + '\n'
                     id_to_update = existing_executions[exec_index]["id"]
                     # Remove exec from list in case 2 exec with same name
                     existing_executions[exec_index].clear()
                 elif new_exec["providerId"] is not None:
                     kc.create_execution(new_exec, flowAlias=flow_alias_parent, realm=realm)
-                    changed = True
+                    exec_found = True
+                    after += str(new_exec) + '\n'
                 elif new_exec["displayName"] is not None:
                     kc.create_subflow(new_exec["displayName"], flow_alias_parent, realm=realm)
+                    exec_found = True
+                    after += str(new_exec) + '\n'
+                if exec_found:
                     changed = True
-                if changed:
                     if exec_index != -1:
                         # Update the existing execution
                         updated_exec = {
@@ -248,7 +258,8 @@ def create_or_update_executions(kc, config, realm='master'):
                             kc.update_authentication_executions(flow_alias_parent, updated_exec, realm=realm)
                         diff = exec_index - new_exec_index
                         kc.change_execution_priority(updated_exec["id"], diff, realm=realm)
-        return changed
+                        after += str(kc.get_executions_representation(config, realm=realm)[new_exec_index]) + '\n'
+        return changed, dict(before=before, after=after)
     except Exception as e:
         kc.module.fail_json(msg='Could not create or update executions for authentication flow %s in realm %s: %s'
                             % (config["alias"], realm, str(e)))
@@ -358,8 +369,10 @@ def main():
             # Configure the executions for the flow
             if module.check_mode:
                 module.exit_json(**result)
-            if create_or_update_executions(kc=kc, config=new_auth_repr, realm=realm):
-                result['changed'] = True
+            changed, diff = create_or_update_executions(kc=kc, config=new_auth_repr, realm=realm)
+            result['changed'] |= changed
+            if module._diff:
+                result['diff'] = diff
             # Get executions created
             exec_repr = kc.get_executions_representation(config=new_auth_repr, realm=realm)
             if exec_repr is not None:

tests/unit/plugins/modules/identity/keycloak/test_keycloak_authentication.py

@@ -343,7 +343,7 @@ class TestKeycloakAuthentication(ModuleTestCase):
         self.assertEqual(len(mock_get_authentication_flow_by_alias.mock_calls), 1)
         self.assertEqual(len(mock_copy_auth_flow.mock_calls), 0)
         self.assertEqual(len(mock_create_empty_auth_flow.mock_calls), 1)
-        self.assertEqual(len(mock_get_executions_representation.mock_calls), 2)
+        self.assertEqual(len(mock_get_executions_representation.mock_calls), 3)
         self.assertEqual(len(mock_delete_authentication_flow_by_id.mock_calls), 0)
 
         # Verify that the module's changed status matches what is expected
@@ -434,7 +434,7 @@ class TestKeycloakAuthentication(ModuleTestCase):
         self.assertEqual(len(mock_get_authentication_flow_by_alias.mock_calls), 1)
         self.assertEqual(len(mock_copy_auth_flow.mock_calls), 0)
         self.assertEqual(len(mock_create_empty_auth_flow.mock_calls), 0)
-        self.assertEqual(len(mock_get_executions_representation.mock_calls), 2)
+        self.assertEqual(len(mock_get_executions_representation.mock_calls), 3)
         self.assertEqual(len(mock_delete_authentication_flow_by_id.mock_calls), 0)
 
         # Verify that the module's changed status matches what is expected
@@ -611,7 +611,7 @@ class TestKeycloakAuthentication(ModuleTestCase):
         self.assertEqual(len(mock_get_authentication_flow_by_alias.mock_calls), 1)
         self.assertEqual(len(mock_copy_auth_flow.mock_calls), 0)
         self.assertEqual(len(mock_create_empty_auth_flow.mock_calls), 1)
-        self.assertEqual(len(mock_get_executions_representation.mock_calls), 2)
+        self.assertEqual(len(mock_get_executions_representation.mock_calls), 3)
         self.assertEqual(len(mock_delete_authentication_flow_by_id.mock_calls), 1)
 
         # Verify that the module's changed status matches what is expected