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 <[email protected]> Reviewed-by: Mark Chappell Reviewed-by: Bikouo Aubin
ansible-collections · Dec 22, 2023 · b6b27bb · b6b27bb
1 parent cef7268
commit b6b27bb
Show file tree

Hide file tree

Showing 14 changed files with 324 additions and 53 deletions.
diff --git a/changelogs/fragments/20231219-iam-consistency.yml 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
@@ -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
@@ -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
@@ -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,38 +310,48 @@ 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"],
                 module.check_mode,
             )
 
         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
@@ -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: