From 114bc855c00a4efd91c346a0ddb0b89aa191d26f Mon Sep 17 00:00:00 2001
From: Jakob Meng <code@jakobmeng.de>
Date: Tue, 8 Nov 2022 11:53:30 +0100
Subject: [PATCH] Refactored keystone_federation_protocol{,_info} modules

Change-Id: I6f9eef184538df4feff49dd6ed659a9b9ac1289f
---
 .../defaults/main.yml                         |   5 +
 .../tasks/main.yml                            | 168 ++++----------
 .../modules/keystone_federation_protocol.py   | 215 +++++++++---------
 .../keystone_federation_protocol_info.py      |  93 ++++----
 4 files changed, 211 insertions(+), 270 deletions(-)

diff --git a/ci/roles/keystone_federation_protocol/defaults/main.yml b/ci/roles/keystone_federation_protocol/defaults/main.yml
index d665338..2d4cf87 100644
--- a/ci/roles/keystone_federation_protocol/defaults/main.yml
+++ b/ci/roles/keystone_federation_protocol/defaults/main.yml
@@ -9,6 +9,11 @@ idp_remote_ids:
 # Minimal Domain definition
 domain_name: 'test-domain'
 
+expected_fields:
+  - id
+  - mapping_id
+  - name
+
 # Minimal Mapping definition
 mapping_name_1: 'ansible-test-mapping-1'
 mapping_name_2: 'ansible-test-mapping-2'
diff --git a/ci/roles/keystone_federation_protocol/tasks/main.yml b/ci/roles/keystone_federation_protocol/tasks/main.yml
index 3a76818..c7eb21c 100644
--- a/ci/roles/keystone_federation_protocol/tasks/main.yml
+++ b/ci/roles/keystone_federation_protocol/tasks/main.yml
@@ -28,42 +28,24 @@
       openstack.cloud.identity_domain:
         name: '{{ domain_name }}'
       register: create_domain
-    - assert:
-        that:
-        - create_domain is successful
-        - '"id" in create_domain'
-    - name: 'Store domain ID as fact'
-      set_fact:
-        domain_id: '{{ create_domain.id }}'
 
     - name: 'Create test Identity Provider'
       openstack.cloud.federation_idp:
         state: 'present'
         name: '{{ idp_name }}'
-        domain_id: '{{ domain_id }}'
-      register: create_idp
-    - assert:
-        that:
-        - create_idp is successful
+        domain_id: '{{ create_domain.id }}'
 
     - name: 'Create test mapping (1)'
       openstack.cloud.federation_mapping:
         state: 'present'
         name: '{{ mapping_name_1 }}'
         rules: '{{ mapping_rules_1 }}'
-      register: create_mapping
-    - assert:
-        that:
-        - create_mapping is successful
+
     - name: 'Create test mapping (2)'
       openstack.cloud.federation_mapping:
         state: 'present'
         name: '{{ mapping_name_2 }}'
         rules: '{{ mapping_rules_2 }}'
-      register: create_mapping
-    - assert:
-        that:
-        - create_mapping is successful
 
     # We *should* have a blank slate to start with, but we also shouldn't
     # explode if I(state=absent) and the IDP doesn't exist
@@ -71,10 +53,6 @@
       openstack.cloud.keystone_federation_protocol:
         state: 'absent'
         name: '{{ protocol_name }}'
-      register: delete_protocol
-    - assert:
-        that:
-        - delete_protocol is successful
 
     # ========================================================================
     #   Creation
@@ -86,19 +64,19 @@
         name: '{{ protocol_name }}'
         mapping_id: '{{ mapping_name_1 }}'
       register: create_protocol
+
     - assert:
         that:
-        - create_protocol is successful
         - create_protocol is changed
 
     - name: 'Fetch Protocol info (should be absent)'
       openstack.cloud.keystone_federation_protocol_info:
         name: '{{ protocol_name }}'
       register: protocol_info
