Refactor ARN validation code (#1619) (#1622)

SUMMARY Adds resource_id and resource_type to parse_aws_arn() return value. Adds validate_aws_arn() to handle common pattern matching for ARNs. ISSUE TYPE Feature Pull Request COMPONENT NAME ec2_instance iam_user ADDITIONAL INFORMATION Related to ansible-collections/community.aws#1846 - We've been doing things like assuming the aws partition. Reviewed-by: Alina Buzachis (cherry picked from commit 344dbd1)
ansible-collections · Jun 27, 2023 · a16d0f7 · a16d0f7
1 parent de4eefe
commit a16d0f7
Show file tree

Hide file tree

Showing 7 changed files with 437 additions and 25 deletions.
diff --git a/changelogs/fragments/1846-arn-validation.yml b/changelogs/fragments/1846-arn-validation.yml
@@ -0,0 +1,5 @@
+minor_changes:
+- ec2_instance - refactored ARN validation handling (https://github.com/ansible-collections/amazon.aws/pull/1619).
+- iam_user - refactored ARN validation handling (https://github.com/ansible-collections/amazon.aws/pull/1619).
+- module_utils.arn - added ``validate_aws_arn`` function to handle common pattern matching for ARNs (https://github.com/ansible-collections/amazon.aws/pull/1619).
+- module_utils.arn - add ``resource_id`` and ``resource_type`` to ``parse_aws_arn`` return values (https://github.com/ansible-collections/amazon.aws/pull/1619).
diff --git a/plugins/module_utils/arn.py b/plugins/module_utils/arn.py
@@ -6,14 +6,46 @@
 import re
 
 
+def validate_aws_arn(
+    arn, partition=None, service=None, region=None, account_id=None, resource=None, resource_type=None, resource_id=None
+):
+    details = parse_aws_arn(arn)
+
+    if not details:
+        return False
+
+    if partition and details.get("partition") != partition:
+        return False
+    if service and details.get("service") != service:
+        return False
+    if region and details.get("region") != region:
+        return False
+    if account_id and details.get("account_id") != account_id:
+        return False
+    if resource and details.get("resource") != resource:
+        return False
+    if resource_type and details.get("resource_type") != resource_type:
+        return False
+    if resource_id and details.get("resource_id") != resource_id:
+        return False
+
+    return True
+
+
 def parse_aws_arn(arn):
     """
+    Based on https://docs.aws.amazon.com/IAM/latest/UserGuide/reference-arns.html
+
     The following are the general formats for ARNs.
         arn:partition:service:region:account-id:resource-id
         arn:partition:service:region:account-id:resource-type/resource-id
         arn:partition:service:region:account-id:resource-type:resource-id
     The specific formats depend on the resource.
     The ARNs for some resources omit the Region, the account ID, or both the Region and the account ID.
+
+    Note: resource_type handling is very naive, for complex cases it may be necessary to use
+    "resource" directly instead of resource_type, this will include the resource type and full ID,
+    including all paths.
     """
     m = re.search(r"arn:(aws(-([a-z\-]+))?):([\w-]+):([a-z0-9\-]*):(\d*|aws|aws-managed):(.*)", arn)
     if m is None:
@@ -25,6 +57,12 @@ def parse_aws_arn(arn):
     result.update(dict(account_id=m.group(6)))
     result.update(dict(resource=m.group(7)))
 
+    m2 = re.search(r"^(.*?)[:/](.+)$", m.group(7))
+    if m2 is None:
+        result.update(dict(resource_type=None, resource_id=m.group(7)))
+    else:
+        result.update(dict(resource_type=m2.group(1), resource_id=m2.group(2)))
+
     return result
 
 

diff --git a/plugins/modules/ec2_instance.py b/plugins/modules/ec2_instance.py
@@ -975,7 +975,7 @@
 from ansible.module_utils.common.dict_transformations import snake_dict_to_camel_dict
 from ansible.module_utils.six import string_types
 
-from ansible_collections.amazon.aws.plugins.module_utils.arn import parse_aws_arn
+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.botocore import is_boto3_error_message
 from ansible_collections.amazon.aws.plugins.module_utils.ec2 import ensure_ec2_tags
@@ -1791,8 +1791,7 @@ def pretty_instance(i):
 
 
 def determine_iam_role(name_or_arn):
-    result = parse_aws_arn(name_or_arn)
-    if result and result["service"] == "iam" and result["resource"].startswith("instance-profile/"):
+    if validate_aws_arn(name_or_arn, service="iam", resource_type="instance-profile"):
         return name_or_arn
     iam = module.client("iam", retry_decorator=AWSRetry.jittered_backoff())
     try:

diff --git a/plugins/modules/iam_user.py b/plugins/modules/iam_user.py
@@ -182,8 +182,9 @@
 
 from ansible.module_utils.common.dict_transformations import camel_dict_to_snake_dict
 
-from ansible_collections.amazon.aws.plugins.module_utils.modules import AnsibleAWSModule
+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.modules import AnsibleAWSModule
 from ansible_collections.amazon.aws.plugins.module_utils.tagging import ansible_dict_to_boto3_tag_list
 from ansible_collections.amazon.aws.plugins.module_utils.tagging import boto3_tag_list_to_ansible_dict
 from ansible_collections.amazon.aws.plugins.module_utils.tagging import compare_aws_tags
@@ -208,7 +209,7 @@ def convert_friendly_names_to_arns(connection, module, policy_names):
     # List comprehension that looks for any policy in the 'policy_names' list
     # that does not begin with 'arn'. If there aren't any, short circuit.
     # If there are, translate friendly name to the full arn
-    if not any(not policy.startswith("arn:") for policy in policy_names if policy is not None):
+    if all(validate_aws_arn(policy, service="iam") for policy in policy_names if policy is not None):
         return policy_names
     allpolicies = {}
     paginator = connection.get_paginator("list_policies")
@@ -218,7 +219,7 @@ def convert_friendly_names_to_arns(connection, module, policy_names):
         allpolicies[policy["PolicyName"]] = policy["Arn"]
         allpolicies[policy["Arn"]] = policy["Arn"]
     try:
-        return [allpolicies[policy] for policy in policy_names]
+        return [allpolicies[policy] for policy in policy_names if policy is not None]
     except KeyError as e:
         module.fail_json(msg="Couldn't find policy: " + str(e))
 

diff --git a/tests/unit/module_utils/arn/test_parse_aws_arn.py b/tests/unit/module_utils/arn/test_parse_aws_arn.py
@@ -24,20 +24,26 @@
         region="us-east-1",
         account_id="123456789012",
         resource="outpost/op-1234567890abcdef0",
+        resource_type="outpost",
+        resource_id="op-1234567890abcdef0",
     ),
     dict(
         partition="aws-gov",
         service="outpost",
         region="us-gov-east-1",
         account_id="123456789012",
         resource="outpost/op-1234567890abcdef0",
+        resource_type="outpost",
+        resource_id="op-1234567890abcdef0",
     ),
     dict(
         partition="aws-cn",
         service="outpost",
         region="us-east-1",
         account_id="123456789012",
         resource="outpost/op-1234567890abcdef0",
+        resource_type="outpost",
+        resource_id="op-1234567890abcdef0",
     ),
     # Start the account ID with 0s, it's a 12 digit *string*, if someone treats
     # it as an integer the leading 0s can disappear.
@@ -47,35 +53,111 @@
         region="us-east-1",
         account_id="000123000123",
         resource="outpost/op-1234567890abcdef0",
+        resource_type="outpost",
+        resource_id="op-1234567890abcdef0",
     ),
     # S3 doesn't "need" region/account_id as bucket names are globally unique
-    dict(partition="aws", service="s3", region="", account_id="", resource="bucket/object"),
+    dict(
+        partition="aws",
+        service="s3",
+        region="",
+        account_id="",
+        resource="bucket/object",
+        resource_type="bucket",
+        resource_id="object",
+    ),
     # IAM is a 'global' service, so the ARNs don't have regions
-    dict(partition="aws", service="iam", region="", account_id="123456789012", resource="policy/foo/bar/PolicyName"),
     dict(
-        partition="aws", service="iam", region="", account_id="123456789012", resource="instance-profile/ExampleProfile"
+        partition="aws",
+        service="iam",
+        region="",
+        account_id="123456789012",
+        resource="policy/foo/bar/PolicyName",
+        resource_type="policy",
+        resource_id="foo/bar/PolicyName",
+    ),
+    dict(
+        partition="aws",
+        service="iam",
+        region="",
+        account_id="123456789012",
+        resource="instance-profile/ExampleProfile",
+        resource_type="instance-profile",
+        resource_id="ExampleProfile",
+    ),
+    dict(
+        partition="aws",
+        service="iam",
+        region="",
+        account_id="123456789012",
+        resource="root",
+        resource_type=None,
+        resource_id="root",
     ),
-    dict(partition="aws", service="iam", region="", account_id="123456789012", resource="root"),
     # Some examples with different regions
-    dict(partition="aws", service="sqs", region="eu-west-3", account_id="123456789012", resource="example-queue"),
-    dict(partition="aws", service="sqs", region="us-gov-east-1", account_id="123456789012", resource="example-queue"),
-    dict(partition="aws", service="sqs", region="sa-east-1", account_id="123456789012", resource="example-queue"),
-    dict(partition="aws", service="sqs", region="ap-northeast-2", account_id="123456789012", resource="example-queue"),
-    dict(partition="aws", service="sqs", region="ca-central-1", account_id="123456789012", resource="example-queue"),
+    dict(
+        partition="aws",
+        service="sqs",
+        region="eu-west-3",
+        account_id="123456789012",
+        resource="example-queue",
+        resource_type=None,
+        resource_id="example-queue",
+    ),
+    dict(
+        partition="aws",
+        service="sqs",
+        region="us-gov-east-1",
+        account_id="123456789012",
+        resource="example-queue",
+        resource_type=None,
+        resource_id="example-queue",
+    ),
+    dict(
+        partition="aws",
+        service="sqs",
+        region="sa-east-1",
+        account_id="123456789012",
+        resource="example-queue",
+        resource_type=None,
+        resource_id="example-queue",
+    ),
+    dict(
+        partition="aws",
+        service="sqs",
+        region="ap-northeast-2",
+        account_id="123456789012",
+        resource="example-queue",
+        resource_type=None,
+        resource_id="example-queue",
+    ),
+    dict(
+        partition="aws",
+        service="sqs",
+        region="ca-central-1",
+        account_id="123456789012",
+        resource="example-queue",
+        resource_type=None,
+        resource_id="example-queue",
+    ),
     # Some more unusual service names
     dict(
         partition="aws",
         service="network-firewall",
         region="us-east-1",
         account_id="123456789012",
         resource="stateful-rulegroup/ExampleDomainList",
+        resource_type="stateful-rulegroup",
+        resource_id="ExampleDomainList",
     ),
     dict(
         partition="aws",
         service="resource-groups",
         region="us-east-1",
         account_id="123456789012",
         resource="group/group-name",
+        resource_type="group",
+        resource_id="group-name",
     ),
     # A special case for resources AWS curate
     dict(
@@ -84,29 +166,99 @@
         region="us-east-1",
         account_id="aws-managed",
         resource="stateful-rulegroup/BotNetCommandAndControlDomainsActionOrder",
+        resource_type="stateful-rulegroup",
+        resource_id="BotNetCommandAndControlDomainsActionOrder",
+    ),
+    dict(
+        partition="aws",
+        service="iam",
+        region="",
+        account_id="aws",
+        resource="policy/AWSDirectConnectReadOnlyAccess",
+        resource_type="policy",
+        resource_id="AWSDirectConnectReadOnlyAccess",
     ),
-    dict(partition="aws", service="iam", region="", account_id="aws", resource="policy/AWSDirectConnectReadOnlyAccess"),
     # Examples merged in from test_arn.py
-    dict(partition="aws-us-gov", service="iam", region="", account_id="0123456789", resource="role/foo-role"),
-    dict(partition="aws", service="iam", region="", account_id="123456789012", resource="user/dev/*"),
-    dict(partition="aws", service="iam", region="", account_id="123456789012", resource="user:test"),
-    dict(partition="aws-cn", service="iam", region="", account_id="123456789012", resource="user:test"),
-    dict(partition="aws", service="iam", region="", account_id="123456789012", resource="user"),
-    dict(partition="aws", service="s3", region="", account_id="", resource="my_corporate_bucket/*"),
-    dict(partition="aws", service="s3", region="", account_id="", resource="my_corporate_bucket/Development/*"),
+    dict(
+        partition="aws-us-gov",
+        service="iam",
+        region="",
+        account_id="0123456789",
+        resource="role/foo-role",
+        resource_type="role",
+        resource_id="foo-role",
+    ),
+    dict(
+        partition="aws",
+        service="iam",
+        region="",
+        account_id="123456789012",
+        resource="user/dev/*",
+        resource_type="user",
+        resource_id="dev/*",
+    ),
+    dict(
+        partition="aws",
+        service="iam",
+        region="",
+        account_id="123456789012",
+        resource="user:test",
+        resource_type="user",
+        resource_id="test",
+    ),
+    dict(
+        partition="aws-cn",
+        service="iam",
+        region="",
+        account_id="123456789012",
+        resource="user:test",
+        resource_type="user",
+        resource_id="test",
+    ),
+    dict(
+        partition="aws",
+        service="iam",
+        region="",
+        account_id="123456789012",
+        resource="user",
+        resource_type=None,
+        resource_id="user",
+    ),
+    dict(
+        partition="aws",
+        service="s3",
+        region="",
+        account_id="",
+        resource="my_corporate_bucket/*",
+        resource_type="my_corporate_bucket",
+        resource_id="*",
+    ),
+    dict(
+        partition="aws",
+        service="s3",
+        region="",
+        account_id="",
+        resource="my_corporate_bucket/Development/*",
+        resource_type="my_corporate_bucket",
+        resource_id="Development/*",
+    ),
     dict(
         partition="aws",
         service="rds",
         region="es-east-1",
         account_id="000000000000",
         resource="snapshot:rds:my-db-snapshot",
+        resource_type="snapshot",
+        resource_id="rds:my-db-snapshot",
     ),
     dict(
         partition="aws",
         service="cloudformation",
         region="us-east-1",
         account_id="012345678901",
         resource="changeSet/Ansible-StackName-c6884247ede41eb0",
+        resource_type="changeSet",
+        resource_id="Ansible-StackName-c6884247ede41eb0",
     ),
 ]