From e380da46acb74e89c9f7ea45b6ba8b10ad10dcec Mon Sep 17 00:00:00 2001 From: Radu-Ionut Mocanu Date: Thu, 12 Sep 2024 16:27:54 +0300 Subject: [PATCH] =?UTF-8?q?feat:=20return=20diff=20for=20present=20and=20u?= =?UTF-8?q?pdate,=20return=20object=20state=20after=20u=E2=80=A6=20(#210)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: return diff for present and update, return object state after update in check mode * feat: return user object in check_mode in present, state, add tests * fix: get_users_by_identifier was ignoring the depth argument * test: fix tests, added more tests, updated example in docs * fix: check case when existing object is none in present state * [no ci] doc: added note in checkmode docs * [no ci] update module version * [no ci] updated changelog --- docs/changelog.md | 11 + docs/usage/check_mode_and_diff.md | 15 +- galaxy.yml | 2 +- plugins/module_utils/common_ionos_methods.py | 2 +- plugins/module_utils/common_ionos_module.py | 160 +++++++++---- plugins/modules/user.py | 45 ++-- tests/user-management/user-test.yml | 240 ++++++++++++++++++- 7 files changed, 397 insertions(+), 78 deletions(-) diff --git a/docs/changelog.md b/docs/changelog.md index adff2266..c0bc1126 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -1,5 +1,16 @@ # Changelog +## 7.5.0 +### Features +* Added check_mode and diff to the user module +* Added ignored_properties to the user module +### Changes +* use filters when retrieving user to speed up the request +### Fixes +* mark RegEx pattern as a raw string (fix by @jaudriga) +### Docs +* Added declarative modules page to summary + ## 7.5.0-beta.1 ### Features * Added check_mode and diff to the user module diff --git a/docs/usage/check_mode_and_diff.md b/docs/usage/check_mode_and_diff.md index 0d1c049c..e6acbf37 100644 --- a/docs/usage/check_mode_and_diff.md +++ b/docs/usage/check_mode_and_diff.md @@ -7,6 +7,7 @@ Read bellow for info on how the IONOS Ansible module handles check mode and diff ### Check Mode When using check_mode the playbook will not make changes in the API and a message will be returned if such changes would have been made. Example: "user would be updated.". The returned state is "changed". For operations that do not cause changes the regular response will be returned. +> **_NOTE:_** Please note that the value returned when using check mode is not exactly the same as what is returned by the server and even though check mode is passing without error, errors may occur when making the api calls. ### Diff When using diff an additional property will be returned on the object showing the object states with before and after. These states only include the attributes which are checked by Ansible for update and recreate. @@ -59,6 +60,18 @@ Output: } }, "failed": false, - "msg": "User would be updated" + "msg": "User would be updated", + "user": { + "id": "", + "properties": { + "administrator": false, + "email": "", + "firstname": "John", + "force_sec_auth": false, + "groups": "", + "lastname": "Doe", + "user_password": "user password will be updated" + } + } } } diff --git a/galaxy.yml b/galaxy.yml index 92e86335..e24358e5 100644 --- a/galaxy.yml +++ b/galaxy.yml @@ -5,7 +5,7 @@ namespace: ionoscloudsdk name: ionoscloud # The version of the collection. -version: 7.5.0-beta.1 +version: 7.5.0 readme: README.md diff --git a/plugins/module_utils/common_ionos_methods.py b/plugins/module_utils/common_ionos_methods.py index 3644bad2..83b3cb1f 100644 --- a/plugins/module_utils/common_ionos_methods.py +++ b/plugins/module_utils/common_ionos_methods.py @@ -71,7 +71,7 @@ def get_users_by_identifier(api, all_users, identifier_value, depth=2): # check if identifier_value is a UUID then try to get the user try: uuid.UUID(identifier_value) - user = api.um_users_find_by_id(identifier_value, depth=2) + user = api.um_users_find_by_id(identifier_value, depth=depth) all_users.items += [user] except Exception as e: pass diff --git a/plugins/module_utils/common_ionos_module.py b/plugins/module_utils/common_ionos_module.py index 1dad361c..b59f37fe 100644 --- a/plugins/module_utils/common_ionos_module.py +++ b/plugins/module_utils/common_ionos_module.py @@ -36,15 +36,29 @@ def _should_update_object(self, existing_object, clients): """ pass - def calculate_object_diff(self, existing_object, clients): + + def get_object_before(self, existing_object, clients): + """ + Return a dict with the 'before' state for the object + + existing_object : Ionoscloud object returned by API object + clients: authenticated ionoscloud clients list. + + Returns: + dict, a dict with the object properties before the task is executed + """ + pass + + + def get_object_after(self, existing_object, clients): """ - Calculate before and after for the object, only used in diff mode + Return a dict with the 'after' state for the object existing_object : Ionoscloud object returned by API object clients: authenticated ionoscloud clients list. Returns: - dict, a dict with 2 keys: 'before' and 'after' which compares only the attributes watched by ansible in their states + dict, a dict with the object properties after the task is executed """ pass @@ -82,9 +96,15 @@ def _get_object_identifier(self): def update_replace_object(self, existing_object, clients): module = self.module obj_identifier = self._get_object_identifier() if self._get_object_identifier() is not None else self._get_object_name() - module_diff = {} + object_after = self.get_object_after(existing_object, clients) + + returned_json = {} + if module._diff: - module_diff = self.calculate_object_diff(existing_object, clients) + returned_json['diff'] = { + 'before': self.get_object_before(existing_object, clients), + 'after': object_after, + } if self._should_replace_object(existing_object, clients): @@ -93,49 +113,67 @@ def update_replace_object(self, existing_object, clients): if module.check_mode: return { - 'changed': True, - 'msg': '{object_name} {object_name_identifier} would be recreated'.format( - object_name=self.object_name, object_name_identifier=obj_identifier, - ), - 'diff': module_diff, + **returned_json, + **{ + 'changed': True, + 'msg': '{object_name} {object_name_identifier} would be recreated'.format( + object_name=self.object_name, object_name_identifier=obj_identifier, + ), + self.returned_key: { + 'id': existing_object.id, + 'properties': object_after, + }, + }, } new_object = self._create_object(existing_object, clients).to_dict() self._remove_object(existing_object, clients) return { - 'changed': True, - 'failed': False, - 'action': 'create', - 'diff': module_diff, - self.returned_key: new_object, + **returned_json, + **{ + 'changed': True, + 'failed': False, + 'action': 'create', + self.returned_key: new_object, + }, } if self._should_update_object(existing_object, clients): if module.check_mode: return { - 'changed': True, - 'msg': '{object_name} {object_name_identifier} would be updated'.format( - object_name=self.object_name, object_name_identifier=obj_identifier, - ), - 'diff': module_diff, + **returned_json, + **{ + 'changed': True, + 'msg': '{object_name} {object_name_identifier} would be updated'.format( + object_name=self.object_name, object_name_identifier=obj_identifier, + ), + self.returned_key: { + 'id': existing_object.id, + 'properties': object_after, + }, + }, } # Update return { - 'changed': True, - 'failed': False, - 'action': 'update', - 'diff': module_diff, - self.returned_key: self._update_object(existing_object, clients).to_dict() + **returned_json, + **{ + 'changed': True, + 'failed': False, + 'action': 'update', + self.returned_key: self._update_object(existing_object, clients).to_dict(), + }, } # No action return { - 'changed': False, - 'failed': False, - 'action': 'create', - 'diff': module_diff, - self.returned_key: existing_object.to_dict() + **returned_json, + **{ + 'changed': False, + 'failed': False, + 'action': 'create', + self.returned_key: existing_object.to_dict(), + }, } @@ -148,19 +186,37 @@ def present_object(self, clients): if existing_object: return self.update_replace_object(existing_object, clients) + returned_json = {} + object_after = self.get_object_after(existing_object, clients) + if self.module._diff: + returned_json['diff'] = { + 'before': {}, + 'after': object_after, + } + if self.module.check_mode: return { - 'skipped': True, - 'msg': '{object_name} {object_name_identifier} would be created'.format( - object_name=self.object_name, object_name_identifier=self._get_object_name(), - ) + **returned_json, + **{ + 'changed': True, + 'msg': '{object_name} {object_name_identifier} would be created'.format( + object_name=self.object_name, object_name_identifier=self._get_object_name(), + ), + self.returned_key: { + 'id': '', + 'properties': object_after, + }, + }, } return { - 'changed': True, - 'failed': False, - 'action': 'create', - self.returned_key: self._create_object(None, clients).to_dict() + **returned_json, + **{ + 'changed': True, + 'failed': False, + 'action': 'create', + self.returned_key: self._create_object(None, clients).to_dict(), + }, } @@ -208,20 +264,34 @@ def absent_object(self, clients): self.module.exit_json(changed=False) return + returned_json = {} + + if self.module._diff: + returned_json['diff'] = { + 'before': self.get_object_before(existing_object, clients), + 'after': {}, + } + if self.module.check_mode: return { - 'skipped': True, - 'msg': '{object_name} {object_name_identifier} would be deleted'.format( - object_name=self.object_name, object_name_identifier=self._get_object_identifier(), - ) + **returned_json, + **{ + 'changed': True, + 'msg': '{object_name} {object_name_identifier} would be deleted'.format( + object_name=self.object_name, object_name_identifier=self._get_object_identifier(), + ), + }, } self._remove_object(existing_object, clients) return { - 'action': 'delete', - 'changed': True, - 'id': existing_object.id, + **returned_json, + **{ + 'action': 'delete', + 'changed': True, + 'id': existing_object.id, + }, } diff --git a/plugins/modules/user.py b/plugins/modules/user.py index c2354692..bd733915 100644 --- a/plugins/modules/user.py +++ b/plugins/modules/user.py @@ -323,26 +323,33 @@ def _should_update_object(self, existing_object, clients): and 'groups' not in ignored_properties ) - def calculate_object_diff(self, existing_object, clients): + + def get_object_before(self, existing_object, clients): + return { + 'lastname': existing_object.properties.lastname, + 'firstname': existing_object.properties.firstname, + 'email': existing_object.properties.email, + 'administrator': existing_object.properties.administrator, + 'force_sec_auth': existing_object.properties.force_sec_auth, + 'user_password': '', + 'groups': '', + } + + + def get_object_after(self, existing_object, clients): + try: + object_properties = existing_object.properties + except AttributeError: + object_properties = ionoscloud.UserProperties() + return { - 'before': { - 'lastname': existing_object.properties.lastname, - 'firstname': existing_object.properties.firstname, - 'email': existing_object.properties.email, - 'administrator': existing_object.properties.administrator, - 'force_sec_auth': existing_object.properties.force_sec_auth, - 'user_password': '', - 'groups': '', - }, - 'after': { - 'lastname': existing_object.properties.lastname if self.module.params.get('lastname') is None else self.module.params.get('lastname'), - 'firstname': existing_object.properties.firstname if self.module.params.get('firstname') is None else self.module.params.get('firstname'), - 'email': existing_object.properties.email if self.module.params.get('email') is None else self.module.params.get('email'), - 'administrator': existing_object.properties.administrator if self.module.params.get('administrator') is None else self.module.params.get('administrator'), - 'force_sec_auth': existing_object.properties.force_sec_auth if self.module.params.get('force_sec_auth') is None else self.module.params.get('force_sec_auth'), - 'user_password': '' if self.module.params.get('user_password') is None else 'user password will be updated', - 'groups': '' if self.module.params.get('groups') is None else 'user groups will be updated', - } + 'lastname': object_properties.lastname if self.module.params.get('lastname') is None else self.module.params.get('lastname'), + 'firstname': object_properties.firstname if self.module.params.get('firstname') is None else self.module.params.get('firstname'), + 'email': object_properties.email if self.module.params.get('email') is None else self.module.params.get('email'), + 'administrator': object_properties.administrator if self.module.params.get('administrator') is None else self.module.params.get('administrator'), + 'force_sec_auth': object_properties.force_sec_auth if self.module.params.get('force_sec_auth') is None else self.module.params.get('force_sec_auth'), + 'user_password': '' if self.module.params.get('user_password') is None else 'user password will be updated', + 'groups': '' if self.module.params.get('groups') is None else 'user groups will be updated', } diff --git a/tests/user-management/user-test.yml b/tests/user-management/user-test.yml index 92e44940..f4ae769a 100644 --- a/tests/user-management/user-test.yml +++ b/tests/user-management/user-test.yml @@ -10,7 +10,7 @@ random_user: "no-reply{{100000000 |random}}@example.com" run_once: yes - - name: Create user + - name: Create user check 1 ionoscloudsdk.ionoscloud.user: firstname: John lastname: Doe @@ -26,8 +26,50 @@ - name: Asserting that check_mode and diff work correctly assert: that: - - user_response.msg == "User {{ random_user }} would be created" + - user_response.changed == True + - user_response.msg == 'User ' + random_user + ' would be created' + - user_response.diff.before == {} + - user_response.diff.after.email == random_user + - user_response.diff.after.firstname == 'John' + - user_response.diff.after.lastname == 'Doe' + - user_response.diff.after.groups == '' + - user_response.diff.after.user_password == 'user password will be updated' + - user_response.user.id == '' + - user_response.user.properties.email == random_user + - user_response.user.properties.firstname == 'John' + - user_response.user.properties.lastname == 'Doe' + - user_response.user.properties.administrator == False + - user_response.user.properties.force_sec_auth == False + - user_response.user.properties.groups == '' + - user_response.user.properties.user_password == 'user password will be updated' + msg: "check_mode and diff don't work correctly" + + - name: Create user check 2 + ionoscloudsdk.ionoscloud.user: + firstname: John + lastname: Doe + email: "{{ random_user }}" + administrator: false + user_password: "{{ password }}" + force_sec_auth: false + state: present + check_mode: true + register: user_response + + - name: Asserting that check_mode and diff work correctly + assert: + that: + - user_response.changed == True + - user_response.msg == 'User ' + random_user + ' would be created' - user_response.diff is not defined + - user_response.user.id == '' + - user_response.user.properties.email == random_user + - user_response.user.properties.firstname == 'John' + - user_response.user.properties.lastname == 'Doe' + - user_response.user.properties.administrator == False + - user_response.user.properties.force_sec_auth == False + - user_response.user.properties.groups == '' + - user_response.user.properties.user_password == 'user password will be updated' msg: "check_mode and diff don't work correctly" - name: Create user @@ -39,6 +81,28 @@ user_password: "{{ password }}" force_sec_auth: false state: present + register: user_response_created + diff: true + + - name: Asserting that check_mode and diff work correctly + assert: + that: + - user_response_created.changed == True + - user_response_created.msg is not defined + - user_response_created.diff.before == {} + - user_response_created.diff.after.email == random_user + - user_response_created.diff.after.firstname == 'John' + - user_response_created.diff.after.lastname == 'Doe' + - user_response_created.diff.after.groups == '' + - user_response_created.diff.after.user_password == 'user password will be updated' + - user_response_created.user.properties.email == random_user + - user_response_created.user.properties.firstname == 'John' + - user_response_created.user.properties.lastname == 'Doe' + - user_response_created.user.properties.administrator == False + - user_response_created.user.properties.force_sec_auth == False + - user_response_created.user.properties.groups is not defined + - user_response_created.user.properties.user_password is not defined + msg: "check_mode and diff don't work correctly" - name: Create user ionoscloudsdk.ionoscloud.user: @@ -47,7 +111,7 @@ email: "{{ random_user }}" administrator: false user_password: "{{ password }}" - force_sec_auth: false + force_sec_auth: true state: present check_mode: true diff: false @@ -56,8 +120,17 @@ - name: Asserting that check_mode and diff work correctly assert: that: - - user_response.msg == "User {{ random_user }} would be updated" - - user_response.diff == {} + - user_response.changed == True + - user_response.msg == 'User ' + random_user + ' would be updated' + - user_response.diff is not defined + - user_response.user.id == user_response_created.user.id + - user_response.user.properties.email == random_user + - user_response.user.properties.firstname == 'John' + - user_response.user.properties.lastname == 'Doe' + - user_response.user.properties.groups == '' + - user_response.user.properties.administrator == False + - user_response.user.properties.force_sec_auth == True + - user_response.user.properties.user_password == 'user password will be updated' msg: "check_mode and diff don't work correctly" - name: Create user @@ -76,15 +149,23 @@ - name: Asserting that check_mode and diff work correctly assert: that: - - user_response.msg == "User {{ random_user }} would be updated" + - user_response.changed == True + - user_response.msg == 'User ' + random_user + ' would be updated' - user_response.diff.before.administrator == user_response.diff.after.administrator == false - - user_response.diff.before.email == user_response.diff.after.email == "{{ random_user }}" + - user_response.diff.before.email == user_response.diff.after.email == random_user - user_response.diff.before.firstname == user_response.diff.after.firstname == 'John' - user_response.diff.before.lastname == user_response.diff.after.lastname == 'Doe' - user_response.diff.before.groups == user_response.diff.after.groups == '' - - user_response.diff.before.user_password != user_response.diff.after.user_password - user_response.diff.before.user_password == '' - user_response.diff.after.user_password == 'user password will be updated' + - user_response.user.id == user_response_created.user.id + - user_response.user.properties.email == random_user + - user_response.user.properties.firstname == 'John' + - user_response.user.properties.lastname == 'Doe' + - user_response.user.properties.groups == '' + - user_response.user.properties.administrator == False + - user_response.user.properties.force_sec_auth == False + - user_response.user.properties.user_password == 'user password will be updated' msg: "check_mode and diff don't work correctly" - name: Create user @@ -103,10 +184,11 @@ - name: Asserting that check_mode and diff work correctly 2 assert: that: - - user_response.msg == "User {{ random_user }} would be updated" + - user_response.changed == True + - user_response.msg == "User " + random_user + " would be updated" - user_response.diff.before.administrator == false - user_response.diff.after.administrator == true - - user_response.diff.before.email == user_response.diff.after.email == "{{ random_user }}" + - user_response.diff.before.email == user_response.diff.after.email == random_user - user_response.diff.before.firstname == 'John' - user_response.diff.after.firstname == 'John changed' - user_response.diff.before.lastname == 'Doe' @@ -211,6 +293,78 @@ ionoscloudsdk.ionoscloud.group: name: "{{ name }} 2" + - name: Update user check 1 + ionoscloudsdk.ionoscloud.user: + lastname: Doe new + user: "{{ random_user }}" + force_sec_auth: true + groups: + - "{{ name }} 1" + - "{{ name }} 2" + state: update + register: user_response + check_mode: true + + - name: Asserting that check_mode and diff work correctly + assert: + that: + - user_response.changed == True + - user_response.msg == 'User ' + random_user + ' would be updated' + - user_response.diff is not defined + - user_response.user.id == user_response_created.user.id + - user_response.user.properties.email == random_user + - user_response.user.properties.firstname == 'John Changed' + - user_response.user.properties.lastname == 'Doe new' + - user_response.user.properties.administrator == False + - user_response.user.properties.force_sec_auth == True + - user_response.user.properties.groups == 'user groups will be updated' + - user_response.user.properties.user_password == '' + msg: "check_mode and diff don't work correctly" + + - name: Update user check 2 + ionoscloudsdk.ionoscloud.user: + firstname: John new + lastname: Doe + user: "{{ random_user }}" + administrator: true + force_sec_auth: true + groups: + - "{{ name }} 1" + - "{{ name }} 2" + state: update + register: user_response + diff: true + check_mode: true + + - name: Asserting that check_mode and diff work correctly + assert: + that: + - user_response.changed == True + - user_response.msg == 'User ' + random_user + ' would be updated' + - user_response.diff.before.email == random_user + - user_response.diff.before.firstname == 'John Changed' + - user_response.diff.before.lastname == 'Doe Changed' + - user_response.diff.before.administrator == False + - user_response.diff.before.force_sec_auth == False + - user_response.diff.before.groups == '' + - user_response.diff.before.user_password == '' + - user_response.diff.after.email == random_user + - user_response.diff.after.firstname == 'John new' + - user_response.diff.after.lastname == 'Doe' + - user_response.diff.after.administrator == True + - user_response.diff.after.force_sec_auth == True + - user_response.diff.after.groups == 'user groups will be updated' + - user_response.diff.after.user_password == '' + - user_response.user.id == user_response_created.user.id + - user_response.user.properties.email == random_user + - user_response.user.properties.firstname == 'John new' + - user_response.user.properties.lastname == 'Doe' + - user_response.user.properties.administrator == True + - user_response.user.properties.force_sec_auth == True + - user_response.user.properties.groups == 'user groups will be updated' + - user_response.user.properties.user_password == '' + msg: "check_mode and diff don't work correctly" + - name: Update user ionoscloudsdk.ionoscloud.user: firstname: John @@ -223,6 +377,35 @@ - "{{ name }} 2" state: update register: user_response + diff: true + + - name: Asserting that check_mode and diff work correctly + assert: + that: + - user_response.changed == True + - user_response.msg is not defined + - user_response.diff.before.email == random_user + - user_response.diff.before.firstname == 'John Changed' + - user_response.diff.before.lastname == 'Doe Changed' + - user_response.diff.before.administrator == False + - user_response.diff.before.force_sec_auth == False + - user_response.diff.before.groups == '' + - user_response.diff.before.user_password == '' + - user_response.diff.after.email == random_user + - user_response.diff.after.firstname == 'John' + - user_response.diff.after.lastname == 'Doe' + - user_response.diff.after.administrator == False + - user_response.diff.after.force_sec_auth == False + - user_response.diff.after.groups == 'user groups will be updated' + - user_response.diff.after.user_password == '' + - user_response.user.properties.email == random_user + - user_response.user.properties.firstname == 'John' + - user_response.user.properties.lastname == 'Doe' + - user_response.user.properties.administrator == False + - user_response.user.properties.force_sec_auth == False + - user_response.user.properties.groups is not defined + - user_response.user.properties.user_password is not defined + msg: "check_mode and diff don't work correctly" - name: Debug - show response debug: @@ -283,19 +466,54 @@ - name: Asserting that check_mode and diff work correctly assert: that: + - user_response.changed == True - user_response.msg == "User {{ random_user }} would be deleted" - - user_response.diff is not defined + - user_response.diff.before.administrator == false + - user_response.diff.before.administrator == false + - user_response.diff.before.email == random_user + - user_response.diff.before.firstname == 'John' + - user_response.diff.before.lastname == 'Doe' + - user_response.diff.before.groups == '' + - user_response.diff.before.user_password == '' + - user_response.diff.after == {} msg: "check_mode and diff don't work correctly" - name: Delete user ionoscloudsdk.ionoscloud.user: user: "{{ random_user }}" state: absent + diff: true + register: user_response + + - name: Asserting that check_mode and diff work correctly + assert: + that: + - user_response.changed == True + - user_response.msg is not defined + - user_response.diff.before.administrator == false + - user_response.diff.before.administrator == false + - user_response.diff.before.email == random_user + - user_response.diff.before.firstname == 'John' + - user_response.diff.before.lastname == 'Doe' + - user_response.diff.before.groups == '' + - user_response.diff.before.user_password == '' + - user_response.diff.after == {} + msg: "check_mode and diff don't work correctly" - name: Delete user ionoscloudsdk.ionoscloud.user: user: "non-existent-user" state: absent + diff: true + register: user_response + + - name: Asserting that check_mode and diff work correctly + assert: + that: + - user_response.changed == False + - user_response.msg is not defined + - user_response.diff is not defined + msg: "check_mode and diff don't work correctly" - name: Delete group ionoscloudsdk.ionoscloud.group: