diff --git a/changelogs/fragments/1059-iam_user-add-new-ret-val-handle-errors.yml b/changelogs/fragments/1059-iam_user-add-new-ret-val-handle-errors.yml new file mode 100644 index 00000000000..77cc115f7aa --- /dev/null +++ b/changelogs/fragments/1059-iam_user-add-new-ret-val-handle-errors.yml @@ -0,0 +1,5 @@ +minor_changes: +- iam_user - add `user` value to return data structure to deprecate old `iam_user` (https://github.com/ansible-collections/community.aws/pull/1059). +bugfixes: +- iam_user - don't delete user login profile on check mode (https://github.com/ansible-collections/community.aws/pull/1059). +- iam_user_info - gracefully handle when no users are found (https://github.com/ansible-collections/community.aws/pull/1059). diff --git a/plugins/modules/iam_user.py b/plugins/modules/iam_user.py index 6266d992003..b820c60dedb 100644 --- a/plugins/modules/iam_user.py +++ b/plugins/modules/iam_user.py @@ -161,20 +161,20 @@ user_id: description: the stable and unique string identifying the user type: str - sample: AGPAIDBWE12NSFINE55TM + sample: "AGPAIDBWE12NSFINE55TM" user_name: description: the friendly name that identifies the user type: str - sample: testuser1 + sample: "testuser1" path: description: the path to the user type: str - sample: / + sample: "/" tags: description: user tags type: dict returned: always - sample: '{"Env": "Prod"}' + sample: {"Env": "Prod"} ''' try: @@ -228,10 +228,6 @@ def convert_friendly_names_to_arns(connection, module, policy_names): def wait_iam_exists(connection, module): - if module.check_mode: - return - if not module.params.get('wait'): - return user_name = module.params.get('name') wait_timeout = module.params.get('wait_timeout') @@ -263,6 +259,7 @@ def create_or_update_login_profile(connection, module): try: retval = connection.update_login_profile(**user_params) except is_boto3_error_code('NoSuchEntity'): + # Login profile does not yet exist - create it try: retval = connection.create_login_profile(**user_params) except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: @@ -274,14 +271,26 @@ def create_or_update_login_profile(connection, module): def delete_login_profile(connection, module): - + ''' + Deletes a users login profile. + Parameters: + connection: IAM client + module: AWSModule + Returns: + (bool): True if login profile deleted, False if no login profile found to delete + ''' user_params = dict() user_params['UserName'] = module.params.get('name') - try: - connection.delete_login_profile(**user_params) - except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: - module.fail_json_aws(e, msg="Unable to delete user login profile") + # User does not have login profile - nothing to delete + if not user_has_login_profile(connection, module, user_params['UserName']): + return False + + if not module.check_mode: + try: + connection.delete_login_profile(**user_params) + except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: # pylint: disable=duplicate-except + module.fail_json_aws(e, msg="Unable to delete user login profile") return True @@ -331,6 +340,9 @@ def create_or_update_user(connection, module): update_result = update_user_tags(connection, module, params, user) if module.params['update_password'] == "always" and module.params.get('password') is not None: + # Can't compare passwords, so just return changed on check mode runs + if module.check_mode: + module.exit_json(changed=True) login_profile_result, login_profile_data = create_or_update_login_profile(connection, module) if login_profile_data.get('LoginProfile', {}).get('PasswordResetRequired', False): @@ -382,7 +394,7 @@ def create_or_update_user(connection, module): # `LoginProfile` is only returned on `create_login_profile` method user['user']['password_reset_required'] = login_profile_data.get('LoginProfile', {}).get('PasswordResetRequired', False) - module.exit_json(changed=changed, iam_user=user) + module.exit_json(changed=changed, iam_user=user, user=user['user']) def destroy_user(connection, module): @@ -412,7 +424,7 @@ def destroy_user(connection, module): connection.delete_access_key(UserName=user_name, AccessKeyId=access_key["AccessKeyId"]) # Remove user's login profile (console password) - delete_user_login_profile(connection, module, user_name) + delete_login_profile(connection, module) # Remove user's ssh public keys ssh_public_keys = connection.list_ssh_public_keys(UserName=user_name)["SSHPublicKeys"] @@ -495,6 +507,25 @@ def delete_user_login_profile(connection, module, user_name): module.fail_json_aws(e, msg="Unable to delete login profile for user {0}".format(user_name)) +def user_has_login_profile(connection, module, name): + ''' + Returns whether or not given user has a login profile. + Parameters: + connection: IAM client + module: AWSModule + name (str): Username of user + Returns: + (bool): True if user had login profile, False if not + ''' + try: + connection.get_login_profile(UserName=name) + except is_boto3_error_code('NoSuchEntity'): + return False + except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: # pylint: disable=duplicate-except + module.fail_json_aws(e, msg="Unable to get login profile for user {0}".format(name)) + return True + + def update_user_tags(connection, module, params, user): user_name = params['UserName'] existing_tags = user['user']['tags'] @@ -543,6 +574,9 @@ def main(): mutually_exclusive=[['password', 'remove_password']], ) + module.deprecate("The 'iam_user' return key is deprecated and will be replaced by 'user'. Both values are returned for now.", + date='2024-05-01', collection_name='community.aws') + connection = module.client('iam') state = module.params.get("state") diff --git a/plugins/modules/iam_user_info.py b/plugins/modules/iam_user_info.py index e8fa1ac028a..ee6224880cd 100644 --- a/plugins/modules/iam_user_info.py +++ b/plugins/modules/iam_user_info.py @@ -111,6 +111,7 @@ from ansible.module_utils.common.dict_transformations import camel_dict_to_snake_dict from ansible_collections.amazon.aws.plugins.module_utils.core import AnsibleAWSModule +from ansible_collections.amazon.aws.plugins.module_utils.core import is_boto3_error_code from ansible_collections.amazon.aws.plugins.module_utils.ec2 import AWSRetry from ansible_collections.amazon.aws.plugins.module_utils.ec2 import boto3_tag_list_to_ansible_dict @@ -142,14 +143,18 @@ def list_iam_users(connection, module): params['UserName'] = name try: iam_users.append(connection.get_user(**params)['User']) - except (ClientError, BotoCoreError) as e: + except is_boto3_error_code('NoSuchEntity'): + pass + except (ClientError, BotoCoreError) as e: # pylint: disable=duplicate-except module.fail_json_aws(e, msg="Couldn't get IAM user info for user %s" % name) if group: params['GroupName'] = group try: iam_users = list_iam_users_with_backoff(connection, 'get_group', **params)['Users'] - except (ClientError, BotoCoreError) as e: + except is_boto3_error_code('NoSuchEntity'): + pass + except (ClientError, BotoCoreError) as e: # pylint: disable=duplicate-except module.fail_json_aws(e, msg="Couldn't get IAM user info for group %s" % group) if name: iam_users = [user for user in iam_users if user['UserName'] == name] @@ -158,7 +163,9 @@ def list_iam_users(connection, module): params['PathPrefix'] = path try: iam_users = list_iam_users_with_backoff(connection, 'list_users', **params)['Users'] - except (ClientError, BotoCoreError) as e: + except is_boto3_error_code('NoSuchEntity'): + pass + except (ClientError, BotoCoreError) as e: # pylint: disable=duplicate-except module.fail_json_aws(e, msg="Couldn't get IAM user info for path %s" % path) if name: iam_users = [user for user in iam_users if user['UserName'] == name] diff --git a/tests/integration/targets/iam_user/tasks/main.yml b/tests/integration/targets/iam_user/tasks/main.yml index e62d64e3791..69623e6ff33 100644 --- a/tests/integration/targets/iam_user/tasks/main.yml +++ b/tests/integration/targets/iam_user/tasks/main.yml @@ -44,6 +44,18 @@ that: - iam_user is changed + - name: ensure test user exists (no change - check mode) + iam_user: + name: "{{ test_user }}" + state: present + register: iam_user + check_mode: yes + + - name: assert that user would not change + assert: + that: + - iam_user is not changed + - name: ensure test user exists (no change) iam_user: name: "{{ test_user }}" @@ -61,8 +73,8 @@ - assert: that: - - 'test_iam_user.arn.startswith("arn:aws:iam")' - - 'test_iam_user.arn.endswith("user/" + test_user )' + - test_iam_user.arn.startswith("arn:aws:iam") + - test_iam_user.arn.endswith("user/" + test_user ) - test_iam_user.create_date is not none - test_iam_user.path == '{{ test_path }}' - test_iam_user.user_id is not none @@ -82,8 +94,6 @@ name: "{{ test_user }}" register: iam_user_info - - debug: var=iam_user_info - - assert: that: - iam_user_info.iam_users | length == 1 @@ -94,6 +104,8 @@ - iam_user_info.iam_users[0].user_name == test_iam_user.user_name - iam_user_info.iam_users[0].tags | length == 0 + # ------------------------------------------------------------------------------------------ + - name: create test user with password (check mode) iam_user: name: "{{ test_user3 }}" @@ -113,6 +125,7 @@ password: "{{ test_password }}" password_reset_required: yes state: present + wait: False register: iam_user - name: assert that the second user is created @@ -137,8 +150,22 @@ - iam_user_info.iam_users[0].user_name == test_iam_user.user_name - iam_user_info.iam_users[0].tags | length == 0 + # ------------------------------------------------------------------------------------------ ## Test tags creation / updates - - name: "Add Tag" + - name: Add Tag (check mode) + iam_user: + name: "{{ test_user }}" + state: present + tags: + TagA: ValueA + register: iam_user + check_mode: yes + + - assert: + that: + - iam_user is changed + + - name: Add Tag iam_user: name: "{{ test_user }}" state: present @@ -154,7 +181,20 @@ - '"TagA" in iam_user.iam_user.user.tags' - iam_user.iam_user.user.tags.TagA == "ValueA" - - name: "Add Tag (no change)" + - name: Add Tag (no change - check mode) + iam_user: + name: "{{ test_user }}" + state: present + tags: + TagA: ValueA + register: iam_user + check_mode: yes + + - assert: + that: + - iam_user is not changed + + - name: Add Tag (no change) iam_user: name: "{{ test_user }}" state: present @@ -235,6 +275,19 @@ - '"TagA" in iam_user.iam_user.user.tags' - iam_user.iam_user.user.tags.TagA == "ValueA" + - name: "Change Tag (check mode)" + iam_user: + name: "{{ test_user }}" + state: present + tags: + TagA: AnotherValueA + register: iam_user + check_mode: yes + + - assert: + that: + - iam_user is changed + - name: "Change Tag" iam_user: name: "{{ test_user }}" @@ -264,7 +317,34 @@ - iam_user.iam_user.user.user_name == test_user - iam_user.iam_user.user.tags | length == 0 + - name: "Remove All Tags (no change)" + iam_user: + name: "{{ test_user }}" + state: present + tags: {} + register: iam_user + + - assert: + that: + - iam_user is not changed + - iam_user.iam_user.user.user_name == test_user + - iam_user.iam_user.user.tags | length == 0 + + # ------------------------------------------------------------------------------------------ ## Test user password update + - name: test update IAM password with on_create only (check mode) + iam_user: + name: "{{ test_user3 }}" + password: "{{ test_new_password }}" + update_password: "on_create" + state: present + register: iam_user_update + check_mode: yes + + - assert: + that: + - iam_user_update is not changed + - name: test update IAM password with on_create only iam_user: name: "{{ test_user3 }}" @@ -277,6 +357,18 @@ that: - iam_user_update is not changed + - name: update IAM password (check mode) + iam_user: + name: "{{ test_user3 }}" + password: "{{ test_new_password }}" + state: present + register: iam_user_update + check_mode: yes + + - assert: + that: + - iam_user_update is changed + # flakey, there is no waiter for login profiles # Login Profile for User ansible-user-c cannot be modified while login profile is being created. - name: update IAM password @@ -302,13 +394,13 @@ # - ServiceQuotasReadOnlyAccess # - name: attach managed policy to user (check mode) - check_mode: yes iam_user: name: "{{ test_user }}" state: present managed_policy: - arn:aws:iam::aws:policy/AWSDenyAll register: iam_user + check_mode: yes - name: assert that the user is changed assert: @@ -328,6 +420,20 @@ that: - iam_user is changed + - name: ensure managed policy is attached to user (no change - check mode) + iam_user: + name: "{{ test_user }}" + state: present + managed_policy: + - arn:aws:iam::aws:policy/AWSDenyAll + register: iam_user + check_mode: yes + + - name: assert that the user hasn't changed + assert: + that: + - iam_user is not changed + - name: ensure managed policy is attached to user (no change) iam_user: name: "{{ test_user }}" @@ -341,6 +447,8 @@ that: - iam_user is not changed + # ------------------------------------------------------------------------------------------ + - name: attach different managed policy to user (check mode) check_mode: yes iam_user: @@ -370,6 +478,21 @@ that: - iam_user is changed + - name: attach different managed policy to user (no change - check mode) + iam_user: + name: "{{ test_user }}" + state: present + managed_policy: + - arn:aws:iam::aws:policy/ServiceQuotasReadOnlyAccess + purge_policy: no + register: iam_user + check_mode: yes + + - name: assert that the user hasn't changed + assert: + that: + - iam_user is not changed + - name: Check first policy wasn't purged iam_user: name: "{{ test_user }}" @@ -415,6 +538,8 @@ that: - iam_user is not changed + # ------------------------------------------------------------------------------------------ + - name: Remove one of the managed policies - with purge (check mode) check_mode: yes iam_user: @@ -444,7 +569,7 @@ that: - iam_user is changed - - name: Check we only have the one policy attached + - name: Remove one of the managed policies - with purge (no change - check mode) iam_user: name: "{{ test_user }}" state: present @@ -452,12 +577,29 @@ - arn:aws:iam::aws:policy/ServiceQuotasReadOnlyAccess purge_policy: yes register: iam_user + check_mode: yes - - name: assert that the user changed + - name: assert that the user hasn't changed + assert: + that: + - iam_user is not changed + + - name: Remove one of the managed policies - with purge (no change) + iam_user: + name: "{{ test_user }}" + state: present + managed_policy: + - arn:aws:iam::aws:policy/ServiceQuotasReadOnlyAccess + purge_policy: yes + register: iam_user + + - name: assert that the user hasn't changed assert: that: - iam_user is not changed + # ------------------------------------------------------------------------------------------ + - name: ensure group exists iam_group: name: "{{ test_group }}" @@ -599,6 +741,26 @@ that: - not iam_group.changed + # ------------------------------------------------------------------------------------------ + + - name: Remove user with attached policy (check mode) + iam_user: + name: "{{ test_user }}" + state: absent + register: iam_user + check_mode: yes + + - name: get info on IAM user(s) after deleting in check mode + iam_user_info: + name: "{{ test_user }}" + register: iam_user_info + + - name: Assert user was not removed in check mode + assert: + that: + - iam_user.changed + - iam_user_info.iam_users | length == 1 + - name: Remove user with attached policy iam_user: name: "{{ test_user }}" @@ -607,29 +769,52 @@ - name: get info on IAM user(s) after deleting iam_user_info: - group: "{{ test_user }}" - ignore_errors: yes + name: "{{ test_user }}" register: iam_user_info - name: Assert user was removed assert: that: - iam_user.changed - - "'cannot be found' in iam_user_info.msg" + - iam_user_info.iam_users | length == 0 + + - name: Remove user with attached policy (idempotent - check mode) + iam_user: + name: "{{ test_user }}" + state: absent + register: iam_user + check_mode: yes + + - name: Assert no change + assert: + that: + - not iam_user.changed - name: Remove user with attached policy (idempotent) iam_user: name: "{{ test_user }}" state: absent - ignore_errors: yes register: iam_user - - name: Assert user was removed + - name: Assert no change assert: that: - not iam_user.changed + # ------------------------------------------------------------------------------------------ ## Test user password removal + - name: Delete IAM password (check mode) + iam_user: + name: "{{ test_user3 }}" + remove_password: yes + state: present + register: iam_user_password_removal + check_mode: yes + + - assert: + that: + - iam_user_password_removal is changed + - name: Delete IAM password iam_user: name: "{{ test_user3 }}" @@ -641,6 +826,29 @@ that: - iam_user_password_removal is changed + - name: Delete IAM password again (check mode) + iam_user: + name: "{{ test_user3 }}" + remove_password: yes + state: present + register: iam_user_password_removal + check_mode: yes + + - assert: + that: + - iam_user_password_removal is not changed + + - name: Delete IAM password again + iam_user: + name: "{{ test_user3 }}" + remove_password: yes + state: present + register: iam_user_password_removal + + - assert: + that: + - iam_user_password_removal is not changed + always: - name: remove group iam_group: diff --git a/tests/sanity/ignore-2.9.txt b/tests/sanity/ignore-2.9.txt index c9c875d60e5..d0253b5fc3f 100644 --- a/tests/sanity/ignore-2.9.txt +++ b/tests/sanity/ignore-2.9.txt @@ -3,3 +3,4 @@ plugins/modules/ec2_metric_alarm.py pylint:ansible-deprecated-no-version plugins/modules/elb_network_lb.py pylint:ansible-deprecated-no-version plugins/modules/iam_policy.py pylint:ansible-deprecated-no-version plugins/modules/iam_role.py pylint:ansible-deprecated-no-version +plugins/modules/iam_user.py pylint:ansible-deprecated-no-version