-      ignore_errors: yes
+
     - assert:
         that:
-        - protocol_info is failed
+        - protocol_info.protocols | length == 0
 
     - name: 'Create protocol'
       openstack.cloud.keystone_federation_protocol:
@@ -106,21 +84,19 @@
         name: '{{ protocol_name }}'
         mapping_id: '{{ mapping_name_1 }}'
       register: create_protocol
+
     - assert:
         that:
-        - create_protocol is successful
         - create_protocol is changed
-        - '"protocol" in create_protocol'
-        - '"id" in protocol'
-        - '"name" in protocol'
-        - '"idp_id" in protocol'
-        - '"mapping_id" in protocol'
-        - protocol.id == protocol_name
-        - protocol.name == protocol_name
-        - protocol.idp_id == idp_name
-        - protocol.mapping_id == mapping_name_1
-      vars:
-        protocol: '{{ create_protocol.protocol }}'
+        - create_protocol.protocol.id == protocol_name
+        - create_protocol.protocol.name == protocol_name
+        - create_protocol.protocol.mapping_id == mapping_name_1
+
+    - name: assert return values of keystone_federation_protocol module
+      assert:
+        that:
+          # allow new fields to be introduced but prevent fields from being removed
+          - expected_fields|difference(create_protocol.protocol.keys())|length == 0
 
     - name: 'Create protocol (retry - no change) - CHECK MODE'
       check_mode: yes
@@ -129,9 +105,9 @@
         name: '{{ protocol_name }}'
         mapping_id: '{{ mapping_name_1 }}'
       register: create_protocol
+
     - assert:
         that:
-        - create_protocol is successful
         - create_protocol is not changed
 
     - name: 'Create protocol (retry - no change)'
@@ -140,21 +116,13 @@
         name: '{{ protocol_name }}'
         mapping_id: '{{ mapping_name_1 }}'
       register: create_protocol
+
     - assert:
         that:
-        - create_protocol is successful
         - create_protocol is not changed
-        - '"protocol" in create_protocol'
-        - '"id" in protocol'
-        - '"name" in protocol'
-        - '"idp_id" in protocol'
-        - '"mapping_id" in protocol'
-        - protocol.id == protocol_name
-        - protocol.name == protocol_name
-        - protocol.idp_id == idp_name
-        - protocol.mapping_id == mapping_name_1
-      vars:
-        protocol: '{{ create_protocol.protocol }}'
+        - create_protocol.protocol.id == protocol_name
+        - create_protocol.protocol.name == protocol_name
+        - create_protocol.protocol.mapping_id == mapping_name_1
 
     # ========================================================================
     #   Update
@@ -166,9 +134,9 @@
         name: '{{ protocol_name }}'
         mapping_id: '{{ mapping_name_2 }}'
       register: update_protocol
+
     - assert:
         that:
-        - update_protocol is successful
         - update_protocol is changed
 
     - name: 'Update protocol'
@@ -177,21 +145,13 @@
         name: '{{ protocol_name }}'
         mapping_id: '{{ mapping_name_2 }}'
       register: update_protocol
+
     - assert:
         that:
-        - update_protocol is successful
         - update_protocol is changed
-        - '"protocol" in update_protocol'
-        - '"id" in protocol'
-        - '"name" in protocol'
-        - '"idp_id" in protocol'
-        - '"mapping_id" in protocol'
-        - protocol.id == protocol_name
-        - protocol.name == protocol_name
-        - protocol.idp_id == idp_name
-        - protocol.mapping_id == mapping_name_2
-      vars:
-        protocol: '{{ update_protocol.protocol }}'
+        - update_protocol.protocol.id == protocol_name
+        - update_protocol.protocol.name == protocol_name
+        - update_protocol.protocol.mapping_id == mapping_name_2
 
     - name: 'Update protocol (retry - no change) - CHECK MODE'
       check_mode: yes
@@ -200,9 +160,9 @@
         name: '{{ protocol_name }}'
         mapping_id: '{{ mapping_name_2 }}'
       register: update_protocol
+
     - assert:
         that:
-        - update_protocol is successful
         - update_protocol is not changed
 
     - name: 'Update protocol (retry - no change)'
@@ -211,21 +171,13 @@
         name: '{{ protocol_name }}'
         mapping_id: '{{ mapping_name_2 }}'
       register: update_protocol
+
     - assert:
         that:
-        - update_protocol is successful
         - update_protocol is not changed
-        - '"protocol" in update_protocol'
-        - '"id" in protocol'
-        - '"name" in protocol'
-        - '"idp_id" in protocol'
-        - '"mapping_id" in protocol'
-        - protocol.id == protocol_name
-        - protocol.name == protocol_name
-        - protocol.idp_id == idp_name
-        - protocol.mapping_id == mapping_name_2
-      vars:
-        protocol: '{{ update_protocol.protocol }}'
+        - update_protocol.protocol.id == protocol_name
+        - update_protocol.protocol.name == protocol_name
+        - update_protocol.protocol.mapping_id == mapping_name_2
 
     # ========================================================================
     #   Create second protocol to test openstack.cloud.keystone_federation_protocol_info
@@ -238,19 +190,10 @@
       register: create_protocol_2
     - assert:
         that:
-        - create_protocol_2 is successful
         - create_protocol_2 is changed
-        - '"protocol" in create_protocol_2'
-        - '"id" in protocol'
-        - '"name" in protocol'
-        - '"idp_id" in protocol'
-        - '"mapping_id" in protocol'
-        - protocol.id == protocol_name_2
-        - protocol.name == protocol_name_2
-        - protocol.idp_id == idp_name
-        - protocol.mapping_id == mapping_name_1
-      vars:
-        protocol: '{{ create_protocol_2.protocol }}'
+        - create_protocol_2.protocol.id == protocol_name_2
+        - create_protocol_2.protocol.name == protocol_name_2
+        - create_protocol_2.protocol.mapping_id == mapping_name_1
 
     # ========================================================================
     #   Basic tests of openstack.cloud.keystone_federation_protocol_info
@@ -259,49 +202,36 @@
       openstack.cloud.keystone_federation_protocol_info:
         name: '{{ protocol_name }}'
       register: protocol_info
+
+    - name: Check info about protocols
+      assert:
+        that:
+          - protocol_info.protocols|length > 0
+          # allow new fields to be introduced but prevent fields from being removed
+          - expected_fields|difference(protocol_info.protocols[0].keys())|length == 0
+
     - assert:
         that:
-        - protocol_info is successful
-        - '"protocols" in protocol_info'
-        - protocol_info.protocols | length == 1
-        - '"id" in protocol'
-        - '"name" in protocol'
-        - '"idp_id" in protocol'
-        - '"mapping_id" in protocol'
-        - protocol.id == protocol_name
-        - protocol.name == protocol_name
-        - protocol.idp_id == idp_name
-        - protocol.mapping_id == mapping_name_2
-      vars:
-        protocol: '{{ protocol_info.protocols[0] }}'
+        - protocol_info.protocols[0].id == protocol_name
+        - protocol_info.protocols[0].name == protocol_name
+        - protocol_info.protocols[0].mapping_id == mapping_name_2
 
     - name: 'Fetch Protocol info (all protocols on our test IDP)'
       openstack.cloud.keystone_federation_protocol_info: {}
         # idp_id defined in defaults at the start
       register: protocol_info
+
     - assert:
         that:
-        - protocol_info is successful
-        - '"protocols" in protocol_info'
         # We created the IDP, and we're going to delete it:
         # we should be able to trust what's attached to it
         - protocol_info.protocols | length == 2
-        - '"id" in protocol_1'
-        - '"name" in protocol_1'
-        - '"idp_id" in protocol_1'
-        - '"mapping_id" in protocol_1'
-        - '"id" in protocol_2'
-        - '"name" in protocol_2'
-        - '"idp_id" in protocol_2'
-        - '"mapping_id" in protocol_2'
         - protocol_name in (protocol_info.protocols | map(attribute='id'))
         - protocol_name in (protocol_info.protocols | map(attribute='id'))
         - protocol_name_2 in (protocol_info.protocols | map(attribute='name'))
         - protocol_name_2 in (protocol_info.protocols | map(attribute='name'))
         - mapping_name_1 in (protocol_info.protocols | map(attribute='mapping_id'))
         - mapping_name_2 in (protocol_info.protocols | map(attribute='mapping_id'))
-        - protocol_1.idp_id == idp_name
-        - protocol_2.idp_id == idp_name
       vars:
         protocol_1: '{{ protocol_info.protocols[0] }}'
         protocol_2: '{{ protocol_info.protocols[1] }}'
@@ -315,9 +245,9 @@
         state: 'absent'
         name: '{{ protocol_name }}'
       register: update_protocol
+
     - assert:
         that:
-        - update_protocol is successful
         - update_protocol is changed
 
     - name: 'Delete protocol'
@@ -325,9 +255,9 @@
         state: 'absent'
         name: '{{ protocol_name }}'
       register: update_protocol
+
     - assert:
         that:
-        - update_protocol is successful
         - update_protocol is changed
 
     - name: 'Delete protocol (retry - no change) - CHECK MODE'
@@ -336,9 +266,9 @@
         state: 'absent'
         name: '{{ protocol_name }}'
       register: update_protocol
+
     - assert:
         that:
-        - update_protocol is successful
         - update_protocol is not changed
 
     - name: 'Delete protocol (retry - no change)'
@@ -346,9 +276,9 @@
         state: 'absent'
         name: '{{ protocol_name }}'
       register: update_protocol
+
     - assert:
         that:
-        - update_protocol is successful
         - update_protocol is not changed
 
     # ========================================================================
diff --git a/plugins/modules/keystone_federation_protocol.py b/plugins/modules/keystone_federation_protocol.py
index f6ce704..2d6aae7 100644
--- a/plugins/modules/keystone_federation_protocol.py
+++ b/plugins/modules/keystone_federation_protocol.py
@@ -4,62 +4,85 @@
 # Copyright: Ansible Project
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
-DOCUMENTATION = '''
+DOCUMENTATION = r'''
 ---
 module: keystone_federation_protocol
-short_description: manage a federation Protocol
+short_description: Manage a Keystone federation protocol
 author: OpenStack Ansible SIG
 description:
-  - Manage a federation Protocol.
+  - Manage a Keystone federation protocol.
 options:
   name:
     description:
-      - The name of the Protocol.
+      - ID or name of the federation protocol.
+      - This attribute cannot be updated.
     type: str
     required: true
     aliases: ['id']
-  state:
+  idp:
     description:
-      - Whether the protocol should be C(present) or C(absent).
-    choices: ['present', 'absent']
-    default: present
+      - ID or name of the identity provider this protocol is associated with.
+      - This attribute cannot be updated.
+    aliases: ['idp_id', 'idp_name']
+    required: true
     type: str
-  idp_id:
+  mapping:
     description:
-      - The name of the Identity Provider this Protocol is associated with.
-    aliases: ['idp_name']
-    required: true
+      - ID or name of the mapping to use for this protocol.
+      - Required when creating a new protocol.
     type: str
-  mapping_id:
+    aliases: ['mapping_id', 'mapping_name']
+  state:
     description:
-      - The name of the Mapping to use for this Protocol.'
-      - Required when creating a new Protocol.
+      - Whether the protocol should be C(present) or C(absent).
+    choices: ['present', 'absent']
+    default: present
     type: str
-    aliases: ['mapping_name']
+notes:
+    - Name equals the ID of a federation protocol.
+    - Name equals the ID of an identity provider.
+    - Name equals the ID of a mapping.
 requirements:
   - "python >= 3.6"
-  - "openstacksdk >= 0.44"
+  - "openstacksdk"
 extends_documentation_fragment:
   - openstack.cloud.openstack
 '''
 
-EXAMPLES = '''
+EXAMPLES = r'''
 - name: Create a protocol
   openstack.cloud.keystone_federation_protocol:
     cloud: example_cloud
     name: example_protocol
-    idp_id: example_idp
-    mapping_id: example_mapping
+    idp: example_idp
+    mapping: example_mapping
 
 - name: Delete a protocol
   openstack.cloud.keystone_federation_protocol:
     cloud: example_cloud
     name: example_protocol
-    idp_id: example_idp
+    idp: example_idp
     state: absent
 '''
 
-RETURN = '''
+RETURN = r'''
+protocol:
+    description: Dictionary describing the federation protocol.
+    returned: always
+    type: dict
+    contains:
+        id:
+            description: ID of the federation protocol.
+            returned: success
+            type: str
+        mapping_id:
+            description: The definition of the federation protocol.
+            returned: success
+            type: str
+        name:
+            description: Name of the protocol. Equal to C(id).
+            returned: success
+            type: str
 '''
 
 from ansible_collections.openstack.cloud.plugins.module_utils.openstack import OpenStackModule
@@ -69,115 +92,91 @@ class IdentityFederationProtocolModule(OpenStackModule):
     argument_spec = dict(
         name=dict(required=True, aliases=['id']),
         state=dict(default='present', choices=['absent', 'present']),
-        idp_id=dict(required=True, aliases=['idp_name']),
-        mapping_id=dict(aliases=['mapping_name']),
+        idp=dict(required=True, aliases=['idp_id', 'idp_name']),
+        mapping=dict(aliases=['mapping_id', 'mapping_name']),
     )
     module_kwargs = dict(
+        required_if=[
+            ('state', 'present', ('mapping',)),
+        ],
         supports_check_mode=True
     )
 
-    def normalize_protocol(self, protocol):
-        """
-        Normalizes the protocol definitions so that the outputs are consistent with the
-        parameters
-
-        - "name" (parameter) == "id" (SDK)
-        """
-        if protocol is None:
-            return None
-
-        _protocol = protocol.to_dict()
-        _protocol['name'] = protocol['id']
-        # As of 0.44 SDK doesn't copy the URI parameters over, so let's add them
-        _protocol['idp_id'] = protocol['idp_id']
-        return _protocol
-
-    def delete_protocol(self, protocol):
-        """
-        Delete an existing Protocol
-
-        returns: the "Changed" state
-        """
-        if protocol is None:
-            return False
-
-        if self.ansible.check_mode:
-            return True
-
-        self.conn.identity.delete_federation_protocol(None, protocol)
-        return True
+    def run(self):
+        state = self.params['state']
 
-    def create_protocol(self, name):
-        """
-        Create a new Protocol
+        id = self.params['name']
+        idp_id = self.params['idp']
+        protocol = self.conn.identity.find_federation_protocol(idp_id, id)
 
-        returns: the "Changed" state and the new protocol
-        """
         if self.ansible.check_mode:
-            return True, None
+            self.exit_json(changed=self._will_change(state, protocol))
 
-        idp_name = self.params.get('idp_id')
-        mapping_id = self.params.get('mapping_id')
+        if state == 'present' and not protocol:
+            # Create protocol
+            protocol = self._create()
+            self.exit_json(changed=True,
+                           protocol=protocol.to_dict(computed=False))
 
-        attributes = {
-            'idp_id': idp_name,
-            'mapping_id': mapping_id,
-        }
+        elif state == 'present' and protocol:
+            # Update protocol
+            update = self._build_update(protocol)
+            if update:
+                protocol = self._update(protocol, update)
 
-        protocol = self.conn.identity.create_federation_protocol(id=name, **attributes)
-        return (True, protocol)
+            self.exit_json(changed=bool(update),
+                           protocol=protocol.to_dict(computed=False))
 
-    def update_protocol(self, protocol):
-        """
-        Update an existing Protocol
+        elif state == 'absent' and protocol:
+            # Delete protocol
+            self._delete(protocol)
+            self.exit_json(changed=True)
 
-        returns: the "Changed" state and the new protocol
-        """
-        mapping_id = self.params.get('mapping_id')
+        elif state == 'absent' and not protocol:
+            # Do nothing
+            self.exit_json(changed=False)
 
-        attributes = {}
+    def _build_update(self, protocol):
+        update = {}
 
-        if (mapping_id is not None) and (mapping_id != protocol.mapping_id):
-            attributes['mapping_id'] = mapping_id
+        attributes = dict(
+            (k, self.params[p])
+            for (p, k) in {'mapping': 'mapping_id'}.items()
+            if p in self.params and self.params[p] is not None
+            and self.params[p] != protocol[k])
 
-        if not attributes:
-            return False, protocol
+        if attributes:
+            update['attributes'] = attributes
 
-        if self.ansible.check_mode:
-            return True, None
+        return update
 
-        new_protocol = self.conn.identity.update_federation_protocol(None, protocol, **attributes)
-        return (True, new_protocol)
+    def _create(self):
+        return self.conn.identity.create_federation_protocol(
+            id=self.params['name'],
+            idp_id=self.params['idp'],
+            mapping_id=self.params['mapping'])
 
-    def run(self):
-        """ Module entry point """
-        name = self.params.get('name')
-        state = self.params.get('state')
-        idp = self.params.get('idp_id')
-        changed = False
+    def _delete(self, protocol):
+        self.conn.identity.delete_federation_protocol(None, protocol)
 
-        protocol = self.conn.identity.find_federation_protocol(idp, name)
+    def _update(self, protocol, update):
+        attributes = update.get('attributes')
+        if attributes:
+            protocol = self.conn.identity.update_federation_protocol(
+                protocol.idp_id, protocol.id, **attributes)
 
-        if state == 'absent':
-            if protocol is not None:
-                changed = self.delete_protocol(protocol)
-            self.exit_json(changed=changed)
+        return protocol
 
-        # state == 'present'
+    def _will_change(self, state, protocol):
+        if state == 'present' and not protocol:
+            return True
+        elif state == 'present' and protocol:
+            return bool(self._build_update(protocol))
+        elif state == 'absent' and protocol:
+            return True
         else:
-            if protocol is None:
-                if self.params.get('mapping_id') is None:
-                    self.fail_json(
-                        msg='A mapping_id must be passed when creating'
-                        ' a protocol')
-                (changed, protocol) = self.create_protocol(name)
-                protocol = self.normalize_protocol(protocol)
-                self.exit_json(changed=changed, protocol=protocol)
-
-            else:
-                (changed, new_protocol) = self.update_protocol(protocol)
-                new_protocol = self.normalize_protocol(new_protocol)
-                self.exit_json(changed=changed, protocol=new_protocol)
+            # state == 'absent' and not protocol:
+            return False
 
 
 def main():
diff --git a/plugins/modules/keystone_federation_protocol_info.py b/plugins/modules/keystone_federation_protocol_info.py
index 855aa06..d02c197 100644
--- a/plugins/modules/keystone_federation_protocol_info.py
+++ b/plugins/modules/keystone_federation_protocol_info.py
@@ -4,47 +4,67 @@
 # Copyright: Ansible Project
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
-DOCUMENTATION = '''
+DOCUMENTATION = r'''
 ---
 module: keystone_federation_protocol_info
-short_description: get information about federation Protocols
+short_description: Fetch Keystone federation protocols
 author: OpenStack Ansible SIG
 description:
-  - Get information about federation Protocols.
+  - Fetch Keystone federation protocols.
 options:
   name:
     description:
-      - The name of the Protocol.
+      - ID or name of the federation protocol.
     type: str
     aliases: ['id']
-  idp_id:
+  idp:
     description:
-      - The name of the Identity Provider this Protocol is associated with.
-    aliases: ['idp_name']
+      - ID or name of the identity provider this protocol is associated with.
+    aliases: ['idp_id', 'idp_name']
     required: true
     type: str
+notes:
+    - Name equals the ID of a federation protocol.
+    - Name equals the ID of an identity provider.
 requirements:
   - "python >= 3.6"
-  - "openstacksdk >= 0.44"
+  - "openstacksdk"
 extends_documentation_fragment:
   - openstack.cloud.openstack
 '''
 
