From b6b27bb02539fca8771d040d4e61eaf628324fe7 Mon Sep 17 00:00:00 2001 From: Mark Chappell Date: Fri, 22 Dec 2023 11:58:31 +0100 Subject: [PATCH] Various IAM module cleanup (#1933) Various IAM module cleanup SUMMARY Consistently use "path" and "name" for IAM modules (with "path_prefix", "prefix" and "TYPE_name" as aliases) Consistently use "path_prefix" and "name" for IAM info modules (with "path", "prefix" and "TYPE_name" as aliases) Consistently test "path" and "name" for validity Spit out warning when we would update path, but it's not supported by the APIs (could become an error at a later date) Do not set "/" as the explicit default for paths ISSUE TYPE Feature Pull Request COMPONENT NAME plugins/module_utils/iam.py plugins/modules/iam_group.py plugins/modules/iam_instance_profile.py plugins/modules/iam_managed_policy.py plugins/modules/iam_role.py plugins/modules/iam_role_info.py plugins/modules/iam_user.py plugins/modules/iam_user_info.py ADDITIONAL INFORMATION Reviewed-by: Mandar Kulkarni Reviewed-by: Mark Chappell Reviewed-by: Bikouo Aubin --- .../fragments/20231219-iam-consistency.yml | 34 ++++++++ plugins/module_utils/iam.py | 40 +++++++++ plugins/modules/iam_group.py | 13 ++- plugins/modules/iam_instance_profile.py | 83 +++++++++++++------ plugins/modules/iam_managed_policy.py | 27 ++++-- plugins/modules/iam_role.py | 42 +++++++--- plugins/modules/iam_role_info.py | 4 +- plugins/modules/iam_user.py | 13 ++- plugins/modules/iam_user_info.py | 17 +++- .../iam_instance_profile/tasks/main.yml | 13 +++ .../iam_role/tasks/creation_deletion.yml | 11 +++ .../targets/iam_user/tasks/main.yml | 2 +- .../iam/test_validate_iam_identifiers.py | 78 +++++++++++++++++ ...test_iam.py => test_get_aws_account_id.py} | 0 14 files changed, 324 insertions(+), 53 deletions(-) create mode 100644 changelogs/fragments/20231219-iam-consistency.yml create mode 100644 tests/unit/module_utils/iam/test_validate_iam_identifiers.py rename tests/unit/module_utils/{test_iam.py => test_get_aws_account_id.py} (100%) diff --git a/changelogs/fragments/20231219-iam-consistency.yml b/changelogs/fragments/20231219-iam-consistency.yml new file mode 100644 index 00000000000..9c1b2e65968 --- /dev/null +++ b/changelogs/fragments/20231219-iam-consistency.yml @@ -0,0 +1,34 @@ +--- +minor_changes: + - iam_group - ``group_name`` has been added as an alias to ``name`` for consistency with other IAM modules (https://github.com/ansible-collections/amazon.aws/pull/1933). + - iam_group - Basic testing of ``name`` and ``path`` has been added to improve error messages (https://github.com/ansible-collections/amazon.aws/pull/1933). + + - iam_instance_profile - the ``prefix`` parameter has been renamed ``path`` for consistency with other IAM modules, ``prefix`` remains as an alias. + No change to playbooks is required (https://github.com/ansible-collections/amazon.aws/pull/1933). + - iam_instance_profile - Basic testing of ``name`` and ``path`` has been added to improve error messages (https://github.com/ansible-collections/amazon.aws/pull/1933). + - iam_instance_profile - the default value for ``path`` has been removed. New instances will still be created with a default path of ``/``. + No change to playbooks is required (https://github.com/ansible-collections/amazon.aws/pull/1933). + - iam_instance_profile - attempting to change the ``path`` for an existing profile will now generate a warning, previously this was silently ignored (https://github.com/ansible-collections/amazon.aws/pull/1933). + - iam_instance_profile - Basic testing of ``name`` and ``path`` has been added to improve error messages (https://github.com/ansible-collections/amazon.aws/pull/1933). + + - iam_managed_policy - the ``policy_name`` parameter has been renamed ``name`` for consistency with other IAM modules, ``policy_name`` remains as an alias. + No change to playbooks is required (https://github.com/ansible-collections/amazon.aws/pull/1933). + - iam_managed_policy - the ``policy_description`` parameter has been renamed ``description`` for consistency with other IAM modules, ``policy_description`` remains as an alias. + No change to playbooks is required (https://github.com/ansible-collections/amazon.aws/pull/1933). + - iam_managed_policy - Basic testing of ``name`` and ``path`` has been added to improve error messages (https://github.com/ansible-collections/amazon.aws/pull/1933). + + - iam_role_info - ``path`` and ``prefix`` have been added as aliases to ``path_prefix`` for consistency with other IAM modules (https://github.com/ansible-collections/amazon.aws/pull/1933). + + - iam_role - ``role_name`` has been added as an alias to ``name`` for consistency with other IAM modules (https://github.com/ansible-collections/amazon.aws/pull/1933). + - iam_role - ``prefix`` and ``path_prefix`` have been added as aliases to ``path`` for consistency with other IAM modules (https://github.com/ansible-collections/amazon.aws/pull/1933). + - iam_role - the default value for ``path`` has been removed. New roles will still be created with a default path of ``/``. + No change to playbooks is required (https://github.com/ansible-collections/amazon.aws/pull/1933). + - iam_role - attempting to change the ``path`` for an existing profile will now generate a warning, previously this was silently ignored (https://github.com/ansible-collections/amazon.aws/pull/1933). + - iam_role - Basic testing of ``name`` and ``path`` has been added to improve error messages (https://github.com/ansible-collections/amazon.aws/pull/1933). + + - iam_user_info - the ``path`` parameter has been renamed ``path_prefix`` for consistency with other IAM modules, ``path`` remains as an alias. + No change to playbooks is required (https://github.com/ansible-collections/amazon.aws/pull/1933). + - iam_user_info - ``prefix`` has been added as an alias to ``path_prefix`` for consistency with other IAM modules (https://github.com/ansible-collections/amazon.aws/pull/1933). + + - iam_user - ``user_name`` has been added as an alias to ``name`` for consistency with other IAM modules (https://github.com/ansible-collections/amazon.aws/pull/1933). + - iam_user - Basic testing of ``name`` and ``path`` has been added to improve error messages (https://github.com/ansible-collections/amazon.aws/pull/1933). diff --git a/plugins/module_utils/iam.py b/plugins/module_utils/iam.py index 1c4e7cd9644..787588b43df 100644 --- a/plugins/module_utils/iam.py +++ b/plugins/module_utils/iam.py @@ -3,6 +3,7 @@ # Copyright (c) 2017 Ansible Project # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) +import re from copy import deepcopy try: @@ -166,6 +167,7 @@ def get_aws_account_info(module): def create_iam_instance_profile(client, name, path, tags): boto3_tags = ansible_dict_to_boto3_tag_list(tags or {}) + path = path or "/" try: result = _create_instance_profile(client, InstanceProfileName=name, Path=path, Tags=boto3_tags) except ( @@ -309,3 +311,41 @@ def untag_iam_instance_profile(client, name, tags): botocore.exceptions.ClientError, ) as e: # pylint: disable=duplicate-except raise AnsibleIAMError(message="Unable to untag instance profile", exception=e) + + +def _validate_iam_name(resource_type, name=None): + if name is None: + return None + LENGTHS = {"role": 64, "user": 64} + regex = r"[\w+=,.@-]+" + max_length = LENGTHS.get(resource_type, 128) + if len(name) > max_length: + return f"Length of {resource_type} name may not exceed {max_length}" + if not re.fullmatch(regex, name): + return f"{resource_type} name must match pattern {regex}" + return None + + +def _validate_iam_path(resource_type, path=None): + if path is None: + return None + regex = r"\/([\w+=,.@-]+\/)*" + max_length = 512 + if len(path) > max_length: + return f"Length of {resource_type} path may not exceed {max_length}" + if not path.endswith("/") or not path.startswith("/"): + return f"{resource_type} path must begin and end with /" + if not re.fullmatch(regex, path): + return f"{resource_type} path must match pattern {regex}" + return None + + +def validate_iam_identifiers(resource_type, name=None, path=None): + name_problem = _validate_iam_name(resource_type, name) + if name_problem: + return name_problem + path_problem = _validate_iam_path(resource_type, path) + if path_problem: + return path_problem + + return None diff --git a/plugins/modules/iam_group.py b/plugins/modules/iam_group.py index 760d7d050db..dc52ea8cdf4 100644 --- a/plugins/modules/iam_group.py +++ b/plugins/modules/iam_group.py @@ -20,10 +20,12 @@ description: - The name of the group. - >- - Note: group names are unique within an account. Paths (I(path)) do B(not) affect + Note: Group names are unique within an account. Paths (I(path)) do B(not) affect the uniqueness requirements of I(name). For example it is not permitted to have both C(/Path1/MyGroup) and C(/Path2/MyGroup) in the same account. + - The alias C(group_name) was added in release 7.2.0. required: true + aliases: ['group_name'] type: str path: description: @@ -204,6 +206,7 @@ from ansible_collections.amazon.aws.plugins.module_utils.botocore import is_boto3_error_code from ansible_collections.amazon.aws.plugins.module_utils.iam import AnsibleIAMError from ansible_collections.amazon.aws.plugins.module_utils.iam import convert_managed_policy_names_to_arns +from ansible_collections.amazon.aws.plugins.module_utils.iam import validate_iam_identifiers from ansible_collections.amazon.aws.plugins.module_utils.modules import AnsibleAWSModule from ansible_collections.amazon.aws.plugins.module_utils.retries import AWSRetry @@ -444,7 +447,7 @@ def list_all_policies(connection, module): def main(): argument_spec = dict( - name=dict(required=True), + name=dict(aliases=["group_name"], required=True), path=dict(aliases=["prefix", "path_prefix"]), managed_policies=dict(default=[], type="list", aliases=["managed_policy"], elements="str"), users=dict(default=[], type="list", elements="str"), @@ -458,6 +461,12 @@ def main(): supports_check_mode=True, ) + identifier_problem = validate_iam_identifiers( + "group", name=module.params.get("name"), path=module.params.get("path") + ) + if identifier_problem: + module.fail_json(msg=identifier_problem) + connection = module.client("iam", retry_decorator=AWSRetry.jittered_backoff()) state = module.params.get("state") diff --git a/plugins/modules/iam_instance_profile.py b/plugins/modules/iam_instance_profile.py index 790fa412d60..2ae31cedec9 100644 --- a/plugins/modules/iam_instance_profile.py +++ b/plugins/modules/iam_instance_profile.py @@ -22,17 +22,24 @@ default: "present" name: description: - - Name of an instance profile. - aliases: - - instance_profile_name + - Name of the instance profile. + - >- + Note: Profile names are unique within an account. Paths (I(path)) do B(not) affect + the uniqueness requirements of I(name). For example it is not permitted to have both + C(/Path1/MyProfile) and C(/Path2/MyProfile) in the same account. + aliases: ["instance_profile_name"] type: str required: True - prefix: + path: description: - - The path prefix for filtering the results. - aliases: ["path_prefix", "path"] + - The instance profile path. + - For more information about IAM paths, see the AWS IAM identifiers documentation + U(https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_identifiers.html). + - Updating the path on an existing profile is not currently supported and will result in a + warning. + - The parameter was renamed from C(prefix) to C(path) in release 7.2.0. + aliases: ["path_prefix", "prefix"] type: str - default: "/" role: description: - The name of the role to attach to the instance profile. @@ -47,19 +54,32 @@ """ EXAMPLES = r""" -- name: Find all existing IAM instance profiles - amazon.aws.iam_instance_profile_info: - register: result +- name: Create Instance Profile + amazon.aws.iam_instance_profile: + name: "ExampleInstanceProfile" + role: "/OurExamples/MyExampleRole" + path: "/OurExamples/" + tags: + ExampleTag: Example Value + register: profile_result -- name: Describe a single instance profile - amazon.aws.iam_instance_profile_info: - name: MyIAMProfile - register: result +- name: Create second Instance Profile with default path + amazon.aws.iam_instance_profile: + name: "ExampleInstanceProfile2" + role: "/OurExamples/MyExampleRole" + tags: + ExampleTag: Another Example Value + register: profile_result -- name: Find all IAM instance profiles starting with /some/path/ +- name: Find all IAM instance profiles starting with /OurExamples/ amazon.aws.iam_instance_profile_info: - prefile: /some/path/ + path_prefix: /OurExamples/ register: result + +- name: Delete second Instance Profile + amazon.aws.iam_instance_profile: + name: "ExampleInstanceProfile2" + state: absent """ RETURN = r""" @@ -116,6 +136,7 @@ from ansible_collections.amazon.aws.plugins.module_utils.iam import remove_role_from_iam_instance_profile from ansible_collections.amazon.aws.plugins.module_utils.iam import tag_iam_instance_profile from ansible_collections.amazon.aws.plugins.module_utils.iam import untag_iam_instance_profile +from ansible_collections.amazon.aws.plugins.module_utils.iam import validate_iam_identifiers from ansible_collections.amazon.aws.plugins.module_utils.modules import AnsibleAWSModule from ansible_collections.amazon.aws.plugins.module_utils.retries import AWSRetry from ansible_collections.amazon.aws.plugins.module_utils.tagging import compare_aws_tags @@ -277,7 +298,7 @@ def main(): """ argument_spec = dict( name=dict(aliases=["instance_profile_name"], required=True), - prefix=dict(aliases=["path_prefix", "path"], default="/"), + path=dict(aliases=["path_prefix", "prefix"]), state=dict(choices=["absent", "present"], default="present"), tags=dict(aliases=["resource_tags"], type="dict"), purge_tags=dict(type="bool", default=True), @@ -289,30 +310,40 @@ def main(): supports_check_mode=True, ) - client = module.client("iam", retry_decorator=AWSRetry.jittered_backoff()) + name = module.params.get("name") + state = module.params.get("state") + path = module.params.get("path") - try: - name = module.params["name"] - prefix = module.params["prefix"] - state = module.params["state"] + identifier_problem = validate_iam_identifiers("instance profile", name=name, path=path) + if identifier_problem: + module.fail_json(msg=identifier_problem) - original_profile = describe_iam_instance_profile(client, name, prefix) + client = module.client("iam", retry_decorator=AWSRetry.jittered_backoff()) + try: + original_profile = describe_iam_instance_profile(client, name, path) if state == "absent": changed = ensure_absent( original_profile, client, name, - prefix, + path, module.check_mode, ) final_profile = None else: + # As of botocore 1.34.3, the APIs don't support updating the Name or Path + if original_profile and path and original_profile.get("path") != path: + module.warn( + "iam_instance_profile doesn't support updating the path: " + f"current path '{original_profile.get('path')}', requested path '{path}'" + ) + changed, final_profile = ensure_present( original_profile, client, name, - prefix, + path, module.params["tags"], module.params["purge_tags"], module.params["role"], @@ -320,7 +351,7 @@ def main(): ) if not module.check_mode: - final_profile = describe_iam_instance_profile(client, name, prefix) + final_profile = describe_iam_instance_profile(client, name, path) except AnsibleIAMError as e: if e.exception: diff --git a/plugins/modules/iam_managed_policy.py b/plugins/modules/iam_managed_policy.py index c0654416d1f..9a6cef89516 100644 --- a/plugins/modules/iam_managed_policy.py +++ b/plugins/modules/iam_managed_policy.py @@ -13,15 +13,23 @@ description: - Allows creating and removing managed IAM policies options: - policy_name: + name: description: - The name of the managed policy. + - >- + Note: Policy names are unique within an account. Paths (I(path)) do B(not) affect + the uniqueness requirements of I(name). For example it is not permitted to have both + C(/Path1/MyPolicy) and C(/Path2/MyPolicy) in the same account. + - The parameter was renamed from C(policy_name) to C(name) in release 7.2.0. required: true type: str - policy_description: + aliases: ["policy_name"] + description: description: - A helpful description of this policy, this value is immutable and only set when creating a new policy. + - The parameter was renamed from C(policy_description) to C(description) in release 7.2.0. default: '' + aliases: ["policy_description"] type: str policy: description: @@ -134,6 +142,7 @@ from ansible.module_utils.common.dict_transformations import camel_dict_to_snake_dict from ansible_collections.amazon.aws.plugins.module_utils.botocore import is_boto3_error_code +from ansible_collections.amazon.aws.plugins.module_utils.iam import validate_iam_identifiers from ansible_collections.amazon.aws.plugins.module_utils.modules import AnsibleAWSModule from ansible_collections.amazon.aws.plugins.module_utils.policy import compare_policies from ansible_collections.amazon.aws.plugins.module_utils.retries import AWSRetry @@ -271,8 +280,8 @@ def detach_all_entities(policy, **kwargs): def create_or_update_policy(existing_policy): - name = module.params.get("policy_name") - description = module.params.get("policy_description") + name = module.params.get("name") + description = module.params.get("description") default = module.params.get("make_default") only = module.params.get("only_version") @@ -345,8 +354,8 @@ def main(): global client argument_spec = dict( - policy_name=dict(required=True), - policy_description=dict(default=""), + name=dict(required=True, aliases=["policy_name"]), + description=dict(default="", aliases=["policy_description"]), policy=dict(type="json"), make_default=dict(type="bool", default=True), only_version=dict(type="bool", default=False), @@ -359,9 +368,13 @@ def main(): supports_check_mode=True, ) - name = module.params.get("policy_name") + name = module.params.get("name") state = module.params.get("state") + identifier_problem = validate_iam_identifiers("policy", name=name) + if identifier_problem: + module.fail_json(msg=identifier_problem) + try: client = module.client("iam", retry_decorator=AWSRetry.jittered_backoff()) except (botocore.exceptions.ClientError, botocore.exceptions.BotoCoreError) as e: diff --git a/plugins/modules/iam_role.py b/plugins/modules/iam_role.py index c2625586f3a..d0b9f7f361a 100644 --- a/plugins/modules/iam_role.py +++ b/plugins/modules/iam_role.py @@ -17,14 +17,25 @@ options: path: description: - - The path to the role. For more information about paths, see U(https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_identifiers.html). - default: "/" + - The path of the role. + - For more information about IAM paths, see the AWS IAM identifiers documentation + U(https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_identifiers.html). + - Updating the path on an existing role is not currently supported and will result in a + warning. + - C(path_prefix) and C(prefix) were added as aliases in release 7.2.0. type: str + aliases: ["prefix", "path_prefix"] name: description: - - The name of the role to create. + - The name of the role. + - >- + Note: Role names are unique within an account. Paths (I(path)) do B(not) affect + the uniqueness requirements of I(name). For example it is not permitted to have both + C(/Path1/MyRole) and C(/Path2/MyRole) in the same account. + - C(role_name) was added as an alias in release 7.2.0. required: true type: str + aliases: ["role_name"] description: description: - Provides a description of the role. @@ -227,6 +238,7 @@ from ansible_collections.amazon.aws.plugins.module_utils.arn import validate_aws_arn from ansible_collections.amazon.aws.plugins.module_utils.botocore import is_boto3_error_code +from ansible_collections.amazon.aws.plugins.module_utils.iam import validate_iam_identifiers from ansible_collections.amazon.aws.plugins.module_utils.modules import AnsibleAWSModule from ansible_collections.amazon.aws.plugins.module_utils.policy import compare_policies from ansible_collections.amazon.aws.plugins.module_utils.retries import AWSRetry @@ -330,7 +342,7 @@ def remove_inline_policies(module, client, role_name): def generate_create_params(module): params = dict() - params["Path"] = module.params.get("path") + params["Path"] = module.params.get("path") or "/" params["RoleName"] = module.params.get("name") params["AssumeRolePolicyDocument"] = module.params.get("assume_role_policy_document") if module.params.get("description") is not None: @@ -503,6 +515,13 @@ def create_or_update_role(module, client): current_duration = role.get("MaxSessionDuration") current_permissions_boundary = role.get("PermissionsBoundary", {}).get("PermissionsBoundaryArn", "") + # As of botocore 1.34.3, the APIs don't support updating the Name or Path + if path is not None and role.get("Path") != path: + module.warn( + "iam_role doesn't support updating the path: " + f"current path '{role.get('Path')}', requested path '{path}'" + ) + # Update attributes changed |= update_role_tags(module, client, role_name, tags, purge_tags) changed |= update_role_assumed_policy(module, client, role_name, assumed_policy, current_assumed_policy) @@ -535,6 +554,7 @@ def create_or_update_role(module, client): def create_instance_profiles(module, client, role_name, path): + path = path or "/" # Fetch existing Profiles try: instance_profiles = client.list_instance_profiles_for_role(RoleName=role_name, aws_retry=True)[ @@ -705,8 +725,8 @@ def update_role_tags(module, client, role_name, new_tags, purge_tags): def main(): argument_spec = dict( - name=dict(type="str", required=True), - path=dict(type="str", default="/"), + name=dict(type="str", aliases=["role_name"], required=True), + path=dict(type="str", aliases=["path_prefix", "prefix"]), assume_role_policy_document=dict(type="json"), managed_policies=dict(type="list", aliases=["managed_policy"], elements="str"), max_session_duration=dict(type="int"), @@ -753,10 +773,12 @@ def main(): max_session_duration = module.params.get("max_session_duration") if max_session_duration < 3600 or max_session_duration > 43200: module.fail_json(msg="max_session_duration must be between 1 and 12 hours (3600 and 43200 seconds)") - if module.params.get("path"): - path = module.params.get("path") - if not path.endswith("/") or not path.startswith("/"): - module.fail_json(msg="path must begin and end with /") + + identifier_problem = validate_iam_identifiers( + "role", name=module.params.get("name"), path=module.params.get("path") + ) + if identifier_problem: + module.fail_json(msg=identifier_problem) client = module.client("iam", retry_decorator=AWSRetry.jittered_backoff()) diff --git a/plugins/modules/iam_role_info.py b/plugins/modules/iam_role_info.py index b56a35a3e9c..b7215941089 100644 --- a/plugins/modules/iam_role_info.py +++ b/plugins/modules/iam_role_info.py @@ -26,7 +26,9 @@ description: - Prefix of role to restrict IAM role search for. - Mutually exclusive with I(name). + - C(path) and C(prefix) were added as aliases in release 7.2.0. type: str + aliases: ["path", "prefix"] extends_documentation_fragment: - amazon.aws.common.modules - amazon.aws.region.modules @@ -261,7 +263,7 @@ def main(): """ argument_spec = dict( name=dict(aliases=["role_name"]), - path_prefix=dict(), + path_prefix=dict(aliases=["path", "prefix"]), ) module = AnsibleAWSModule( diff --git a/plugins/modules/iam_user.py b/plugins/modules/iam_user.py index 1e361698443..8ad1d842c6f 100644 --- a/plugins/modules/iam_user.py +++ b/plugins/modules/iam_user.py @@ -23,11 +23,13 @@ Note: user names are unique within an account. Paths (I(path)) do B(not) affect the uniqueness requirements of I(name). For example it is not permitted to have both C(/Path1/MyUser) and C(/Path2/MyUser) in the same account. + - C(user_name) was added as an alias in release 7.2.0. required: true type: str + aliases: ['user_name'] path: description: - - The path for the user name. + - The path for the user. - For more information about IAM paths, see the AWS IAM identifiers documentation U(https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_identifiers.html). aliases: ['prefix', 'path_prefix'] @@ -227,6 +229,7 @@ from ansible_collections.amazon.aws.plugins.module_utils.botocore import is_boto3_error_code from ansible_collections.amazon.aws.plugins.module_utils.iam import AnsibleIAMError from ansible_collections.amazon.aws.plugins.module_utils.iam import convert_managed_policy_names_to_arns +from ansible_collections.amazon.aws.plugins.module_utils.iam import validate_iam_identifiers from ansible_collections.amazon.aws.plugins.module_utils.modules import AnsibleAWSModule from ansible_collections.amazon.aws.plugins.module_utils.retries import AWSRetry from ansible_collections.amazon.aws.plugins.module_utils.tagging import ansible_dict_to_boto3_tag_list @@ -735,7 +738,7 @@ def get_user(connection, name): def main(): argument_spec = dict( - name=dict(required=True, type="str"), + name=dict(required=True, type="str", aliases=["user_name"]), path=dict(type="str", aliases=["prefix", "path_prefix"]), boundary=dict(type="str", aliases=["boundary_policy_arn", "permissions_boundary"]), password=dict(type="str", no_log=True), @@ -763,6 +766,12 @@ def main(): collection_name="amazon.aws", ) + identifier_problem = validate_iam_identifiers( + "user", name=module.params.get("name"), path=module.params.get("path") + ) + if identifier_problem: + module.fail_json(msg=identifier_problem) + retry_decorator = AWSRetry.jittered_backoff(catch_extra_error_codes=["EntityTemporarilyUnmodifiable"]) connection = module.client("iam", retry_decorator=retry_decorator) diff --git a/plugins/modules/iam_user_info.py b/plugins/modules/iam_user_info.py index 304b449862e..f1683ece7cd 100644 --- a/plugins/modules/iam_user_info.py +++ b/plugins/modules/iam_user_info.py @@ -19,20 +19,25 @@ name: description: - The name of the IAM user to look for. + - C(user_name) was added as an alias in release 7.2.0. required: false type: str + aliases: ["user_name"] group: description: - The group name name of the IAM user to look for. Mutually exclusive with C(path). + - C(group_name) was added as an alias in release 7.2.0. required: false type: str - path: + aliases: ["group_name"] + path_prefix: description: - The path to the IAM user. Mutually exclusive with C(group). - If specified, then would get all user names whose path starts with user provided value. required: false default: '/' type: str + aliases: ["path", "prefix"] extends_documentation_fragment: - amazon.aws.common.modules - amazon.aws.region.modules @@ -130,7 +135,7 @@ def describe_iam_user(user): def list_iam_users(connection, module): name = module.params.get("name") group = module.params.get("group") - path = module.params.get("path") + path = module.params.get("path_prefix") params = dict() iam_users = [] @@ -171,10 +176,14 @@ def list_iam_users(connection, module): def main(): - argument_spec = dict(name=dict(), group=dict(), path=dict(default="/")) + argument_spec = dict( + name=dict(aliases=["user_name"]), + group=dict(aliases=["group_name"]), + path_prefix=dict(aliases=["path", "prefix"], default="/"), + ) module = AnsibleAWSModule( - argument_spec=argument_spec, mutually_exclusive=[["group", "path"]], supports_check_mode=True + argument_spec=argument_spec, mutually_exclusive=[["group", "path_prefix"]], supports_check_mode=True ) connection = module.client("iam") diff --git a/tests/integration/targets/iam_instance_profile/tasks/main.yml b/tests/integration/targets/iam_instance_profile/tasks/main.yml index 7aebb3a2aca..794b7a4ae7d 100644 --- a/tests/integration/targets/iam_instance_profile/tasks/main.yml +++ b/tests/integration/targets/iam_instance_profile/tasks/main.yml @@ -260,6 +260,19 @@ # =================================================================== + - name: Update path for complex Instance Profile - no change can be made + amazon.aws.iam_instance_profile: + name: "{{ test_profile_complex }}" + role: "{{ test_role }}-2" + path: "{{ test_path }}subpath/" + register: profile_result + + - ansible.builtin.assert: + that: + - profile_result is not changed + + # =================================================================== + - name: List all Instance Profiles (no filter) amazon.aws.iam_instance_profile_info: register: profile_info diff --git a/tests/integration/targets/iam_role/tasks/creation_deletion.yml b/tests/integration/targets/iam_role/tasks/creation_deletion.yml index 5b9a8d4c3ce..9c81019c8e4 100644 --- a/tests/integration/targets/iam_role/tasks/creation_deletion.yml +++ b/tests/integration/targets/iam_role/tasks/creation_deletion.yml @@ -222,6 +222,17 @@ - iam_role is not changed - iam_role.iam_role.role_name == test_role +- name: Minimal IAM Role with updated path (no change) + community.aws.iam_role: + name: "{{ test_role }}" + path: "{{ test_path }}subpath/" + register: iam_role + +- ansible.builtin.assert: + that: + - iam_role is not changed + - iam_role.iam_role.role_name == test_role + - name: iam_role_info after Role creation community.aws.iam_role_info: name: "{{ test_role }}" diff --git a/tests/integration/targets/iam_user/tasks/main.yml b/tests/integration/targets/iam_user/tasks/main.yml index ab66d501acc..675b9a5b1be 100644 --- a/tests/integration/targets/iam_user/tasks/main.yml +++ b/tests/integration/targets/iam_user/tasks/main.yml @@ -17,7 +17,7 @@ ansible.builtin.assert: that: - iam_user_info_path_group is failed - - 'iam_user_info_path_group.msg == "parameters are mutually exclusive: group|path"' + - iam_user_info_path_group.msg.startswith("parameters are mutually exclusive") - name: Create test user (check mode) amazon.aws.iam_user: diff --git a/tests/unit/module_utils/iam/test_validate_iam_identifiers.py b/tests/unit/module_utils/iam/test_validate_iam_identifiers.py new file mode 100644 index 00000000000..66098297ff6 --- /dev/null +++ b/tests/unit/module_utils/iam/test_validate_iam_identifiers.py @@ -0,0 +1,78 @@ +import pytest + +from ansible_collections.amazon.aws.plugins.module_utils.iam import validate_iam_identifiers + +# See also: https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_iam-quotas.html +validate_test_data = [ + ( + dict(), # Input + None, # Output role + None, # Output user + None, # Output generic + ), + (dict(path="/"), None, None, None), + (dict(name="Example"), None, None, None), + # Path tests + ( + dict(path="/12345abcd"), + "path must begin and end with /", + "path must begin and end with /", + "path must begin and end with /", + ), + (dict(path="/12345abcd/"), None, None, None), + (dict(path=f"/{'12345abcd0'*51}/"), None, None, None), # Max length 512 chars + ( + dict(path=f"/{'12345abcd/'*51}a/"), + "path may not exceed 512", + "path may not exceed 512", + "path may not exceed 512", + ), + (dict(path="/12345+=,.@_-abcd/"), None, None, None), # limited allowed special characters + (dict(path="/12345&abcd/"), "path must match pattern", "path must match pattern", "path must match pattern"), + (dict(path="/12345:abcd/"), "path must match pattern", "path must match pattern", "path must match pattern"), + # Name tests + (dict(name="12345abcd"), None, None, None), + (dict(name=f"{'12345abcd0'*6}1234"), None, None, None), # Max length + (dict(name=f"{'12345abcd0'*6}12345"), "name may not exceed 64", "name may not exceed 64", None), + (dict(name=f"{'12345abcd0'*12}12345678"), "name may not exceed 64", "name may not exceed 64", None), + ( + dict(name=f"{'12345abcd0'*12}123456789"), + "name may not exceed 64", + "name may not exceed 64", + "name may not exceed 128", + ), + (dict(name="12345+=,.@_-abcd"), None, None, None), # limited allowed special characters + (dict(name="12345&abcd"), "name must match pattern", "name must match pattern", "name must match pattern"), + (dict(name="12345:abcd"), "name must match pattern", "name must match pattern", "name must match pattern"), + (dict(name="/12345/abcd/"), "name must match pattern", "name must match pattern", "name must match pattern"), + # Dual tests + (dict(path="/example/", name="Example"), None, None, None), + (dict(path="/exa:ple/", name="Example"), "path", "path", "path"), + (dict(path="/example/", name="Exa:ple"), "name", "name", "name"), +] + + +@pytest.mark.parametrize("input_params, output_role, output_user, output_generic", validate_test_data) +def test_scrub_none_parameters(input_params, output_role, output_user, output_generic): + # Role and User have additional length constraints + return_role = validate_iam_identifiers("role", **input_params) + return_user = validate_iam_identifiers("user", **input_params) + return_generic = validate_iam_identifiers("generic", **input_params) + + if output_role is None: + assert return_role is None + else: + assert return_role is not None + assert output_role in return_role + if output_user is None: + assert return_user is None + else: + assert return_user is not None + assert output_user in return_user + + # Defaults + if output_generic is None: + assert return_generic is None + else: + assert return_generic is not None + assert output_generic in return_generic diff --git a/tests/unit/module_utils/test_iam.py b/tests/unit/module_utils/test_get_aws_account_id.py similarity index 100% rename from tests/unit/module_utils/test_iam.py rename to tests/unit/module_utils/test_get_aws_account_id.py