-EXAMPLES = '''
-- name: Describe a protocol
+EXAMPLES = r'''
+- name: Fetch all federation protocols attached to an identity provider
   openstack.cloud.keystone_federation_protocol_info:
     cloud: example_cloud
-    name: example_protocol
-    idp_id: example_idp
-    mapping_name: example_mapping
+    idp: example_idp
 
-- name: Describe all protocols attached to an IDP
+- name: Fetch federation protocol by name
   openstack.cloud.keystone_federation_protocol_info:
     cloud: example_cloud
-    idp_id: example_idp
+    idp: example_idp
+    name: example_protocol
 '''
 
-RETURN = '''
+RETURN = r'''
+protocols:
+    description: List of federation protocol dictionaries.
+    returned: always
+    type: list
+    elements: dict
+    contains:
+        id:
+            description: ID of the federation protocol.
+            returned: success
+            type: str
+        mapping_id:
+            description: The definition of the federation protocol.
+            returned: success
+            type: str
+        name:
+            description: Name of the protocol. Equal to C(id).
+            returned: success
+            type: str
 '''
 
 from ansible_collections.openstack.cloud.plugins.module_utils.openstack import OpenStackModule
@@ -53,42 +73,29 @@ from ansible_collections.openstack.cloud.plugins.module_utils.openstack import O
 class IdentityFederationProtocolInfoModule(OpenStackModule):
     argument_spec = dict(
         name=dict(aliases=['id']),
-        idp_id=dict(required=True, aliases=['idp_name']),
+        idp=dict(required=True, aliases=['idp_id', 'idp_name']),
     )
+
     module_kwargs = dict(
         supports_check_mode=True
     )
 
-    def normalize_protocol(self, protocol):
-        """
-        Normalizes the protocol definitions so that the outputs are consistent with the
-        parameters
-
-        - "name" (parameter) == "id" (SDK)
-        """
-        if protocol is None:
-            return None
-
-        _protocol = protocol.to_dict()
-        _protocol['name'] = protocol['id']
-        # As of 0.44 SDK doesn't copy the URI parameters over, so let's add them
-        _protocol['idp_id'] = protocol['idp_id']
-        return _protocol
-
     def run(self):
-        """ Module entry point """
+        # name is id for federation protocols
+        id = self.params['name']
 
-        name = self.params.get('name')
-        idp = self.params.get('idp_id')
-
-        if name:
-            protocol = self.conn.identity.get_federation_protocol(idp, name)
-            protocol = self.normalize_protocol(protocol)
-            self.exit_json(changed=False, protocols=[protocol])
+        # name is id for identity providers
+        idp_id = self.params['idp']
 
+        if id:
+            protocol = self.conn.identity.find_federation_protocol(idp_id, id)
+            protocols = [protocol] if protocol else []
         else:
-            protocols = list(map(self.normalize_protocol, self.conn.identity.federation_protocols(idp)))
-            self.exit_json(changed=False, protocols=protocols)
+            protocols = self.conn.identity.federation_protocols(idp_id)
+
+        self.exit_json(changed=False,
+                       protocols=[p.to_dict(computed=False)
+                                  for p in protocols])
 
 
 def main():
-- 
GitLab